feat!: support setting the username for the relational database (#891)

* Support setting the username for the relational database

fixes #889

* update crd+documentation

internal/resources/datastore/datastore_storage_config.go

@@ -6,7 +6,6 @@ package datastore
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
@@ -125,19 +124,6 @@ func (r *Config) mutate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.
 		default:
 			password = []byte(uuid.New().String())
 		}
-		// the coalesce function prioritizes the return value stored in the TenantControlPlane status,
-		// although this is going to be populated by the UpdateTenantControlPlaneStatus handler of the resource datastore-setup:
-		// the default value will be used for fresh new configurations, and preserving a previous one:
-		// this will keep us safe from naming changes cases as occurred with the following commit:
-		// https://github.com/clastix/kamaji/pull/203/commits/09ce38f489cccca72ab728a259bc8fb2cf6e4770
-		coalesceFn := func(fromStatus string) []byte {
-			if len(fromStatus) > 0 {
-				return []byte(fromStatus)
-			}
-			// The dash character (-) must be replaced with an underscore, PostgreSQL is complaining about it:
-			// https://github.com/clastix/kamaji/issues/328
-			return []byte(strings.ReplaceAll(fmt.Sprintf("%s_%s", tenantControlPlane.GetNamespace(), tenantControlPlane.GetName()), "-", "_"))
-		}
 
 		finalizersList := sets.New[string](r.resource.GetFinalizers()...)
 		finalizersList.Insert(finalizers.DatastoreSecretFinalizer)
@@ -161,7 +147,25 @@ func (r *Config) mutate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.
 			username = u
 			password = p
 		} else {
-			username = coalesceFn(tenantControlPlane.Status.Storage.Setup.User)
+			// prioritize the username stored in the TenantControlPlane status,
+			// although this is going to be populated by the UpdateTenantControlPlaneStatus handler of the resource datastore-setup:
+			// the default value will be used for fresh new configurations, and preserving a previous one:
+			// this will keep us safe from naming changes cases as occurred with the following commit:
+			// https://github.com/clastix/kamaji/pull/203/commits/09ce38f489cccca72ab728a259bc8fb2cf6e4770
+			switch {
+			case len(tenantControlPlane.Status.Storage.Setup.User) > 0:
+				// for existing TCPs, the dataStoreSchema will be adopted from the status,
+				// as the mutating webhook only takes care of TCP creations, not updates
+				username = []byte(tenantControlPlane.Status.Storage.Setup.User)
+				tenantControlPlane.Spec.DataStoreUsername = string(username)
+			case len(tenantControlPlane.Spec.DataStoreUsername) > 0:
+				// for new TCPs, the spec field will have been provided by the user
+				// or defaulted by the defaulting webhook
+				username = []byte(tenantControlPlane.Spec.DataStoreUsername)
+			default:
+				// this can only happen on TCP creations when the webhook is not installed
+				return fmt.Errorf("cannot build datastore storage config, username must either exist in Spec or Status")
+			}
 		}
 
 		var dataStoreSchema string

internal/resources/datastore/datastore_storage_config_test.go

@@ -60,16 +60,17 @@ var _ = Describe("DatastoreStorageConfig", func() {
 		}
 	})
 
