Enhance database environment variable injection in Deployment

apps/operator/internal/controller/database/injection.go

@@ -2,30 +2,52 @@ package database
 
 import (
 	"strconv"
+	"strings"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 )
 
-const dbPortEnvName = "DB_PORT"
+const (
+	dbPortEnvName      = "DB_PORT"
+	dbNameEnvName      = "DB_NAME"
+	databaseURLEnvName = "DATABASE_URL"
+	// postgresDatabaseURLTemplate uses Kubernetes $(VAR) expansion from earlier env entries.
+	postgresDatabaseURLTemplate = "postgres://$(DB_USER):$(DB_PASS)@$(DB_HOST):$(DB_PORT)/$(DB_NAME)"
+)
+
+// connectionURLTemplateForDBType returns a DATABASE_URL value for the given engine type.
+// Only types with a defined URL layout are supported; others return ok=false.
+func connectionURLTemplateForDBType(dbType string) (template string, ok bool) {
+	switch strings.ToLower(strings.TrimSpace(dbType)) {
+	case "postgres", "postgresql":
+		return postgresDatabaseURLTemplate, true
+	default:
+		return "", false
+	}
+}
 
 // databaseSecretEnvVarNames lists env vars resolved from Secret keys.
 var databaseSecretEnvVarNames = []string{"DB_HOST", "DB_USER", "DB_PASS"}
 
 // InjectDatabaseEnvVars patches a Deployment's first container to include
 // DB_HOST, DB_USER, DB_PASS env vars referencing the given K8s Secret.
 // The function is idempotent.
