From 46cba19d3a81a3a0aa107271da6faa47cd5c549e Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 1 Nov 2022 18:24:29 -0400 Subject: [PATCH 01/11] Create Role + Binding attaching PodSecurityPolicy to managed ServiceAccount --- internal/k8s/gatewayclient/gatewayclient.go | 11 +++- internal/k8s/reconciler/deployer.go | 58 ++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) 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/reconciler/deployer.go b/internal/k8s/reconciler/deployer.go index 3f8a92527..08502f0be 100644 --- a/internal/k8s/reconciler/deployer.go +++ b/internal/k8s/reconciler/deployer.go @@ -4,8 +4,9 @@ import ( "context" "encoding/json" "fmt" - "github.com/hashicorp/consul-api-gateway/internal/consul" + capi "github.com/hashicorp/consul/api" + rbac "k8s.io/api/rbac/v1" "github.com/hashicorp/go-hclog" apps "k8s.io/api/apps/v1" @@ -14,6 +15,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" @@ -80,6 +82,44 @@ func (d *GatewayDeployer) Deploy(ctx context.Context, gateway *K8sGateway) error return d.ensureService(ctx, gateway.Config, gateway.Gateway) } +func roleForGateway(gateway *gwv1beta1.Gateway) *rbac.Role { + return &rbac.Role{ + ObjectMeta: meta.ObjectMeta{ + Name: gateway.Name, + Namespace: gateway.Namespace, + Labels: utils.LabelsForGateway(gateway), + }, + Rules: []rbac.PolicyRule{{ + APIGroups: []string{"policy"}, + Resources: []string{"podsecuritypolicies"}, + ResourceNames: []string{"consul-api-gateway"}, // TODO Accept policy name as config on GatewayClassConfig + Verbs: []string{"use"}, + }}, + } +} + +func roleBindingForGateway(gateway *gwv1beta1.Gateway, role *rbac.Role, serviceAccount *core.ServiceAccount) *rbac.RoleBinding { + return &rbac.RoleBinding{ + ObjectMeta: meta.ObjectMeta{ + Name: gateway.Name, + Namespace: gateway.Namespace, + Labels: utils.LabelsForGateway(gateway), + }, + RoleRef: rbac.RoleRef{ + APIGroup: role.GroupVersionKind().Group, + Kind: role.GroupVersionKind().Kind, + Name: role.Name, + }, + Subjects: []rbac.Subject{ + { + Kind: serviceAccount.GroupVersionKind().Kind, + Name: serviceAccount.Name, + Namespace: serviceAccount.Namespace, + }, + }, + } +} + func (d *GatewayDeployer) ensureServiceAccount(ctx context.Context, config apigwv1alpha1.GatewayClassConfig, gateway *gwv1beta1.Gateway) error { // Create service account for the gateway serviceAccount := config.ServiceAccountFor(gateway) @@ -87,7 +127,21 @@ 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 := roleForGateway(gateway) + if _, err := d.client.EnsureExists(ctx, role); err != nil { + return err + } + + binding := roleBindingForGateway(gateway, role, serviceAccount) + if _, err := d.client.EnsureExists(ctx, binding); err != nil { + return err + } + + return nil } // ensureSecret makes sure there is a Secret in the same namespace as the Gateway From 59deffc1fd7da77749fbb785308e7878b90f584f Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 1 Nov 2022 18:28:13 -0400 Subject: [PATCH 02/11] Regenerate mocks --- .../k8s/gatewayclient/mocks/gatewayclient.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) 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() From 74539a8204edf5a63de8954185c1254c3d93cc2b Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Tue, 1 Nov 2022 18:35:10 -0400 Subject: [PATCH 03/11] Update kubebuilder annotations to include roles + bindings --- config/rbac/role.yaml | 10 ++++++++++ internal/k8s/controllers/gateway_controller.go | 1 + 2 files changed, 11 insertions(+) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e53d98556..812a11de3 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -238,3 +238,13 @@ rules: - get - patch - update +- 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..f1282b630 100644 --- a/internal/k8s/controllers/gateway_controller.go +++ b/internal/k8s/controllers/gateway_controller.go @@ -47,6 +47,7 @@ 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 // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. From 7349560a75ad064f57a0dc70a275f5b9569efccd Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 2 Nov 2022 14:03:50 -0400 Subject: [PATCH 04/11] Add PSP to GatewayClassConfig, create Role + Binding for each Gateway --- ...sul.hashicorp.com_gatewayclassconfigs.yaml | 8 ++- config/rbac/role.yaml | 6 ++ .../k8s/controllers/gateway_controller.go | 1 + internal/k8s/reconciler/deployer.go | 55 ++++------------- pkg/apis/v1alpha1/types.go | 60 ++++++++++++++++++- 5 files changed, 80 insertions(+), 50 deletions(-) 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 281449055..8b09a0f5f 100644 --- a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml @@ -59,8 +59,8 @@ 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. type: string managed: description: Whether deployments should be run with "managed" @@ -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 ServiceAccount if Managed + 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 812a11de3..de0ec3c85 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -238,6 +238,12 @@ rules: - get - patch - update +- apiGroups: + - policy + resources: + - podsecuritypolicies + verbs: + - use - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/internal/k8s/controllers/gateway_controller.go b/internal/k8s/controllers/gateway_controller.go index f1282b630..2cdda236c 100644 --- a/internal/k8s/controllers/gateway_controller.go +++ b/internal/k8s/controllers/gateway_controller.go @@ -48,6 +48,7 @@ type GatewayReconciler struct { //+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/reconciler/deployer.go b/internal/k8s/reconciler/deployer.go index 08502f0be..5a7a46cf6 100644 --- a/internal/k8s/reconciler/deployer.go +++ b/internal/k8s/reconciler/deployer.go @@ -6,8 +6,6 @@ import ( "fmt" capi "github.com/hashicorp/consul/api" - rbac "k8s.io/api/rbac/v1" - "github.com/hashicorp/go-hclog" apps "k8s.io/api/apps/v1" core "k8s.io/api/core/v1" @@ -82,44 +80,6 @@ func (d *GatewayDeployer) Deploy(ctx context.Context, gateway *K8sGateway) error return d.ensureService(ctx, gateway.Config, gateway.Gateway) } -func roleForGateway(gateway *gwv1beta1.Gateway) *rbac.Role { - return &rbac.Role{ - ObjectMeta: meta.ObjectMeta{ - Name: gateway.Name, - Namespace: gateway.Namespace, - Labels: utils.LabelsForGateway(gateway), - }, - Rules: []rbac.PolicyRule{{ - APIGroups: []string{"policy"}, - Resources: []string{"podsecuritypolicies"}, - ResourceNames: []string{"consul-api-gateway"}, // TODO Accept policy name as config on GatewayClassConfig - Verbs: []string{"use"}, - }}, - } -} - -func roleBindingForGateway(gateway *gwv1beta1.Gateway, role *rbac.Role, serviceAccount *core.ServiceAccount) *rbac.RoleBinding { - return &rbac.RoleBinding{ - ObjectMeta: meta.ObjectMeta{ - Name: gateway.Name, - Namespace: gateway.Namespace, - Labels: utils.LabelsForGateway(gateway), - }, - RoleRef: rbac.RoleRef{ - APIGroup: role.GroupVersionKind().Group, - Kind: role.GroupVersionKind().Kind, - Name: role.Name, - }, - Subjects: []rbac.Subject{ - { - Kind: serviceAccount.GroupVersionKind().Kind, - Name: serviceAccount.Name, - Namespace: serviceAccount.Namespace, - }, - }, - } -} - func (d *GatewayDeployer) ensureServiceAccount(ctx context.Context, config apigwv1alpha1.GatewayClassConfig, gateway *gwv1beta1.Gateway) error { // Create service account for the gateway serviceAccount := config.ServiceAccountFor(gateway) @@ -131,17 +91,22 @@ func (d *GatewayDeployer) ensureServiceAccount(ctx context.Context, config apigw return err } - role := roleForGateway(gateway) + role := config.RoleFor(gateway) + if role == nil { + return nil + } + if _, err := d.client.EnsureExists(ctx, role); err != nil { return err } - binding := roleBindingForGateway(gateway, role, serviceAccount) - if _, err := d.client.EnsureExists(ctx, binding); err != nil { - return err + binding := config.RoleBindingFor(gateway) + if binding == nil { + return 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..c11afe563 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" @@ -128,10 +129,12 @@ type AuthSpec struct { 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. 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 ServiceAccount if Managed + PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` } // +kubebuilder:object:root=true @@ -144,17 +147,68 @@ type GatewayClassConfigList struct { Items []GatewayClassConfig `json:"items"` } +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"}, + }}, + } +} + +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 returns the service account to be created for the given gateway. 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), }, } } From 7fc526675ff9b9d321aa18651c4f65fe1126da37 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 2 Nov 2022 14:06:42 -0400 Subject: [PATCH 05/11] Improve docstrings on AuthSpec type --- ...pi-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml | 6 +++--- pkg/apis/v1alpha1/types.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) 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 8b09a0f5f..b0a0e8f49 100644 --- a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml @@ -60,11 +60,11 @@ spec: properties: account: description: The name of an existing Kubernetes ServiceAccount - to authenticate as. Ignored if !Managed. + to authenticate as. Ignored if !managed. 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 @@ -75,7 +75,7 @@ spec: type: string podSecurityPolicy: description: The name of an existing Kubernetes PodSecurityPolicy - to bind to the ServiceAccount if Managed + to bind to the ServiceAccount if managed. type: string type: object ports: diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index c11afe563..4e20eca16 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -125,15 +125,15 @@ 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 name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if !Managed. + // The name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if !managed. 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 ServiceAccount if Managed + // The name of an existing Kubernetes PodSecurityPolicy to bind to the ServiceAccount if managed. PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` } From 82b9b31e5d6309c164abaaabaacd069ba5697158 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Wed, 2 Nov 2022 16:00:01 -0400 Subject: [PATCH 06/11] Add changelog entry --- .changelog/433.txt | 3 +++ 1 file changed, 3 insertions(+) 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. +``` From e789c5f42783edb191a62664e949b3d2d87f6ae5 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 16:32:59 -0500 Subject: [PATCH 07/11] Update pkg/apis/v1alpha1/types.go --- pkg/apis/v1alpha1/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index 4e20eca16..8b6ea9704 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -129,7 +129,7 @@ type AuthSpec struct { Managed bool `json:"managed,omitempty"` // The Consul auth method used for initial authentication by consul-api-gateway. Method string `json:"method,omitempty"` - // The name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if !managed. + // The name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if managed. Account string `json:"account,omitempty"` // The Consul namespace to use for authentication. Namespace string `json:"namespace,omitempty"` From d41aed4954a0b4ca51b15e17196746c03fdce0a6 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 18:18:49 -0500 Subject: [PATCH 08/11] Add unit test coverage for Role, RoleBinding + ServiceAccount builders --- pkg/apis/v1alpha1/types_test.go | 149 ++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) 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) + }) +} From e1b0a13f7c87a303de144be0867230ffb16a5ddc Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 18:22:01 -0500 Subject: [PATCH 09/11] Improve/add docstrings for Role, RoleBinding + ServiceAccount builders --- pkg/apis/v1alpha1/types.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index 8b6ea9704..6bca28978 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -147,6 +147,9 @@ type GatewayClassConfigList struct { Items []GatewayClassConfig `json:"items"` } +// 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 @@ -167,6 +170,9 @@ func (c *GatewayClassConfig) RoleFor(gw *gwv1beta1.Gateway) *rbac.Role { } } +// 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 { @@ -199,7 +205,9 @@ func (c *GatewayClassConfig) RoleBindingFor(gw *gwv1beta1.Gateway) *rbac.RoleBin } } -// ServiceAccountFor returns the service account to be created for the given gateway. +// 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 From afbccd1174bd9a5a2b90a51a809e018b7985f486 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 18:26:33 -0500 Subject: [PATCH 10/11] Update godocs based on code review feedback, regenerate CRDs --- .../api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml | 4 ++-- pkg/apis/v1alpha1/types.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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 b0a0e8f49..859df815a 100644 --- a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml @@ -60,7 +60,7 @@ spec: properties: account: description: The name of an existing Kubernetes ServiceAccount - to authenticate as. Ignored if !managed. + to authenticate as. Ignored if managed is true. type: string managed: description: Whether deployments should be run with "managed" @@ -75,7 +75,7 @@ spec: type: string podSecurityPolicy: description: The name of an existing Kubernetes PodSecurityPolicy - to bind to the ServiceAccount if managed. + to bind to the ServiceAccount if managed is true. type: string type: object ports: diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index 6bca28978..8757e1c24 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -129,11 +129,11 @@ type AuthSpec struct { Managed bool `json:"managed,omitempty"` // The Consul auth method used for initial authentication by consul-api-gateway. Method string `json:"method,omitempty"` - // The name of an existing Kubernetes ServiceAccount to authenticate as. Ignored if managed. + // 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 ServiceAccount if managed. + // The name of an existing Kubernetes PodSecurityPolicy to bind to the ServiceAccount if managed is true. PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` } From dfbea340bc655d97b0316a1bbc4f07edcccc1e69 Mon Sep 17 00:00:00 2001 From: Nathan Coleman Date: Mon, 7 Nov 2022 18:36:00 -0500 Subject: [PATCH 11/11] Improve godoc --- .../api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml | 2 +- pkg/apis/v1alpha1/types.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 859df815a..273ac5719 100644 --- a/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml +++ b/config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml @@ -75,7 +75,7 @@ spec: type: string podSecurityPolicy: description: The name of an existing Kubernetes PodSecurityPolicy - to bind to the ServiceAccount if managed is true. + to bind to the managed ServiceAccount if managed is true. type: string type: object ports: diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index 8757e1c24..fb97ab089 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -133,7 +133,7 @@ type AuthSpec struct { 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 ServiceAccount if managed is true. + // The name of an existing Kubernetes PodSecurityPolicy to bind to the managed ServiceAccount if managed is true. PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` }