DEVOPS-30046 DEVOPS-30239 dsnexec conversion injects dbproxy (#381)

A bug in the conversion webhook added dbproxy mutating labels when only dsnexec was requested. This would cause issues if the pod could not handle dbproxy listening on port 5432.
infobloxopen · Dec 18, 2024 · 5d9094f · 5d9094f
1 parent a9d8783
commit 5d9094f
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 46 deletions.
diff --git a/internal/webhook/conversion.go b/internal/webhook/conversion.go
@@ -17,10 +17,13 @@ import (
 )
 
 var (
+	// DSNExec annotations
 	DeprecatedAnnotationDSNExecConfig = "infoblox.com/dsnexec-config-secret"
 	DeprecatedAnnotationRemoteDBDSN   = "infoblox.com/remote-db-dsn-secret"
-	DeprecatedAnnotationDBSecretPath  = "infoblox.com/db-secret-path"
-	DeprecatedAnnotationMessages      = "persistance.atlas.infoblox.com/deprecation-messages"
+	// DBProxy annotations
+	DeprecatedAnnotationDBSecretPath = "infoblox.com/db-secret-path"
+
+	DeprecatedAnnotationMessages = "persistance.atlas.infoblox.com/deprecation-messages"
 )
 
 // +kubebuilder:webhook:path=/convert-deprecated-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=podconversion.persistance.atlas.infoblox.com,sideEffects=None,timeoutSeconds=10,admissionReviewVersions=v1
@@ -94,10 +97,18 @@ func (p *podConverter) Handle(ctx context.Context, req admission.Request) admiss
 	}
 
 	// Check if any of the deprecated annotations are present
+	// dsnexec
 	dsnExecConfigSecret := pod.Annotations[DeprecatedAnnotationDSNExecConfig]
 	remoteDBDSNSecret := pod.Annotations[DeprecatedAnnotationRemoteDBDSN]
+	// dbproxy
 	dbSecretPath := pod.Annotations[DeprecatedAnnotationDBSecretPath]
 
+	if pod.Labels[LabelCheckExec] == "enabled" || pod.Labels[LabelCheckProxy] == "enabled" {
+		// This would log on every pod creation in the cluster
+		// log.V(1).Info("Skipped conversion, already converted", "uid", req.UID)
+		return admission.Allowed("Skipped conversion, already converted")
+	}
+
 	if dsnExecConfigSecret == "" && remoteDBDSNSecret == "" && dbSecretPath == "" {
 		// This would log on every pod creation in the cluster
 		// log.V(1).Info("Skipped conversion, no deprecated annotations found", "uid", req.UID)
@@ -112,7 +123,7 @@ func (p *podConverter) Handle(ctx context.Context, req admission.Request) admiss
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
-	log.Info("converted_pod")
+	log.Info("deprecated_pod_annotations_found")
 	return admission.PatchResponseFromRaw(req.Object.Raw, bs)
 }
 
@@ -139,41 +150,45 @@ func convertPod(ctx context.Context, reader client.Reader, class string, pod *co
 		secretName = dbSecretPath
 	}
 
-	log = log.WithValues("secret", secretName)
+	log = log.WithValues("secret", secretName).WithValues("annotations", pod.Annotations).WithValues("labels", pod.Labels)
+
+	log.Info("converting_pod")
 
 	// db-secret-path has a key in it, so remove the key
 	parts := strings.Split(secretName, "/")
 	if len(parts) > 1 {
 		secretName = parts[0]
 	}
 
-	labelConfigExec := pod.Labels[LabelConfigExec]
-	if labelConfigExec == "" && dsnExecConfigSecret != "" {
-		pod.Labels[LabelConfigExec] = pod.Annotations[DeprecatedAnnotationDSNExecConfig]
-		pod.Labels[LabelCheckExec] = "enabled"
-		deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Label "%s" replaces annotation "%s"`, LabelConfigExec, DeprecatedAnnotationDSNExecConfig))
+	var claimName string
+	var err error
+	if claimName, err = getClaimName(ctx, reader, pod.GetNamespace(), secretName); err != nil {
+		log.Error(err, "unable to find claim")
+		return err
 	}
 
-	// Process claims label
-	if pod.Labels[LabelClaim] == "" {
+	// dsnexec
+	if dsnExecConfigSecret != "" && remoteDBDSNSecret != "" {
+		pod.Labels[LabelClaim] = claimName
+		pod.Labels[LabelClass] = class
+		pod.Labels[LabelConfigExec] = dsnExecConfigSecret
+		pod.Labels[LabelCheckExec] = "enabled"
 
-		if pod.Annotations[DeprecatedAnnotationRemoteDBDSN] != "" {
-			deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Label "%s" replaces annotation "%s"`, LabelClaim, DeprecatedAnnotationRemoteDBDSN))
-		}
-		if pod.Annotations[DeprecatedAnnotationDBSecretPath] != "" {
-			deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Label "%s" replaces annotation "%s"`, LabelClaim, DeprecatedAnnotationDBSecretPath))
-		}
+		deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Use label "%s", annotation "%s" is deprecated`, LabelConfigExec, DeprecatedAnnotationDSNExecConfig))
 