-	When("TCP has no dataStoreSchema defined", func() {
+	When("TCP has neither dataStoreSchema nor dataStoreUsername defined", func() {
 		It("should return an error", func() {
 			_, err := resources.Handle(ctx, dsc, tcp)
 			Expect(err).To(HaveOccurred())
 		})
 	})
 
-	When("TCP has dataStoreSchema set in spec", func() {
+	When("TCP has dataStoreSchema and dataStoreUsername set in spec", func() {
 		BeforeEach(func() {
 			tcp.Spec.DataStoreSchema = "custom-prefix"
+			tcp.Spec.DataStoreUsername = "custom-user"
 		})
 
 		It("should create the datastore secret with the schema name from the spec", func() {
@@ -81,10 +82,11 @@ var _ = Describe("DatastoreStorageConfig", func() {
 			Expect(fakeClient.List(ctx, secrets)).To(Succeed())
 			Expect(secrets.Items).To(HaveLen(1))
 			Expect(secrets.Items[0].Data["DB_SCHEMA"]).To(Equal([]byte("custom-prefix")))
+			Expect(secrets.Items[0].Data["DB_USER"]).To(Equal([]byte("custom-user")))
 		})
 	})
 
-	When("TCP has dataStoreSchema set in status, but not in spec", func() {
+	When("TCP has dataStoreSchema and dataStoreUsername set in status, but not in spec", func() {
 		// this test case ensures that existing TCPs (created in a CRD version without
 		// the dataStoreSchema field) correctly adopt the spec field from the status.
 
@@ -92,6 +94,7 @@ var _ = Describe("DatastoreStorageConfig", func() {
 			By("updating the TCP status")
 			Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(tcp), tcp)).To(Succeed())
 			tcp.Status.Storage.Setup.Schema = "existing-schema-name"
+			tcp.Status.Storage.Setup.User = "existing-username"
 			Expect(fakeClient.Status().Update(ctx, tcp)).To(Succeed())
 
 			By("handling the resource")
@@ -104,12 +107,14 @@ var _ = Describe("DatastoreStorageConfig", func() {
 			Expect(fakeClient.List(ctx, secrets)).To(Succeed())
 			Expect(secrets.Items).To(HaveLen(1))
 			Expect(secrets.Items[0].Data["DB_SCHEMA"]).To(Equal([]byte("existing-schema-name")))
+			Expect(secrets.Items[0].Data["DB_USER"]).To(Equal([]byte("existing-username")))
 
 			By("checking the TCP spec")
 			// we have to check the modified struct here (instead of retrieving the object
 			// via the fakeClient), as the TCP resource update is not done by the resources.
 			// Instead, the TCP controller will handle TCP updates after handling all resources
 			tcp.Spec.DataStoreSchema = "existing-schema-name"
+			tcp.Spec.DataStoreUsername = "existing-username"
 		})
 	})
 })

internal/webhook/handlers/tcp_defaults.go

@@ -76,4 +76,11 @@ func (t TenantControlPlaneDefaults) defaultUnsetFields(tcp *kamajiv1alpha1.Tenan
 		dss := strings.ReplaceAll(fmt.Sprintf("%s_%s", tcp.GetNamespace(), tcp.GetName()), "-", "_")
 		tcp.Spec.DataStoreSchema = dss
 	}
+
+	if len(tcp.Spec.DataStoreUsername) == 0 {
+		// The dash character (-) must be replaced with an underscore, PostgreSQL is complaining about it:
+		// https://github.com/clastix/kamaji/issues/328
+		username := strings.ReplaceAll(fmt.Sprintf("%s_%s", tcp.GetNamespace(), tcp.GetName()), "-", "_")
+		tcp.Spec.DataStoreUsername = username
+	}
 }

internal/webhook/handlers/tcp_defaults_test.go

@@ -49,7 +49,7 @@ var _ = Describe("TCP Defaulting Webhook", func() {
 		It("should issue all required patches", func() {
 			ops, err := t.OnCreate(tcp)(ctx, admission.Request{})
 			Expect(err).ToNot(HaveOccurred())
-			Expect(ops).To(HaveLen(3))
+			Expect(ops).To(HaveLen(4))
 		})
 
 		It("should default the dataStore", func() {
@@ -60,12 +60,15 @@ var _ = Describe("TCP Defaulting Webhook", func() {
 			))
 		})
 
-		It("should default the dataStoreSchema to the expected value", func() {
+		It("should default the dataStoreSchema and dataStoreUsername to the expected value", func() {
 			ops, err := t.OnCreate(tcp)(ctx, admission.Request{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(ops).To(ContainElement(
 				jsonpatch.Operation{Operation: "add", Path: "/spec/dataStoreSchema", Value: "default_tcp"},
 			))
+			Expect(ops).To(ContainElement(
+				jsonpatch.Operation{Operation: "add", Path: "/spec/dataStoreUsername", Value: "default_tcp"},
+			))
 		})
 	})
 
@@ -73,6 +76,7 @@ var _ = Describe("TCP Defaulting Webhook", func() {
 		BeforeEach(func() {
 			tcp.Spec.DataStore = "etcd"
 			tcp.Spec.DataStoreSchema = "my_tcp"
+			tcp.Spec.DataStoreUsername = "my_tcp"
 			tcp.Spec.ControlPlane.Deployment.Replicas = ptr.To(int32(2))
 		})