-
Notifications
You must be signed in to change notification settings - Fork 16
Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433
Changes from all commits
46cba19
59deffc
74539a8
7349560
7fc5266
82b9b31
e789c5f
d41aed4
e1b0a13
afbccd1
dfbea34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:enhancement | ||
Add optional `podSecurityPolicy` to GatewayClassConfig CRD. If set and "managed" ServiceAccounts are being used, a Role and RoleBinding are created to attach the named `PodSecurityPolicy` to the managed ServiceAccount. | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,8 @@ type GatewayReconciler struct { | |
//+kubebuilder:rbac:groups=core,resources=services,verbs=list;get;create;update;watch | ||
//+kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=list;get;create;watch | ||
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=create;update;get;list;watch | ||
//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=list;get;create;watch | ||
//+kubebuilder:rbac:groups=policy,resources=podsecuritypolicies,verbs=use | ||
Comment on lines
+50
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The RBAC generated from these annotations is only used for our own testing inside this repo. These changes reflect the changes that have already been made to the Consul Helm chart in hashicorp/consul-k8s#1672 |
||
|
||
// Reconcile is part of the main kubernetes reconciliation loop which aims to | ||
// move the current state of the cluster closer to the desired state. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,9 +78,10 @@ type Client interface { | |
CreateOrUpdateSecret(ctx context.Context, secret *core.Secret, mutators ...func() error) (bool, error) | ||
CreateOrUpdateService(ctx context.Context, service *core.Service, mutators ...func() error) (bool, error) | ||
DeleteService(ctx context.Context, service *core.Service) error | ||
EnsureExists(ctx context.Context, obj client.Object, mutators ...func() error) (bool, error) | ||
EnsureServiceAccount(ctx context.Context, owner *gwv1beta1.Gateway, serviceAccount *core.ServiceAccount) error | ||
|
||
//referencepolicy | ||
// referencepolicy | ||
GetReferenceGrantsInNamespace(ctx context.Context, namespace string) ([]gwv1alpha2.ReferenceGrant, error) | ||
} | ||
|
||
|
@@ -437,6 +438,14 @@ func (g *gatewayClient) DeleteService(ctx context.Context, service *core.Service | |
return nil | ||
} | ||
|
||
func (g *gatewayClient) EnsureExists(ctx context.Context, obj client.Object, mutators ...func() error) (bool, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a really clean implementation! |
||
op, err := controllerutil.CreateOrUpdate(ctx, g.Client, obj, multiMutatorFn(mutators)) | ||
if err != nil { | ||
return false, NewK8sError(err) | ||
} | ||
return op != controllerutil.OperationResultNone, nil | ||
} | ||
|
||
func (g *gatewayClient) EnsureServiceAccount(ctx context.Context, owner *gwv1beta1.Gateway, serviceAccount *core.ServiceAccount) error { | ||
created := &core.ServiceAccount{} | ||
key := types.NamespacedName{Name: serviceAccount.Name, Namespace: serviceAccount.Namespace} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,16 @@ import ( | |
"context" | ||
"encoding/json" | ||
"fmt" | ||
"github.com/hashicorp/consul-api-gateway/internal/consul" | ||
capi "github.com/hashicorp/consul/api" | ||
|
||
capi "github.com/hashicorp/consul/api" | ||
"github.com/hashicorp/go-hclog" | ||
apps "k8s.io/api/apps/v1" | ||
core "k8s.io/api/core/v1" | ||
meta "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/types" | ||
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" | ||
|
||
"github.com/hashicorp/consul-api-gateway/internal/consul" | ||
"github.com/hashicorp/consul-api-gateway/internal/k8s/builder" | ||
"github.com/hashicorp/consul-api-gateway/internal/k8s/gatewayclient" | ||
"github.com/hashicorp/consul-api-gateway/internal/k8s/utils" | ||
|
@@ -87,7 +87,26 @@ func (d *GatewayDeployer) ensureServiceAccount(ctx context.Context, config apigw | |
return nil | ||
} | ||
|
||
return d.client.EnsureServiceAccount(ctx, gateway, serviceAccount) | ||
if err := d.client.EnsureServiceAccount(ctx, gateway, serviceAccount); err != nil { | ||
return err | ||
} | ||
|
||
role := config.RoleFor(gateway) | ||
if role == nil { | ||
return nil | ||
} | ||
Comment on lines
+94
to
+97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following the pattern already established by the |
||
|
||
if _, err := d.client.EnsureExists(ctx, role); err != nil { | ||
return err | ||
} | ||
|
||
binding := config.RoleBindingFor(gateway) | ||
if binding == nil { | ||
return nil | ||
} | ||
|
||
_, err := d.client.EnsureExists(ctx, binding) | ||
return err | ||
} | ||
|
||
// ensureSecret makes sure there is a Secret in the same namespace as the Gateway | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package v1alpha1 | |
import ( | ||
appsv1 "k8s.io/api/apps/v1" | ||
corev1 "k8s.io/api/core/v1" | ||
rbac "k8s.io/api/rbac/v1" | ||
"k8s.io/apimachinery/pkg/api/equality" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" | ||
|
@@ -124,14 +125,16 @@ type CopyAnnotationsSpec struct { | |
} | ||
|
||
type AuthSpec struct { | ||
// Whether deployments should be run with "managed" service accounts created by the gateway controller. | ||
// Whether deployments should be run with "managed" Kubernetes ServiceAccounts created by the gateway controller. | ||
Managed bool `json:"managed,omitempty"` | ||
// The Consul auth method used for initial authentication by consul-api-gateway. | ||
Method string `json:"method,omitempty"` | ||
// The Kubernetes service account to authenticate as. | ||
// The name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if managed is true. | ||
Account string `json:"account,omitempty"` | ||
// The Consul namespace to use for authentication. | ||
Namespace string `json:"namespace,omitempty"` | ||
// The name of an existing Kubernetes PodSecurityPolicy to bind to the managed ServiceAccount if managed is true. | ||
PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` | ||
} | ||
|
||
// +kubebuilder:object:root=true | ||
|
@@ -144,17 +147,76 @@ type GatewayClassConfigList struct { | |
Items []GatewayClassConfig `json:"items"` | ||
} | ||
|
||
// ServiceAccountFor returns the service account to be created for the given gateway. | ||
// RoleFor constructs a Kubernetes Role for the specified Gateway based | ||
// on the GatewayClassConfig. If the GatewayClassConfig is configured in | ||
// such a way that does not require a Role, nil is returned. | ||
func (c *GatewayClassConfig) RoleFor(gw *gwv1beta1.Gateway) *rbac.Role { | ||
if !c.Spec.ConsulSpec.AuthSpec.Managed || c.Spec.ConsulSpec.AuthSpec.PodSecurityPolicy == "" { | ||
return nil | ||
} | ||
|
||
return &rbac.Role{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: gw.Name, | ||
Namespace: gw.Namespace, | ||
Labels: utils.LabelsForGateway(gw), | ||
}, | ||
Rules: []rbac.PolicyRule{{ | ||
APIGroups: []string{"policy"}, | ||
Resources: []string{"podsecuritypolicies"}, | ||
ResourceNames: []string{c.Spec.ConsulSpec.AuthSpec.PodSecurityPolicy}, | ||
Verbs: []string{"use"}, | ||
}}, | ||
} | ||
} | ||
|
||
// RoleBindingFor constructs a Kubernetes RoleBinding for the specified Gateway | ||
// based on the GatewayClassConfig. If the GatewayClassConfig is configured in | ||
// such a way that does not require a RoleBinding, nil is returned. | ||
func (c *GatewayClassConfig) RoleBindingFor(gw *gwv1beta1.Gateway) *rbac.RoleBinding { | ||
serviceAccount := c.ServiceAccountFor(gw) | ||
if serviceAccount == nil { | ||
return nil | ||
} | ||
|
||
role := c.RoleFor(gw) | ||
if role == nil { | ||
return nil | ||
} | ||
|
||
return &rbac.RoleBinding{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: gw.Name, | ||
Namespace: gw.Namespace, | ||
Labels: utils.LabelsForGateway(gw), | ||
}, | ||
RoleRef: rbac.RoleRef{ | ||
APIGroup: "rbac.authorization.k8s.io", | ||
Kind: "Role", | ||
Comment on lines
+194
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looked for some constants for these but had no luck finding any in the K8s SDK |
||
Name: role.Name, | ||
}, | ||
Subjects: []rbac.Subject{ | ||
{ | ||
Kind: "ServiceAccount", | ||
Name: serviceAccount.Name, | ||
Namespace: serviceAccount.Namespace, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
// ServiceAccountFor constructs a Kubernetes ServiceAccount for the specified | ||
// Gateway based on the GatewayClassConfig. If the GatewayClassConfig is configured | ||
// in such a way that does not require a ServiceAccount, nil is returned. | ||
func (c *GatewayClassConfig) ServiceAccountFor(gw *gwv1beta1.Gateway) *corev1.ServiceAccount { | ||
if !c.Spec.ConsulSpec.AuthSpec.Managed { | ||
return nil | ||
} | ||
labels := utils.LabelsForGateway(gw) | ||
return &corev1.ServiceAccount{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: gw.Name, | ||
Namespace: gw.Namespace, | ||
Labels: labels, | ||
Labels: utils.LabelsForGateway(gw), | ||
}, | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a huge fan of having this named explicitly after
podSecurityPolicy
since it's already going away w/ K8s 1.25; however, the replacement for it is different enough that I don't think it makes sense to try and munge the concepts together into one field. Thoughts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually like that it's so explicit. That will make it easier when we remove PSPs in a few years. What is our support policy for K8s versions?