PTEUDO-1449 PTEUDO-1385 update mutatingwebhook API to consider dsnexe…

…c and dbproxy (#320)

new field to declare if you want dbproxy, dsnexec or both
    install xnr in a separate group, which is unused in code for now
    add tests for dsnexec webhook

Dockerfile

@@ -17,8 +17,6 @@ COPY api/ api/
 COPY internal/controller/ internal/controller/
 COPY internal/webhook/ internal/webhook/
 COPY pkg/ pkg/
-# FIXME: move this to pkg/
-COPY webhook/ webhook/
 # FIXME: config is for raw manifest yaml that we use in helm, remove this when possible
 COPY config/ config/
 

cmd/main.go

@@ -35,7 +35,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	persistancev1 "github.com/infobloxopen/db-controller/api/v1"
 	"github.com/infobloxopen/db-controller/internal/controller"
@@ -44,7 +43,6 @@ import (
 	"github.com/infobloxopen/db-controller/pkg/databaseclaim"
 	"github.com/infobloxopen/db-controller/pkg/rdsauth"
 	"github.com/infobloxopen/db-controller/pkg/roleclaim"
-	dbwebhook "github.com/infobloxopen/db-controller/webhook"
 
 	persistanceinfobloxcomv1alpha1 "github.com/infobloxopen/db-controller/api/persistance.infoblox.com/v1alpha1"
 	// +kubebuilder:scaffold:imports
@@ -96,7 +94,6 @@ func main() {
 	var metricsDepYamlPath string
 	var metricsConfigYamlPath string
 	var enableDBProxyWebhook bool
-	var enableDSNExecWebhook bool
 
 	flag.StringVar(&class, "class", "default", "The class of claims this db-controller instance needs to address.")
 
@@ -105,7 +102,6 @@ func main() {
 	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(&enableDSNExecWebhook, "enable-dsnexec", false, "Enable Dsnexec webhook. Enabling this option will cause the db-controller to inject dsnexec container into pods with the infoblox.com/remote-db-dsn-secret and infoblox.com/dsnexec-config-secret annotations set.")
 
 	opts := zap.Options{
 		Development: true,
@@ -247,30 +243,12 @@ func main() {
 			Namespace:  namespace,
 			Class:      class,
 			DBProxyImg: os.Getenv("DBPROXY_IMAGE"),
+			DSNExecImg: os.Getenv("DSNEXEC_IMAGE"),
 		}); err != nil {
 			setupLog.Error(err, "failed to setup webhooks")
 			os.Exit(1)
 		}
 	}
-	if enableDSNExecWebhook {
-
-		cfg, err := dbwebhook.ParseConfig(dsnExecSidecarConfigPath)
-
-		if err != nil {
-			setupLog.Error(err, "could not parse dsnexec  sidecar configuration")
-			os.Exit(1)
-		}
-		setupLog.Info("dnsexec-controller", "config", cfg)
-
-		mgr.GetWebhookServer().Register("/mutate-dsnexec", &webhook.Admission{
-			Handler: &dbwebhook.DsnExecInjector{
-				Name:                 "Dsnexec",
-				Client:               mgr.GetClient(),
-				DsnExecSidecarConfig: cfg,
-				Decoder:              admission.NewDecoder(mgr.GetScheme()),
-			},
-		})
-	}
 
 	setupLog.Info("starting manager")
 	if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {

dsnexec/pkg/dsnexec/dsnexec.go

@@ -5,6 +5,7 @@ import (
 	"database/sql"
 	"fmt"
 	"math/bits"
+	"net/url"
 	"strconv"
 	"strings"
 	"sync"
@@ -13,6 +14,7 @@ import (
 	"github.com/Masterminds/sprig/v3"
 	_ "github.com/infobloxopen/db-controller/dsnexec/pkg/shelldb"
 	_ "github.com/lib/pq"
+	log "github.com/sirupsen/logrus"
 )
 
 // Hanlder is an instance of dsnexec.
@@ -85,14 +87,20 @@ func (w *Handler) exec() error {
 
 		parsedOpts, err := parse(source.DSN)
 		if err != nil {
-			return fmt.Errorf("failed to parse dsn: %v", err)
+			return fmt.Errorf("failed to parse dsn: %s", err)
 		}
 		parsedOpts["raw_dsn"] = source.DSN
 		argContext[name] = parsedOpts
 	}
+
+	dsnURL, err := url.Parse(w.config.Destination.DSN)
+	if err == nil {
+		log.Infof("destination dsn: %s", dsnURL.Redacted())
+	}
+
 	db, err := sql.Open(w.config.Destination.Driver, w.config.Destination.DSN)
 	if err != nil {
-		return fmt.Errorf("failed to open destination database: %v", err)
+		return fmt.Errorf("failed to open destination database: %s", err)
 	}
 	defer db.Close()
 
@@ -108,6 +116,7 @@ func (w *Handler) exec() error {
 		cmd := bs.String()
 		if len(v.Args) == 0 {
 			if _, err := db.Exec(cmd); err != nil {
+				log.Infof("failed to execute sql command: %s err: %s", cmd, err)
 				return fmt.Errorf("failed to execute sql: %v", err)
 			}
 			continue

helm/db-controller/templates/_helpers.tpl

@@ -60,3 +60,10 @@ Create the name of the service account to use
 {{- default "default" .Values.serviceAccount.name }}
 {{- end }}
 {{- end }}
+
+{{- define "db-controller.group" -}}
+{{- if not (eq .Values.dbController.class "default") -}}
+    {{- .Values.dbController.class -}}.
+{{- end -}}
+persistance.infoblox.com
+{{- end }}

helm/db-controller/templates/deployment.yaml

@@ -52,10 +52,8 @@ spec:
             - --health-probe-bind-address=:{{ .Values.healthProbe.port }}
             - --leader-elect
             - --enable-db-proxy={{ .Values.dbproxy.enabled }}
-            - --enable-dsnexec={{ .Values.dsnexec.enabled }}
             - --config-file=/etc/config/config.yaml
             - --dsnexec-sidecar-config-path=config/dsnexec/dsnexecsidecar.json
-            - --db-identifier-prefix={{ tpl .Values.db.identifier.prefix . }}
             - --class={{ .Values.dbController.class }}
             - -zap-encoder={{ .Values.zapLogger.encoding }}
             - -zap-log-level={{ .Values.zapLogger.level }}

helm/db-controller/templates/mutatingwebhookconfiguration.yaml

@@ -1,12 +1,11 @@
-{{- if or ( .Values.dbproxy.enabled ) ( .Values.dsnexec.enabled ) }}
+{{- if .Values.dbproxy.enabled }}
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
 metadata:
   name: {{ include "db-controller.fullname" . }}
   annotations:
     cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ include "db-controller.fullname" . }}-webhook
 webhooks:
-{{- if .Values.dbproxy.enabled }}
 - clientConfig:
     service:
       name: {{ include "db-controller.fullname" . }}
@@ -16,11 +15,16 @@ webhooks:
   sideEffects: None
   admissionReviewVersions: ["v1"]
   failurePolicy: Fail
-  name: persistance.atlas.infoblox.com
+  name: dbproxy.persistance.atlas.infoblox.com
   objectSelector:
     matchExpressions:
-      - key: "persistance.atlas.infoblox.com/databaseclaim"
+      # This will locate a databaseclaim or a dbroleclaim
+      - key: "persistance.atlas.infoblox.com/claim"
         operator: "Exists"
+      - key: "persistance.atlas.infoblox.com/dbproxy"
+        operator: "In"
+        values:
+          - "enabled"
         # Important to prevent multiple db-controllers from stepping on each other
       - key: "persistance.atlas.infoblox.com/class"
         operator: "In"
@@ -37,25 +41,30 @@ webhooks:
     resources:
     - pods
     scope: "Namespaced"
-{{- end }}
-{{- if .Values.dsnexec.enabled }}
 - clientConfig:
     service:
       name: {{ include "db-controller.fullname" . }}
-      path: /mutate-dsnexec
+      path: /mutate--v1-pod
       port: 9443
       namespace: {{ .Release.Namespace }}
   sideEffects: None
   admissionReviewVersions: ["v1"]
-  failurePolicy: Ignore
-  name: dsnexec-injector.infoblox.com
+  failurePolicy: Fail
+  name: dsnexec.persistance.atlas.infoblox.com
   objectSelector:
     matchExpressions:
-      # Disable dsn-exec until we reimplement it in #PTEUDO-1385
-      - key: "donotenablethiswithoutwritingtests"
-        Operator: "In"
+      # This will locate a databaseclaim or a dbroleclaim
+      - key: "persistance.atlas.infoblox.com/claim"
+        operator: "Exists"
+      - key: "persistance.atlas.infoblox.com/dsnexec"
+        operator: "In"
         values:
-          - "mytestsareallpassing"
+          - "enabled"
+        # Important to prevent multiple db-controllers from stepping on each other
+      - key: "persistance.atlas.infoblox.com/class"
+        operator: "In"
+        values:
+          - {{ .Values.dbController.class | quote }}
   rules:
   - apiGroups:
     - ""
@@ -68,4 +77,3 @@ webhooks:
     - pods
     scope: "Namespaced"
 {{- end }}
-{{- end }}

helm/db-controller/templates/test/dbproxy.yaml

@@ -118,7 +118,7 @@ metadata:
 spec:
   containers:
     - name: postgres
-      image: postgres:15
+      image: {{ .Values.tools.postgres.repository }}:{{ .Values.tools.postgres.tag }}
       env:
         - name: POSTGRES_USER
           value: "myuser"
@@ -130,12 +130,13 @@ spec:
 apiVersion: v1
 kind: Pod
 metadata:
-  name: {{ .Release.Name }}-dbproxy-test-proxy
+  name: {{ .Release.Name }}-dbproxy-test
   namespace: {{ .Release.Namespace }}
   labels:
     {{- include "db-controller.labels" . | nindent 4 }}
-    persistance.atlas.infoblox.com/databaseclaim: {{ .Release.Name }}-dbproxy-test
+    persistance.atlas.infoblox.com/claim: {{ .Release.Name }}-dbproxy-test
     persistance.atlas.infoblox.com/class: {{ .Values.dbController.class | quote }}
+    persistance.atlas.infoblox.com/dbproxy: enabled
   annotations:
     helm.sh/hook: test
     helm.sh/hook-delete-policy: "before-hook-creation,hook-succeeded"