From 0d984c0c2373eab687073d5e3725f75fd6f69352 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 18:52:03 -0500 Subject: [PATCH] Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided (#433) * Create Role + Binding attaching PodSecurityPolicy to managed ServiceAccount * Regenerate mocks * Update kubebuilder annotations to include roles + bindings * Add PSP to GatewayClassConfig, create Role + Binding for each Gateway * Improve docstrings on AuthSpec type * Add changelog entry * Update pkg/apis/v1alpha1/types.go * Add unit test coverage for Role, RoleBinding + ServiceAccount builders * Improve/add docstrings for Role, RoleBinding + ServiceAccount builders * Update godocs based on code review feedback, regenerate CRDs * Improve godoc --- .changelog/433.txt | 3 + ...sul.hashicorp.com_gatewayclassconfigs.yaml | 10 +- config/rbac/role.yaml | 16 ++ .../k8s/controllers/gateway_controller.go | 2 + internal/k8s/gatewayclient/gatewayclient.go | 11 +- .../k8s/gatewayclient/mocks/gatewayclient.go | 20 +++ internal/k8s/reconciler/deployer.go | 25 ++- pkg/apis/v1alpha1/types.go | 72 ++++++++- pkg/apis/v1alpha1/types_test.go | 149 ++++++++++++++++++ 9 files changed, 296 insertions(+), 12 deletions(-) create mode 100644 .changelog/433.txt diff --git a/.changelog/433.txt b/.changelog/433.txt new file mode 100644 index 000000000..549b82a16 --- /dev/null +++ b/.changelog/433.txt @@ -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. +``` diff --git a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml index e7060d854..a457ded1d 100644 --- a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml @@ -59,12 +59,12 @@ spec: description: Consul authentication information properties: account: - description: The Kubernetes service account to authenticate - as. + description: The name of an existing Kubernetes ServiceAccount + to authenticate as. Ignored if managed is true. type: string managed: description: Whether deployments should be run with "managed" - service accounts created by the gateway controller. + Kubernetes ServiceAccounts created by the gateway controller. type: boolean method: description: The Consul auth method used for initial authentication @@ -73,6 +73,10 @@ spec: namespace: description: The Consul namespace to use for authentication. type: string + podSecurityPolicy: + description: The name of an existing Kubernetes PodSecurityPolicy + to bind to the managed ServiceAccount if managed is true. + type: string type: object ports: description: The information about Consul's ports diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e53d98556..de0ec3c85 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -238,3 +238,19 @@ rules: - get - patch - update +- apiGroups: + - policy + resources: + - podsecuritypolicies + verbs: + - use +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - get + - list + - watch diff --git a/internal/k8s/controllers/gateway_controller.go b/internal/k8s/controllers/gateway_controller.go index bf85f09a0..2cdda236c 100644 --- a/internal/k8s/controllers/gateway_controller.go +++ b/internal/k8s/controllers/gateway_controller.go @@ -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 // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. diff --git a/internal/k8s/gatewayclient/gatewayclient.go b/internal/k8s/gatewayclient/gatewayclient.go index 19be4d240..fdff9650e 100644 --- a/internal/k8s/gatewayclient/gatewayclient.go +++ b/internal/k8s/gatewayclient/gatewayclient.go @@ -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) { + 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} diff --git a/internal/k8s/gatewayclient/mocks/gatewayclient.go b/internal/k8s/gatewayclient/mocks/gatewayclient.go index e9331c532..50f3a0f21 100644 --- a/internal/k8s/gatewayclient/mocks/gatewayclient.go +++ b/internal/k8s/gatewayclient/mocks/gatewayclient.go @@ -130,6 +130,26 @@ func (mr *MockClientMockRecorder) DeploymentForGateway(ctx, gw interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeploymentForGateway", reflect.TypeOf((*MockClient)(nil).DeploymentForGateway), ctx, gw) } +// EnsureExists mocks base method. +func (m *MockClient) EnsureExists(ctx context.Context, obj client.Object, mutators ...func() error) (bool, error) { + m.ctrl.T.Helper() + varargs := []interface{}{ctx, obj} + for _, a := range mutators { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "EnsureExists", varargs...) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// EnsureExists indicates an expected call of EnsureExists. +func (mr *MockClientMockRecorder) EnsureExists(ctx, obj interface{}, mutators ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx, obj}, mutators...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureExists", reflect.TypeOf((*MockClient)(nil).EnsureExists), varargs...) +} + // EnsureFinalizer mocks base method. func (m *MockClient) EnsureFinalizer(ctx context.Context, object client.Object, finalizer string) (bool, error) { m.ctrl.T.Helper() diff --git a/internal/k8s/reconciler/deployer.go b/internal/k8s/reconciler/deployer.go index 3f8a92527..5a7a46cf6 100644 --- a/internal/k8s/reconciler/deployer.go +++ b/internal/k8s/reconciler/deployer.go @@ -4,9 +4,8 @@ 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" @@ -14,6 +13,7 @@ import ( "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 + } + + 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 diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index f2f93c4d6..fb97ab089 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -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", + 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), }, } } diff --git a/pkg/apis/v1alpha1/types_test.go b/pkg/apis/v1alpha1/types_test.go index 5485017b9..115352a40 100644 --- a/pkg/apis/v1alpha1/types_test.go +++ b/pkg/apis/v1alpha1/types_test.go @@ -3,9 +3,13 @@ package v1alpha1 import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" core "k8s.io/api/core/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" + gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/hashicorp/consul-api-gateway/internal/k8s/utils" ) func TestGatewayClassConfigDeepCopy(t *testing.T) { @@ -43,3 +47,148 @@ func TestGatewayClassConfigDeepCopy(t *testing.T) { copyConfigListObject := configList.DeepCopyObject() require.Equal(t, copyConfigList, copyConfigListObject) } + +func TestGatewayClassConfig_RoleFor(t *testing.T) { + t.Run("unmanaged auth", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{Managed: false}, + }, + }, + } + + assert.Nil(t, gcc.RoleFor(&gwv1beta1.Gateway{})) + }) + + t.Run("managed auth with no podSecurityPolicy", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{ + Managed: true, + PodSecurityPolicy: "", + }, + }, + }, + } + assert.Nil(t, gcc.RoleFor(&gwv1beta1.Gateway{})) + }) + + t.Run("managed auth with podSecurityPolicy", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{ + Managed: true, + PodSecurityPolicy: "myPodSecurityPolicy", + }, + }, + }, + } + + gw := &gwv1beta1.Gateway{ObjectMeta: meta.ObjectMeta{Name: t.Name(), Namespace: t.Name()}} + + role := gcc.RoleFor(gw) + require.NotNil(t, role) + assert.Equal(t, t.Name(), role.Name) + assert.Equal(t, t.Name(), role.Namespace) + assert.Equal(t, utils.LabelsForGateway(gw), role.Labels) + + require.Len(t, role.Rules, 1) + assert.ElementsMatch(t, []string{"policy"}, role.Rules[0].APIGroups) + assert.ElementsMatch(t, []string{"podsecuritypolicies"}, role.Rules[0].Resources) + assert.ElementsMatch(t, []string{"myPodSecurityPolicy"}, role.Rules[0].ResourceNames) + assert.ElementsMatch(t, []string{"use"}, role.Rules[0].Verbs) + }) +} + +func TestGatewayClassConfig_RoleBindingFor(t *testing.T) { + t.Run("unmanaged auth", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{Managed: false}, + }, + }, + } + + assert.Nil(t, gcc.ServiceAccountFor(&gwv1beta1.Gateway{})) + }) + + t.Run("managed auth with no podSecurityPolicy", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{ + Managed: true, + PodSecurityPolicy: "", + }, + }, + }, + } + assert.Nil(t, gcc.RoleFor(&gwv1beta1.Gateway{})) + }) + + t.Run("managed auth with podSecurityPolicy", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{ + Managed: true, + PodSecurityPolicy: "myPodSecurityPolicy", + }, + }, + }, + } + + gw := &gwv1beta1.Gateway{ObjectMeta: meta.ObjectMeta{Name: t.Name(), Namespace: t.Name()}} + + binding := gcc.RoleBindingFor(gw) + require.NotNil(t, binding) + assert.Equal(t, gw.Name, binding.Name) + assert.Equal(t, gw.Namespace, binding.Namespace) + assert.Equal(t, utils.LabelsForGateway(gw), binding.Labels) + + assert.Equal(t, "rbac.authorization.k8s.io", binding.RoleRef.APIGroup) + assert.Equal(t, "Role", binding.RoleRef.Kind) + assert.Equal(t, gw.Name, binding.RoleRef.Name) + + require.Len(t, binding.Subjects, 1) + assert.Equal(t, "ServiceAccount", binding.Subjects[0].Kind) + assert.Equal(t, gw.Name, binding.Subjects[0].Name) + assert.Equal(t, gw.Namespace, binding.Subjects[0].Namespace) + }) +} + +func TestGatewayClassConfig_ServiceAccountFor(t *testing.T) { + t.Run("unmanaged auth", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{Managed: false}, + }, + }, + } + + assert.Nil(t, gcc.ServiceAccountFor(&gwv1beta1.Gateway{})) + }) + + t.Run("managed auth", func(t *testing.T) { + gcc := &GatewayClassConfig{ + Spec: GatewayClassConfigSpec{ + ConsulSpec: ConsulSpec{ + AuthSpec: AuthSpec{Managed: true}, + }, + }, + } + + gw := &gwv1beta1.Gateway{ObjectMeta: meta.ObjectMeta{Name: t.Name(), Namespace: t.Name()}} + + sa := gcc.ServiceAccountFor(gw) + require.NotNil(t, sa) + assert.Equal(t, t.Name(), sa.Name) + assert.Equal(t, t.Name(), sa.Namespace) + assert.Equal(t, utils.LabelsForGateway(gw), sa.Labels) + }) +}