-		var claimName string
-		var err error
-		if claimName, err = getClaimName(ctx, reader, pod.GetNamespace(), secretName); err != nil {
-			log.Error(err, "unable to find claim")
-			return err
+		if pod.Annotations[DeprecatedAnnotationRemoteDBDSN] != "" {
+			deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Use label "%s", annotation "%s" is deprecated`, LabelClaim, DeprecatedAnnotationRemoteDBDSN))
 		}
+	}
 
+	// dbproxy
+	if dbSecretPath != "" {
 		pod.Labels[LabelClaim] = claimName
 		pod.Labels[LabelClass] = class
 		pod.Labels[LabelCheckProxy] = "enabled"
+
+		deprecationMsgs = append(deprecationMsgs, fmt.Sprintf(`Label "%s" replaces annotation "%s"`, LabelClaim, DeprecatedAnnotationDBSecretPath))
+
 	}
 
 	// Remove deprecated annotations

diff --git a/internal/webhook/conversion_test.go b/internal/webhook/conversion_test.go
@@ -96,30 +96,33 @@ var _ = Describe("annotation conversions", func() {
 
 	It("should convert deprecated pod", func() {
 
-		By("Check dbproxy pod is mutated")
+		By("Check deprecated dbproxy annotations are converted to labels")
 		name := "deprecated-dbproxy"
 		pod := makeConvertedPod(name, class, dbcSecretName, "")
 		Expect(pod.Annotations).To(HaveKey(DeprecatedAnnotationMessages))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClaim, dbcName))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelCheckProxy, "enabled"))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClass, class))
+		Expect(pod.Labels).ToNot(HaveKey(LabelConfigExec))
 
-		By("Check dsnexec pod is converted")
+		By("Check dsnexec annotations are converted to labels")
 		name = "deprecated-dsnexec"
 		pod = makeConvertedPod(name, class, dbcSecretName, configSecretName)
 		Expect(pod.Annotations).To(HaveKey(DeprecatedAnnotationMessages))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClaim, dbcName))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelConfigExec, configSecretName))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelCheckExec, "enabled"))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClass, class))
+		Expect(pod.Labels).ToNot(HaveKey(LabelCheckProxy))
 
-		By("Check dbproxy dsnexec combo pod is converted")
+		By("Check dbproxy dsnexec combo annotations are converted to labels")
 		name = "deprecated-both"
 		pod = makeConvertedPod(name, class, dbcSecretName, configSecretName)
 		Expect(pod.Annotations).To(HaveKey(DeprecatedAnnotationMessages))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClaim, dbcName))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelConfigExec, configSecretName))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelCheckProxy, "enabled"))
+		Expect(pod.Labels).To(HaveKeyWithValue(LabelCheckExec, "enabled"))
 		Expect(pod.Labels).To(HaveKeyWithValue(LabelClass, class))
 
 	})
@@ -173,31 +176,28 @@ func makeDeprecatedPod(name, secretName, configSecret string) *corev1.Pod {
 			},
 		},
 	}
+	if pod.Annotations == nil {
+		pod.Annotations = map[string]string{}
+	}
+	if pod.Labels == nil {
+		pod.Labels = map[string]string{}
+	}
 	Expect(secretName).NotTo(BeEmpty())
-
 	switch name {
 	case "deprecated-dbproxy":
-		pod.Annotations = map[string]string{
-			DeprecatedAnnotationDBSecretPath: secretName + "/" + "dsn.txt",
-		}
+		pod.Annotations[DeprecatedAnnotationDBSecretPath] = secretName + "/" + "dsn.txt"
+
 	case "deprecated-both":
-		pod.Annotations = map[string]string{
-			DeprecatedAnnotationDBSecretPath: secretName + "/" + "dsn.txt",
-		}
+		pod.Annotations[DeprecatedAnnotationDBSecretPath] = secretName + "/" + "dsn.txt"
 		fallthrough
 	case "deprecated-dsnexec":
 		Expect(configSecret).NotTo(BeEmpty())
-		pod.Annotations = map[string]string{
-			DeprecatedAnnotationRemoteDBDSN:   secretName,
-			DeprecatedAnnotationDSNExecConfig: configSecret,
-		}
+
+		pod.Annotations[DeprecatedAnnotationRemoteDBDSN] = secretName
+		pod.Annotations[DeprecatedAnnotationDSNExecConfig] = configSecret
 	case "deprecated-converted":
-		pod.Annotations = map[string]string{
-			DeprecatedAnnotationDBSecretPath: secretName + "/" + "dsn.txt",
-		}
-		pod.Labels = map[string]string{
-			LabelConfigExec: "default-db",
-		}
+		pod.Annotations[DeprecatedAnnotationDBSecretPath] = secretName + "/" + "dsn.txt"
+		pod.Labels[LabelConfigExec] = "default-db"
 	case "deprecated-none":
 	}
 

diff --git a/internal/webhook/defaulter.go b/internal/webhook/defaulter.go
@@ -91,12 +91,12 @@ func (p *podAnnotator) Default(ctx context.Context, obj runtime.Object) error {
 	}
 
 	log := logf.FromContext(ctx).WithName("defaulter").WithValues("pod", nn)
-	log.Info("processing")
 
 	if pod.Labels == nil || len(pod.Labels[LabelClaim]) == 0 {
 		log.Info("Skipping Pod")
 		return nil
 	}
+	log.Info("processing")
 
 	claimName := pod.Labels[LabelClaim]
 

diff --git a/scripts/check-sidecars.sh b/scripts/check-sidecars.sh
@@ -0,0 +1,28 @@
+#!/bin/bash -x
+
+
+# Optional namespace argument
+namespace_filter=${1:-}
+
+# Determine the kubectl command based on whether a namespace filter is provided
+if [[ -n "$namespace_filter" ]]; then
+  echo "Filtering by namespace: $namespace_filter"
+  kubectl_command="kubectl -n $namespace_filter get pods -l persistance.atlas.infoblox.com/dbproxy -o=jsonpath='{range .items[*]}{.metadata.namespace} {.metadata.name}{\"\\n\"}{end}'"
+else
+  echo "Checking all namespaces"
+  kubectl_command="kubectl -A get pods -l persistance.atlas.infoblox.com/dbproxy -o=jsonpath='{range .items[*]}{.metadata.namespace} {.metadata.name}{\"\\n\"}{end}'"
+fi
+
+# Get the pods
+pods=$(eval "$kubectl_command")
+
+# Iterate over each pod and run the psql command
+while read -r namespace pod; do
+    echo "Processing pod $pod in namespace $namespace"
+
+     # Execute the SELECT 1 command and capture output
+    kubectl -n "$namespace" -c dbproxy exec "$pod" -- sh -c "psql \$(cat /dbproxy/uri_dsn.txt) -c 'SELECT 1'"
+    kubectl -n "$namespace" -c dbproxy exec "$pod" -- sh -c "cat /run/dbproxy/pgbouncer.ini"
+    kubectl -n "$namespace" -c dbproxy exec "$pod" -- sh -c "cat /run/dbproxy/userlist.txt"
+
+done <<< "$pods"
diff --git a/scripts/psql-open.sh b/scripts/psql-open.sh
@@ -19,8 +19,14 @@ open_psql() {
     secret_name=$(kubectl get databaseclaim "$claim_name" -n "$namespace" -o jsonpath='{.spec.secretName}')
 
     if [[ -z "$secret_name" ]]; then
-        echo "Error: Unable to find secret name for $claim_name in namespace $namespace"
-        return 1
+
+        secret_name=$(kubectl get dbroleclaim "$claim_name" -n "$namespace" -o jsonpath='{.spec.secretName}')
+        if [[ -z "$secret_name" ]]; then
+            echo "Error: Unable to find secret name for dbc: $claim_name in namespace $namespace"
+            echo "Error: Unable to find secret name for dbroleclaim: $claim_name in namespace $namespace"
+            return 1
+        fi
+
     fi
 
     # Get the DSN from the secret
@@ -31,7 +37,7 @@ open_psql() {
         return 1
     fi
 
-    printf "DatabaseClaim: %s/%s\n" "$namespace" "$claim_name"
+    printf "Claim: %s/%s\n" "$namespace" "$claim_name"
     # If a psql command is provided, execute it; otherwise, open a psql prompt
     if [[ -n "$psql_command" ]]; then
         kubectl exec deploy/db-controller -c manager -n db-controller -- psql "$dsn" -c "$psql_command"