From 5d9094f476cb969e883b97e861f3d9587f7e0a66 Mon Sep 17 00:00:00 2001 From: Drew Wells Date: Wed, 18 Dec 2024 07:37:13 -0600 Subject: [PATCH] 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. --- internal/webhook/conversion.go | 59 ++++++++++++++++++----------- internal/webhook/conversion_test.go | 40 +++++++++---------- internal/webhook/defaulter.go | 2 +- scripts/check-sidecars.sh | 28 ++++++++++++++ scripts/psql-open.sh | 12 ++++-- 5 files changed, 95 insertions(+), 46 deletions(-) create mode 100755 scripts/check-sidecars.sh diff --git a/internal/webhook/conversion.go b/internal/webhook/conversion.go index dcfc795e..05c3283e 100644 --- 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,7 +150,9 @@ 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, "/") @@ -147,33 +160,35 @@ func convertPod(ctx context.Context, reader client.Reader, class string, pod *co 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 index c0007403..0abc393a 100644 --- a/internal/webhook/conversion_test.go +++ b/internal/webhook/conversion_test.go @@ -96,15 +96,16 @@ 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)) @@ -112,14 +113,16 @@ var _ = Describe("annotation conversions", func() { 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 index 34bb7b57..9ae70c81 100644 --- 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 new file mode 100755 index 00000000..30b0fe3c --- /dev/null +++ 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 index bbd41929..cb0cd7b4 100755 --- 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"