diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 8682b3ea5..df146c8e5 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -371,7 +371,7 @@ jobs: strategy: fail-fast: false matrix: - database-provider: [deployment, cnpg, external] + database-provider: [deployment, cnpg, external, secret] # This job now also absorbs the Konflux build wait that plan-images used to # hold: kind-up runs first and overlaps the remote build, the # wait-on-check-action steps then block for whatever build time is left (up @@ -404,7 +404,7 @@ jobs: env: # Exercise both the default per-gateway Deployment path and the # explicitly selected CNPG path through the full E2E workflow. - DATABASE_PROVIDER: ${{ matrix.database-provider }} + DATABASE_PROVIDER: ${{ matrix.database-provider == 'secret' && 'deployment' || matrix.database-provider }} KIND_ENABLE_OIDC: "true" # Deploy Jaeger and point the web-console BFF at it so the browser # trace verification below has a collector to export to (WEB-TRACE-10). @@ -499,6 +499,10 @@ jobs: make kind-env echo "::endgroup::" + - name: Configure controller database admin Secret + if: matrix.database-provider == 'secret' + run: bash tests/fixtures/controller-database/install-kind.sh + # Seed platform resources now that the working-tree images are live (the # swap above rolled them in). Deferred from kind-up so the seed exercises # this PR's request contract instead of the baseline placeholder image. @@ -506,7 +510,7 @@ jobs: # is rejected, rather than surfacing later as a confusing discovery miss. - name: Seed platform resources env: - DATABASE_PROVIDER: ${{ matrix.database-provider }} + DATABASE_PROVIDER: ${{ matrix.database-provider == 'secret' && 'deployment' || matrix.database-provider }} SEED_STRICT: "true" run: make kind-seed @@ -517,7 +521,8 @@ jobs: # Test the candidate installer before its public main URL is available. OPENSHELL_INSTALL_SCRIPT_URL: file://${{ github.workspace }}/scripts/install-openshell.sh # Mirror the provider used to create this matrix job's cluster. - DATABASE_PROVIDER: ${{ matrix.database-provider }} + DATABASE_PROVIDER: ${{ matrix.database-provider == 'secret' && 'deployment' || matrix.database-provider }} + GATEWAY_DATABASE_ADMIN_SECRET_NAME: ${{ matrix.database-provider == 'secret' && 'gateway-database-admin' || '' }} E2E_PROVISION_TIMEOUT: "300" E2E_SANDBOX_TIMEOUT: "180" # Force plain, non-graphical output from the openshell CLI. Off a TTY diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index a41ecc347..77c6efa7d 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -124,6 +124,10 @@ jobs: - name: Run control plane unit tests working-directory: components/control-plane run: go test -count=1 ./... + - name: Test controller database Secret with PostgreSQL TLS + env: + CONTAINER_ENGINE: docker + run: bash scripts/test-controller-database-secret.sh test-cli: name: Go - CLI diff --git a/components/control-plane/cmd/hypershell-controller/main.go b/components/control-plane/cmd/hypershell-controller/main.go index 184bc5b18..a3c9fec9b 100644 --- a/components/control-plane/cmd/hypershell-controller/main.go +++ b/components/control-plane/cmd/hypershell-controller/main.go @@ -72,8 +72,8 @@ func registerWithBackoff(ctx context.Context, regClient *registration.Client) (s // API server or apiserver cannot delay the GC reconciler's launch indefinitely. const instanceLabelBackfillTimeout = 2 * time.Minute -func managedDatabaseWatchEligible(clientset *kubernetes.Clientset, dynamicClient dynamic.Interface) bool { - return clientset != nil && dynamicClient != nil +func managedDatabaseWatchEligible(clientset *kubernetes.Clientset, dynamicClient dynamic.Interface, databaseSecret string) bool { + return databaseSecret == "" && clientset != nil && dynamicClient != nil } func main() { @@ -200,6 +200,10 @@ func main() { } } + if cfg.GatewayDatabaseAdminSecretName != "" && (clientset == nil || dynamicClient == nil) { + log.Fatalf("GATEWAY_DATABASE_ADMIN_SECRET_NAME requires Kubernetes clients") + } + // DATABASE_PROVIDER=cnpg is a hard startup precondition: the control plane // must fail cleanly here, before any watch/reconcile loop starts, when the // exact CNPG API resources this codebase depends on (clusters, databases, @@ -207,7 +211,7 @@ func main() { // deferring the failure to the first CNPG-backed reconciliation deep // inside the gateway/database reconcilers. DATABASE_PROVIDER=deployment (the // default) never reaches this check and has no CNPG dependency at all. - if cfg.DatabaseProvider == config.DatabaseProviderCNPG { + if cfg.GatewayDatabaseAdminSecretName == "" && cfg.DatabaseProvider == config.DatabaseProviderCNPG { if clientset == nil { log.Fatalf("DATABASE_PROVIDER=cnpg requires an in-cluster Kubernetes client to verify the CNPG API prerequisites") } @@ -249,8 +253,10 @@ func main() { clusterReconciler := reconciler.NewManagedClusterReconciler() var databaseReconciler watcher.Handler[*pb.ManagedDatabase] - if managedDatabaseWatchEligible(clientset, dynamicClient) { + if managedDatabaseWatchEligible(clientset, dynamicClient, cfg.GatewayDatabaseAdminSecretName) { databaseReconciler = reconciler.NewManagedDatabaseReconciler(dynamicClient, clientset, conn, cfg.Namespace) + } else if cfg.GatewayDatabaseAdminSecretName != "" { + log.Printf("INFO ManagedDatabase watch disabled: GATEWAY_DATABASE_ADMIN_SECRET_NAME selects %s/%s", cfg.Namespace, cfg.GatewayDatabaseAdminSecretName) } else { log.Printf("WARN ManagedDatabase watch disabled: both Kubernetes typed and dynamic clients are required") } @@ -299,8 +305,11 @@ func main() { var gatewayReconciler watcher.Handler[*pb.Gateway] if clientset != nil && dynamicClient != nil { - gr, grErr := reconciler.NewGatewayReconciler(dynamicClient, clientset, conn, manifestsDir, cfg.Namespace, keycloakConfig, exposurePort) + gr, grErr := reconciler.NewGatewayReconciler(dynamicClient, clientset, conn, manifestsDir, cfg.Namespace, keycloakConfig, exposurePort, cfg.GatewayDatabaseAdminSecretName) if grErr != nil { + if cfg.GatewayDatabaseAdminSecretName != "" { + log.Fatalf("initialize gateway reconciler with GATEWAY_DATABASE_ADMIN_SECRET_NAME: %v", grErr) + } log.Printf("WARN gateway reconciler disabled: %v", grErr) gatewayReconciler = reconciler.NewStubGatewayReconciler() } else { diff --git a/components/control-plane/cmd/hypershell-controller/main_test.go b/components/control-plane/cmd/hypershell-controller/main_test.go index 789dc658a..5642ac81d 100644 --- a/components/control-plane/cmd/hypershell-controller/main_test.go +++ b/components/control-plane/cmd/hypershell-controller/main_test.go @@ -32,9 +32,17 @@ func TestManagedDatabaseWatchEligible(t *testing.T) { if !tt.dynamic { gotDynamic = nil } - if got := managedDatabaseWatchEligible(gotTyped, gotDynamic); got != tt.want { + if got := managedDatabaseWatchEligible(gotTyped, gotDynamic, ""); got != tt.want { t.Fatalf("eligible = %v, want %v", got, tt.want) } }) } } + +func TestManagedDatabaseWatchDisabledWithSecret(t *testing.T) { + typed := &kubernetes.Clientset{} + dynamic := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + if managedDatabaseWatchEligible(typed, dynamic, "gateway-postgres") { + t.Fatal("ManagedDatabase watch must be disabled with a controller database Secret") + } +} diff --git a/components/control-plane/internal/config/config.go b/components/control-plane/internal/config/config.go index 30a144f29..0dcdeea85 100644 --- a/components/control-plane/internal/config/config.go +++ b/components/control-plane/internal/config/config.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" "time" + + "k8s.io/apimachinery/pkg/util/validation" ) // Database provider values for DATABASE_PROVIDER. DatabaseProviderDeployment @@ -84,6 +86,10 @@ type Config struct { // (see internal/reconciler.ManagedDatabaseReconciler), so gateways backed // by CNPG remain compatible even when this default is "deployment". DatabaseProvider string + + // GatewayDatabaseAdminSecretName names the admin Secret in Namespace. + // A non-empty value selects the controller-local database path. + GatewayDatabaseAdminSecretName string } func Load() (*Config, error) { @@ -107,7 +113,17 @@ func Load() (*Config, error) { GatewayReconcileWorkers: getEnvInt("GATEWAY_RECONCILE_WORKERS", DefaultGatewayReconcileWorkers, 1), - DatabaseProvider: databaseProvider, + DatabaseProvider: databaseProvider, + GatewayDatabaseAdminSecretName: os.Getenv("GATEWAY_DATABASE_ADMIN_SECRET_NAME"), + } + + if cfg.GatewayDatabaseAdminSecretName != "" { + if problems := validation.IsDNS1123Subdomain(cfg.GatewayDatabaseAdminSecretName); len(problems) != 0 { + return nil, fmt.Errorf("invalid GATEWAY_DATABASE_ADMIN_SECRET_NAME: %s", strings.Join(problems, "; ")) + } + if problems := validation.IsDNS1123Label(cfg.Namespace); len(problems) != 0 { + return nil, fmt.Errorf("invalid HYPERSHELL_NAMESPACE for GATEWAY_DATABASE_ADMIN_SECRET_NAME: %s", strings.Join(problems, "; ")) + } } if cfg.GRPCServerAddr == "" { diff --git a/components/control-plane/internal/config/config_test.go b/components/control-plane/internal/config/config_test.go index 0de118d04..749f43de5 100644 --- a/components/control-plane/internal/config/config_test.go +++ b/components/control-plane/internal/config/config_test.go @@ -163,3 +163,30 @@ func TestResolveDatabaseProvider(t *testing.T) { } } } + +func TestLoadGatewayDatabaseAdminSecretName(t *testing.T) { + t.Setenv("DATABASE_PROVIDER", "cnpg") + for _, tc := range []struct { + name, secret, namespace string + wantErr bool + }{ + {"unset", "", "hypershell", false}, + {"configured", "gateway-postgres", "hypershell", false}, + {"dotted name", "gateway.postgres", "hypershell", false}, + {"namespace reference rejected", "other/secret", "hypershell", true}, + {"spaces rejected", " gateway-postgres ", "hypershell", true}, + {"invalid namespace", "gateway-postgres", "other/namespace", true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("GATEWAY_DATABASE_ADMIN_SECRET_NAME", tc.secret) + t.Setenv("HYPERSHELL_NAMESPACE", tc.namespace) + cfg, err := Load() + if (err != nil) != tc.wantErr { + t.Fatalf("Load() error = %v, want error %v", err, tc.wantErr) + } + if err == nil && cfg.GatewayDatabaseAdminSecretName != tc.secret { + t.Fatalf("secret = %q, want %q", cfg.GatewayDatabaseAdminSecretName, tc.secret) + } + }) + } +} diff --git a/components/control-plane/internal/gateway/config.go b/components/control-plane/internal/gateway/config.go index 2ebc4aafd..6936b7eb6 100644 --- a/components/control-plane/internal/gateway/config.go +++ b/components/control-plane/internal/gateway/config.go @@ -63,18 +63,17 @@ type CNPGConfig struct { ClusterNamespace string } -// ExternalDBConfig locates the admin credentials for an external -// ManagedDatabase. CredentialsNamespace is the value of -// ManagedDatabase.connection_secret: the NAMESPACE holding the credentials, not -// a Secret name. It must satisfy the hypershell-managed-db- prefix rule, and -// the control plane reads exactly one fixed-name Secret -// (hypershell-managed-db-credentials) inside it. -// -// ManagedDatabaseID is carried for diagnostics only: single-shot cleanup logs -// it so an operator can tie an orphaned role/database back to its registration. +// ExternalDBConfig locates PostgreSQL admin credentials. A configured +// CredentialsSecretName selects a Secret in the controller namespace. +// Otherwise, CredentialsNamespace comes from ManagedDatabase.connection_secret +// and must use the hypershell-managed-db- prefix. That path reads the fixed +// hypershell-managed-db-credentials Secret. ManagedDatabaseID is for diagnostics. type ExternalDBConfig struct { CredentialsNamespace string - ManagedDatabaseID string + // CredentialsSecretName selects a controller-configured Secret. + // Empty uses the fixed name and namespace rules for ManagedDatabase. + CredentialsSecretName string + ManagedDatabaseID string } // DefaultSandboxImage resolves the base image tenant sandbox pods launch from. diff --git a/components/control-plane/internal/gateway/external_db.go b/components/control-plane/internal/gateway/external_db.go index 07d7119a3..23dbe3b74 100644 --- a/components/control-plane/internal/gateway/external_db.go +++ b/components/control-plane/internal/gateway/external_db.go @@ -3,6 +3,7 @@ package gateway import ( "context" cryptoRand "crypto/rand" + "crypto/x509" "database/sql" "encoding/hex" "errors" @@ -13,6 +14,7 @@ import ( "os" "reflect" "regexp" + "strconv" "strings" // register postgres driver and use typed error codes for status mapping @@ -41,22 +43,16 @@ func (r *externalDatabaseReconciler) Reconcile(ctx context.Context, _ dynamic.In return nil } -// Delete drops the gateway's external database and role. Cleanup is -// unconditional, single-shot and best-effort: the gateway is already removed -// from the API server, there is no tombstone and no retry queue, so no later -// event re-delivers this work. -// -// It therefore always returns nil. A failure is logged at ERROR naming the -// gateway and ManagedDatabase IDs (never credentials) so the orphaned role and -// database are discoverable; operators reclaim them with the runbook in -// openshell-gateway-database-external.spec.md ยง Operator Runbook. Returning an -// error instead would strand gateway finalization on a retry that never -// succeeds. +// Delete drops the gateway database and role. A configured admin Secret uses +// the live delete queue for retries. Legacy mode retains best-effort cleanup. func (r *externalDatabaseReconciler) Delete(ctx context.Context, _ dynamic.Interface, clientset kubernetes.Interface, gatewayID string) error { if gatewayID == "" || r.cfg.CredentialsNamespace == "" { return nil } if err := DeleteExternalDatabaseResources(ctx, clientset, r.cfg, gatewayID); err != nil { + if r.cfg.CredentialsSecretName != "" { + return err + } log.Printf("ERROR gateway %s: external database cleanup failed on ManagedDatabase %s; role and database %q may remain on the external server and require manual removal (see the external database spec's operator runbook): %v", gatewayID, r.cfg.ManagedDatabaseID, externalGatewayDBName(gatewayID), err) } @@ -141,19 +137,54 @@ func readExternalAdminSecret(ctx context.Context, clientset kubernetes.Interface return nil, fmt.Errorf("connection_secret validation: %w", err) } - secret, err := clientset.CoreV1().Secrets(credentialsNamespace).Get(ctx, externalCredentialsSecretName, metav1.GetOptions{}) + params, err := readAdminSecret(ctx, clientset, credentialsNamespace, externalCredentialsSecretName) + if err == nil && params.sslmode == "" { + params.sslmode = "require" + } + return params, err +} + +// readExternalAdminCredentials uses the configured Secret or the legacy reference. +func readExternalAdminCredentials(ctx context.Context, clientset kubernetes.Interface, cfg ExternalDBConfig) (*externalAdminParams, error) { + if cfg.CredentialsSecretName == "" { + return readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + } + params, err := readAdminSecret(ctx, clientset, cfg.CredentialsNamespace, cfg.CredentialsSecretName) + if err != nil { + return nil, err + } + // The controller override is an opt-in production path. Require both + // certificate and hostname verification before any SQL operation. + if params.sslmode == "" { + params.sslmode = "verify-full" + } + if params.sslmode != "verify-full" { + return nil, fmt.Errorf("admin Secret %s/%s requires sslmode=verify-full", cfg.CredentialsNamespace, cfg.CredentialsSecretName) + } + if !x509.NewCertPool().AppendCertsFromPEM([]byte(params.sslrootcert)) { + return nil, fmt.Errorf("admin Secret %s/%s requires a valid PEM sslrootcert", cfg.CredentialsNamespace, cfg.CredentialsSecretName) + } + port, err := strconv.Atoi(params.port) + if err != nil || port < 1 || port > 65535 { + return nil, fmt.Errorf("admin Secret %s/%s has an invalid port", cfg.CredentialsNamespace, cfg.CredentialsSecretName) + } + return params, nil +} + +func readAdminSecret(ctx context.Context, clientset kubernetes.Interface, credentialsNamespace, secretName string) (*externalAdminParams, error) { + secret, err := clientset.CoreV1().Secrets(credentialsNamespace).Get(ctx, secretName, metav1.GetOptions{}) if err != nil { if k8serrors.IsNotFound(err) { - return nil, fmt.Errorf("secret %q not found in namespace %q", externalCredentialsSecretName, credentialsNamespace) + return nil, fmt.Errorf("secret %q not found in namespace %q", secretName, credentialsNamespace) } - return nil, fmt.Errorf("read Secret %q in namespace %q: %w", externalCredentialsSecretName, credentialsNamespace, err) + return nil, fmt.Errorf("read Secret %q in namespace %q: %w", secretName, credentialsNamespace, err) } get := func(key string) string { return string(secret.Data[key]) } required := []string{"host", "port", "user", "password"} for _, k := range required { if get(k) == "" { - return nil, fmt.Errorf("secret %q in namespace %q is missing required key %q", externalCredentialsSecretName, credentialsNamespace, k) + return nil, fmt.Errorf("secret %q in namespace %q is missing required key %q", secretName, credentialsNamespace, k) } } @@ -162,9 +193,6 @@ func readExternalAdminSecret(ctx context.Context, clientset kubernetes.Interface dbname = "postgres" } sslmode := get("sslmode") - if sslmode == "" { - sslmode = "require" - } switch sslmode { case "disable": log.Printf("WARN external DB credentials in namespace %s: sslmode=disable is insecure; use verify-full for production", credentialsNamespace) @@ -327,7 +355,7 @@ func mapConnErrorToStatus(err error) string { // admin role's CREATEDB and CREATEROLE attributes, and returns a // closed-vocabulary status string. It is side-effect-free on the server. func ProbeExternalServer(ctx context.Context, clientset kubernetes.Interface, cfg ExternalDBConfig) string { - params, err := readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + params, err := readExternalAdminCredentials(ctx, clientset, cfg) if err != nil { log.Printf("INFO external DB probe (namespace %s): %s: %v", cfg.CredentialsNamespace, ExternalDBStatusSecretInvalid, err) return ExternalDBStatusSecretInvalid @@ -381,7 +409,7 @@ func ReconcileExternalDatabaseResources( gatewayID string, cfg ExternalDBConfig, ) error { - params, err := readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + params, err := readExternalAdminCredentials(ctx, clientset, cfg) if err != nil { return fmt.Errorf("read external admin credentials: %w", err) } @@ -455,6 +483,14 @@ func ReconcileExternalDatabaseResources( // server. Recover by deleting the tenant Secret, which makes the branch above // re-apply a fresh password on the next reconcile. + // A non-superuser needs membership to create a database owned by the + // gateway role. PostgreSQL 16 and later do not grant SET ROLE on creation. + if cfg.CredentialsSecretName != "" { + if _, err := db.ExecContext(ctx, fmt.Sprintf("GRANT %s TO %s", pgQuoteIdent(pgName), pgQuoteIdent(params.user))); err != nil { + return fmt.Errorf("grant gateway role to database admin: %w", err) + } + } + // Database: create if absent. var dbExists bool if err := db.QueryRowContext(ctx, @@ -484,30 +520,8 @@ func ReconcileExternalDatabaseResources( return fmt.Errorf("GRANT CONNECT for gateway %s: %w", gatewayID, err) } - // Write or refresh the tenant credentials Secret. - // - // Tenant TLS: cap at "require" when the admin connection uses "verify-full". - // Verifying the server certificate from the gateway pod needs the CA bundle - // mounted into that pod, which is the deferred CA-distribution follow-up in - // the spec. Until then the tenant connection is encrypted but unverified. - tenantSSLMode := params.sslmode - if tenantSSLMode == "verify-full" || tenantSSLMode == "verify-ca" { - tenantSSLMode = "require" - } - tenantQ := url.Values{"sslmode": {tenantSSLMode}} - tenantBase := fmt.Sprintf("postgresql://%s:%s@%s:%s/%s", - url.QueryEscape(pgName), url.QueryEscape(password), params.host, params.port, pgName) - dbURI := tenantBase + "?" + tenantQ.Encode() - - desiredData := map[string][]byte{ - "host": []byte(params.host), - "port": []byte(params.port), - "dbname": []byte(pgName), - "user": []byte(pgName), - "password": []byte(password), - "sslmode": []byte(tenantSSLMode), - "uri": []byte(dbURI), - } + // The gateway receives its own credentials and the public CA bundle. + desiredData := externalTenantSecretData(params, pgName, password) desiredLabels := map[string]string{ "app.kubernetes.io/name": "openshell", "app.kubernetes.io/component": "database", @@ -553,9 +567,7 @@ func ReconcileExternalDatabaseResources( // gateway's database and role on the external server. Idempotent: absent // objects are treated as success. // -// A non-nil error means the objects may still exist on the server. Deletion is -// single-shot (see externalDatabaseReconciler.Delete), so the caller logs the -// failure rather than scheduling a retry. +// A non-nil error means the objects may still exist on the server. func DeleteExternalDatabaseResources( ctx context.Context, clientset kubernetes.Interface, @@ -566,7 +578,7 @@ func DeleteExternalDatabaseResources( return nil } - params, err := readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + params, err := readExternalAdminCredentials(ctx, clientset, cfg) if err != nil { return fmt.Errorf("cannot read external admin credentials: %w", err) } @@ -577,6 +589,10 @@ func DeleteExternalDatabaseResources( } defer release() + return deleteExternalSQLResources(ctx, db, gatewayID) +} + +func deleteExternalSQLResources(ctx context.Context, db *sql.DB, gatewayID string) error { pgName := externalGatewayDBName(gatewayID) // Terminate active backends so DROP DATABASE is not blocked. @@ -593,7 +609,10 @@ func DeleteExternalDatabaseResources( // Drop the gateway database. This is a cluster-level operation and must // come before DROP ROLE because the role owns the database. var dbExists bool - if err := db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_database WHERE datname = $1)", pgName).Scan(&dbExists); err == nil && dbExists { + if err := db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_database WHERE datname = $1)", pgName).Scan(&dbExists); err != nil { + return fmt.Errorf("check database before cleanup: %w", err) + } + if dbExists { if _, err := db.ExecContext(ctx, fmt.Sprintf("DROP DATABASE %s WITH (FORCE)", pgQuoteIdent(pgName))); err != nil { return fmt.Errorf("DROP DATABASE failed: %w", err) } @@ -602,7 +621,10 @@ func DeleteExternalDatabaseResources( // Drop role (safe once the database it owned is gone). var roleExists bool - if err := db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", pgName).Scan(&roleExists); err == nil && roleExists { + if err := db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname = $1)", pgName).Scan(&roleExists); err != nil { + return fmt.Errorf("check role before cleanup: %w", err) + } + if roleExists { if _, err := db.ExecContext(ctx, fmt.Sprintf("DROP ROLE %s", pgQuoteIdent(pgName))); err != nil { return fmt.Errorf("DROP ROLE failed: %w", err) } @@ -611,6 +633,26 @@ func DeleteExternalDatabaseResources( return nil } +// externalTenantCAPath matches the projected CA file in the gateway Deployment. +const externalTenantCAPath = "/etc/openshell-db/ca.crt" + +func externalTenantSecretData(params *externalAdminParams, pgName, password string) map[string][]byte { + q := url.Values{"sslmode": {params.sslmode}} + data := map[string][]byte{ + "host": []byte(params.host), "port": []byte(params.port), + "dbname": []byte(pgName), "user": []byte(pgName), + "password": []byte(password), "sslmode": []byte(params.sslmode), + } + if params.sslrootcert != "" { + data["sslrootcert"] = []byte(params.sslrootcert) + q.Set("sslrootcert", externalTenantCAPath) + } + uri := url.URL{Scheme: "postgresql", User: url.UserPassword(pgName, password), + Host: net.JoinHostPort(params.host, params.port), Path: "/" + pgName, RawQuery: q.Encode()} + data["uri"] = []byte(uri.String()) + return data +} + // pgQuoteIdent quotes a PostgreSQL identifier to prevent SQL injection. // Only safe for identifiers produced from internal gateway IDs. func pgQuoteIdent(name string) string { diff --git a/components/control-plane/internal/gateway/external_db_cleanup_test.go b/components/control-plane/internal/gateway/external_db_cleanup_test.go new file mode 100644 index 000000000..1f6ceed71 --- /dev/null +++ b/components/control-plane/internal/gateway/external_db_cleanup_test.go @@ -0,0 +1,63 @@ +package gateway + +import ( + "context" + "database/sql" + "database/sql/driver" + "errors" + "io" + "strings" + "testing" +) + +var errCleanupQuery = errors.New("catalog query failed") + +type cleanupQueryDriver struct{} +type cleanupQueryConn struct{ failedCatalog string } +type absentObjectRows struct{ read bool } + +func (cleanupQueryDriver) Open(name string) (driver.Conn, error) { + return &cleanupQueryConn{failedCatalog: name}, nil +} +func (*cleanupQueryConn) Prepare(string) (driver.Stmt, error) { + return nil, errors.New("not supported") +} +func (*cleanupQueryConn) Begin() (driver.Tx, error) { return nil, errors.New("not supported") } +func (*cleanupQueryConn) Close() error { return nil } +func (*cleanupQueryConn) ExecContext(context.Context, string, []driver.NamedValue) (driver.Result, error) { + return driver.RowsAffected(0), nil +} +func (c *cleanupQueryConn) QueryContext(_ context.Context, query string, _ []driver.NamedValue) (driver.Rows, error) { + if strings.Contains(query, c.failedCatalog) { + return nil, errCleanupQuery + } + return &absentObjectRows{}, nil +} +func (*absentObjectRows) Columns() []string { return []string{"exists"} } +func (*absentObjectRows) Close() error { return nil } +func (r *absentObjectRows) Next(values []driver.Value) error { + if r.read { + return io.EOF + } + values[0] = false + r.read = true + return nil +} + +func init() { sql.Register("hypershell-cleanup-query-test", cleanupQueryDriver{}) } + +func TestExternalCleanupReturnsCatalogErrors(t *testing.T) { + for _, catalog := range []string{"pg_database", "pg_roles"} { + t.Run(catalog, func(t *testing.T) { + db, err := sql.Open("hypershell-cleanup-query-test", catalog) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + err = deleteExternalSQLResources(context.Background(), db, "test") + if !errors.Is(err, errCleanupQuery) { + t.Fatalf("failed catalog query was treated as successful cleanup: %v", err) + } + }) + } +} diff --git a/components/control-plane/internal/gateway/external_db_integration_test.go b/components/control-plane/internal/gateway/external_db_integration_test.go new file mode 100644 index 000000000..29e77b13d --- /dev/null +++ b/components/control-plane/internal/gateway/external_db_integration_test.go @@ -0,0 +1,195 @@ +//go:build integration + +package gateway + +import ( + "context" + "database/sql" + "fmt" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sfake "k8s.io/client-go/kubernetes/fake" +) + +// TestControllerDatabaseLifecycle uses a real TLS server and a non-superuser +// account. Run it with scripts/test-controller-database-secret.sh. +func TestControllerDatabaseLifecycle(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + t.Cleanup(cancel) + ca, err := os.ReadFile(os.Getenv("TEST_DATABASE_CA")) + if err != nil { + t.Fatal("TEST_DATABASE_CA must name the fixture CA") + } + port := os.Getenv("TEST_DATABASE_PORT") + if port == "" { + t.Fatal("TEST_DATABASE_PORT must name the fixture port") + } + cfg := ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "gateway-database-admin"} + admin := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: cfg.CredentialsSecretName, Namespace: cfg.CredentialsNamespace}, + Data: map[string][]byte{"host": []byte("localhost"), "port": []byte(port), "user": []byte("provisioner"), + "password": []byte("fixture-admin"), "sslrootcert": ca}} + client := k8sfake.NewSimpleClientset(admin) + params, err := readExternalAdminCredentials(ctx, client, cfg) + if err != nil { + t.Fatal(err) + } + db, release, err := openAdminConn(ctx, params) + if err != nil { + t.Fatal("fixture admin cannot connect") + } + t.Cleanup(release) + var superuser bool + if err := db.QueryRowContext(ctx, "SELECT rolsuper FROM pg_roles WHERE rolname = current_user").Scan(&superuser); err != nil || superuser { + t.Fatalf("fixture must use a non-superuser: %v", err) + } + gatewayID := fmt.Sprintf("test%d", time.Now().UnixNano()) + name := externalGatewayDBName(gatewayID) + tenantNS := "gateway-test" + reconcile := func() error { return ReconcileExternalDatabaseResources(ctx, client, tenantNS, gatewayID, cfg) } + cleanup := func() error { return (&externalDatabaseReconciler{cfg: cfg}).Delete(ctx, nil, client, gatewayID) } + t.Cleanup(func() { + if err := cleanup(); err != nil { + t.Errorf("fixture cleanup: %v", err) + } + }) + if err := reconcile(); err != nil { + t.Fatal(err) + } + tenant, err := client.CoreV1().Secrets(tenantNS).Get(ctx, tenantGatewayDBSecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + firstPassword := string(tenant.Data["password"]) + if string(tenant.Data["sslmode"]) != "verify-full" || string(tenant.Data["sslrootcert"]) != string(ca) { + t.Fatal("tenant TLS configuration is incomplete") + } + if err := reconcile(); err != nil { + t.Fatal(err) + } + tenant, err = client.CoreV1().Secrets(tenantNS).Get(ctx, tenantGatewayDBSecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if string(tenant.Data["password"]) != firstPassword { + t.Fatal("reconcile changed tenant password") + } + uri, err := url.Parse(string(tenant.Data["uri"])) + if err != nil { + t.Fatal("invalid tenant URI") + } + q := uri.Query() + q.Set("sslrootcert", os.Getenv("TEST_DATABASE_CA")) + uri.RawQuery = q.Encode() + tenantDB, err := sql.Open("postgres", uri.String()) + if err != nil { + t.Fatal("invalid tenant connection") + } + defer func() { _ = tenantDB.Close() }() + var tls bool + if err := tenantDB.QueryRowContext(ctx, "SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid()").Scan(&tls); err != nil || !tls { + t.Fatalf("tenant TLS connection failed: %v", err) + } + if _, err := tenantDB.ExecContext(ctx, "CREATE TABLE lifecycle_test (id integer PRIMARY KEY)"); err != nil { + t.Fatal(err) + } + + // Reject a trusted certificate for a different hostname. + badHost := *params + badHost.host = "127.0.0.1" + if _, closeBad, err := openAdminConn(ctx, &badHost); err == nil { + closeBad() + t.Fatal("hostname mismatch was accepted") + } + badCA := *params + badCA.sslrootcert = validTestCA(t) + if _, closeBad, err := openAdminConn(ctx, &badCA); err == nil { + closeBad() + t.Fatal("untrusted CA was accepted") + } + badCAPath := filepath.Join(t.TempDir(), "untrusted.pem") + if err := os.WriteFile(badCAPath, []byte(badCA.sslrootcert), 0600); err != nil { + t.Fatal(err) + } + q.Set("sslrootcert", badCAPath) + uri.RawQuery = q.Encode() + rejected, err := sql.Open("postgres", uri.String()) + if err != nil { + t.Fatal(err) + } + if err := rejected.PingContext(ctx); err == nil { + t.Fatal("tenant accepted an untrusted CA") + } + _ = rejected.Close() + + q.Set("sslrootcert", os.Getenv("TEST_DATABASE_CA")) + uri.Host = "127.0.0.1:" + port + uri.RawQuery = q.Encode() + wrongHost, err := sql.Open("postgres", uri.String()) + if err != nil { + t.Fatal(err) + } + if err := wrongHost.PingContext(ctx); err == nil { + t.Fatal("tenant accepted a hostname mismatch") + } + _ = wrongHost.Close() + + // The next operation must read the new password from the Secret. + if _, err := db.ExecContext(ctx, "ALTER ROLE provisioner PASSWORD 'fixture-rotated'"); err != nil { + t.Fatal("fixture password change failed") + } + if err := reconcile(); err == nil { + t.Fatal("stale admin password was accepted") + } + admin.Data["password"] = []byte("fixture-rotated") + if _, err := client.CoreV1().Secrets(cfg.CredentialsNamespace).Update(ctx, admin, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + if err := reconcile(); err != nil { + t.Fatal(err) + } + + // A missing Secret must fail both operations and must allow a later retry. + if err := client.CoreV1().Secrets(cfg.CredentialsNamespace).Delete(ctx, cfg.CredentialsSecretName, metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + if err := reconcile(); err == nil { + t.Fatal("missing Secret provisioned a database") + } + if err := cleanup(); err == nil { + t.Fatal("missing Secret hid failed cleanup") + } + if _, err := client.CoreV1().Secrets(cfg.CredentialsNamespace).Create(ctx, admin, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + + // A role with an extra object cannot be dropped. The database drop succeeds; + // role failure must be returned, then a retry must remove the remaining role. + object := pgQuoteIdent(name + "_blocker") + if _, err := db.ExecContext(ctx, "CREATE DATABASE "+object+" OWNER "+pgQuoteIdent(name)); err != nil { + t.Fatal(err) + } + + if err := cleanup(); err == nil || !strings.Contains(err.Error(), "DROP ROLE") { + t.Fatalf("expected DROP ROLE error: %v", err) + } + if _, err := db.ExecContext(ctx, "DROP DATABASE "+object); err != nil { + t.Fatal(err) + } + if err := cleanup(); err != nil { + t.Fatal(err) + } + if err := cleanup(); err != nil { + t.Fatalf("cleanup is not idempotent: %v", err) + } + var remains bool + if err := db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM pg_roles WHERE rolname=$1) OR EXISTS(SELECT 1 FROM pg_database WHERE datname=$1)", name).Scan(&remains); err != nil || remains { + t.Fatalf("database or role remains: %v", err) + } +} diff --git a/components/control-plane/internal/gateway/external_db_test.go b/components/control-plane/internal/gateway/external_db_test.go index 2b672a854..a2c27f7cb 100644 --- a/components/control-plane/internal/gateway/external_db_test.go +++ b/components/control-plane/internal/gateway/external_db_test.go @@ -2,12 +2,20 @@ package gateway import ( "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "encoding/pem" "errors" + "fmt" + "math/big" "net" "net/url" "os" "strings" "testing" + "time" pq "github.com/lib/pq" corev1 "k8s.io/api/core/v1" @@ -431,3 +439,156 @@ func TestExternalGatewayDBName(t *testing.T) { t.Errorf("externalGatewayDBName() = %q, want lowercased gw_ name", got) } } + +func TestControllerDatabaseSecret(t *testing.T) { + ctx := context.Background() + cfg := ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "gateway-postgres"} + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: cfg.CredentialsSecretName, Namespace: cfg.CredentialsNamespace}, + Data: map[string][]byte{"host": []byte("db.example.test"), "port": []byte("5432"), "user": []byte("admin"), "password": []byte("test-password"), "sslrootcert": []byte(validTestCA(t))}, + } + client := k8sfake.NewSimpleClientset(secret) + params, err := readExternalAdminCredentials(ctx, client, cfg) + if err != nil { + t.Fatal(err) + } + if params.host != "db.example.test" || params.user != "admin" { + t.Fatal("credentials do not match the configured Secret") + } + actions := client.Actions() + if len(actions) != 1 || actions[0].GetVerb() != "get" || actions[0].GetNamespace() != cfg.CredentialsNamespace { + t.Fatalf("unexpected Secret access: %v", actions) + } + + // Read current credentials on the next reconciliation. + secret.Data["password"] = []byte("replacement-password") + if _, err := client.CoreV1().Secrets(cfg.CredentialsNamespace).Update(ctx, secret, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + params, err = readExternalAdminCredentials(ctx, client, cfg) + if err != nil || params.password != "replacement-password" { + t.Fatalf("updated credentials were not read: %v", err) + } + + // An API reference must still obey the legacy namespace restriction. + cfg.CredentialsSecretName = "" + if _, err := readExternalAdminCredentials(ctx, client, cfg); err == nil { + t.Fatal("legacy namespace restriction was bypassed") + } +} + +func TestControllerDatabaseSecretFailureDoesNotFallBack(t *testing.T) { + for _, missing := range []bool{true, false} { + t.Run(fmt.Sprintf("missing=%v", missing), func(t *testing.T) { + cfg := ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "gateway-postgres"} + client := k8sfake.NewSimpleClientset() + if !missing { + _, err := client.CoreV1().Secrets(cfg.CredentialsNamespace).Create(context.Background(), &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: cfg.CredentialsSecretName}, + Data: map[string][]byte{"host": []byte("db.example.test")}, + }, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + } + client.ClearActions() + if err := ReconcileExternalDatabaseResources(context.Background(), client, "gateway-ns", "gateway-id", cfg); err == nil { + t.Fatal("provisioning must fail when credentials are missing or invalid") + } + if err := DeleteExternalDatabaseResources(context.Background(), client, cfg, "gateway-id"); err == nil { + t.Fatal("cleanup must report missing or invalid credentials") + } + for _, action := range client.Actions() { + if action.GetVerb() != "get" || action.GetResource().Resource != "secrets" || action.GetNamespace() != cfg.CredentialsNamespace { + t.Fatalf("unexpected fallback action: %v", action) + } + } + }) + } +} + +func validTestCA(t *testing.T) string { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + template := &x509.Certificate{SerialNumber: big.NewInt(1), IsCA: true, BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, NotBefore: time.Now().Add(-time.Hour), NotAfter: time.Now().Add(time.Hour)} + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + return string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) +} + +func TestControllerAdminSecretTLSValidation(t *testing.T) { + for _, tc := range []struct { + name, mode, ca, port string + valid bool + }{ + {"defaults", "", validTestCA(t), "5432", true}, + {"verified", "verify-full", validTestCA(t), "5432", true}, + {"plaintext", "disable", validTestCA(t), "5432", false}, + {"unverified", "require", validTestCA(t), "5432", false}, + {"no hostname check", "verify-ca", validTestCA(t), "5432", false}, + {"missing CA", "verify-full", "", "5432", false}, + {"invalid CA", "verify-full", testCAPEM, "5432", false}, + {"invalid port", "verify-full", validTestCA(t), "65536", false}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "admin"} + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "admin", Namespace: "hypershell"}, + Data: map[string][]byte{"host": []byte("db.test"), "port": []byte(tc.port), "user": []byte("admin"), + "password": []byte("secret-value"), "sslmode": []byte(tc.mode), "sslrootcert": []byte(tc.ca)}} + params, err := readExternalAdminCredentials(context.Background(), k8sfake.NewSimpleClientset(secret), cfg) + if (err == nil) != tc.valid { + t.Fatalf("valid=%v: %v", tc.valid, err) + } + if err == nil && params.sslmode != "verify-full" { + t.Fatal("TLS mode was not verified") + } + if err != nil && strings.Contains(err.Error(), "secret-value") { + t.Fatal("error contains password") + } + }) + } +} + +func TestControllerAdminSecretDeleteReturnsError(t *testing.T) { + r := &externalDatabaseReconciler{cfg: ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "missing"}} + if err := r.Delete(context.Background(), nil, k8sfake.NewSimpleClientset(), "gateway"); err == nil { + t.Fatal("cleanup failure must reach the retry queue") + } +} + +func TestExternalTenantSecretPreservesTLS(t *testing.T) { + for _, mode := range []string{"verify-full", "verify-ca", "require", "disable"} { + t.Run(mode, func(t *testing.T) { + p := &externalAdminParams{host: "2001:db8::1", port: "5432", user: "admin", password: "admin-password", sslmode: mode} + if mode == "verify-full" || mode == "verify-ca" { + p.sslrootcert = validTestCA(t) + } + data := externalTenantSecretData(p, "gw_test", "tenant-password") + uri, err := url.Parse(string(data["uri"])) + if err != nil { + t.Fatal(err) + } + if uri.Hostname() != p.host || uri.Query().Get("sslmode") != mode { + t.Fatal("host or TLS mode changed") + } + if mode == "verify-full" || mode == "verify-ca" { + if uri.Query().Get("sslrootcert") != externalTenantCAPath || string(data["sslrootcert"]) != p.sslrootcert { + t.Fatal("CA was not propagated") + } + } else if uri.Query().Get("sslrootcert") != "" { + t.Fatal("legacy mode needs no CA mount") + } + for _, value := range data { + if strings.Contains(string(value), "admin-password") { + t.Fatal("admin password reached tenant") + } + } + }) + } +} diff --git a/components/control-plane/internal/gateway/manifests_test.go b/components/control-plane/internal/gateway/manifests_test.go index f9589bb18..b0f369f9c 100644 --- a/components/control-plane/internal/gateway/manifests_test.go +++ b/components/control-plane/internal/gateway/manifests_test.go @@ -231,3 +231,50 @@ func TestApplyCredentialDriverDeploymentOverrides_VaultAddsVolume(t *testing.T) t.Errorf("expected volume name vault-sa-token, got %v", vol["name"]) } } + +func TestGatewayDatabaseCAMount(t *testing.T) { + manifests, err := LoadGatewayManifests("../../manifests/gateway") + if err != nil { + t.Fatal(err) + } + for _, objects := range manifests { + for _, object := range objects { + if object.GetKind() != "Deployment" || object.GetName() != "openshell-gateway" { + continue + } + volumes, _, _ := unstructured.NestedSlice(object.Object, "spec", "template", "spec", "volumes") + projected := false + for _, volume := range volumes { + v := volume.(map[string]interface{}) + if v["name"] != "database-ca" { + continue + } + secret := v["secret"].(map[string]interface{}) + items := secret["items"].([]interface{}) + item := items[0].(map[string]interface{}) + if secret["secretName"] != tenantGatewayDBSecretName || secret["optional"] != true || len(items) != 1 || item["key"] != "sslrootcert" || item["path"] != "ca.crt" { + t.Fatal("CA volume must project only the optional public CA") + } + projected = true + } + if !projected { + t.Fatal("database CA volume is missing") + } + containers, _, _ := unstructured.NestedSlice(object.Object, "spec", "template", "spec", "containers") + for _, container := range containers { + c := container.(map[string]interface{}) + if c["name"] != "openshell-gateway" { + continue + } + for _, mount := range c["volumeMounts"].([]interface{}) { + m := mount.(map[string]interface{}) + if m["name"] == "database-ca" && m["mountPath"] == "/etc/openshell-db" && m["readOnly"] == true { + return + } + } + } + t.Fatal("gateway has no read-only database CA mount") + } + } + t.Fatal("gateway deployment not found") +} diff --git a/components/control-plane/internal/gateway/reconciler.go b/components/control-plane/internal/gateway/reconciler.go index 0a587680e..9d68f5e8e 100644 --- a/components/control-plane/internal/gateway/reconciler.go +++ b/components/control-plane/internal/gateway/reconciler.go @@ -303,9 +303,12 @@ func DeleteGatewayResources( cleanupCtx, cleanupCancel := context.WithTimeout(ctx, 2*time.Minute) defer cleanupCancel() if delErr := dbReconciler.Delete(cleanupCtx, dynamicClient, clientset, opts.GatewayID); delErr != nil { - // Transient error (server unreachable, DDL failure): return so the - // delete-reconcile retries. Terminal errors (admin secret unreadable) - // are handled inside Delete and return nil; in-cluster cleanup still runs. + reason := "Database cleanup failed. Check controller database configuration and retry cleanup." + if opts.ExternalDB.CredentialsSecretName != "" { + reason = fmt.Sprintf("Database cleanup failed using Secret %s/%s. Check controller logs and retry cleanup.", + opts.ExternalDB.CredentialsNamespace, opts.ExternalDB.CredentialsSecretName) + } + recordOrphan(ctx, opts, "PostgreSQLDatabase", externalGatewayDBName(opts.GatewayID), reason) return fmt.Errorf("database cleanup for gateway %s: %w", opts.GatewayID, delErr) } } else { diff --git a/components/control-plane/internal/gateway/reconciler_test.go b/components/control-plane/internal/gateway/reconciler_test.go index bee8b1127..35e2bb097 100644 --- a/components/control-plane/internal/gateway/reconciler_test.go +++ b/components/control-plane/internal/gateway/reconciler_test.go @@ -3,6 +3,11 @@ package gateway import ( "context" "fmt" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "net/http" + "net/http/httptest" + "strings" "testing" corev1 "k8s.io/api/core/v1" @@ -578,3 +583,31 @@ func TestReconcileRouteResourcesTLSIssuer(t *testing.T) { } }) } + +func TestNamedDatabaseCleanupRecordsFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer server.Close() + client, err := kubernetes.NewForConfig(&rest.Config{Host: server.URL}) + if err != nil { + t.Fatal(err) + } + dynamicClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) + recorded := false + opts := ReconcileOpts{GatewayID: "test", DatabaseProvider: "external", + ExternalDB: ExternalDBConfig{CredentialsNamespace: "hypershell", CredentialsSecretName: "missing-admin"}, + RecordOrphan: func(_ context.Context, kind, name, reason string) { + if kind != "PostgreSQLDatabase" || name != "gw_test" || !strings.Contains(reason, "hypershell/missing-admin") { + t.Fatalf("cleanup warning lacks resource or Secret reference: %s %s %s", kind, name, reason) + } + recorded = true + }, + } + if err := DeleteGatewayResources(context.Background(), dynamicClient, client, "gateway-test", opts); err == nil { + t.Fatal("cleanup failure did not reach the retry queue") + } + if !recorded { + t.Fatal("cleanup failure did not record a warning") + } +} diff --git a/components/control-plane/internal/reconciler/gateway_database_secret_test.go b/components/control-plane/internal/reconciler/gateway_database_secret_test.go new file mode 100644 index 000000000..a5e86b65d --- /dev/null +++ b/components/control-plane/internal/reconciler/gateway_database_secret_test.go @@ -0,0 +1,31 @@ +package reconciler + +import ( + "context" + pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "testing" +) + +func TestResolveControllerDatabaseSecret(t *testing.T) { + // A nil gRPC connection makes any API lookup fail this test. + r := &GatewayReconciler{controlPlaneNamespace: "hypershell", databaseSecret: "gateway-postgres"} + for _, databaseID := range []string{"", "ignored-database-id"} { + cfg, err := r.resolveDatabaseConfig(context.Background(), &pb.Gateway{DatabaseId: databaseID}) + if err != nil { + t.Fatal(err) + } + if cfg.Provider != "external" || cfg.ExternalDB.CredentialsNamespace != "hypershell" || cfg.ExternalDB.CredentialsSecretName != "gateway-postgres" { + t.Fatalf("unexpected database configuration: %+v", cfg) + } + if cfg.ExternalDB.ManagedDatabaseID != "" || cfg.SourceNamespace != "" || cfg.CNPG.ClusterNamespace != "" { + t.Fatalf("legacy database configuration is active: %+v", cfg) + } + } +} + +func TestDatabaseSecretUnsetKeepsLegacyRequirement(t *testing.T) { + r := &GatewayReconciler{} + if _, err := r.resolveDatabaseConfig(context.Background(), &pb.Gateway{}); err == nil { + t.Fatal("legacy database lookup must require database_id") + } +} diff --git a/components/control-plane/internal/reconciler/reconciler.go b/components/control-plane/internal/reconciler/reconciler.go index 256b16fbb..8cddbde14 100644 --- a/components/control-plane/internal/reconciler/reconciler.go +++ b/components/control-plane/internal/reconciler/reconciler.go @@ -1438,6 +1438,7 @@ type GatewayReconciler struct { keycloakClient *keycloak.Client keycloakConfig *gateway.KeycloakConfig exposure exposure.Port + databaseSecret string } func NewGatewayReconciler( @@ -1448,6 +1449,7 @@ func NewGatewayReconciler( controlPlaneNamespace string, keycloakConfig *gateway.KeycloakConfig, exposurePort exposure.Port, + databaseSecret string, ) (*GatewayReconciler, error) { manifests, err := gateway.LoadGatewayManifests(manifestsDir) if err != nil { @@ -1492,6 +1494,7 @@ func NewGatewayReconciler( keycloakClient: kcClient, keycloakConfig: keycloakConfig, exposure: exposurePort, + databaseSecret: databaseSecret, }, nil } @@ -1530,7 +1533,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. forgetGatewayProvisionObservation(event.ResourceID) var deleteDBConfig databaseConfig var deleteErrs []error - if gw.DatabaseId != "" { + if r.databaseSecret != "" || gw.DatabaseId != "" { var dbErr error deleteDBConfig, dbErr = r.resolveDatabaseConfig(ctx, gw) if dbErr != nil { @@ -1711,7 +1714,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. } var dbConfig databaseConfig - if gw.DatabaseId != "" { + if r.databaseSecret != "" || gw.DatabaseId != "" { var resolveErr error dbConfig, resolveErr = r.resolveDatabaseConfig(ctx, gw) if resolveErr != nil { @@ -2504,6 +2507,16 @@ func (r *GatewayReconciler) resolveReleaseImage(ctx context.Context, gw *pb.Gate } func (r *GatewayReconciler) resolveDatabaseConfig(ctx context.Context, gw *pb.Gateway) (databaseConfig, error) { + if r.databaseSecret != "" { + return databaseConfig{ + Provider: "external", + ExternalDB: gateway.ExternalDBConfig{ + CredentialsNamespace: r.controlPlaneNamespace, + CredentialsSecretName: r.databaseSecret, + }, + }, nil + } + if gw.DatabaseId == "" { return databaseConfig{}, fmt.Errorf("gateway has no database_id; assign a ManagedDatabase to the gateway") } diff --git a/components/control-plane/manifests/gateway/deployment.yaml b/components/control-plane/manifests/gateway/deployment.yaml index 6c064ff34..ffb9d4622 100644 --- a/components/control-plane/manifests/gateway/deployment.yaml +++ b/components/control-plane/manifests/gateway/deployment.yaml @@ -73,6 +73,9 @@ spec: - name: tls-cert mountPath: /etc/openshell-tls/server readOnly: true + - name: database-ca + mountPath: /etc/openshell-db + readOnly: true - name: tmp mountPath: /tmp ports: @@ -127,5 +130,13 @@ spec: secret: secretName: openshell-server-tls defaultMode: 288 + - name: database-ca + secret: + secretName: openshell-gateway-db-credentials + optional: true + defaultMode: 288 + items: + - key: sslrootcert + path: ca.crt - name: tmp emptyDir: {} diff --git a/deploy/components/gateway-database-admin-secret/README.md b/deploy/components/gateway-database-admin-secret/README.md new file mode 100644 index 000000000..3b3adbdcd --- /dev/null +++ b/deploy/components/gateway-database-admin-secret/README.md @@ -0,0 +1,84 @@ +# Gateway database admin Secret + +Add this component to the Kustomize overlay that deploys the controller. Set the +namespace to the same value as `HYPERSHELL_NAMESPACE`. Change the remote key and +`ClusterSecretStore` reference for your environment. For an overlay at +`deploy//kustomization.yaml`, use this relative path: + +```yaml +components: + - ../components/gateway-database-admin-secret +``` + +Terraform can create the cloud PostgreSQL server, network rules, admin account, +and secret-manager entry. Install External Secrets Operator and configure the +`gateway-databases` ClusterSecretStore with workload identity and read access to +that entry. The [ESO provider documentation](https://external-secrets.io/latest/provider/aws-secrets-manager/) +describes the AWS setup. Other providers use the same ExternalSecret contract. + +The remote entry must be a JSON object with these fields: + +| Field | Value | +| --- | --- | +| `host` | PostgreSQL DNS name that matches its server certificate | +| `port` | PostgreSQL port, for example `5432` | +| `user` | Provisioning account name | +| `password` | Provisioning account password | +| `sslrootcert` | PEM CA bundle for the server certificate | +| `dbname` | Optional admin database; default `postgres` | + +Keep credential values out of Git and Terraform output. If Terraform manages a +password, protect its state as a credential store. Use the cloud service's secret +integration where available. The repository needs only the store reference, +remote key, and controller environment variable. + +The admin account needs `CREATEDB`, `CREATEROLE`, and permission to manage each +gateway role and terminate its sessions (`pg_signal_backend` on PostgreSQL). +The controller grants each gateway role to the admin account so that a +non-superuser can create and drop the gateway database. Cloud services can impose +additional restrictions. Verify these permissions on your chosen service before +production use. The tests use PostgreSQL 18 with a non-superuser account. + +Apply the store and ExternalSecret before the controller configuration. Wait for +`ExternalSecret/gateway-database-admin` to report `Ready=True`. A missing or +invalid Secret makes gateway provisioning fail and retry. There is no fallback +to an in-cluster database. Both database connections use `verify-full`. The +gateway receives only its own credentials and the public CA bundle. + +The API keeps its existing ManagedDatabase records and `Gateway.database_id` +behavior. These records do not select the server while the override is enabled. +This component does not move existing gateway data. Use a new installation, or +complete a separate data migration before enabling or disabling it. + +## Credential and CA updates + +ESO refreshes the Kubernetes Secret every minute. The controller reads current +admin credentials for each database operation; no controller restart is required +after an admin password update. A Secret update alone does not enqueue all +gateways. Running gateways skip full provisioning. For a CA change, update the +`sslrootcert` field in each existing gateway's `openshell-gateway-db-credentials` +Secret with the public bundle and restart its `openshell-gateway` Deployment. +A GitOps job can perform these two operations without access to the admin password. +Keep both old and new CA certificates in the bundle during a CA transition. Changing the server host requires a data migration. + +## Failed deletion + +A failed database cleanup returns an error and records an `IncompleteFinalization` +warning Event. The live controller queue retries the deletion. Fix credentials, +network access, or SQL permissions and check that the database and role disappear. +The retry queue does not survive a controller restart. + +After a restart, use the gateway ID from the Event to check the shared PostgreSQL +server. The database and role names are `gw_` plus the lowercase gateway ID. Verify +that the gateway is absent from the API and that the database is no longer in use. +Connect with the admin account through a protected credential file. Drop the +identified database with `DROP DATABASE "gw_" WITH (FORCE)`, then drop the role +with `DROP ROLE "gw_"`. Do not drop the shared server. Check both `pg_database` +and `pg_roles` to confirm removal. + +## Tests + +Run `bash scripts/test-controller-database-secret.sh` for real PostgreSQL lifecycle +and TLS tests. The `secret` Kind CI case applies this ExternalSecret through ESO's +Fake provider, then runs the deployed gateway API and CLI tests. The Fake provider +is only a test fixture. It does not test cloud identity or cloud network access. diff --git a/deploy/components/gateway-database-admin-secret/external-secret.yaml b/deploy/components/gateway-database-admin-secret/external-secret.yaml new file mode 100644 index 000000000..f260b76dd --- /dev/null +++ b/deploy/components/gateway-database-admin-secret/external-secret.yaml @@ -0,0 +1,23 @@ +apiVersion: external-secrets.io/v1 +kind: ExternalSecret +metadata: + name: gateway-database-admin + namespace: hypershell-system +spec: + refreshPolicy: Periodic + refreshInterval: 1m + secretStoreRef: + kind: ClusterSecretStore + name: gateway-databases + target: + name: gateway-database-admin + creationPolicy: Owner + deletionPolicy: Retain + template: + engineVersion: v2 + mergePolicy: Merge + data: + sslmode: verify-full + dataFrom: + - extract: + key: hypershell/production/gateway-database diff --git a/deploy/components/gateway-database-admin-secret/kustomization.yaml b/deploy/components/gateway-database-admin-secret/kustomization.yaml new file mode 100644 index 000000000..b3eeef26b --- /dev/null +++ b/deploy/components/gateway-database-admin-secret/kustomization.yaml @@ -0,0 +1,21 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component +resources: + - external-secret.yaml +patches: + - target: + kind: Deployment + name: hypershell-controller + patch: |- + apiVersion: apps/v1 + kind: Deployment + metadata: + name: hypershell-controller + spec: + template: + spec: + containers: + - name: controller + env: + - name: GATEWAY_DATABASE_ADMIN_SECRET_NAME + value: gateway-database-admin diff --git a/scripts/test-controller-database-secret.sh b/scripts/test-controller-database-secret.sh new file mode 100755 index 000000000..a57e0aeba --- /dev/null +++ b/scripts/test-controller-database-secret.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Run the controller database tests against a disposable TLS PostgreSQL server. +set -euo pipefail +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +ENGINE=${CONTAINER_ENGINE:-$(command -v podman || command -v docker)} +FIXTURE=$(mktemp -d) +NAME="hypershell-database-test-$$" +cleanup() { + "$ENGINE" rm -f "$NAME" >/dev/null 2>&1 || true + rm -rf "$FIXTURE" +} +trap cleanup EXIT +bash "$ROOT/tests/fixtures/controller-database/generate.sh" "$FIXTURE" +"$ENGINE" run -d --name "$NAME" -p 127.0.0.1::5432 \ + -e POSTGRES_PASSWORD=fixture-root --entrypoint sh \ + -v "$FIXTURE:/fixture:Z" \ + -v "$FIXTURE/init.sql:/docker-entrypoint-initdb.d/init.sql:Z" \ + docker.io/library/postgres:18@sha256:a02db8cac496f15b094798a38254f14d6e00741f709360e5e00bb6668ea31636 \ + -c 'cp /fixture/server.key /tmp/server.key && chown postgres:postgres /tmp/server.key && chmod 600 /tmp/server.key && exec docker-entrypoint.sh postgres -c ssl=on -c ssl_cert_file=/fixture/server.crt -c ssl_key_file=/tmp/server.key' \ + >/dev/null +for _ in $(seq 1 60); do + if "$ENGINE" exec "$NAME" pg_isready -h 127.0.0.1 -U postgres >/dev/null 2>&1; then break; fi + sleep 1 +done +"$ENGINE" exec "$NAME" pg_isready -h 127.0.0.1 -U postgres >/dev/null +export TEST_DATABASE_PORT +TEST_DATABASE_PORT=$("$ENGINE" port "$NAME" 5432/tcp | awk -F: '{print $NF}') +export TEST_DATABASE_CA="$FIXTURE/ca.crt" +cd "$ROOT/components/control-plane" +go test -tags=integration -count=1 -v ./internal/gateway -run '^TestControllerDatabaseLifecycle$' diff --git a/specs/platform/openshell-gateway-database-external.spec.md b/specs/platform/openshell-gateway-database-external.spec.md index 34dd599b1..842605a10 100644 --- a/specs/platform/openshell-gateway-database-external.spec.md +++ b/specs/platform/openshell-gateway-database-external.spec.md @@ -1,6 +1,6 @@ # OpenShell Gateway Database Specification - External Provider -**Date:** 2026-09-07 +**Date:** 2026-09-15 **Status:** Active **Parent:** [`openshell-gateway-database.spec.md`](./openshell-gateway-database.spec.md) - gateway database provisioning @@ -26,6 +26,138 @@ required and no in-cluster PostgreSQL workload is created. PostgreSQL is the only supported backend. +### Requirement: Controller Database Admin Secret + +`GATEWAY_DATABASE_ADMIN_SECRET_NAME` MAY select a PostgreSQL admin Secret in +`HYPERSHELL_NAMESPACE`. A non-empty value SHALL select the controller-local +provisioning path. The value SHALL be a valid Secret name, without a namespace +prefix. The controller SHALL skip its `ManagedDatabase` watch, database lookup, +and CNPG startup check in this mode. It SHALL use the external SQL provisioner +for gateways assigned to this controller. It SHALL NOT provision a server. + +#### Scenario: Override enabled + +- GIVEN a controller with a configured admin Secret name +- WHEN it provisions a gateway +- THEN it SHALL use that Secret, even if the gateway has a `database_id` +- AND it SHALL create a separate SQL database and login role for that gateway +- AND it SHALL NOT read or provision a `ManagedDatabase` + +#### Scenario: Override absent + +- GIVEN the variable is unset or empty +- WHEN the controller starts +- THEN the existing database selection and watch behavior SHALL remain active + +### Requirement: Admin Credential Synchronization + +The Secret SHALL contain `host`, `port`, `user`, `password`, and `sslrootcert`. +`sslrootcert` SHALL contain a PEM CA bundle. `dbname` MAY select the maintenance +database and defaults to `postgres`. `sslmode` defaults to `verify-full` and +SHALL NOT select a weaker mode for this override. The controller SHALL read the +current Secret for each database provisioning or cleanup attempt. It SHALL NOT +write the admin Secret or log credential values. + +The provisioning account SHALL have `CREATEDB`, `CREATEROLE`, and permission to +manage the gateway roles and terminate their sessions. The controller SHALL grant +each gateway role to that account so a non-superuser can create its database. + +#### Scenario: Non-superuser provisioning account + +- GIVEN a PostgreSQL account with the required privileges and no superuser access +- WHEN the controller provisions and deletes a gateway +- THEN it SHALL create and remove the gateway database and role successfully + +The Secret MAY be synchronized from a secret manager by External Secrets +Operator. GitOps configuration SHALL contain Secret references and field mappings; +it SHALL NOT contain production passwords. Provisioning SHALL fail and retry +when credentials are unavailable or invalid. It SHALL NOT use another database +provider as a fallback. + +#### Scenario: Secret arrives after the controller + +- GIVEN the selected Secret does not exist +- WHEN the controller receives a gateway +- THEN provisioning SHALL fail without creating a database server +- WHEN synchronization creates a valid Secret +- THEN a later retry SHALL provision the gateway + +#### Scenario: Admin password changes + +- GIVEN the server password and synchronized Secret have been updated +- WHEN a provisioning or cleanup attempt starts +- THEN it SHALL use the current credentials without a controller restart + +### Requirement: Gateway Database Certificate Verification + +In override mode, both admin and gateway connections SHALL use `verify-full`. +The controller SHALL copy the CA bundle into `openshell-gateway-db-credentials`, +mount that key read-only in the gateway, and reference its path in the gateway +DSN. It SHALL NOT copy admin credentials into the gateway namespace. This CA +propagation SHALL also preserve `verify-ca` and `verify-full` when explicitly +selected through the existing external provider. + +#### Scenario: Trusted server + +- GIVEN a server certificate signed by the configured CA with a matching hostname +- WHEN the controller and gateway connect +- THEN both connections SHALL verify the certificate and succeed + +#### Scenario: Untrusted server or wrong hostname + +- GIVEN an untrusted server certificate or a hostname mismatch +- WHEN the controller or gateway connects +- THEN the connection SHALL fail without a TLS downgrade + +### Requirement: Observable Database Cleanup Failures + +For the override, gateway deletion SHALL remove its SQL database and role. +An absent object SHALL count as already removed. A failed existence query, +connection, or DROP statement SHALL NOT count as successful cleanup. The +controller SHALL return the error to the gateway retry queue and record an +`IncompleteFinalization` Warning Event in the controller namespace. The Event +SHALL identify the gateway and Secret reference without credential values. + +Retries use the existing in-memory queue. A controller restart can lose a pending +delete event; this change SHALL NOT claim recovery across restarts. The operator +runbook SHALL describe how to find and remove remaining SQL resources. The +controller SHALL never delete the admin Secret or the PostgreSQL server. + +#### Scenario: Database is unavailable during deletion + +- GIVEN a gateway delete event and an unavailable database +- WHEN cleanup fails +- THEN the controller SHALL record the failure and return an error +- WHEN the database becomes available during the same controller process +- THEN a queued retry SHALL remove the database and role + +#### Scenario: Cleanup query fails + +- GIVEN a failed database or role existence query +- WHEN cleanup processes the result +- THEN it SHALL return an error instead of reporting that the object is absent + +### Requirement: Compatibility and Deployment Scope + +The override SHALL preserve the API, schema, and SDK contracts. The API still +assigns `database_id`; its default `deployment` provider still creates a +`ManagedDatabase` record per gateway. This controller ignores and retains those +records. Database inventory therefore does not represent its SQL databases. +Existing API registration requirements for `cnpg` and `external` still apply. + +Configure the override before provisioning gateways that use it. The configured +server must be correct for all assigned gateways. Enabling the override, changing +the endpoint, or removing the override SHALL NOT migrate data. The contracts below +describe the existing path; the override requirements above take precedence for +controllers that set the variable. + +#### Scenario: Existing API clients + +- GIVEN an existing client and the API's default database provider +- WHEN the client creates a gateway for a controller with the override +- THEN the API SHALL keep its existing request, response, and record behavior +- AND the controller SHALL use its configured Secret for SQL provisioning + ### Contracts this spec builds on External mode participates in the shared gateway-database contracts defined by the @@ -74,7 +206,7 @@ responsibility, out-of-band, before an `external` ManagedDatabase is created: 5. **Server-side log verbosity restricted.** `CREATE ROLE` and `ALTER ROLE` statements carry the plaintext password in the statement text (the PostgreSQL wire protocol has no separate credential-binding channel for these statements). - Operators SHOULD set `log_statement` to `'mod'` or lower, or enable server-side + Operators SHOULD set `log_statement` to `'none'`, or enable server-side log redaction, on any external server used with this provider. HyperShell redacts credentials in its own application logs and error messages; server-side redaction is the operator's responsibility and HyperShell cannot enforce it. @@ -544,8 +676,7 @@ files in the tenant namespace. `sslmode=verify-full` is the recommended hardenin is opt-in: when the admin Secret carries `sslrootcert`, the reconciler SHALL propagate the CA into the tenant namespace and set `verify-full`, which requires the gateway workload to mount and reference the CA. Distributing/mounting the CA into the -gateway workload is tracked as a follow-up; v1 MAY ship with `require` as the -enforced default and `verify-full` behind that follow-up. +gateway workload SHALL use the mounted CA without reducing the requested TLS mode. #### Scenario: Credentials Secret written with the default TLS mode diff --git a/tests/e2e/e2e-openshell.sh b/tests/e2e/e2e-openshell.sh index bfc9f6d72..811f1dfb2 100755 --- a/tests/e2e/e2e-openshell.sh +++ b/tests/e2e/e2e-openshell.sh @@ -704,11 +704,18 @@ if [[ -n "$GW_DB_ID" ]]; then fi if [[ -n "$DB_GW_NAMESPACE" ]]; then dim " Database namespace: ${DB_GW_NAMESPACE}" -else +elif [[ -z "${GATEWAY_DATABASE_ADMIN_SECRET_NAME:-}" ]]; then fail_test "Could not resolve database namespace for gateway ${GW_ID}" fi -if [[ "${DB_PROVIDER}" == "cnpg" ]]; then +if [[ -n "${GATEWAY_DATABASE_ADMIN_SECRET_NAME:-}" ]]; then + DB_TEST_NAME=$($CLI -n "$GW_NAMESPACE" get secret openshell-gateway-db-credentials -o jsonpath='{.data.dbname}' | base64 -d) + if bash tests/fixtures/controller-database/assert-kind.sh provision "$DB_TEST_NAME" "$GW_NAMESPACE" "$DB_GW_NAMESPACE"; then + pass "Secret override uses the shared TLS server and bypasses ManagedDatabase provisioning" + else + fail_test "Secret override database checks failed" + fi +elif [[ "${DB_PROVIDER}" == "cnpg" ]]; then # CNPG provider: verify Database CR, DatabaseRole CR, and client TLS CNPG_GW_NAMESPACE="${DB_GW_NAMESPACE}" CNPG_CR_NAME="gw-$(echo "${GW_ID}" | tr '[:upper:]' '[:lower:]')" @@ -1924,7 +1931,7 @@ else e2e_dump_namespace_gc_logs "${E2E_HS_NAMESPACE}" "$CLI" fi - if [[ "${DB_PROVIDER}" == "deployment" && -n "${GW_DB_ID:-}" ]]; then + if [[ "${DB_PROVIDER}" == "deployment" && -n "${GW_DB_ID:-}" && -z "${GATEWAY_DATABASE_ADMIN_SECRET_NAME:-}" ]]; then dim " Waiting for dedicated ManagedDatabase ${GW_DB_ID} and namespace ${DB_GW_NAMESPACE} to be deleted..." DB_GONE=false DB_GC_DEADLINE=$(($(date +%s) + E2E_GC_TIMEOUT)) @@ -1946,6 +1953,14 @@ else fi fi + if [[ -n "${GATEWAY_DATABASE_ADMIN_SECRET_NAME:-}" && -n "${DB_TEST_NAME:-}" ]]; then + if bash tests/fixtures/controller-database/assert-kind.sh delete "$DB_TEST_NAME"; then + pass "Secret override removed the gateway database and role" + else + fail_test "Secret override left a gateway database or role" + fi + fi + # 11a. Periodic reaper (NamespaceGCReconciler + recordGCEvent). Orphan namespace # was seeded after gateway provisioning; validate reap + Event without blocking # earlier steps on the sweep interval. diff --git a/tests/fixtures/controller-database/assert-kind.sh b/tests/fixtures/controller-database/assert-kind.sh new file mode 100644 index 000000000..3eec9db82 --- /dev/null +++ b/tests/fixtures/controller-database/assert-kind.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -euo pipefail +PHASE=${1:?phase required} +DB_NAME=${2:?database name required} +[[ "$DB_NAME" =~ ^gw_[a-z0-9]+$ ]] || { echo 'Invalid fixture database name' >&2; exit 1; } +if [[ "$PHASE" == provision ]]; then + GW_NAMESPACE=${3:?gateway namespace required} + DB_NAMESPACE=${4:-} + [[ $(kubectl -n "$GW_NAMESPACE" get secret openshell-gateway-db-credentials -o jsonpath='{.data.sslmode}' | base64 -d) == verify-full ]] + if [[ -n "$DB_NAMESPACE" ]] && kubectl get namespace "$DB_NAMESPACE" >/dev/null 2>&1; then + echo 'Secret override created a ManagedDatabase namespace' >&2 + exit 1 + fi + if kubectl -n "$GW_NAMESPACE" get deployment openshell-gateway-db >/dev/null 2>&1; then + echo 'Secret override created an in-cluster gateway database' >&2 + exit 1 + fi + COUNT=$(kubectl -n database-secret-test exec deployment/gateway-database -- \ + psql -U postgres -tAc "SELECT count(*) FROM pg_stat_ssl s JOIN pg_stat_activity a USING (pid) WHERE a.datname='$DB_NAME' AND s.ssl") + [[ "$COUNT" -gt 0 ]] || { echo 'Gateway has no live PostgreSQL TLS connection' >&2; exit 1; } +elif [[ "$PHASE" == delete ]]; then + for _ in $(seq 1 30); do + COUNT=$(kubectl -n database-secret-test exec deployment/gateway-database -- \ + psql -U postgres -tAc "SELECT (SELECT count(*) FROM pg_database WHERE datname='$DB_NAME') + (SELECT count(*) FROM pg_roles WHERE rolname='$DB_NAME')") + [[ "$COUNT" == 0 ]] && exit 0 + sleep 2 + done + echo 'Gateway database or role remains after deletion' >&2 + exit 1 +else + echo 'Unknown fixture assertion phase' >&2 + exit 1 +fi diff --git a/tests/fixtures/controller-database/generate.sh b/tests/fixtures/controller-database/generate.sh new file mode 100755 index 000000000..fd5dd6003 --- /dev/null +++ b/tests/fixtures/controller-database/generate.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# Generate disposable test credentials. These files must not enter Git. +set -euo pipefail +DIR=${1:?fixture directory required} +mkdir -p "$DIR" +chmod 755 "$DIR" +openssl req -x509 -newkey rsa:2048 -nodes -days 2 -subj /CN=fixture-ca \ + -keyout "$DIR/ca.key" -out "$DIR/ca.crt" >/dev/null 2>&1 +openssl req -newkey rsa:2048 -nodes -subj /CN=localhost \ + -keyout "$DIR/server.key" -out "$DIR/server.csr" >/dev/null 2>&1 +printf '%s\n' 'subjectAltName=DNS:localhost,DNS:gateway-database.database-secret-test.svc' > "$DIR/extensions" +openssl x509 -req -in "$DIR/server.csr" -CA "$DIR/ca.crt" -CAkey "$DIR/ca.key" \ + -CAcreateserial -days 2 -extfile "$DIR/extensions" -out "$DIR/server.crt" >/dev/null 2>&1 +# The test container copies the server key to a private file owned by PostgreSQL. +chmod 644 "$DIR/server.crt" "$DIR/ca.crt" +chmod 600 "$DIR/server.key" +cat > "$DIR/init.sql" <<'SQL' +CREATE ROLE provisioner LOGIN CREATEDB CREATEROLE PASSWORD 'fixture-admin'; +GRANT pg_signal_backend TO provisioner; +GRANT CREATE ON SCHEMA public TO provisioner; +SQL diff --git a/tests/fixtures/controller-database/install-kind.sh b/tests/fixtures/controller-database/install-kind.sh new file mode 100755 index 000000000..4a7ec5f7d --- /dev/null +++ b/tests/fixtures/controller-database/install-kind.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +# Install a disposable TLS server and synchronize its admin Secret through ESO. +set -euo pipefail +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd) +NS=${HYPERSHELL_NAMESPACE:-hypershell-system} +DIR=$(mktemp -d) +trap 'rm -rf "$DIR"' EXIT +bash "$ROOT/tests/fixtures/controller-database/generate.sh" "$DIR" +kubectl create namespace database-secret-test --dry-run=client -o yaml | kubectl apply -f - +kubectl -n database-secret-test create secret generic database-fixture \ + --from-file="$DIR/server.crt" --from-file="$DIR/server.key" --from-file="$DIR/init.sql" \ + --dry-run=client -o yaml | kubectl apply -f - +kubectl apply -f "$ROOT/tests/fixtures/controller-database/postgres.yaml" +kubectl -n database-secret-test rollout status deployment/gateway-database --timeout=180s +helm upgrade --install database-test-external-secrets external-secrets \ + --repo https://charts.external-secrets.io --version 0.20.4 \ + --namespace external-secrets --create-namespace --wait --timeout 180s >/dev/null +# The test uses ESO's Fake provider. Only disposable fixture values enter it. +jq -n --rawfile ca "$DIR/ca.crt" '{apiVersion:"external-secrets.io/v1",kind:"ClusterSecretStore", + metadata:{name:"gateway-databases"},spec:{provider:{fake:{data:[{ + key:"hypershell/production/gateway-database",value:({host:"gateway-database.database-secret-test.svc",port:"5432", + user:"provisioner",password:"fixture-admin",sslrootcert:$ca}|tojson)}]}}}}' | kubectl apply -f - +kubectl wait --for=condition=Ready clustersecretstore/gateway-databases --timeout=60s +sed "s/namespace: hypershell-system/namespace: ${NS}/" \ + "$ROOT/deploy/components/gateway-database-admin-secret/external-secret.yaml" | kubectl apply -f - +kubectl -n "$NS" wait --for=condition=Ready externalsecret/gateway-database-admin --timeout=90s +# Verify that a secret-manager update reaches the same Kubernetes Secret. +kubectl -n database-secret-test exec deployment/gateway-database -- \ + psql -U postgres -v ON_ERROR_STOP=1 -c "ALTER ROLE provisioner PASSWORD 'fixture-rotated'" >/dev/null +kubectl get clustersecretstore gateway-databases -o json | \ + jq '.spec.provider.fake.data[0].value |= (fromjson | .password="fixture-rotated" | tojson)' | kubectl apply -f - +kubectl -n "$NS" annotate externalsecret gateway-database-admin force-sync="$(date +%s)" --overwrite >/dev/null +SYNCED=false +for _ in $(seq 1 45); do + VALUE=$(kubectl -n "$NS" get secret gateway-database-admin -o jsonpath='{.data.password}' | base64 -d) + if [[ "$VALUE" == fixture-rotated ]]; then SYNCED=true; break; fi + sleep 2 +done +[[ "$SYNCED" == true ]] || { echo 'ESO did not synchronize the new password' >&2; exit 1; } +kubectl -n "$NS" set env deployment/hypershell-controller GATEWAY_DATABASE_ADMIN_SECRET_NAME=gateway-database-admin +kubectl -n "$NS" rollout status deployment/hypershell-controller --timeout=180s diff --git a/tests/fixtures/controller-database/postgres.yaml b/tests/fixtures/controller-database/postgres.yaml new file mode 100644 index 000000000..4aa8d0e03 --- /dev/null +++ b/tests/fixtures/controller-database/postgres.yaml @@ -0,0 +1,58 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: gateway-database + namespace: database-secret-test +spec: + replicas: 1 + selector: + matchLabels: + app: gateway-database + template: + metadata: + labels: + app: gateway-database + spec: + containers: + - name: postgres + image: postgres:18@sha256:a02db8cac496f15b094798a38254f14d6e00741f709360e5e00bb6668ea31636 + command: [sh, -c] + args: + - >- + cp /fixture/server.key /tmp/server.key && + chown postgres:postgres /tmp/server.key && chmod 600 /tmp/server.key && + exec docker-entrypoint.sh postgres -c ssl=on + -c ssl_cert_file=/fixture/server.crt -c ssl_key_file=/tmp/server.key + env: + - name: POSTGRES_PASSWORD + value: fixture-root + ports: + - containerPort: 5432 + readinessProbe: + exec: + command: [pg_isready, -h, 127.0.0.1, -U, postgres] + periodSeconds: 2 + volumeMounts: + - name: fixture + mountPath: /fixture + readOnly: true + - name: fixture + mountPath: /docker-entrypoint-initdb.d + readOnly: true + volumes: + - name: fixture + secret: + secretName: database-fixture + defaultMode: 292 +--- +apiVersion: v1 +kind: Service +metadata: + name: gateway-database + namespace: database-secret-test +spec: + selector: + app: gateway-database + ports: + - port: 5432 + targetPort: 5432