diff --git a/docs/services/index.md b/docs/services/index.md index 5267e180..e5760334 100644 --- a/docs/services/index.md +++ b/docs/services/index.md @@ -83,15 +83,15 @@ creating one instance per host for redundancy: ## Database Credentials -Each service instance is automatically provisioned with two dedicated -database users. The Control Plane manages these credentials; you do not -need to create or rotate them manually. The credentials are: - -- `svc_{service_id}_ro` is a read-only user with read access to the - database; this user is the default for most service types. -- `svc_{service_id}_rw` is a read-write user with read and write access - to the database; this user is provisioned when the service needs - read/write access. +Each service connects to the database as a user you specify with the +`connect_as` field. The `connect_as` value must reference a username +in your `database_users` array. The Control Plane uses those +credentials to generate the service's connection string and to configure +any required role grants (for example, granting the anonymous role to +a PostgREST authenticator). + +You own and manage the `connect_as` user. Removing a service does not +drop the underlying database user. ## Next Steps diff --git a/docs/services/managing.md b/docs/services/managing.md index 8881e12e..2c4c343c 100644 --- a/docs/services/managing.md +++ b/docs/services/managing.md @@ -12,7 +12,7 @@ The following table describes the fields in a service spec: | Field | Type | Required | Description | |-------|------|----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `service_id` | string | Yes | A unique identifier for this service within the database. Used in credential names (`svc_{service_id}_ro` / `svc_{service_id}_rw`). | +| `service_id` | string | Yes | A unique identifier for this service within the database. | | `service_type` | string | Yes | The type of service to run. One of: `mcp`, `rag`, `postgrest`. | | `version` | string | Yes | The service version in semver format (e.g., `1.0.0`) or the literal `latest`. | | `host_ids` | array | Yes | The IDs of the hosts to run this service on. One instance is created per host. | @@ -20,6 +20,7 @@ The following table describes the fields in a service spec: | `port` | integer | No | Host port to publish the service on. Set to `0` to let Docker assign a random port. When omitted, the service is not reachable from outside the Docker network. | | `cpus` | string | No | CPU limit for the service container. Accepts a decimal (e.g., `"0.5"`) or millicpu suffix (e.g., `"500m"`). Defaults to container defaults if unspecified. | | `memory` | string | No | Memory limit for the service container in SI or IEC notation (e.g., `"512M"`, `"1GiB"`). Defaults to container defaults if unspecified. | +| `connect_as` | string | Yes | Username of the `database_users` entry the service connects to Postgres as. Must exist in `database_users`. | | `database_connection` | object | No | Optional routing configuration for how the service connects to the database. See [Database Connection Routing](#database-connection-routing). | ## Adding a Service @@ -82,6 +83,14 @@ database with a PostgREST service instance. The service exposes the "nodes": [ { "name": "n1", "host_ids": ["host-1"] } ], + "database_users": [ + { + "username": "app", + "password": "changeme", + "db_owner": true, + "attributes": ["LOGIN"] + } + ], "services": [ { "service_id": "api", @@ -89,6 +98,7 @@ database with a PostgREST service instance. The service exposes the "version": "latest", "host_ids": ["host-1"], "port": 3100, + "connect_as": "app", "config": { "jwt_secret": "a-secret-key-of-at-least-32-characters" } @@ -157,8 +167,9 @@ service instances for that service and revokes its database credentials. Removing a service is irreversible. The Control Plane deletes all service instances, their configuration, and their data directories. - Database credentials for the service are revoked. Any clients - connected to the service lose access immediately. + Any clients connected to the service lose access immediately. + The `connect_as` database user is **not** dropped — it is + customer-managed and may be shared with other services or applications. ## Checking Service Status diff --git a/docs/services/postgrest.md b/docs/services/postgrest.md index 9d6a5831..65e2ca7a 100644 --- a/docs/services/postgrest.md +++ b/docs/services/postgrest.md @@ -8,11 +8,11 @@ on every deploy. ## Overview The Control Plane provisions a PostgREST container on each host you -specify. The container connects to the database using -automatically-managed credentials and serves HTTP at your configured -port. Anonymous requests run as the configured `db_anon_role`. -JWT-authenticated requests switch to any role granted to the -authenticator. +specify. The container connects to the database as the user specified +in `connect_as` (a `database_users` entry you control) and serves HTTP +at your configured port. Anonymous requests run as the configured +`db_anon_role`. JWT-authenticated requests switch to any role granted +to the `connect_as` user. See [Managing Services](managing.md) for instructions on adding, updating, and removing services. The sections below cover @@ -54,7 +54,8 @@ using a signed token. Omit these fields to run in anonymous-only mode. ### Read-Only API (No JWT) This example provisions a PostgREST service with default settings. All -requests run as the anonymous role. +requests run as the anonymous role. The `connect_as` field names the +`database_users` entry PostgREST connects to Postgres as. === "curl" @@ -68,6 +69,14 @@ requests run as the anonymous role. "nodes": [ { "name": "n1", "host_ids": ["host-1"] } ], + "database_users": [ + { + "username": "app", + "password": "changeme", + "db_owner": true, + "attributes": ["LOGIN"] + } + ], "services": [ { "service_id": "api", @@ -75,6 +84,7 @@ requests run as the anonymous role. "version": "latest", "host_ids": ["host-1"], "port": 3100, + "connect_as": "app", "config": {} } ] @@ -85,7 +95,8 @@ requests run as the anonymous role. ### JWT-Authenticated API This example enables JWT authentication. Clients send a signed token to -switch to a specific PostgreSQL role. +switch to a specific PostgreSQL role. Every role a JWT can claim must be +granted to the `connect_as` user. === "curl" @@ -99,6 +110,14 @@ switch to a specific PostgreSQL role. "nodes": [ { "name": "n1", "host_ids": ["host-1"] } ], + "database_users": [ + { + "username": "app", + "password": "changeme", + "db_owner": true, + "attributes": ["LOGIN"] + } + ], "services": [ { "service_id": "api", @@ -106,6 +125,7 @@ switch to a specific PostgreSQL role. "version": "latest", "host_ids": ["host-1"], "port": 3100, + "connect_as": "app", "config": { "jwt_secret": "a-secret-key-of-at-least-32-characters" } @@ -132,6 +152,14 @@ before deploying. "nodes": [ { "name": "n1", "host_ids": ["host-1"] } ], + "database_users": [ + { + "username": "app", + "password": "changeme", + "db_owner": true, + "attributes": ["LOGIN"] + } + ], "services": [ { "service_id": "api", @@ -139,6 +167,7 @@ before deploying. "version": "latest", "host_ids": ["host-1"], "port": 3100, + "connect_as": "app", "config": { "db_schemas": "public,api", "jwt_secret": "a-secret-key-of-at-least-32-characters" @@ -198,10 +227,9 @@ the PostgreSQL role PostgREST uses for the request. --data '{"name": "Widget", "price": 9.99}' ``` -The `role` claim must name a PostgreSQL role granted to the PostgREST -authenticator user. The authenticator username is visible in the -`service_instances` field of your database status response. Grant your -application roles to that user before sending authenticated requests. +The `role` claim must name a PostgreSQL role granted to the `connect_as` +user. Grant your application roles to the `connect_as` user in +PostgreSQL before sending authenticated requests. ## Preflight Checks diff --git a/e2e/postgrest_test.go b/e2e/postgrest_test.go index ddb6c5ac..b18e9205 100644 --- a/e2e/postgrest_test.go +++ b/e2e/postgrest_test.go @@ -16,6 +16,7 @@ import ( ) // postgrestSpec returns a ServiceSpec for a PostgREST service on the given host. +// PostgREST connects as the "admin" database user (connect_as). func postgrestSpec(hostID string, port int, config map[string]any) *controlplane.ServiceSpec { if config == nil { config = map[string]any{} @@ -27,11 +28,13 @@ func postgrestSpec(hostID string, port int, config map[string]any) *controlplane HostIds: []controlplane.Identifier{controlplane.Identifier(hostID)}, Port: pointerTo(port), Config: config, + ConnectAs: "admin", } } // postgrestBaseSpec returns the common database spec used across PostgREST tests. // services is appended directly so callers control what services are included. +// The "admin" user serves as the PostgREST connect_as user. func postgrestBaseSpec(dbName string, nodeHosts []string, services []*controlplane.ServiceSpec) *controlplane.DatabaseSpec { nodes := make([]*controlplane.DatabaseNodeSpec, len(nodeHosts)) for i, h := range nodeHosts { @@ -262,9 +265,11 @@ func TestPostgRESTHealthCheck(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode, "PostgREST root endpoint should return 200") } -// TestPostgRESTServiceUserRoles verifies the CP created the correct Postgres -// roles for the PostgREST authenticator. -func TestPostgRESTServiceUserRoles(t *testing.T) { +// TestPostgRESTAuthenticatorRole verifies the connect_as user was configured +// correctly as a PostgREST authenticator by PostgRESTAuthenticatorResource: +// NOINHERIT must be set and the db_anon_role must be granted to it. +// No svc_* auto-created roles should exist for PostgREST. +func TestPostgRESTAuthenticatorRole(t *testing.T) { t.Parallel() fixture.SkipIfServicesUnsupported(t) @@ -294,29 +299,15 @@ func TestPostgRESTServiceUserRoles(t *testing.T) { require.NoError(t, err) defer conn.Close(ctx) - // The RW service user must have NOINHERIT (rolinherit = false). - rows, err := conn.Query(ctx, ` - SELECT rolname, rolinherit - FROM pg_roles - WHERE rolname LIKE 'svc_%' - AND rolname LIKE '%_rw' - ORDER BY rolname - `) - require.NoError(t, err) - defer rows.Close() - - found := false - for rows.Next() { - var rolname string - var rolinherit bool - require.NoError(t, rows.Scan(&rolname, &rolinherit)) - assert.False(t, rolinherit, "RW service role %s must have NOINHERIT (rolinherit = false)", rolname) - found = true - t.Logf("role %s: rolinherit=%v", rolname, rolinherit) - } - assert.True(t, found, "expected at least one _rw service role") + // The connect_as user ("admin") must have NOINHERIT set by the authenticator resource. + var rolinherit bool + err = conn.QueryRow(ctx, + `SELECT rolinherit FROM pg_roles WHERE rolname = 'admin'`, + ).Scan(&rolinherit) + require.NoError(t, err, "connect_as user 'admin' must exist in pg_roles") + assert.False(t, rolinherit, "connect_as user must have NOINHERIT (rolinherit = false)") - // The RW role must be granted the anon role (pgedge_application_read_only). + // The db_anon_role (pgedge_application_read_only by default) must be granted to the connect_as user. var anonGranted bool err = conn.QueryRow(ctx, ` SELECT EXISTS ( @@ -324,12 +315,20 @@ func TestPostgRESTServiceUserRoles(t *testing.T) { FROM pg_auth_members m JOIN pg_roles r ON m.member = r.oid JOIN pg_roles g ON m.roleid = g.oid - WHERE r.rolname LIKE 'svc_%_rw' + WHERE r.rolname = 'admin' AND g.rolname = 'pgedge_application_read_only' ) `).Scan(&anonGranted) require.NoError(t, err) - assert.True(t, anonGranted, "RW service role must be granted pgedge_application_read_only") + assert.True(t, anonGranted, "connect_as user must be granted the db_anon_role") + + // No auto-created svc_* roles should exist for PostgREST. + var svcRoleCount int + err = conn.QueryRow(ctx, + `SELECT COUNT(*) FROM pg_roles WHERE rolname LIKE 'svc_%'`, + ).Scan(&svcRoleCount) + require.NoError(t, err) + assert.Equal(t, 0, svcRoleCount, "no svc_* roles should be created for PostgREST with connect_as") } // TestPostgRESTAddToExistingDatabase verifies PostgREST provisions correctly @@ -401,7 +400,8 @@ func TestPostgRESTRemove(t *testing.T) { assert.Empty(t, db.ServiceInstances, "service instances should be empty after removal") - // Verify the service user roles are dropped from Postgres. + // Verify the connect_as user still exists after removal — it is a + // customer-managed database_users entry, not an auto-created service role. conn, err := db.ConnectToInstance(ctx, ConnectionOptions{ Matcher: And(WithNode("n1"), WithRole("primary")), Username: "admin", @@ -410,12 +410,12 @@ func TestPostgRESTRemove(t *testing.T) { require.NoError(t, err) defer conn.Close(ctx) - var count int - err = conn.QueryRow(ctx, ` - SELECT COUNT(*) FROM pg_roles WHERE rolname LIKE 'svc_%' - `).Scan(&count) + var exists bool + err = conn.QueryRow(ctx, + `SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'admin')`, + ).Scan(&exists) require.NoError(t, err) - assert.Equal(t, 0, count, "all service roles should be dropped after PostgREST removal") + assert.True(t, exists, "connect_as user must not be dropped when PostgREST is removed") } // TestPostgRESTConfigUpdate verifies updating PostgREST config updates the @@ -505,33 +505,24 @@ func TestPostgRESTMultiHostDBURI(t *testing.T) { waitForPostgRESTRunning(ctx, t, db, "postgrest-api", host1, 5*time.Minute) - // Connect to Postgres and confirm service roles exist on all nodes. - for _, nodeName := range []string{"n1"} { - conn, err := db.ConnectToInstance(ctx, ConnectionOptions{ - Matcher: And(WithNode(nodeName), WithRole("primary")), - Username: "admin", - Password: "testpassword", - }) - if err != nil { - // The primary moved — find it on whichever node is primary. - conn, err = db.ConnectToInstance(ctx, ConnectionOptions{ - Matcher: WithRole("primary"), - Username: "admin", - Password: "testpassword", - }) - } - require.NoError(t, err, "failed to connect to primary on node %s", nodeName) - defer conn.Close(ctx) - - var count int - err = conn.QueryRow(ctx, ` - SELECT COUNT(*) FROM pg_roles WHERE rolname LIKE 'svc_%_rw' - `).Scan(&count) - require.NoError(t, err) - assert.GreaterOrEqual(t, count, 1, "RW service role must exist on node %s", nodeName) - } + // Connect to Postgres and confirm the connect_as user was configured + // as an authenticator (NOINHERIT + anon role granted). + conn, err := db.ConnectToInstance(ctx, ConnectionOptions{ + Matcher: WithRole("primary"), + Username: "admin", + Password: "testpassword", + }) + require.NoError(t, err, "failed to connect to primary") + defer conn.Close(ctx) + + var rolinherit bool + err = conn.QueryRow(ctx, + `SELECT rolinherit FROM pg_roles WHERE rolname = 'admin'`, + ).Scan(&rolinherit) + require.NoError(t, err) + assert.False(t, rolinherit, "connect_as user must have NOINHERIT on multi-host deployment") - t.Log("multi-host PostgREST provisioned, service roles present on all nodes") + t.Log("multi-host PostgREST provisioned, connect_as user configured as authenticator") } // TestPostgRESTFailover provisions PostgREST on a 3-node database, triggers a diff --git a/server/internal/api/apiv1/validate.go b/server/internal/api/apiv1/validate.go index f324bed5..9323f039 100644 --- a/server/internal/api/apiv1/validate.go +++ b/server/internal/api/apiv1/validate.go @@ -361,8 +361,8 @@ func validateServiceSpec(svc *api.ServiceSpec, path []string, isUpdate bool, dbU } // Validate connect_as references a valid database_users entry. - // MCP and RAG both require connect_as; PostgREST will adopt it in a future change. - if svc.ServiceType == "mcp" || svc.ServiceType == "rag" { + // Required for MCP, PostgREST, and RAG — all three use connect_as credentials. + if svc.ServiceType == "mcp" || svc.ServiceType == "postgrest" || svc.ServiceType == "rag" { errs = append(errs, validateConnectAs(svc, dbUsers, path)...) } diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index 767cd6b2..ffc37d11 100644 --- a/server/internal/api/apiv1/validate_test.go +++ b/server/internal/api/apiv1/validate_test.go @@ -1284,9 +1284,10 @@ func TestValidateDatabaseSpec(t *testing.T) { func TestValidateServiceSpec(t *testing.T) { for _, tc := range []struct { - name string - svc *api.ServiceSpec - expected []string + name string + svc *api.ServiceSpec + expected []string + noDefaultConnectAs bool // skip auto-filling ConnectAs so missing-connect_as cases work }{ { name: "valid MCP service with Anthropic", @@ -1508,6 +1509,34 @@ func TestValidateServiceSpec(t *testing.T) { "config: jwt_secret must be at least 32 characters", }, }, + { + name: "postgrest missing connect_as", + svc: &api.ServiceSpec{ + ServiceID: "my-postgrest", + ServiceType: "postgrest", + Version: "latest", + HostIds: []api.Identifier{"host-1"}, + // ConnectAs intentionally left empty + }, + noDefaultConnectAs: true, + expected: []string{ + "connect_as: connect_as is required", + }, + }, + { + name: "postgrest connect_as references unknown user", + svc: &api.ServiceSpec{ + ServiceID: "my-postgrest", + ServiceType: "postgrest", + Version: "latest", + HostIds: []api.Identifier{"host-1"}, + ConnectAs: "nonexistent", + }, + noDefaultConnectAs: true, + expected: []string{ + `connect_as: connect_as "nonexistent" does not match any database_users entry`, + }, + }, { name: "invalid version format", svc: &api.ServiceSpec{ @@ -1826,8 +1855,9 @@ func TestValidateServiceSpec(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - // Default connect_as to "app" for tests that don't set it - if tc.svc.ConnectAs == "" { + // Default connect_as to "app" for tests that don't set it, + // unless noDefaultConnectAs is true (used to test missing connect_as). + if tc.svc.ConnectAs == "" && !tc.noDefaultConnectAs { tc.svc.ConnectAs = "app" } testDBUsers := []*api.DatabaseUserSpec{ diff --git a/server/internal/api/middleware.go b/server/internal/api/middleware.go index 9dc08949..a16ce021 100644 --- a/server/internal/api/middleware.go +++ b/server/internal/api/middleware.go @@ -15,8 +15,10 @@ func addMiddleware(logger zerolog.Logger, next http.Handler) http.Handler { var evt *zerolog.Event switch { - case status >= 400 && status <= 599: + case status >= 500: evt = log.Error() + case status >= 400: + evt = log.Warn() case r.URL.Path == "/v1/version": // The version endpoint is used for health checks evt = log.Debug() diff --git a/server/internal/database/postgrest_service_config_test.go b/server/internal/database/postgrest_service_config_test.go index b2cbe7ed..3914a4e4 100644 --- a/server/internal/database/postgrest_service_config_test.go +++ b/server/internal/database/postgrest_service_config_test.go @@ -35,7 +35,7 @@ func parseConf(t *testing.T, data []byte) map[string]string { func makeTestConn() database.PostgRESTConnParams { return database.PostgRESTConnParams{ - Username: "svc_pgrest", + Username: "myapp", Password: "testpass", DatabaseName: "mydb", DatabaseHosts: []database.ServiceHostEntry{{Host: "pg-host1", Port: 5432}}, @@ -121,7 +121,7 @@ func TestGenerateConf_DBURIContainsCredentials(t *testing.T) { MaxRows: 1000, } conn := database.PostgRESTConnParams{ - Username: "svc_pgrest", + Username: "myapp", Password: "s3cr3t", DatabaseName: "mydb", DatabaseHosts: []database.ServiceHostEntry{{Host: "pg-host1", Port: 5432}}, @@ -131,7 +131,7 @@ func TestGenerateConf_DBURIContainsCredentials(t *testing.T) { m := parseConf(t, data) uri, ok := m["db-uri"] require.True(t, ok, "db-uri must be present in postgrest.conf") - assert.Contains(t, uri, "svc_pgrest") + assert.Contains(t, uri, "myapp") assert.Contains(t, uri, "s3cr3t") assert.Contains(t, uri, "pg-host1") assert.Contains(t, uri, "mydb") @@ -145,7 +145,7 @@ func TestGenerateConf_DBURIMultiHost(t *testing.T) { MaxRows: 1000, } conn := database.PostgRESTConnParams{ - Username: "svc_pgrest", + Username: "myapp", Password: "pass", DatabaseName: "mydb", DatabaseHosts: []database.ServiceHostEntry{ diff --git a/server/internal/orchestrator/swarm/orchestrator.go b/server/internal/orchestrator/swarm/orchestrator.go index fd06dfd2..b7878557 100644 --- a/server/internal/orchestrator/swarm/orchestrator.go +++ b/server/internal/orchestrator/swarm/orchestrator.go @@ -446,11 +446,11 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan } // Service user role resources (manages database user lifecycle). - // MCP uses connect_as credentials from database_users — no ServiceUserRole needed. - // PostgREST/RAG still uses ServiceUserRole until it adopts connect_as. + // MCP and PostgREST use connect_as credentials — no ServiceUserRole needed. + // RAG still uses ServiceUserRole. var canonicalROID, canonicalRWID resource.Identifier var serviceUserRoleRO, serviceUserRoleRW *ServiceUserRole - if spec.ServiceSpec.ServiceType != "mcp" { + if spec.ServiceSpec.ServiceType == "rag" { canonicalROID = ServiceUserRoleIdentifier(spec.ServiceSpec.ServiceID, ServiceUserRoleRO) canonicalRWID = ServiceUserRoleIdentifier(spec.ServiceSpec.ServiceID, ServiceUserRoleRW) serviceUserRoleRO = &ServiceUserRole{ @@ -524,12 +524,12 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan DBAnonRole: postgrestConfig.DBAnonRole, } authenticator := &PostgRESTAuthenticatorResource{ - ServiceID: spec.ServiceSpec.ServiceID, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: spec.NodeName, - DBAnonRole: postgrestConfig.DBAnonRole, - UserRoleID: canonicalRWID, + ServiceID: spec.ServiceSpec.ServiceID, + DatabaseID: spec.DatabaseID, + DatabaseName: spec.DatabaseName, + NodeName: spec.NodeName, + DBAnonRole: postgrestConfig.DBAnonRole, + ConnectAsUsername: spec.ConnectAsUsername, } dataDir := &filesystem.DirResource{ ID: dataDirID, @@ -544,6 +544,8 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan HostID: spec.HostID, DirResourceID: dataDirID, Config: postgrestConfig, + ConnectAsUsername: spec.ConnectAsUsername, + ConnectAsPassword: spec.ConnectAsPassword, DatabaseName: spec.DatabaseName, DatabaseHosts: spec.DatabaseHosts, TargetSessionAttrs: spec.TargetSessionAttrs, @@ -590,16 +592,14 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan orchestratorResources = append(orchestratorResources, serviceSpecificResources...) orchestratorResources = append(orchestratorResources, serviceInstanceSpec, serviceInstance) - // Append per-node ServiceUserRole resources for each additional database node. - // MCP does not use ServiceUserRole — skip for MCP. - // The canonical resources (above) cover spec.NodeName; all other nodes get - // their own RO and RW role that sources credentials from the canonical. - if spec.ServiceSpec.ServiceType != "mcp" { - for _, nodeInst := range spec.DatabaseNodes { - if nodeInst.NodeName == spec.NodeName { - continue - } - perNodeRWID := ServiceUserRolePerNodeIdentifier(spec.ServiceSpec.ServiceID, ServiceUserRoleRW, nodeInst.NodeName) + // Append per-node resources for each additional database node. + // RAG uses per-node ServiceUserRole; PostgREST uses per-node authenticator. + // The canonical resources (above) cover spec.NodeName. + for _, nodeInst := range spec.DatabaseNodes { + if nodeInst.NodeName == spec.NodeName { + continue + } + if spec.ServiceSpec.ServiceType == "rag" { orchestratorResources = append(orchestratorResources, &ServiceUserRole{ ServiceID: spec.ServiceSpec.ServiceID, @@ -618,18 +618,18 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan CredentialSource: &canonicalRWID, }, ) - if spec.ServiceSpec.ServiceType == "postgrest" { - orchestratorResources = append(orchestratorResources, - &PostgRESTAuthenticatorResource{ - ServiceID: spec.ServiceSpec.ServiceID, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: nodeInst.NodeName, - DBAnonRole: parsedPostgRESTConfig.DBAnonRole, - UserRoleID: perNodeRWID, - }, - ) - } + } + if spec.ServiceSpec.ServiceType == "postgrest" { + orchestratorResources = append(orchestratorResources, + &PostgRESTAuthenticatorResource{ + ServiceID: spec.ServiceSpec.ServiceID, + DatabaseID: spec.DatabaseID, + DatabaseName: spec.DatabaseName, + NodeName: nodeInst.NodeName, + DBAnonRole: parsedPostgRESTConfig.DBAnonRole, + ConnectAsUsername: spec.ConnectAsUsername, + }, + ) } } diff --git a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go index 78792204..6cbf2521 100644 --- a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go +++ b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" - "github.com/jackc/pgx/v5" "github.com/rs/zerolog" "github.com/samber/do" @@ -26,8 +25,8 @@ func PostgRESTAuthenticatorIdentifier(serviceID, nodeName string) resource.Ident } // PostgRESTAuthenticatorResource configures a PostgreSQL role as a PostgREST -// authenticator. It depends on the corresponding ServiceUserRole (which creates -// the basic LOGIN+group-role user) and adds PostgREST-specific configuration: +// authenticator. It targets the connect_as database user and adds +// PostgREST-specific configuration: // // - ALTER ROLE ... WITH NOINHERIT (required for PostgREST's SET ROLE mechanism) // - GRANT CONNECT ON DATABASE to the authenticator user @@ -35,15 +34,13 @@ func PostgRESTAuthenticatorIdentifier(serviceID, nodeName string) resource.Ident // // On Update, the anonymous role grant is reconciled within a single transaction // to prevent transient loss of anon-role membership when the anon role changes. -// The actual DROP ROLE is handled by ServiceUserRole.Delete; this resource only -// revokes the CONNECT privilege it added. type PostgRESTAuthenticatorResource struct { - ServiceID string `json:"service_id"` - DatabaseID string `json:"database_id"` - DatabaseName string `json:"database_name"` - NodeName string `json:"node_name"` - DBAnonRole string `json:"db_anon_role"` - UserRoleID resource.Identifier `json:"user_role_id"` // the RW ServiceUserRole this wraps + ServiceID string `json:"service_id"` + DatabaseID string `json:"database_id"` + DatabaseName string `json:"database_name"` + NodeName string `json:"node_name"` + DBAnonRole string `json:"db_anon_role"` + ConnectAsUsername string `json:"connect_as_username"` // the database_users entry PostgREST connects as } func (r *PostgRESTAuthenticatorResource) ResourceVersion() string { return "1" } @@ -60,7 +57,7 @@ func (r *PostgRESTAuthenticatorResource) Executor() resource.Executor { func (r *PostgRESTAuthenticatorResource) Dependencies() []resource.Identifier { return []resource.Identifier{ database.NodeResourceIdentifier(r.NodeName), - r.UserRoleID, + database.PostgresDatabaseResourceIdentifier(r.NodeName, r.DatabaseName), } } @@ -76,7 +73,7 @@ func (r *PostgRESTAuthenticatorResource) desiredAnonRole() string { } func (r *PostgRESTAuthenticatorResource) authenticatorUsername() string { - return database.GenerateServiceUsername(r.ServiceID, ServiceUserRoleRW) + return r.ConnectAsUsername } // Refresh checks whether the role has NOINHERIT. If not — new deployment or @@ -155,8 +152,9 @@ func (r *PostgRESTAuthenticatorResource) Update(ctx context.Context, rc *resourc return r.reconcileGrants(ctx, rc) } -// reconcileGrants revokes stale anon role grants and re-applies the desired -// ones within a single transaction to prevent transient loss of membership. +// reconcileGrants revokes the previously-granted anon role (if changed) and +// re-applies the desired one within a single transaction to prevent transient +// loss of membership. func (r *PostgRESTAuthenticatorResource) reconcileGrants(ctx context.Context, rc *resource.Context) error { primary, err := database.GetPrimaryInstance(ctx, rc, r.NodeName) if err != nil { @@ -176,8 +174,9 @@ func (r *PostgRESTAuthenticatorResource) reconcileGrants(ctx context.Context, rc username := r.authenticatorUsername() desiredAnon := r.desiredAnonRole() + previousAnon := r.previousAnonRole(rc) - if err := r.revokeStaleAnonRoles(ctx, tx, username, desiredAnon); err != nil { + if err := r.revokeStaleAnonRoles(ctx, tx, username, desiredAnon, previousAnon); err != nil { return err } @@ -194,69 +193,36 @@ func (r *PostgRESTAuthenticatorResource) reconcileGrants(ctx context.Context, rc return nil } -// revokeStaleAnonRoles revokes any previously-granted anon roles that are no -// longer the desired one. The query is scoped to known anon role candidates so -// that base group roles granted by ServiceUserRole (pgedge_application, -// pgedge_application_read_only) are never touched. Must be called within a -// transaction for atomicity. -func (r *PostgRESTAuthenticatorResource) revokeStaleAnonRoles(ctx context.Context, conn postgres.Executor, username, desiredAnon string) error { - // Only query memberships that this resource could have granted — the set of - // known anon role names. This prevents accidentally revoking base group - // roles that ServiceUserRole manages. - currentRoles, err := postgres.Query[string]{ - SQL: `SELECT r.rolname - FROM pg_auth_members m - JOIN pg_roles r ON m.roleid = r.oid - JOIN pg_roles u ON m.member = u.oid - WHERE u.rolname = @username - AND r.rolname != 'pgedge_application' - AND r.rolname != 'pgedge_application_read_only'`, - Args: pgx.NamedArgs{"username": username}, - }.Scalars(ctx, conn) - if err != nil { - return fmt.Errorf("failed to query anon role memberships for %q: %w", username, err) - } - for _, current := range currentRoles { - if current != desiredAnon { - if _, err := conn.Exec(ctx, fmt.Sprintf("REVOKE %s FROM %s", // #nosec G201 -- sanitizeIdentifier quotes all identifiers - sanitizeIdentifier(current), sanitizeIdentifier(username))); err != nil { - return fmt.Errorf("failed to revoke stale anon role %q from %q: %w", current, username, err) - } - } +// previousAnonRole reads the previously-stored resource from rc.State and +// returns the anon role it had applied. Returns empty string when no prior +// state exists (first apply) so the caller can skip the revoke entirely. +func (r *PostgRESTAuthenticatorResource) previousAnonRole(rc *resource.Context) string { + stored, ok := rc.State.Get(r.Identifier()) + if !ok { + return "" } - return nil -} - -func (r *PostgRESTAuthenticatorResource) Delete(ctx context.Context, rc *resource.Context) error { - logger, err := do.Invoke[zerolog.Logger](rc.Injector) + prev, err := resource.ToResource[*PostgRESTAuthenticatorResource](stored) if err != nil { - return err - } - username := r.authenticatorUsername() - logger = logger.With(). - Str("service_id", r.ServiceID). - Str("username", username). - Logger() - - if r.DatabaseName == "" { - return nil + return "" } + return prev.desiredAnonRole() +} - primary, err := database.GetPrimaryInstance(ctx, rc, r.NodeName) - if err != nil { - logger.Warn().Err(err).Msg("failed to get primary instance, skipping REVOKE CONNECT") +// revokeStaleAnonRoles revokes the previously-applied anon role when it +// differs from the desired one. Only the exact role this resource previously +// granted is targeted — customer-managed grants on the connect_as user are +// never touched. +func (r *PostgRESTAuthenticatorResource) revokeStaleAnonRoles(ctx context.Context, conn postgres.Executor, username, desiredAnon, previousAnon string) error { + if previousAnon == "" || previousAnon == desiredAnon { return nil } - conn, err := primary.Connection(ctx, rc, "postgres") - if err != nil { - logger.Warn().Err(err).Msg("failed to connect to primary instance, skipping REVOKE CONNECT") - return nil + if _, err := conn.Exec(ctx, fmt.Sprintf("REVOKE %s FROM %s", // #nosec G201 -- sanitizeIdentifier quotes all identifiers + sanitizeIdentifier(previousAnon), sanitizeIdentifier(username))); err != nil { + return fmt.Errorf("failed to revoke stale anon role %q from %q: %w", previousAnon, username, err) } - defer conn.Close(ctx) + return nil +} - if _, rErr := conn.Exec(ctx, fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM %s", // #nosec G201 -- sanitizeIdentifier quotes all identifiers - sanitizeIdentifier(r.DatabaseName), sanitizeIdentifier(username))); rErr != nil { - logger.Warn().Err(rErr).Msg("failed to revoke CONNECT privilege, continuing") - } +func (r *PostgRESTAuthenticatorResource) Delete(_ context.Context, _ *resource.Context) error { return nil } diff --git a/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go b/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go new file mode 100644 index 00000000..9e31a081 --- /dev/null +++ b/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go @@ -0,0 +1,85 @@ +package swarm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pgEdge/control-plane/server/internal/database" + "github.com/pgEdge/control-plane/server/internal/resource" +) + +func TestPostgRESTAuthenticatorResource_ResourceVersion(t *testing.T) { + r := &PostgRESTAuthenticatorResource{} + assert.Equal(t, "1", r.ResourceVersion()) +} + +func TestPostgRESTAuthenticatorResource_DiffIgnore(t *testing.T) { + r := &PostgRESTAuthenticatorResource{} + assert.Nil(t, r.DiffIgnore()) +} + +func TestPostgRESTAuthenticatorResourceIdentifier(t *testing.T) { + id := PostgRESTAuthenticatorIdentifier("api", "n1") + assert.Equal(t, "api-auth-n1", id.ID) + assert.Equal(t, ResourceTypePostgRESTAuthenticator, id.Type) +} + +func TestPostgRESTAuthenticatorResource_Identifier(t *testing.T) { + r := &PostgRESTAuthenticatorResource{ServiceID: "api", NodeName: "n2"} + id := r.Identifier() + assert.Equal(t, "api-auth-n2", id.ID) + assert.Equal(t, ResourceTypePostgRESTAuthenticator, id.Type) +} + +func TestPostgRESTAuthenticatorResource_Executor(t *testing.T) { + r := &PostgRESTAuthenticatorResource{NodeName: "n1"} + assert.Equal(t, resource.PrimaryExecutor("n1"), r.Executor()) +} + +func TestPostgRESTAuthenticatorResource_TypeDependencies(t *testing.T) { + r := &PostgRESTAuthenticatorResource{} + assert.Nil(t, r.TypeDependencies()) +} + +func TestPostgRESTAuthenticatorResource_Dependencies(t *testing.T) { + r := &PostgRESTAuthenticatorResource{ + NodeName: "n1", + DatabaseName: "storefront", + } + deps := r.Dependencies() + + require.Len(t, deps, 2) + assert.Equal(t, database.NodeResourceIdentifier("n1"), deps[0]) + assert.Equal(t, database.PostgresDatabaseResourceIdentifier("n1", "storefront"), deps[1]) +} + +func TestPostgRESTAuthenticatorResource_DesiredAnonRole(t *testing.T) { + t.Run("empty string falls back to default", func(t *testing.T) { + r := &PostgRESTAuthenticatorResource{DBAnonRole: ""} + assert.Equal(t, "pgedge_application_read_only", r.desiredAnonRole()) + }) + + t.Run("custom anon role is preserved", func(t *testing.T) { + r := &PostgRESTAuthenticatorResource{DBAnonRole: "web_anon"} + assert.Equal(t, "web_anon", r.desiredAnonRole()) + }) +} + +func TestPostgRESTAuthenticatorResource_AuthenticatorUsername(t *testing.T) { + r := &PostgRESTAuthenticatorResource{ConnectAsUsername: "app"} + assert.Equal(t, "app", r.authenticatorUsername()) +} + +func TestPostgRESTAuthenticatorIdentifier_DifferentNodes(t *testing.T) { + id1 := PostgRESTAuthenticatorIdentifier("api", "n1") + id2 := PostgRESTAuthenticatorIdentifier("api", "n2") + assert.NotEqual(t, id1, id2, "different nodes should produce different identifiers") +} + +func TestPostgRESTAuthenticatorIdentifier_DifferentServices(t *testing.T) { + id1 := PostgRESTAuthenticatorIdentifier("svc-a", "n1") + id2 := PostgRESTAuthenticatorIdentifier("svc-b", "n1") + assert.NotEqual(t, id1, id2, "different services should produce different identifiers") +} diff --git a/server/internal/orchestrator/swarm/postgrest_config_resource.go b/server/internal/orchestrator/swarm/postgrest_config_resource.go index 9565510a..12a5e34d 100644 --- a/server/internal/orchestrator/swarm/postgrest_config_resource.go +++ b/server/internal/orchestrator/swarm/postgrest_config_resource.go @@ -26,15 +26,15 @@ func PostgRESTConfigResourceIdentifier(serviceInstanceID string) resource.Identi // PostgRESTConfigResource manages the postgrest.conf file on the host filesystem. // The file is bind-mounted read-only into the container and includes the db-uri -// with embedded credentials. +// with embedded credentials from the connect_as database user. type PostgRESTConfigResource struct { ServiceInstanceID string `json:"service_instance_id"` ServiceID string `json:"service_id"` HostID string `json:"host_id"` DirResourceID string `json:"dir_resource_id"` Config *database.PostgRESTServiceConfig `json:"config"` - Username string `json:"username"` - Password string `json:"password"` + ConnectAsUsername string `json:"connect_as_username"` + ConnectAsPassword string `json:"connect_as_password"` DatabaseName string `json:"database_name"` DatabaseHosts []database.ServiceHostEntry `json:"database_hosts"` TargetSessionAttrs string `json:"target_session_attrs,omitempty"` @@ -45,7 +45,7 @@ func (r *PostgRESTConfigResource) ResourceVersion() string { } func (r *PostgRESTConfigResource) DiffIgnore() []string { - return []string{"/username", "/password"} + return nil } func (r *PostgRESTConfigResource) Identifier() resource.Identifier { @@ -59,7 +59,6 @@ func (r *PostgRESTConfigResource) Executor() resource.Executor { func (r *PostgRESTConfigResource) Dependencies() []resource.Identifier { return []resource.Identifier{ filesystem.DirResourceIdentifier(r.DirResourceID), - ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRW), } } @@ -113,23 +112,16 @@ func (r *PostgRESTConfigResource) Delete(ctx context.Context, rc *resource.Conte return nil } -// populateCredentials reads the RW service user's username and password from -// the corresponding ServiceUserRole resource state. Called at Create/Update -// time so that credentials are never stale. -func (r *PostgRESTConfigResource) populateCredentials(rc *resource.Context) error { - rwRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRW)) - if err != nil { - return fmt.Errorf("failed to get RW service user role: %w", err) - } - r.Username = rwRole.Username - r.Password = rwRole.Password +// populateCredentials is a no-op — credentials come from ConnectAsUsername/ +// ConnectAsPassword set at construction time from the connect_as database user. +func (r *PostgRESTConfigResource) populateCredentials(_ *resource.Context) error { return nil } func (r *PostgRESTConfigResource) writeConfigFile(fs afero.Fs, dirPath string) error { content, err := r.Config.GenerateConf(database.PostgRESTConnParams{ - Username: r.Username, - Password: r.Password, + Username: r.ConnectAsUsername, + Password: r.ConnectAsPassword, DatabaseName: r.DatabaseName, DatabaseHosts: r.DatabaseHosts, TargetSessionAttrs: r.TargetSessionAttrs, diff --git a/server/internal/orchestrator/swarm/postgrest_config_resource_test.go b/server/internal/orchestrator/swarm/postgrest_config_resource_test.go new file mode 100644 index 00000000..cf5c0f33 --- /dev/null +++ b/server/internal/orchestrator/swarm/postgrest_config_resource_test.go @@ -0,0 +1,165 @@ +package swarm + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pgEdge/control-plane/server/internal/database" + "github.com/pgEdge/control-plane/server/internal/filesystem" + "github.com/pgEdge/control-plane/server/internal/resource" +) + +func TestPostgRESTConfigResource_ResourceVersion(t *testing.T) { + r := &PostgRESTConfigResource{} + assert.Equal(t, "1", r.ResourceVersion()) +} + +func TestPostgRESTConfigResource_DiffIgnore(t *testing.T) { + r := &PostgRESTConfigResource{} + assert.Nil(t, r.DiffIgnore()) +} + +func TestPostgRESTConfigResourceIdentifier(t *testing.T) { + id := PostgRESTConfigResourceIdentifier("inst-1") + assert.Equal(t, "inst-1", id.ID) + assert.Equal(t, ResourceTypePostgRESTConfig, id.Type) +} + +func TestPostgRESTConfigResource_Identifier(t *testing.T) { + r := &PostgRESTConfigResource{ServiceInstanceID: "inst-abc"} + id := r.Identifier() + assert.Equal(t, "inst-abc", id.ID) + assert.Equal(t, ResourceTypePostgRESTConfig, id.Type) +} + +func TestPostgRESTConfigResource_Executor(t *testing.T) { + r := &PostgRESTConfigResource{HostID: "host-2"} + assert.Equal(t, resource.HostExecutor("host-2"), r.Executor()) +} + +func TestPostgRESTConfigResource_TypeDependencies(t *testing.T) { + r := &PostgRESTConfigResource{} + assert.Nil(t, r.TypeDependencies()) +} + +func TestPostgRESTConfigResource_Dependencies(t *testing.T) { + r := &PostgRESTConfigResource{ + ServiceInstanceID: "inst-1", + DirResourceID: "inst-1-data", + } + deps := r.Dependencies() + + require.Len(t, deps, 1) + assert.Equal(t, filesystem.DirResourceIdentifier("inst-1-data"), deps[0]) +} + +func TestPostgRESTConfigResource_WriteConfigFile(t *testing.T) { + fs := afero.NewMemMapFs() + dirPath := "/var/lib/pgedge/services/inst-1" + require.NoError(t, fs.MkdirAll(dirPath, 0o755)) + + cfg := &database.PostgRESTServiceConfig{ + DBSchemas: "public", + DBAnonRole: "pgedge_application_read_only", + DBPool: 10, + MaxRows: 1000, + } + r := &PostgRESTConfigResource{ + Config: cfg, + ConnectAsUsername: "myapp", + ConnectAsPassword: "s3cr3t", + DatabaseName: "mydb", + DatabaseHosts: []database.ServiceHostEntry{{Host: "pg-host1", Port: 5432}}, + } + + err := r.writeConfigFile(fs, dirPath) + require.NoError(t, err) + + data, err := afero.ReadFile(fs, filepath.Join(dirPath, "postgrest.conf")) + require.NoError(t, err, "postgrest.conf must exist after writeConfigFile") + + content := string(data) + assert.Contains(t, content, "db-uri") + assert.Contains(t, content, "myapp", "username must appear in db-uri") + assert.Contains(t, content, "s3cr3t", "password must appear in db-uri") + assert.Contains(t, content, "pg-host1", "host must appear in db-uri") + assert.Contains(t, content, "mydb", "database name must appear in db-uri") + assert.Contains(t, content, "db-schemas") + assert.Contains(t, content, "public") + assert.Contains(t, content, "db-anon-role") + assert.Contains(t, content, "pgedge_application_read_only") +} + +func TestPostgRESTConfigResource_WriteConfigFile_JWTFields(t *testing.T) { + fs := afero.NewMemMapFs() + dirPath := "/var/lib/pgedge/services/inst-jwt" + require.NoError(t, fs.MkdirAll(dirPath, 0o755)) + + secret := "a-very-long-jwt-secret-that-is-at-least-32-chars" + cfg := &database.PostgRESTServiceConfig{ + DBSchemas: "public", + DBAnonRole: "web_anon", + DBPool: 5, + MaxRows: 500, + JWTSecret: &secret, + } + r := &PostgRESTConfigResource{ + Config: cfg, + ConnectAsUsername: "app", + ConnectAsPassword: "pass", + DatabaseName: "mydb", + DatabaseHosts: []database.ServiceHostEntry{{Host: "pg-host1", Port: 5432}}, + } + + err := r.writeConfigFile(fs, dirPath) + require.NoError(t, err) + + data, err := afero.ReadFile(fs, filepath.Join(dirPath, "postgrest.conf")) + require.NoError(t, err) + + content := string(data) + assert.Contains(t, content, "jwt-secret") + assert.Contains(t, content, secret) +} + +func TestPostgRESTConfigResource_WriteConfigFile_MultiHost(t *testing.T) { + fs := afero.NewMemMapFs() + dirPath := "/var/lib/pgedge/services/inst-multi" + require.NoError(t, fs.MkdirAll(dirPath, 0o755)) + + cfg := &database.PostgRESTServiceConfig{ + DBSchemas: "public", + DBAnonRole: "web_anon", + DBPool: 10, + MaxRows: 1000, + } + r := &PostgRESTConfigResource{ + Config: cfg, + ConnectAsUsername: "app", + ConnectAsPassword: "pass", + DatabaseName: "mydb", + DatabaseHosts: []database.ServiceHostEntry{ + {Host: "pg-host1", Port: 5432}, + {Host: "pg-host2", Port: 5432}, + }, + TargetSessionAttrs: "read-write", + } + + err := r.writeConfigFile(fs, dirPath) + require.NoError(t, err) + + data, err := afero.ReadFile(fs, filepath.Join(dirPath, "postgrest.conf")) + require.NoError(t, err) + + content := string(data) + assert.Contains(t, content, "pg-host1") + assert.Contains(t, content, "pg-host2") + assert.True(t, strings.Contains(content, "target_session_attrs") || + strings.Contains(content, "read-write"), + "multi-host URI should contain target_session_attrs or read-write") +} diff --git a/server/internal/orchestrator/swarm/postgrest_preflight_resource_test.go b/server/internal/orchestrator/swarm/postgrest_preflight_resource_test.go new file mode 100644 index 00000000..9df544e2 --- /dev/null +++ b/server/internal/orchestrator/swarm/postgrest_preflight_resource_test.go @@ -0,0 +1,101 @@ +package swarm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/pgEdge/control-plane/server/internal/database" + "github.com/pgEdge/control-plane/server/internal/resource" +) + +func TestPostgRESTPreflightResource_ResourceVersion(t *testing.T) { + r := &PostgRESTPreflightResource{} + assert.Equal(t, "1", r.ResourceVersion()) +} + +func TestPostgRESTPreflightResource_DiffIgnore(t *testing.T) { + r := &PostgRESTPreflightResource{} + assert.Nil(t, r.DiffIgnore()) +} + +func TestPostgRESTPreflightResourceIdentifier(t *testing.T) { + id := PostgRESTPreflightResourceIdentifier("my-service") + assert.Equal(t, "my-service", id.ID) + assert.Equal(t, ResourceTypePostgRESTPreflightResource, id.Type) +} + +func TestPostgRESTPreflightResource_Identifier(t *testing.T) { + r := &PostgRESTPreflightResource{ServiceID: "api"} + id := r.Identifier() + assert.Equal(t, "api", id.ID) + assert.Equal(t, ResourceTypePostgRESTPreflightResource, id.Type) +} + +func TestPostgRESTPreflightResource_Executor(t *testing.T) { + r := &PostgRESTPreflightResource{NodeName: "n1"} + assert.Equal(t, resource.PrimaryExecutor("n1"), r.Executor()) +} + +func TestPostgRESTPreflightResource_TypeDependencies(t *testing.T) { + r := &PostgRESTPreflightResource{} + assert.Nil(t, r.TypeDependencies()) +} + +func TestPostgRESTPreflightResource_Dependencies(t *testing.T) { + r := &PostgRESTPreflightResource{ + NodeName: "n1", + DatabaseName: "storefront", + } + deps := r.Dependencies() + + require.Len(t, deps, 1) + assert.Equal(t, database.PostgresDatabaseResourceIdentifier("n1", "storefront"), deps[0]) +} + +func TestSplitSchemas(t *testing.T) { + tests := []struct { + name string + input string + want []string + }{ + { + name: "empty string", + input: "", + want: []string{}, + }, + { + name: "single schema", + input: "public", + want: []string{"public"}, + }, + { + name: "multiple schemas", + input: "public,api,private", + want: []string{"public", "api", "private"}, + }, + { + name: "schemas with whitespace", + input: " public , api , private ", + want: []string{"public", "api", "private"}, + }, + { + name: "trailing comma", + input: "public,", + want: []string{"public"}, + }, + { + name: "leading comma", + input: ",public", + want: []string{"public"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := splitSchemas(tt.input) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/server/internal/orchestrator/swarm/service_instance.go b/server/internal/orchestrator/swarm/service_instance.go index 387922e3..a7775074 100644 --- a/server/internal/orchestrator/swarm/service_instance.go +++ b/server/internal/orchestrator/swarm/service_instance.go @@ -62,16 +62,9 @@ func (s *ServiceInstanceResource) Executor() resource.Executor { } func (s *ServiceInstanceResource) Dependencies() []resource.Identifier { - deps := []resource.Identifier{ + return []resource.Identifier{ ServiceInstanceSpecResourceIdentifier(s.ServiceInstanceID), } - // MCP and RAG get credentials from database_users (connect_as) — - // no ServiceUserRole dependency. PostgREST still uses ServiceUserRole. - if s.ServiceType == "postgrest" { - deps = append(deps, ServiceUserRoleIdentifier(s.ServiceSpecID, ServiceUserRoleRO)) - deps = append(deps, ServiceUserRoleIdentifier(s.ServiceSpecID, ServiceUserRoleRW)) - } - return deps } func (s *ServiceInstanceResource) TypeDependencies() []resource.Type { diff --git a/server/internal/orchestrator/swarm/service_instance_spec.go b/server/internal/orchestrator/swarm/service_instance_spec.go index e86f72eb..3eac142b 100644 --- a/server/internal/orchestrator/swarm/service_instance_spec.go +++ b/server/internal/orchestrator/swarm/service_instance_spec.go @@ -66,18 +66,16 @@ func (s *ServiceInstanceSpecResource) Dependencies() []resource.Identifier { NetworkResourceIdentifier(s.DatabaseNetworkID), } - // MCP and RAG get credentials from database_users (connect_as) — - // no ServiceUserRole dependency. PostgREST still uses ServiceUserRole. - if s.ServiceSpec.ServiceType == "postgrest" { - deps = append(deps, ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, ServiceUserRoleRO)) - deps = append(deps, ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, ServiceUserRoleRW)) - } - switch s.ServiceSpec.ServiceType { case "mcp": deps = append(deps, MCPConfigResourceIdentifier(s.ServiceInstanceID)) case "postgrest": deps = append(deps, PostgRESTConfigResourceIdentifier(s.ServiceInstanceID)) + // Wait for preflight (which waits for the DB to exist) before starting + // the container. Without this the Docker service starts before Patroni + // has bootstrapped the database and PostgREST fails with "database + // does not exist". + deps = append(deps, PostgRESTPreflightResourceIdentifier(s.ServiceSpec.ServiceID)) case "rag": deps = append(deps, RAGConfigResourceIdentifier(s.ServiceInstanceID), @@ -94,33 +92,10 @@ func (s *ServiceInstanceSpecResource) TypeDependencies() []resource.Type { } func (s *ServiceInstanceSpecResource) populateCredentials(rc *resource.Context) error { - // MCP and RAG source credentials from database_users (connect_as). - // RAG credentials go into the config file via RAGConfigResource, not the - // container spec, so s.Credentials remains nil for RAG regardless. - // Clear any stale credentials that may have been persisted by the legacy - // ServiceUserRole path before this migration. - if s.ServiceSpec.ServiceType == "mcp" || s.ServiceSpec.ServiceType == "rag" { - s.Credentials = nil - return nil - } - - // PostgREST authenticates to Postgres as the RW service user (NOINHERIT, - // granted the anon role). All other service types use the RO service user. - mode := ServiceUserRoleRO - role := "pgedge_application_read_only" - if s.ServiceSpec.ServiceType == "postgrest" { - mode = ServiceUserRoleRW - role = "postgrest_authenticator" - } - userRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, mode)) - if err != nil { - return fmt.Errorf("failed to get service user role from state: %w", err) - } - s.Credentials = &database.ServiceUser{ - Username: userRole.Username, - Password: userRole.Password, - Role: role, - } + // All current service types (mcp, postgrest, rag) source credentials from + // database_users (connect_as) — credentials go into the config file, not + // the container spec. + s.Credentials = nil return nil } @@ -130,7 +105,7 @@ func (s *ServiceInstanceSpecResource) Refresh(ctx context.Context, rc *resource. return fmt.Errorf("failed to get database network from state: %w", err) } - // Populate credentials from the ServiceUserRole resource + // Credentials are nil for all current service types — they use config files. if err := s.populateCredentials(rc); err != nil { return err } diff --git a/server/internal/orchestrator/swarm/service_spec_test.go b/server/internal/orchestrator/swarm/service_spec_test.go index 3a60152a..40b942ad 100644 --- a/server/internal/orchestrator/swarm/service_spec_test.go +++ b/server/internal/orchestrator/swarm/service_spec_test.go @@ -476,6 +476,7 @@ func makePostgRESTSpecOpts() *ServiceContainerSpecOptions { ServiceSpec: &database.ServiceSpec{ ServiceID: "svc-1", ServiceType: "postgrest", + ConnectAs: "myapp", }, ServiceInstanceID: "inst-1", DatabaseID: "db-1", @@ -485,10 +486,9 @@ func makePostgRESTSpecOpts() *ServiceContainerSpecOptions { Hostname: "postgrest-host1", CohortMemberID: "node-abc", ServiceImage: &ServiceImage{Tag: "postgrest/postgrest:latest"}, - Credentials: &database.ServiceUser{ - Username: "svc_postgrest_host1", - Password: "supersecret", - }, + // Credentials are nil for PostgREST — the connect_as user's credentials + // go into postgrest.conf via PostgRESTConfigResource, not the container spec. + Credentials: nil, DatabaseNetworkID: "net-1", DatabaseHosts: []database.ServiceHostEntry{{Host: "pg-host1", Port: 5432}}, DataPath: "/var/lib/pgedge/services/inst-1",