PTEUDO-2142 remove validation of DSNName (#370)

Previously, customizing the dsnname would throw a validation error
    and prevent any processing of the databaseclaim. This allows the
    customization but ensures the normalized fields are always
    populated.

internal/controller/databaseclaim_controller_test.go

@@ -33,6 +33,8 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	persistancev1 "github.com/infobloxopen/db-controller/api/v1"
+	v1 "github.com/infobloxopen/db-controller/api/v1"
+	"github.com/infobloxopen/db-controller/internal/dockerdb"
 	"github.com/infobloxopen/db-controller/pkg/hostparams"
 )
 
@@ -67,6 +69,25 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			password, ok := parsedDSN.User.Password()
 			Expect(ok).To(BeTrue())
 
+			secret := &corev1.Secret{}
+			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
+			Expect(err).To(HaveOccurred())
+			Expect(client.IgnoreNotFound(err)).To(Succeed())
+
+			By(fmt.Sprintf("creating master credentials: %s", secretName))
+			secret = &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      secretName,
+					Namespace: "default",
+				},
+				StringData: map[string]string{
+					"password": password,
+				},
+				Type: "Opaque",
+			}
+			Expect(k8sClient.Create(ctx, secret)).To(Succeed())
+
+			By("creating master databaseclaims")
 			Expect(client.IgnoreNotFound(err)).To(Succeed())
 			resource := &persistancev1.DatabaseClaim{
 				TypeMeta: metav1.TypeMeta{
@@ -78,6 +99,8 @@ var _ = Describe("DatabaseClaim Controller", func() {
 					Namespace: "default",
 				},
 				Spec: persistancev1.DatabaseClaimSpec{
+					// TODO: remove customization of DSNName
+					DSNName:               "fixme.txt",
 					Class:                 ptr.To(""),
 					DatabaseName:          "sample_app",
 					SecretName:            secretName,
@@ -90,22 +113,14 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			}
 			Expect(k8sClient.Create(ctx, resource)).To(Succeed())
 
-			secret := &corev1.Secret{}
-			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
-			Expect(err).To(HaveOccurred())
-			Expect(client.IgnoreNotFound(err)).To(Succeed())
+			By("Mocking master credentials")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, resource)
+			Expect(err).ToNot(HaveOccurred())
 
-			secret = &corev1.Secret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      secretName,
-					Namespace: "default",
-				},
-				StringData: map[string]string{
-					"password": password,
-				},
-				Type: "Opaque",
-			}
-			Expect(k8sClient.Create(ctx, secret)).To(Succeed())
+			credSecretName := fmt.Sprintf("%s-%s-%s", env, resourceName, hostParams.Hash())
+
+			cleanup := dockerdb.MockRDSCredentials(GinkgoT(), ctx, k8sClient, testDSN, credSecretName)
+			DeferCleanup(cleanup)
 
 		})
 
@@ -146,6 +161,42 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(claim.Status.Error).To(Equal(""))
 		})
 
