Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433

nathancoleman · 2022-11-01T22:26:17Z

Note Depends on the changes proposed in hashicorp/consul-k8s#1672 which was recently merged.

Changes proposed in this PR:

When creating a Deployment for a given Gateway, we create a "managed" ServiceAccount if the GatewayClassConfig indicates that we should.

With the changes in this PR, we will also create a Role and RoleBinding attaching the named podSecurityPolicy from the GatewayClassConfig if one has been provided.

Note For convenience, the Helm chart changes merged in hashicorp/consul-k8s#1672 will create a PodSecurityPolicy that is reasonable for these deployments when .Values.global.enablePodSecurityPolicies; however, users may also create their own PodSecurityPolicy and reference that in the GatewayClassConfig if they wish.

How I've tested this PR:

Create a K8s cluster that requires PodSecurityPolicies. In GKE, this is:

$ gcloud beta container clusters create cluster-1 --region us-east1 --enable-pod-security-policy

Install the Consul Helm chart from hashicorp/consul-k8s#1672 and exercise Consul API Gateway functionality.

How I expect reviewers to test this PR:

See above

Checklist:

Tests added
CHANGELOG entry added

Run make changelog-entry for guidance in authoring a changelog entry, and
commit the resulting file, which should have a name matching your PR number.
Entries should use imperative present tense (e.g. Add support for...)

…ccount

nathancoleman

Personal review

nathancoleman · 2022-11-07T20:42:19Z

config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml

@@ -73,6 +73,10 @@ spec:
                      namespace:
                        description: The Consul namespace to use for authentication.
                        type: string
+                      podSecurityPolicy:


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?

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?

nathancoleman · 2022-11-07T20:43:58Z

internal/k8s/controllers/gateway_controller.go

+//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=list;get;create;watch
+//+kubebuilder:rbac:groups=policy,resources=podsecuritypolicies,verbs=use


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

nathancoleman · 2022-11-07T20:45:34Z

internal/k8s/reconciler/deployer.go

+	role := config.RoleFor(gateway)
+	if role == nil {
+		return nil
+	}


Following the pattern already established by the ServiceAccount code immediately above: return nil if we shouldn't be creating a Role and move along.

pkg/apis/v1alpha1/types.go

nathancoleman · 2022-11-07T20:47:08Z

pkg/apis/v1alpha1/types.go

+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "Role",


Looked for some constants for these but had no luck finding any in the K8s SDK

t-eckert

Nice! I like that you included some refactoring work as well to keep things tidy

t-eckert · 2022-11-07T21:55:05Z

config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml

@@ -73,6 +73,10 @@ spec:
                      namespace:
                        description: The Consul namespace to use for authentication.
                        type: string
+                      podSecurityPolicy:


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?

t-eckert · 2022-11-07T21:58:46Z

internal/k8s/gatewayclient/gatewayclient.go

@@ -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) {


This is a really clean implementation!

config/crd/bases/api-gateway.consul.hashicorp.com_gatewayclassconfigs.yaml

nathancoleman added the pr/no-changelog Skip the CI check that requires a changelog entry label Nov 1, 2022

nathancoleman force-pushed the psp branch from 9f0f161 to 695fe5f Compare November 2, 2022 18:04

nathancoleman changed the title ~~Create Role + Binding attaching PodSecurityPolicy to managed ServiceAccount~~ Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided Nov 2, 2022

nathancoleman removed the pr/no-changelog Skip the CI check that requires a changelog entry label Nov 2, 2022

nathancoleman marked this pull request as ready for review November 3, 2022 18:42

nathancoleman force-pushed the psp branch from 7137ad8 to 9837e3b Compare November 3, 2022 19:20

nathancoleman requested review from sarahalsmiller, andrewstucki and mikemorris November 4, 2022 21:28

nathancoleman added 6 commits November 4, 2022 17:56

Create Role + Binding attaching PodSecurityPolicy to managed ServiceA…

46cba19

…ccount

Regenerate mocks

59deffc

Update kubebuilder annotations to include roles + bindings

74539a8

Add PSP to GatewayClassConfig, create Role + Binding for each Gateway

7349560

Improve docstrings on AuthSpec type

7fc5266

Add changelog entry

82b9b31

nathancoleman force-pushed the psp branch from 9837e3b to 82b9b31 Compare November 4, 2022 21:57

nathancoleman commented Nov 7, 2022

View reviewed changes

nathancoleman requested a review from t-eckert November 7, 2022 20:47

Update pkg/apis/v1alpha1/types.go

e789c5f

t-eckert approved these changes Nov 7, 2022

View reviewed changes

nathancoleman added 4 commits November 7, 2022 18:18

Add unit test coverage for Role, RoleBinding + ServiceAccount builders

d41aed4

Improve/add docstrings for Role, RoleBinding + ServiceAccount builders

e1b0a13

Update godocs based on code review feedback, regenerate CRDs

afbccd1

Improve godoc

dfbea34

nathancoleman merged commit 0d984c0 into main Nov 7, 2022

nathancoleman deleted the psp branch November 7, 2022 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433

Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433

nathancoleman commented Nov 1, 2022 •

edited

Loading

nathancoleman left a comment

nathancoleman Nov 7, 2022

t-eckert Nov 7, 2022

nathancoleman Nov 7, 2022

nathancoleman Nov 7, 2022

nathancoleman Nov 7, 2022

t-eckert left a comment

t-eckert Nov 7, 2022

t-eckert Nov 7, 2022

		//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=list;get;create;watch
		//+kubebuilder:rbac:groups=policy,resources=podsecuritypolicies,verbs=use

Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433

Accept PodSecurityPolicy name, bind to managed ServiceAccount if provided #433

Conversation

nathancoleman commented Nov 1, 2022 • edited Loading

Changes proposed in this PR:

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

nathancoleman left a comment

Choose a reason for hiding this comment

nathancoleman Nov 7, 2022

Choose a reason for hiding this comment

t-eckert Nov 7, 2022

Choose a reason for hiding this comment

nathancoleman Nov 7, 2022

Choose a reason for hiding this comment

nathancoleman Nov 7, 2022

Choose a reason for hiding this comment

nathancoleman Nov 7, 2022

Choose a reason for hiding this comment

t-eckert left a comment

Choose a reason for hiding this comment

t-eckert Nov 7, 2022

Choose a reason for hiding this comment

t-eckert Nov 7, 2022

Choose a reason for hiding this comment

nathancoleman commented Nov 1, 2022 •

edited

Loading