-func InjectDatabaseEnvVars(deploy *appsv1.Deployment, secretName string) bool {
-	changed, _ := InjectDatabaseEnvVarsForContainer(deploy, secretName, "", DefaultPostgresPort)
+// dbName is the logical database name (e.g. from the database trait); when empty,
+// DB_NAME is not injected.
+// dbType selects whether DATABASE_URL is set (only types with a known URL template, e.g. postgres).
+func InjectDatabaseEnvVars(deploy *appsv1.Deployment, secretName, dbName, dbType string) bool {
+	changed, _ := InjectDatabaseEnvVarsForContainer(deploy, secretName, "", DefaultPostgresPort, dbName, dbType)
 	return changed
 }
 
 // InjectDatabaseEnvVarsForContainer patches a Deployment container to include
 // DB_HOST, DB_USER, DB_PASS env vars referencing the given K8s Secret, plus a
-// literal DB_PORT value.
+// literal DB_PORT value. When dbName is non-empty, it sets DB_NAME. DATABASE_URL
+// is added only when dbType has a defined connection string template (see connectionURLTemplateForDBType).
 // If preferredContainerName is not found, it falls back to the first container.
 // Returns (changed, exactMatch).
-func InjectDatabaseEnvVarsForContainer(deploy *appsv1.Deployment, secretName, preferredContainerName string, dbPort int32) (bool, bool) {
+func InjectDatabaseEnvVarsForContainer(deploy *appsv1.Deployment, secretName, preferredContainerName string, dbPort int32, dbName, dbType string) (bool, bool) {
 	if len(deploy.Spec.Template.Spec.Containers) == 0 {
 		return false, false
 	}
@@ -99,9 +121,39 @@ func InjectDatabaseEnvVarsForContainer(deploy *appsv1.Deployment, secretName, pr
 		changed = true
 	}
 
+	if dbName != "" {
+		if ensureLiteralEnvVar(container, dbNameEnvName, dbName) {
+			changed = true
+		}
+		if urlTpl, ok := connectionURLTemplateForDBType(dbType); ok {
+			if ensureLiteralEnvVar(container, databaseURLEnvName, urlTpl) {
+				changed = true
+			}
+		}
+	}
+
 	return changed, exactMatch
 }
 
+func ensureLiteralEnvVar(container *corev1.Container, name, value string) bool {
+	for i := range container.Env {
+		if container.Env[i].Name != name {
+			continue
+		}
+		if container.Env[i].Value == value && container.Env[i].ValueFrom == nil {
+			return false
+		}
+		container.Env[i].Value = value
+		container.Env[i].ValueFrom = nil
+		return true
+	}
+	container.Env = append(container.Env, corev1.EnvVar{
+		Name:  name,
+		Value: value,
+	})
+	return true
+}
+
 func selectTargetContainerIndex(containers []corev1.Container, preferredContainerName string) (int, bool) {
 	if len(containers) == 0 {
 		return -1, false

apps/operator/internal/controller/database/injection_test.go

@@ -7,6 +7,37 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+func TestConnectionURLTemplateForDBType(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		dbType string
+		ok     bool
+	}{
+		{"postgres", true},
+		{"POSTGRES", true},
+		{"postgresql", true},
+		{" PostgreSQL ", true},
+		{"mysql", false},
+		{"", false},
+	}
+	for _, tt := range tests {
+		name := tt.dbType
+		if name == "" {
+			name = "empty"
+		}
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			got, ok := connectionURLTemplateForDBType(tt.dbType)
+			if ok != tt.ok {
+				t.Fatalf("ok: got %v, want %v (template %q)", ok, tt.ok, got)
+			}
+			if tt.ok && got != postgresDatabaseURLTemplate {
+				t.Fatalf("template: got %q, want %q", got, postgresDatabaseURLTemplate)
+			}
+		})
+	}
+}
+
 func TestInjectDatabaseEnvVars(t *testing.T) {
 	t.Run("InjectsAllEnvVars", func(t *testing.T) {
 		deploy := &appsv1.Deployment{
@@ -27,14 +58,14 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret")
+		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret", "api-server-db", "postgres")
 		if !changed {
 			t.Fatal("Expected InjectDatabaseEnvVars to return true (changed)")
 		}
 
 		container := deploy.Spec.Template.Spec.Containers[0]
-		if len(container.Env) != 5 {
-			t.Fatalf("Expected 5 env vars, got %d", len(container.Env))
+		if len(container.Env) != 7 {
+			t.Fatalf("Expected 7 env vars, got %d", len(container.Env))
 		}
 
 		expectedEnvs := map[string]string{
@@ -68,6 +99,16 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 				}
 				foundDBPort = true
 			}
+			if env.Name == "DB_NAME" {
+				if env.Value != "api-server-db" || env.ValueFrom != nil {
+					t.Errorf("Expected DB_NAME literal api-server-db, got %+v", env)
+				}
+			}
+			if env.Name == "DATABASE_URL" {
+				if env.Value != postgresDatabaseURLTemplate || env.ValueFrom != nil {
+					t.Errorf("Expected DATABASE_URL from template, got %+v", env)
+				}
+			}
 		}
 		if len(expectedEnvs) > 0 {
 			t.Errorf("Missing expected env vars: %v", expectedEnvs)
@@ -77,6 +118,44 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 		}
 	})
 
+	t.Run("OmitsDatabaseURLWhenDBTypeUnsupported", func(t *testing.T) {
+		deploy := &appsv1.Deployment{
+			Spec: appsv1.DeploymentSpec{
+				Template: corev1.PodTemplateSpec{
+					Spec: corev1.PodSpec{
+						Containers: []corev1.Container{
+							{
+								Name:  "api-server",
+								Image: "myregistry/api:v1",
+								Env: []corev1.EnvVar{
+									{Name: "PORT", Value: "3000"},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret", "api-server-db", "mysql")
+		if !changed {
+			t.Fatal("Expected InjectDatabaseEnvVars to return true (changed)")
+		}
+
+		container := deploy.Spec.Template.Spec.Containers[0]
+		if len(container.Env) != 6 {
+			t.Fatalf("Expected 6 env vars (no DATABASE_URL), got %d", len(container.Env))
+		}
+		for _, env := range container.Env {
+			if env.Name == "DATABASE_URL" {
+				t.Fatalf("unexpected DATABASE_URL for dbType mysql: %+v", env)
+			}
+			if env.Name == "DB_NAME" && env.Value != "api-server-db" {
+				t.Errorf("DB_NAME: expected api-server-db, got %q", env.Value)
+			}
+		}
+	})
+
 	t.Run("Idempotent", func(t *testing.T) {
 		deploy := &appsv1.Deployment{
 			Spec: appsv1.DeploymentSpec{
@@ -90,13 +169,13 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret")
+		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret", "api-server-db", "postgres")
 		if !changed {
 			t.Fatal("Expected first injection to report changes")
 		}
 		firstCount := len(deploy.Spec.Template.Spec.Containers[0].Env)
 
-		changed = InjectDatabaseEnvVars(deploy, "api-server-db-secret")
+		changed = InjectDatabaseEnvVars(deploy, "api-server-db-secret", "api-server-db", "postgres")
 		if changed {
 			t.Error("Expected second injection to report no changes (idempotent)")
 		}
@@ -132,14 +211,14 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret")
+		changed := InjectDatabaseEnvVars(deploy, "api-server-db-secret", "api-server-db", "postgres")
 		if !changed {
 			t.Fatal("Expected InjectDatabaseEnvVars to return true when existing env vars have wrong source")
 		}
 
 		container := deploy.Spec.Template.Spec.Containers[0]
-		if len(container.Env) != 5 {
-			t.Fatalf("Expected 5 env vars, got %d", len(container.Env))
+		if len(container.Env) != 7 {
+			t.Fatalf("Expected 7 env vars, got %d", len(container.Env))
 		}
 
 		for _, env := range container.Env {
@@ -176,7 +255,7 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed := InjectDatabaseEnvVars(deploy, "test-secret")
+		changed := InjectDatabaseEnvVars(deploy, "test-secret", "", "")
 		if changed {
 			t.Error("Expected no changes for Deployment with no containers")
 		}
@@ -196,7 +275,7 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "api-server-db-secret", "api-server", 5433)
+		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "api-server-db-secret", "api-server", 5433, "api-server-db", "postgres")
 		if !changed {
 			t.Fatal("Expected env injection changes")
 		}
@@ -210,13 +289,22 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 		}
 
 		appContainer := deploy.Spec.Template.Spec.Containers[1]
-		expected := map[string]bool{"DB_HOST": false, "DB_USER": false, "DB_PASS": false, "DB_PORT": false}
+		expected := map[string]bool{
+			"DB_HOST": false, "DB_USER": false, "DB_PASS": false, "DB_PORT": false,
+			"DB_NAME": false, "DATABASE_URL": false,
+		}
 		for _, env := range appContainer.Env {
 			if _, ok := expected[env.Name]; ok {
 				expected[env.Name] = true
 				if env.Name == "DB_PORT" && env.Value != "5433" {
 					t.Errorf("Expected DB_PORT=5433, got %q", env.Value)
 				}
+				if env.Name == "DB_NAME" && env.Value != "api-server-db" {
+					t.Errorf("Expected DB_NAME=api-server-db, got %q", env.Value)
+				}
+				if env.Name == "DATABASE_URL" && env.Value != postgresDatabaseURLTemplate {
+					t.Errorf("Expected DATABASE_URL template, got %q", env.Value)
+				}
 			}
 		}
 		for name, found := range expected {
@@ -239,15 +327,15 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "app-db-secret", "missing-app", 5432)
+		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "app-db-secret", "missing-app", 5432, "only-container-db", "postgres")
 		if !changed {
 			t.Fatal("Expected env injection changes")
 		}
 		if exactMatch {
 			t.Fatal("Expected fallback because preferred container does not exist")
 		}
-		if len(deploy.Spec.Template.Spec.Containers[0].Env) != 4 {
-			t.Fatalf("Expected 4 injected DB env vars, got %d", len(deploy.Spec.Template.Spec.Containers[0].Env))
+		if len(deploy.Spec.Template.Spec.Containers[0].Env) != 6 {
+			t.Fatalf("Expected 6 injected DB env vars, got %d", len(deploy.Spec.Template.Spec.Containers[0].Env))
 		}
 	})
 
@@ -264,7 +352,7 @@ func TestInjectDatabaseEnvVars(t *testing.T) {
 			},
 		}
 
-		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "app-db-secret", "", 5432)
+		changed, exactMatch := InjectDatabaseEnvVarsForContainer(deploy, "app-db-secret", "", 5432, "app-db", "postgres")
 		if !changed {
 			t.Fatal("Expected env injection changes")
 		}

apps/operator/internal/controller/database/reconciler.go

@@ -143,10 +143,7 @@ func (r *Reconciler) ReconcileInstances(ctx context.Context, app *appv1alpha1.He
 		dbHost := GetDatabaseHost(dbTrait.ComponentName)
 		secretName := GetDatabaseSecretName(dbTrait.ComponentName)
 
-		effectiveDBName := dbTrait.Properties.DBName
-		if effectiveDBName == "" {
-			effectiveDBName = fmt.Sprintf("%s-db", dbTrait.ComponentName)
-		}
+		effectiveDBName := EffectiveDatabaseName(dbTrait)
 
 		version := dbTrait.Properties.Version
 		if version == "" {
@@ -329,7 +326,8 @@ func (r *Reconciler) ReconcileInjection(ctx context.Context, app *appv1alpha1.He
 			port = DefaultPostgresPort
 		}
 
-		changed, exactContainerMatch := InjectDatabaseEnvVarsForContainer(deploy, secretName, dbTrait.ComponentName, int32(port))
+		dbName := EffectiveDatabaseName(dbTrait)
+		changed, exactContainerMatch := InjectDatabaseEnvVarsForContainer(deploy, secretName, dbTrait.ComponentName, int32(port), dbName, dbTrait.Properties.DBType)
 		if !exactContainerMatch {
 			fallbackContainer := "<no-containers>"
 			if len(deploy.Spec.Template.Spec.Containers) > 0 {

apps/operator/internal/controller/database/reconciler_test.go

@@ -481,15 +481,33 @@ func TestReconcileInjection(t *testing.T) {
 		}
 
 		container := updatedDeploy.Spec.Template.Spec.Containers[0]
-		expectedEnvNames := map[string]bool{"DB_HOST": false, "DB_USER": false, "DB_PASS": false}
+		expectedEnvNames := map[string]bool{
+			"DB_HOST": false, "DB_USER": false, "DB_PASS": false,
+			"DB_PORT": false, "DB_NAME": false, "DATABASE_URL": false,
+		}
 		for _, env := range container.Env {
 			if _, ok := expectedEnvNames[env.Name]; ok {
 				expectedEnvNames[env.Name] = true
-				if env.ValueFrom == nil || env.ValueFrom.SecretKeyRef == nil {
-					t.Errorf("Env %s should reference a secret", env.Name)
-				} else if env.ValueFrom.SecretKeyRef.Name != "api-server-db-secret" {
-					t.Errorf("Env %s: expected secret name %q, got %q",
-						env.Name, "api-server-db-secret", env.ValueFrom.SecretKeyRef.Name)
+				switch env.Name {
+				case "DB_HOST", "DB_USER", "DB_PASS":
+					if env.ValueFrom == nil || env.ValueFrom.SecretKeyRef == nil {
+						t.Errorf("Env %s should reference a secret", env.Name)
+					} else if env.ValueFrom.SecretKeyRef.Name != "api-server-db-secret" {
+						t.Errorf("Env %s: expected secret name %q, got %q",
+							env.Name, "api-server-db-secret", env.ValueFrom.SecretKeyRef.Name)
+					}
+				case "DB_PORT":
+					if env.Value != "5432" || env.ValueFrom != nil {
+						t.Errorf("Env DB_PORT: expected literal 5432, got %+v", env)
+					}
+				case "DB_NAME":
+					if env.Value != "mydb" || env.ValueFrom != nil {
+						t.Errorf("Env DB_NAME: expected literal mydb, got %+v", env)
+					}
+				case "DATABASE_URL":
+					if env.Value != postgresDatabaseURLTemplate || env.ValueFrom != nil {
+						t.Errorf("Env DATABASE_URL: expected template value, got %+v", env)
+					}
 				}
 			}
 		}