From 439380057b01e6648c3e21b3349993337a39e5cd Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Tue, 7 Feb 2023 10:35:16 -0500 Subject: [PATCH 01/12] kubebuilder webhook --- config/default/webhookcainjection_patch.yaml | 14 +- config/webhook/kustomization.yaml | 21 +++ config/webhook/kustomizeconfig.yaml | 14 +- config/webhook/manifests.yaml | 27 +++ controllers/secrets-webhook/secretswebhook.go | 162 ++++++++++++++++++ .../secrets-webhook/secretswebhook_test.go | 134 +++++++++++++++ main.go | 4 + 7 files changed, 362 insertions(+), 14 deletions(-) create mode 100644 controllers/secrets-webhook/secretswebhook.go create mode 100644 controllers/secrets-webhook/secretswebhook_test.go diff --git a/config/default/webhookcainjection_patch.yaml b/config/default/webhookcainjection_patch.yaml index 740135388..042be2151 100644 --- a/config/default/webhookcainjection_patch.yaml +++ b/config/default/webhookcainjection_patch.yaml @@ -1,12 +1,12 @@ # This patch add annotation to admission webhook config and # the variables $(CERTIFICATE_NAMESPACE) and $(CERTIFICATE_NAME) will be substituted by kustomize. -# apiVersion: admissionregistration.k8s.io/v1 -# kind: MutatingWebhookConfiguration -# metadata: -# name: mutating-webhook-configuration -# annotations: -# cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) -# --- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: k8ssandra-operator-mutating-webhook-configuration + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE_K8S)/$(CERTIFICATE_NAME_K8S) +--- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml index bdb14e5a3..17ade3177 100644 --- a/config/webhook/kustomization.yaml +++ b/config/webhook/kustomization.yaml @@ -20,3 +20,24 @@ patchesJson6902: - op: replace path: /webhooks/0/clientConfig/service/name value: k8ssandra-operator-webhook-service + +# adding the objectSelector prevents the bootstrapping problem +# where the mutation request for the operator pod would be +# sent before the operator pod is created +patchesJson6902: +- target: + group: admissionregistration.k8s.io + version: v1 + name: k8ssandra-operator-mutating-webhook-configuration + kind: MutatingWebhookConfiguration + patch: |- + - op: replace + path: /webhooks/0/clientConfig/service/name + value: k8ssandra-operator-webhook-service + - op: add + path: /webhooks/0/objectSelector + value: + matchExpressions: + - key: control-plane + operator: NotIn + values: ["k8ssandra-operator"] \ No newline at end of file diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml index 5659500fe..25e21e3c9 100644 --- a/config/webhook/kustomizeconfig.yaml +++ b/config/webhook/kustomizeconfig.yaml @@ -4,18 +4,18 @@ nameReference: - kind: Service version: v1 fieldSpecs: - # - kind: MutatingWebhookConfiguration - # group: admissionregistration.k8s.io - # path: webhooks/clientConfig/service/name + - kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name - kind: ValidatingWebhookConfiguration group: admissionregistration.k8s.io path: webhooks/clientConfig/service/name namespace: -# - kind: MutatingWebhookConfiguration -# group: admissionregistration.k8s.io -# path: webhooks/clientConfig/service/namespace -# create: true +- kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true - kind: ValidatingWebhookConfiguration group: admissionregistration.k8s.io path: webhooks/clientConfig/service/namespace diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 90a113679..faaa8a994 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,5 +1,32 @@ --- apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + creationTimestamp: null + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-v1-pod-secrets-inject + failurePolicy: Fail + name: mpod.kb.io + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - pods + sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: creationTimestamp: null diff --git a/controllers/secrets-webhook/secretswebhook.go b/controllers/secrets-webhook/secretswebhook.go new file mode 100644 index 000000000..65f624f2d --- /dev/null +++ b/controllers/secrets-webhook/secretswebhook.go @@ -0,0 +1,162 @@ +package secrets_webhook + +import ( + "context" + "encoding/json" + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "net/http" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + log "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +// +kubebuilder:webhook:path=/mutate-v1-pod-secrets-inject,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=mpod.kb.io,admissionReviewVersions=v1,sideEffects=None + +func SetupSecretsInjectorWebhook(mgr ctrl.Manager) { + mgr.GetWebhookServer().Register("/mutate-v1-pod-secrets-inject", &webhook.Admission{Handler: &podSecretsInjector{Client: mgr.GetClient()}}) +} + +// podSecretsInjector is an admission handler that mutates pod manifests +// to include mechanisms for mounting secrets +type podSecretsInjector struct { + Client client.Client + decoder *admission.Decoder +} + +// podSecretsInjector Implements admission.Handler. +var _ admission.Handler = &podSecretsInjector{} + +// InjectDecoder injects the decoder into the podSecretsInjector +func (p *podSecretsInjector) InjectDecoder(d *admission.Decoder) error { + p.decoder = d + return nil +} + +func (p *podSecretsInjector) Handle(ctx context.Context, req admission.Request) admission.Response { + logger := log.FromContext(ctx).WithValues("podSecretsInjector", req.Namespace) + + pod := &corev1.Pod{} + err := p.decoder.Decode(req, pod) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + copy := pod.DeepCopy() + + err = p.mutatePods(ctx, copy, logger) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + marshaledPod, err := json.Marshal(copy) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod) +} + +// e.g. k8ssandra.io/inject-secret=[{secretName=my-secret, path=/etc/credentials/cassandra}] +const secretInjectionAnnotation = "k8ssandra.io/inject-secret" + +type SecretInjection struct { + SecretName string `json:"secretName"` + Path string `json:"path"` +} + +// mutatePods injects the secret mounting configuration into the pod +func (p *podSecretsInjector) mutatePods(ctx context.Context, pod *corev1.Pod, logger logr.Logger) error { + if pod.Annotations == nil { + logger.Info("no annotations exist") + return nil + } + + secretsStr := pod.Annotations[secretInjectionAnnotation] + if len(secretsStr) == 0 { + logger.Info("no secret annotation exists") + return nil + } + + var secrets []SecretInjection + if err := json.Unmarshal([]byte(secretsStr), &secrets); err != nil { + logger.Error(err, "unable to unmarhsal secrets annotation", "annotation", secretsStr) + return err + } + + for _, secret := range secrets { + // get secret name from injection annotation + secretName := secret.SecretName + mountPath := secret.Path + logger.Info("creating volume and volume mount for secret", + "secret", secretName, + "secret path", mountPath, + ) + + volume := corev1.Volume{ + Name: secretName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: secretName, + }, + }, + } + injectVolume(pod, volume) + + volumeMount := corev1.VolumeMount{ + Name: secretName, + MountPath: mountPath, + } + injectVolumeMount(pod, volumeMount) + logger.Info("added volume and volumeMount to podSpec", + "secret", secretName, + "secret path", mountPath, + ) + } + + return nil +} + +// injectVolume attaches a volume to the pod spec +func injectVolume(pod *corev1.Pod, volume corev1.Volume) { + if !hasVolume(pod.Spec.Volumes, volume) { + pod.Spec.Volumes = append(pod.Spec.Volumes, volume) + } +} + +// injectVolumeMount attaches a volumeMount to all containers in the pod spec +func injectVolumeMount(pod *corev1.Pod, volumeMount corev1.VolumeMount) { + for i, container := range pod.Spec.Containers { + if !hasVolumeMount(container, volumeMount) { + pod.Spec.Containers[i].VolumeMounts = append(container.VolumeMounts, volumeMount) + } + } + for i, container := range pod.Spec.InitContainers { + if !hasVolumeMount(container, volumeMount) { + pod.Spec.Containers[i].VolumeMounts = append(container.VolumeMounts, volumeMount) + } + } +} + +// hasVolume returns true if volume exists, false otherwise +func hasVolume(volumes []corev1.Volume, volume corev1.Volume) bool { + for _, v := range volumes { + if v.Name == volume.Name { + return true + } + } + return false +} + +// hasVolumeMount returns true if volume mount exists, false otherwise +func hasVolumeMount(container corev1.Container, volumeMount corev1.VolumeMount) bool { + for _, mount := range container.VolumeMounts { + if mount.Name == volumeMount.Name { + return true + } + } + return false +} diff --git a/controllers/secrets-webhook/secretswebhook_test.go b/controllers/secrets-webhook/secretswebhook_test.go new file mode 100644 index 000000000..7944ea3d8 --- /dev/null +++ b/controllers/secrets-webhook/secretswebhook_test.go @@ -0,0 +1,134 @@ +package secrets_webhook + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + log "sigs.k8s.io/controller-runtime/pkg/log" +) + +func TestInjectSecretsc(t *testing.T) { + want := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "k8ssandra.io/inject-secret": `[{"secretName": "mySecret", "path": "/my/secret/path"}]`, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + VolumeMounts: []corev1.VolumeMount{{ + Name: "mySecret", + MountPath: "/my/secret/path", + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "mySecret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "mySecret", + }, + }, + }}, + }, + } + + pod := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "k8ssandra.io/inject-secret": `[{"secretName": "mySecret", "path": "/my/secret/path"}]`, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + }}, + }, + } + + p := &podSecretsInjector{} + + ctx := context.TODO() + err := p.mutatePods(ctx, pod, log.FromContext(ctx)) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, want, pod) +} + +func TestInjectSecretsMultiMutate(t *testing.T) { + injectionAnnotation := `[{"secretName": "mySecret", "path": "/my/secret/path"}, + {"secretName": "myOtherSecret", "path": "/my/other/secret/path"}]` + + want := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "k8ssandra.io/inject-secret": injectionAnnotation, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "mySecret", + MountPath: "/my/secret/path", + }, + { + Name: "myOtherSecret", + MountPath: "/my/other/secret/path", + }, + }, + }}, + Volumes: []corev1.Volume{ + { + Name: "mySecret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "mySecret", + }, + }, + }, + { + Name: "myOtherSecret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "myOtherSecret", + }, + }, + }, + }, + }, + } + + pod := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "k8ssandra.io/inject-secret": injectionAnnotation, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + }}, + }, + } + + p := &podSecretsInjector{} + + ctx := context.TODO() + err := p.mutatePods(ctx, pod, log.FromContext(ctx)) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, want, pod) +} diff --git a/main.go b/main.go index 9a314690d..83c59ff73 100644 --- a/main.go +++ b/main.go @@ -61,6 +61,7 @@ import ( medusactrl "github.com/k8ssandra/k8ssandra-operator/controllers/medusa" reaperctrl "github.com/k8ssandra/k8ssandra-operator/controllers/reaper" replicationctrl "github.com/k8ssandra/k8ssandra-operator/controllers/replication" + secretswebhook "github.com/k8ssandra/k8ssandra-operator/controllers/secrets-webhook" stargatectrl "github.com/k8ssandra/k8ssandra-operator/controllers/stargate" // +kubebuilder:scaffold:imports ) @@ -291,11 +292,14 @@ func main() { os.Exit(1) } + secretswebhook.SetupSecretsInjectorWebhook(mgr) + setupLog.Info("starting manager") if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1) } + } // getWatchNamespace returns the Namespace the operator should be watching for changes From a31c116941b822c6e0891ae37f5d28f7d6a74314 Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Tue, 21 Feb 2023 13:57:09 -0500 Subject: [PATCH 02/12] add secretswebhook to testenv --- pkg/test/testenv.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/test/testenv.go b/pkg/test/testenv.go index 2ad82af7f..db0a2b325 100644 --- a/pkg/test/testenv.go +++ b/pkg/test/testenv.go @@ -16,6 +16,7 @@ import ( reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1" promapi "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + secretswebhook "github.com/k8ssandra/k8ssandra-operator/controllers/secrets-webhook" "github.com/k8ssandra/k8ssandra-operator/pkg/clientcache" "github.com/k8ssandra/k8ssandra-operator/test/framework" "github.com/k8ssandra/k8ssandra-operator/test/kustomize" @@ -260,6 +261,8 @@ func (e *MultiClusterTestEnv) Start(ctx context.Context, t *testing.T, initRecon return err } + secretswebhook.SetupSecretsInjectorWebhook(k8sManager) + go func() { err = k8sManager.Start(ctx) if err != nil { From a3f311de21998011f81838ed012a15eff6bf8336 Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Tue, 21 Feb 2023 15:04:27 -0500 Subject: [PATCH 03/12] test namespace patching --- config/components/single-namespace/kustomization.yaml | 5 +++++ test/framework/e2e_framework.go | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/config/components/single-namespace/kustomization.yaml b/config/components/single-namespace/kustomization.yaml index 8c4f5203b..07af2f558 100644 --- a/config/components/single-namespace/kustomization.yaml +++ b/config/components/single-namespace/kustomization.yaml @@ -27,6 +27,11 @@ replacements: kind: ValidatingWebhookConfiguration fieldPaths: - webhooks.0.clientConfig.service.namespace + - select: + name: k8ssandra-operator-mutating-webhook-configuration + kind: MutatingWebhookConfiguration + fieldPaths: + - webhooks.0.clientConfig.service.namespace patchesStrategicMerge: - |- apiVersion: v1 diff --git a/test/framework/e2e_framework.go b/test/framework/e2e_framework.go index 3af1d7521..1de13a2e4 100644 --- a/test/framework/e2e_framework.go +++ b/test/framework/e2e_framework.go @@ -179,6 +179,11 @@ replacements: kind: ValidatingWebhookConfiguration fieldPaths: - webhooks.1.clientConfig.service.namespace + - select: + name: k8ssandra-operator-mutating-webhook-configuration + kind: MutatingWebhookConfiguration + fieldPaths: + - webhooks.0.clientConfig.service.namespace ` dataPlaneTmpl := ` @@ -236,6 +241,11 @@ replacements: kind: ValidatingWebhookConfiguration fieldPaths: - webhooks.1.clientConfig.service.namespace + - select: + name: k8ssandra-operator-mutating-webhook-configuration + kind: MutatingWebhookConfiguration + fieldPaths: + - webhooks.0.clientConfig.service.namespace ` err := generateKustomizationFile(fmt.Sprintf("k8ssandra-operator/%s", controlPlaneDir), config, controlPlaneTmpl) From c3f224424aba34d7ff8b36e1ee691f2a84b4e72a Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Wed, 22 Feb 2023 14:52:52 -0500 Subject: [PATCH 04/12] fix tests --- controllers/medusa/controllers_test.go | 25 +++++++++++++++++++++++-- pkg/test/testenv.go | 2 +- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/controllers/medusa/controllers_test.go b/controllers/medusa/controllers_test.go index bc5b6f967..b7bf69596 100644 --- a/controllers/medusa/controllers_test.go +++ b/controllers/medusa/controllers_test.go @@ -9,6 +9,7 @@ import ( cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" k8ssandractrl "github.com/k8ssandra/k8ssandra-operator/controllers/k8ssandra" + secretswebhook "github.com/k8ssandra/k8ssandra-operator/controllers/secrets-webhook" "github.com/k8ssandra/k8ssandra-operator/pkg/clientcache" "github.com/k8ssandra/k8ssandra-operator/pkg/config" "k8s.io/client-go/kubernetes/scheme" @@ -81,7 +82,15 @@ func setupMedusaBackupTestEnv(t *testing.T, ctx context.Context) *testutils.Mult } for _, env := range testEnv.GetDataPlaneEnvTests() { - dataPlaneMgr, err := ctrl.NewManager(env.Config, ctrl.Options{Scheme: scheme.Scheme}) + dataPlaneMgr, err := ctrl.NewManager( + env.Config, + ctrl.Options{ + Scheme: scheme.Scheme, + Host: env.WebhookInstallOptions.LocalServingHost, + Port: env.WebhookInstallOptions.LocalServingPort, + CertDir: env.WebhookInstallOptions.LocalServingCertDir, + }, + ) if err != nil { return err } @@ -94,6 +103,8 @@ func setupMedusaBackupTestEnv(t *testing.T, ctx context.Context) *testutils.Mult if err != nil { return err } + secretswebhook.SetupSecretsInjectorWebhook(dataPlaneMgr) + go func() { err := dataPlaneMgr.Start(ctx) if err != nil { @@ -214,7 +225,15 @@ func setupMedusaTaskTestEnv(t *testing.T, ctx context.Context) *testutils.MultiC } for _, env := range testEnv.GetDataPlaneEnvTests() { - dataPlaneMgr, err := ctrl.NewManager(env.Config, ctrl.Options{Scheme: scheme.Scheme}) + dataPlaneMgr, err := ctrl.NewManager( + env.Config, + ctrl.Options{ + Scheme: scheme.Scheme, + Host: env.WebhookInstallOptions.LocalServingHost, + Port: env.WebhookInstallOptions.LocalServingPort, + CertDir: env.WebhookInstallOptions.LocalServingCertDir, + }, + ) if err != nil { return err } @@ -233,6 +252,8 @@ func setupMedusaTaskTestEnv(t *testing.T, ctx context.Context) *testutils.MultiC Scheme: scheme.Scheme, ClientFactory: medusaClientFactory, }).SetupWithManager(dataPlaneMgr) + secretswebhook.SetupSecretsInjectorWebhook(dataPlaneMgr) + if err != nil { return err } diff --git a/pkg/test/testenv.go b/pkg/test/testenv.go index db0a2b325..48d701c27 100644 --- a/pkg/test/testenv.go +++ b/pkg/test/testenv.go @@ -105,6 +105,7 @@ func (e *TestEnv) Start(ctx context.Context, t *testing.T, initReconcilers func( if err != nil { return err } + secretswebhook.SetupSecretsInjectorWebhook(k8sManager) go func() { err = k8sManager.Start(ctx) @@ -260,7 +261,6 @@ func (e *MultiClusterTestEnv) Start(ctx context.Context, t *testing.T, initRecon if err != nil { return err } - secretswebhook.SetupSecretsInjectorWebhook(k8sManager) go func() { From 731d6f0ce3982e0643ec4675a8d19b7a5dbe948a Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Wed, 22 Feb 2023 15:04:31 -0500 Subject: [PATCH 05/12] rename secrets webhook unit tests --- controllers/secrets-webhook/secretswebhook_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/secrets-webhook/secretswebhook_test.go b/controllers/secrets-webhook/secretswebhook_test.go index 7944ea3d8..55f6abfc8 100644 --- a/controllers/secrets-webhook/secretswebhook_test.go +++ b/controllers/secrets-webhook/secretswebhook_test.go @@ -10,7 +10,7 @@ import ( log "sigs.k8s.io/controller-runtime/pkg/log" ) -func TestInjectSecretsc(t *testing.T) { +func TestMutatePodsSingleSecret(t *testing.T) { want := &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ Name: "test", @@ -53,7 +53,7 @@ func TestInjectSecretsc(t *testing.T) { p := &podSecretsInjector{} - ctx := context.TODO() + ctx := context.Background() err := p.mutatePods(ctx, pod, log.FromContext(ctx)) if err != nil { t.Fatal(err) @@ -62,7 +62,7 @@ func TestInjectSecretsc(t *testing.T) { assert.Equal(t, want, pod) } -func TestInjectSecretsMultiMutate(t *testing.T) { +func TestMutatePodsMutliSecret(t *testing.T) { injectionAnnotation := `[{"secretName": "mySecret", "path": "/my/secret/path"}, {"secretName": "myOtherSecret", "path": "/my/other/secret/path"}]` @@ -124,7 +124,7 @@ func TestInjectSecretsMultiMutate(t *testing.T) { p := &podSecretsInjector{} - ctx := context.TODO() + ctx := context.Background() err := p.mutatePods(ctx, pod, log.FromContext(ctx)) if err != nil { t.Fatal(err) From 4bc5524b2d9b6aa9c0b41c7c270ed9791bac022f Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 10:31:03 -0500 Subject: [PATCH 06/12] more comment info --- controllers/secrets-webhook/secretswebhook.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/controllers/secrets-webhook/secretswebhook.go b/controllers/secrets-webhook/secretswebhook.go index 65f624f2d..f7c1dab75 100644 --- a/controllers/secrets-webhook/secretswebhook.go +++ b/controllers/secrets-webhook/secretswebhook.go @@ -60,7 +60,7 @@ func (p *podSecretsInjector) Handle(ctx context.Context, req admission.Request) return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod) } -// e.g. k8ssandra.io/inject-secret=[{secretName=my-secret, path=/etc/credentials/cassandra}] +// e.g. k8ssandra.io/inject-secret: '[{ "secretName": "test-secret", "path": "/etc/test/test-secret" }]' const secretInjectionAnnotation = "k8ssandra.io/inject-secret" type SecretInjection struct { @@ -71,19 +71,23 @@ type SecretInjection struct { // mutatePods injects the secret mounting configuration into the pod func (p *podSecretsInjector) mutatePods(ctx context.Context, pod *corev1.Pod, logger logr.Logger) error { if pod.Annotations == nil { - logger.Info("no annotations exist") + logger.Info("no annotations exist", "podName", pod.Name, "namespace", pod.Namespace) return nil } secretsStr := pod.Annotations[secretInjectionAnnotation] if len(secretsStr) == 0 { - logger.Info("no secret annotation exists") + logger.Info("no secret annotation exists", "podName", pod.Name, "namespace", pod.Namespace) return nil } var secrets []SecretInjection if err := json.Unmarshal([]byte(secretsStr), &secrets); err != nil { - logger.Error(err, "unable to unmarhsal secrets annotation", "annotation", secretsStr) + logger.Error(err, "unable to unmarhsal secrets annotation", + "annotation", secretsStr, + "podName", pod.Name, + "namespace", pod.Namespace, + ) return err } @@ -94,6 +98,8 @@ func (p *podSecretsInjector) mutatePods(ctx context.Context, pod *corev1.Pod, lo logger.Info("creating volume and volume mount for secret", "secret", secretName, "secret path", mountPath, + "podName", pod.Name, + "namespace", pod.Namespace, ) volume := corev1.Volume{ @@ -114,6 +120,8 @@ func (p *podSecretsInjector) mutatePods(ctx context.Context, pod *corev1.Pod, lo logger.Info("added volume and volumeMount to podSpec", "secret", secretName, "secret path", mountPath, + "podName", pod.Name, + "namespace", pod.Namespace, ) } From 23bf3a60322377f3b66a827adc1573ec71e6b0a2 Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 10:31:19 -0500 Subject: [PATCH 07/12] unit tests --- .../secrets-webhook/secretswebhook_test.go | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/controllers/secrets-webhook/secretswebhook_test.go b/controllers/secrets-webhook/secretswebhook_test.go index 55f6abfc8..b6031c570 100644 --- a/controllers/secrets-webhook/secretswebhook_test.go +++ b/controllers/secrets-webhook/secretswebhook_test.go @@ -2,14 +2,116 @@ package secrets_webhook import ( "context" + "encoding/json" + "fmt" "testing" "github.com/stretchr/testify/assert" + admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/scheme" log "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) +func TestHandleSinceSecretSuccess(t *testing.T) { + p := &podSecretsInjector{} + d, err := admission.NewDecoder(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + p.InjectDecoder(d) + + pod := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "k8ssandra.io/inject-secret": `[{"secretName": "mySecret", "path": "/my/secret/path"}]`, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + }}, + }, + } + pBytes, err := json.Marshal(pod) + if err != nil { + t.Fatal(err) + } + + req := webhook.AdmissionRequest{AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "test123", + Name: "foo", + Namespace: "bar", + Resource: metav1.GroupVersionResource{ + Version: "v1", + Resource: "pods", + }, + Operation: "CREATE", + Object: runtime.RawExtension{Raw: pBytes}, + }} + + resp := p.Handle(context.Background(), req) + fmt.Println(fmt.Sprintf("%v", resp)) + assert.Equal(t, true, resp.AdmissionResponse.Allowed) + // 2 patches for addition of volume and volumeMount + assert.Equal(t, len(resp.Patches), 2) + assert.Equal(t, resp.Patches[0].Operation, "add") + assert.Equal(t, resp.Patches[0].Path, "/spec/volumes") + assert.Equal(t, resp.Patches[1].Operation, "add") + assert.Equal(t, resp.Patches[1].Path, "/spec/containers/0/volumeMounts") +} + +func TestHandleSinceSecretNoPatch(t *testing.T) { + p := &podSecretsInjector{} + d, err := admission.NewDecoder(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + p.InjectDecoder(d) + + pod := &corev1.Pod{ + ObjectMeta: v1.ObjectMeta{ + Name: "test", + Annotations: map[string]string{ + "fake-annotation": `fake-val`, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "test", + }}, + }, + } + pBytes, err := json.Marshal(pod) + if err != nil { + t.Fatal(err) + } + + req := webhook.AdmissionRequest{AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "test123", + Name: "foo", + Namespace: "bar", + Resource: metav1.GroupVersionResource{ + Version: "v1", + Resource: "pods", + }, + Operation: "CREATE", + Object: runtime.RawExtension{Raw: pBytes}, + }} + + resp := p.Handle(context.Background(), req) + fmt.Println(fmt.Sprintf("%v", resp)) + assert.Equal(t, true, resp.AdmissionResponse.Allowed) + // no injection annotation, no patch + assert.Equal(t, len(resp.Patches), 0) +} + func TestMutatePodsSingleSecret(t *testing.T) { want := &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ From b7163850e074ada6deb68b342d091e7466c30b0a Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 10:50:32 -0500 Subject: [PATCH 08/12] refactor unit test --- .../secrets-webhook/secretswebhook_test.go | 77 ++++++++----------- 1 file changed, 33 insertions(+), 44 deletions(-) diff --git a/controllers/secrets-webhook/secretswebhook_test.go b/controllers/secrets-webhook/secretswebhook_test.go index b6031c570..bee2280d0 100644 --- a/controllers/secrets-webhook/secretswebhook_test.go +++ b/controllers/secrets-webhook/secretswebhook_test.go @@ -19,12 +19,7 @@ import ( ) func TestHandleSinceSecretSuccess(t *testing.T) { - p := &podSecretsInjector{} - d, err := admission.NewDecoder(scheme.Scheme) - if err != nil { - t.Fatal(err) - } - p.InjectDecoder(d) + p := setupSecretsInjector(t) pod := &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ @@ -39,22 +34,7 @@ func TestHandleSinceSecretSuccess(t *testing.T) { }}, }, } - pBytes, err := json.Marshal(pod) - if err != nil { - t.Fatal(err) - } - - req := webhook.AdmissionRequest{AdmissionRequest: admissionv1.AdmissionRequest{ - UID: "test123", - Name: "foo", - Namespace: "bar", - Resource: metav1.GroupVersionResource{ - Version: "v1", - Resource: "pods", - }, - Operation: "CREATE", - Object: runtime.RawExtension{Raw: pBytes}, - }} + req := createRequest(t, pod) resp := p.Handle(context.Background(), req) fmt.Println(fmt.Sprintf("%v", resp)) @@ -68,12 +48,7 @@ func TestHandleSinceSecretSuccess(t *testing.T) { } func TestHandleSinceSecretNoPatch(t *testing.T) { - p := &podSecretsInjector{} - d, err := admission.NewDecoder(scheme.Scheme) - if err != nil { - t.Fatal(err) - } - p.InjectDecoder(d) + p := setupSecretsInjector(t) pod := &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ @@ -88,22 +63,7 @@ func TestHandleSinceSecretNoPatch(t *testing.T) { }}, }, } - pBytes, err := json.Marshal(pod) - if err != nil { - t.Fatal(err) - } - - req := webhook.AdmissionRequest{AdmissionRequest: admissionv1.AdmissionRequest{ - UID: "test123", - Name: "foo", - Namespace: "bar", - Resource: metav1.GroupVersionResource{ - Version: "v1", - Resource: "pods", - }, - Operation: "CREATE", - Object: runtime.RawExtension{Raw: pBytes}, - }} + req := createRequest(t, pod) resp := p.Handle(context.Background(), req) fmt.Println(fmt.Sprintf("%v", resp)) @@ -234,3 +194,32 @@ func TestMutatePodsMutliSecret(t *testing.T) { assert.Equal(t, want, pod) } + +func setupSecretsInjector(t *testing.T) *podSecretsInjector { + p := &podSecretsInjector{} + d, err := admission.NewDecoder(scheme.Scheme) + if err != nil { + t.Fatal(err) + } + p.InjectDecoder(d) + return p +} + +func createRequest(t *testing.T, pod *corev1.Pod) webhook.AdmissionRequest { + pBytes, err := json.Marshal(pod) + if err != nil { + t.Fatal(err) + } + + return webhook.AdmissionRequest{AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "test123", + Name: "foo", + Namespace: "bar", + Resource: metav1.GroupVersionResource{ + Version: "v1", + Resource: "pods", + }, + Operation: "CREATE", + Object: runtime.RawExtension{Raw: pBytes}, + }} +} From 0aea222516cdafb1dc5ac6afc073988c03b1f53a Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 13:03:17 -0500 Subject: [PATCH 09/12] util refactor --- controllers/secrets-webhook/secretswebhook.go | 27 +++---------------- pkg/utils/deployment.go | 14 ++++++---- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/controllers/secrets-webhook/secretswebhook.go b/controllers/secrets-webhook/secretswebhook.go index f7c1dab75..55a974aa3 100644 --- a/controllers/secrets-webhook/secretswebhook.go +++ b/controllers/secrets-webhook/secretswebhook.go @@ -7,6 +7,7 @@ import ( corev1 "k8s.io/api/core/v1" "net/http" + "github.com/k8ssandra/k8ssandra-operator/pkg/utils" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" log "sigs.k8s.io/controller-runtime/pkg/log" @@ -130,7 +131,7 @@ func (p *podSecretsInjector) mutatePods(ctx context.Context, pod *corev1.Pod, lo // injectVolume attaches a volume to the pod spec func injectVolume(pod *corev1.Pod, volume corev1.Volume) { - if !hasVolume(pod.Spec.Volumes, volume) { + if _, found := utils.ContainsVolume(pod.Spec.Volumes, volume.Name); !found { pod.Spec.Volumes = append(pod.Spec.Volumes, volume) } } @@ -138,33 +139,13 @@ func injectVolume(pod *corev1.Pod, volume corev1.Volume) { // injectVolumeMount attaches a volumeMount to all containers in the pod spec func injectVolumeMount(pod *corev1.Pod, volumeMount corev1.VolumeMount) { for i, container := range pod.Spec.Containers { - if !hasVolumeMount(container, volumeMount) { + if utils.FindVolumeMount(&container, volumeMount.Name) == nil { pod.Spec.Containers[i].VolumeMounts = append(container.VolumeMounts, volumeMount) } } for i, container := range pod.Spec.InitContainers { - if !hasVolumeMount(container, volumeMount) { + if utils.FindVolumeMount(&container, volumeMount.Name) == nil { pod.Spec.Containers[i].VolumeMounts = append(container.VolumeMounts, volumeMount) } } } - -// hasVolume returns true if volume exists, false otherwise -func hasVolume(volumes []corev1.Volume, volume corev1.Volume) bool { - for _, v := range volumes { - if v.Name == volume.Name { - return true - } - } - return false -} - -// hasVolumeMount returns true if volume mount exists, false otherwise -func hasVolumeMount(container corev1.Container, volumeMount corev1.VolumeMount) bool { - for _, mount := range container.VolumeMounts { - if mount.Name == volumeMount.Name { - return true - } - } - return false -} diff --git a/pkg/utils/deployment.go b/pkg/utils/deployment.go index 4fa3e383d..02d550974 100644 --- a/pkg/utils/deployment.go +++ b/pkg/utils/deployment.go @@ -35,14 +35,18 @@ func FindVolumeMount(container *corev1.Container, name string) *corev1.VolumeMou func FindVolume(deployment *appsv1.Deployment, name string) (int, bool) { if deployment != nil { - for i, volume := range deployment.Spec.Template.Spec.Volumes { - if volume.Name == name { - return i, true - } - } + return ContainsVolume(deployment.Spec.Template.Spec.Volumes, name) } return -1, false +} +func ContainsVolume(volumes []corev1.Volume, name string) (int, bool) { + for i, volume := range volumes { + if volume.Name == name { + return i, true + } + } + return -1, false } func FindAndGetVolume(deployment *appsv1.Deployment, name string) *corev1.Volume { From c4d8790d85bfe2348802372605e82cc74b7c05f3 Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 13:45:32 -0500 Subject: [PATCH 10/12] changelog --- CHANGELOG/CHANGELOG-1.6.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG/CHANGELOG-1.6.md b/CHANGELOG/CHANGELOG-1.6.md index 76bbe10e6..f00f2309f 100644 --- a/CHANGELOG/CHANGELOG-1.6.md +++ b/CHANGELOG/CHANGELOG-1.6.md @@ -24,3 +24,4 @@ When cutting a new release, update the `unreleased` heading to the tag being gen * [BUGFIX] [#854](https://github.com/k8ssandra/k8ssandra-operator/issues/854) Use Patch() to update K8ssandraTask status. * [CHANGE] [#887](https://github.com/k8ssandra/k8ssandra-operator/issues/887) Fix CVE-2022-32149. * [CHANGE] [#891](https://github.com/k8ssandra/k8ssandra-operator/issues/848) Update golang.org/x/net to fix several CVEs. +* [FEATURE] [#598](https://github.com/k8ssandra/k8ssandra-operator/issues/598) Create a mutating webhook for the internal secrets provider From 77f691b04f2a73cc4ef29a79ca8bb402c66aa1d2 Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 13:48:06 -0500 Subject: [PATCH 11/12] fix issue # --- CHANGELOG/CHANGELOG-1.6.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG/CHANGELOG-1.6.md b/CHANGELOG/CHANGELOG-1.6.md index f00f2309f..0fe7a7f59 100644 --- a/CHANGELOG/CHANGELOG-1.6.md +++ b/CHANGELOG/CHANGELOG-1.6.md @@ -24,4 +24,4 @@ When cutting a new release, update the `unreleased` heading to the tag being gen * [BUGFIX] [#854](https://github.com/k8ssandra/k8ssandra-operator/issues/854) Use Patch() to update K8ssandraTask status. * [CHANGE] [#887](https://github.com/k8ssandra/k8ssandra-operator/issues/887) Fix CVE-2022-32149. * [CHANGE] [#891](https://github.com/k8ssandra/k8ssandra-operator/issues/848) Update golang.org/x/net to fix several CVEs. -* [FEATURE] [#598](https://github.com/k8ssandra/k8ssandra-operator/issues/598) Create a mutating webhook for the internal secrets provider +* [FEATURE] [#605](https://github.com/k8ssandra/k8ssandra-operator/issues/598) Create a mutating webhook for the internal secrets provider From 8ca9d37b4402714beef2dff654b13ca756861dcc Mon Sep 17 00:00:00 2001 From: Steven Seidman Date: Thu, 23 Feb 2023 14:43:02 -0500 Subject: [PATCH 12/12] fix test name --- controllers/secrets-webhook/secretswebhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/secrets-webhook/secretswebhook_test.go b/controllers/secrets-webhook/secretswebhook_test.go index bee2280d0..05921e3dc 100644 --- a/controllers/secrets-webhook/secretswebhook_test.go +++ b/controllers/secrets-webhook/secretswebhook_test.go @@ -18,7 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -func TestHandleSinceSecretSuccess(t *testing.T) { +func TestHandleInjectSecretSuccess(t *testing.T) { p := setupSecretsInjector(t) pod := &corev1.Pod{ @@ -47,7 +47,7 @@ func TestHandleSinceSecretSuccess(t *testing.T) { assert.Equal(t, resp.Patches[1].Path, "/spec/containers/0/volumeMounts") } -func TestHandleSinceSecretNoPatch(t *testing.T) { +func TestHandleInjectSecretNoPatch(t *testing.T) { p := setupSecretsInjector(t) pod := &corev1.Pod{