From 5d6b26d1a875fad6b01c144c91c2af3a46332204 Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Wed, 22 Apr 2026 17:54:36 +0500 Subject: [PATCH 1/3] feat: require db_anon_role for PostgREST, remove pgedge_application_read_only default `desiredAnonRole()` previously fell back to `pgedge_application_read_only` when no anon role was supplied, blocking removal of the deprecated built-in roles in PLAT-559. `db_anon_role` is now required at validation time. Omitting it returns a 400 with `db_anon_role is required`. All tests updated accordingly. PLAT-568 --- server/internal/api/apiv1/validate_test.go | 17 ++-- .../database/postgrest_service_config.go | 10 ++- .../database/postgrest_service_config_test.go | 79 +++++++++++-------- .../swarm/postgrest_authenticator_resource.go | 5 +- .../postgrest_authenticator_resource_test.go | 4 +- .../swarm/postgrest_config_resource_test.go | 4 +- 6 files changed, 65 insertions(+), 54 deletions(-) diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index ffc37d11..0d3e7ecc 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", diff --git a/server/internal/database/postgrest_service_config.go b/server/internal/database/postgrest_service_config.go index 9c2c21e2..7fda6971 100644 --- a/server/internal/database/postgrest_service_config.go +++ b/server/internal/database/postgrest_service_config.go @@ -13,7 +13,7 @@ import ( // optional; defaults are 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"` 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) { From 7e75ed09a63a00c75866620387d2392eb26a7d1f Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Wed, 22 Apr 2026 18:03:30 +0500 Subject: [PATCH 2/3] fix: correct DBAnonRole comment and isolate PostgREST validation tests Struct comment on PostgRESTServiceConfig incorrectly stated all fields are optional with defaults. Updated to note DBAnonRole is required with no default, and added an inline field comment to match. Three validate_test.go cases (db_pool range, unknown config key, jwt_secret too short) were missing db_anon_role in their Config maps, causing an extra validation error that obscured the intended assertion. Added db_anon_role to each fixture. PLAT-568 --- server/internal/api/apiv1/validate_test.go | 9 ++++++--- server/internal/database/postgrest_service_config.go | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index 0d3e7ecc..378793da 100644 --- a/server/internal/api/apiv1/validate_test.go +++ b/server/internal/api/apiv1/validate_test.go @@ -1473,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{ @@ -1488,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{ @@ -1503,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 7fda6971..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"` + 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"` From a9237aff522272e8b3f8329a00d88f4ae3b87a79 Mon Sep 17 00:00:00 2001 From: moizpgedge Date: Wed, 22 Apr 2026 18:39:30 +0500 Subject: [PATCH 3/3] fix: update E2E PostgREST fixtures for required db_anon_role All PostgREST E2E tests were failing because db_anon_role is now required and the test fixtures did not supply it. - postgrestSpec() injects db_anon_role: "anon" when not provided - postgrestBaseSpec() adds an "anon" (NOLOGIN) user to DatabaseUsers - TestPostgRESTMultiHostDBURI and TestPostgRESTFailover get the same anon user added to their inline DatabaseSpec - TestPostgRESTAuthenticatorRole assertion updated from pgedge_application_read_only to anon PLAT-568 --- e2e/postgrest_test.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) 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{