+		It("Should have DSN and URIDSN keys populated", func() {
+			By("Reconciling the created resource")
+			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			var claim persistancev1.DatabaseClaim
+			err = k8sClient.Get(ctx, typeNamespacedName, &claim)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(claim.Status.Error).To(Equal(""))
+
+			By("Checking the user credentials secret")
+
+			secret := &corev1.Secret{}
+			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
+			Expect(err).NotTo(HaveOccurred())
+
+			for _, key := range []string{v1.DSNKey, v1.DSNURIKey, "fixme.txt", "uri_fixme.txt"} {
+				Expect(secret.Data[key]).NotTo(BeNil())
+			}
+			oldKey := secret.Data[v1.DSNKey]
+			Expect(secret.Data[v1.DSNKey]).To(Equal(secret.Data["fixme.txt"]))
+			Expect(secret.Data[v1.DSNURIKey]).To(Equal(secret.Data["uri_fixme.txt"]))
+			// Slow down the test so creds are rotated, 60ns rotation time
+			By("Rotate passwords and verify credentials are updated")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(secret.Data[v1.DSNKey]).NotTo(Equal(oldKey))
+			Expect(secret.Data[v1.DSNKey]).To(Equal(secret.Data["fixme.txt"]))
+			Expect(secret.Data[v1.DSNURIKey]).To(Equal(secret.Data["uri_fixme.txt"]))
+
+		})
+
 		It("Should succeed with no error status to reconcile CR with DBVersion", func() {
 			By("Updating CR with a DB Version")
 

internal/controller/databasecontroller_migrate_test.go

@@ -36,6 +36,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	persistancev1 "github.com/infobloxopen/db-controller/api/v1"
+	v1 "github.com/infobloxopen/db-controller/api/v1"
 	"github.com/infobloxopen/db-controller/internal/dockerdb"
 	"github.com/infobloxopen/db-controller/pkg/hostparams"
 	"github.com/infobloxopen/db-controller/pkg/pgctl"
@@ -152,7 +153,7 @@ var _ = Describe("claim migrate", func() {
 					Namespace: "default",
 				},
 				StringData: map[string]string{
-					"uri_dsn.txt": testDSN,
+					v1.DSNURIKey: testDSN,
 				},
 				Type: "Opaque",
 			}

internal/dockerdb/mockdb.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"database/sql"
 	"net/url"
-	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 
@@ -23,7 +22,16 @@ func MockRDS(t GinkgoTInterface, ctx context.Context, cli client.Client, secretN
 		DockerTag: "15",
 	})
 
-	fakeDSN = strings.Replace(fakeDSN, "localhost", "127.0.0.1", 1)
+	cleanSecret := MockRDSCredentials(t, ctx, cli, fakeDSN, secretName)
+
+	return dbCli, fakeDSN, func() {
+		cleanSecret()
+		clean()
+	}
+
+}
+
+func MockRDSCredentials(t GinkgoTInterface, ctx context.Context, cli client.Client, fakeDSN, secretName string) func() {
 
 	u, err := url.Parse(fakeDSN)
 	if err != nil {
@@ -47,10 +55,9 @@ func MockRDS(t GinkgoTInterface, ctx context.Context, cli client.Client, secretN
 		t.Fatalf("failed to create secret: %v", err)
 	}
 
-	return dbCli, fakeDSN, func() {
+	return func() {
 		if err := cli.Delete(ctx, secret); err != nil {
 			t.Logf("failed to delete secret: %v", err)
 		}
-		clean()
 	}
 }

internal/dockerdb/testdb.go

@@ -282,6 +282,8 @@ CREATE ROLE alloydbsuperuser WITH INHERIT LOGIN`)
 		logger.Info(string(out))
 		os.Exit(1)
 	}
+
+	dsn = strings.Replace(dsn, "localhost", "127.0.0.1", 1)
 	// TODO: change this to debug logging, just timing jenkins for now
 	logger.Info("db_connected", "dsn", dsn, "duration", time.Since(now))
 

pkg/databaseclaim/databaseclaim.go

@@ -19,7 +19,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	v1 "github.com/infobloxopen/db-controller/api/v1"
 	"github.com/infobloxopen/db-controller/pkg/auth"
@@ -146,12 +145,9 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	if err := validateDBClaim(&dbClaim); err != nil {
-		res, err := r.manageError(ctx, &dbClaim, err)
-		if dbClaim.Status.Error != "" {
-			metrics.ErrorStateClaims.Inc()
-		}
-		// TerminalError, do not requeue
-		return res, reconcile.TerminalError(err)
+		// Validation is weak until all apps are moved to new API
+		logr.Error(err, "dbclaim_failed_validation")
+		// FIXME: mark the claim status as error
 	}
 
 	if dbClaim.Spec.Class == nil {

pkg/databaseclaim/secrets.go

@@ -52,7 +52,7 @@ func (r *DatabaseClaimReconciler) createOrUpdateSecret(ctx context.Context, dbCl
 		return nil
 	}
 
-	return r.updateSecret(ctx, dsn, dsnURI, replicaDsnURI, connInfo, gs)
+	return r.updateSecret(ctx, dbClaim, dsn, dsnURI, replicaDsnURI, connInfo, gs)
 }
 
 func (r *DatabaseClaimReconciler) createSecret(ctx context.Context, dbClaim *v1.DatabaseClaim, dsn, dbURI, replicaDbURI string, connInfo *v1.DatabaseClaimConnectionInfo) error {
@@ -88,12 +88,17 @@ func (r *DatabaseClaimReconciler) createSecret(ctx context.Context, dbClaim *v1.
 			"sslmode":           []byte(connInfo.SSLMode),
 		},
 	}
+	if dbClaim.Spec.DSNName != "" && dbClaim.Spec.DSNName != v1.DSNKey {
+		secret.Data[dbClaim.Spec.DSNName] = []byte(dsn)
+		secret.Data["uri_"+dbClaim.Spec.DSNName] = []byte(dbURI)
+	}
+
 	logr.Info("creating connection info SECRET: "+secret.Name, "secret", secret.Name, "namespace", secret.Namespace)
 
 	return r.Client.Create(ctx, secret)
 }
 
-func (r *DatabaseClaimReconciler) updateSecret(ctx context.Context, dsn, dbURI, replicaDsnURI string, connInfo *v1.DatabaseClaimConnectionInfo, exSecret *corev1.Secret) error {
+func (r *DatabaseClaimReconciler) updateSecret(ctx context.Context, dbClaim *v1.DatabaseClaim, dsn, dbURI, replicaDsnURI string, connInfo *v1.DatabaseClaimConnectionInfo, exSecret *corev1.Secret) error {
 
 	logr := log.FromContext(ctx)
 
@@ -108,6 +113,11 @@ func (r *DatabaseClaimReconciler) updateSecret(ctx context.Context, dsn, dbURI,
 	exSecret.Data["sslmode"] = []byte(connInfo.SSLMode)
 	logr.Info("updating connection info SECRET: "+exSecret.Name, "secret", exSecret.Name, "namespace", exSecret.Namespace)
 
+	if dbClaim.Spec.DSNName != "" && dbClaim.Spec.DSNName != v1.DSNKey {
+		exSecret.Data[dbClaim.Spec.DSNName] = []byte(dsn)
+		exSecret.Data["uri_"+dbClaim.Spec.DSNName] = []byte(dbURI)
+	}
+
 	return r.Client.Update(ctx, exSecret)
 }
 

pkg/databaseclaim/secrets_test.go

@@ -87,7 +87,19 @@ func TestUpdateSecret(t *testing.T) {
 		Data: make(map[string][]byte),
 	}
 
-	err := dbClaimReconciler.updateSecret(ctx, "dsn", "dsnUri", "ro_dsnUri", &claimConnInfo, &secret)
+	dbClaim := v1.DatabaseClaim{
+		TypeMeta: metav1.TypeMeta{},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "dbClaim",
+			Namespace: "testNamespace",
+		},
+		Spec: v1.DatabaseClaimSpec{
+			SecretName: "create-master-secret",
+		},
+		Status: v1.DatabaseClaimStatus{},
+	}
+
+	err := dbClaimReconciler.updateSecret(ctx, &dbClaim, "dsn", "dsnUri", "ro_dsnUri", &claimConnInfo, &secret)
 
 	Expect(secret.Data[v1.DSNKey]).To(Equal([]byte("dsn")))
 	Expect(secret.Data[v1.DSNURIKey]).To(Equal([]byte("dsnUri")))