diff --git a/server/internal/database/service_instance.go b/server/internal/database/service_instance.go index 0a5b533f..1ac5d1e6 100644 --- a/server/internal/database/service_instance.go +++ b/server/internal/database/service_instance.go @@ -1,10 +1,7 @@ package database import ( - "crypto/sha256" - "encoding/hex" "fmt" - "strings" "time" "github.com/pgEdge/control-plane/server/internal/ds" @@ -26,12 +23,9 @@ type ServiceInstance struct { HostID string `json:"host_id"` State ServiceInstanceState `json:"state"` Status *ServiceInstanceStatus `json:"status,omitempty"` - // Credentials is only populated during provisioning workflows. It is not - // persisted to etcd and will be nil when read from the store. - Credentials *ServiceUser `json:"credentials,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - Error string `json:"error,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + Error string `json:"error,omitempty"` } type ServiceInstanceStatus struct { @@ -56,89 +50,6 @@ type HealthCheckResult struct { CheckedAt time.Time `json:"checked_at"` } -// ServiceUser represents database credentials for a service instance. -// -// Each service instance receives two dedicated database users: one read-only (RO) and -// one read-write (RW). The active user is selected based on the service's allow_writes -// setting. This provides security isolation between service instances. -// -// # Credential Generation -// -// Credentials are generated during service instance provisioning by the CreateServiceUser -// workflow activity. The username is deterministic (based on service instance ID and -// mode), while the password is cryptographically random. -// -// # Security Properties -// -// - Unique per service instance (not shared between instances) -// - 32-character random passwords -// - Storage in etcd alongside service instance metadata -// - Injected into service containers via config.yaml -type ServiceUser struct { - Username string `json:"username"` // Format: "svc_{service_id}_{mode}" - Password string `json:"password"` // 32-character cryptographically random string - Role string `json:"role"` // Database role, e.g., "pgedge_application_read_only" or "pgedge_application" -} - -// GenerateServiceUsername creates a deterministic username for a service. -// -// # Username Format -// -// The username follows the pattern: "svc_{service_id}_{mode}" -// -// Example: -// -// service_id: "mcp-server", mode: "ro" -// Generated username: "svc_mcp_server_ro" -// -// # Rationale -// -// - "svc_" prefix: Clearly identifies service accounts vs. application users -// - service_id: Uniquely identifies the service within the database -// - mode: Distinguishes RO ("ro") from RW ("rw") users for the same service -// - Deterministic: Same service_id + mode always generates the same username -// - Shared: One database user role per service per mode, shared across all instances -// -// # Uniqueness -// -// Service IDs are unique within a database. By using the service_id and mode, we -// guarantee uniqueness even when multiple services exist on the same database. -// -// # PostgreSQL Compatibility -// -// PostgreSQL identifier length limit is 63 characters. For short names the full -// service_id is used directly. When the username exceeds 63 characters, the -// function appends an 8-character hex hash (from SHA-256 of the full untruncated -// name) to a truncated prefix. This guarantees uniqueness even when two inputs -// share a long common prefix. -// -// Short name format: svc_{service_id}_{mode} -// Long name format: svc_{truncated service_id}_{8-hex-hash}_{mode} -func GenerateServiceUsername(serviceID string, mode string) string { - // Sanitize hyphens to underscores for PostgreSQL compatibility. - // Hyphens in identifiers require double-quoting in SQL. - serviceID = strings.ReplaceAll(serviceID, "-", "_") - username := fmt.Sprintf("svc_%s_%s", serviceID, mode) - - if len(username) <= 63 { - return username - } - - // Hash the full untruncated username for uniqueness - h := sha256.Sum256([]byte(username)) - hashSuffix := hex.EncodeToString(h[:4]) // 8 hex chars - - // svc_ (4) + prefix + _ (1) + hash (8) + _ (1) + mode (len) = 14 + len(mode) + prefix - // Max prefix = 63 - 14 - len(mode) - maxPrefix := 63 - 14 - len(mode) - raw := serviceID - if len(raw) > maxPrefix { - raw = raw[:maxPrefix] - } - - return fmt.Sprintf("svc_%s_%s_%s", raw, hashSuffix, mode) -} - // GenerateServiceInstanceID creates a unique ID for a service instance. // Format: {database_id}-{service_id}-{host_id} func GenerateServiceInstanceID(databaseID, serviceID, hostID string) string { @@ -167,13 +78,12 @@ type ServiceInstanceSpec struct { DatabaseName string HostID string CohortMemberID string - Credentials *ServiceUser DatabaseNetworkID string - NodeName string // Database node name (for ServiceUserRole PrimaryExecutor routing) + NodeName string // Database node name for PrimaryExecutor routing DatabaseHosts []ServiceHostEntry // Ordered list of Postgres host:port entries TargetSessionAttrs string // libpq target_session_attrs value Port *int // Service instance published port (optional, 0 = random) - DatabaseNodes []*NodeInstances // All database nodes; used to create per-node ServiceUserRole resources + DatabaseNodes []*NodeInstances // All database nodes; used to create per-node resources ConnectAsUsername string // Username from database_users (resolved from ServiceSpec.ConnectAs) ConnectAsPassword string // Password from database_users (resolved from ServiceSpec.ConnectAs) } diff --git a/server/internal/database/service_instance_test.go b/server/internal/database/service_instance_test.go deleted file mode 100644 index f47fa0ab..00000000 --- a/server/internal/database/service_instance_test.go +++ /dev/null @@ -1,175 +0,0 @@ -package database - -import ( - "testing" -) - -func TestGenerateServiceUsername(t *testing.T) { - tests := []struct { - name string - serviceID string - mode string - want string - }{ - { - name: "standard service instance ro", - serviceID: "mcp-server", - mode: "ro", - want: "svc_mcp_server_ro", - }, - { - name: "standard service instance rw", - serviceID: "mcp-server", - mode: "rw", - want: "svc_mcp_server_rw", - }, - { - name: "multiple services on same database - service 1", - serviceID: "appmcp-1", - mode: "ro", - want: "svc_appmcp_1_ro", - }, - { - name: "multiple services on same database - service 2", - serviceID: "appmcp-2", - mode: "ro", - want: "svc_appmcp_2_ro", - }, - { - name: "service with multi-part service ID", - serviceID: "my-mcp-service", - mode: "ro", - want: "svc_my_mcp_service_ro", - }, - { - name: "simple service ID", - serviceID: "mcp", - mode: "ro", - want: "svc_mcp_ro", - }, - { - name: "long service ID uses hash suffix", - serviceID: "very-long-service-name-that-exceeds-postgres-limit-significantly", - mode: "ro", - want: "", // computed below - }, - { - name: "long names with shared prefix produce different usernames (case A)", - serviceID: "very-long-service-name-that-exceeds-postgres-limit-AAA", - mode: "ro", - want: "", // computed below - }, - { - name: "long names with shared prefix produce different usernames (case B)", - serviceID: "very-long-service-name-that-exceeds-postgres-limit-BBB", - mode: "ro", - want: "", // computed below - }, - { - name: "long service ID with rw mode still fits", - serviceID: "very-long-service-name-that-exceeds-postgres-limit-significantly", - mode: "rw", - want: "", // computed below, just check length - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := GenerateServiceUsername(tt.serviceID, tt.mode) - if tt.want != "" { - if got != tt.want { - t.Errorf("GenerateServiceUsername() = %v, want %v", got, tt.want) - } - } - if len(got) > 63 { - t.Errorf("GenerateServiceUsername() length = %d, must be <= 63", len(got)) - } - if len(got) < 4 || got[:4] != "svc_" { - t.Errorf("GenerateServiceUsername() = %v, must start with svc_", got) - } - }) - } - - // Verify long names with shared prefix produce different usernames - a := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-AAA", "ro") - b := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-BBB", "ro") - if a == b { - t.Errorf("long names with shared prefix should produce different usernames, both got %v", a) - } - - // Verify different modes produce different usernames for the same serviceID - roUser := GenerateServiceUsername("mcp-server", "ro") - rwUser := GenerateServiceUsername("mcp-server", "rw") - if roUser == rwUser { - t.Errorf("different modes should produce different usernames, both got %v", roUser) - } -} - -func TestGenerateServiceInstanceID(t *testing.T) { - tests := []struct { - name string - databaseID string - serviceID string - hostID string - want string - }{ - { - name: "standard service instance", - databaseID: "db1", - serviceID: "mcp-server", - hostID: "host1", - want: "db1-mcp-server-host1", - }, - { - name: "multi-part identifiers", - databaseID: "my-database", - serviceID: "my-service", - hostID: "my-host", - want: "my-database-my-service-my-host", - }, - { - name: "simple identifiers", - databaseID: "db", - serviceID: "svc", - hostID: "h1", - want: "db-svc-h1", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := GenerateServiceInstanceID(tt.databaseID, tt.serviceID, tt.hostID) - if got != tt.want { - t.Errorf("GenerateServiceInstanceID() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestGenerateDatabaseNetworkID(t *testing.T) { - tests := []struct { - name string - databaseID string - want string - }{ - { - name: "standard database", - databaseID: "db1", - want: "db1", - }, - { - name: "multi-part database ID", - databaseID: "my-database", - want: "my-database", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := GenerateDatabaseNetworkID(tt.databaseID) - if got != tt.want { - t.Errorf("GenerateDatabaseNetworkID() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/server/internal/orchestrator/swarm/orchestrator.go b/server/internal/orchestrator/swarm/orchestrator.go index b7878557..641d0896 100644 --- a/server/internal/orchestrator/swarm/orchestrator.go +++ b/server/internal/orchestrator/swarm/orchestrator.go @@ -445,30 +445,6 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan Allocator: o.dbNetworkAllocator, } - // Service user role resources (manages database user lifecycle). - // 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 == "rag" { - canonicalROID = ServiceUserRoleIdentifier(spec.ServiceSpec.ServiceID, ServiceUserRoleRO) - canonicalRWID = ServiceUserRoleIdentifier(spec.ServiceSpec.ServiceID, ServiceUserRoleRW) - serviceUserRoleRO = &ServiceUserRole{ - ServiceID: spec.ServiceSpec.ServiceID, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: spec.NodeName, - Mode: ServiceUserRoleRO, - } - serviceUserRoleRW = &ServiceUserRole{ - ServiceID: spec.ServiceSpec.ServiceID, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: spec.NodeName, - Mode: ServiceUserRoleRW, - } - } - // Service data directory resource (host-side bind mount directory) dataDirID := spec.ServiceInstanceID + "-data" @@ -565,7 +541,6 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan Hostname: serviceName, CohortMemberID: o.swarmNodeID, ServiceImage: serviceImage, - Credentials: spec.Credentials, DatabaseNetworkID: databaseNetwork.Name, DatabaseHosts: spec.DatabaseHosts, TargetSessionAttrs: spec.TargetSessionAttrs, @@ -586,40 +561,15 @@ func (o *Orchestrator) generateMCPInstanceResources(spec *database.ServiceInstan // Build the full resource list. orchestratorResources := []resource.Resource{databaseNetwork} - if serviceUserRoleRO != nil { - orchestratorResources = append(orchestratorResources, serviceUserRoleRO, serviceUserRoleRW) - } orchestratorResources = append(orchestratorResources, serviceSpecificResources...) orchestratorResources = append(orchestratorResources, serviceInstanceSpec, serviceInstance) - // 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, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: nodeInst.NodeName, - Mode: ServiceUserRoleRO, - CredentialSource: &canonicalROID, - }, - &ServiceUserRole{ - ServiceID: spec.ServiceSpec.ServiceID, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - NodeName: nodeInst.NodeName, - Mode: ServiceUserRoleRW, - CredentialSource: &canonicalRWID, - }, - ) - } - if spec.ServiceSpec.ServiceType == "postgrest" { + // Append per-node PostgREST authenticator resources for each additional database node. + if spec.ServiceSpec.ServiceType == "postgrest" { + for _, nodeInst := range spec.DatabaseNodes { + if nodeInst.NodeName == spec.NodeName { + continue + } orchestratorResources = append(orchestratorResources, &PostgRESTAuthenticatorResource{ ServiceID: spec.ServiceSpec.ServiceID, @@ -661,8 +611,7 @@ func (o *Orchestrator) buildServiceInstanceResources(spec *database.ServiceInsta } // generateRAGInstanceResources returns the resources needed for one RAG service -// instance. RAG only requires read access, so a single ServiceUserRoleRO is -// created per database node using the same canonical+per-node pattern as MCP. +// instance. func (o *Orchestrator) generateRAGInstanceResources(spec *database.ServiceInstanceSpec) (*database.ServiceInstanceResources, error) { // Get service image. serviceImage, err := o.serviceVersions.GetServiceImage(spec.ServiceSpec.ServiceType, spec.ServiceSpec.Version) @@ -750,16 +699,15 @@ func (o *Orchestrator) generateRAGInstanceResources(spec *database.ServiceInstan // KeysDirID is the parent data dir; the actual keys subdir path is derived at runtime. serviceName := ServiceInstanceName(spec.DatabaseID, spec.ServiceSpec.ServiceID, spec.HostID) serviceInstanceSpec := &ServiceInstanceSpecResource{ - ServiceInstanceID: spec.ServiceInstanceID, - ServiceSpec: spec.ServiceSpec, - DatabaseID: spec.DatabaseID, - DatabaseName: spec.DatabaseName, - HostID: spec.HostID, - ServiceName: serviceName, - Hostname: serviceName, - CohortMemberID: o.swarmNodeID, + ServiceInstanceID: spec.ServiceInstanceID, + ServiceSpec: spec.ServiceSpec, + DatabaseID: spec.DatabaseID, + DatabaseName: spec.DatabaseName, + HostID: spec.HostID, + ServiceName: serviceName, + Hostname: serviceName, + CohortMemberID: o.swarmNodeID, ServiceImage: serviceImage, - Credentials: spec.Credentials, DatabaseNetworkID: databaseNetwork.Name, DatabaseHosts: spec.DatabaseHosts, TargetSessionAttrs: spec.TargetSessionAttrs, diff --git a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go index 6cbf2521..53ff1732 100644 --- a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go +++ b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/rs/zerolog" "github.com/samber/do" @@ -17,6 +18,12 @@ var _ resource.Resource = (*PostgRESTAuthenticatorResource)(nil) const ResourceTypePostgRESTAuthenticator resource.Type = "swarm.postgrest_authenticator" +// sanitizeIdentifier quotes a string for use as a PostgreSQL identifier. +// It doubles any internal double-quotes and wraps the result in double-quotes. +func sanitizeIdentifier(name string) string { + return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` +} + func PostgRESTAuthenticatorIdentifier(serviceID, nodeName string) resource.Identifier { return resource.Identifier{ ID: serviceID + "-auth-" + nodeName, @@ -135,7 +142,7 @@ func (r *PostgRESTAuthenticatorResource) Create(ctx context.Context, rc *resourc statements := postgres.Statements{ postgres.Statement{SQL: fmt.Sprintf("ALTER ROLE %s WITH NOINHERIT;", sanitizeIdentifier(username))}, // #nosec G201 -- sanitizeIdentifier quotes all identifiers postgres.Statement{SQL: fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s;", sanitizeIdentifier(r.DatabaseName), sanitizeIdentifier(username))}, // #nosec G201 - postgres.Statement{SQL: fmt.Sprintf("GRANT %s TO %s;", sanitizeIdentifier(anonRole), sanitizeIdentifier(username))}, // #nosec G201 + postgres.Statement{SQL: fmt.Sprintf("GRANT %s TO %s;", sanitizeIdentifier(anonRole), sanitizeIdentifier(username))}, // #nosec G201 } if err := statements.Exec(ctx, tx); err != nil { return fmt.Errorf("failed to configure PostgREST authenticator %q: %w", username, err) diff --git a/server/internal/orchestrator/swarm/rag_instance_resources_test.go b/server/internal/orchestrator/swarm/rag_instance_resources_test.go index 7a4fcbf0..aa4d3355 100644 --- a/server/internal/orchestrator/swarm/rag_instance_resources_test.go +++ b/server/internal/orchestrator/swarm/rag_instance_resources_test.go @@ -114,7 +114,6 @@ func TestGenerateRAGInstanceResources_MultiNode(t *testing.T) { result, err := o.generateRAGInstanceResources(spec) require.NoError(t, err) - // Multi-node with connect_as: no ServiceUserRole resources regardless of node count. // Network + DirResource + Keys + Preflight + Config + InstanceSpec + ServiceInstance = 7. require.Len(t, result.Resources, 7) assert.Equal(t, ResourceTypeNetwork, result.Resources[0].Identifier.Type) diff --git a/server/internal/orchestrator/swarm/resources.go b/server/internal/orchestrator/swarm/resources.go index 663f5866..53873cad 100644 --- a/server/internal/orchestrator/swarm/resources.go +++ b/server/internal/orchestrator/swarm/resources.go @@ -7,7 +7,6 @@ func RegisterResourceTypes(registry *resource.Registry) { resource.RegisterResourceType[*PostgresService](registry, ResourceTypePostgresService) resource.RegisterResourceType[*ServiceInstanceSpecResource](registry, ResourceTypeServiceInstanceSpec) resource.RegisterResourceType[*ServiceInstanceResource](registry, ResourceTypeServiceInstance) - resource.RegisterResourceType[*ServiceUserRole](registry, ResourceTypeServiceUserRole) resource.RegisterResourceType[*Network](registry, ResourceTypeNetwork) resource.RegisterResourceType[*EtcdCreds](registry, ResourceTypeEtcdCreds) resource.RegisterResourceType[*PatroniConfig](registry, ResourceTypePatroniConfig) diff --git a/server/internal/orchestrator/swarm/service_instance_spec.go b/server/internal/orchestrator/swarm/service_instance_spec.go index 3eac142b..2b46793f 100644 --- a/server/internal/orchestrator/swarm/service_instance_spec.go +++ b/server/internal/orchestrator/swarm/service_instance_spec.go @@ -34,7 +34,6 @@ type ServiceInstanceSpecResource struct { Hostname string `json:"hostname"` CohortMemberID string `json:"cohort_member_id"` ServiceImage *ServiceImage `json:"service_image"` - Credentials *database.ServiceUser `json:"credentials"` DatabaseNetworkID string `json:"database_network_id"` DatabaseHosts []database.ServiceHostEntry `json:"database_hosts"` // Ordered Postgres host:port entries TargetSessionAttrs string `json:"target_session_attrs"` // libpq target_session_attrs @@ -91,25 +90,12 @@ func (s *ServiceInstanceSpecResource) TypeDependencies() []resource.Type { return nil } -func (s *ServiceInstanceSpecResource) populateCredentials(rc *resource.Context) error { - // 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 -} - func (s *ServiceInstanceSpecResource) Refresh(ctx context.Context, rc *resource.Context) error { network, err := resource.FromContext[*Network](rc, NetworkResourceIdentifier(s.DatabaseNetworkID)) if err != nil { return fmt.Errorf("failed to get database network from state: %w", err) } - // Credentials are nil for all current service types — they use config files. - if err := s.populateCredentials(rc); err != nil { - return err - } - // Resolve the data directory path from the DirResource (only if one exists). var dataPath string if s.DataDirID != "" { @@ -135,7 +121,6 @@ func (s *ServiceInstanceSpecResource) Refresh(ctx context.Context, rc *resource. Hostname: s.Hostname, CohortMemberID: s.CohortMemberID, ServiceImage: s.ServiceImage, - Credentials: s.Credentials, DatabaseNetworkID: network.NetworkID, DatabaseHosts: s.DatabaseHosts, TargetSessionAttrs: s.TargetSessionAttrs, diff --git a/server/internal/orchestrator/swarm/service_spec.go b/server/internal/orchestrator/swarm/service_spec.go index 8315a472..f1187448 100644 --- a/server/internal/orchestrator/swarm/service_spec.go +++ b/server/internal/orchestrator/swarm/service_spec.go @@ -63,7 +63,6 @@ type ServiceContainerSpecOptions struct { Hostname string CohortMemberID string ServiceImage *ServiceImage - Credentials *database.ServiceUser DatabaseNetworkID string DatabaseHosts []database.ServiceHostEntry // Ordered Postgres host:port entries TargetSessionAttrs string // libpq target_session_attrs diff --git a/server/internal/orchestrator/swarm/service_spec_test.go b/server/internal/orchestrator/swarm/service_spec_test.go index 40b942ad..f747b344 100644 --- a/server/internal/orchestrator/swarm/service_spec_test.go +++ b/server/internal/orchestrator/swarm/service_spec_test.go @@ -51,11 +51,6 @@ func TestServiceContainerSpec(t *testing.T) { ServiceImage: &ServiceImage{ Tag: "ghcr.io/pgedge/postgres-mcp:latest", }, - Credentials: &database.ServiceUser{ - Username: "svc_db1mcp", - Password: "testpassword", - Role: "pgedge_application_read_only", - }, DatabaseNetworkID: "db1-database", Port: intPtr(8080), DataPath: "/var/lib/pgedge/services/db1-mcp-server-host1", @@ -333,7 +328,6 @@ func TestServiceContainerSpec_ExtraNetworks(t *testing.T) { Hostname: "mcp-host1", CohortMemberID: "node-123", ServiceImage: &ServiceImage{Tag: "ghcr.io/pgedge/postgres-mcp:latest"}, - Credentials: &database.ServiceUser{Username: "svc_mcp", Password: "pw"}, DatabaseNetworkID: "db1-database", Port: intPtr(8080), DataPath: "/var/lib/pgedge/services/db1-mcp-host1", @@ -401,7 +395,6 @@ func TestServiceContainerSpec_RAGHasConfigVersionEnv(t *testing.T) { Hostname: "rag-host1", CohortMemberID: "node-123", ServiceImage: &ServiceImage{Tag: "ghcr.io/pgedge/rag-server:latest"}, - Credentials: &database.ServiceUser{Username: "svc_rag", Password: "pw"}, DatabaseNetworkID: "db1-database", Port: intPtr(0), DataPath: "/var/lib/pgedge/services/db1-rag-host1", @@ -452,7 +445,6 @@ func TestServiceContainerSpec_NoExtraNetworks(t *testing.T) { Hostname: "mcp-host1", CohortMemberID: "node-123", ServiceImage: &ServiceImage{Tag: "ghcr.io/pgedge/postgres-mcp:latest"}, - Credentials: &database.ServiceUser{Username: "svc_mcp", Password: "pw"}, DatabaseNetworkID: "db1-database", Port: intPtr(8080), DataPath: "/var/lib/pgedge/services/db1-mcp-host1", @@ -486,9 +478,6 @@ func makePostgRESTSpecOpts() *ServiceContainerSpecOptions { Hostname: "postgrest-host1", CohortMemberID: "node-abc", ServiceImage: &ServiceImage{Tag: "postgrest/postgrest:latest"}, - // 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", @@ -555,7 +544,7 @@ func TestServiceContainerSpec_PostgREST_EnvVars(t *testing.T) { t.Errorf("env var %s = %q, want %q", key, got, want) } } - // Credentials and connection details must not appear as env vars. + // Connection details must not appear as env vars. for _, forbidden := range []string{"PGUSER", "PGPASSWORD", "PGHOST", "PGPORT", "PGDATABASE", "PGRST_DB_URI", "PGTARGETSESSIONATTRS"} { if _, ok := envMap[forbidden]; ok { t.Errorf("env var %s must not be set (credentials belong in postgrest.conf)", forbidden) diff --git a/server/internal/orchestrator/swarm/service_user_role.go b/server/internal/orchestrator/swarm/service_user_role.go deleted file mode 100644 index df5e5634..00000000 --- a/server/internal/orchestrator/swarm/service_user_role.go +++ /dev/null @@ -1,263 +0,0 @@ -package swarm - -import ( - "context" - "fmt" - "strings" - - "github.com/rs/zerolog" - "github.com/samber/do" - - "github.com/pgEdge/control-plane/server/internal/database" - "github.com/pgEdge/control-plane/server/internal/postgres" - "github.com/pgEdge/control-plane/server/internal/resource" - "github.com/pgEdge/control-plane/server/internal/utils" -) - -var _ resource.Resource = (*ServiceUserRole)(nil) - -const ResourceTypeServiceUserRole resource.Type = "swarm.service_user_role" - -const ( - ServiceUserRoleRO = "ro" - ServiceUserRoleRW = "rw" -) - -// sanitizeIdentifier quotes a string for use as a PostgreSQL identifier. -// It doubles any internal double-quotes and wraps the result in double-quotes. -func sanitizeIdentifier(name string) string { - return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` -} - -func ServiceUserRoleIdentifier(serviceInstanceID string, mode string) resource.Identifier { - return resource.Identifier{ - ID: serviceInstanceID + "-" + mode, - Type: ResourceTypeServiceUserRole, - } -} - -func ServiceUserRolePerNodeIdentifier(serviceID, mode, nodeName string) resource.Identifier { - return resource.Identifier{ - ID: serviceID + "-" + mode + "-" + nodeName, - Type: ResourceTypeServiceUserRole, - } -} - -// ServiceUserRole manages the lifecycle of a database user for a service. -// -// Two ServiceUserRole resources are created per service: one with Mode set to -// ServiceUserRoleRO (read-only) and one with Mode set to ServiceUserRoleRW -// (read-write). Mode determines which group role the user is granted membership -// in: pgedge_application_read_only for RO, or pgedge_application for RW. All -// permissions are inherited via the group role — no custom grants are applied. -// -// On Create, a deterministic username (incorporating the mode) and a random -// password are generated, the Postgres role is created with the appropriate -// group role membership, and the credentials are stored in the resource state. -// On subsequent reconciliation cycles, credentials are reused from the persisted -// state (no password regeneration). -// -// When CredentialSource is nil, this is the canonical resource: it generates -// credentials and runs on the first node. When CredentialSource is non-nil, -// this is a per-node resource: it reads credentials from the canonical resource -// and runs on its own node's primary. -type ServiceUserRole struct { - ServiceID string `json:"service_id"` - DatabaseID string `json:"database_id"` - DatabaseName string `json:"database_name"` - NodeName string `json:"node_name"` // Database node name for PrimaryExecutor routing - Mode string `json:"mode"` // ServiceUserRoleRO or ServiceUserRoleRW - Username string `json:"username"` - Password string `json:"password"` // Generated on Create, persisted in state - CredentialSource *resource.Identifier `json:"credential_source,omitempty"` -} - -func (r *ServiceUserRole) ResourceVersion() string { - return "4" -} - -func (r *ServiceUserRole) DiffIgnore() []string { - return []string{ - "/node_name", - "/mode", - "/service_type", - "/db_anon_role", - "/username", - "/password", - "/credential_source", - } -} - -func (r *ServiceUserRole) Identifier() resource.Identifier { - if r.CredentialSource != nil { - return ServiceUserRolePerNodeIdentifier(r.ServiceID, r.Mode, r.NodeName) - } - return ServiceUserRoleIdentifier(r.ServiceID, r.Mode) -} - -func (r *ServiceUserRole) Executor() resource.Executor { - return resource.PrimaryExecutor(r.NodeName) -} - -func (r *ServiceUserRole) Dependencies() []resource.Identifier { - nodeID := database.NodeResourceIdentifier(r.NodeName) - if r.CredentialSource != nil { - return []resource.Identifier{nodeID, *r.CredentialSource} - } - return []resource.Identifier{nodeID} -} - -func (r *ServiceUserRole) TypeDependencies() []resource.Type { - return nil -} - -func (r *ServiceUserRole) Refresh(ctx context.Context, rc *resource.Context) error { - // If username or password is empty, the resource state is from before we - // added credential management. Return ErrNotFound to trigger recreation. - if r.Username == "" || r.Password == "" { - return resource.ErrNotFound - } - - // Verify the role actually exists in pg_roles. This guards against the role - // being dropped externally, or a failed Create that left the state persisted - // but the role absent from Postgres. - primary, err := database.GetPrimaryInstance(ctx, rc, r.NodeName) - if err != nil { - return fmt.Errorf("failed to get primary instance: %w", err) - } - conn, err := primary.Connection(ctx, rc, "postgres") - if err != nil { - return fmt.Errorf("failed to connect to postgres on node %s: %w", r.NodeName, err) - } - defer conn.Close(ctx) - - needsCreate, err := postgres.UserRoleNeedsCreate(r.Username).Scalar(ctx, conn) - if err != nil { - logger, logErr := do.Invoke[zerolog.Logger](rc.Injector) - if logErr == nil { - logger.Warn().Err(err).Str("username", r.Username).Msg("pg_roles query failed during service user role refresh") - } - return fmt.Errorf("pg_roles query failed: %w", err) - } - if needsCreate { - return resource.ErrNotFound - } - return nil -} - -func (r *ServiceUserRole) Create(ctx context.Context, rc *resource.Context) error { - logger, err := do.Invoke[zerolog.Logger](rc.Injector) - if err != nil { - return err - } - logger = logger.With(). - Str("service_id", r.ServiceID). - Str("database_id", r.DatabaseID). - Logger() - logger.Info().Msg("creating service user role") - - if r.CredentialSource != nil { - // Per-node resource: read credentials from the canonical resource in state. - canonical, err := resource.FromContext[*ServiceUserRole](rc, *r.CredentialSource) - if err != nil { - return fmt.Errorf("canonical service user role %s must be created before per-node role: %w", r.CredentialSource, err) - } - r.Username = canonical.Username - r.Password = canonical.Password - } else { - // Canonical resource: generate credentials. - r.Username = database.GenerateServiceUsername(r.ServiceID, r.Mode) - password, err := utils.RandomString(32) - if err != nil { - return fmt.Errorf("failed to generate password: %w", err) - } - r.Password = password - } - - if err := r.createUserRole(ctx, rc); err != nil { - return fmt.Errorf("failed to create service user role: %w", err) - } - - logger.Info().Str("username", r.Username).Msg("service user role created successfully") - return nil -} - -func (r *ServiceUserRole) createUserRole(ctx context.Context, rc *resource.Context) error { - primary, err := database.GetPrimaryInstance(ctx, rc, r.NodeName) - if err != nil { - return fmt.Errorf("failed to get primary instance: %w", err) - } - conn, err := primary.Connection(ctx, rc, "postgres") - if err != nil { - return fmt.Errorf("failed to connect to database postgres on node %s: %w", r.NodeName, err) - } - defer conn.Close(ctx) - - var groupRole string - switch r.Mode { - case ServiceUserRoleRO: - groupRole = "pgedge_application_read_only" - case ServiceUserRoleRW: - groupRole = "pgedge_application" - default: - return fmt.Errorf("unknown service user role mode: %q", r.Mode) - } - statements, err := postgres.CreateUserRole(postgres.UserRoleOptions{ - Name: r.Username, - Password: r.Password, - Attributes: []string{"LOGIN"}, - Roles: []string{groupRole}, - }) - if err != nil { - return fmt.Errorf("failed to generate create user role statements: %w", err) - } - if err := statements.Exec(ctx, conn); err != nil { - return fmt.Errorf("failed to create service user: %w", err) - } - - return nil -} - -func (r *ServiceUserRole) Update(ctx context.Context, rc *resource.Context) error { - return nil -} - -func (r *ServiceUserRole) Delete(ctx context.Context, rc *resource.Context) error { - logger, err := do.Invoke[zerolog.Logger](rc.Injector) - if err != nil { - return err - } - logger = logger.With(). - Str("service_id", r.ServiceID). - Str("database_id", r.DatabaseID). - Str("username", r.Username). - Logger() - logger.Info().Msg("deleting service user from database") - - primary, err := database.GetPrimaryInstance(ctx, rc, r.NodeName) - if err != nil { - // During deletion, connection failures are non-fatal — the database - // may already be gone or unreachable. - logger.Warn().Err(err).Msg("failed to get primary instance, skipping user deletion") - return nil - } - conn, err := primary.Connection(ctx, rc, "postgres") - if err != nil { - logger.Warn().Err(err).Msg("failed to connect to primary instance, skipping user deletion") - return nil - } - defer conn.Close(ctx) - - // Drop the user role - // Using IF EXISTS to handle cases where the user was already dropped manually - _, err = conn.Exec(ctx, fmt.Sprintf("DROP ROLE IF EXISTS %s", sanitizeIdentifier(r.Username))) - if err != nil { - logger.Warn().Err(err).Msg("failed to drop user role, continuing anyway") - // Don't fail the deletion if we can't drop the user - this prevents - // the resource from getting stuck in a failed state - return nil - } - - logger.Info().Msg("service user deleted successfully") - return nil -} diff --git a/server/internal/orchestrator/swarm/service_user_role_test.go b/server/internal/orchestrator/swarm/service_user_role_test.go deleted file mode 100644 index 5f0daf7a..00000000 --- a/server/internal/orchestrator/swarm/service_user_role_test.go +++ /dev/null @@ -1,214 +0,0 @@ -package swarm - -import ( - "fmt" - "testing" - - "github.com/pgEdge/control-plane/server/internal/database" - "github.com/pgEdge/control-plane/server/internal/resource" -) - -func TestServiceUserRoleIdentifier(t *testing.T) { - t.Run("canonical resource uses service+mode identifier", func(t *testing.T) { - r := &ServiceUserRole{ - ServiceID: "svc-abc", - Mode: ServiceUserRoleRO, - } - got := r.Identifier() - want := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRO) - if got != want { - t.Errorf("Identifier() = %v, want %v", got, want) - } - }) - - t.Run("per-node resource uses service+mode+node identifier", func(t *testing.T) { - canonicalID := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRW) - r := &ServiceUserRole{ - ServiceID: "svc-abc", - Mode: ServiceUserRoleRW, - NodeName: "n2", - CredentialSource: &canonicalID, - } - got := r.Identifier() - want := ServiceUserRolePerNodeIdentifier("svc-abc", ServiceUserRoleRW, "n2") - if got != want { - t.Errorf("Identifier() = %v, want %v", got, want) - } - }) - - t.Run("canonical and per-node identifiers are distinct", func(t *testing.T) { - canonical := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRO) - perNode := ServiceUserRolePerNodeIdentifier("svc-abc", ServiceUserRoleRO, "n1") - if canonical == perNode { - t.Errorf("canonical and per-node identifiers should differ, both = %v", canonical) - } - }) -} - -func TestServiceUserRolePerNodeIdentifier(t *testing.T) { - t.Run("format is service-mode-node", func(t *testing.T) { - got := ServiceUserRolePerNodeIdentifier("svc-abc", "ro", "n2") - want := resource.Identifier{ - ID: "svc-abc-ro-n2", - Type: ResourceTypeServiceUserRole, - } - if got != want { - t.Errorf("ServiceUserRolePerNodeIdentifier() = %v, want %v", got, want) - } - }) - - t.Run("different nodes produce different identifiers", func(t *testing.T) { - id1 := ServiceUserRolePerNodeIdentifier("svc-abc", "ro", "n1") - id2 := ServiceUserRolePerNodeIdentifier("svc-abc", "ro", "n2") - if id1 == id2 { - t.Errorf("different nodes should produce different identifiers, both = %v", id1) - } - }) -} - -func TestServiceUserRoleDependencies(t *testing.T) { - t.Run("canonical resource depends only on its node", func(t *testing.T) { - r := &ServiceUserRole{ - ServiceID: "svc-abc", - NodeName: "n1", - Mode: ServiceUserRoleRO, - } - deps := r.Dependencies() - nodeID := database.NodeResourceIdentifier("n1") - if len(deps) != 1 { - t.Fatalf("canonical resource Dependencies() = %v, want 1 dependency", deps) - } - if deps[0] != nodeID { - t.Errorf("canonical resource dependency = %v, want %v", deps[0], nodeID) - } - }) - - t.Run("per-node resource depends on node and canonical", func(t *testing.T) { - canonicalID := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRO) - r := &ServiceUserRole{ - ServiceID: "svc-abc", - Mode: ServiceUserRoleRO, - NodeName: "n2", - CredentialSource: &canonicalID, - } - deps := r.Dependencies() - if len(deps) != 2 { - t.Fatalf("per-node resource Dependencies() = %v, want 2 dependencies", deps) - } - nodeID := database.NodeResourceIdentifier("n2") - if deps[0] != nodeID { - t.Errorf("per-node resource deps[0] = %v, want node %v", deps[0], nodeID) - } - if deps[1] != canonicalID { - t.Errorf("per-node resource deps[1] = %v, want canonical %v", deps[1], canonicalID) - } - }) -} - -// buildServiceUserRoles replicates the orchestrator's per-node resource -// construction logic so it can be tested without a full Orchestrator instance. -func buildServiceUserRoles(serviceID, databaseID, databaseName, firstNodeName string, extraNodeNames []string) []*ServiceUserRole { - canonicalROID := ServiceUserRoleIdentifier(serviceID, ServiceUserRoleRO) - canonicalRWID := ServiceUserRoleIdentifier(serviceID, ServiceUserRoleRW) - - roles := []*ServiceUserRole{ - {ServiceID: serviceID, DatabaseID: databaseID, DatabaseName: databaseName, NodeName: firstNodeName, Mode: ServiceUserRoleRO}, - {ServiceID: serviceID, DatabaseID: databaseID, DatabaseName: databaseName, NodeName: firstNodeName, Mode: ServiceUserRoleRW}, - } - for _, nodeName := range extraNodeNames { - roles = append(roles, - &ServiceUserRole{ServiceID: serviceID, DatabaseID: databaseID, DatabaseName: databaseName, NodeName: nodeName, Mode: ServiceUserRoleRO, CredentialSource: &canonicalROID}, - &ServiceUserRole{ServiceID: serviceID, DatabaseID: databaseID, DatabaseName: databaseName, NodeName: nodeName, Mode: ServiceUserRoleRW, CredentialSource: &canonicalRWID}, - ) - } - return roles -} - -func TestServiceUserRolePerNodeProvisioning(t *testing.T) { - tests := []struct { - name string - extraNodes []string - wantTotal int - wantCanonical int - wantPerNode int - }{ - {"single node", nil, 2, 2, 0}, - {"two nodes", []string{"n2"}, 4, 2, 2}, - {"three nodes", []string{"n2", "n3"}, 6, 2, 4}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - roles := buildServiceUserRoles("svc-abc", "db-abc", "appdb", "n1", tt.extraNodes) - - if len(roles) != tt.wantTotal { - t.Errorf("got %d ServiceUserRole resources, want %d", len(roles), tt.wantTotal) - } - - var canonicals, perNodes []*ServiceUserRole - for _, r := range roles { - if r.CredentialSource == nil { - canonicals = append(canonicals, r) - } else { - perNodes = append(perNodes, r) - } - } - - if len(canonicals) != tt.wantCanonical { - t.Errorf("got %d canonical resources, want %d", len(canonicals), tt.wantCanonical) - } - if len(perNodes) != tt.wantPerNode { - t.Errorf("got %d per-node resources, want %d", len(perNodes), tt.wantPerNode) - } - - // Verify per-node resources reference the correct canonical identifier. - canonicalROID := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRO) - canonicalRWID := ServiceUserRoleIdentifier("svc-abc", ServiceUserRoleRW) - for _, pn := range perNodes { - var wantSource resource.Identifier - switch pn.Mode { - case ServiceUserRoleRO: - wantSource = canonicalROID - case ServiceUserRoleRW: - wantSource = canonicalRWID - default: - t.Errorf("unexpected mode %q", pn.Mode) - continue - } - if *pn.CredentialSource != wantSource { - t.Errorf("per-node %s CredentialSource = %v, want %v", pn.Mode, *pn.CredentialSource, wantSource) - } - } - - // Verify per-node resources are routed to the correct node. - for i, nodeName := range tt.extraNodes { - roRole := perNodes[i*2] - rwRole := perNodes[i*2+1] - if roRole.NodeName != nodeName { - t.Errorf("per-node RO role[%d].NodeName = %q, want %q", i, roRole.NodeName, nodeName) - } - if rwRole.NodeName != nodeName { - t.Errorf("per-node RW role[%d].NodeName = %q, want %q", i, rwRole.NodeName, nodeName) - } - } - }) - } -} - -func TestServiceUserRolePerNodeIdentifierUniqueness(t *testing.T) { - // All identifiers across a 3-node cluster must be unique. - nodes := []string{"n1", "n2", "n3"} - roles := buildServiceUserRoles("svc-abc", "db-abc", "appdb", nodes[0], nodes[1:]) - - seen := make(map[resource.Identifier]string) - for i, r := range roles { - id := r.Identifier() - if prev, exists := seen[id]; exists { - t.Errorf("duplicate identifier %v: role[%d] and %s", id, i, prev) - } - seen[id] = fmt.Sprintf("role[%d] node=%s mode=%s", i, r.NodeName, r.Mode) - } -} - - - diff --git a/server/internal/workflows/plan_update.go b/server/internal/workflows/plan_update.go index 760f9599..f8947f97 100644 --- a/server/internal/workflows/plan_update.go +++ b/server/internal/workflows/plan_update.go @@ -61,7 +61,6 @@ func (w *Workflows) PlanUpdate(ctx workflow.Context, input *PlanUpdateInput) (*P } // Generate service instance resources. - // Use first node as canonical node for ServiceUserRole credential generation. var nodeName string if len(nodeInstances) > 0 { nodeName = nodeInstances[0].NodeName