PTEUDO-1652 convert deprecated annotations (#341)

As part of 1.10 a number of changes were made to mutating webhooks
    for better resiliency of the k8s clusters. One drawback is a
    number of apps would no longer get sidecars.

    This introduces the same dangerous webhook as before but only to
    convert the pods from old annotation syntax to new label syntax.
    Removes redundant webhook tests from controller, it's too difficult
    standing up a manager to bind the mutating webhooks to.

cmd/main.go

@@ -94,14 +94,16 @@ func main() {
 	var metricsDepYamlPath string
 	var metricsConfigYamlPath string
 	var enableDBProxyWebhook bool
+	var enableDeprecatedConversionWebhook bool
 
 	flag.StringVar(&class, "class", "default", "The class of claims this db-controller instance needs to address.")
 
 	flag.StringVar(&configFile, "config-file", "/etc/config/config.yaml", "Database connection string to with root credentials.")
 	flag.StringVar(&dsnExecSidecarConfigPath, "dsnexec-sidecar-config-path", "/etc/config/dsnexec/dsnexecsidecar.json", "Mutating webhook sidecar configuration.")
 	flag.StringVar(&metricsDepYamlPath, "metrics-dep-yaml", "/config/postgres-exporter/deployment.yaml", "path to the metrics deployment yaml")
 	flag.StringVar(&metricsConfigYamlPath, "metrics-config-yaml", "/config/postgres-exporter/config.yaml", "path to the metrics config yaml")
-	flag.BoolVar(&enableDBProxyWebhook, "enable-db-proxy", false, "Enable DB Proxy webhook. Enabling this option will cause the db-controller to inject db proxy pod into pods with the infoblox.com/db-secret-path annotation set.")
+	flag.BoolVar(&enableDBProxyWebhook, "enable-db-proxy", false, "Enable DB Proxy webhook. See docs for usage: https://infobloxopen.github.io/db-controller/#quick-start")
+	flag.BoolVar(&enableDBProxyWebhook, "enable-deprecation-conversion-webhook", false, "Enable conversion of deprecated pods using dbproxy and/or dsnexec annotations")
 
 	opts := zap.Options{
 		Development: true,
@@ -253,6 +255,17 @@ func main() {
 			setupLog.Error(err, "failed to setup webhooks")
 			os.Exit(1)
 		}
+
+		if enableDeprecatedConversionWebhook {
+
+			if err := mutating.SetupConversionWebhookWithManager(mgr, mutating.SetupConfig{
+				Namespace: namespace,
+				Class:     class,
+			}); err != nil {
+				setupLog.Error(err, "failed to setup conversion webhooks")
+				os.Exit(1)
+			}
+		}
 	}
 
 	setupLog.Info("starting manager")

config/webhook/manifests.yaml

@@ -12,7 +12,8 @@ webhooks:
       namespace: system
       path: /mutate--v1-pod
   failurePolicy: Fail
-  name: persistance.atlas.infoblox.com
+  name: dbproxy.persistance.atlas.infoblox.com
+  reinvocationPolicy: IfNeeded
   rules:
   - apiGroups:
     - ""
@@ -24,3 +25,47 @@ webhooks:
     resources:
     - pods
   sideEffects: None
+  timeoutSeconds: 10
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate--v1-pod
+  failurePolicy: Fail
+  name: dsnexec.persistance.atlas.infoblox.com
+  reinvocationPolicy: IfNeeded
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - pods
+  sideEffects: None
+  timeoutSeconds: 10
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /convert-deprecated-pod
+  failurePolicy: Fail
+  name: podconversion.persistance.atlas.infoblox.com
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - pods
+  sideEffects: None
+  timeoutSeconds: 10

helm/db-controller/templates/deployment.yaml

@@ -49,6 +49,7 @@ spec:
             - --health-probe-bind-address=:{{ .Values.healthProbe.port }}
             - --leader-elect
             - --enable-db-proxy={{ .Values.dbproxy.enabled }}
+            - --enable-deprecation-conversion-webhook={{ .Values.deprecationConversionWebhook.enabled }}
             - --config-file=/etc/config/config.yaml
             - --dsnexec-sidecar-config-path=config/dsnexec/dsnexecsidecar.json
             - --class={{ .Values.dbController.class }}

helm/db-controller/templates/mutatingwebhookconfiguration.yaml

@@ -9,13 +9,41 @@ webhooks:
 - clientConfig:
     service:
       name: {{ include "db-controller.fullname" . }}
-      path: /mutate--v1-pod
+      path: /convert-deprecated-pod
       port: 9443
       namespace: {{ .Release.Namespace }}
+  admissionReviewVersions: ["v1"]
+  # Support existing pods that are not using labels to
+  # indicate db-controller sidecar mutations are needed.
+  # Use an ignore failurepolicy because this intercepts
+  # all pod mutations in the cluster.
+  failurePolicy: Ignore
+  name: podconversion.persistance.atlas.infoblox.com
+  reinvocationPolicy: Never
+  objectSelector: {}
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - pods
+    scope: "Namespaced"
   sideEffects: None
+  timeoutSeconds: 10
+- clientConfig:
+    service:
+      name: {{ include "db-controller.fullname" . }}
+      path: /mutate--v1-pod
+      port: 9443
+      namespace: {{ .Release.Namespace }}
   admissionReviewVersions: ["v1"]
   failurePolicy: Fail
   name: dbproxy.persistance.atlas.infoblox.com
+  reinvocationPolicy: IfNeeded
   objectSelector:
     matchExpressions:
       # This will locate a databaseclaim or a dbroleclaim
@@ -41,6 +69,8 @@ webhooks:
     resources:
     - pods
     scope: "Namespaced"
+  sideEffects: None
+  timeoutSeconds: 10
 - clientConfig:
     service:
       name: {{ include "db-controller.fullname" . }}
@@ -49,8 +79,10 @@ webhooks:
       namespace: {{ .Release.Namespace }}
   sideEffects: None
   admissionReviewVersions: ["v1"]
+  timeoutSeconds: 10
   failurePolicy: Fail
   name: dsnexec.persistance.atlas.infoblox.com
+  reinvocationPolicy: IfNeeded
   objectSelector:
     matchExpressions:
       # This will locate a databaseclaim or a dbroleclaim

helm/db-controller/values.yaml

@@ -122,6 +122,9 @@ dsnexec:
     repository: "ghcr.io/infobloxopen/dsnexec"
     tag: ""
 
+deprecationConversionWebhook:
+  enabled: false
+
 zapLogger:
   encoding: json
   level: info

internal/controller/databaseclaim_controller_test.go

@@ -18,8 +18,6 @@ package controller
 
 import (
 	"context"
-	"crypto/tls"
-	"errors"
 	"fmt"
 	"net/url"
 
@@ -32,12 +30,9 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	persistancev1 "github.com/infobloxopen/db-controller/api/v1"
-	mutating "github.com/infobloxopen/db-controller/internal/webhook"
 	"github.com/infobloxopen/db-controller/pkg/hostparams"
 )
 
@@ -196,71 +191,5 @@ var _ = Describe("DatabaseClaim Controller", func() {
 
 		})
 
-		It("dbproxy mutated pod", func() {
-
-			class := "testenv"
-			// FIXME: can this be done without standing up
-			// a full manager?
-			mgr, err := manager.New(cfg, manager.Options{
-				Scheme: k8sClient.Scheme(),
-				WebhookServer: webhook.NewServer(webhook.Options{
-					Port:    testEnv.WebhookInstallOptions.LocalServingPort,
-					Host:    testEnv.WebhookInstallOptions.LocalServingHost,
-					CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir,
-					TLSOpts: []func(*tls.Config){func(config *tls.Config) {}},
-				}),
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			Expect(mutating.SetupWebhookWithManager(mgr, mutating.SetupConfig{
-				Namespace:  namespace,
-				Class:      class,
-				DBProxyImg: "test-db-proxy:latest",
-				DSNExecImg: "test-dsn-exec:latest",
-			})).To(Succeed())
-
-			mgrCtx, cancel := context.WithCancel(context.Background())
-			go func() {
-				Expect(mgr.Start(mgrCtx)).To(Succeed())
-			}()
-			DeferCleanup(func() {
-				cancel()
-				<-mgrCtx.Done()
-				Expect(errors.Is(mgrCtx.Err(), context.Canceled)).To(BeTrue())
-			})
-
-			pod := corev1.Pod{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-pod",
-					Namespace: "default",
-					Labels: map[string]string{
-						mutating.LabelCheckProxy: "enabled",
-						mutating.LabelClaim:      resourceName,
-						mutating.LabelClass:      class,
-					},
-				},
-				Spec: corev1.PodSpec{
-					Containers: []corev1.Container{
-						{
-							Name:  "test-container",
-							Image: "busybox",
-							Command: []string{
-								"sleep",
-								"3600",
-							},
-						},
-					},
-				},
-			}
-			Expect(k8sClient.Create(ctx, &pod)).To(Succeed())
-			Expect(pod.Spec.Volumes).To(HaveLen(1))
-			Expect(pod.Spec.Volumes[0].VolumeSource.Secret.SecretName).To(Equal(secretName))
-			Expect(pod.Annotations[mutating.AnnotationInjectedProxy]).To(Equal("true"))
-			Expect(pod.Spec.Containers).To(HaveLen(2))
-			Expect(pod.Spec.Containers[1].Env).To(HaveLen(1))
-			envvar := pod.Spec.Containers[1].Env[0]
-			Expect(envvar.Name).To(Equal("DBPROXY_CREDENTIAL"))
-			Expect(envvar.Value).To(Equal("/dbproxy/uri_dsn.txt"))
-		})
 	})
 })

internal/controller/suite_test.go

@@ -57,6 +57,7 @@ var controllerReconciler *DatabaseClaimReconciler
 var namespace string
 var logger logr.Logger
 var env = "testenv"
+var class = "testenv"
 
 // Stand up postgres in a container
 var (
@@ -92,7 +93,7 @@ var _ = BeforeSuite(func() {
 		},
 		ErrorIfCRDPathMissing: true,
 		WebhookInstallOptions: envtest.WebhookInstallOptions{
-			Paths: []string{filepath.Join("testdata", "mutatingwebhook.yaml")},
+			// Paths: []string{filepath.Join("..", "..", "config", "webhook")},
 		},
 		// The BinaryAssetsDirectory is only required if you want to run the tests directly
 		// without call the makefile target test. If not informed it will look for the
@@ -101,6 +102,7 @@ var _ = BeforeSuite(func() {
 		// the tests directly. When we run make test it will be setup and used automatically.
 		BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s",
 			fmt.Sprintf("1.30.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
+		AttachControlPlaneOutput: false,
 	}
 
 	var err error