diff --git a/e2e/postgrest_test.go b/e2e/postgrest_test.go index b18e9205..4d634aa2 100644 --- a/e2e/postgrest_test.go +++ b/e2e/postgrest_test.go @@ -16,11 +16,15 @@ import ( ) // postgrestSpec returns a ServiceSpec for a PostgREST service on the given host. -// PostgREST connects as the "admin" database user (connect_as). +// PostgREST connects as the "admin" database user (connect_as) and defaults to +// "anon" as the db_anon_role when not explicitly provided. func postgrestSpec(hostID string, port int, config map[string]any) *controlplane.ServiceSpec { if config == nil { config = map[string]any{} } + if _, ok := config["db_anon_role"]; !ok { + config["db_anon_role"] = "anon" + } return &controlplane.ServiceSpec{ ServiceID: "postgrest-api", ServiceType: "postgrest", @@ -55,6 +59,10 @@ func postgrestBaseSpec(dbName string, nodeHosts []string, services []*controlpla DbOwner: pointerTo(true), Attributes: []string{"LOGIN", "SUPERUSER"}, }, + { + Username: "anon", + Attributes: []string{"NOLOGIN"}, + }, }, Port: pointerTo(0), Nodes: nodes, @@ -307,7 +315,7 @@ func TestPostgRESTAuthenticatorRole(t *testing.T) { 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 db_anon_role (pgedge_application_read_only by default) must be granted to the connect_as user. + // The db_anon_role ("anon") must be granted to the connect_as user. var anonGranted bool err = conn.QueryRow(ctx, ` SELECT EXISTS ( @@ -316,7 +324,7 @@ func TestPostgRESTAuthenticatorRole(t *testing.T) { JOIN pg_roles r ON m.member = r.oid JOIN pg_roles g ON m.roleid = g.oid WHERE r.rolname = 'admin' - AND g.rolname = 'pgedge_application_read_only' + AND g.rolname = 'anon' ) `).Scan(&anonGranted) require.NoError(t, err) @@ -492,6 +500,10 @@ func TestPostgRESTMultiHostDBURI(t *testing.T) { DbOwner: pointerTo(true), Attributes: []string{"LOGIN", "SUPERUSER"}, }, + { + Username: "anon", + Attributes: []string{"NOLOGIN"}, + }, }, Port: pointerTo(0), Nodes: []*controlplane.DatabaseNodeSpec{ @@ -551,6 +563,10 @@ func TestPostgRESTFailover(t *testing.T) { DbOwner: pointerTo(true), Attributes: []string{"LOGIN", "SUPERUSER"}, }, + { + Username: "anon", + Attributes: []string{"NOLOGIN"}, + }, }, Port: pointerTo(0), Nodes: []*controlplane.DatabaseNodeSpec{ diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index ffc37d11..378793da 100644 --- a/server/internal/api/apiv1/validate_test.go +++ b/server/internal/api/apiv1/validate_test.go @@ -797,7 +797,7 @@ func TestValidateDatabaseSpec(t *testing.T) { HostIds: []api.Identifier{"host-1"}, ConnectAs: "app", Port: utils.PointerTo(8080), - Config: map[string]any{}, + Config: map[string]any{"db_anon_role": "web_anon"}, }, }, }, @@ -841,7 +841,7 @@ func TestValidateDatabaseSpec(t *testing.T) { HostIds: []api.Identifier{"host-2"}, ConnectAs: "app", Port: utils.PointerTo(8080), - Config: map[string]any{}, + Config: map[string]any{"db_anon_role": "web_anon"}, }, }, }, @@ -912,7 +912,7 @@ func TestValidateDatabaseSpec(t *testing.T) { Version: "1.0.0", HostIds: []api.Identifier{"host-1"}, ConnectAs: "app", - Config: map[string]any{}, + Config: map[string]any{"db_anon_role": "web_anon"}, }, }, }, @@ -953,7 +953,7 @@ func TestValidateDatabaseSpec(t *testing.T) { HostIds: []api.Identifier{"host-1"}, ConnectAs: "app", Port: utils.PointerTo(0), - Config: map[string]any{}, + Config: map[string]any{"db_anon_role": "web_anon"}, }, }, }, @@ -1341,7 +1341,7 @@ func TestValidateServiceSpec(t *testing.T) { ServiceType: "postgrest", Version: "14.5", HostIds: []api.Identifier{"host-1"}, - Config: map[string]any{}, + Config: map[string]any{"db_anon_role": "web_anon"}, }, }, { @@ -1417,13 +1417,14 @@ func TestValidateServiceSpec(t *testing.T) { }, }, { - name: "valid postgrest with nil config", + name: "postgrest with nil config fails — db_anon_role required", svc: &api.ServiceSpec{ ServiceID: "my-postgrest", ServiceType: "postgrest", Version: "latest", HostIds: []api.Identifier{"host-1"}, }, + expected: []string{"db_anon_role is required"}, }, { name: "RAG service with nil config requires pipelines", @@ -1438,7 +1439,7 @@ func TestValidateServiceSpec(t *testing.T) { }, }, { - name: "valid postgrest with defaults", + name: "postgrest with empty config fails — db_anon_role required", svc: &api.ServiceSpec{ ServiceID: "my-postgrest", ServiceType: "postgrest", @@ -1446,7 +1447,7 @@ func TestValidateServiceSpec(t *testing.T) { HostIds: []api.Identifier{"host-1"}, Config: map[string]any{}, }, - expected: []string{}, + expected: []string{"db_anon_role is required"}, }, { name: "valid postgrest with all config fields", @@ -1472,7 +1473,8 @@ func TestValidateServiceSpec(t *testing.T) { Version: "latest", HostIds: []api.Identifier{"host-1"}, Config: map[string]any{ - "db_pool": float64(99), + "db_anon_role": "anon", + "db_pool": float64(99), }, }, expected: []string{ @@ -1487,7 +1489,8 @@ func TestValidateServiceSpec(t *testing.T) { Version: "latest", HostIds: []api.Identifier{"host-1"}, Config: map[string]any{ - "invalid_key": "value", + "db_anon_role": "anon", + "invalid_key": "value", }, }, expected: []string{ @@ -1502,7 +1505,8 @@ func TestValidateServiceSpec(t *testing.T) { Version: "latest", HostIds: []api.Identifier{"host-1"}, Config: map[string]any{ - "jwt_secret": "tooshort", + "db_anon_role": "anon", + "jwt_secret": "tooshort", }, }, expected: []string{ diff --git a/server/internal/database/postgrest_service_config.go b/server/internal/database/postgrest_service_config.go index 9c2c21e2..19860f2e 100644 --- a/server/internal/database/postgrest_service_config.go +++ b/server/internal/database/postgrest_service_config.go @@ -9,11 +9,11 @@ import ( ) // PostgRESTServiceConfig is the typed internal representation of PostgREST service -// configuration. Parsed from ServiceSpec.Config map[string]any. All fields are -// optional; defaults are applied when absent. +// configuration. Parsed from ServiceSpec.Config map[string]any. DBAnonRole is +// required; all other fields are optional with defaults applied when absent. type PostgRESTServiceConfig struct { DBSchemas string `json:"db_schemas"` // default: "public" - DBAnonRole string `json:"db_anon_role"` // default: "pgedge_application_read_only" + DBAnonRole string `json:"db_anon_role"` // required: no default DBPool int `json:"db_pool"` // default: 10, range: 1-30 MaxRows int `json:"max_rows"` // default: 1000, range: 1-10000 JWTSecret *string `json:"jwt_secret,omitempty"` @@ -53,9 +53,11 @@ func ParsePostgRESTServiceConfig(config map[string]any) (*PostgRESTServiceConfig } } - // db_anon_role — optional string, default "pgedge_application_read_only" - dbAnonRole := "pgedge_application_read_only" - if v, ok := config["db_anon_role"]; ok { + // db_anon_role — required string + var dbAnonRole string + if v, ok := config["db_anon_role"]; !ok { + errs = append(errs, fmt.Errorf("db_anon_role is required")) + } else { s, sOk := v.(string) if !sOk { errs = append(errs, fmt.Errorf("db_anon_role must be a string")) diff --git a/server/internal/database/postgrest_service_config_test.go b/server/internal/database/postgrest_service_config_test.go index 3914a4e4..45b8294f 100644 --- a/server/internal/database/postgrest_service_config_test.go +++ b/server/internal/database/postgrest_service_config_test.go @@ -45,7 +45,7 @@ func makeTestConn() database.PostgRESTConnParams { func TestGenerateConf_CoreFields(t *testing.T) { cfg := &database.PostgRESTServiceConfig{ DBSchemas: "public", - DBAnonRole: "pgedge_application_read_only", + DBAnonRole: "web_anon", DBPool: 10, MaxRows: 1000, } @@ -53,7 +53,7 @@ func TestGenerateConf_CoreFields(t *testing.T) { require.NoError(t, err) m := parseConf(t, data) assert.Equal(t, "public", m["db-schemas"]) - assert.Equal(t, "pgedge_application_read_only", m["db-anon-role"]) + assert.Equal(t, "web_anon", m["db-anon-role"]) assert.Equal(t, "10", m["db-pool"]) assert.Equal(t, "1000", m["db-max-rows"]) } @@ -164,17 +164,10 @@ func TestGenerateConf_DBURIMultiHost(t *testing.T) { } func TestParsePostgRESTServiceConfig(t *testing.T) { - t.Run("defaults applied for empty config", func(t *testing.T) { - cfg, errs := database.ParsePostgRESTServiceConfig(map[string]any{}) - require.Empty(t, errs) - assert.Equal(t, "public", cfg.DBSchemas) - assert.Equal(t, "pgedge_application_read_only", cfg.DBAnonRole) - assert.Equal(t, 10, cfg.DBPool) - assert.Equal(t, 1000, cfg.MaxRows) - assert.Nil(t, cfg.JWTSecret) - assert.Nil(t, cfg.JWTAud) - assert.Nil(t, cfg.JWTRoleClaimKey) - assert.Nil(t, cfg.ServerCORSAllowedOrigins) + t.Run("db_anon_role required — empty config returns error", func(t *testing.T) { + _, errs := database.ParsePostgRESTServiceConfig(map[string]any{}) + require.Len(t, errs, 1) + assert.Contains(t, errs[0].Error(), "db_anon_role is required") }) t.Run("all fields overridden", func(t *testing.T) { @@ -205,7 +198,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_schemas wrong type", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_schemas": 123, + "db_anon_role": "web_anon", + "db_schemas": 123, }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_schemas must be a string") @@ -213,7 +207,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_schemas empty string", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_schemas": "", + "db_anon_role": "web_anon", + "db_schemas": "", }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_schemas must not be empty") @@ -237,7 +232,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_pool below range", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": float64(0), + "db_anon_role": "web_anon", + "db_pool": float64(0), }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_pool must be between 1 and 30") @@ -245,7 +241,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_pool above range", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": float64(31), + "db_anon_role": "web_anon", + "db_pool": float64(31), }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_pool must be between 1 and 30") @@ -253,7 +250,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_pool wrong type", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": "ten", + "db_anon_role": "web_anon", + "db_pool": "ten", }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_pool must be an integer") @@ -261,7 +259,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_pool non-integer float", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": float64(5.5), + "db_anon_role": "web_anon", + "db_pool": float64(5.5), }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "db_pool must be an integer") @@ -269,13 +268,15 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("db_pool boundary values", func(t *testing.T) { cfg, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": float64(1), + "db_anon_role": "web_anon", + "db_pool": float64(1), }) require.Empty(t, errs) assert.Equal(t, 1, cfg.DBPool) cfg, errs = database.ParsePostgRESTServiceConfig(map[string]any{ - "db_pool": float64(30), + "db_anon_role": "web_anon", + "db_pool": float64(30), }) require.Empty(t, errs) assert.Equal(t, 30, cfg.DBPool) @@ -283,7 +284,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("max_rows below range", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "max_rows": float64(0), + "db_anon_role": "web_anon", + "max_rows": float64(0), }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "max_rows must be between 1 and 10000") @@ -291,7 +293,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("max_rows above range", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "max_rows": float64(10001), + "db_anon_role": "web_anon", + "max_rows": float64(10001), }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "max_rows must be between 1 and 10000") @@ -299,13 +302,15 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("max_rows boundary values", func(t *testing.T) { cfg, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "max_rows": float64(1), + "db_anon_role": "web_anon", + "max_rows": float64(1), }) require.Empty(t, errs) assert.Equal(t, 1, cfg.MaxRows) cfg, errs = database.ParsePostgRESTServiceConfig(map[string]any{ - "max_rows": float64(10000), + "db_anon_role": "web_anon", + "max_rows": float64(10000), }) require.Empty(t, errs) assert.Equal(t, 10000, cfg.MaxRows) @@ -313,7 +318,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("jwt_secret too short", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "jwt_secret": "short", + "db_anon_role": "web_anon", + "jwt_secret": "short", }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "jwt_secret must be at least 32 characters") @@ -321,7 +327,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("jwt_secret exactly 32 chars", func(t *testing.T) { cfg, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "jwt_secret": "12345678901234567890123456789012", + "db_anon_role": "web_anon", + "jwt_secret": "12345678901234567890123456789012", }) require.Empty(t, errs) require.NotNil(t, cfg.JWTSecret) @@ -330,7 +337,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("jwt_secret wrong type", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "jwt_secret": 12345, + "db_anon_role": "web_anon", + "jwt_secret": 12345, }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "jwt_secret must be a string") @@ -338,7 +346,8 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("unknown config key", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "unknown_key": "value", + "db_anon_role": "web_anon", + "unknown_key": "value", }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), `unknown config key "unknown_key"`) @@ -346,8 +355,9 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("multiple unknown config keys", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "foo": "bar", - "baz": 123, + "db_anon_role": "web_anon", + "foo": "bar", + "baz": 123, }) require.Len(t, errs, 1) assert.Contains(t, errs[0].Error(), "unknown config keys:") @@ -357,9 +367,10 @@ func TestParsePostgRESTServiceConfig(t *testing.T) { t.Run("multiple errors collected", func(t *testing.T) { _, errs := database.ParsePostgRESTServiceConfig(map[string]any{ - "db_schemas": "", - "db_pool": float64(0), - "max_rows": "not-a-number", + "db_anon_role": "web_anon", + "db_schemas": "", + "db_pool": float64(0), + "max_rows": "not-a-number", }) require.Len(t, errs, 3) }) diff --git a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go index 6cbf2521..f8a0cf65 100644 --- a/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go +++ b/server/internal/orchestrator/swarm/postgrest_authenticator_resource.go @@ -66,10 +66,7 @@ func (r *PostgRESTAuthenticatorResource) TypeDependencies() []resource.Type { } func (r *PostgRESTAuthenticatorResource) desiredAnonRole() string { - if r.DBAnonRole != "" { - return r.DBAnonRole - } - return "pgedge_application_read_only" + return r.DBAnonRole } func (r *PostgRESTAuthenticatorResource) authenticatorUsername() string { diff --git a/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go b/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go index 9e31a081..0d8487aa 100644 --- a/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go +++ b/server/internal/orchestrator/swarm/postgrest_authenticator_resource_test.go @@ -56,9 +56,9 @@ func TestPostgRESTAuthenticatorResource_Dependencies(t *testing.T) { } func TestPostgRESTAuthenticatorResource_DesiredAnonRole(t *testing.T) { - t.Run("empty string falls back to default", func(t *testing.T) { + t.Run("empty string returns empty", func(t *testing.T) { r := &PostgRESTAuthenticatorResource{DBAnonRole: ""} - assert.Equal(t, "pgedge_application_read_only", r.desiredAnonRole()) + assert.Equal(t, "", r.desiredAnonRole()) }) t.Run("custom anon role is preserved", func(t *testing.T) { diff --git a/server/internal/orchestrator/swarm/postgrest_config_resource_test.go b/server/internal/orchestrator/swarm/postgrest_config_resource_test.go index cf5c0f33..fc19dd01 100644 --- a/server/internal/orchestrator/swarm/postgrest_config_resource_test.go +++ b/server/internal/orchestrator/swarm/postgrest_config_resource_test.go @@ -65,7 +65,7 @@ func TestPostgRESTConfigResource_WriteConfigFile(t *testing.T) { cfg := &database.PostgRESTServiceConfig{ DBSchemas: "public", - DBAnonRole: "pgedge_application_read_only", + DBAnonRole: "web_anon", DBPool: 10, MaxRows: 1000, } @@ -92,7 +92,7 @@ func TestPostgRESTConfigResource_WriteConfigFile(t *testing.T) { 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") + assert.Contains(t, content, "web_anon") } func TestPostgRESTConfigResource_WriteConfigFile_JWTFields(t *testing.T) {