diff --git a/.forbidden-terms-whitelist.json b/.forbidden-terms-whitelist.json index 2fb2bb8f..aeb97dae 100644 --- a/.forbidden-terms-whitelist.json +++ b/.forbidden-terms-whitelist.json @@ -31,12 +31,12 @@ }, { "filename": "specs/platform/global-architecture.spec.md", - "line": 1164, + "line": 1190, "rationale": "Example directory names 'vteam-stage'/'vteam-uat' in a GitOps directory-tree illustration reference real external cluster environments; they are example paths, not the discouraged term." }, { "filename": "CLAUDE.md", "line": 125, - "rationale": "The em dash appears inside a convention rule that documents the character itself as a forbidden term; it must be shown literally to be unambiguous." + "rationale": "The em dash appears inside the convention rule that documents the character itself as a forbidden term; it must be shown literally to be unambiguous." } ] diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 34ad440c..4fef606b 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -349,7 +349,7 @@ jobs: strategy: fail-fast: false matrix: - database-provider: [deployment, cnpg] + database-provider: [deployment, cnpg, external] # 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 diff --git a/Makefile b/Makefile index 9329fbc9..7a3ba0ce 100644 --- a/Makefile +++ b/Makefile @@ -112,6 +112,11 @@ help: @echo "" @echo " Kind Specific" @echo " kind-env Print environment variables for local setup" + @echo " kind-up Create cluster + deploy all components (OIDC enabled)" + @echo " LOCAL_IMAGES=true: build from working tree (default)" + @echo " LOCAL_IMAGES=true BUILD_SOURCE=baseline: build from origin/main" + @echo " LOCAL_IMAGES=true KIND_SKIP_BUILD=true: reuse existing local images" + @echo " KIND_SKIP_SEED=true: defer seeding (run kind-seed later)" @echo " kind-seed Seed platform resources into a running cluster" @echo " SKIP_SEED=true: defer seeding during kind-up / openshift-up" @echo " SEED_STRICT=true: fail the command if seeding is incomplete" diff --git a/components/api-server/openapi/openapi.managedDatabases.yaml b/components/api-server/openapi/openapi.managedDatabases.yaml index 92ff98d7..603d2206 100644 --- a/components/api-server/openapi/openapi.managedDatabases.yaml +++ b/components/api-server/openapi/openapi.managedDatabases.yaml @@ -244,6 +244,12 @@ components: type: string connection_secret: type: string + description: >- + For provider "external": the NAMESPACE holding the admin credentials + Secret, not a Secret name. Must be a bare namespace name (no "/") + prefixed with "hypershell-managed-db-" and a valid DNS-1123 label. + The Secret inside it always has the fixed name + "hypershell-managed-db-credentials". Ignored by other providers. status: type: string # NEW SCHEMA START @@ -276,6 +282,12 @@ components: type: string connection_secret: type: string + description: >- + For provider "external": the NAMESPACE holding the admin credentials + Secret, not a Secret name. Must be a bare namespace name (no "/") + prefixed with "hypershell-managed-db-" and a valid DNS-1123 label. + The Secret inside it always has the fixed name + "hypershell-managed-db-credentials". Ignored by other providers. status: type: string parameters: diff --git a/components/api-server/pkg/api/grpc/hypershell/v1/managed_databases.pb.go b/components/api-server/pkg/api/grpc/hypershell/v1/managed_databases.pb.go index b53236fc..86fa11bc 100644 --- a/components/api-server/pkg/api/grpc/hypershell/v1/managed_databases.pb.go +++ b/components/api-server/pkg/api/grpc/hypershell/v1/managed_databases.pb.go @@ -22,17 +22,21 @@ const ( ) type ManagedDatabase struct { - state protoimpl.MessageState `protogen:"open.v1"` - Metadata *ObjectReference `protobuf:"bytes,1,opt,name=metadata,proto3" json:"metadata,omitempty"` - Name string `protobuf:"bytes,2,opt,name=name,proto3" json:"name,omitempty"` - Provider string `protobuf:"bytes,4,opt,name=provider,proto3" json:"provider,omitempty"` - Region *string `protobuf:"bytes,5,opt,name=region,proto3,oneof" json:"region,omitempty"` - Engine *string `protobuf:"bytes,6,opt,name=engine,proto3,oneof" json:"engine,omitempty"` - EngineVersion *string `protobuf:"bytes,7,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` - InstanceClass *string `protobuf:"bytes,8,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` - ConnectionSecret *string `protobuf:"bytes,9,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` - Status *string `protobuf:"bytes,10,opt,name=status,proto3,oneof" json:"status,omitempty"` - Namespace string `protobuf:"bytes,11,opt,name=namespace,proto3" json:"namespace,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Metadata *ObjectReference `protobuf:"bytes,1,opt,name=metadata,proto3" json:"metadata,omitempty"` + Name string `protobuf:"bytes,2,opt,name=name,proto3" json:"name,omitempty"` + Provider string `protobuf:"bytes,4,opt,name=provider,proto3" json:"provider,omitempty"` + Region *string `protobuf:"bytes,5,opt,name=region,proto3,oneof" json:"region,omitempty"` + Engine *string `protobuf:"bytes,6,opt,name=engine,proto3,oneof" json:"engine,omitempty"` + EngineVersion *string `protobuf:"bytes,7,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` + InstanceClass *string `protobuf:"bytes,8,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". + ConnectionSecret *string `protobuf:"bytes,9,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` + Status *string `protobuf:"bytes,10,opt,name=status,proto3,oneof" json:"status,omitempty"` + Namespace string `protobuf:"bytes,11,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -138,15 +142,19 @@ func (x *ManagedDatabase) GetNamespace() string { } type CreateManagedDatabaseRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` - Provider string `protobuf:"bytes,3,opt,name=provider,proto3" json:"provider,omitempty"` - Region *string `protobuf:"bytes,4,opt,name=region,proto3,oneof" json:"region,omitempty"` - Engine *string `protobuf:"bytes,5,opt,name=engine,proto3,oneof" json:"engine,omitempty"` - EngineVersion *string `protobuf:"bytes,6,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` - InstanceClass *string `protobuf:"bytes,7,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` - ConnectionSecret *string `protobuf:"bytes,8,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` - Status *string `protobuf:"bytes,9,opt,name=status,proto3,oneof" json:"status,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + Provider string `protobuf:"bytes,3,opt,name=provider,proto3" json:"provider,omitempty"` + Region *string `protobuf:"bytes,4,opt,name=region,proto3,oneof" json:"region,omitempty"` + Engine *string `protobuf:"bytes,5,opt,name=engine,proto3,oneof" json:"engine,omitempty"` + EngineVersion *string `protobuf:"bytes,6,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` + InstanceClass *string `protobuf:"bytes,7,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". + ConnectionSecret *string `protobuf:"bytes,8,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` + Status *string `protobuf:"bytes,9,opt,name=status,proto3,oneof" json:"status,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -370,16 +378,20 @@ func (x *GetManagedDatabaseResponse) GetManagedDatabase() *ManagedDatabase { } type UpdateManagedDatabaseRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` - Name *string `protobuf:"bytes,2,opt,name=name,proto3,oneof" json:"name,omitempty"` - Provider *string `protobuf:"bytes,4,opt,name=provider,proto3,oneof" json:"provider,omitempty"` - Region *string `protobuf:"bytes,5,opt,name=region,proto3,oneof" json:"region,omitempty"` - Engine *string `protobuf:"bytes,6,opt,name=engine,proto3,oneof" json:"engine,omitempty"` - EngineVersion *string `protobuf:"bytes,7,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` - InstanceClass *string `protobuf:"bytes,8,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` - ConnectionSecret *string `protobuf:"bytes,9,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` - Status *string `protobuf:"bytes,10,opt,name=status,proto3,oneof" json:"status,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` + Name *string `protobuf:"bytes,2,opt,name=name,proto3,oneof" json:"name,omitempty"` + Provider *string `protobuf:"bytes,4,opt,name=provider,proto3,oneof" json:"provider,omitempty"` + Region *string `protobuf:"bytes,5,opt,name=region,proto3,oneof" json:"region,omitempty"` + Engine *string `protobuf:"bytes,6,opt,name=engine,proto3,oneof" json:"engine,omitempty"` + EngineVersion *string `protobuf:"bytes,7,opt,name=engine_version,json=engineVersion,proto3,oneof" json:"engine_version,omitempty"` + InstanceClass *string `protobuf:"bytes,8,opt,name=instance_class,json=instanceClass,proto3,oneof" json:"instance_class,omitempty"` + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". + ConnectionSecret *string `protobuf:"bytes,9,opt,name=connection_secret,json=connectionSecret,proto3,oneof" json:"connection_secret,omitempty"` + Status *string `protobuf:"bytes,10,opt,name=status,proto3,oneof" json:"status,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } diff --git a/components/api-server/pkg/api/openapi/api/openapi.yaml b/components/api-server/pkg/api/openapi/api/openapi.yaml index 65e687da..57825045 100644 --- a/components/api-server/pkg/api/openapi/api/openapi.yaml +++ b/components/api-server/pkg/api/openapi/api/openapi.yaml @@ -2289,6 +2289,11 @@ components: instance_class: type: string connection_secret: + description: "For provider \"external\": the NAMESPACE holding the admin\ + \ credentials Secret, not a Secret name. Must be a bare namespace name\ + \ (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123\ + \ label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\"\ + . Ignored by other providers." type: string status: type: string @@ -2382,6 +2387,11 @@ components: instance_class: type: string connection_secret: + description: "For provider \"external\": the NAMESPACE holding the admin\ + \ credentials Secret, not a Secret name. Must be a bare namespace name\ + \ (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123\ + \ label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\"\ + . Ignored by other providers." type: string status: type: string diff --git a/components/api-server/pkg/api/openapi/docs/ManagedDatabase.md b/components/api-server/pkg/api/openapi/docs/ManagedDatabase.md index 2f7b4c8c..787b7ddd 100644 --- a/components/api-server/pkg/api/openapi/docs/ManagedDatabase.md +++ b/components/api-server/pkg/api/openapi/docs/ManagedDatabase.md @@ -16,7 +16,7 @@ Name | Type | Description | Notes **Engine** | Pointer to **string** | | [optional] **EngineVersion** | Pointer to **string** | | [optional] **InstanceClass** | Pointer to **string** | | [optional] -**ConnectionSecret** | Pointer to **string** | | [optional] +**ConnectionSecret** | Pointer to **string** | For provider \"external\": the NAMESPACE holding the admin credentials Secret, not a Secret name. Must be a bare namespace name (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123 label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\". Ignored by other providers. | [optional] **Status** | Pointer to **string** | | [optional] ## Methods diff --git a/components/api-server/pkg/api/openapi/docs/ManagedDatabasePatchRequest.md b/components/api-server/pkg/api/openapi/docs/ManagedDatabasePatchRequest.md index 33b2af4a..bf5029e7 100644 --- a/components/api-server/pkg/api/openapi/docs/ManagedDatabasePatchRequest.md +++ b/components/api-server/pkg/api/openapi/docs/ManagedDatabasePatchRequest.md @@ -10,7 +10,7 @@ Name | Type | Description | Notes **Engine** | Pointer to **string** | | [optional] **EngineVersion** | Pointer to **string** | | [optional] **InstanceClass** | Pointer to **string** | | [optional] -**ConnectionSecret** | Pointer to **string** | | [optional] +**ConnectionSecret** | Pointer to **string** | For provider \"external\": the NAMESPACE holding the admin credentials Secret, not a Secret name. Must be a bare namespace name (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123 label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\". Ignored by other providers. | [optional] **Status** | Pointer to **string** | | [optional] ## Methods diff --git a/components/api-server/pkg/api/openapi/model_managed_database.go b/components/api-server/pkg/api/openapi/model_managed_database.go index 9aa68849..87f41121 100644 --- a/components/api-server/pkg/api/openapi/model_managed_database.go +++ b/components/api-server/pkg/api/openapi/model_managed_database.go @@ -22,20 +22,21 @@ var _ MappedNullable = &ManagedDatabase{} // ManagedDatabase struct for ManagedDatabase type ManagedDatabase struct { - Id *string `json:"id,omitempty"` - Kind *string `json:"kind,omitempty"` - Href *string `json:"href,omitempty"` - CreatedAt *time.Time `json:"created_at,omitempty"` - UpdatedAt *time.Time `json:"updated_at,omitempty"` - Name string `json:"name"` - Provider string `json:"provider"` - Namespace *string `json:"namespace,omitempty"` - Region *string `json:"region,omitempty"` - Engine *string `json:"engine,omitempty"` - EngineVersion *string `json:"engine_version,omitempty"` - InstanceClass *string `json:"instance_class,omitempty"` - ConnectionSecret *string `json:"connection_secret,omitempty"` - Status *string `json:"status,omitempty"` + Id *string `json:"id,omitempty"` + Kind *string `json:"kind,omitempty"` + Href *string `json:"href,omitempty"` + CreatedAt *time.Time `json:"created_at,omitempty"` + UpdatedAt *time.Time `json:"updated_at,omitempty"` + Name string `json:"name"` + Provider string `json:"provider"` + Namespace *string `json:"namespace,omitempty"` + Region *string `json:"region,omitempty"` + Engine *string `json:"engine,omitempty"` + EngineVersion *string `json:"engine_version,omitempty"` + InstanceClass *string `json:"instance_class,omitempty"` + // For provider \"external\": the NAMESPACE holding the admin credentials Secret, not a Secret name. Must be a bare namespace name (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123 label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\". Ignored by other providers. + ConnectionSecret *string `json:"connection_secret,omitempty"` + Status *string `json:"status,omitempty"` } type _ManagedDatabase ManagedDatabase diff --git a/components/api-server/pkg/api/openapi/model_managed_database_patch_request.go b/components/api-server/pkg/api/openapi/model_managed_database_patch_request.go index 51f918fa..d1f6d8ea 100644 --- a/components/api-server/pkg/api/openapi/model_managed_database_patch_request.go +++ b/components/api-server/pkg/api/openapi/model_managed_database_patch_request.go @@ -19,12 +19,13 @@ var _ MappedNullable = &ManagedDatabasePatchRequest{} // ManagedDatabasePatchRequest struct for ManagedDatabasePatchRequest type ManagedDatabasePatchRequest struct { - Name *string `json:"name,omitempty"` - Provider *string `json:"provider,omitempty"` - Region *string `json:"region,omitempty"` - Engine *string `json:"engine,omitempty"` - EngineVersion *string `json:"engine_version,omitempty"` - InstanceClass *string `json:"instance_class,omitempty"` + Name *string `json:"name,omitempty"` + Provider *string `json:"provider,omitempty"` + Region *string `json:"region,omitempty"` + Engine *string `json:"engine,omitempty"` + EngineVersion *string `json:"engine_version,omitempty"` + InstanceClass *string `json:"instance_class,omitempty"` + // For provider \"external\": the NAMESPACE holding the admin credentials Secret, not a Secret name. Must be a bare namespace name (no \"/\") prefixed with \"hypershell-managed-db-\" and a valid DNS-1123 label. The Secret inside it always has the fixed name \"hypershell-managed-db-credentials\". Ignored by other providers. ConnectionSecret *string `json:"connection_secret,omitempty"` Status *string `json:"status,omitempty"` } diff --git a/components/api-server/plugins/gateways/placement.go b/components/api-server/plugins/gateways/placement.go index ce20bfd0..325a6ea7 100644 --- a/components/api-server/plugins/gateways/placement.go +++ b/components/api-server/plugins/gateways/placement.go @@ -14,6 +14,13 @@ type DatabaseLookup interface { FindSole(ctx context.Context) (databaseID string, err error) } +// DatabaseSelector resolves placement when more than one candidate +// ManagedDatabase is allowed to exist. FindOldest returns the earliest-created +// candidate, or "" when none exist. +type DatabaseSelector interface { + FindOldest(ctx context.Context) (databaseID string, err error) +} + type DatabaseCreator interface { CreateForGateway(ctx context.Context, gatewayName string) (databaseID string, err error) } @@ -94,6 +101,36 @@ func NewDeploymentPlacement(dbs DatabaseCreator) PlacementResolver { return &deploymentPlacement{dbs: dbs} } +// externalPlacement assigns every new gateway to the first-created external +// ManagedDatabase. More than one registration is not an error: an operator may +// register a second external server ahead of a migration without intending to +// move where new gateways land. Only the empty result is rejected. +// +// Selection happens at gateway creation only. Once assigned, database_id is +// fixed for the gateway's lifetime, so a later registration never relocates an +// existing gateway. +type externalPlacement struct { + dbs DatabaseSelector +} + +func NewExternalPlacement(dbs DatabaseSelector) PlacementResolver { + return &externalPlacement{dbs: dbs} +} + +func (p *externalPlacement) Resolve(ctx context.Context, gw *Gateway) error { + gw.DatabaseId = "" + + dbID, err := p.dbs.FindOldest(ctx) + if err != nil { + return newPlacementDependencyError("resolve external database", err) + } + if dbID == "" { + return newPlacementValidationError("no external ManagedDatabase is registered; register one before creating gateways") + } + gw.DatabaseId = dbID + return nil +} + func (p *deploymentPlacement) Resolve(ctx context.Context, gw *Gateway) error { // A deployment database is dedicated to exactly one gateway. Ignore any // client-provided database_id and always create the server-owned resource. diff --git a/components/api-server/plugins/gateways/placement_test.go b/components/api-server/plugins/gateways/placement_test.go index 6f6c7316..9681b32b 100644 --- a/components/api-server/plugins/gateways/placement_test.go +++ b/components/api-server/plugins/gateways/placement_test.go @@ -4,6 +4,10 @@ import ( "context" "errors" "testing" + "time" + + "github.com/openshift-online/hypershell/components/api-server/plugins/managedDatabases" + "github.com/openshift-online/rh-trex-ai/pkg/api" ) type fakeDatabaseLookup struct { @@ -18,6 +22,20 @@ func (f *fakeDatabaseLookup) FindSole(ctx context.Context) (string, error) { return f.sole, nil } +type fakeDatabaseSelector struct { + oldest string + oldestErr error + calls int +} + +func (f *fakeDatabaseSelector) FindOldest(ctx context.Context) (string, error) { + f.calls++ + if f.oldestErr != nil { + return "", f.oldestErr + } + return f.oldest, nil +} + type fakePlacementResolver struct { err error } @@ -162,3 +180,130 @@ func TestGatewayServiceMapsPlacementErrors(t *testing.T) { } }) } + +// --- external placement: oldest-wins --- + +// External placement owns database_id like every other mode: a caller-supplied +// value is discarded and replaced with the server-side selection. +func TestExternalPlacementIgnoresExplicitDatabaseID(t *testing.T) { + dbs := &fakeDatabaseSelector{oldest: "oldest-db-id"} + placement := NewExternalPlacement(dbs) + + gw := &Gateway{Name: "gw1", DatabaseId: "client-supplied-db-id"} + if err := placement.Resolve(context.Background(), gw); err != nil { + t.Fatalf("Resolve() unexpected error: %v", err) + } + if gw.DatabaseId != "oldest-db-id" { + t.Fatalf("DatabaseId = %q, want the server-selected ID", gw.DatabaseId) + } +} + +// More than one registered external ManagedDatabase is NOT an error: placement +// picks the first-created one rather than rejecting the gateway creation. +func TestExternalPlacementAcceptsMultipleDatabases(t *testing.T) { + dbs := &fakeDatabaseSelector{oldest: "first-created"} + placement := NewExternalPlacement(dbs) + + gw := &Gateway{Name: "gw1"} + if err := placement.Resolve(context.Background(), gw); err != nil { + t.Fatalf("Resolve() unexpected error: %v", err) + } + if gw.DatabaseId != "first-created" { + t.Fatalf("DatabaseId = %q, want %q", gw.DatabaseId, "first-created") + } + if dbs.calls != 1 { + t.Fatalf("FindOldest called %d times, want 1", dbs.calls) + } +} + +// Zero registered external ManagedDatabases is still rejected, as a validation +// error so the API returns 400 rather than 500. +func TestExternalPlacementRejectsWhenNoneRegistered(t *testing.T) { + placement := NewExternalPlacement(&fakeDatabaseSelector{oldest: ""}) + + err := placement.Resolve(context.Background(), &Gateway{Name: "gw1"}) + if err == nil { + t.Fatal("Resolve() = nil, want an error when no external ManagedDatabase is registered") + } + if !IsPlacementValidationError(err) { + t.Fatalf("Resolve() error = %v, want validation classification", err) + } +} + +func TestExternalPlacementLookupFailureIsDependencyError(t *testing.T) { + placement := NewExternalPlacement(&fakeDatabaseSelector{oldestErr: errors.New("database unavailable")}) + + err := placement.Resolve(context.Background(), &Gateway{Name: "gw1"}) + if err == nil { + t.Fatal("Resolve() = nil, want lookup error") + } + if IsPlacementValidationError(err) { + t.Fatalf("Resolve() error = %v, want dependency classification", err) + } +} + +// --- pickOldestManagedDatabase --- + +func mdb(id, provider string, createdAt time.Time) *managedDatabases.ManagedDatabase { + return &managedDatabases.ManagedDatabase{ + Meta: api.Meta{ID: id, CreatedAt: createdAt}, + Provider: provider, + } +} + +func TestPickOldestManagedDatabase(t *testing.T) { + t0 := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + t1 := t0.Add(time.Hour) + t2 := t0.Add(2 * time.Hour) + + t.Run("empty list returns empty", func(t *testing.T) { + if got := pickOldestManagedDatabase(nil, ProviderExternal); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) + + t.Run("single candidate", func(t *testing.T) { + list := managedDatabases.ManagedDatabaseList{mdb("a", ProviderExternal, t1)} + if got := pickOldestManagedDatabase(list, ProviderExternal); got != "a" { + t.Fatalf("got %q, want %q", got, "a") + } + }) + + t.Run("picks earliest created regardless of list order", func(t *testing.T) { + list := managedDatabases.ManagedDatabaseList{ + mdb("newest", ProviderExternal, t2), + mdb("oldest", ProviderExternal, t0), + mdb("middle", ProviderExternal, t1), + } + if got := pickOldestManagedDatabase(list, ProviderExternal); got != "oldest" { + t.Fatalf("got %q, want %q", got, "oldest") + } + }) + + t.Run("ties broken by ID ascending", func(t *testing.T) { + list := managedDatabases.ManagedDatabaseList{ + mdb("bbb", ProviderExternal, t0), + mdb("aaa", ProviderExternal, t0), + } + if got := pickOldestManagedDatabase(list, ProviderExternal); got != "aaa" { + t.Fatalf("got %q, want %q", got, "aaa") + } + }) + + t.Run("filters by provider", func(t *testing.T) { + list := managedDatabases.ManagedDatabaseList{ + mdb("cnpg-older", ProviderCNPG, t0), + mdb("external-newer", ProviderExternal, t2), + } + if got := pickOldestManagedDatabase(list, ProviderExternal); got != "external-newer" { + t.Fatalf("got %q, want %q", got, "external-newer") + } + }) + + t.Run("no candidate of the requested provider", func(t *testing.T) { + list := managedDatabases.ManagedDatabaseList{mdb("c", ProviderCNPG, t0)} + if got := pickOldestManagedDatabase(list, ProviderExternal); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) +} diff --git a/components/api-server/plugins/gateways/plugin.go b/components/api-server/plugins/gateways/plugin.go index 6ae7e006..f39b79cc 100644 --- a/components/api-server/plugins/gateways/plugin.go +++ b/components/api-server/plugins/gateways/plugin.go @@ -32,7 +32,8 @@ type ServiceLocator struct { } type dbLookupAdapter struct { - svc managedDatabases.ManagedDatabaseService + svc managedDatabases.ManagedDatabaseService + provider string // when non-empty, FindSole filters to this provider } func (a *dbLookupAdapter) FindSole(ctx context.Context) (string, error) { @@ -40,12 +41,58 @@ func (a *dbLookupAdapter) FindSole(ctx context.Context) (string, error) { if err != nil { return "", err } - if len(all) == 1 { - return all[0].ID, nil + var matches []*managedDatabases.ManagedDatabase + for _, db := range all { + if a.provider == "" || db.Provider == a.provider { + matches = append(matches, db) + } + } + if len(matches) == 1 { + return matches[0].ID, nil } return "", nil } +// FindOldest returns the earliest-created ManagedDatabase matching the +// adapter's provider filter, or "" when none match. Ordering is creation +// timestamp ascending with ID ascending as the tie-break, so the choice is +// deterministic and concurrent gateway creations agree without coordination. +// IDs are time-sortable KSUIDs, so the tie-break agrees with creation order. +func (a *dbLookupAdapter) FindOldest(ctx context.Context) (string, error) { + all, err := a.svc.All(ctx) + if err != nil { + return "", err + } + return pickOldestManagedDatabase(all, a.provider), nil +} + +// pickOldestManagedDatabase returns the ID of the earliest-created +// ManagedDatabase matching provider (empty provider matches all), or "" when +// none match. Ordering is creation timestamp ascending with ID ascending as the +// tie-break, so the result is deterministic and independent of the order the +// DAO returned rows in. IDs are time-sortable KSUIDs, so the tie-break agrees +// with creation order. +func pickOldestManagedDatabase(all managedDatabases.ManagedDatabaseList, provider string) string { + var oldest *managedDatabases.ManagedDatabase + for _, db := range all { + if db == nil { + continue + } + if provider != "" && db.Provider != provider { + continue + } + if oldest == nil || + db.CreatedAt.Before(oldest.CreatedAt) || + (db.CreatedAt.Equal(oldest.CreatedAt) && db.ID < oldest.ID) { + oldest = db + } + } + if oldest == nil { + return "" + } + return oldest.ID +} + type dbCreatorAdapter struct { svc managedDatabases.ManagedDatabaseService provider string @@ -79,11 +126,15 @@ func NewServiceLocator(env *environments.Env) ServiceLocator { return ServiceLocator{ gateway: func() GatewayService { var placement PlacementResolver - if mdSvc := managedDatabases.Service(&env.Services); mdSvc != nil { - if databaseProvider == ProviderDeployment { + mdSvc := managedDatabases.Service(&env.Services) + if mdSvc != nil { + switch databaseProvider { + case ProviderDeployment: placement = NewDeploymentPlacement(&dbCreatorAdapter{svc: mdSvc, provider: databaseProvider}) - } else { - placement = NewCNPGPlacement(&dbLookupAdapter{svc: mdSvc}) + case ProviderCNPG: + placement = NewCNPGPlacement(&dbLookupAdapter{svc: mdSvc, provider: ProviderCNPG}) + case ProviderExternal: + placement = NewExternalPlacement(&dbLookupAdapter{svc: mdSvc, provider: ProviderExternal}) } } diff --git a/components/api-server/plugins/gateways/provider.go b/components/api-server/plugins/gateways/provider.go index d7395f3d..c493f4ce 100644 --- a/components/api-server/plugins/gateways/provider.go +++ b/components/api-server/plugins/gateways/provider.go @@ -7,25 +7,32 @@ import "fmt" // auto-creates a dedicated deployment-backed ManagedDatabase per gateway // (deploymentPlacement) and needs no CNPG APIs. ProviderCNPG selects // CNPG-backed placement (cnpgPlacement) and resolves database_id against the -// sole existing ManagedDatabase. +// sole existing ManagedDatabase with provider "cnpg". ProviderExternal selects +// external-server placement (externalPlacement): database_id is resolved to the +// first-created ManagedDatabase with provider "external". Several external +// ManagedDatabases may be registered; create-gateway is rejected only when none +// are. const ( ProviderDeployment = "deployment" ProviderCNPG = "cnpg" + ProviderExternal = "external" ) // resolveDatabaseProvider validates a raw DATABASE_PROVIDER environment // value read at gateway-service construction time (server startup). Unset or -// empty resolves to ProviderDeployment; any value other than "deployment" or -// "cnpg" is a startup configuration error, never an implicit fallback to -// "cnpg". +// empty resolves to ProviderDeployment; any value other than "deployment", +// "cnpg", or "external" is a startup configuration error, never an implicit +// fallback. func resolveDatabaseProvider(raw string) (string, error) { switch raw { case "", ProviderDeployment: return ProviderDeployment, nil case ProviderCNPG: return ProviderCNPG, nil + case ProviderExternal: + return ProviderExternal, nil default: - return "", fmt.Errorf("invalid DATABASE_PROVIDER %q: must be %q or %q (unset defaults to %q)", - raw, ProviderCNPG, ProviderDeployment, ProviderDeployment) + return "", fmt.Errorf("invalid DATABASE_PROVIDER %q: must be %q, %q, or %q (unset defaults to %q)", + raw, ProviderCNPG, ProviderDeployment, ProviderExternal, ProviderDeployment) } } diff --git a/components/api-server/plugins/managedDatabases/service.go b/components/api-server/plugins/managedDatabases/service.go index d96aaaed..d1dc96da 100644 --- a/components/api-server/plugins/managedDatabases/service.go +++ b/components/api-server/plugins/managedDatabases/service.go @@ -2,6 +2,8 @@ package managedDatabases import ( "context" + "regexp" + "strings" "github.com/openshift-online/rh-trex-ai/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/db" @@ -15,8 +17,34 @@ const managedDatabasesLockType db.LockType = "managed_databases" const ( providerCNPG = "cnpg" providerDeployment = "deployment" + providerExternal = "external" ) +// externalCredentialsNamespacePrefix is the reserved prefix for the namespace +// holding an external server's admin credentials. ManagedDatabase.connection_secret +// names that NAMESPACE, not a Secret: the credentials are provisioned out-of-band, +// normally before HyperShell is installed, so they must not depend on the control +// plane instance namespace existing. +// +// The prefix is a security boundary, not a convention. Combined with the fixed +// Secret name below it bounds what the control plane can be made to read to a +// single deliberately-named Secret inside deliberately-created namespaces, +// preventing an API-level reference from pointing the reconciler at an unrelated +// Secret such as hypershell-db-app. The same values are enforced by the control +// plane (gateway/external_db.go). See naming-multitenancy.spec.md §6.2. +const externalCredentialsNamespacePrefix = "hypershell-managed-db-" + +// externalCredentialsSecretName is the fixed name of the Secret read inside a +// hypershell-managed-db- namespace. It is not configurable. +const externalCredentialsSecretName = "hypershell-managed-db-credentials" + +// dns1123LabelMaxLength is the Kubernetes limit for a namespace name. Namespace +// names are DNS-1123 labels, not subdomains, so dots are not permitted. +const dns1123LabelMaxLength = 63 + +// dns1123LabelPattern matches a valid DNS-1123 label (a valid namespace name). +var dns1123LabelPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + type ManagedDatabaseService interface { Get(ctx context.Context, id string) (*ManagedDatabase, *errors.ServiceError) GetUnscoped(ctx context.Context, id string) (*ManagedDatabase, *errors.ServiceError) @@ -95,17 +123,47 @@ func (s *sqlManagedDatabaseService) ListDeleted(ctx context.Context, offset, lim } func isSupportedProvider(provider string) bool { - return provider == providerCNPG || provider == providerDeployment + return provider == providerCNPG || provider == providerDeployment || provider == providerExternal } func unsupportedProviderError(provider string) *errors.ServiceError { - return errors.Validation("unsupported provider %q: supported providers are \"cnpg\" and \"deployment\"", provider) + return errors.Validation("unsupported provider %q: supported providers are \"cnpg\", \"deployment\", and \"external\"", provider) +} + +// validateExternalConnectionSecret checks the connection_secret reference for +// external ManagedDatabases. The value names the NAMESPACE holding the admin +// credentials Secret, so it must be a bare namespace name (no "/"), carry the +// reserved prefix, and be a valid DNS-1123 label. The Secret inside it always +// has the fixed name externalCredentialsSecretName. +func validateExternalConnectionSecret(secret *string) *errors.ServiceError { + if secret == nil || *secret == "" { + return errors.Validation("connection_secret is required for provider \"external\": it names the namespace holding the %q Secret", externalCredentialsSecretName) + } + value := *secret + if strings.Contains(value, "/") { + return errors.Validation("connection_secret must be a bare namespace name without a \"/\": it names the namespace holding the %q Secret, not the Secret itself", externalCredentialsSecretName) + } + if !strings.HasPrefix(value, externalCredentialsNamespacePrefix) { + return errors.Validation("connection_secret namespace %q must begin with the reserved prefix %q", value, externalCredentialsNamespacePrefix) + } + if len(value) > dns1123LabelMaxLength { + return errors.Validation("connection_secret namespace %q is %d characters; a namespace name may be at most %d", value, len(value), dns1123LabelMaxLength) + } + if !dns1123LabelPattern.MatchString(value) { + return errors.Validation("connection_secret namespace %q is not a valid DNS-1123 label: use lowercase alphanumerics and '-', starting and ending with an alphanumeric", value) + } + return nil } func (s *sqlManagedDatabaseService) Create(ctx context.Context, managedDatabase *ManagedDatabase) (*ManagedDatabase, *errors.ServiceError) { if !isSupportedProvider(managedDatabase.Provider) { return nil, unsupportedProviderError(managedDatabase.Provider) } + if managedDatabase.Provider == providerExternal { + if svcErr := validateExternalConnectionSecret(managedDatabase.ConnectionSecret); svcErr != nil { + return nil, svcErr + } + } managedDatabase.CaptureTraceContext(ctx) managedDatabase, err := s.managedDatabaseDao.Create(ctx, managedDatabase) @@ -142,6 +200,11 @@ func (s *sqlManagedDatabaseService) Replace(ctx context.Context, managedDatabase if isSupportedProvider(persisted.Provider) && managedDatabase.Provider != persisted.Provider { return nil, errors.Validation("provider cannot be changed from %q to %q", persisted.Provider, managedDatabase.Provider) } + if managedDatabase.Provider == providerExternal { + if svcErr := validateExternalConnectionSecret(managedDatabase.ConnectionSecret); svcErr != nil { + return nil, svcErr + } + } managedDatabase.CaptureTraceContext(ctx) managedDatabase, err = s.managedDatabaseDao.Replace(ctx, managedDatabase) diff --git a/components/api-server/plugins/managedDatabases/service_test.go b/components/api-server/plugins/managedDatabases/service_test.go new file mode 100644 index 00000000..816e5f43 --- /dev/null +++ b/components/api-server/plugins/managedDatabases/service_test.go @@ -0,0 +1,75 @@ +package managedDatabases + +import ( + "strings" + "testing" +) + +// connection_secret names the NAMESPACE holding the fixed-name admin +// credentials Secret, not a Secret name. The reserved prefix plus the fixed +// Secret name are a security boundary: together they bound what the control +// plane can be made to read. See +// specs/platform/openshell-gateway-database-external.spec.md. +func TestValidateExternalConnectionSecret(t *testing.T) { + ptr := func(s string) *string { return &s } + + cases := []struct { + name string + input *string + wantErr bool + }{ + {name: "nil", input: nil, wantErr: true}, + {name: "empty", input: ptr(""), wantErr: true}, + {name: "namespace/name form rejected", input: ptr("hypershell/hypershell-managed-db-credentials"), wantErr: true}, + {name: "slash anywhere rejected", input: ptr("hypershell-managed-db-a/b"), wantErr: true}, + {name: "missing reserved prefix", input: ptr("my-namespace"), wantErr: true}, + {name: "control plane namespace rejected", input: ptr("hypershell"), wantErr: true}, + {name: "platform db secret name rejected", input: ptr("hypershell-db-app"), wantErr: true}, + {name: "uppercase is not a DNS-1123 label", input: ptr("hypershell-managed-db-Prod"), wantErr: true}, + {name: "dots are not allowed in a namespace name", input: ptr("hypershell-managed-db-us.east.1"), wantErr: true}, + {name: "underscore is not a DNS-1123 label", input: ptr("hypershell-managed-db-us_east"), wantErr: true}, + {name: "trailing dash", input: ptr("hypershell-managed-db-"), wantErr: true}, + {name: "over 63 characters", input: ptr(externalCredentialsNamespacePrefix + strings.Repeat("a", 64)), wantErr: true}, + + {name: "valid", input: ptr("hypershell-managed-db-us-east-1"), wantErr: false}, + {name: "valid with digits", input: ptr("hypershell-managed-db-123abc"), wantErr: false}, + {name: "exactly 63 characters", input: ptr(externalCredentialsNamespacePrefix + strings.Repeat("a", dns1123LabelMaxLength-len(externalCredentialsNamespacePrefix))), wantErr: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateExternalConnectionSecret(tc.input) + if tc.wantErr && err == nil { + t.Errorf("expected a validation error, got nil") + } + if !tc.wantErr && err != nil { + t.Errorf("unexpected validation error: %v", err) + } + }) + } +} + +// The prefix and the fixed Secret name are duplicated in the control plane +// (components/control-plane/internal/gateway/external_db.go). If either side +// changes, the reference stops resolving, so pin both here. +func TestExternalCredentialsConstantsArePinned(t *testing.T) { + if externalCredentialsNamespacePrefix != "hypershell-managed-db-" { + t.Errorf("namespace prefix = %q; the control plane expects %q", externalCredentialsNamespacePrefix, "hypershell-managed-db-") + } + if externalCredentialsSecretName != "hypershell-managed-db-credentials" { + t.Errorf("secret name = %q; the control plane expects %q", externalCredentialsSecretName, "hypershell-managed-db-credentials") + } +} + +func TestIsSupportedProvider(t *testing.T) { + for _, p := range []string{providerCNPG, providerDeployment, providerExternal} { + if !isSupportedProvider(p) { + t.Errorf("isSupportedProvider(%q) = false, want true", p) + } + } + for _, p := range []string{"", "postgres", "CNPG", "rds"} { + if isSupportedProvider(p) { + t.Errorf("isSupportedProvider(%q) = true, want false", p) + } + } +} diff --git a/components/api-server/proto/hypershell/v1/managed_databases.proto b/components/api-server/proto/hypershell/v1/managed_databases.proto index 8b1ea2af..d0a4cb58 100644 --- a/components/api-server/proto/hypershell/v1/managed_databases.proto +++ b/components/api-server/proto/hypershell/v1/managed_databases.proto @@ -16,6 +16,10 @@ message ManagedDatabase { optional string engine = 6; optional string engine_version = 7; optional string instance_class = 8; + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". optional string connection_secret = 9; optional string status = 10; string namespace = 11; @@ -30,6 +34,10 @@ message CreateManagedDatabaseRequest { optional string engine = 5; optional string engine_version = 6; optional string instance_class = 7; + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". optional string connection_secret = 8; optional string status = 9; } @@ -56,6 +64,10 @@ message UpdateManagedDatabaseRequest { optional string engine = 6; optional string engine_version = 7; optional string instance_class = 8; + // For provider "external": the NAMESPACE holding the admin credentials + // Secret, not a Secret name. Bare namespace name (no "/"), prefixed with + // "hypershell-managed-db-", a valid DNS-1123 label. The Secret inside it is + // always named "hypershell-managed-db-credentials". optional string connection_secret = 9; optional string status = 10; } diff --git a/components/cli/cmd/hypershell/create/managedDatabase/cmd.go b/components/cli/cmd/hypershell/create/managedDatabase/cmd.go index ab527ce7..c8bb8906 100644 --- a/components/cli/cmd/hypershell/create/managedDatabase/cmd.go +++ b/components/cli/cmd/hypershell/create/managedDatabase/cmd.go @@ -40,7 +40,7 @@ var Cmd = &cobra.Command{ func init() { fs := Cmd.Flags() - fs.StringVar(&args.connectionSecret, "connection-secret", "", "connection_secret value.") + fs.StringVar(&args.connectionSecret, "connection-secret", "", "For provider \"external\": the NAMESPACE holding the admin credentials Secret. Must be a bare namespace name prefixed with \"hypershell-managed-db-\"; the Secret inside it is always named \"hypershell-managed-db-credentials\".") fs.StringVar(&args.engine, "engine", "", "engine value.") fs.StringVar(&args.engineVersion, "engine-version", "", "engine_version value.") fs.StringVar(&args.instanceClass, "instance-class", "", "instance_class value.") diff --git a/components/control-plane/go.mod b/components/control-plane/go.mod index f3cede44..de05975c 100644 --- a/components/control-plane/go.mod +++ b/components/control-plane/go.mod @@ -6,6 +6,8 @@ toolchain go1.26.7 require ( github.com/golang-jwt/jwt/v4 v4.5.0 + github.com/google/uuid v1.6.0 + github.com/lib/pq v1.10.9 github.com/openshift-online/hypershell/components/api-server v0.0.0-00010101000000-000000000000 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.70.0 go.opentelemetry.io/otel v1.45.0 @@ -46,7 +48,6 @@ require ( github.com/go-openapi/swag/typeutils v0.26.0 // indirect github.com/go-openapi/swag/yamlutils v0.26.0 // indirect github.com/google/gnostic-models v0.7.1 // indirect - github.com/google/uuid v1.6.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.29.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect diff --git a/components/control-plane/go.sum b/components/control-plane/go.sum index 86434954..abff7cbc 100644 --- a/components/control-plane/go.sum +++ b/components/control-plane/go.sum @@ -68,6 +68,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= +github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= diff --git a/components/control-plane/internal/config/config.go b/components/control-plane/internal/config/config.go index 551848c5..0bc3c751 100644 --- a/components/control-plane/internal/config/config.go +++ b/components/control-plane/internal/config/config.go @@ -16,9 +16,12 @@ import ( // requires the CNPG operator CRDs to be installed; see // gateway.RequireCNPGAPI, which the control-plane entrypoint uses to fail // startup cleanly when they are not. +// DatabaseProviderExternal selects an externally-managed PostgreSQL server; +// the control plane issues DDL in-process and requires no CNPG operator. const ( DatabaseProviderDeployment = "deployment" DatabaseProviderCNPG = "cnpg" + DatabaseProviderExternal = "external" ) // DefaultGatewayReconcileWorkers is the fallback size of the gateway reconcile @@ -67,9 +70,9 @@ type Config struct { // DatabaseProvider is the control-plane-wide default ManagedDatabase // provider, resolved from DATABASE_PROVIDER by resolveDatabaseProvider. - // It is always either DatabaseProviderDeployment or DatabaseProviderCNPG; - // Load returns an error for any other DATABASE_PROVIDER value instead of - // silently falling back to CNPG. Existing ManagedDatabase resources keep + // It is always one of DatabaseProviderDeployment, DatabaseProviderCNPG or + // DatabaseProviderExternal; Load returns an error for any other + // DATABASE_PROVIDER value instead of silently falling back. Existing ManagedDatabase resources keep // reconciling per their own Provider field regardless of this default // (see internal/reconciler.ManagedDatabaseReconciler), so gateways backed // by CNPG remain compatible even when this default is "deployment". @@ -107,19 +110,21 @@ func Load() (*Config, error) { } // resolveDatabaseProvider validates a raw DATABASE_PROVIDER value into one of -// the two supported providers. Unset or empty means DatabaseProviderDeployment +// the three supported providers. Unset or empty means DatabaseProviderDeployment // (deployment-backed ManagedDatabase placement is the default and requires no -// CNPG APIs); any value other than "deployment" or "cnpg" is a startup -// configuration error rather than a silent fallback to CNPG. +// CNPG APIs); any value other than "deployment", "cnpg" or "external" is a +// startup configuration error rather than a silent fallback. func resolveDatabaseProvider(raw string) (string, error) { switch raw { case "", DatabaseProviderDeployment: return DatabaseProviderDeployment, nil case DatabaseProviderCNPG: return DatabaseProviderCNPG, nil + case DatabaseProviderExternal: + return DatabaseProviderExternal, nil default: - return "", fmt.Errorf("invalid DATABASE_PROVIDER %q: must be %q or %q (unset defaults to %q)", - raw, DatabaseProviderCNPG, DatabaseProviderDeployment, DatabaseProviderDeployment) + return "", fmt.Errorf("invalid DATABASE_PROVIDER %q: must be %q, %q, or %q (unset defaults to %q)", + raw, DatabaseProviderCNPG, DatabaseProviderDeployment, DatabaseProviderExternal, DatabaseProviderDeployment) } } diff --git a/components/control-plane/internal/gateway/cnpg_db.go b/components/control-plane/internal/gateway/cnpg_db.go new file mode 100644 index 00000000..555caa6b --- /dev/null +++ b/components/control-plane/internal/gateway/cnpg_db.go @@ -0,0 +1,366 @@ +package gateway + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "log" + "net/url" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" +) + +type cnpgDatabaseReconciler struct { + cnpg CNPGConfig +} + +func (r *cnpgDatabaseReconciler) Reconcile(ctx context.Context, dynamicClient dynamic.Interface, clientset kubernetes.Interface, tenantNamespace, gatewayID, rotateAnnotation string) error { + if err := reconcileCNPGDatabaseResources(ctx, dynamicClient, clientset, tenantNamespace, gatewayID, r.cnpg); err != nil { + return fmt.Errorf("reconcile CNPG database resources in %s: %w", tenantNamespace, err) + } + if rotateAnnotation != "" { + if err := rotateCNPGDatabaseCredentials(ctx, clientset, tenantNamespace, gatewayID, r.cnpg, rotateAnnotation); err != nil { + return fmt.Errorf("rotate database credentials in %s: %w", tenantNamespace, err) + } + } + return nil +} + +func (r *cnpgDatabaseReconciler) Delete(ctx context.Context, dynamicClient dynamic.Interface, clientset kubernetes.Interface, gatewayID string) error { + if gatewayID == "" { + return nil + } + if r.cnpg.ClusterNamespace == "" { + log.Printf("WARN gateway %s: CNPG cluster namespace unknown; Database, DatabaseRole, and password Secret were not deleted and may require manual cleanup", gatewayID) + return nil + } + deleteCNPGResources(ctx, dynamicClient, clientset, gatewayID, r.cnpg) + return nil +} + +func cnpgResourceName(gatewayID string) string { + return "gw-" + strings.ToLower(gatewayID) +} + +func cnpgPGName(gatewayID string) string { + return "gw_" + strings.ToLower(gatewayID) +} + +func reconcileCNPGDatabaseResources( + ctx context.Context, + dynamicClient dynamic.Interface, + clientset kubernetes.Interface, + tenantNamespace string, + gatewayID string, + cnpg CNPGConfig, +) error { + crName := cnpgResourceName(gatewayID) + pgName := cnpgPGName(gatewayID) + passwordSecretName := crName + "-credentials" + + log.Printf("INFO CNPG provisioning: gateway=%s cr=%s db=%s cluster=%s/%s tenant=%s", + gatewayID, crName, pgName, cnpg.ClusterNamespace, cnpg.ClusterName, tenantNamespace) + + _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) + if err != nil { + if !k8serrors.IsNotFound(err) { + return fmt.Errorf("get CNPG password secret: %w", err) + } + + passwordBytes := make([]byte, 32) + if _, err := rand.Read(passwordBytes); err != nil { + return fmt.Errorf("generate database password: %w", err) + } + password := hex.EncodeToString(passwordBytes) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: passwordSecretName, + Namespace: cnpg.ClusterNamespace, + Labels: map[string]string{ + "cnpg.io/reload": "true", + "hypershell.redhat.io/managed": "true", + "hypershell.redhat.io/gateway-namespace": tenantNamespace, + }, + }, + Type: corev1.SecretTypeBasicAuth, + StringData: map[string]string{ + "username": pgName, + "password": password, + }, + } + if _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("create CNPG password secret: %w", err) + } + log.Printf("INFO created CNPG password secret %s in %s", passwordSecretName, cnpg.ClusterNamespace) + } else { + log.Printf("DEBUG CNPG password secret %s already exists in %s, skipping creation", passwordSecretName, cnpg.ClusterNamespace) + } + + log.Printf("INFO reconciling CNPG DatabaseRole %s in %s (cluster=%s)", crName, cnpg.ClusterNamespace, cnpg.ClusterName) + role := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "postgresql.cnpg.io/v1", + "kind": "DatabaseRole", + "metadata": map[string]interface{}{ + "name": crName, + "namespace": cnpg.ClusterNamespace, + "labels": map[string]interface{}{ + "hypershell.redhat.io/managed": "true", + "hypershell.redhat.io/gateway-namespace": tenantNamespace, + }, + }, + "spec": map[string]interface{}{ + "cluster": map[string]interface{}{ + "name": cnpg.ClusterName, + }, + "name": pgName, + "login": true, + "passwordSecret": map[string]interface{}{ + "name": passwordSecretName, + }, + "databaseRoleReclaimPolicy": "delete", + }, + }, + } + if err := reconcileResource(ctx, dynamicClient, role); err != nil { + return fmt.Errorf("reconcile CNPG DatabaseRole: %w", err) + } + + log.Printf("INFO reconciling CNPG Database %s in %s (owner=%s)", crName, cnpg.ClusterNamespace, pgName) + db := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "postgresql.cnpg.io/v1", + "kind": "Database", + "metadata": map[string]interface{}{ + "name": crName, + "namespace": cnpg.ClusterNamespace, + "labels": map[string]interface{}{ + "hypershell.redhat.io/managed": "true", + "hypershell.redhat.io/gateway-namespace": tenantNamespace, + }, + }, + "spec": map[string]interface{}{ + "cluster": map[string]interface{}{ + "name": cnpg.ClusterName, + }, + "name": pgName, + "owner": pgName, + "databaseReclaimPolicy": "delete", + }, + }, + } + if err := reconcileResource(ctx, dynamicClient, db); err != nil { + return fmt.Errorf("reconcile CNPG Database: %w", err) + } + + log.Printf("INFO waiting for CNPG Database %s/%s to become ready (timeout=2m)", cnpg.ClusterNamespace, crName) + if err := waitForCNPGDatabase(ctx, dynamicClient, cnpg.ClusterNamespace, crName, 2*time.Minute); err != nil { + return fmt.Errorf("wait for CNPG database: %w", err) + } + + gwSecretName := "openshell-gateway-db-credentials" + _, err = clientset.CoreV1().Secrets(tenantNamespace).Get(ctx, gwSecretName, metav1.GetOptions{}) + if err != nil { + if !k8serrors.IsNotFound(err) { + return fmt.Errorf("get gateway credentials secret: %w", err) + } + + cnpgSecret, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("read CNPG password secret: %w", err) + } + passwordBytes, ok := cnpgSecret.Data["password"] + if !ok || len(passwordBytes) == 0 { + return fmt.Errorf("CNPG password secret %s/%s has no password key", cnpg.ClusterNamespace, passwordSecretName) + } + password := string(passwordBytes) + + host := fmt.Sprintf("%s-rw.%s.svc.cluster.local", cnpg.ClusterName, cnpg.ClusterNamespace) + dbURI := fmt.Sprintf("postgresql://%s:%s@%s:5432/%s?sslmode=require", + pgName, url.QueryEscape(password), host, pgName) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: gwSecretName, + Namespace: tenantNamespace, + Labels: map[string]string{ + "app.kubernetes.io/name": "openshell", + "app.kubernetes.io/component": "database", + "app.kubernetes.io/managed-by": "hypershell-control-plane", + "hypershell.redhat.io/managed": "true", + }, + }, + Type: corev1.SecretTypeOpaque, + StringData: map[string]string{ + "host": host, + "port": "5432", + "dbname": pgName, + "user": pgName, + "password": password, + "uri": dbURI, + }, + } + if _, err := clientset.CoreV1().Secrets(tenantNamespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("create gateway credentials secret: %w", err) + } + log.Printf("INFO created gateway credentials secret %s in %s (host=%s db=%s)", gwSecretName, tenantNamespace, host, pgName) + } else { + log.Printf("DEBUG gateway credentials secret %s already exists in %s, skipping creation", gwSecretName, tenantNamespace) + } + + log.Printf("INFO CNPG database provisioning complete for gateway %s in %s", gatewayID, tenantNamespace) + return nil +} + +func waitForCNPGDatabase(ctx context.Context, dynamicClient dynamic.Interface, namespace, name string, timeout time.Duration) error { + databaseGVR := schema.GroupVersionResource{ + Group: "postgresql.cnpg.io", + Version: "v1", + Resource: "databases", + } + + deadline := time.After(timeout) + ticker := time.NewTicker(2 * time.Second) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-deadline: + return fmt.Errorf("timed out waiting for CNPG Database %s/%s to become ready", namespace, name) + case <-ticker.C: + obj, err := dynamicClient.Resource(databaseGVR).Namespace(namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + log.Printf("DEBUG CNPG Database %s/%s not found yet, waiting...", namespace, name) + } else { + log.Printf("WARN error checking CNPG Database %s/%s: %v", namespace, name, err) + } + continue + } + applied, _, _ := unstructured.NestedBool(obj.Object, "status", "applied") + if applied { + log.Printf("INFO CNPG Database %s/%s is ready (status.applied=true)", namespace, name) + return nil + } + log.Printf("DEBUG CNPG Database %s/%s exists but not ready (status.applied=%v)", namespace, name, applied) + } + } +} + +func deleteCNPGResources( + ctx context.Context, + dynamicClient dynamic.Interface, + clientset kubernetes.Interface, + gatewayID string, + cnpg CNPGConfig, +) { + crName := cnpgResourceName(gatewayID) + ns := cnpg.ClusterNamespace + log.Printf("INFO deleting CNPG resources for gateway %s: cr=%s namespace=%s", gatewayID, crName, ns) + + databaseGVR := schema.GroupVersionResource{ + Group: "postgresql.cnpg.io", + Version: "v1", + Resource: "databases", + } + if err := dynamicClient.Resource(databaseGVR).Namespace(ns).Delete(ctx, crName, metav1.DeleteOptions{}); err != nil { + if !k8serrors.IsNotFound(err) { + log.Printf("WARN failed to delete CNPG Database %s: %v", crName, err) + } + } else { + log.Printf("INFO deleted CNPG Database %s from %s", crName, ns) + } + + roleGVR := schema.GroupVersionResource{ + Group: "postgresql.cnpg.io", + Version: "v1", + Resource: "databaseroles", + } + if err := dynamicClient.Resource(roleGVR).Namespace(ns).Delete(ctx, crName, metav1.DeleteOptions{}); err != nil { + if !k8serrors.IsNotFound(err) { + log.Printf("WARN failed to delete CNPG DatabaseRole %s: %v", crName, err) + } + } else { + log.Printf("INFO deleted CNPG DatabaseRole %s from %s", crName, ns) + } + + passwordSecretName := crName + "-credentials" + if err := clientset.CoreV1().Secrets(ns).Delete(ctx, passwordSecretName, metav1.DeleteOptions{}); err != nil { + if !k8serrors.IsNotFound(err) { + log.Printf("WARN failed to delete CNPG password secret %s: %v", passwordSecretName, err) + } + } else { + log.Printf("INFO deleted CNPG password secret %s from %s", passwordSecretName, ns) + } +} + +func rotateCNPGDatabaseCredentials( + ctx context.Context, + clientset kubernetes.Interface, + tenantNamespace string, + gatewayID string, + cnpg CNPGConfig, + rotateTimestamp string, +) error { + gwSecretName := "openshell-gateway-db-credentials" + existing, err := clientset.CoreV1().Secrets(tenantNamespace).Get(ctx, gwSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("get gateway credentials secret for rotation: %w", err) + } + + lastRotation := existing.Annotations["hypershell.redhat.io/last-db-rotation"] + if lastRotation == rotateTimestamp { + log.Printf("DEBUG database credentials in %s already rotated at %s, skipping", tenantNamespace, rotateTimestamp) + return nil + } + + passwordBytes := make([]byte, 32) + if _, err := rand.Read(passwordBytes); err != nil { + return fmt.Errorf("generate new database password: %w", err) + } + newPassword := hex.EncodeToString(passwordBytes) + + crName := cnpgResourceName(gatewayID) + pgName := cnpgPGName(gatewayID) + passwordSecretName := crName + "-credentials" + + cnpgSecret, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("get CNPG password secret for rotation: %w", err) + } + cnpgSecret.Data["password"] = []byte(newPassword) + if _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Update(ctx, cnpgSecret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("update CNPG password secret: %w", err) + } + log.Printf("INFO updated CNPG password secret %s in %s", passwordSecretName, cnpg.ClusterNamespace) + + host := fmt.Sprintf("%s-rw.%s.svc.cluster.local", cnpg.ClusterName, cnpg.ClusterNamespace) + newURI := fmt.Sprintf("postgresql://%s:%s@%s:5432/%s?sslmode=require", + pgName, url.QueryEscape(newPassword), host, pgName) + + existing.Data["password"] = []byte(newPassword) + existing.Data["uri"] = []byte(newURI) + if existing.Annotations == nil { + existing.Annotations = make(map[string]string) + } + existing.Annotations["hypershell.redhat.io/last-db-rotation"] = rotateTimestamp + + if _, err := clientset.CoreV1().Secrets(tenantNamespace).Update(ctx, existing, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("update gateway credentials secret after rotation: %w", err) + } + + log.Printf("INFO rotated database credentials in %s (timestamp=%s)", tenantNamespace, rotateTimestamp) + return nil +} diff --git a/components/control-plane/internal/gateway/config.go b/components/control-plane/internal/gateway/config.go index 09b66055..dc31e267 100644 --- a/components/control-plane/internal/gateway/config.go +++ b/components/control-plane/internal/gateway/config.go @@ -63,6 +63,20 @@ 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. +type ExternalDBConfig struct { + CredentialsNamespace string + ManagedDatabaseID string +} + // DefaultSandboxImage resolves the base image tenant sandbox pods launch from. // It is overridable via GATEWAY_SANDBOX_IMAGE so clusters whose nodes cannot // reach ghcr.io (e.g. IBM ROKS) can point it at an in-cluster registry mirror, @@ -166,13 +180,15 @@ type ReconcileOpts struct { HasCertManager bool HasGatewayAPI bool HasCNPG bool - // DatabaseProvider is the ManagedDatabase provider ("cnpg" or "deployment"). + // DatabaseProvider is the ManagedDatabase provider ("cnpg", "deployment", or "external"). DatabaseProvider string CNPG CNPGConfig // DeploymentDBNamespace is the namespace where the Deployment-managed // database lives. Used when DatabaseProvider is "deployment" to copy // credentials into the tenant namespace. DeploymentDBNamespace string + // ExternalDB carries the admin Secret reference for the external provider. + ExternalDB ExternalDBConfig ControlPlaneNamespace string Images ImageDefaults // SkipNetworkPolicies disables creation of the per-tenant gateway diff --git a/components/control-plane/internal/gateway/db_reconciler.go b/components/control-plane/internal/gateway/db_reconciler.go new file mode 100644 index 00000000..8945d949 --- /dev/null +++ b/components/control-plane/internal/gateway/db_reconciler.go @@ -0,0 +1,62 @@ +package gateway + +import ( + "context" + "fmt" + + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" +) + +// DatabaseReconciler is implemented by each database provider (cnpg, deployment, external). +// Reconcile provisions or updates database resources for a gateway tenant namespace. +// Delete removes out-of-namespace database resources. A non-nil error signals a transient +// failure that the caller should retry. CNPG and deployment providers always return nil. +// The external provider also always returns nil: its cleanup is unconditional and +// single-shot (no tombstone, no retry queue), so a failure is logged rather than retried. +type DatabaseReconciler interface { + Reconcile(ctx context.Context, dynamicClient dynamic.Interface, clientset kubernetes.Interface, tenantNamespace, gatewayID, rotateAnnotation string) error + Delete(ctx context.Context, dynamicClient dynamic.Interface, clientset kubernetes.Interface, gatewayID string) error +} + +// newDatabaseReconciler constructs the correct DatabaseReconciler for opts.DatabaseProvider. +// Returns an error for invalid configurations (missing required fields, CNPG not available). +// Returns a noopDatabaseReconciler for the legacy empty-provider case. +func newDatabaseReconciler(opts ReconcileOpts) (DatabaseReconciler, error) { + switch opts.DatabaseProvider { + case "": + return &noopDatabaseReconciler{}, nil + case "cnpg": + if opts.CNPG.ClusterNamespace == "" { + return nil, fmt.Errorf("CNPG cluster namespace is required for gateway database reconciliation") + } + if !opts.HasCNPG { + return nil, fmt.Errorf("CNPG operator is required but not available on the cluster: gateway deployment blocked") + } + return &cnpgDatabaseReconciler{cnpg: opts.CNPG}, nil + case "deployment": + if opts.DeploymentDBNamespace == "" { + return nil, fmt.Errorf("deployment database namespace is required for gateway database reconciliation") + } + return &deploymentDatabaseReconciler{dbNamespace: opts.DeploymentDBNamespace}, nil + case "external": + if opts.ExternalDB.CredentialsNamespace == "" { + return nil, fmt.Errorf("external database connection_secret (credentials namespace) is required for gateway database reconciliation") + } + return &externalDatabaseReconciler{cfg: opts.ExternalDB}, nil + default: + return nil, fmt.Errorf("unsupported database provider %q", opts.DatabaseProvider) + } +} + +// noopDatabaseReconciler handles legacy gateways with no DatabaseProvider set. +// It preserves any existing database resources without touching them. +type noopDatabaseReconciler struct{} + +func (r *noopDatabaseReconciler) Reconcile(_ context.Context, _ dynamic.Interface, _ kubernetes.Interface, _, _, _ string) error { + return nil +} + +func (r *noopDatabaseReconciler) Delete(_ context.Context, _ dynamic.Interface, _ kubernetes.Interface, _ string) error { + return nil +} diff --git a/components/control-plane/internal/gateway/deployment_db.go b/components/control-plane/internal/gateway/deployment_db.go new file mode 100644 index 00000000..df92914f --- /dev/null +++ b/components/control-plane/internal/gateway/deployment_db.go @@ -0,0 +1,182 @@ +package gateway + +import ( + "context" + "fmt" + "log" + "net/url" + "reflect" + "time" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" +) + +type deploymentDatabaseReconciler struct { + dbNamespace string +} + +func (r *deploymentDatabaseReconciler) Reconcile(ctx context.Context, _ dynamic.Interface, clientset kubernetes.Interface, tenantNamespace, _, _ string) error { + return reconcileDeploymentDatabaseCredentials(ctx, clientset, r.dbNamespace, tenantNamespace) +} + +func (r *deploymentDatabaseReconciler) Delete(_ context.Context, _ dynamic.Interface, _ kubernetes.Interface, _ string) error { + return nil +} + +const deploymentDatabaseName = `openshell-gateway-db` +const deploymentReadinessWaitTimeout = 2 * time.Minute +const deploymentReadinessPollInterval = 2 * time.Second + +type deploymentReadinessWaitOptions struct{ timeout, pollInterval time.Duration } + +func defaultDeploymentReadinessWaitOptions() deploymentReadinessWaitOptions { + return deploymentReadinessWaitOptions{timeout: deploymentReadinessWaitTimeout, pollInterval: deploymentReadinessPollInterval} +} + +func waitForDeploymentReady(ctx context.Context, clientset kubernetes.Interface, namespace, name string, opts deploymentReadinessWaitOptions) error { + if opts.timeout <= 0 || opts.pollInterval <= 0 { + return fmt.Errorf(`wait for deployment %s/%s: invalid wait options`, namespace, name) + } + observation := `not ready` + check := func() bool { + ready, reason, err := DeploymentReadiness(ctx, clientset, namespace, name) + if err != nil { + observation = `readiness check temporarily failed` + return false + } + if ready { + return true + } + if reason != `` { + observation = reason + } + return false + } + if err := ctx.Err(); err != nil { + return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) + } + if check() { + return nil + } + if err := ctx.Err(); err != nil { + return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) + } + timer := time.NewTimer(opts.timeout) + defer timer.Stop() + ticker := time.NewTicker(opts.pollInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, ctx.Err()) + case <-timer.C: + return fmt.Errorf(`timed out waiting for deployment %s/%s to become ready: %s`, namespace, name, observation) + case <-ticker.C: + if check() { + return nil + } + if err := ctx.Err(); err != nil { + return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) + } + } + } +} + +func reconcileDeploymentDatabaseCredentials(ctx context.Context, clientset kubernetes.Interface, sourceNamespace, tenantNamespace string, options ...deploymentReadinessWaitOptions) error { + waitOpts := defaultDeploymentReadinessWaitOptions() + if len(options) > 0 { + waitOpts = options[0] + } + if err := waitForDeploymentReady(ctx, clientset, sourceNamespace, deploymentDatabaseName, waitOpts); err != nil { + return fmt.Errorf(`deployment database %s/%s is not ready: %w`, sourceNamespace, deploymentDatabaseName, err) + } + if err := copyDeploymentDatabaseCredentials(ctx, clientset, sourceNamespace, tenantNamespace); err != nil { + return fmt.Errorf(`copy deployment database credentials to %s: %w`, tenantNamespace, err) + } + return nil +} + +func copyDeploymentDatabaseCredentials( + ctx context.Context, + clientset kubernetes.Interface, + sourceNamespace string, + tenantNamespace string, +) error { + const ( + sourceSecretName = "openshell-db-credentials" + gwSecretName = "openshell-gateway-db-credentials" + ) + + sourceSecret, err := clientset.CoreV1().Secrets(sourceNamespace).Get(ctx, sourceSecretName, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("read source database credentials from %s/%s: %w", sourceNamespace, sourceSecretName, err) + } + + required := map[string]string{} + for _, key := range []string{"dbname", "user", "password"} { + value := string(sourceSecret.Data[key]) + if value == "" { + return fmt.Errorf("source database credentials %s/%s is missing required key %q", sourceNamespace, sourceSecretName, key) + } + required[key] = value + } + + host := fmt.Sprintf("openshell-gateway-db.%s.svc.cluster.local", sourceNamespace) + port := "5432" + dbURI := fmt.Sprintf("postgresql://%s:%s@%s:%s/%s?sslmode=disable", + required["user"], url.QueryEscape(required["password"]), host, port, required["dbname"]) + desiredData := map[string][]byte{ + "host": []byte(host), + "port": []byte(port), + "dbname": []byte(required["dbname"]), + "user": []byte(required["user"]), + "password": []byte(required["password"]), + "uri": []byte(dbURI), + } + desiredLabels := map[string]string{ + "app.kubernetes.io/name": "openshell", + "app.kubernetes.io/component": "database", + "app.kubernetes.io/managed-by": "hypershell-control-plane", + "hypershell.redhat.io/managed": "true", + } + + secrets := clientset.CoreV1().Secrets(tenantNamespace) + existing, err := secrets.Get(ctx, gwSecretName, metav1.GetOptions{}) + if err != nil && !k8serrors.IsNotFound(err) { + return fmt.Errorf("get gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) + } + if k8serrors.IsNotFound(err) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: gwSecretName, Namespace: tenantNamespace, Labels: desiredLabels}, + Type: corev1.SecretTypeOpaque, + Data: desiredData, + } + if _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("create gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) + } + log.Printf("INFO copied deployment database credentials to %s (host=%s db=%s)", tenantNamespace, host, required["dbname"]) + return nil + } + + updated := existing.DeepCopy() + if updated.Labels == nil { + updated.Labels = map[string]string{} + } + for key, value := range desiredLabels { + updated.Labels[key] = value + } + updated.Type = corev1.SecretTypeOpaque + updated.Data = desiredData + if reflect.DeepEqual(existing.Labels, updated.Labels) && existing.Type == updated.Type && reflect.DeepEqual(existing.Data, updated.Data) { + return nil + } + if _, err := secrets.Update(ctx, updated, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("update gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) + } + log.Printf("INFO updated deployment database credentials in %s (host=%s db=%s)", tenantNamespace, host, required["dbname"]) + return nil +} diff --git a/components/control-plane/internal/gateway/deployment_readiness_test.go b/components/control-plane/internal/gateway/deployment_db_test.go similarity index 100% rename from components/control-plane/internal/gateway/deployment_readiness_test.go rename to components/control-plane/internal/gateway/deployment_db_test.go diff --git a/components/control-plane/internal/gateway/deployment_readiness.go b/components/control-plane/internal/gateway/deployment_readiness.go deleted file mode 100644 index 8317a1ce..00000000 --- a/components/control-plane/internal/gateway/deployment_readiness.go +++ /dev/null @@ -1,79 +0,0 @@ -package gateway - -import ( - "context" - "fmt" - "k8s.io/client-go/kubernetes" - "time" -) - -const deploymentDatabaseName = `openshell-gateway-db` -const deploymentReadinessWaitTimeout = 2 * time.Minute -const deploymentReadinessPollInterval = 2 * time.Second - -type deploymentReadinessWaitOptions struct{ timeout, pollInterval time.Duration } - -func defaultDeploymentReadinessWaitOptions() deploymentReadinessWaitOptions { - return deploymentReadinessWaitOptions{timeout: deploymentReadinessWaitTimeout, pollInterval: deploymentReadinessPollInterval} -} -func waitForDeploymentReady(ctx context.Context, clientset kubernetes.Interface, namespace, name string, opts deploymentReadinessWaitOptions) error { - if opts.timeout <= 0 || opts.pollInterval <= 0 { - return fmt.Errorf(`wait for deployment %s/%s: invalid wait options`, namespace, name) - } - observation := `not ready` - check := func() bool { - ready, reason, err := DeploymentReadiness(ctx, clientset, namespace, name) - if err != nil { - observation = `readiness check temporarily failed` - return false - } - if ready { - return true - } - if reason != `` { - observation = reason - } - return false - } - if err := ctx.Err(); err != nil { - return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) - } - if check() { - return nil - } - if err := ctx.Err(); err != nil { - return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) - } - timer := time.NewTimer(opts.timeout) - defer timer.Stop() - ticker := time.NewTicker(opts.pollInterval) - defer ticker.Stop() - for { - select { - case <-ctx.Done(): - return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, ctx.Err()) - case <-timer.C: - return fmt.Errorf(`timed out waiting for deployment %s/%s to become ready: %s`, namespace, name, observation) - case <-ticker.C: - if check() { - return nil - } - if err := ctx.Err(); err != nil { - return fmt.Errorf(`wait for deployment %s/%s to become ready: %w`, namespace, name, err) - } - } - } -} -func reconcileDeploymentDatabaseCredentials(ctx context.Context, clientset kubernetes.Interface, sourceNamespace, tenantNamespace string, options ...deploymentReadinessWaitOptions) error { - waitOpts := defaultDeploymentReadinessWaitOptions() - if len(options) > 0 { - waitOpts = options[0] - } - if err := waitForDeploymentReady(ctx, clientset, sourceNamespace, deploymentDatabaseName, waitOpts); err != nil { - return fmt.Errorf(`deployment database %s/%s is not ready: %w`, sourceNamespace, deploymentDatabaseName, err) - } - if err := copyDeploymentDatabaseCredentials(ctx, clientset, sourceNamespace, tenantNamespace); err != nil { - return fmt.Errorf(`copy deployment database credentials to %s: %w`, tenantNamespace, err) - } - return nil -} diff --git a/components/control-plane/internal/gateway/external_db.go b/components/control-plane/internal/gateway/external_db.go new file mode 100644 index 00000000..07d7119a --- /dev/null +++ b/components/control-plane/internal/gateway/external_db.go @@ -0,0 +1,625 @@ +package gateway + +import ( + "context" + cryptoRand "crypto/rand" + "database/sql" + "encoding/hex" + "errors" + "fmt" + "log" + "net" + "net/url" + "os" + "reflect" + "regexp" + "strings" + + // register postgres driver and use typed error codes for status mapping + pq "github.com/lib/pq" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" +) + +type externalDatabaseReconciler struct { + cfg ExternalDBConfig +} + +// Reconcile provisions the gateway's external role and database. The +// rotateAnnotation parameter is deliberately ignored: external mode does not +// implement credential rotation, so the hypershell.redhat.io/rotate-db-credentials +// annotation is inert for external-backed gateways. See +// openshell-gateway-database-external.spec.md § Requirement: No Credential +// Rotation (External Mode). +func (r *externalDatabaseReconciler) Reconcile(ctx context.Context, _ dynamic.Interface, clientset kubernetes.Interface, tenantNamespace, gatewayID, _ string) error { + if err := ReconcileExternalDatabaseResources(ctx, clientset, tenantNamespace, gatewayID, r.cfg); err != nil { + return fmt.Errorf("reconcile external database resources in %s: %w", tenantNamespace, err) + } + 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. +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 { + 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) + } + return nil +} + +// externalAdminParams holds the admin connection parameters read from the +// credentials Secret. It is only ever alive for the duration of one reconcile. +type externalAdminParams struct { + host string + port string + user string + password string + dbname string + sslmode string + sslrootcert string // optional inline PEM CA bundle; enables certificate verification +} + +// externalCredentialsNamespacePrefix is the reserved prefix for the namespace +// holding an external server's admin credentials. ManagedDatabase.connection_secret +// names that NAMESPACE, not a Secret: the credentials are provisioned out-of-band, +// normally before HyperShell is installed, so they must not depend on the control +// plane instance namespace existing. +// +// The prefix is a security boundary, not a convention. Combined with the fixed +// Secret name below it bounds what the control plane can be made to read to a +// single deliberately-named Secret inside deliberately-created namespaces, +// preventing an API-level reference from pointing this reconciler at an unrelated +// Secret such as hypershell-db-app. The same values are enforced by the API server +// (plugins/managedDatabases/service.go). See naming-multitenancy.spec.md §6.2. +const externalCredentialsNamespacePrefix = "hypershell-managed-db-" + +// externalCredentialsSecretName is the fixed name of the only Secret this +// reconciler reads inside a hypershell-managed-db- namespace. +const externalCredentialsSecretName = "hypershell-managed-db-credentials" + +// dns1123LabelMaxLength is the Kubernetes limit for a namespace name. +const dns1123LabelMaxLength = 63 + +// dns1123LabelPattern matches a valid DNS-1123 label (a valid namespace name). +var dns1123LabelPattern = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$`) + +// Closed-vocabulary status strings for the external DB probe and reconciler. +// Exported so callers (e.g. the ManagedDatabase reconciler) share the same +// vocabulary without re-declaring literals. +const ( + ExternalDBStatusReady = "Ready" + ExternalDBStatusUnreachable = "Failed: unreachable" + ExternalDBStatusTLSFailed = "Failed: tls_failed" + ExternalDBStatusAuthFailed = "Failed: auth_failed" + ExternalDBStatusInsufficientPrivilege = "Failed: insufficient_privilege" + ExternalDBStatusSecretInvalid = "Failed: secret_invalid" +) + +// validateCredentialsNamespace enforces the connection_secret reference rules +// (no slash, reserved prefix, valid DNS-1123 label) before any read is +// attempted. This runs again here, after API-server validation, so a row +// written before the rule existed cannot cause a read. +func validateCredentialsNamespace(namespace string) error { + if namespace == "" { + return fmt.Errorf("connection_secret namespace is empty") + } + if strings.Contains(namespace, "/") { + return fmt.Errorf("connection_secret %q contains a '/'; it must be a bare namespace name", namespace) + } + if !strings.HasPrefix(namespace, externalCredentialsNamespacePrefix) { + return fmt.Errorf("connection_secret namespace %q does not start with the required prefix %q", namespace, externalCredentialsNamespacePrefix) + } + if len(namespace) > dns1123LabelMaxLength { + return fmt.Errorf("connection_secret namespace %q exceeds the %d-character namespace name limit", namespace, dns1123LabelMaxLength) + } + if !dns1123LabelPattern.MatchString(namespace) { + return fmt.Errorf("connection_secret namespace %q is not a valid DNS-1123 label", namespace) + } + return nil +} + +// readExternalAdminSecret validates the credentials namespace and reads the +// fixed-name admin Secret from it. No other Secret in that namespace is read. +func readExternalAdminSecret(ctx context.Context, clientset kubernetes.Interface, credentialsNamespace string) (*externalAdminParams, error) { + if err := validateCredentialsNamespace(credentialsNamespace); err != nil { + return nil, fmt.Errorf("connection_secret validation: %w", err) + } + + secret, err := clientset.CoreV1().Secrets(credentialsNamespace).Get(ctx, externalCredentialsSecretName, 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("read Secret %q in namespace %q: %w", externalCredentialsSecretName, 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) + } + } + + dbname := get("dbname") + if dbname == "" { + 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) + case "require", "allow", "prefer": + log.Printf("WARN external DB credentials in namespace %s: sslmode=%s encrypts the connection but does not verify the server certificate; use verify-full with sslrootcert for production external servers", credentialsNamespace, sslmode) + } + + return &externalAdminParams{ + host: get("host"), + port: get("port"), + user: get("user"), + password: get("password"), + dbname: dbname, + sslmode: sslmode, + sslrootcert: get("sslrootcert"), + }, nil +} + +// dsn builds the admin connection string. sslrootcert is supplied by the +// operator as inline PEM, but lib/pq only accepts a file-system path for that +// parameter, so the PEM is materialised into a private temp file whose lifetime +// is bounded by the returned cleanup func. cleanup is always non-nil. +func (p *externalAdminParams) dsn() (string, func(), error) { + noop := func() {} + + // Use URL format so special characters in credentials are safely percent-encoded + // instead of breaking the space-delimited keyword DSN. + u := &url.URL{ + Scheme: "postgres", + User: url.UserPassword(p.user, p.password), + Host: net.JoinHostPort(p.host, p.port), + Path: "/" + p.dbname, + } + q := url.Values{ + "sslmode": {p.sslmode}, + "connect_timeout": {"10"}, + } + + cleanup := noop + if p.sslrootcert != "" { + f, err := os.CreateTemp("", "hypershell-external-db-ca-*.pem") + if err != nil { + return "", noop, fmt.Errorf("materialise CA bundle: %w", err) + } + path := f.Name() + cleanup = func() { + if rerr := os.Remove(path); rerr != nil && !os.IsNotExist(rerr) { + log.Printf("WARN external DB: remove temporary CA bundle: %v", rerr) + } + } + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + cleanup() + return "", noop, fmt.Errorf("secure CA bundle: %w", err) + } + if _, err := f.WriteString(p.sslrootcert); err != nil { + _ = f.Close() + cleanup() + return "", noop, fmt.Errorf("write CA bundle: %w", err) + } + if err := f.Close(); err != nil { + cleanup() + return "", noop, fmt.Errorf("close CA bundle: %w", err) + } + q.Set("sslrootcert", path) + } + + u.RawQuery = q.Encode() + return u.String(), cleanup, nil +} + +// openAdminConn opens a short-lived PostgreSQL admin connection. The returned +// release func closes the connection and removes any temporary CA bundle; +// callers must always defer it. It is non-nil on every return path. +// PingContext errors are returned unwrapped so callers can errors.As them for +// typed classification (*pq.Error / net.Error) via mapConnErrorToStatus. +func openAdminConn(ctx context.Context, params *externalAdminParams) (*sql.DB, func(), error) { + noop := func() {} + + dsn, cleanupCA, err := params.dsn() + if err != nil { + return nil, noop, err + } + + db, err := sql.Open("postgres", dsn) + if err != nil { + cleanupCA() + return nil, noop, fmt.Errorf("open admin connection: %w", err) + } + db.SetMaxOpenConns(1) + + release := func() { + if cerr := db.Close(); cerr != nil { + log.Printf("WARN external DB: close admin connection: %v", cerr) + } + // The CA file must outlive the connection: lib/pq reads it when a + // pooled connection is (re)established, not only at Open. + cleanupCA() + } + + if err := db.PingContext(ctx); err != nil { + release() + return nil, noop, err + } + return db, release, nil +} + +// mapConnErrorToStatus maps a raw connection error to a closed-vocabulary +// ManagedDatabase status string. The raw error is never returned. +func mapConnErrorToStatus(err error) string { + if err == nil { + return ExternalDBStatusReady + } + msg := err.Error() + lower := strings.ToLower(msg) + + // Network-level failures: typed check first, then best-effort string matching + // for errors the driver returns as unwrapped plain strings. + var netErr net.Error + if errors.As(err, &netErr) { + return ExternalDBStatusUnreachable + } + if strings.Contains(lower, "connection refused") || + strings.Contains(lower, "no such host") || + strings.Contains(lower, "i/o timeout") || + strings.Contains(lower, "network") { + return ExternalDBStatusUnreachable + } + // Auth failures: typed SQLSTATE check runs before TLS string matching so that + // a pq.Error with code 28P01/28000 is always classified auth_failed even when + // its message incidentally contains "ssl" (e.g. SSL-wrapped auth rejections). + var pqErr *pq.Error + if errors.As(err, &pqErr) { + switch pqErr.Code { + case "28P01", "28000": // invalid_password, invalid_authorization_specification + return ExternalDBStatusAuthFailed + } + } + // TLS/auth best-effort string matching: only reached for unwrapped driver errors + // where the typed checks above did not match. Result is a status label only + // (no control-flow branch on it), so a misclassification has low impact. + if strings.Contains(lower, "tls") || + strings.Contains(lower, "certificate") || + strings.Contains(lower, "x509") || + strings.Contains(lower, "ssl") { + return ExternalDBStatusTLSFailed + } + if strings.Contains(lower, "password authentication failed") || + strings.Contains(lower, "28p01") || + strings.Contains(lower, "28000") { + return ExternalDBStatusAuthFailed + } + // Unclassified errors: deliberately collapsed to unreachable. This is a + // status label only; no control-flow branch depends on the value, so a + // misclassification has low impact. + return ExternalDBStatusUnreachable +} + +// ProbeExternalServer opens a short-lived admin connection, verifies the +// 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) + if err != nil { + log.Printf("INFO external DB probe (namespace %s): %s: %v", cfg.CredentialsNamespace, ExternalDBStatusSecretInvalid, err) + return ExternalDBStatusSecretInvalid + } + + db, release, err := openAdminConn(ctx, params) + if err != nil { + status := mapConnErrorToStatus(err) + log.Printf("INFO external DB probe (namespace %s): %s (connection error redacted)", cfg.CredentialsNamespace, status) + return status + } + defer release() + + var rolsuper, rolcreatedb, rolcreaterole bool + row := db.QueryRowContext(ctx, "SELECT rolsuper, rolcreatedb, rolcreaterole FROM pg_roles WHERE rolname = current_user") + if err := row.Scan(&rolsuper, &rolcreatedb, &rolcreaterole); err != nil { + log.Printf("INFO external DB probe (namespace %s): %s (query error redacted)", cfg.CredentialsNamespace, ExternalDBStatusUnreachable) + return ExternalDBStatusUnreachable + } + + // Superusers have implicit CREATE DATABASE / CREATE ROLE privileges regardless + // of rolcreatedb/rolcreaterole, so short-circuit on rolsuper. + if !rolsuper && (!rolcreatedb || !rolcreaterole) { + log.Printf("INFO external DB probe (namespace %s): %s (rolsuper=%v rolcreatedb=%v rolcreaterole=%v)", + cfg.CredentialsNamespace, ExternalDBStatusInsufficientPrivilege, rolsuper, rolcreatedb, rolcreaterole) + return ExternalDBStatusInsufficientPrivilege + } + + log.Printf("INFO external DB probe (namespace %s): %s", cfg.CredentialsNamespace, ExternalDBStatusReady) + return ExternalDBStatusReady +} + +// externalGatewayDBName returns the PostgreSQL role/database name for a +// gateway. Role and database share the name. +func externalGatewayDBName(gatewayID string) string { + return "gw_" + strings.ToLower(gatewayID) +} + +// tenantGatewayDBSecretName is the tenant-namespace Secret the gateway workload +// consumes as --db-url. +const tenantGatewayDBSecretName = "openshell-gateway-db-credentials" + +// ReconcileExternalDatabaseResources provisions a dedicated role and database +// on the external server for gatewayID, then writes the tenant credentials +// Secret. It is idempotent: re-running against an already-provisioned gateway +// makes no destructive change and does not regenerate the password. +func ReconcileExternalDatabaseResources( + ctx context.Context, + clientset kubernetes.Interface, + tenantNamespace string, + gatewayID string, + cfg ExternalDBConfig, +) error { + params, err := readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + if err != nil { + return fmt.Errorf("read external admin credentials: %w", err) + } + + pgName := externalGatewayDBName(gatewayID) + + // Determine password: reuse from existing tenant Secret (create-or-skip), + // or generate a new one. + password := "" + existingSecret, secretErr := clientset.CoreV1().Secrets(tenantNamespace).Get(ctx, tenantGatewayDBSecretName, metav1.GetOptions{}) + if secretErr != nil && !k8serrors.IsNotFound(secretErr) { + return fmt.Errorf("get gateway credentials secret %s/%s: %w", tenantNamespace, tenantGatewayDBSecretName, secretErr) + } + secretExists := secretErr == nil + if secretExists { + password = string(existingSecret.Data["password"]) + } + + freshPassword := password == "" + if freshPassword { + passwordBytes := make([]byte, 32) + if _, err := cryptoRand.Read(passwordBytes); err != nil { + return fmt.Errorf("generate database password: %w", err) + } + password = hex.EncodeToString(passwordBytes) + } + + // Open admin connection and issue idempotent DDL. + db, release, err := openAdminConn(ctx, params) + if err != nil { + return fmt.Errorf("connect to external server (%s): connection failed (credentials redacted)", mapConnErrorToStatus(err)) + } + defer release() + + // Role: create if absent; if present but we generated a new password + // (because the Secret was missing), apply the new password to the role. + var roleExists bool + 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 existence for gateway %s: %w", gatewayID, err) + } + + if !roleExists { + // lib/pq cannot parameterize CREATE ROLE / ALTER ROLE, so the password is + // interpolated into the statement text. On servers with log_statement=all + // the password will appear in the server log; operators should restrict + // log verbosity or use server-side log redaction accordingly. + if _, err := db.ExecContext(ctx, + fmt.Sprintf("CREATE ROLE %s LOGIN PASSWORD '%s'", pgQuoteIdent(pgName), pgQuoteLiteral(password)), + ); err != nil { + return fmt.Errorf("CREATE ROLE for gateway %s: DDL execution failed (credentials redacted)", gatewayID) + } + log.Printf("INFO created external DB role %s for gateway %s", pgName, gatewayID) + } else if freshPassword { + // Secret was absent but the role exists: sync the new password to + // PostgreSQL so the gateway can authenticate with the Secret we are about + // to write. This is provisioning repair, NOT credential rotation -- + // external mode has no rotation (see the external database spec). Do not + // route the rotate-db-credentials annotation here. + if _, err := db.ExecContext(ctx, + fmt.Sprintf("ALTER ROLE %s PASSWORD '%s'", pgQuoteIdent(pgName), pgQuoteLiteral(password)), + ); err != nil { + return fmt.Errorf("ALTER ROLE password for gateway %s: DDL execution failed (credentials redacted)", gatewayID) + } + log.Printf("INFO repaired external DB role password for gateway %s (tenant Secret was absent)", gatewayID) + } + // Out-of-band password drift (the tenant Secret exists but the server-side role + // password was changed externally) is not reconciled: detecting it would require + // a round-trip login on every reconcile, which is too expensive against a shared + // server. Recover by deleting the tenant Secret, which makes the branch above + // re-apply a fresh password on the next reconcile. + + // Database: create if absent. + var dbExists bool + 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 existence for gateway %s: %w", gatewayID, err) + } + if !dbExists { + // CREATE DATABASE cannot run inside a transaction block and has no IF NOT EXISTS. + if _, err := db.ExecContext(ctx, + fmt.Sprintf("CREATE DATABASE %s OWNER %s", pgQuoteIdent(pgName), pgQuoteIdent(pgName)), + ); err != nil { + return fmt.Errorf("CREATE DATABASE for gateway %s: %w", gatewayID, err) + } + log.Printf("INFO created external DB database %s for gateway %s", pgName, gatewayID) + } + + // Isolation: revoke PUBLIC connect, grant only the gateway role. + if _, err := db.ExecContext(ctx, + fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM PUBLIC", pgQuoteIdent(pgName)), + ); err != nil { + return fmt.Errorf("REVOKE CONNECT for gateway %s: %w", gatewayID, err) + } + if _, err := db.ExecContext(ctx, + fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s", pgQuoteIdent(pgName), pgQuoteIdent(pgName)), + ); err != nil { + 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), + } + desiredLabels := map[string]string{ + "app.kubernetes.io/name": "openshell", + "app.kubernetes.io/component": "database", + "app.kubernetes.io/managed-by": "hypershell-control-plane", + "hypershell.redhat.io/managed": "true", + } + + if !secretExists { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tenantGatewayDBSecretName, + Namespace: tenantNamespace, + Labels: desiredLabels, + }, + Type: corev1.SecretTypeOpaque, + Data: desiredData, + } + if _, err := clientset.CoreV1().Secrets(tenantNamespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { + return fmt.Errorf("create gateway credentials secret %s/%s: %w", tenantNamespace, tenantGatewayDBSecretName, err) + } + log.Printf("INFO created external gateway credentials secret %s in %s", tenantGatewayDBSecretName, tenantNamespace) + } else { + updated := existingSecret.DeepCopy() + if updated.Labels == nil { + updated.Labels = map[string]string{} + } + for k, v := range desiredLabels { + updated.Labels[k] = v + } + updated.Data = desiredData + if !reflect.DeepEqual(existingSecret.Data, updated.Data) || !reflect.DeepEqual(existingSecret.Labels, updated.Labels) { + if _, err := clientset.CoreV1().Secrets(tenantNamespace).Update(ctx, updated, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("update gateway credentials secret %s/%s: %w", tenantNamespace, tenantGatewayDBSecretName, err) + } + } + } + + log.Printf("INFO external DB provisioning complete for gateway %s in namespace %s", gatewayID, tenantNamespace) + return nil +} + +// DeleteExternalDatabaseResources terminates active connections and drops the +// 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. +func DeleteExternalDatabaseResources( + ctx context.Context, + clientset kubernetes.Interface, + cfg ExternalDBConfig, + gatewayID string, +) error { + if cfg.CredentialsNamespace == "" || gatewayID == "" { + return nil + } + + params, err := readExternalAdminSecret(ctx, clientset, cfg.CredentialsNamespace) + if err != nil { + return fmt.Errorf("cannot read external admin credentials: %w", err) + } + + db, release, err := openAdminConn(ctx, params) + if err != nil { + return fmt.Errorf("cannot connect to external server (%s): connection failed (credentials redacted)", mapConnErrorToStatus(err)) + } + defer release() + + pgName := externalGatewayDBName(gatewayID) + + // Terminate active backends so DROP DATABASE is not blocked. + // Terminating another role's backends needs superuser or pg_signal_backend + // membership; a failure here is not fatal because DROP DATABASE ... WITH + // (FORCE) below performs the same termination server-side. + if _, err := db.ExecContext(ctx, + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = $1 AND pid <> pg_backend_pid()", + pgName, + ); err != nil { + log.Printf("WARN external DB cleanup for gateway %s: terminate backends failed (proceeding): %v", gatewayID, err) + } + + // 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.ExecContext(ctx, fmt.Sprintf("DROP DATABASE %s WITH (FORCE)", pgQuoteIdent(pgName))); err != nil { + return fmt.Errorf("DROP DATABASE failed: %w", err) + } + log.Printf("INFO dropped external database %s for gateway %s", pgName, gatewayID) + } + + // 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.ExecContext(ctx, fmt.Sprintf("DROP ROLE %s", pgQuoteIdent(pgName))); err != nil { + return fmt.Errorf("DROP ROLE failed: %w", err) + } + log.Printf("INFO dropped external role %s for gateway %s", pgName, gatewayID) + } + return nil +} + +// pgQuoteIdent quotes a PostgreSQL identifier to prevent SQL injection. +// Only safe for identifiers produced from internal gateway IDs. +func pgQuoteIdent(name string) string { + return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` +} + +// pgQuoteLiteral quotes a string literal for use in SQL by doubling single +// quotes. Safe here because every interpolated value is a hex-encoded password +// (character set [0-9a-f]); DO NOT reuse this function for arbitrary user input. +func pgQuoteLiteral(s string) string { + return strings.ReplaceAll(s, "'", "''") +} diff --git a/components/control-plane/internal/gateway/external_db_test.go b/components/control-plane/internal/gateway/external_db_test.go new file mode 100644 index 00000000..2b672a85 --- /dev/null +++ b/components/control-plane/internal/gateway/external_db_test.go @@ -0,0 +1,433 @@ +package gateway + +import ( + "context" + "errors" + "net" + "net/url" + "os" + "strings" + "testing" + + pq "github.com/lib/pq" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sfake "k8s.io/client-go/kubernetes/fake" +) + +// --- validateCredentialsNamespace --- +// +// connection_secret names the NAMESPACE holding the fixed-name credentials +// Secret, so the value must be a bare, prefixed, valid DNS-1123 label. + +func TestValidateCredentialsNamespace(t *testing.T) { + cases := []struct { + name string + input string + wantErr bool + }{ + {name: "empty", input: "", wantErr: true}, + {name: "namespace slash", input: "hypershell-managed-db-foo/bar", wantErr: true}, + {name: "missing prefix", input: "my-namespace", wantErr: true}, + {name: "control plane namespace rejected", input: "hypershell", wantErr: true}, + {name: "uppercase is not a DNS-1123 label", input: "hypershell-managed-db-Prod", wantErr: true}, + {name: "dots are not allowed in a namespace name", input: "hypershell-managed-db-a.b", wantErr: true}, + {name: "trailing dash", input: "hypershell-managed-db-", wantErr: true}, + {name: "over 63 characters", input: "hypershell-managed-db-" + strings.Repeat("a", 64), wantErr: true}, + {name: "valid", input: "hypershell-managed-db-prod", wantErr: false}, + {name: "valid with numbers", input: "hypershell-managed-db-123abc", wantErr: false}, + {name: "exactly 63 characters", input: "hypershell-managed-db-" + strings.Repeat("a", 63-len("hypershell-managed-db-")), wantErr: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := validateCredentialsNamespace(tc.input) + if tc.wantErr && err == nil { + t.Errorf("expected error for input %q, got nil", tc.input) + } + if !tc.wantErr && err != nil { + t.Errorf("unexpected error for input %q: %v", tc.input, err) + } + }) + } +} + +// --- pgQuoteIdent --- + +func TestPgQuoteIdent(t *testing.T) { + cases := []struct { + input string + want string + }{ + {`simple`, `"simple"`}, + {`with"quote`, `"with""quote"`}, + {`gw_abc123`, `"gw_abc123"`}, + } + for _, tc := range cases { + t.Run(tc.input, func(t *testing.T) { + got := pgQuoteIdent(tc.input) + if got != tc.want { + t.Errorf("pgQuoteIdent(%q) = %q, want %q", tc.input, got, tc.want) + } + }) + } +} + +// --- pgQuoteLiteral --- + +func TestPgQuoteLiteral(t *testing.T) { + cases := []struct { + input string + want string + }{ + {`abc`, `abc`}, + {`it's`, `it''s`}, + {`''`, `''''`}, + {`abcdef0123456789`, `abcdef0123456789`}, + } + for _, tc := range cases { + t.Run(tc.input, func(t *testing.T) { + got := pgQuoteLiteral(tc.input) + if got != tc.want { + t.Errorf("pgQuoteLiteral(%q) = %q, want %q", tc.input, got, tc.want) + } + }) + } +} + +// --- mapConnErrorToStatus --- + +func TestMapConnErrorToStatus(t *testing.T) { + cases := []struct { + name string + err error + want string + }{ + {name: "nil", err: nil, want: ExternalDBStatusReady}, + {name: "connection refused", err: errors.New("connection refused"), want: ExternalDBStatusUnreachable}, + {name: "no such host", err: errors.New("dial tcp: no such host"), want: ExternalDBStatusUnreachable}, + {name: "network error", err: errors.New("network unreachable"), want: ExternalDBStatusUnreachable}, + {name: "tls error", err: errors.New("tls: certificate signed by unknown authority"), want: ExternalDBStatusTLSFailed}, + {name: "x509 error", err: errors.New("x509: certificate has expired"), want: ExternalDBStatusTLSFailed}, + {name: "ssl error", err: errors.New("ssl SYSCALL error"), want: ExternalDBStatusTLSFailed}, + {name: "pq auth 28P01", err: &pq.Error{Code: "28P01"}, want: ExternalDBStatusAuthFailed}, + {name: "pq auth 28000", err: &pq.Error{Code: "28000"}, want: ExternalDBStatusAuthFailed}, + // pq.Error with SSL in message must still be auth_failed (typed check beats string match) + {name: "pq auth 28P01 with ssl message", err: &pq.Error{Code: "28P01", Message: "SSL connection required"}, want: ExternalDBStatusAuthFailed}, + {name: "password authentication failed", err: errors.New("password authentication failed for user foo"), want: ExternalDBStatusAuthFailed}, + {name: "net.Error timeout", err: &fakeNetError{timeout: true}, want: ExternalDBStatusUnreachable}, + {name: "unknown", err: errors.New("some unexpected error"), want: ExternalDBStatusUnreachable}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := mapConnErrorToStatus(tc.err) + if got != tc.want { + t.Errorf("mapConnErrorToStatus(%v) = %q, want %q", tc.err, got, tc.want) + } + }) + } +} + +type fakeNetError struct{ timeout bool } + +func (e *fakeNetError) Error() string { return "fake net error" } +func (e *fakeNetError) Timeout() bool { return e.timeout } +func (e *fakeNetError) Temporary() bool { return false } + +var _ net.Error = (*fakeNetError)(nil) + +// --- readExternalAdminSecret --- + +func TestReadExternalAdminSecret(t *testing.T) { + ctx := context.Background() + credentialsNS := "hypershell-managed-db-kind" + + newSecret := func(data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: externalCredentialsSecretName, Namespace: credentialsNS}, + Data: data, + } + } + + t.Run("secret not found", func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + if _, err := readExternalAdminSecret(ctx, client, credentialsNS); err == nil { + t.Fatal("expected error for missing secret, got nil") + } + }) + + t.Run("invalid namespace reference is rejected before any read", func(t *testing.T) { + // A Secret with the fixed name exists in a namespace that does NOT carry + // the reserved prefix. Validation must reject the reference rather than + // read it. + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: externalCredentialsSecretName, Namespace: "kube-system"}, + Data: map[string][]byte{ + "host": []byte("h"), "port": []byte("5432"), + "user": []byte("u"), "password": []byte("p"), + }, + } + client := k8sfake.NewSimpleClientset(secret) + if _, err := readExternalAdminSecret(ctx, client, "kube-system"); err == nil { + t.Fatal("expected error for unprefixed namespace, got nil") + } + }) + + t.Run("only the fixed-name Secret is read", func(t *testing.T) { + // A differently-named Secret in an otherwise valid namespace must not be + // picked up: the reachable set is exactly one name. + other := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "some-other-secret", Namespace: credentialsNS}, + Data: map[string][]byte{ + "host": []byte("h"), "port": []byte("5432"), + "user": []byte("u"), "password": []byte("p"), + }, + } + client := k8sfake.NewSimpleClientset(other) + if _, err := readExternalAdminSecret(ctx, client, credentialsNS); err == nil { + t.Fatal("expected error when only a differently-named Secret exists, got nil") + } + }) + + t.Run("secret missing required key", func(t *testing.T) { + client := k8sfake.NewSimpleClientset(newSecret(map[string][]byte{ + "host": []byte("db.example.com"), + "port": []byte("5432"), + // user and password missing + })) + if _, err := readExternalAdminSecret(ctx, client, credentialsNS); err == nil { + t.Fatal("expected error for missing key, got nil") + } + }) + + t.Run("valid secret defaults dbname and sslmode", func(t *testing.T) { + client := k8sfake.NewSimpleClientset(newSecret(map[string][]byte{ + "host": []byte("db.example.com"), + "port": []byte("5432"), + "user": []byte("admin"), + "password": []byte("s3cr3t"), + })) + params, err := readExternalAdminSecret(ctx, client, credentialsNS) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if params.dbname != "postgres" { + t.Errorf("dbname = %q, want %q", params.dbname, "postgres") + } + if params.sslmode != "require" { + t.Errorf("sslmode = %q, want %q", params.sslmode, "require") + } + }) + + t.Run("valid secret with explicit dbname and sslmode", func(t *testing.T) { + client := k8sfake.NewSimpleClientset(newSecret(map[string][]byte{ + "host": []byte("db.example.com"), + "port": []byte("5432"), + "user": []byte("admin"), + "password": []byte("s3cr3t"), + "dbname": []byte("mydb"), + "sslmode": []byte("disable"), + })) + params, err := readExternalAdminSecret(ctx, client, credentialsNS) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if params.dbname != "mydb" { + t.Errorf("dbname = %q, want %q", params.dbname, "mydb") + } + if params.sslmode != "disable" { + t.Errorf("sslmode = %q, want %q", params.sslmode, "disable") + } + }) + + t.Run("inline PEM sslrootcert is carried through", func(t *testing.T) { + client := k8sfake.NewSimpleClientset(newSecret(map[string][]byte{ + "host": []byte("db.example.com"), + "port": []byte("5432"), + "user": []byte("admin"), + "password": []byte("s3cr3t"), + "sslmode": []byte("verify-full"), + "sslrootcert": []byte(testCAPEM), + })) + params, err := readExternalAdminSecret(ctx, client, credentialsNS) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if params.sslrootcert != testCAPEM { + t.Errorf("sslrootcert = %q, want the inline PEM", params.sslrootcert) + } + }) +} + +// --- dsn: URL format, connect_timeout, inline-PEM CA materialisation --- + +const testCAPEM = "-----BEGIN CERTIFICATE-----\nZmFrZQ==\n-----END CERTIFICATE-----\n" + +func TestDSNConnectTimeout(t *testing.T) { + p := &externalAdminParams{ + host: "db.example.com", port: "5432", + user: "admin", password: "pass", + dbname: "postgres", sslmode: "require", + } + dsn, cleanup, err := p.dsn() + if err != nil { + t.Fatalf("dsn() unexpected error: %v", err) + } + defer cleanup() + if !strings.Contains(dsn, "connect_timeout=10") { + t.Errorf("dsn does not contain connect_timeout=10: %s", dsn) + } +} + +func TestDSNURLFormat(t *testing.T) { + t.Run("special chars in password are percent-encoded", func(t *testing.T) { + p := &externalAdminParams{host: "h", port: "5432", user: "u", password: "p@ss w0rd!", dbname: "db", sslmode: "require"} + dsn, cleanup, err := p.dsn() + if err != nil { + t.Fatalf("dsn() unexpected error: %v", err) + } + defer cleanup() + if strings.Contains(dsn, "p@ss w0rd!") { + t.Errorf("password not encoded in dsn: %s", dsn) + } + if !strings.Contains(dsn, "p%40ss+w0rd%21") && !strings.Contains(dsn, "p%40ss%20w0rd%21") { + t.Errorf("expected percent-encoded password in dsn: %s", dsn) + } + }) +} + +func TestDSNSslrootcert(t *testing.T) { + t.Run("no sslrootcert omitted from dsn", func(t *testing.T) { + p := &externalAdminParams{host: "h", port: "5432", user: "u", password: "p", dbname: "db", sslmode: "require"} + dsn, cleanup, err := p.dsn() + if err != nil { + t.Fatalf("dsn() unexpected error: %v", err) + } + defer cleanup() + if strings.Contains(dsn, "sslrootcert") { + t.Errorf("unexpected sslrootcert in dsn: %s", dsn) + } + }) + + // The operator supplies inline PEM, but lib/pq only accepts a path, so dsn() + // must materialise the PEM into a real file and point the DSN at it. + t.Run("inline PEM is materialised to a private file", func(t *testing.T) { + p := &externalAdminParams{host: "h", port: "5432", user: "u", password: "p", dbname: "db", sslmode: "verify-full", sslrootcert: testCAPEM} + dsn, cleanup, err := p.dsn() + if err != nil { + t.Fatalf("dsn() unexpected error: %v", err) + } + + parsed, perr := url.Parse(dsn) + if perr != nil { + t.Fatalf("dsn is not a valid URL: %v", perr) + } + path := parsed.Query().Get("sslrootcert") + if path == "" { + t.Fatal("dsn missing sslrootcert parameter") + } + if path == testCAPEM { + t.Fatal("dsn passed inline PEM verbatim; lib/pq needs a file path") + } + + info, serr := os.Stat(path) + if serr != nil { + t.Fatalf("CA bundle not written to %s: %v", path, serr) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("CA bundle permissions = %o, want 600", perm) + } + content, rerr := os.ReadFile(path) + if rerr != nil { + t.Fatalf("read CA bundle: %v", rerr) + } + if string(content) != testCAPEM { + t.Errorf("CA bundle content = %q, want the inline PEM", string(content)) + } + + cleanup() + if _, serr := os.Stat(path); !os.IsNotExist(serr) { + t.Errorf("cleanup did not remove the CA bundle at %s", path) + } + }) + + t.Run("cleanup is safe to call when no CA file was written", func(t *testing.T) { + p := &externalAdminParams{host: "h", port: "5432", user: "u", password: "p", dbname: "db", sslmode: "require"} + _, cleanup, err := p.dsn() + if err != nil { + t.Fatalf("dsn() unexpected error: %v", err) + } + cleanup() + cleanup() + }) +} + +// --- DeleteExternalDatabaseResources early-exit paths --- + +func TestDeleteExternalDatabaseResourcesEarlyExit(t *testing.T) { + ctx := context.Background() + cfg := ExternalDBConfig{CredentialsNamespace: "hypershell-managed-db-kind", ManagedDatabaseID: "md-1"} + + t.Run("empty gatewayID returns nil without connecting", func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + if err := DeleteExternalDatabaseResources(ctx, client, cfg, ""); err != nil { + t.Errorf("expected nil for empty gatewayID, got %v", err) + } + }) + + t.Run("empty credentials namespace returns nil without connecting", func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + empty := ExternalDBConfig{CredentialsNamespace: "", ManagedDatabaseID: "md-1"} + if err := DeleteExternalDatabaseResources(ctx, client, empty, "gw-abc123"); err != nil { + t.Errorf("expected nil for empty credentials namespace, got %v", err) + } + }) + + t.Run("missing secret returns error", func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + if err := DeleteExternalDatabaseResources(ctx, client, cfg, "gw-abc123"); err == nil { + t.Error("expected error when the credentials Secret is missing, got nil") + } + }) +} + +// --- single-shot cleanup semantics --- + +// Cleanup is unconditional and single-shot: there is no tombstone and no retry +// queue, so the reconciler must swallow the failure (after logging it) rather +// than returning an error that would strand gateway finalization on a retry +// that can never be re-delivered. +func TestExternalDatabaseReconcilerDeleteNeverReturnsError(t *testing.T) { + ctx := context.Background() + client := k8sfake.NewSimpleClientset() // no credentials Secret -> cleanup fails + r := &externalDatabaseReconciler{cfg: ExternalDBConfig{ + CredentialsNamespace: "hypershell-managed-db-kind", + ManagedDatabaseID: "md-1", + }} + + if err := r.Delete(ctx, nil, client, "gw-abc123"); err != nil { + t.Fatalf("Delete() = %v, want nil (cleanup is best-effort and single-shot)", err) + } +} + +func TestExternalDatabaseReconcilerDeleteNoopsWithoutIdentifiers(t *testing.T) { + ctx := context.Background() + client := k8sfake.NewSimpleClientset() + + r := &externalDatabaseReconciler{cfg: ExternalDBConfig{CredentialsNamespace: "hypershell-managed-db-kind"}} + if err := r.Delete(ctx, nil, client, ""); err != nil { + t.Errorf("Delete() with empty gatewayID = %v, want nil", err) + } + + r = &externalDatabaseReconciler{cfg: ExternalDBConfig{}} + if err := r.Delete(ctx, nil, client, "gw-abc123"); err != nil { + t.Errorf("Delete() with empty config = %v, want nil", err) + } +} + +// --- externalGatewayDBName --- + +func TestExternalGatewayDBName(t *testing.T) { + if got := externalGatewayDBName("2J5K7M9PqrsTvwxyz"); got != "gw_2j5k7m9pqrstvwxyz" { + t.Errorf("externalGatewayDBName() = %q, want lowercased gw_ name", got) + } +} diff --git a/components/control-plane/internal/gateway/reconciler.go b/components/control-plane/internal/gateway/reconciler.go index 8e60dcfd..8491a4a5 100644 --- a/components/control-plane/internal/gateway/reconciler.go +++ b/components/control-plane/internal/gateway/reconciler.go @@ -10,9 +10,7 @@ import ( "errors" "fmt" "log" - "net/url" "os" - "reflect" "sort" "strings" "sync" @@ -84,36 +82,12 @@ func ReconcileGateway( } } - switch opts.DatabaseProvider { - case "": - // Legacy gateways may not have a database_id. Preserve any existing - // database resources and continue reconciling the rest of the gateway. - case "cnpg": - if opts.CNPG.ClusterNamespace == "" { - return fmt.Errorf("CNPG cluster namespace is required for gateway database reconciliation in namespace %s", nsConfig.Name) - } - if !opts.HasCNPG { - return fmt.Errorf("CNPG operator is required but not available on the cluster: gateway deployment blocked for namespace %s", nsConfig.Name) - } - - if err := reconcileCNPGDatabaseResources(ctx, dynamicClient, clientset, nsConfig.Name, opts.GatewayID, opts.CNPG); err != nil { - return fmt.Errorf("reconcile CNPG database resources in %s: %w", nsConfig.Name, err) - } - - if opts.RotateDBCredentials != "" { - if err := rotateCNPGDatabaseCredentials(ctx, clientset, nsConfig.Name, opts.GatewayID, opts.CNPG, opts.RotateDBCredentials); err != nil { - return fmt.Errorf("rotate database credentials in %s: %w", nsConfig.Name, err) - } - } - case "deployment": - if opts.DeploymentDBNamespace == "" { - return fmt.Errorf("deployment database namespace is required for gateway database reconciliation in namespace %s", nsConfig.Name) - } - if err := reconcileDeploymentDatabaseCredentials(ctx, clientset, opts.DeploymentDBNamespace, nsConfig.Name); err != nil { - return fmt.Errorf("copy deployment database credentials to %s: %w", nsConfig.Name, err) - } - default: - return fmt.Errorf("unsupported database provider %q for gateway in namespace %s", opts.DatabaseProvider, nsConfig.Name) + dbReconciler, err := newDatabaseReconciler(opts) + if err != nil { + return fmt.Errorf("database provider for gateway in namespace %s: %w", nsConfig.Name, err) + } + if err := dbReconciler.Reconcile(ctx, dynamicClient, clientset, nsConfig.Name, opts.GatewayID, opts.RotateDBCredentials); err != nil { + return err } if nsConfig.Gateway.CredentialDriver == nil { @@ -257,12 +231,17 @@ func DeleteGatewayResources( } } - if opts.HasCNPG && opts.GatewayID != "" { - if opts.CNPG.ClusterNamespace == "" { - log.Printf("WARN gateway %s: CNPG cluster namespace unknown; Database, DatabaseRole, and password Secret were not deleted and may require manual cleanup", opts.GatewayID) - } else { - deleteCNPGResources(ctx, dynamicClient, clientset, opts.GatewayID, opts.CNPG) + if dbReconciler, err := newDatabaseReconciler(opts); err == nil { + 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. + return fmt.Errorf("database cleanup for gateway %s: %w", opts.GatewayID, delErr) } + } else { + log.Printf("WARN gateway %s: cannot construct database reconciler for delete: %v", opts.GatewayID, err) } for _, credNS := range credentialNamespaces { @@ -1282,346 +1261,6 @@ func applyTrustedCAOverrides(obj *unstructured.Unstructured) { _ = unstructured.SetNestedSlice(obj.Object, containers, "spec", "template", "spec", "containers") } -func cnpgResourceName(gatewayID string) string { - return "gw-" + strings.ToLower(gatewayID) -} - -func cnpgPGName(gatewayID string) string { - return "gw_" + strings.ToLower(gatewayID) -} - -func reconcileCNPGDatabaseResources( - ctx context.Context, - dynamicClient dynamic.Interface, - clientset *kubernetes.Clientset, - tenantNamespace string, - gatewayID string, - cnpg CNPGConfig, -) error { - crName := cnpgResourceName(gatewayID) - pgName := cnpgPGName(gatewayID) - passwordSecretName := crName + "-credentials" - - log.Printf("INFO CNPG provisioning: gateway=%s cr=%s db=%s cluster=%s/%s tenant=%s", - gatewayID, crName, pgName, cnpg.ClusterNamespace, cnpg.ClusterName, tenantNamespace) - - _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) - if err != nil { - if !k8serrors.IsNotFound(err) { - return fmt.Errorf("get CNPG password secret: %w", err) - } - - passwordBytes := make([]byte, 32) - if _, err := rand.Read(passwordBytes); err != nil { - return fmt.Errorf("generate database password: %w", err) - } - password := hex.EncodeToString(passwordBytes) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: passwordSecretName, - Namespace: cnpg.ClusterNamespace, - Labels: map[string]string{ - "cnpg.io/reload": "true", - "hypershell.redhat.io/managed": "true", - "hypershell.redhat.io/gateway-namespace": tenantNamespace, - }, - }, - Type: corev1.SecretTypeBasicAuth, - StringData: map[string]string{ - "username": pgName, - "password": password, - }, - } - if _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { - return fmt.Errorf("create CNPG password secret: %w", err) - } - log.Printf("INFO created CNPG password secret %s in %s", passwordSecretName, cnpg.ClusterNamespace) - } else { - log.Printf("DEBUG CNPG password secret %s already exists in %s, skipping creation", passwordSecretName, cnpg.ClusterNamespace) - } - - log.Printf("INFO reconciling CNPG DatabaseRole %s in %s (cluster=%s)", crName, cnpg.ClusterNamespace, cnpg.ClusterName) - role := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "postgresql.cnpg.io/v1", - "kind": "DatabaseRole", - "metadata": map[string]interface{}{ - "name": crName, - "namespace": cnpg.ClusterNamespace, - "labels": map[string]interface{}{ - "hypershell.redhat.io/managed": "true", - "hypershell.redhat.io/gateway-namespace": tenantNamespace, - }, - }, - "spec": map[string]interface{}{ - "cluster": map[string]interface{}{ - "name": cnpg.ClusterName, - }, - "name": pgName, - "login": true, - "passwordSecret": map[string]interface{}{ - "name": passwordSecretName, - }, - "databaseRoleReclaimPolicy": "delete", - }, - }, - } - if err := reconcileResource(ctx, dynamicClient, role); err != nil { - return fmt.Errorf("reconcile CNPG DatabaseRole: %w", err) - } - - log.Printf("INFO reconciling CNPG Database %s in %s (owner=%s)", crName, cnpg.ClusterNamespace, pgName) - db := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "postgresql.cnpg.io/v1", - "kind": "Database", - "metadata": map[string]interface{}{ - "name": crName, - "namespace": cnpg.ClusterNamespace, - "labels": map[string]interface{}{ - "hypershell.redhat.io/managed": "true", - "hypershell.redhat.io/gateway-namespace": tenantNamespace, - }, - }, - "spec": map[string]interface{}{ - "cluster": map[string]interface{}{ - "name": cnpg.ClusterName, - }, - "name": pgName, - "owner": pgName, - "databaseReclaimPolicy": "delete", - }, - }, - } - if err := reconcileResource(ctx, dynamicClient, db); err != nil { - return fmt.Errorf("reconcile CNPG Database: %w", err) - } - - log.Printf("INFO waiting for CNPG Database %s/%s to become ready (timeout=2m)", cnpg.ClusterNamespace, crName) - if err := waitForCNPGDatabase(ctx, dynamicClient, cnpg.ClusterNamespace, crName, 2*time.Minute); err != nil { - return fmt.Errorf("wait for CNPG database: %w", err) - } - - gwSecretName := "openshell-gateway-db-credentials" - _, err = clientset.CoreV1().Secrets(tenantNamespace).Get(ctx, gwSecretName, metav1.GetOptions{}) - if err != nil { - if !k8serrors.IsNotFound(err) { - return fmt.Errorf("get gateway credentials secret: %w", err) - } - - cnpgSecret, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("read CNPG password secret: %w", err) - } - passwordBytes, ok := cnpgSecret.Data["password"] - if !ok || len(passwordBytes) == 0 { - return fmt.Errorf("CNPG password secret %s/%s has no password key", cnpg.ClusterNamespace, passwordSecretName) - } - password := string(passwordBytes) - - host := fmt.Sprintf("%s-rw.%s.svc.cluster.local", cnpg.ClusterName, cnpg.ClusterNamespace) - dbURI := fmt.Sprintf("postgresql://%s:%s@%s:5432/%s?sslmode=require", - pgName, url.QueryEscape(password), host, pgName) - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: gwSecretName, - Namespace: tenantNamespace, - Labels: map[string]string{ - "app.kubernetes.io/name": "openshell", - "app.kubernetes.io/component": "database", - "app.kubernetes.io/managed-by": "hypershell-control-plane", - "hypershell.redhat.io/managed": "true", - }, - }, - Type: corev1.SecretTypeOpaque, - StringData: map[string]string{ - "host": host, - "port": "5432", - "dbname": pgName, - "user": pgName, - "password": password, - "uri": dbURI, - }, - } - if _, err := clientset.CoreV1().Secrets(tenantNamespace).Create(ctx, secret, metav1.CreateOptions{}); err != nil { - return fmt.Errorf("create gateway credentials secret: %w", err) - } - log.Printf("INFO created gateway credentials secret %s in %s (host=%s db=%s)", gwSecretName, tenantNamespace, host, pgName) - } else { - log.Printf("DEBUG gateway credentials secret %s already exists in %s, skipping creation", gwSecretName, tenantNamespace) - } - - log.Printf("INFO CNPG database provisioning complete for gateway %s in %s", gatewayID, tenantNamespace) - return nil -} - -func copyDeploymentDatabaseCredentials( - ctx context.Context, - clientset kubernetes.Interface, - sourceNamespace string, - tenantNamespace string, -) error { - const ( - sourceSecretName = "openshell-db-credentials" - gwSecretName = "openshell-gateway-db-credentials" - ) - - sourceSecret, err := clientset.CoreV1().Secrets(sourceNamespace).Get(ctx, sourceSecretName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("read source database credentials from %s/%s: %w", sourceNamespace, sourceSecretName, err) - } - - required := map[string]string{} - for _, key := range []string{"dbname", "user", "password"} { - value := string(sourceSecret.Data[key]) - if value == "" { - return fmt.Errorf("source database credentials %s/%s is missing required key %q", sourceNamespace, sourceSecretName, key) - } - required[key] = value - } - - host := fmt.Sprintf("openshell-gateway-db.%s.svc.cluster.local", sourceNamespace) - port := "5432" - dbURI := fmt.Sprintf("postgresql://%s:%s@%s:%s/%s?sslmode=disable", - required["user"], url.QueryEscape(required["password"]), host, port, required["dbname"]) - desiredData := map[string][]byte{ - "host": []byte(host), - "port": []byte(port), - "dbname": []byte(required["dbname"]), - "user": []byte(required["user"]), - "password": []byte(required["password"]), - "uri": []byte(dbURI), - } - desiredLabels := map[string]string{ - "app.kubernetes.io/name": "openshell", - "app.kubernetes.io/component": "database", - "app.kubernetes.io/managed-by": "hypershell-control-plane", - "hypershell.redhat.io/managed": "true", - } - - secrets := clientset.CoreV1().Secrets(tenantNamespace) - existing, err := secrets.Get(ctx, gwSecretName, metav1.GetOptions{}) - if err != nil && !k8serrors.IsNotFound(err) { - return fmt.Errorf("get gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) - } - if k8serrors.IsNotFound(err) { - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: gwSecretName, Namespace: tenantNamespace, Labels: desiredLabels}, - Type: corev1.SecretTypeOpaque, - Data: desiredData, - } - if _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}); err != nil { - return fmt.Errorf("create gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) - } - log.Printf("INFO copied deployment database credentials to %s (host=%s db=%s)", tenantNamespace, host, required["dbname"]) - return nil - } - - updated := existing.DeepCopy() - if updated.Labels == nil { - updated.Labels = map[string]string{} - } - for key, value := range desiredLabels { - updated.Labels[key] = value - } - updated.Type = corev1.SecretTypeOpaque - updated.Data = desiredData - if reflect.DeepEqual(existing.Labels, updated.Labels) && existing.Type == updated.Type && reflect.DeepEqual(existing.Data, updated.Data) { - return nil - } - if _, err := secrets.Update(ctx, updated, metav1.UpdateOptions{}); err != nil { - return fmt.Errorf("update gateway credentials secret %s/%s: %w", tenantNamespace, gwSecretName, err) - } - log.Printf("INFO updated deployment database credentials in %s (host=%s db=%s)", tenantNamespace, host, required["dbname"]) - return nil -} - -func waitForCNPGDatabase(ctx context.Context, dynamicClient dynamic.Interface, namespace, name string, timeout time.Duration) error { - databaseGVR := schema.GroupVersionResource{ - Group: "postgresql.cnpg.io", - Version: "v1", - Resource: "databases", - } - - deadline := time.After(timeout) - ticker := time.NewTicker(2 * time.Second) - defer ticker.Stop() - - for { - select { - case <-ctx.Done(): - return ctx.Err() - case <-deadline: - return fmt.Errorf("timed out waiting for CNPG Database %s/%s to become ready", namespace, name) - case <-ticker.C: - obj, err := dynamicClient.Resource(databaseGVR).Namespace(namespace).Get(ctx, name, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) { - log.Printf("DEBUG CNPG Database %s/%s not found yet, waiting...", namespace, name) - } else { - log.Printf("WARN error checking CNPG Database %s/%s: %v", namespace, name, err) - } - continue - } - applied, _, _ := unstructured.NestedBool(obj.Object, "status", "applied") - if applied { - log.Printf("INFO CNPG Database %s/%s is ready (status.applied=true)", namespace, name) - return nil - } - log.Printf("DEBUG CNPG Database %s/%s exists but not ready (status.applied=%v)", namespace, name, applied) - } - } -} - -func deleteCNPGResources( - ctx context.Context, - dynamicClient dynamic.Interface, - clientset *kubernetes.Clientset, - gatewayID string, - cnpg CNPGConfig, -) { - crName := cnpgResourceName(gatewayID) - ns := cnpg.ClusterNamespace - log.Printf("INFO deleting CNPG resources for gateway %s: cr=%s namespace=%s", gatewayID, crName, ns) - - databaseGVR := schema.GroupVersionResource{ - Group: "postgresql.cnpg.io", - Version: "v1", - Resource: "databases", - } - if err := dynamicClient.Resource(databaseGVR).Namespace(ns).Delete(ctx, crName, metav1.DeleteOptions{}); err != nil { - if !k8serrors.IsNotFound(err) { - log.Printf("WARN failed to delete CNPG Database %s: %v", crName, err) - } - } else { - log.Printf("INFO deleted CNPG Database %s from %s", crName, ns) - } - - roleGVR := schema.GroupVersionResource{ - Group: "postgresql.cnpg.io", - Version: "v1", - Resource: "databaseroles", - } - if err := dynamicClient.Resource(roleGVR).Namespace(ns).Delete(ctx, crName, metav1.DeleteOptions{}); err != nil { - if !k8serrors.IsNotFound(err) { - log.Printf("WARN failed to delete CNPG DatabaseRole %s: %v", crName, err) - } - } else { - log.Printf("INFO deleted CNPG DatabaseRole %s from %s", crName, ns) - } - - passwordSecretName := crName + "-credentials" - if err := clientset.CoreV1().Secrets(ns).Delete(ctx, passwordSecretName, metav1.DeleteOptions{}); err != nil { - if !k8serrors.IsNotFound(err) { - log.Printf("WARN failed to delete CNPG password secret %s: %v", passwordSecretName, err) - } - } else { - log.Printf("INFO deleted CNPG password secret %s from %s", passwordSecretName, ns) - } -} - func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig *NamespaceConfig) error { kc := keycloak.NewClient( opts.Keycloak.ServerURL, @@ -1688,65 +1327,6 @@ func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig * return nil } -func rotateCNPGDatabaseCredentials( - ctx context.Context, - clientset *kubernetes.Clientset, - tenantNamespace string, - gatewayID string, - cnpg CNPGConfig, - rotateTimestamp string, -) error { - gwSecretName := "openshell-gateway-db-credentials" - existing, err := clientset.CoreV1().Secrets(tenantNamespace).Get(ctx, gwSecretName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("get gateway credentials secret for rotation: %w", err) - } - - lastRotation := existing.Annotations["hypershell.redhat.io/last-db-rotation"] - if lastRotation == rotateTimestamp { - log.Printf("DEBUG database credentials in %s already rotated at %s, skipping", tenantNamespace, rotateTimestamp) - return nil - } - - passwordBytes := make([]byte, 32) - if _, err := rand.Read(passwordBytes); err != nil { - return fmt.Errorf("generate new database password: %w", err) - } - newPassword := hex.EncodeToString(passwordBytes) - - crName := cnpgResourceName(gatewayID) - pgName := cnpgPGName(gatewayID) - passwordSecretName := crName + "-credentials" - - cnpgSecret, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Get(ctx, passwordSecretName, metav1.GetOptions{}) - if err != nil { - return fmt.Errorf("get CNPG password secret for rotation: %w", err) - } - cnpgSecret.Data["password"] = []byte(newPassword) - if _, err := clientset.CoreV1().Secrets(cnpg.ClusterNamespace).Update(ctx, cnpgSecret, metav1.UpdateOptions{}); err != nil { - return fmt.Errorf("update CNPG password secret: %w", err) - } - log.Printf("INFO updated CNPG password secret %s in %s", passwordSecretName, cnpg.ClusterNamespace) - - host := fmt.Sprintf("%s-rw.%s.svc.cluster.local", cnpg.ClusterName, cnpg.ClusterNamespace) - newURI := fmt.Sprintf("postgresql://%s:%s@%s:5432/%s?sslmode=require", - pgName, url.QueryEscape(newPassword), host, pgName) - - existing.Data["password"] = []byte(newPassword) - existing.Data["uri"] = []byte(newURI) - if existing.Annotations == nil { - existing.Annotations = make(map[string]string) - } - existing.Annotations["hypershell.redhat.io/last-db-rotation"] = rotateTimestamp - - if _, err := clientset.CoreV1().Secrets(tenantNamespace).Update(ctx, existing, metav1.UpdateOptions{}); err != nil { - return fmt.Errorf("update gateway credentials secret after rotation: %w", err) - } - - log.Printf("INFO rotated database credentials in %s (timestamp=%s)", tenantNamespace, rotateTimestamp) - return nil -} - // reconcileCredentialKEK uses create-or-skip (not update-or-create) because // replacing an existing key would render all previously encrypted credentials // unrecoverable. diff --git a/components/control-plane/internal/reconciler/managed_database_lifecycle_test.go b/components/control-plane/internal/reconciler/managed_database_lifecycle_test.go index fce72e3b..3717567a 100644 --- a/components/control-plane/internal/reconciler/managed_database_lifecycle_test.go +++ b/components/control-plane/internal/reconciler/managed_database_lifecycle_test.go @@ -76,7 +76,7 @@ func TestManagedDatabaseDeleteNilTombstoneUsesLastSeenAndRetainsOnFailure(t *tes } func TestManagedDatabaseReconcilerNilClientsReturnsError(t *testing.T) { - r := NewManagedDatabaseReconciler(nil, nil, nil, "") + r := NewManagedDatabaseReconciler(nil, nil, nil, "hypershell") err := r.Handle(context.Background(), watcher.Event[*pb.ManagedDatabase]{Type: watcher.EventDeleted, ResourceID: "db-1", Resource: &pb.ManagedDatabase{Namespace: "must-not-guess", Provider: "deployment"}}) if err == nil { t.Fatal("want nil client error") diff --git a/components/control-plane/internal/reconciler/managed_database_test.go b/components/control-plane/internal/reconciler/managed_database_test.go index b96eb5ef..f2d8836c 100644 --- a/components/control-plane/internal/reconciler/managed_database_test.go +++ b/components/control-plane/internal/reconciler/managed_database_test.go @@ -21,7 +21,7 @@ import ( ) func TestNewManagedDatabaseReconcilerWithoutKubernetesClient(t *testing.T) { - r := NewManagedDatabaseReconciler(nil, nil, nil, "") + r := NewManagedDatabaseReconciler(nil, nil, nil, "hypershell") if r.hasCNPG { t.Fatal("hasCNPG = true without a Kubernetes client, want false") } diff --git a/components/control-plane/internal/reconciler/namespace.go b/components/control-plane/internal/reconciler/namespace.go index 37557575..2d0dd73e 100644 --- a/components/control-plane/internal/reconciler/namespace.go +++ b/components/control-plane/internal/reconciler/namespace.go @@ -180,12 +180,6 @@ func (r *NamespaceGCReconciler) grpcLiveNamespaces(ctx context.Context) (map[str listCtx, cancel := context.WithTimeout(ctx, gatewayListTimeout) defer cancel() client := pb.NewGatewayServiceClient(r.grpcConn) - // Deliberately unfiltered by cluster_id: the live set only ever PROTECTS - // namespaces from reaping, and gateway namespaces are globally unique, so a - // superset that includes other clusters' gateways can never cause a wrong - // delete -- whereas filtering to this cluster could reap a co-located hub's - // namespace during the same-cluster test. Reaping decisions are further gated - // by a local label selector, so foreign entries in the live set are inert. gateways, err := listAllGateways(listCtx, client, "") if err != nil { return nil, fmt.Errorf("list gateways: %w", err) diff --git a/components/control-plane/internal/reconciler/reconciler.go b/components/control-plane/internal/reconciler/reconciler.go index 8ea6b29c..98eebce8 100644 --- a/components/control-plane/internal/reconciler/reconciler.go +++ b/components/control-plane/internal/reconciler/reconciler.go @@ -182,6 +182,8 @@ func (r *ManagedDatabaseReconciler) handleOne(ctx context.Context, event watcher err = r.handleCNPGDatabase(ctx, event, db) case "deployment": err = r.handleDeploymentDatabase(ctx, event, db) + case "external": + err = r.handleExternalDatabase(ctx, event, db) default: log.Printf("WARN ManagedDatabase %s has unsupported provider %q, skipping", event.ResourceID, db.Provider) return nil @@ -305,6 +307,34 @@ func (r *ManagedDatabaseReconciler) handleDeploymentDatabase(ctx context.Context return nil } +func (r *ManagedDatabaseReconciler) handleExternalDatabase(ctx context.Context, event watcher.Event[*pb.ManagedDatabase], db *pb.ManagedDatabase) error { + if event.Type == watcher.EventDeleted { + // External databases are not provisioned by HyperShell; only the per-gateway + // DDL objects (roles and databases) are cleaned up by the gateway reconciler + // when each gateway is deleted. The ManagedDatabase itself is register-only. + log.Printf("INFO ManagedDatabase %s (external) deleted, no control-plane resources to clean up", event.ResourceID) + return nil + } + + log.Printf("INFO reconciling ManagedDatabase %s name=%s provider=external (event=%d)", + event.ResourceID, db.Name, event.Type) + + if db.GetConnectionSecret() == "" { + newStatus := gateway.ExternalDBStatusSecretInvalid + r.updateManagedDatabaseStatusIfChanged(ctx, event.ResourceID, managedDatabaseStatus(db), newStatus) + log.Printf("WARN ManagedDatabase %s has no connection_secret; cannot probe external server", event.ResourceID) + return nil + } + + cfg := gateway.ExternalDBConfig{ + CredentialsNamespace: db.GetConnectionSecret(), + ManagedDatabaseID: event.ResourceID, + } + newStatus := gateway.ProbeExternalServer(ctx, r.clientset, cfg) + r.updateManagedDatabaseStatusIfChanged(ctx, event.ResourceID, managedDatabaseStatus(db), newStatus) + return nil +} + func (r *ManagedDatabaseReconciler) reconcileCNPGCluster(ctx context.Context, db *pb.ManagedDatabase) error { namespace := db.Namespace @@ -1306,6 +1336,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. CNPG: deleteDBConfig.CNPG, DatabaseProvider: deleteDBConfig.Provider, DeploymentDBNamespace: deleteDBConfig.SourceNamespace, + ExternalDB: deleteDBConfig.ExternalDB, ControlPlaneNamespace: r.controlPlaneNamespace, KeycloakClient: r.keycloakClient, GatewayID: event.ResourceID, @@ -1501,6 +1532,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. DatabaseProvider: dbConfig.Provider, CNPG: dbConfig.CNPG, DeploymentDBNamespace: dbConfig.SourceNamespace, + ExternalDB: dbConfig.ExternalDB, ControlPlaneNamespace: r.controlPlaneNamespace, GatewayID: event.ResourceID, UpdateRouteAddress: r.makeRouteAddressUpdater(event.ResourceID), @@ -1897,13 +1929,6 @@ const gatewayListPageSize = 500 // gateway. The list endpoint is server-side paginated (default page size 20), // so callers that must reason about the whole fleet (the namespace reaper and // the health reconciler) cannot rely on a single unpaged request. -// -// clusterID, when non-empty, scopes the listing server-side to gateways with -// that cluster_id. The health reconciler passes its managed-cluster identity so -// it never stamps a foreign cluster's gateway; the namespace reaper passes "" -// on purpose (an unfiltered live set is a safe superset that only ever protects -// namespaces, and gateway namespaces are globally unique, so it never reaps -// another cluster's namespace). func listAllGateways(ctx context.Context, client pb.GatewayServiceClient, clusterID string) ([]*pb.Gateway, error) { var all []*pb.Gateway for page := int32(1); ; page++ { @@ -2093,6 +2118,7 @@ type databaseConfig struct { Provider string CNPG gateway.CNPGConfig SourceNamespace string + ExternalDB gateway.ExternalDBConfig } // selectGatewayImage applies database-backed gateway version selection: a @@ -2167,6 +2193,14 @@ func (r *GatewayReconciler) resolveDatabaseConfig(ctx context.Context, gw *pb.Ga Provider: "deployment", SourceNamespace: db.Namespace, }, nil + case "external": + return databaseConfig{ + Provider: "external", + ExternalDB: gateway.ExternalDBConfig{ + CredentialsNamespace: db.GetConnectionSecret(), + ManagedDatabaseID: gw.DatabaseId, + }, + }, nil default: return databaseConfig{}, fmt.Errorf("ManagedDatabase %s has unsupported provider %q", gw.DatabaseId, db.Provider) } diff --git a/components/sdk-go/client/client.go b/components/sdk-go/client/client.go index 847fa5d5..3a3d2849 100644 --- a/components/sdk-go/client/client.go +++ b/components/sdk-go/client/client.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/gateway_api.go b/components/sdk-go/client/gateway_api.go index 00bb5525..b8c30828 100644 --- a/components/sdk-go/client/gateway_api.go +++ b/components/sdk-go/client/gateway_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/gateway_network_api.go b/components/sdk-go/client/gateway_network_api.go index 75d2411a..058eac09 100644 --- a/components/sdk-go/client/gateway_network_api.go +++ b/components/sdk-go/client/gateway_network_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/gateway_release_api.go b/components/sdk-go/client/gateway_release_api.go index 72c64bf8..a2d16d74 100644 --- a/components/sdk-go/client/gateway_release_api.go +++ b/components/sdk-go/client/gateway_release_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/iterator.go b/components/sdk-go/client/iterator.go index a2792ca0..eb7fdac7 100644 --- a/components/sdk-go/client/iterator.go +++ b/components/sdk-go/client/iterator.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/managed_cluster_api.go b/components/sdk-go/client/managed_cluster_api.go index 646d6726..f8a3d6b4 100644 --- a/components/sdk-go/client/managed_cluster_api.go +++ b/components/sdk-go/client/managed_cluster_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/managed_database_api.go b/components/sdk-go/client/managed_database_api.go index 6c86ed2c..b3867701 100644 --- a/components/sdk-go/client/managed_database_api.go +++ b/components/sdk-go/client/managed_database_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/open_shell_gateway_service_account_api.go b/components/sdk-go/client/open_shell_gateway_service_account_api.go index 0cc42149..acc419f4 100644 --- a/components/sdk-go/client/open_shell_gateway_service_account_api.go +++ b/components/sdk-go/client/open_shell_gateway_service_account_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/role_api.go b/components/sdk-go/client/role_api.go index 8cc27d75..c8b05d12 100644 --- a/components/sdk-go/client/role_api.go +++ b/components/sdk-go/client/role_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/client/role_binding_api.go b/components/sdk-go/client/role_binding_api.go index 76a4cd1f..ff98444d 100644 --- a/components/sdk-go/client/role_binding_api.go +++ b/components/sdk-go/client/role_binding_api.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package client diff --git a/components/sdk-go/types/base.go b/components/sdk-go/types/base.go index 33610fdb..c5522aa3 100644 --- a/components/sdk-go/types/base.go +++ b/components/sdk-go/types/base.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/gateway.go b/components/sdk-go/types/gateway.go index 1541d672..4305e007 100644 --- a/components/sdk-go/types/gateway.go +++ b/components/sdk-go/types/gateway.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/gateway_network.go b/components/sdk-go/types/gateway_network.go index c57107ce..fd613fc2 100644 --- a/components/sdk-go/types/gateway_network.go +++ b/components/sdk-go/types/gateway_network.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/gateway_release.go b/components/sdk-go/types/gateway_release.go index 9303e0de..d0ba5590 100644 --- a/components/sdk-go/types/gateway_release.go +++ b/components/sdk-go/types/gateway_release.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/list_options.go b/components/sdk-go/types/list_options.go index 3e76f6cb..101fd81b 100644 --- a/components/sdk-go/types/list_options.go +++ b/components/sdk-go/types/list_options.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/managed_cluster.go b/components/sdk-go/types/managed_cluster.go index d85e3d70..3a3daff4 100644 --- a/components/sdk-go/types/managed_cluster.go +++ b/components/sdk-go/types/managed_cluster.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/managed_database.go b/components/sdk-go/types/managed_database.go index 8407ac9e..a7c2f360 100644 --- a/components/sdk-go/types/managed_database.go +++ b/components/sdk-go/types/managed_database.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/open_shell_gateway_service_account.go b/components/sdk-go/types/open_shell_gateway_service_account.go index 5364c0d2..f6ab3ada 100644 --- a/components/sdk-go/types/open_shell_gateway_service_account.go +++ b/components/sdk-go/types/open_shell_gateway_service_account.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/role.go b/components/sdk-go/types/role.go index 10bca77a..b7f13835 100644 --- a/components/sdk-go/types/role.go +++ b/components/sdk-go/types/role.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-go/types/role_binding.go b/components/sdk-go/types/role_binding.go index 5a863d2d..bdcd8a35 100644 --- a/components/sdk-go/types/role_binding.go +++ b/components/sdk-go/types/role_binding.go @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e package types diff --git a/components/sdk-typescript/src/base.ts b/components/sdk-typescript/src/base.ts index 3049004e..ca6fc5e9 100644 --- a/components/sdk-typescript/src/base.ts +++ b/components/sdk-typescript/src/base.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e export type ObjectReference = { id: string; diff --git a/components/sdk-typescript/src/client.ts b/components/sdk-typescript/src/client.ts index 3093afc8..b88179bf 100644 --- a/components/sdk-typescript/src/client.ts +++ b/components/sdk-typescript/src/client.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig } from './base.js'; import { GatewayAPI } from './gateway_api.js'; diff --git a/components/sdk-typescript/src/gateway.ts b/components/sdk-typescript/src/gateway.ts index de4e3184..c77b0b2f 100644 --- a/components/sdk-typescript/src/gateway.ts +++ b/components/sdk-typescript/src/gateway.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/gateway_api.ts b/components/sdk-typescript/src/gateway_api.ts index 4e71256d..726cecb8 100644 --- a/components/sdk-typescript/src/gateway_api.ts +++ b/components/sdk-typescript/src/gateway_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/gateway_network.ts b/components/sdk-typescript/src/gateway_network.ts index 97fe414a..02448a25 100644 --- a/components/sdk-typescript/src/gateway_network.ts +++ b/components/sdk-typescript/src/gateway_network.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/gateway_network_api.ts b/components/sdk-typescript/src/gateway_network_api.ts index d08e74a2..a142abdb 100644 --- a/components/sdk-typescript/src/gateway_network_api.ts +++ b/components/sdk-typescript/src/gateway_network_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/gateway_release.ts b/components/sdk-typescript/src/gateway_release.ts index c915a33f..48142a1d 100644 --- a/components/sdk-typescript/src/gateway_release.ts +++ b/components/sdk-typescript/src/gateway_release.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/gateway_release_api.ts b/components/sdk-typescript/src/gateway_release_api.ts index f990e48a..1b904380 100644 --- a/components/sdk-typescript/src/gateway_release_api.ts +++ b/components/sdk-typescript/src/gateway_release_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/index.ts b/components/sdk-typescript/src/index.ts index ee5fcee8..c943373d 100644 --- a/components/sdk-typescript/src/index.ts +++ b/components/sdk-typescript/src/index.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e export { SDKClient } from './client.js'; export type { SDKClientConfig, ListOptions, RequestOptions, ObjectReference, ListMeta, APIError } from './base.js'; diff --git a/components/sdk-typescript/src/managed_cluster.ts b/components/sdk-typescript/src/managed_cluster.ts index a648937d..10d0fcd4 100644 --- a/components/sdk-typescript/src/managed_cluster.ts +++ b/components/sdk-typescript/src/managed_cluster.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/managed_cluster_api.ts b/components/sdk-typescript/src/managed_cluster_api.ts index f83d1b15..271718ce 100644 --- a/components/sdk-typescript/src/managed_cluster_api.ts +++ b/components/sdk-typescript/src/managed_cluster_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/managed_database.ts b/components/sdk-typescript/src/managed_database.ts index a156b437..2c0d96c5 100644 --- a/components/sdk-typescript/src/managed_database.ts +++ b/components/sdk-typescript/src/managed_database.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/managed_database_api.ts b/components/sdk-typescript/src/managed_database_api.ts index e3b31217..8d88e627 100644 --- a/components/sdk-typescript/src/managed_database_api.ts +++ b/components/sdk-typescript/src/managed_database_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/open_shell_gateway_service_account.ts b/components/sdk-typescript/src/open_shell_gateway_service_account.ts index fdb26eb3..7e95ef1c 100644 --- a/components/sdk-typescript/src/open_shell_gateway_service_account.ts +++ b/components/sdk-typescript/src/open_shell_gateway_service_account.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e export type OpenShellGatewayServiceAccountCapabilities = { diff --git a/components/sdk-typescript/src/open_shell_gateway_service_account_api.ts b/components/sdk-typescript/src/open_shell_gateway_service_account_api.ts index 3e2d5f93..736993ac 100644 --- a/components/sdk-typescript/src/open_shell_gateway_service_account_api.ts +++ b/components/sdk-typescript/src/open_shell_gateway_service_account_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, RequestOptions } from './base.js'; import { sdkFetch } from './base.js'; diff --git a/components/sdk-typescript/src/role.ts b/components/sdk-typescript/src/role.ts index f433f5cf..8b1941fe 100644 --- a/components/sdk-typescript/src/role.ts +++ b/components/sdk-typescript/src/role.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/role_api.ts b/components/sdk-typescript/src/role_api.ts index 1ba0e062..f00af8b8 100644 --- a/components/sdk-typescript/src/role_api.ts +++ b/components/sdk-typescript/src/role_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/components/sdk-typescript/src/role_binding.ts b/components/sdk-typescript/src/role_binding.ts index d995b20d..526df667 100644 --- a/components/sdk-typescript/src/role_binding.ts +++ b/components/sdk-typescript/src/role_binding.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { ObjectReference, ListMeta } from './base.js'; diff --git a/components/sdk-typescript/src/role_binding_api.ts b/components/sdk-typescript/src/role_binding_api.ts index 9678dcbc..efd22670 100644 --- a/components/sdk-typescript/src/role_binding_api.ts +++ b/components/sdk-typescript/src/role_binding_api.ts @@ -1,6 +1,6 @@ // Code generated by trex-sdk-generator from openapi.yaml - DO NOT EDIT. // Source: components/api-server/openapi/openapi.yaml -// Spec SHA256: 3a4b85a8fe6c5a35061a7a9a470d9813d217c510e482871cd237ebf7bd70f446 +// Spec SHA256: 837f62b3bb22826b01c4a731a4da60da819b72f1a9d22b28803d7bc16363051e import type { SDKClientConfig, ListOptions, RequestOptions } from './base.js'; import { sdkFetch, buildQueryString } from './base.js'; diff --git a/deploy/base/api-server.yaml b/deploy/base/api-server.yaml index 4676855f..11f4cd3c 100644 --- a/deploy/base/api-server.yaml +++ b/deploy/base/api-server.yaml @@ -141,6 +141,8 @@ kind: Service metadata: name: hypershell-api-server namespace: hypershell-system + labels: + app: hypershell-api-server spec: selector: app: hypershell-api-server diff --git a/deploy/components/database-external/hypershell-db-secret.yaml b/deploy/components/database-external/hypershell-db-secret.yaml new file mode 100644 index 00000000..70bdbee0 --- /dev/null +++ b/deploy/components/database-external/hypershell-db-secret.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +kind: Secret +metadata: + name: hypershell-db-app + namespace: hypershell-system + labels: + app.kubernetes.io/name: hypershell + app.kubernetes.io/component: database +type: Opaque +stringData: + host: postgres.external-cloud-db.svc.cluster.local + port: "5432" + dbname: hypershell + user: postgres + password: hypershell-kind-admin-password + sslmode: disable diff --git a/deploy/components/database-external/kustomization.yaml b/deploy/components/database-external/kustomization.yaml new file mode 100644 index 00000000..7802fe7c --- /dev/null +++ b/deploy/components/database-external/kustomization.yaml @@ -0,0 +1,45 @@ +apiVersion: kustomize.config.k8s.io/v1alpha1 +kind: Component + +resources: + - hypershell-db-secret.yaml + +patches: + # Remove the CNPG Cluster - the external PostgreSQL server replaces it. + - target: + group: postgresql.cnpg.io + version: v1 + kind: Cluster + name: hypershell-db + patch: | + $patch: delete + apiVersion: postgresql.cnpg.io/v1 + kind: Cluster + metadata: + name: hypershell-db + # The external stand-in server does not terminate TLS; switch sslmode from + # require to disable in both the migrate init container and the api-server + # container. DATABASE_PROVIDER=external is NOT patched here - it is applied + # via kubectl set env after the initial deployment readiness wait in up.sh, + # so the baseline image can start cleanly before the PR image is swapped in. + - target: + kind: Deployment + name: hypershell-api-server + patch: | + apiVersion: apps/v1 + kind: Deployment + metadata: + name: not-used + spec: + template: + spec: + initContainers: + - name: migrate + env: + - name: DB_SSLMODE + value: disable + containers: + - name: api-server + env: + - name: DB_SSLMODE + value: disable diff --git a/deploy/kind/database-external-env-patch.yaml b/deploy/kind/database-external-env-patch.yaml new file mode 100644 index 00000000..1af722f2 --- /dev/null +++ b/deploy/kind/database-external-env-patch.yaml @@ -0,0 +1,11 @@ +# JSON6902 patch adding DATABASE_PROVIDER=external to a Deployment container env. +# Referenced twice (once per target Deployment: hypershell-api-server and +# hypershell-controller) from the Kind up.sh-generated overlay when +# DATABASE_PROVIDER=external is selected. The external provider registers a +# pre-existing PostgreSQL server and issues per-gateway DDL in-process; no +# CNPG operator or per-gateway Deployment database is required. +- op: add + path: /spec/template/spec/containers/0/env/- + value: + name: DATABASE_PROVIDER + value: external diff --git a/packages/gateway-management-ui/src/gateways/editable-command.tsx b/packages/gateway-management-ui/src/gateways/editable-command.tsx index bf1c96a7..19016698 100644 --- a/packages/gateway-management-ui/src/gateways/editable-command.tsx +++ b/packages/gateway-management-ui/src/gateways/editable-command.tsx @@ -136,6 +136,16 @@ function SelectField({ ); } +/** + * A copyable command block whose marked value slots are editable in place. + * + * `templateCommand` carries the edit markers and is highlighted once; `copyText` + * is the same command with the operator's current values resolved and drives + * both the copy button and (identically) a whole-block text selection. Editing a + * field calls `onFieldChange(marker, value)`; a marker used twice in the command + * (the mirrored provider name) is kept in lockstep because both slots read the + * same entry in `values`. + */ export function EditableCommand({ copyAriaLabel, copyText, diff --git a/packages/gateway-management-ui/src/gateways/gateway-connection-steps.test.tsx b/packages/gateway-management-ui/src/gateways/gateway-connection-steps.test.tsx index 66555d2b..2bffbd96 100644 --- a/packages/gateway-management-ui/src/gateways/gateway-connection-steps.test.tsx +++ b/packages/gateway-management-ui/src/gateways/gateway-connection-steps.test.tsx @@ -66,7 +66,7 @@ describe("GatewayConnectionSteps", () => { expect(link.textContent).toContain("Install the OpenShell CLI"); }); - it("highlights all command blocks with Shiki once they resolve", async () => { + it("highlights both command blocks with Shiki once they resolve", async () => { const { container } = renderSteps(readyGateway); await waitFor(() => { diff --git a/scripts/kind/build-images.sh b/scripts/kind/build-images.sh index 37e75416..1d6e9d06 100755 --- a/scripts/kind/build-images.sh +++ b/scripts/kind/build-images.sh @@ -8,47 +8,55 @@ source "${SCRIPT_DIR}/lib.sh" REPO_ROOT="$(git rev-parse --show-toplevel)" BUILD_SOURCE="${BUILD_SOURCE:-worktree}" -WORKTREE_DIR="" -cleanup_worktree() { - if [[ -n "${WORKTREE_DIR}" ]] && [[ -d "${WORKTREE_DIR}" ]]; then - git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" - fi -} +if [[ "${KIND_SKIP_BUILD:-}" == "true" ]]; then + header "Local Images (skip build)" + info "KIND_SKIP_BUILD=true - skipping build, loading existing images" + info " ${api_server_local}" + info " ${control_plane_local}" + info " ${web_console_local}" +else + WORKTREE_DIR="" + cleanup_worktree() { + if [[ -n "${WORKTREE_DIR}" ]] && [[ -d "${WORKTREE_DIR}" ]]; then + git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" + fi + } -if [[ "${BUILD_SOURCE}" == "baseline" ]]; then - header "Building Baseline Images from origin/main" - trap cleanup_worktree EXIT + if [[ "${BUILD_SOURCE}" == "baseline" ]]; then + header "Building Baseline Images from origin/main" + trap cleanup_worktree EXIT - info "Fetching origin/main..." - git fetch origin main --quiet + info "Fetching origin/main..." + git fetch origin main --quiet - WORKTREE_DIR=$(mktemp -d /tmp/hypershell-baseline-XXXXXX) - rm -rf "${WORKTREE_DIR}" - git worktree add --detach "${WORKTREE_DIR}" origin/main --quiet - BUILD_DIR="${WORKTREE_DIR}" - info "Building from origin/main ($(git -C "${WORKTREE_DIR}" rev-parse --short HEAD))" -else - header "Building Images from Working Tree" - BUILD_DIR="${REPO_ROOT}" - info "Building from working tree ($(git rev-parse --short HEAD))" -fi + WORKTREE_DIR=$(mktemp -d /tmp/hypershell-baseline-XXXXXX) + rm -rf "${WORKTREE_DIR}" + git worktree add --detach "${WORKTREE_DIR}" origin/main --quiet + BUILD_DIR="${WORKTREE_DIR}" + info "Building from origin/main ($(git -C "${WORKTREE_DIR}" rev-parse --short HEAD))" + else + header "Building Images from Working Tree" + BUILD_DIR="${REPO_ROOT}" + info "Building from working tree ($(git rev-parse --short HEAD))" + fi -info "Building API server..." -${CONTAINER_ENGINE} build -t "${api_server_local}" \ - -f "${BUILD_DIR}/components/api-server/Dockerfile" \ - --build-arg GIT_VERSION="${build_version}" \ - --build-arg BUILD_TIME="${build_time}" \ - "${BUILD_DIR}/components/api-server" + info "Building API server..." + ${CONTAINER_ENGINE} build -t "${api_server_local}" \ + -f "${BUILD_DIR}/components/api-server/Dockerfile" \ + --build-arg GIT_VERSION="${build_version}" \ + --build-arg BUILD_TIME="${build_time}" \ + "${BUILD_DIR}/components/api-server" -info "Building control plane..." -${CONTAINER_ENGINE} build -t "${control_plane_local}" \ - -f "${BUILD_DIR}/components/control-plane/Dockerfile" "${BUILD_DIR}" + info "Building control plane..." + ${CONTAINER_ENGINE} build -t "${control_plane_local}" \ + -f "${BUILD_DIR}/components/control-plane/Dockerfile" "${BUILD_DIR}" -info "Building web console..." -${CONTAINER_ENGINE} build -t "${web_console_local}" \ - -f "${BUILD_DIR}/components/web-console/Dockerfile" "${BUILD_DIR}" + info "Building web console..." + ${CONTAINER_ENGINE} build -t "${web_console_local}" \ + -f "${BUILD_DIR}/components/web-console/Dockerfile" "${BUILD_DIR}" -success "All images built" + success "All images built" +fi if cluster_exists; then info "Loading images into Kind cluster..." diff --git a/scripts/kind/seed.sh b/scripts/kind/seed.sh index dd0dbd6c..07477a5b 100755 --- a/scripts/kind/seed.sh +++ b/scripts/kind/seed.sh @@ -12,9 +12,9 @@ # SKIP_SEED=true on kind-up and runs `make kind-seed` after the swap. # # Environment: -# DATABASE_PROVIDER cnpg | deployment (default: deployment). Must match the -# provider kind-up provisioned infrastructure for. -# SEED_STRICT when "true", a seeding failure exits non-zero instead of +# DATABASE_PROVIDER cnpg | deployment | external (default: deployment). Must +# match the provider kind-up provisioned infrastructure for. +# KIND_SEED_STRICT when "true", a seeding failure exits non-zero instead of # only warning. CI sets this so a contract regression fails # the job at the seed step with the real HTTP error, rather # than surfacing later as a confusing discovery failure. @@ -29,8 +29,8 @@ require_cluster # DATABASE_PROVIDER unset/empty means "deployment" (mirrors up.sh). DB_PROVIDER="${DATABASE_PROVIDER:-deployment}" -if [[ "${DB_PROVIDER}" != "cnpg" && "${DB_PROVIDER}" != "deployment" ]]; then - error "DATABASE_PROVIDER must be 'cnpg' or 'deployment', got '${DB_PROVIDER}'" +if [[ "${DB_PROVIDER}" != "cnpg" && "${DB_PROVIDER}" != "deployment" && "${DB_PROVIDER}" != "external" ]]; then + error "DATABASE_PROVIDER must be 'cnpg', 'deployment', or 'external', got '${DB_PROVIDER}'" exit 1 fi @@ -166,8 +166,12 @@ if [[ -z "${seed_failed}" ]]; then if [[ -z "${CLUSTER_ID}" ]]; then info "Creating ManagedCluster..." - MC_RAW=$(api_post "${API_URL}/api/hypershell/v1/managed_clusters" \ - "{\"name\":\"local-kind\",\"provider\":\"kind\",\"kubeconfig_secret\":\"kind-kubeconfig\"}") + _mc_body="{\"name\":\"local-kind\",\"provider\":\"kind\",\"kubeconfig_secret\":\"kind-kubeconfig\"" + if [[ "${DB_PROVIDER}" == "external" ]]; then + _mc_body="${_mc_body},\"region\":\"kind-local\"" + fi + _mc_body="${_mc_body}}" + MC_RAW=$(api_post "${API_URL}/api/hypershell/v1/managed_clusters" "${_mc_body}") MC_HTTP=$(echo "${MC_RAW}" | tail -1) MC_RESP=$(echo "${MC_RAW}" | sed '$d') CLUSTER_ID=$(extract_id "${MC_RESP}") @@ -232,8 +236,16 @@ if [[ -z "${seed_failed}" ]]; then if [[ -z "${DATABASE_ID}" ]]; then info "Creating ManagedDatabase (provider=${DB_PROVIDER})..." - MD_RAW=$(api_post "${API_URL}/api/hypershell/v1/managed_databases" \ - "{\"name\":\"openshell-db\",\"provider\":\"${DB_PROVIDER}\"}") + if [[ "${DB_PROVIDER}" == "external" ]]; then + # connection_secret names the NAMESPACE holding the admin credentials + # (created by up.sh), not a Secret name. The Secret inside it is always + # hypershell-managed-db-credentials. + MD_RAW=$(api_post "${API_URL}/api/hypershell/v1/managed_databases" \ + "{\"name\":\"openshell-db\",\"provider\":\"external\",\"connection_secret\":\"hypershell-managed-db-kind\",\"region\":\"kind-local\"}") + else + MD_RAW=$(api_post "${API_URL}/api/hypershell/v1/managed_databases" \ + "{\"name\":\"openshell-db\",\"provider\":\"${DB_PROVIDER}\"}") + fi MD_HTTP=$(echo "${MD_RAW}" | tail -1) MD_RESP=$(echo "${MD_RAW}" | sed '$d') @@ -274,9 +286,16 @@ if [[ -z "${seed_failed}" ]]; then OIDC_JSON="{\\\"issuer\\\":\\\"${KEYCLOAK_OIDC_ISSUER}\\\",\\\"audience\\\":\\\"${KEYCLOAK_OIDC_AUDIENCE}\\\",\\\"roles_claim\\\":\\\"groups\\\",\\\"admin_role\\\":\\\"hypershell-admins\\\",\\\"user_role\\\":\\\"hypershell-users\\\"}" # namespace is server-derived (BeforeCreate sets openshell- from the ksuid); # sending it is rejected as an unknown field (ErrorMalformedRequest / id 17). - # Always send database_id; deployment mode uses the empty placeholder. + # For external mode, send database_id="" so server-side placement resolves it: + # externalPlacement.Resolve selects the sole provider=external ManagedDatabase + # (no region matching - the single registered external DB is always used). + # For other modes, deployment uses "" (auto) and cnpg uses the actual DATABASE_ID. + _gw_database_id="${DATABASE_ID}" + if [[ "${DB_PROVIDER}" == "external" ]]; then + _gw_database_id="" + fi GW_BODY="{\"name\":\"dev-gateway\",\"cluster_id\":\"${CLUSTER_ID}\",\"release_id\":\"${RELEASE_ID}\",\"oidc\":\"${OIDC_JSON}\"" - GW_BODY="${GW_BODY},\"database_id\":\"${DATABASE_ID}\"" + GW_BODY="${GW_BODY},\"database_id\":\"${_gw_database_id}\"" GW_BODY="${GW_BODY},\"route\":\"{\\\"enabled\\\":true}\"" GW_BODY="${GW_BODY}}" GW_RAW=$(api_post "${API_URL}/api/hypershell/v1/gateways" "${GW_BODY}") diff --git a/scripts/kind/up.sh b/scripts/kind/up.sh index 6002f339..f7e7451b 100755 --- a/scripts/kind/up.sh +++ b/scripts/kind/up.sh @@ -181,8 +181,8 @@ echo "" # placement and requires the CNPG operator; any other value is rejected # below rather than silently selected as one provider or the other. DB_PROVIDER="${DATABASE_PROVIDER:-deployment}" -if [[ "${DB_PROVIDER}" != "cnpg" && "${DB_PROVIDER}" != "deployment" ]]; then - error "DATABASE_PROVIDER must be 'cnpg' or 'deployment', got '${DB_PROVIDER}'" +if [[ "${DB_PROVIDER}" != "cnpg" && "${DB_PROVIDER}" != "deployment" && "${DB_PROVIDER}" != "external" ]]; then + error "DATABASE_PROVIDER must be 'cnpg', 'deployment', or 'external', got '${DB_PROVIDER}'" exit 1 fi info "Database provider: ${DB_PROVIDER}" @@ -209,7 +209,7 @@ done for crd in tcproutes.gateway.networking.k8s.io udproutes.gateway.networking.k8s.io; do kube wait --for=delete crd/"$crd" --timeout=30s 2>/dev/null || true done -if [[ "${DB_PROVIDER}" == "deployment" ]]; then +if [[ "${DB_PROVIDER}" == "deployment" || "${DB_PROVIDER}" == "external" ]]; then info "Installing CRDs and controllers (cert-manager, Gateway API, Agent Sandbox) without CNPG..." kustomize build --load-restrictor=LoadRestrictionsNone deploy/kind/infrastructure-no-cnpg | \ kube apply --server-side --force-conflicts -f - @@ -223,7 +223,7 @@ kube wait --for=condition=available deployment/cert-manager -n cert-manager --ti kube wait --for=condition=available deployment/cert-manager-webhook -n cert-manager --timeout=120s info "Waiting for agent-sandbox controller..." kube wait --for=condition=available deployment/agent-sandbox-controller -n agent-sandbox-system --timeout=120s -if [[ "${DB_PROVIDER}" != "deployment" ]]; then +if [[ "${DB_PROVIDER}" == "cnpg" ]]; then info "Waiting for CNPG operator..." kube wait --for=condition=available deployment/cnpg-controller-manager -n cnpg-system --timeout=120s fi @@ -235,7 +235,6 @@ echo "" # --- Build and load local images (offline mode) --- FORCE_ROLLOUT="" if [[ "${LOCAL_IMAGES:-}" == "true" ]]; then - header "Local Images" "${SCRIPT_DIR}/build-images.sh" FORCE_ROLLOUT=true echo "" @@ -280,6 +279,105 @@ kube create secret generic hypershell-oidc-session \ success "OIDC session secret created" echo "" +# --- External PostgreSQL (DATABASE_PROVIDER=external only) --- +# Provision a standalone PostgreSQL in a dedicated namespace so the external +# provider has a real server to probe and issue per-gateway DDL against. +# +# The admin credentials live in their own reserved-prefix namespace +# (hypershell-managed-db-kind), NOT in the HyperShell instance namespace: that +# mirrors production, where the platform team stages the credentials +# out-of-band, normally before HyperShell is installed. The Secret inside it has +# the fixed name hypershell-managed-db-credentials, and +# ManagedDatabase.connection_secret names the NAMESPACE. +if [[ "${DB_PROVIDER}" == "external" ]]; then + header "External Cloud DB (PostgreSQL)" + EXTERNAL_PG_NS="external-cloud-db" + EXTERNAL_PG_PASSWORD="hypershell-kind-admin-password" + EXTERNAL_CREDS_NS="hypershell-managed-db-kind" + info "Deploying standalone PostgreSQL in namespace '${EXTERNAL_PG_NS}'..." + kube create namespace "${EXTERNAL_PG_NS}" --dry-run=client -o yaml | kube apply -f - + kube apply -f - <<'EXTERNAL_PG_EOF' +apiVersion: apps/v1 +kind: Deployment +metadata: + name: postgres + namespace: external-cloud-db +spec: + replicas: 1 + selector: + matchLabels: + app: postgres + template: + metadata: + labels: + app: postgres + spec: + # runAsNonRoot: false and no capabilities.drop are intentional here. + # This stand-in Deployment simulates a cloud-managed external server + # (AWS RDS / IBM Cloud DB) for CI/dev purposes only. The postgres:15 + # entrypoint requires CHOWN/SETUID/SETGID to initialise the data + # directory as root before switching to the postgres user (uid 999); + # dropping ALL capabilities causes a CrashLoopBackOff. The real external + # server runs outside the cluster and is never managed by HyperShell. + # seccompProfile: RuntimeDefault is applied to restrict syscalls within + # the permitted capability set. + securityContext: + runAsNonRoot: false + seccompProfile: + type: RuntimeDefault + containers: + - name: postgres + image: postgres:15 + securityContext: + allowPrivilegeEscalation: false + env: + - name: POSTGRES_PASSWORD + value: hypershell-kind-admin-password + - name: POSTGRES_DB + value: hypershell + ports: + - containerPort: 5432 + readinessProbe: + exec: + command: ["pg_isready", "-U", "postgres"] + initialDelaySeconds: 5 + periodSeconds: 3 +--- +apiVersion: v1 +kind: Service +metadata: + name: postgres + namespace: external-cloud-db +spec: + selector: + app: postgres + ports: + - port: 5432 + targetPort: 5432 +EXTERNAL_PG_EOF + info "Waiting for external PostgreSQL to be ready..." + kube wait --for=condition=available deployment/postgres -n "${EXTERNAL_PG_NS}" --timeout=120s + success "External PostgreSQL ready" + info "Creating credentials namespace '${EXTERNAL_CREDS_NS}'..." + kube create namespace "${EXTERNAL_CREDS_NS}" --dry-run=client -o yaml | kube apply -f - + info "Creating admin Secret 'hypershell-managed-db-credentials' in ${EXTERNAL_CREDS_NS}..." + # sslmode=disable is acceptable ONLY because this stand-in server is in-cluster + # and never reachable from outside. Any real external server must use + # sslmode=require at minimum (verify-full with an inline PEM sslrootcert is + # the recommended hardening). + kube create secret generic hypershell-managed-db-credentials \ + -n "${EXTERNAL_CREDS_NS}" \ + --from-literal=host="postgres.${EXTERNAL_PG_NS}.svc.cluster.local" \ + --from-literal=port="5432" \ + --from-literal=user="postgres" \ + --from-literal=password="${EXTERNAL_PG_PASSWORD}" \ + --from-literal=dbname="postgres" \ + --from-literal=sslmode="disable" \ + --dry-run=client -o yaml | kube apply -f - + success "Admin credentials namespace and Secret created" + echo "" +fi + # --- Deploy all components via kustomize --- header "Deploying Components" @@ -295,6 +393,16 @@ header "Deploying Components" _db_overlay_extra=$'\ncomponents:\n - ../components/database-deployment' if [[ "${DB_PROVIDER}" == "cnpg" ]]; then _db_overlay_extra=$'\npatches:\n - path: ../kind/database-cnpg-env-patch.yaml\n target:\n kind: Deployment\n name: hypershell-api-server\n namespace: hypershell-system\n - path: ../kind/database-cnpg-env-patch.yaml\n target:\n kind: Deployment\n name: hypershell-controller\n namespace: hypershell-system' +elif [[ "${DB_PROVIDER}" == "external" ]]; then + # Use the database-external component: removes the CNPG Cluster, sets + # DB_SSLMODE=disable, and injects the hypershell-db-app Secret pointing to + # the external-cloud-db Postgres provisioned above. The API server and its + # migrate init container both read from that external server; gateways use + # the same server via their per-gateway databases (gw_) created by the + # control plane. DATABASE_PROVIDER=external is NOT patched here - it is + # applied via kubectl set env after the initial deployment readiness wait + # below so the baseline image can start cleanly before the PR image swap. + _db_overlay_extra=$'\ncomponents:\n - ../components/database-external' fi if [[ "${LOCAL_IMAGES:-}" == "true" ]]; then @@ -338,7 +446,7 @@ if [[ "${DB_PROVIDER}" == "deployment" ]]; then info "Waiting for PostgreSQL deployment..." kube wait --for=condition=available deployment/hypershell-postgres -n "${KIND_NAMESPACE}" --timeout=120s success "PostgreSQL deployment ready" -else +elif [[ "${DB_PROVIDER}" == "cnpg" ]]; then if [[ -n "${HYPERSHELL_DATABASE_IMAGE:-}" ]]; then info "Setting API server CNPG cluster image to ${HYPERSHELL_DATABASE_IMAGE}..." kube patch cluster/hypershell-db -n "${KIND_NAMESPACE}" --type merge \ @@ -348,6 +456,7 @@ else kube wait --for=condition=Ready cluster/hypershell-db -n "${KIND_NAMESPACE}" --timeout=300s success "CNPG clusters ready" fi +# external: standalone PG was already provisioned and waited on above if [[ -z "${KIND_KEYCLOAK_URL:-}" ]]; then info "Waiting for Keycloak..." @@ -568,6 +677,7 @@ if ! is_swapped control-plane; then kube wait --for=condition=available deployment/hypershell-controller -n "${KIND_NAMESPACE}" --timeout=120s fi + if is_swapped web-console; then warn "Web console is swapped -- scaling to zero (runs locally via npm)" kube scale deployment/hypershell-web-console -n "${KIND_NAMESPACE}" --replicas=0 @@ -747,6 +857,33 @@ kube rollout status deployment/hypershell-web-console -n "${KIND_NAMESPACE}" --t success "Web console ready" echo "" +if [[ "${DB_PROVIDER}" == "external" ]]; then + # Apply DATABASE_PROVIDER=external after the readiness gate passes. Deferring + # here - not baking it into the kustomize overlay and not setting it before the + # rollout-status wait above - ensures the baseline image (which predates + # external-mode support) can pass the readiness check without crashing. + # In CI the image-swap step runs next; it sets the PR image which supports + # external mode, so the single rollout triggered by set-component-images.sh + # carries both the new image and the new env together. + info "Setting DATABASE_PROVIDER=external on api-server and controller..." + kube set env deployment/hypershell-api-server -c api-server -n "${KIND_NAMESPACE}" \ + DATABASE_PROVIDER=external + kube set env deployment/hypershell-controller -c controller -n "${KIND_NAMESPACE}" \ + DATABASE_PROVIDER=external + if is_swapped api-server || [[ "${LOCAL_IMAGES:-}" == "true" ]]; then + # Local development: working-tree images already support external mode; + # wait for the rollout so seeding does not race ahead of a ready cluster. + # LOCAL_IMAGES=true deploys working-tree images via the kustomize overlay + # (not via swap-component.sh), so is_swapped returns false even though the + # images support external mode - the OR covers that case. + kube rollout status deployment/hypershell-api-server -n "${KIND_NAMESPACE}" --timeout=120s + kube rollout status deployment/hypershell-controller -n "${KIND_NAMESPACE}" --timeout=120s + fi + # CI (baseline, not swapped, not LOCAL_IMAGES): no wait - set-component-images.sh + # triggers the image swap immediately after kind-up, restarting pods with the PR + # image that supports external mode before seeding begins. +fi + # --- Seed platform resources via REST API --- # Seeding lives in seed.sh so CI can run it AFTER the component image swap # (see scripts/kind/seed.sh). Local runs seed inline by default; CI sets diff --git a/specs/platform/data-model.spec.md b/specs/platform/data-model.spec.md index f5ff8929..9975de86 100644 --- a/specs/platform/data-model.spec.md +++ b/specs/platform/data-model.spec.md @@ -12,7 +12,7 @@ Gateways, clusters, databases, releases, and networks are **top-level resources* Current model: - **ManagedCluster** - a Kubernetes cluster registered into the platform. Tracks provider, region, API server URL, and a kubeconfig secret reference. -- **ManagedDatabase** - a database instance provisioned for gateway use. Tracks provider, region, engine type/version, instance class, and a connection secret reference. +- **ManagedDatabase** - a PostgreSQL server that gateways are placed onto. Tracks provider, region, engine type/version, instance class, and a connection secret reference. The `provider` field records how the server came into being: `deployment` (an in-cluster PostgreSQL Deployment dedicated to one gateway), `cnpg` (a shared in-cluster CNPG Cluster), or `external` (a cloud-managed server owned outside HyperShell and registered here). The `external` provider adds **no columns**: it gives the pre-existing `region`, `engine`, `engine_version`, `instance_class` and `connection_secret` fields their first consumer. `connection_secret` names the **namespace** (not the Secret) that holds the administrative connection to an external server, and is subject to a reserved-prefix rule - the Secret inside it has the fixed name `hypershell-managed-db-credentials`. The field name is retained for wire and column compatibility; see [`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md). - **GatewayRelease** - a versioned container image for gateway deployments. Supports rollout strategies with canary percent/duration controls. - **Gateway** - an API gateway instance deployed onto a specific cluster, using a specific release and database, within an API-assigned namespace. Tracks TLS mode, service type, external DNS, and lifecycle phase. - **OpenShellGatewayServiceAccount** - a creator-bound automation identity for one Gateway. It stores an OpenShell role and non-secret Keycloak lifecycle metadata. diff --git a/specs/platform/global-architecture.spec.md b/specs/platform/global-architecture.spec.md index f54b87e5..3e58fab4 100644 --- a/specs/platform/global-architecture.spec.md +++ b/specs/platform/global-architecture.spec.md @@ -928,7 +928,7 @@ the gateway database. | Component | Tool | Purpose | |-----------|------|---------| -| Database operator | CNPG (CloudNativePG) | PostgreSQL lifecycle (replaces per-gateway cloud databases) | +| Database operator | CNPG (CloudNativePG) | PostgreSQL lifecycle for the default, in-cluster provider; not required by the `external` provider | | GitOps | ArgoCD (on every cluster) | Each cluster self-reconciles its own platform state by pulling its path from the central GitOps repo (pull model) | | Secret management | Vault | Stores and rotates secrets with cloud-native drivers | | Identity | Keycloak | OIDC authentication for gateways and console | @@ -937,15 +937,41 @@ the gateway database. | Monitoring | Prometheus | Metrics collection on all clusters | | Dashboards | Grafana | Centralized visualization on hub | -## Database Strategy: CNPG +## Database Strategy -CloudNativePG replaces per-gateway cloud-managed databases (RDS, Cloud SQL). CNPG runs PostgreSQL clusters as Kubernetes-native resources with automated failover, backup, and recovery. +Gateway PostgreSQL is provisioned through one of three providers, selected per +install by `DATABASE_PROVIDER` and recorded on each ManagedDatabase. See +[`openshell-gateway-database.spec.md`](./openshell-gateway-database.spec.md) and, for +the external provider, +[`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md). + +| Provider | Server lifecycle | Operator required | +|---|---|---| +| `deployment` | HyperShell runs a standalone PostgreSQL Deployment per gateway | none | +| `cnpg` | HyperShell runs a shared CNPG `Cluster` | CloudNativePG | +| `external` | Owned externally (AWS RDS/Aurora, IBM Cloud Databases); HyperShell registers the endpoint and provisions a database and role per gateway inside it | none | + +### CNPG is the default, portable choice + +CloudNativePG runs PostgreSQL clusters as Kubernetes-native resources with automated +failover, backup, and recovery. It is portable across clouds and requires no cloud +database offering, which is why it is the recommended provider for a HyperShell +install that has a free choice. + +### `external` is a supported, opt-in alternative + +The `external` provider deliberately reintroduces cloud-managed databases for +operators who must consume one - for compliance, an existing cloud investment, or a +managed backup and HA story they are required to use. Choosing it trades portability +for those properties, and it is opt-in: nothing about `external` is on the default +path. HyperShell never creates, resizes, or deletes an external server; it registers +a pre-existing endpoint and manages only the per-gateway database and role inside it. ### Requirements #### Requirement: CNPG Operator Deployment -The CNPG operator SHALL be deployed on the hub cluster. Each ManagedDatabase SHALL be provisioned as a CNPG Cluster resource in its own dedicated namespace (`openshell-db-`), and gateway databases SHALL be provisioned as logical CNPG `Database` resources inside that ManagedDatabase namespace - not in the gateway namespace. +The CNPG operator SHALL be deployed on the hub cluster when `DATABASE_PROVIDER=cnpg`. The `deployment` and `external` providers SHALL NOT require it. Each ManagedDatabase SHALL be provisioned as a CNPG Cluster resource in its own dedicated namespace (`openshell-db-`), and gateway databases SHALL be provisioned as logical CNPG `Database` resources inside that ManagedDatabase namespace - not in the gateway namespace. ##### Scenario: Gateway Database Provisioning via CNPG @@ -1213,7 +1239,7 @@ repo while syncing a different path. To fork the repo, edit only that file. | Full operator stack on all tiers | Every cluster has ArgoCD, Vault, Keycloak, CNPG, Prometheus - but serves different purposes per tier. Because each cluster's own ArgoCD installs its stack from Git, the stack is present before any tenant workload lands. | | Federated Keycloak chain | RH SSO → Global → Cloud Hub → ManagedCluster - identity flows down, authentication bubbles up | | Vault per tier with distinct purposes | Cloud Hub Vault: service secrets; ManagedCluster Vault: gateway keystores | -| CNPG on all clusters | Kubernetes-native lifecycle, portable across clouds, no vendor lock-in | +| CNPG as the default database provider | Kubernetes-native lifecycle, portable across clouds, no vendor lock-in. The `external` provider is available where an operator must consume a cloud-managed database, trading portability for the provider's compliance, backup, and HA properties. | | PostgreSQL on Cloud Hub as source of truth | All Gateway/ManagedCluster resource state lives in Cloud Hub database | | ManagedClusters can be standard K8s | Maximizes deployment flexibility; only hubs need OpenShift | | Tekton over bash scripts | Deterministic, auditable, cattle-not-pets infrastructure | diff --git a/specs/platform/openshell-gateway-database-external.spec.md b/specs/platform/openshell-gateway-database-external.spec.md new file mode 100644 index 00000000..34dd599b --- /dev/null +++ b/specs/platform/openshell-gateway-database-external.spec.md @@ -0,0 +1,915 @@ +# OpenShell Gateway Database Specification - External Provider + +**Date:** 2026-09-07 +**Status:** Active +**Parent:** [`openshell-gateway-database.spec.md`](./openshell-gateway-database.spec.md) - gateway database provisioning + +--- + +## Purpose + +This specification defines the **`external`** PostgreSQL provisioning mode for +OpenShell gateways, selected by `DATABASE_PROVIDER=external`. + +In `external` mode, PostgreSQL server infrastructure is **provisioned outside +HyperShell** - a cloud-managed database (AWS RDS / Aurora for PostgreSQL, IBM +Cloud Databases for PostgreSQL) created out-of-band by a platform team or IaC. +HyperShell does **not** create, resize, or delete the server. HyperShell +**registers** the pre-existing endpoint as a `ManagedDatabase` and, within it, +provisions **one dedicated PostgreSQL database and one dedicated login role per +gateway**, so each gateway is isolated from every other gateway sharing the same +external server. + +The control plane issues the `CREATE DATABASE` / `CREATE ROLE` / `GRANT` +statements itself, over a short-lived administrative connection. No operator is +required and no in-cluster PostgreSQL workload is created. + +PostgreSQL is the only supported backend. + +### Contracts this spec builds on + +External mode participates in the shared gateway-database contracts defined by the +parent spec, restated here so this document stands on its own: + +- **`ManagedDatabase`** - the resource a `Gateway` points at through its + `database_id` foreign key. It carries a `provider` field that determines how it + is reconciled. +- **`database_id`** - server-owned in every mode. Clients send an empty string; + the API server ignores and replaces any non-empty value. Omitting the property is + invalid. `cluster_id` is stored but has no effect on database placement. +- **`openshell-gateway-db-credentials`** - the Secret in the gateway's tenant + namespace that the gateway workload consumes as `--db-url $(OPENSHELL_DB_URL)`. +- **`DATABASE_PROVIDER`** - the install-wide provider selector, read independently + at startup by both the API server and the control plane. + +### Relationship to global-architecture.spec.md + +[`global-architecture.spec.md`](./global-architecture.spec.md) § Database Strategy +presents `external` as a supported, **opt-in** alternative for operators who must +consume a managed database offering - for compliance, backups, HA, or an existing +cloud investment. Nothing about `external` is on the default path. + +--- + +## Prerequisites + +External mode is **register-only**. The following are the platform administrator's +responsibility, out-of-band, before an `external` ManagedDatabase is created: + +1. **A running PostgreSQL server** (AWS RDS/Aurora or IBM Cloud Databases for + PostgreSQL) reachable from the control-plane cluster. +2. **Network egress / reachability.** The control-plane cluster SHALL have a + network path to the server endpoint (VPC peering, private endpoint/service + endpoint, security-group / ACL allow rules). Loss of reachability is a + provisioning failure, not a silent skip (see readiness below). +3. **An administrative role** on the server with at least `CREATEDB` and + `CREATEROLE` privileges. Full superuser is **not** required; the AWS RDS + `rds_superuser` role and the IBM Cloud Databases administrative user are both + acceptable. The admin role does not need, and SHOULD NOT be granted, the ability + to alter server-level configuration. +4. **A credentials namespace and Secret** - a Kubernetes Namespace whose name + carries the reserved prefix `hypershell-managed-db-`, containing a Secret named + `hypershell-managed-db-credentials` (see Requirement: External Connection + Namespace And Secret). +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 + 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. + +### Assumption: one HyperShell install per external server + +Per-gateway database and role names are `gw_` (KSUID-unique) with no +install-scoped prefix. Sharing one external server across multiple HyperShell +installs is **out of scope** for this spec; doing so risks name collisions and is +not supported. + +--- + +## Architecture + +### ManagedDatabase as a registration of an external server + +An `external` ManagedDatabase is a **registration record**, not a provisioning +target. It carries: + +- `provider: "external"` +- `connection_secret` - the **namespace** holding the administrative credentials + Secret (**required**; see the requirement below for why this field names a + namespace rather than a Secret) +- `region` - the cloud region of the server (informational metadata; not used for + placement) +- `engine`, `engine_version`, `instance_class` - descriptive metadata, + informational only +- `namespace` - assigned by the shared `BeforeCreate` hook but **unused** in + external mode. No Namespace is created for an `external` ManagedDatabase and no + resource is placed in one. + +``` +Platform team (out of band) + ├── AWS RDS / IBM Cloud Databases for PostgreSQL (endpoint + admin user) + └── Namespace hypershell-managed-db- + └── Secret hypershell-managed-db-credentials ──referenced by──┐ + ▼ +ManagedDatabase (provider=external, connection_secret=hypershell-managed-db-) + │ ManagedDatabaseReconciler + ▼ (no namespace, no workload; validates connectivity + admin capability → status) + +Gateway A ──database_id──→ ManagedDatabase (external) +Gateway B ──database_id──→ ManagedDatabase (same, or another registered external) + + │ GatewayReconciler (per gateway), admin connection to external server + ▼ +External server: + ├── ROLE gw_ (LOGIN, owns its database) + └── DATABASE gw_ (owner gw_; CONNECT revoked from PUBLIC) + + └── Secret openshell-gateway-db-credentials (tenant namespace) → gateway --db-url +``` + +### DDL execution: in-process, in the control plane + +The control plane issues DDL **in-process** using a PostgreSQL client, opening a +short-lived admin connection per reconciliation and closing it afterward. It does +**not** maintain a long-lived connection pool and does **not** launch Kubernetes +Jobs to run SQL. + +Rationale: per-gateway provisioning is a database operation, not a Kubernetes one. +In-process execution gives synchronous, structured errors (required by the control +plane conventions - `fmt.Errorf` with context, status set on every error path), +trivially idempotent reconcile (query `pg_database`/`pg_roles`, then act), clean +status transitions, and testability against a throwaway PostgreSQL +(testcontainers). A Job-based approach would reintroduce the asynchronous +log/exit-code parsing that the gRPC-watch reconciler pattern avoids. + +**DDL and server-side logging caveat:** see Prerequisite 5. `CREATE ROLE` and +`ALTER ROLE` carry the plaintext password in the statement text, so on a server +configured with `log_statement = 'all'` or `'ddl'` the password reaches the +server's activity log. Restricting that is the operator's responsibility. + +--- + +## Requirements + +### Requirement: External Provider Validation and Immutability + +The API server SHALL accept `external` as a ManagedDatabase provider value. Once +set to `external`, the provider SHALL NOT transition to any other value; +status-only and other mutable-field updates SHALL preserve it. + +An `external` ManagedDatabase SHALL be rejected at create/replace time if +`connection_secret` is empty. `region`, `engine`, `engine_version` and +`instance_class` are optional metadata, informational only, and SHALL NOT affect +placement or connection behaviour. + +#### Scenario: Create external ManagedDatabase without a connection secret + +- GIVEN a create request with `provider: "external"` and empty `connection_secret` +- WHEN the API server validates the request +- THEN it SHALL reject it as invalid input naming the missing `connection_secret` +- AND SHALL NOT persist the ManagedDatabase + +#### Scenario: Attempt to change provider away from external + +- GIVEN an existing ManagedDatabase with `provider: "external"` +- WHEN a caller attempts to update its provider to another value +- THEN the API server SHALL reject the update as invalid input +- AND the persisted provider SHALL remain `external` + +--- + +### Requirement: External Connection Namespace And Secret + +The `connection_secret` field SHALL name a **Kubernetes Namespace**, not a Secret. +Within that namespace the administrative connection lives in a Secret whose name is +**fixed**: `hypershell-managed-db-credentials`. Per the security standards (secret +references, not inline secrets), no admin credential is stored in the API server +database - only the namespace reference. + +The operator provisions the namespace and the Secret out-of-band, as they provision +the server itself. + +> **Why the reference names a namespace.** The credentials are created before +> HyperShell is installed, by whoever provisions the server. Requiring them to live +> in the HyperShell control-plane instance namespace would force that namespace to +> exist first, inverting the intended install order. A dedicated, prefixed namespace +> lets the platform team stage external database credentials independently of any +> HyperShell deployment, and lets them be managed, RBAC-scoped and lifecycled by the +> team that owns the server. + +#### Reference format + +`connection_secret` SHALL satisfy all of: + +1. It SHALL NOT contain `/`. A `namespace/name` form SHALL be rejected. +2. It SHALL begin with the reserved prefix `hypershell-managed-db-`. +3. It SHALL be a valid DNS-1123 **label** (Kubernetes namespace names are labels, + maximum 63 characters), and therefore also a valid Namespace name. + +The control plane SHALL read exactly one Secret for an `external` ManagedDatabase: +`hypershell-managed-db-credentials` in the namespace named by `connection_secret`. +It SHALL NOT read any other Secret in that namespace and SHALL NOT resolve the +reference in any other namespace. Per +[`naming-multitenancy.spec.md`](../standards/platform/naming-multitenancy.spec.md) +§1, the Secret carries a constant name and is isolated by namespace, so it needs no +instance prefix and SHALL NOT be rewritten by a Kustomize `namePrefix`. + +These rules SHALL be enforced in two places: the API server SHALL validate them at +create, replace and patch and reject a violation as invalid input (so the operator +gets an immediate, actionable error), and the control plane SHALL re-check them at +resolution time and refuse to read a Secret that does not satisfy them, before +issuing any read. + +> **Why the reference is constrained.** The control-plane ServiceAccount holds a +> ClusterRole granting `get`/`list`/`watch` on Secrets across all namespaces, because +> other providers legitimately create and copy Secrets into ManagedDatabase and +> tenant namespaces. Narrowing that grant is not an option here. An unconstrained +> reference would therefore let anyone who can create a ManagedDatabase name **any** +> Secret in the cluster and have the control plane read it and open a PostgreSQL +> connection with its contents - including `hypershell-db-app`, the API server's own +> database credentials, which has exactly the key shape this Secret expects. The +> control plane would then run `CREATE ROLE` / `CREATE DATABASE` inside the +> platform's own database and hand a tenant working credentials to it. HyperShell +> platform administrator is an API-level role that does not imply permission to read +> Secrets in the hub cluster, so this would be a genuine privilege escalation. +> +> The reserved namespace prefix **plus the fixed Secret name** reduce the reachable +> set to a single, deliberately named Secret inside namespaces an operator +> deliberately created as external database credential holders. §6 of the naming +> standard governs that name space. + +The namespace SHALL be registered in the naming standard's resource inventory as +`hypershell-managed-db-` (Namespace), and the Secret as +`hypershell-managed-db-credentials` within it. + +The Secret SHALL contain: + +| Key | Required | Meaning | +|---|---|---| +| `host` | yes | External server hostname/endpoint | +| `port` | yes | External server port (typically `5432`) | +| `user` | yes | Admin role with `CREATEDB` + `CREATEROLE` | +| `password` | yes | Admin role password | +| `dbname` | no | Maintenance/admin database to connect to (default `postgres`) | +| `sslmode` | no | Admin connection TLS mode (default `require`) | +| `sslrootcert` | no | Inline PEM CA bundle used to verify the server certificate when `sslmode` is `verify-ca` or `verify-full`. The control plane materialises it for the driver; operators supply PEM text, never a path | + +Admin credentials SHALL NEVER appear in logs, error strings, telemetry, or API +responses. + +#### Scenario: Reject a namespace-qualified connection secret reference + +- GIVEN a create request with `provider: "external"` and + `connection_secret: "kube-system/hypershell-managed-db-x"` +- WHEN the API server validates the request +- THEN it SHALL reject it as invalid input because the reference contains `/` +- AND SHALL NOT persist the ManagedDatabase + +#### Scenario: Reject a reference outside the reserved prefix + +- GIVEN a create request with `provider: "external"` and + `connection_secret: "default"` +- WHEN the API server validates the request +- THEN it SHALL reject it as invalid input naming the required + `hypershell-managed-db-` prefix +- AND SHALL NOT persist the ManagedDatabase + +#### Scenario: Control plane refuses a non-conforming reference at resolve time + +- GIVEN a persisted `external` ManagedDatabase whose `connection_secret` does not + satisfy the reference format (for example, written before this rule existed) +- WHEN the ManagedDatabaseReconciler resolves it +- THEN it SHALL set status `Failed: secret_invalid` and SHALL NOT read any Secret +- AND SHALL NOT open a connection to any server + +#### Scenario: Credentials Secret missing at reconcile time + +- GIVEN an `external` ManagedDatabase whose `connection_secret` namespace exists but + contains no `hypershell-managed-db-credentials` Secret +- WHEN the ManagedDatabaseReconciler processes it +- THEN it SHALL set status `Failed: secret_invalid` +- AND SHALL NOT read any other Secret in that namespace +- AND SHALL NOT proceed to any gateway provisioning against that server +- AND the next reconciliation SHALL retry + +--- + +### Requirement: ManagedDatabase Reconciliation (provider=external) + +The ManagedDatabaseReconciler SHALL treat an `external` ManagedDatabase as a +**connectivity + capability check**, not an infrastructure-provisioning step. It +SHALL NOT create a Namespace, a workload, a Service, or a PVC, and SHALL create no +Kubernetes resource of any kind. + +For each `external` ManagedDatabase, the reconciler SHALL: + +1. Validate `connection_secret` against the reference format and read + `hypershell-managed-db-credentials` from that namespace. +2. Open a short-lived admin connection to the external server using the Secret's + TLS settings. +3. Verify the admin role can create databases and roles (e.g. confirm + `rolcreatedb` and `rolcreaterole`, or attempt a harmless capability probe). +4. Set status from the closed reason vocabulary below. +5. Close the connection. + +The check SHALL be idempotent and side-effect-free on the external server. + +#### Status reason vocabulary + +The ManagedDatabase `status` field is returned by every read of the resource. In +external mode the reconciler SHALL set it to exactly one of the following values and +SHALL NOT interpolate a driver error into it: + +| Status | Meaning | +|---|---| +| `Provisioning` | The probe has not yet completed | +| `Ready` | Connected, and the admin role has `CREATEDB` and `CREATEROLE` | +| `Failed: secret_invalid` | `connection_secret` fails the reference format, the namespace or the `hypershell-managed-db-credentials` Secret does not resolve, or the Secret is missing a required key | +| `Failed: unreachable` | No network path, DNS failure, or connection timeout | +| `Failed: auth_failed` | The server rejected the admin credentials | +| `Failed: insufficient_privilege` | Connected, but the admin role lacks `CREATEDB` or `CREATEROLE` | +| `Failed: tls_failed` | TLS negotiation or certificate verification failed | + +The reconciler SHALL map every underlying error onto one of these values, defaulting +to `Failed: unreachable` for an unrecognised connection-time error. PostgreSQL driver +errors routinely embed the host, the admin user, and sometimes the full DSN, so the +driver's own message SHALL NOT reach `status`. It MAY be logged, redacted per the +security standards, to give operators a diagnostic path. + +#### Scenario: External server reachable with a capable admin + +- GIVEN an `external` ManagedDatabase whose admin Secret connects successfully + and whose admin role has `CREATEDB` and `CREATEROLE` +- WHEN the ManagedDatabaseReconciler processes it +- THEN it SHALL set status `Ready` +- AND SHALL create no Kubernetes resource + +#### Scenario: External server unreachable + +- GIVEN an `external` ManagedDatabase whose endpoint is not reachable from the + control-plane cluster +- WHEN the ManagedDatabaseReconciler processes it +- THEN it SHALL set status `Failed: unreachable` +- AND the driver's error text SHALL NOT appear in the status +- AND the next reconciliation SHALL retry + +#### Scenario: Driver error is not echoed into status + +- GIVEN an `external` ManagedDatabase whose admin connection fails with a driver + error containing the endpoint hostname and admin username +- WHEN the ManagedDatabaseReconciler sets the resource status +- THEN the status SHALL be one of the closed reason values +- AND SHALL NOT contain the driver's message, the hostname, the username, or any + part of the connection string + +#### Scenario: Admin role lacks required privileges + +- GIVEN an `external` ManagedDatabase whose admin role lacks `CREATEDB` or `CREATEROLE` +- WHEN the ManagedDatabaseReconciler processes it +- THEN it SHALL set status `Failed: insufficient_privilege` +- AND SHALL NOT attempt per-gateway provisioning + +--- + +### Requirement: External Server Selection + +When `DATABASE_PROVIDER=external`, the API server SHALL resolve a new Gateway's +`database_id` (server-owned; caller value ignored and replaced) by selecting from +the registered `external` ManagedDatabases: + +1. Collect all `ManagedDatabase` records with `provider=external`. +2. If **zero** exist, reject the creation with a contextual error. +3. Otherwise, select the ManagedDatabase that was **created first**: order the + candidates by creation timestamp ascending and take the first. Ties SHALL be + broken by ID ascending, which is deterministic and, because IDs are + time-sortable KSUIDs, agrees with creation order. + +More than one registered `external` ManagedDatabase is **not** an error. Selection +is deterministic: the same candidate set always yields the same choice, so +concurrent gateway creations agree without coordination. + +Selection happens **only at gateway creation**. Once assigned, a gateway's +`database_id` is fixed for its lifetime: registering a further `external` +ManagedDatabase never moves an existing gateway, and the first-created +ManagedDatabase remains the placement target for every new gateway while it exists. +A ManagedDatabase cannot be deleted while any Gateway references it, so the +selected registration is guaranteed to remain resolvable for the gateways placed on +it. + +`cluster_id` has no effect on placement in external mode. + +> **Why oldest-wins rather than rejecting ambiguity.** An operator may register a +> second external server ahead of a migration, or to document a standby, without +> intending to change where new gateways land. Rejecting gateway creation whenever a +> second registration exists turns a benign inventory action into an outage for +> gateway provisioning. Oldest-wins keeps placement stable and predictable while +> leaving the registry free. + +#### Scenario: Gateway placement with a single external database + +- GIVEN `DATABASE_PROVIDER=external` and exactly one `external` ManagedDatabase +- WHEN the API server processes the create request +- THEN it SHALL assign that ManagedDatabase's ID as the gateway's `database_id` + +#### Scenario: Gateway placement with several external databases picks the oldest + +- GIVEN `DATABASE_PROVIDER=external` and three `external` ManagedDatabases + registered at different times +- WHEN the API server processes the create request +- THEN it SHALL assign the ID of the ManagedDatabase created first +- AND SHALL NOT reject the creation as ambiguous + +#### Scenario: A newly registered external database does not move existing gateways + +- GIVEN an existing Gateway placed on the first-created `external` ManagedDatabase +- WHEN a further `external` ManagedDatabase is registered +- THEN the existing Gateway's `database_id` SHALL be unchanged +- AND subsequent gateway creations SHALL still select the first-created ManagedDatabase + +#### Scenario: Gateway placement finds no external database + +- GIVEN `DATABASE_PROVIDER=external` and no `external` ManagedDatabase exists +- WHEN the API server processes the create request +- THEN it SHALL reject the creation with a contextual error and create no gateway + +--- + +### Requirement: Per-Gateway Database Provisioning (External Mode) + +In external mode, the GatewayReconciler SHALL provision a dedicated PostgreSQL +database and login role for each gateway by issuing idempotent DDL against the +external server over an admin connection. + +Role and database are both named `gw_` (underscores; PostgreSQL +identifiers avoid hyphens). `` is the gateway's full resource ID, +lowercased. + +For each gateway, the reconciler SHALL: + +1. Resolve the gateway's `database_id` to the `external` ManagedDatabase and read + `hypershell-managed-db-credentials` from its `connection_secret` namespace. +2. Determine the per-gateway password: if the tenant-namespace Secret + `openshell-gateway-db-credentials` already exists with a `password`, **reuse it** + (create-or-skip semantics - do not regenerate on re-reconciliation); otherwise + generate a 32-byte cryptographically random hex password (`crypto/rand`) and treat + it as authoritative, forcing it onto the role in step 3. +3. Open a short-lived admin connection and reconcile, idempotently: + - **Role:** if `gw_` is absent (`SELECT 1 FROM pg_roles ...`), create it + with `LOGIN` and the password. If the role is present **and** the password was + reused from an existing tenant Secret, leave it alone. If the role is present but + the password was newly generated - because the tenant Secret was absent - the + reconciler SHALL `ALTER ROLE gw_ PASSWORD ''` so the role matches + the Secret it is about to write. Writing a freshly generated password into the + tenant Secret without applying it to an existing role produces a gateway that can + never authenticate and that re-reconciliation would not repair. + - **Database:** if `gw_` is absent (`SELECT 1 FROM pg_database ...`), + create it with `OWNER gw_`. (`CREATE DATABASE` cannot run inside a + transaction block and has no `IF NOT EXISTS`; the reconciler SHALL guard it with + an existence check rather than relying on catching an error.) + - **Isolation:** `REVOKE CONNECT ON DATABASE gw_ FROM PUBLIC` and grant + `CONNECT` only to `gw_`, so no other gateway's role can connect. The + owning role's default `public` schema privileges SHALL be scoped so tenants + cannot read or write each other's databases. +4. Write/refresh the tenant-namespace Secret `openshell-gateway-db-credentials` + (see Requirement: Gateway Credentials Secret (External Mode)). +5. Proceed to deploy the gateway workload only after DDL and the credentials Secret + succeed. + +All DDL SHALL be idempotent: re-running against an already-provisioned gateway SHALL +make no destructive change and SHALL NOT regenerate the password. Every non-benign +SQL error SHALL be propagated with context (never swallowed); credentials SHALL NOT +appear in error text. + +> **This is provisioning repair, not credential rotation.** The `ALTER ROLE` in step +> 3 exists solely so a gateway whose tenant Secret was lost can authenticate again. +> External mode has no credential rotation - see Requirement: No Credential Rotation +> (External Mode). + +#### Scenario: New gateway provisioned on an external server + +- GIVEN a new Gateway resolved to an `external` ManagedDatabase +- WHEN the GatewayReconciler processes the event +- THEN it SHALL create role and database `gw_` on the external server if absent +- AND revoke `CONNECT` from `PUBLIC` on that database +- AND write `openshell-gateway-db-credentials` into the tenant namespace +- AND proceed to deploy the gateway workload + +#### Scenario: Re-reconcile an already-provisioned external gateway + +- GIVEN a Gateway whose external role and database already exist +- WHEN the GatewayReconciler re-processes the event +- THEN it SHALL detect both exist and make no destructive change +- AND SHALL NOT regenerate or alter the password + +#### Scenario: Tenant credentials Secret lost while the role still exists + +- GIVEN a Gateway whose external role `gw_` exists on the server +- AND whose tenant-namespace `openshell-gateway-db-credentials` Secret is absent + (for example after tenant-namespace garbage collection or a cluster rebuild) +- WHEN the GatewayReconciler processes the event +- THEN it SHALL generate a new password, apply it to the existing role with + `ALTER ROLE`, and write the matching tenant Secret +- AND the gateway SHALL be able to authenticate without operator intervention + +#### Scenario: External server unreachable during gateway provisioning + +- GIVEN a Gateway resolved to an `external` ManagedDatabase whose server is unreachable +- WHEN the GatewayReconciler attempts DDL +- THEN it SHALL return a contextual error, leave the gateway phase `Provisioning`, + and retry on the next reconciliation, without creating the gateway workload + +--- + +### Requirement: Gateway Credentials Secret (External Mode) + +After per-gateway DDL, the GatewayReconciler SHALL ensure the tenant-namespace +Secret `openshell-gateway-db-credentials` exists, consumed by the gateway workload +via `--db-url $(OPENSHELL_DB_URL)`. + +| Key | Value | +|---|---| +| `host` | external server endpoint (from the admin Secret `host`) | +| `port` | external server port (from the admin Secret `port`) | +| `dbname` | `gw_` | +| `user` | `gw_` | +| `password` | generated per-gateway password | +| `uri` | `postgresql://gw_:@:/gw_?sslmode=` | +| `sslrootcert` | (optional) inline PEM CA bundle, present when `verify-full` is used | + +**TLS:** the connection to a cloud-managed server SHALL be encrypted. Default +`sslmode=require` (encrypt without certificate verification), which needs no extra +files in the tenant namespace. `sslmode=verify-full` is the recommended hardening and +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. + +#### Scenario: Credentials Secret written with the default TLS mode + +- GIVEN an `external` ManagedDatabase whose admin Secret carries no `sslrootcert` +- WHEN the GatewayReconciler writes the tenant credentials Secret +- THEN the `uri` SHALL carry `sslmode=require` +- AND no `sslrootcert` key SHALL be written into the tenant namespace + +--- + +### Requirement: Per-Gateway Cleanup + +When a Gateway backed by an `external` ManagedDatabase is deleted, the control plane +SHALL destroy that gateway's external database and role. Deletion is +**unconditional**: there is no retention policy, no per-database configuration, and +no operator confirmation. + +> **Recovery is the external provider's responsibility.** AWS RDS/Aurora and IBM +> Cloud Databases both provide automated backups and point-in-time recovery, which is +> among the reasons an operator selects a managed offering. HyperShell's drop is not +> the last line of defence against accidental deletion, and HyperShell SHALL NOT +> attempt to be one by retaining orphaned tenant databases on a shared server. + +The Gateway delete watch event SHALL carry at least the gateway ID and its +`database_id`, which is all the state cleanup needs: both PostgreSQL object names +derive from the gateway ID, and the admin connection is resolved through the +`database_id`. Because a ManagedDatabase cannot be deleted while any Gateway +references it, the registration is still resolvable at the moment its last gateway +is deleted. + +Over an admin connection, the control plane SHALL: + +1. Terminate active backends connected to `gw_` (`pg_terminate_backend` + over `pg_stat_activity`), so the drop is not blocked by the gateway's own + lingering connections. +2. `DROP DATABASE gw_` (guarded by an existence check; `DROP DATABASE` + cannot run inside a transaction block). +3. `DROP ROLE gw_`. + +Cleanup SHALL be idempotent: an already-absent database or role counts as successful +cleanup, so replaying a delete is safe. + +#### Cleanup is best-effort; there is no tombstone or retry queue + +External-mode cleanup runs **once**, on the delete event. There is no tombstone +record and no cross-restart retry queue: the gateway is already removed from the API +server, so no later event re-delivers the work. + +Consequently, if the external server is unreachable or the drop otherwise fails, the +role and database **persist on the server with valid credentials**. The control plane +SHALL propagate the failure as a contextual error and SHALL log it at error level, +naming the gateway ID and the ManagedDatabase ID (never the credentials), so the +orphan is discoverable. Operators recover with the runbook below. + +This is a deliberate trade: unconditional, single-shot deletion keeps the delete path +simple and free of persistent state, at the cost of an operator-visible orphan when +the external server is down at exactly the wrong moment. + +#### Scenario: Delete external-backed gateway + +- GIVEN a Gateway backed by an `external` ManagedDatabase +- WHEN the Gateway is deleted +- THEN the control plane SHALL terminate active connections to `gw_`, + drop the database, then drop the role +- AND SHALL remove the tenant-namespace credentials Secret + +#### Scenario: Replay a cleanup whose objects are already gone + +- GIVEN a delete event for a Gateway whose external database and role are absent +- WHEN the control plane runs the cleanup +- THEN it SHALL treat both as successfully cleaned up and SHALL NOT return an error + +#### Scenario: Cleanup fails while the external server is unreachable + +- GIVEN a Gateway delete event whose external server is unreachable +- WHEN the control plane attempts cleanup +- THEN it SHALL propagate a contextual error and log the orphaned gateway ID and + ManagedDatabase ID at error level +- AND SHALL NOT retry the cleanup on a later event +- AND the role and database SHALL remain on the external server until an operator + removes them + +#### Scenario: Delete external ManagedDatabase leaves the server untouched + +- GIVEN an `external` ManagedDatabase with no referencing gateways +- WHEN it is deleted +- THEN the control plane SHALL perform no destructive action on the external server +- AND SHALL create and delete no Kubernetes resource, there being none to reclaim + +HyperShell SHALL NEVER drop, resize, or delete the external **server** itself, and +SHALL NEVER delete the `connection_secret` namespace or the credentials Secret inside +it - both are operator-owned. + +The tenant-namespace `openshell-gateway-db-credentials` Secret is removed by the +existing label-based tenant-namespace cleanup. + +--- + +### Requirement: No Credential Rotation (External Mode) + +External mode SHALL NOT implement database credential rotation. The +`hypershell.redhat.io/rotate-db-credentials` annotation SHALL have **no effect** on a +gateway backed by an `external` ManagedDatabase: the GatewayReconciler SHALL NOT +generate a new password, SHALL NOT issue `ALTER ROLE` in response to it, and SHALL +NOT write `hypershell.redhat.io/last-db-rotation`. + +An operator who must change a gateway's database password does so out-of-band on the +external server, or by deleting and recreating the gateway. + +This is a deliberate scope reduction, not an omission. The `ALTER ROLE` in +Requirement: Per-Gateway Database Provisioning is the provisioning repair path for a +lost tenant Secret and SHALL be retained; it is not reachable from the rotation +annotation. + +#### Scenario: Rotation annotation on an external-backed gateway is inert + +- GIVEN a Gateway backed by an `external` ManagedDatabase +- AND a new value for the annotation `hypershell.redhat.io/rotate-db-credentials` +- WHEN the GatewayReconciler processes the event +- THEN it SHALL NOT generate a new password +- AND SHALL NOT alter the role on the external server +- AND the tenant-namespace `openshell-gateway-db-credentials` Secret SHALL be unchanged + +--- + +### Requirement: Database Credential Security (External Mode) + +- Admin credentials are held only for the duration of a reconciliation and never + persisted by HyperShell beyond the operator-owned Secret they were read from. +- Per-gateway passwords SHALL be generated with `crypto/rand` (32-byte hex), + create-or-skip on re-reconciliation, and never logged. +- Passwords SHALL NEVER appear in log messages, error strings, telemetry, or API + responses. +- The connection to the external server SHOULD always be TLS-encrypted (minimum + `sslmode=require`). `sslmode=disable` is insecure and SHOULD NOT be used in + production. The control plane emits a WARN log when `sslmode=disable` is read from + the admin Secret so operators see the misconfiguration without the reconciler + failing. Development and CI environments that use a local PostgreSQL server without + TLS may set `sslmode=disable`; this is explicitly not recommended for any external + server reachable from outside the cluster. + +#### Scenario: Insecure TLS mode is warned about, not rejected + +- GIVEN an `external` ManagedDatabase whose admin Secret sets `sslmode: disable` +- WHEN the ManagedDatabaseReconciler reads it +- THEN it SHALL emit a WARN log naming the ManagedDatabase +- AND SHALL proceed with the connectivity check rather than failing the resource + +--- + +### Requirement: DATABASE_PROVIDER Selection (External) + +`DATABASE_PROVIDER=external` selects external-backed placement and per-gateway DDL. +It requires a reachable external server and valid credentials at **reconcile** time; +it imposes no startup precondition beyond reading the value itself. + +The full set of valid `DATABASE_PROVIDER` values, and the requirement that any other +value is a fatal startup configuration error, is defined once in the parent spec's +Requirement: DATABASE_PROVIDER Selection And Validation and is not restated here. + +#### Scenario: DATABASE_PROVIDER=external selects external placement + +- GIVEN `DATABASE_PROVIDER=external` +- WHEN the API server or control plane starts +- THEN it SHALL select external-backed placement +- AND SHALL start without contacting any external server + +--- + +## Configuration Reference + +| Variable | Type | Default | Description | +|---|---|---|---| +| `DATABASE_PROVIDER` | env var | `deployment` | Set to `external` to select this provider. | + +No new environment variables are introduced. The external server endpoint, +credentials, and TLS material all live in the `hypershell-managed-db-credentials` +Secret inside the namespace named by `connection_secret`. +`OPENSHELL_DATABASE_IMAGE` is not used in external mode, which runs no in-cluster +PostgreSQL workload. + +--- + +## Configuration Examples + +Credentials namespace and Secret (created out-of-band by the operator, before +HyperShell is installed; referenced as +`connection_secret: "hypershell-managed-db-us-east-1"`): + +```yaml +apiVersion: v1 +kind: Namespace +metadata: + name: hypershell-managed-db-us-east-1 +--- +apiVersion: v1 +kind: Secret +metadata: + name: hypershell-managed-db-credentials # fixed name + namespace: hypershell-managed-db-us-east-1 +type: Opaque +stringData: + host: mydb.abc123.us-east-1.rds.amazonaws.com + port: "5432" + dbname: postgres + user: hypershell_admin # rds_superuser / IBM admin; has CREATEDB + CREATEROLE + password: + sslmode: verify-full + sslrootcert: | + -----BEGIN CERTIFICATE----- + ...cloud provider CA bundle... + -----END CERTIFICATE----- +``` + +ManagedDatabase (registration, created via API): + +```json +{ + "name": "rds-us-east-1", + "provider": "external", + "region": "us-east-1", + "engine": "postgres", + "engine_version": "16", + "connection_secret": "hypershell-managed-db-us-east-1" +} +``` + +Per-gateway objects the GatewayReconciler creates on the external server (illustrative SQL): + +```sql +-- role +CREATE ROLE gw_2j5k7m9pqrstvwxyz LOGIN PASSWORD '<32-byte-hex-random>'; +-- database owned by the role +CREATE DATABASE gw_2j5k7m9pqrstvwxyz OWNER gw_2j5k7m9pqrstvwxyz; +-- isolation +REVOKE CONNECT ON DATABASE gw_2j5k7m9pqrstvwxyz FROM PUBLIC; +GRANT CONNECT ON DATABASE gw_2j5k7m9pqrstvwxyz TO gw_2j5k7m9pqrstvwxyz; +``` + +Gateway credentials Secret (tenant namespace): + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: openshell-gateway-db-credentials + namespace: openshell-a1b2c3d4e5f67890 + labels: + hypershell.redhat.io/managed: "true" +type: Opaque +stringData: + host: mydb.abc123.us-east-1.rds.amazonaws.com + port: "5432" + dbname: gw_2j5k7m9pqrstvwxyz + user: gw_2j5k7m9pqrstvwxyz + password: <32-byte-hex-random> + uri: postgresql://gw_2j5k7m9pqrstvwxyz:@mydb.abc123.us-east-1.rds.amazonaws.com:5432/gw_2j5k7m9pqrstvwxyz?sslmode=require +``` + +--- + +## Operator Runbook: identifying and recovering orphaned objects + +Per-gateway cleanup is best-effort and single-shot (see Requirement: Per-Gateway +Cleanup). If the external server was unreachable when a gateway was deleted, its role +and database persist with valid credentials. Detect and recover them as follows. + +1. **Identify orphaned databases** - connect as the admin user and query: + ```sql + SELECT datname FROM pg_database WHERE datname LIKE 'gw\_%'; + ``` + Compare the result to the gateway IDs currently registered in HyperShell. Any + `gw_` database whose gateway ID is no longer in HyperShell is orphaned. + (`_` is a `LIKE` wildcard, hence the backslash escape.) + +2. **Identify orphaned roles** - similarly: + ```sql + SELECT rolname FROM pg_roles WHERE rolname LIKE 'gw\_%'; + ``` + +3. **Terminate, then drop** - for each orphaned object: + ```sql + SELECT pg_terminate_backend(pid) + FROM pg_stat_activity WHERE datname = 'gw_'; + DROP DATABASE "gw_"; + DROP ROLE "gw_"; + ``` + +4. **Tenant Secret** - `openshell-gateway-db-credentials` in the gateway's tenant + namespace is removed by the platform's label-based namespace cleanup when the + gateway namespace is reclaimed. If the namespace was already deleted, the Secret is + gone. If it persists, delete it manually. + +--- + +## Debugging Reference + +| Symptom | Root Cause | Fix | +|---|---|---| +| ManagedDatabase status `Failed: secret_invalid` | `connection_secret` does not carry the `hypershell-managed-db-` prefix, the namespace does not exist, or it holds no `hypershell-managed-db-credentials` Secret | Correct the reference, or create the namespace and Secret with the fixed name | +| ManagedDatabase status `Failed: unreachable` | No network path from control-plane cluster to endpoint | Fix VPC peering / security groups / private endpoint | +| ManagedDatabase status `Failed: auth_failed` | Wrong admin credentials in the credentials Secret | Correct `user`/`password` in `hypershell-managed-db-credentials` | +| ManagedDatabase status `Failed: insufficient_privilege` | Admin role lacks CREATEDB/CREATEROLE | Grant `rds_superuser` (AWS) / admin role (IBM) or the two privileges | +| ManagedDatabase status `Failed: tls_failed` | `sslmode` requires verification but `sslrootcert` is missing, wrong, or does not match the server certificate | Supply the provider's CA bundle as inline PEM, or lower `sslmode` to `require` | +| Gateway create rejected: no eligible external database | No `external` ManagedDatabase is registered | Register an `external` ManagedDatabase | +| New gateways land on an unexpected external server | Placement selects the **first-created** `external` ManagedDatabase, not the most recent | Check registration timestamps; delete the older registration once its gateways are gone | +| Gateway pod cannot connect | TLS mismatch or wrong host in tenant Secret | Verify `sslmode`/CA and `openshell-gateway-db-credentials` | +| `gw_*` database or role left on the server after gateway deletion | Cleanup ran while the server was unreachable; there is no retry | Follow the Operator Runbook above | + +--- + +## Affected Components (Implementation Impact) + +Spec-driven summary of the code surface a `/reconcile` wave would touch (for +planning only): + +- **API server - ManagedDatabase** + - `plugins/managedDatabases/service.go` - add `external` to supported providers; + require `connection_secret` and enforce the reference format (no `/`, reserved + `hypershell-managed-db-` prefix, DNS-1123 label); keep provider immutable. + - **No data-model change.** `provider`, `region`, `engine`, `engine_version`, + `instance_class` and `connection_secret` already exist in `model.go`, + `migration.go`, `openapi.managedDatabases.yaml` and + `proto/hypershell/v1/managed_databases.proto`. No new column, no migration, no + regenerated stubs. + - **Naming debt (known, deferred):** `connection_secret` now holds a *namespace* + name, not a Secret name. The field, the CLI flag `--connection-secret` and the + OpenAPI/proto field keep their existing names to avoid a migration; the + description in each SHALL state that the value is a namespace. +- **API server - Gateway placement** + - `plugins/gateways/provider.go`, `plugin.go`, `placement.go` - accept + `DATABASE_PROVIDER=external`; `externalPlacement` queries `provider=external` + ordered by creation timestamp then ID ascending, takes the first, and rejects + only the empty result. No cluster/region lookup. +- **Control plane** + - Add a PostgreSQL client dependency and an admin-connection helper (per-reconcile, + short-lived, credentials read as `hypershell-managed-db-credentials` from the + `connection_secret` namespace). + - `internal/config/config.go` - accept `external` value. + - `internal/reconciler/reconciler.go` - `ManagedDatabaseReconciler`: `external` + branch = connectivity/capability check creating no Kubernetes resource; + `GatewayReconciler`: external per-gateway DDL, tenant credentials Secret, + unconditional single-shot deletion. + - The Gateway delete watch event must carry `database_id` alongside the gateway ID. + - **Removals:** any external-mode credential-rotation branch, and any use of + ManagedDatabase delete tombstones or a retry queue for external per-gateway + cleanup. The `ALTER ROLE` in the provisioning repair path is retained. +- **Related specs (amended alongside this one)** + - `openshell-gateway-database.spec.md` - external placement description and admin + workflow row. + - `naming-multitenancy.spec.md` - `hypershell-managed-db-` registered as a + reserved **Namespace** prefix, with the fixed `hypershell-managed-db-credentials` + Secret inside it. + - `data-model.spec.md` - `connection_secret` documented as a namespace reference. + - `global-architecture.spec.md` - § Database Strategy already covers `external`. + +--- + +## References + +- Parent: [`openshell-gateway-database.spec.md`](./openshell-gateway-database.spec.md) +- [`security.spec.md`](../standards/security/security.spec.md) - secret references, not inline secrets +- [`naming-multitenancy.spec.md`](../standards/platform/naming-multitenancy.spec.md) - reserved names +- [`control-plane/conventions.spec.md`](../standards/control-plane/conventions.spec.md) - reconciler error handling, no panic +- [AWS RDS PostgreSQL - master user privileges (`rds_superuser`)](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/CHAP_PostgreSQL.html) +- [IBM Cloud Databases for PostgreSQL - administration](https://cloud.ibm.com/docs/databases-for-postgresql) +- [PostgreSQL - `CREATE DATABASE`, `CREATE ROLE`, `GRANT`/`REVOKE`](https://www.postgresql.org/docs/current/sql-createdatabase.html) diff --git a/specs/platform/openshell-gateway-database.spec.md b/specs/platform/openshell-gateway-database.spec.md index f223aaaf..7f635ff8 100644 --- a/specs/platform/openshell-gateway-database.spec.md +++ b/specs/platform/openshell-gateway-database.spec.md @@ -14,8 +14,9 @@ This specification defines PostgreSQL database provisioning for OpenShell gatewa |---|---|---| | **Deployment** (default) | unset/empty or `deployment` | Uses a standalone PostgreSQL Deployment per gateway. No operator required. Each gateway gets its own dedicated ManagedDatabase (and thus its own PostgreSQL pod) created automatically at gateway creation time. Suitable for environments where installing the CNPG operator is not feasible (e.g. minimal dev clusters), and requires no CNPG APIs at all. | | **CNPG** | `cnpg` | Uses the [CloudNativePG](https://cloudnative-pg.io/) operator. Multiple gateways share one ManagedDatabase's CNPG Cluster; each gateway gets its own logical database inside it. Requires the exact CNPG CRDs this code depends on (`Cluster`, `Database`, `DatabaseRole` in `postgresql.cnpg.io/v1`) to be installed on the cluster. | +| **External** | `external` | The PostgreSQL server is provisioned outside HyperShell as a cloud-managed database (AWS RDS/Aurora, IBM Cloud Databases). HyperShell registers the endpoint and provisions one dedicated database and login role per gateway inside it, issuing the DDL itself. No operator and no in-cluster PostgreSQL workload. Specified in full by [`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md). | -Any `DATABASE_PROVIDER` value other than unset/empty, `deployment`, or `cnpg` is a startup configuration error: the API server and control plane SHALL fail to start rather than silently selecting CNPG. +Any `DATABASE_PROVIDER` value other than unset/empty, `deployment`, `cnpg`, or `external` is a startup configuration error: the API server and control plane SHALL fail to start rather than silently selecting a provider. PostgreSQL is the only supported database backend for HyperShell gateways. @@ -91,18 +92,20 @@ In both modes, the ManagedDatabase namespace is derived from the ManagedDatabase ### Automatic Database Assignment -The Gateway create contract keeps `cluster_id` and `database_id`. The API server stores `cluster_id`, but it SHALL NOT resolve, validate, or use it for database placement. The `database_id` value is server-owned. Clients send an empty string, and the API server SHALL ignore and replace any non-empty value. Omitting the property is invalid. The server-side `database_id` placement strategy depends on `DATABASE_PROVIDER`: +The Gateway create contract keeps `cluster_id` and `database_id`. The API server stores `cluster_id` but SHALL NOT resolve, validate, or use it for database placement in any mode. The `database_id` value is server-owned in every mode. Clients send an empty string, and the API server SHALL ignore and replace any non-empty value. Omitting the property is invalid. The server-side `database_id` placement strategy depends on `DATABASE_PROVIDER`: -**CNPG mode (`cnpgPlacement`):** For every Gateway creation, the API server ignores the requested `database_id` and queries all ManagedDatabases. If exactly one ManagedDatabase exists, the API server assigns its ID. If zero or more than one exist, the API server rejects the request. +**CNPG mode (`cnpgPlacement`):** For every Gateway creation, the API server ignores the requested `database_id` and queries all ManagedDatabases with `provider=cnpg`. If exactly one such ManagedDatabase exists, the API server assigns its ID. If zero or more than one exist, the API server rejects the request. **Deployment mode (`deploymentPlacement`):** For every Gateway creation, the API server ignores the requested `database_id`, creates a new ManagedDatabase (provider=deployment) for that gateway, and assigns its ID. No existing ManagedDatabase is necessary. A caller cannot select a database that another gateway uses. +**External mode (`externalPlacement`):** For every Gateway creation, the API server ignores the requested `database_id` and selects, from all ManagedDatabases with `provider=external`, the one **created first** (creation timestamp ascending, ties broken by ID ascending). More than one registered external ManagedDatabase is not an error; zero is rejected. Selection happens only at creation, so a later registration never moves an existing gateway. No ManagedDatabase is created - external servers are registered by an administrator, never provisioned by HyperShell. Specified in full by [`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md). + The admin workflows differ accordingly: -| | CNPG mode | Deployment mode | -|---|---|---| -| Pre-requisite | Create one ManagedDatabase (provider=cnpg) | None | -| Gateway creation | Provide `name`; database auto-resolved; `cluster_id` has no placement effect | Provide `name`; ManagedDatabase auto-created; `cluster_id` has no placement effect | +| | CNPG mode | Deployment mode | External mode | +|---|---|---|---| +| Pre-requisite | Create one ManagedDatabase (provider=cnpg) | None | Provision the server out-of-band, create the `hypershell-managed-db-` namespace and its `hypershell-managed-db-credentials` Secret, and register at least one ManagedDatabase (provider=external) | +| Gateway creation | Provide `name`; database auto-resolved; `cluster_id` has no placement effect | Provide `name`; ManagedDatabase auto-created; `cluster_id` has no placement effect | Provide `name`; first-created external ManagedDatabase auto-selected; `cluster_id` has no placement effect | In the Kind development environment, `make kind-up` seeds a single `openshell-db` ManagedDatabase in CNPG mode; no seeding is needed in deployment mode. @@ -112,7 +115,7 @@ In the Kind development environment, `make kind-up` seeds a single `openshell-db ### Requirement: ManagedDatabase Provider Validation and Immutability -The API server SHALL accept only `cnpg` and `deployment` as ManagedDatabase provider values. Once a ManagedDatabase has a supported provider, that provider is immutable: REST patches, gRPC updates, and internal callers MAY resend the same value but SHALL NOT transition the resource to another provider. Status-only and other mutable-field updates SHALL preserve the provider. A legacy resource whose persisted provider is unsupported MAY be corrected once to a supported provider; after correction, normal immutability applies. +The API server SHALL accept only `cnpg`, `deployment`, and `external` as ManagedDatabase provider values. The `external` provider carries additional create-time validation specified in [`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md). Once a ManagedDatabase has a supported provider, that provider is immutable: REST patches, gRPC updates, and internal callers MAY resend the same value but SHALL NOT transition the resource to another provider. Status-only and other mutable-field updates SHALL preserve the provider. A legacy resource whose persisted provider is unsupported MAY be corrected once to a supported provider; after correction, normal immutability applies. #### Scenario: Attempt to change a supported provider @@ -404,7 +407,7 @@ The `uri` key provides the full connection string for the gateway's `--db-url` a ### Requirement: Manual Credential Rotation -The GatewayReconciler SHALL support manual database credential rotation for CNPG mode gateways. +The GatewayReconciler SHALL support manual database credential rotation for CNPG mode gateways. Deployment and external mode gateways SHALL NOT support rotation; for external mode see [`openshell-gateway-database-external.spec.md`](./openshell-gateway-database-external.spec.md) § Requirement: No Credential Rotation (External Mode). - To trigger rotation, an operator adds the annotation `hypershell.redhat.io/rotate-db-credentials: ""` to the Gateway resource - When the reconciler detects a new value for this annotation (different from the last-observed value stored on the gateway credentials Secret), it SHALL: @@ -493,6 +496,7 @@ Both the API server and the control plane read `DATABASE_PROVIDER` independently - Unset or empty resolves to `deployment` (the default). - `deployment` selects deployment-backed ManagedDatabase placement, which never requires any CNPG API. - `cnpg` selects CNPG-backed placement, subject to the exact-resource startup check below. +- `external` selects external-backed placement and per-gateway DDL against a registered cloud-managed server. It imposes no CNPG startup check; its preconditions (reachability, a valid admin connection Secret) are evaluated at reconcile time, not at startup. - Any other value is a startup configuration error: the process SHALL fail to start (return/propagate a contextual error and exit non-zero -- never panic) rather than silently falling back to `cnpg` or any other provider. #### Scenario: DATABASE_PROVIDER unset defaults to deployment @@ -511,9 +515,9 @@ Both the API server and the control plane read `DATABASE_PROVIDER` independently #### Scenario: Unsupported DATABASE_PROVIDER value fails startup -- GIVEN `DATABASE_PROVIDER` is set to a value other than unset/empty, `deployment`, or `cnpg` (e.g. `postgres`, or a differently-cased `CNPG`) +- GIVEN `DATABASE_PROVIDER` is set to a value other than unset/empty, `deployment`, `cnpg`, or `external` (e.g. `postgres`, or a differently-cased `CNPG`) - WHEN the API server or the control plane starts -- THEN it SHALL fail to start with an explicit, contextual error naming the invalid value and the two supported values +- THEN it SHALL fail to start with an explicit, contextual error naming the invalid value and the supported values - AND it SHALL NOT silently select `cnpg` (or any other provider) as a fallback --- @@ -554,8 +558,8 @@ This startup check is independent of the per-resource CNPG capability detection | Variable | Type | Default | Description | |---|---|---|---| -| `DATABASE_PROVIDER` | env var | `deployment` (unset/empty resolves to it) | Selects the database deployment mode. Valid values: `deployment`, `cnpg`. Any other value is a startup configuration error. | -| `OPENSHELL_DATABASE_IMAGE` | env var | (unset) | PostgreSQL image override. In CNPG mode: sets `spec.imageName` on the CNPG Cluster CR. In deployment mode: sets the container image for the PostgreSQL Deployment. When unset, CNPG uses its default image; deployment mode uses `postgres:18`. | +| `DATABASE_PROVIDER` | env var | `deployment` (unset/empty resolves to it) | Selects the database deployment mode. Valid values: `deployment`, `cnpg`, `external`. Any other value is a startup configuration error. | +| `OPENSHELL_DATABASE_IMAGE` | env var | (unset) | PostgreSQL image override. In CNPG mode: sets `spec.imageName` on the CNPG Cluster CR. In deployment mode: sets the container image for the PostgreSQL Deployment. When unset, CNPG uses its default image; deployment mode uses `postgres:18`. Not used in external mode, which runs no in-cluster PostgreSQL workload. | > **Removed:** `CNPG_CLUSTER_NAME` and `CNPG_CLUSTER_NAMESPACE` environment variables are no longer used. The database location is derived per-gateway from the ManagedDatabase resource referenced by the gateway's `database_id`. @@ -816,10 +820,10 @@ stringData: |---|---|---|---| | Database CR `status.applied: false` | cnpg | CNPG operator not running or Cluster not ready | Check CNPG operator pods and Cluster status | | DatabaseRole stuck in `Terminating` | cnpg | Role owns objects that prevent DROP | Manually drop owned objects or delete database first | -| Gateway pod cannot connect to database | both | Credentials Secret not created or wrong host | Verify `openshell-gateway-db-credentials` in tenant namespace | -| ManagedDatabase namespace not found | both | ManagedDatabaseReconciler has not yet processed the resource | Check ManagedDatabase status and reconciler logs | +| Gateway pod cannot connect to database | all | Credentials Secret not created or wrong host | Verify `openshell-gateway-db-credentials` in tenant namespace | +| ManagedDatabase namespace not found | deployment, cnpg | ManagedDatabaseReconciler has not yet processed the resource | Check ManagedDatabase status and reconciler logs | | Control plane exits at startup with "DATABASE_PROVIDER=cnpg requires..." | cnpg | `DATABASE_PROVIDER=cnpg` but the CNPG operator (or one of the `clusters`/`databases`/`databaseroles` resources) is not installed | Install/upgrade the CloudNativePG operator, or switch to `DATABASE_PROVIDER=deployment` (also the default when unset) | -| API server or control plane exits at startup with "invalid DATABASE_PROVIDER" | both | `DATABASE_PROVIDER` set to a value other than unset/empty, `deployment`, or `cnpg` | Set `DATABASE_PROVIDER` to `deployment` or `cnpg`, or unset it | +| API server or control plane exits at startup with "invalid DATABASE_PROVIDER" | all | `DATABASE_PROVIDER` set to a value other than unset/empty, `deployment`, `cnpg`, or `external` | Set `DATABASE_PROVIDER` to `deployment`, `cnpg`, or `external`, or unset it | | PostgreSQL Deployment not ready | deployment | Image pull failure or PVC not bound | Check Deployment events and PVC status in ManagedDatabase namespace | | `openshell-db-credentials` Secret missing | deployment | ManagedDatabaseReconciler has not yet completed | Check ManagedDatabase status and reconciler logs | | Password rotation not applied | cnpg | Missing `cnpg.io/reload: "true"` label on password Secret | Add the label to the Secret | diff --git a/specs/standards/platform/naming-multitenancy.spec.md b/specs/standards/platform/naming-multitenancy.spec.md index 11432dfd..8f325698 100644 --- a/specs/standards/platform/naming-multitenancy.spec.md +++ b/specs/standards/platform/naming-multitenancy.spec.md @@ -158,6 +158,11 @@ require per-instance control-plane isolation are deployed once per cluster: │ ├── Cluster: hypershell-db ──> Secret: hypershell-db-app │ └── Deployment: hypershell-api-server, hypershell-controller, ... │ + ├── External DB credential namespaces (created by the operator out-of-band - §6.2) + │ └── Namespace: hypershell-managed-db-us-east-1 (reserved prefix) + │ └── Secret: hypershell-managed-db-credentials (fixed name; the only Secret read here) + │ ▲ referenced by ManagedDatabase.connection_secret (provider: external) + │ ├── ManagedDatabase namespaces (created by the control plane - §4) │ └── Namespace: openshell-db-9f3c1a2b7e5d4068 │ ├── Cluster: openshell-db (shared CNPG instance) @@ -189,6 +194,8 @@ require per-instance control-plane isolation are deployed once per cluster: | `hypershell-controller` | ServiceAccount | Control Plane Instance | Runs the controller; subject of the prefixed ClusterRoleBinding. | | `hypershell-db` | Cluster (CNPG) | Control Plane Instance | Platform DB. **No suffix**; namespace isolates it (§1). | | `hypershell-db-app` | Secret | Control Plane Instance | Auto-generated by CNPG; hardcoded in volume mounts. Must not be renamed. | +| `hypershell-managed-db-` | Namespace | Shared Platform | **Reserved prefix.** Operator-created namespace holding the administrative connection credentials for a registered external PostgreSQL server (`ManagedDatabase` with `provider: external`). A `ManagedDatabase.connection_secret` reference is a bare **namespace** name - never `namespace/name` - that MUST carry this prefix and MUST be a valid DNS-1123 label. Provisioned out-of-band, normally before HyperShell itself, so the credentials do not depend on the control plane instance namespace existing. Not created, modified or deleted by HyperShell. See [`openshell-gateway-database-external.spec.md`](../../platform/openshell-gateway-database-external.spec.md). | +| `hypershell-managed-db-credentials` | Secret | Shared Platform | **Fixed name.** The only Secret HyperShell reads inside a `hypershell-managed-db-` namespace: admin `host`/`port`/`user`/`password` (+ optional `dbname`, `sslmode`, `sslrootcert`) for the external server. The reserved namespace prefix combined with this fixed name is a security boundary, not a convention: together they stop an API-level reference from causing the control plane to read an unrelated Secret such as `hypershell-db-app`. No Secret holding anything other than external database admin credentials may take this name in such a namespace. | | `hypershell-api-server`, `hypershell-controller`, `hypershell-web-console` | Deployment | Control Plane Instance | Core components; no suffix (§1). | | `openshell-db-` | Namespace | ManagedDatabase | Dedicated namespace for a ManagedDatabase's CNPG instance; distinct from gateway namespaces (§4). | | `openshell-db` | Cluster (CNPG) | ManagedDatabase | Shared Postgres instance inside `openshell-db-`; may back many gateways. | diff --git a/specs/web-console/architecture.spec.md b/specs/web-console/architecture.spec.md index 2bfb6dfa..ca27728a 100644 --- a/specs/web-console/architecture.spec.md +++ b/specs/web-console/architecture.spec.md @@ -494,7 +494,7 @@ The selected tab SHALL use the validated `tab` search parameter per `WEB-DATA-02 The `Details` tab SHALL contain the existing gateway description list (status, cluster, endpoint, CLI connection command, namespace, release identifier, and managed-database identifier), using the same values, loading, unavailable, and placement-resolution behavior required elsewhere in this specification. -The `Connection` tab SHALL guide the user through four ordered steps using an accessible ordered structure with a visible label and description for each step: +The `Connection` tab SHALL guide the user through three ordered steps using an accessible ordered structure with a visible label and description for each step: 1. **Log in to the gateway.** The step SHALL present the `openshell gateway add` command produced from the authorized gateway response as a read-only PatternFly Clipboard Copy value, using the same command construction, shell-argument encoding, and missing-value handling required by `WEB-UI-03`. When required connection values are absent, the step SHALL explain that login is unavailable rather than present an incomplete command. @@ -502,8 +502,6 @@ The `Connection` tab SHALL guide the user through four ordered steps using an ac 3. **Create a sandbox.** The step SHALL present an `openshell sandbox create` command that names a sandbox, specifies CPU and memory resource requests and limits via a `--driver-config-json` flag, attaches the provider from step 2, and launches the agent, as a read-only Clipboard Copy value. The default resource values SHALL be CPU requests `100m`, CPU limits `500m`, memory requests `512Mi`, and memory limits `512Mi`. -4. **Connect to a sandbox.** The step SHALL present an `openshell sandbox connect` command that references the sandbox name from step 3 and an `--editor` flag. The sandbox name field SHALL share state with the create step so both commands stay in sync. The editor field SHALL be a constrained inline selector defaulting to `cursor`, offering only the supported values (`cursor`, `vscode`). An adjacent info alert SHALL document the supported values and link to the sandbox connect documentation on the canonical docs host. - The `Connection` tab SHALL also contain a secondary `Create or manage service accounts` link with its introductory content, before the walkthrough. The link SHALL set `tab=service-accounts`. It SHALL not start a create operation. The `Service accounts` tab SHALL own the gateway-scoped automation identity workflow. It SHALL include the heading, create action, responsive collection, one-time credential handoff, command groups, and lifecycle actions defined in [`openshell-gateway-service-accounts.spec.md`](../platform/openshell-gateway-service-accounts.spec.md). @@ -523,7 +521,7 @@ Every Clipboard Copy control SHALL have a localized accessible name and visible - GIVEN an authenticated user opens a gateway detail page - WHEN the page renders - THEN the `Connection` tab SHALL be selected -- AND the four ordered connection steps SHALL be visible +- AND the three ordered connection steps SHALL be visible - AND the `Service accounts` tab SHALL be available - AND the operational description list SHALL be available under the `Details` tab diff --git a/tests/e2e/e2e-openshell.sh b/tests/e2e/e2e-openshell.sh index ecaf4c8c..a61eb0b5 100755 --- a/tests/e2e/e2e-openshell.sh +++ b/tests/e2e/e2e-openshell.sh @@ -19,14 +19,14 @@ # Environment variables: # E2E_INFRA_DRIVER Infra driver override: kind, openshift (default: auto-detected) # E2E_NAMESPACE Namespace for e2e resources (default: openshell-e2e) -# E2E_GATEWAY_NAME Gateway name (default: e2e-gw) +# E2E_GATEWAY_NAME Gateway name (default: e2e-gw-, unique per run) # E2E_MODE Run depth: long (default, every step) or short (essential steps) # E2E_SANDBOX_TIMEOUT Seconds to wait for sandbox (default: 120) # E2E_PROVISION_TIMEOUT Seconds to wait for gateway provisioning (default: 180) # E2E_GC_TIMEOUT Seconds to wait for namespace GC after delete (default: 180) # E2E_ORPHAN_GC_TIMEOUT Seconds to wait for periodic orphan namespace GC (default: 90) # E2E_SKIP_CLEANUP Set to 1 to keep test resources after run (default: 0) -# DATABASE_PROVIDER Database provider: deployment or cnpg (default: deployment) +# DATABASE_PROVIDER Database provider: deployment, cnpg, or external (default: deployment) # E2E_CNPG_NAMESPACE Namespace where the CNPG operator runs (default: cnpg-system) # OPENSHELL_BIN Path to the openshell CLI binary (default: openshell) set -euo pipefail @@ -42,6 +42,7 @@ source "${SCRIPT_DIR}/lib.sh" # default: unset/empty DATABASE_PROVIDER means deployment, see # specs/platform/openshell-gateway-database.spec.md) # cnpg = CloudNativePG operator (CRDs: Cluster, Database, DatabaseRole) +# external = pre-existing server outside the cluster; no in-cluster DB workload DB_PROVIDER="${DATABASE_PROVIDER:-deployment}" # --- Driver selection and validation --- @@ -267,7 +268,7 @@ if [[ "${DB_PROVIDER}" == "cnpg" ]]; then fail_test "CloudNativePG CRDs not found" fi else - dim " CNPG checks skipped (DATABASE_PROVIDER=deployment)" + dim " CNPG checks skipped (DATABASE_PROVIDER=${DB_PROVIDER})" fi show_cmd "$CLI get deployment agent-sandbox-controller -n agent-sandbox-system" @@ -637,6 +638,10 @@ if [[ "${DB_PROVIDER}" == "cnpg" ]]; then else fail_test "Client TLS secret not found" fi +elif [[ "${DB_PROVIDER}" == "external" ]]; then + # External provider: no in-cluster database workload. The sole check is the + # gateway-namespace credentials secret, verified below for all providers. + dim " External database: no in-cluster DB deployment to verify" else # Deployment provider: verify DB Deployment readiness and credentials secret show_cmd "$CLI get deployment openshell-gateway-db -n ${DB_GW_NAMESPACE}" diff --git a/tests/e2e/lib.sh b/tests/e2e/lib.sh index 709423c4..d47676b1 100755 --- a/tests/e2e/lib.sh +++ b/tests/e2e/lib.sh @@ -92,7 +92,7 @@ retry_until() { # --- Environment defaults --- : "${E2E_NAMESPACE:=openshell-e2e}" -: "${E2E_GATEWAY_NAME:=e2e-gw}" +: "${E2E_GATEWAY_NAME:=e2e-gw-$(head -c4 /dev/urandom | od -An -tx1 | tr -d ' \n')}" : "${E2E_MODE:=long}" : "${E2E_SANDBOX_TIMEOUT:=120}" : "${E2E_PROVISION_TIMEOUT:=180}"