feat: add postgrest as a supported service type#298
Conversation
Add PostgREST to the Control Plane API as a valid service type with config validation. All config fields are optional with sensible defaults. Changes: - Add "postgrest" to the service_type enum in Goa DSL and regenerate API types - Replace hardcoded MCP-only check with allowlist-based service type validation and switch-based config dispatch - Implement ParsePostgRESTServiceConfig() with fields: db_schemas, db_anon_role, db_pool, max_rows, jwt_secret, jwt_aud, jwt_role_claim_key, server_cors_allowed_origins PLAT-498
…st validation tests
📝 WalkthroughWalkthroughThis PR extends service type support by adding "postgrest" alongside the existing "mcp" type. It introduces PostgREST service configuration validation, parsing logic, and comprehensive tests. The orchestrator provisioning remains restricted to MCP services pending future work. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/internal/api/apiv1/validate.go`:
- Around line 252-257: The validation currently allows "postgrest" but the swarm
orchestrator only supports "mcp", so update the allowlist in validate.go to only
permit "mcp" (remove "postgrest") wherever supportedServiceTypes is defined (the
occurrences around svc.ServiceType checks at the shown block and the similar
block at lines 283-287), ensuring validation appends a newValidationError via
appendPath(path, "service_type") for any non-"mcp" values so invalid inputs are
rejected before provisioning.
In `@server/internal/database/postgrest_service_config.go`:
- Around line 54-55: Current code sets dbAnonRole :=
"pgedge_application_read_only" which implicitly enables anonymous access; change
to make anonymous access opt-in by default (set dbAnonRole to empty string or
nil) and remove the implicit default role. Also update the related block that
handles jwtSecret (jwtSecret variable/logic used around lines 95–100) so that if
dbAnonRole is non-empty you require/validate a jwtSecret is explicitly provided,
otherwise leave jwtSecret optional; ensure any code that serializes or writes
PostgREST config omits db_anon_role when dbAnonRole is empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53c6c627-2897-419c-ab51-6999bfdc3769
⛔ Files ignored due to path filters (6)
api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/types.gois excluded by!**/gen/**api/apiv1/gen/http/openapi.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi.yamlis excluded by!**/gen/**api/apiv1/gen/http/openapi3.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi3.yamlis excluded by!**/gen/**
📒 Files selected for processing (6)
api/apiv1/design/database.goserver/internal/api/apiv1/validate.goserver/internal/api/apiv1/validate_test.goserver/internal/database/postgrest_service_config.goserver/internal/database/postgrest_service_config_test.goserver/internal/orchestrator/swarm/orchestrator.go
| // Validate service_type allowlist | ||
| supportedServiceTypes := []string{"mcp", "postgrest"} | ||
| if !slices.Contains(supportedServiceTypes, svc.ServiceType) { | ||
| err := fmt.Errorf("unsupported service type %q (supported: %s)", | ||
| svc.ServiceType, strings.Join(supportedServiceTypes, ", ")) | ||
| errs = append(errs, newValidationError(err, appendPath(path, "service_type"))) |
There was a problem hiding this comment.
Reject postgrest here until the provisioning path exists.
This now admits postgrest as a valid API input, but server/internal/orchestrator/swarm/orchestrator.go still rejects every non-mcp service type and that error is propagated straight out of GenerateServiceInstanceResources. The result is a request that passes validation and only fails once the workflow starts provisioning. Please surface this as a validation error here until the swarm implementation is ready.
Also applies to: 283-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/internal/api/apiv1/validate.go` around lines 252 - 257, The validation
currently allows "postgrest" but the swarm orchestrator only supports "mcp", so
update the allowlist in validate.go to only permit "mcp" (remove "postgrest")
wherever supportedServiceTypes is defined (the occurrences around
svc.ServiceType checks at the shown block and the similar block at lines
283-287), ensuring validation appends a newValidationError via appendPath(path,
"service_type") for any non-"mcp" values so invalid inputs are rejected before
provisioning.
| // db_anon_role — optional string, default "pgedge_application_read_only" | ||
| dbAnonRole := "pgedge_application_read_only" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "postgrest_service_config.go" | head -5Repository: pgEdge/control-plane
Length of output: 118
🏁 Script executed:
# Read the postgrest_service_config.go file to understand the full context
cat -n server/internal/database/postgrest_service_config.goRepository: pgEdge/control-plane
Length of output: 5676
🏁 Script executed:
# Search for any validation logic or initialization that might mitigate the concern
rg -A 5 -B 5 "db_anon_role|jwt_secret" server/internal/database/postgrest_service_config.goRepository: pgEdge/control-plane
Length of output: 2038
🏁 Script executed:
# Check if there's any usage of this config in tests or main code to understand security context
rg -l "postgrest_service_config\|PostgRESTServiceConfig" server/ --type go | head -10Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check for any comments or documentation about the security design
rg -B 3 -A 3 "db_anon_role|pgedge_application_read_only" server/ --type go | head -40Repository: pgEdge/control-plane
Length of output: 2959
🏁 Script executed:
# Check if there are any integration points or examples that show intended usage
rg -l "PostgRESTServiceConfig\|db_anon_role" server/ --type goRepository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Look for test cases or examples that might shed light on intended behavior
find server -name "*test.go" -type f -exec rg -l "postgrest\|PostgREST" {} \;Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check if db_anon_role can be set to empty/nil/unset anywhere
rg -A 10 -B 5 "db_anon_role.*=" server/ --type go | head -50Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check where PostgRESTServiceConfig is used and if there are any guards or warnings
rg -B 5 -A 10 "ParsePostgRESTServiceConfig\|PostgRESTServiceConfig" server/ --type go | head -80Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check if there's any documentation or comments about security implications
rg -i "anonymous|security|auth" server/internal/database/ --type goRepository: pgEdge/control-plane
Length of output: 338
🏁 Script executed:
# Look at the service instance/spec to see how this config is deployed
rg -B 5 -A 15 "postgrest\|PostgREST" server/internal/orchestrator/swarm/service_instance_spec.go 2>/dev/null | head -50Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Search for README or documentation about PostgREST setup
find . -name "README*" -o -name "*.md" | xargs grep -l -i "postgrest\|anonymous" 2>/dev/null | head -5Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check if there are environment variables or deployment configs that might override defaults
find server -name "*.go" | xargs grep -l "db_anon_role\|postgrest.*config" | head -10Repository: pgEdge/control-plane
Length of output: 217
🏁 Script executed:
# Look for any comments in the file about design decisions or security considerations
head -100 server/internal/database/postgrest_service_config.go | grep -i "optional\|default\|anonymous\|security"Repository: pgEdge/control-plane
Length of output: 917
🏁 Script executed:
# Check if there's validation that requires jwt_secret when db_anon_role is set
rg -B 10 -A 10 "jwt_secret.*required\|require.*jwt" server/internal/database/ --type goRepository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Understand PostgREST behavior by checking if there's any related documentation or config examples
find . -type f \( -name "*.md" -o -name "*.txt" \) | xargs grep -i "postgrest" 2>/dev/null | head -20Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check the test file to understand intended behavior and validation
cat -n server/internal/database/postgrest_service_config_test.goRepository: pgEdge/control-plane
Length of output: 8398
🏁 Script executed:
# Check the api validation test to see if there's any auth-related validation
rg -B 5 -A 10 "postgrest\|PostgREST" server/internal/api/apiv1/validate_test.goRepository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check how this config is actually deployed and used in the service
rg -B 5 -A 15 "DBAnonRole|db_anon_role" server/internal/orchestrator/swarm/ --type goRepository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check if there's any validation at service creation time that enforces auth
rg -B 3 -A 10 "service.*spec\|ServiceSpec" server/internal/orchestrator/swarm/service_spec.go | head -60Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Check if there's any documentation or design docs about this choice
find . -type f \( -name "*.md" -o -name "*.txt" \) -exec grep -l "anon\|anonymous\|authentication" {} \; 2>/dev/null | head -5Repository: pgEdge/control-plane
Length of output: 174
🏁 Script executed:
# Check the service credentials doc for any context about design decisions
cat -n docs/development/service-credentials.md | head -100Repository: pgEdge/control-plane
Length of output: 4868
🏁 Script executed:
# Check if there's any configuration validation at the service creation level that might mitigate this
rg -B 5 -A 15 "PostgRESTServiceConfig\|postgrest.*config" server/internal/orchestrator/swarm/service_instance_spec.go 2>/dev/null | head -80Repository: pgEdge/control-plane
Length of output: 46
🏁 Script executed:
# Look for any README content about PostgREST setup or security
cat README.md | grep -A 20 -B 5 -i "postgrest\|anonymous" 2>/dev/nullRepository: pgEdge/control-plane
Length of output: 46
Make anonymous access opt-in, not the default.
PostgREST uses db_anon_role to permit unauthenticated requests; when unset, anonymous access is blocked. The current defaults enable implicit anonymous access: an empty config applies db_anon_role = "pgedge_application_read_only" while leaving jwt_secret optional. The pgedge_application_read_only role grants SELECT on all tables across configured schemas, so a minimal configuration inadvertently exposes data without an explicit authentication decision. Either leave db_anon_role unset by default or require jwt_secret whenever it is defaulted.
Also applies to: lines 95–100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/internal/database/postgrest_service_config.go` around lines 54 - 55,
Current code sets dbAnonRole := "pgedge_application_read_only" which implicitly
enables anonymous access; change to make anonymous access opt-in by default (set
dbAnonRole to empty string or nil) and remove the implicit default role. Also
update the related block that handles jwtSecret (jwtSecret variable/logic used
around lines 95–100) so that if dbAnonRole is non-empty you require/validate a
jwtSecret is explicitly provided, otherwise leave jwtSecret optional; ensure any
code that serializes or writes PostgREST config omits db_anon_role when
dbAnonRole is empty.
rshoemaker
left a comment
rshoemaker
left a comment
There was a problem hiding this comment.
This looks pretty good as a foundational PR - no major concerns with it.
2 participants
Add PostgREST to the Control Plane API as a valid service type with config validation. All config fields are optional with sensible defaults.
Changes:
PLAT-498
Summary
Adds
postgrestas a valid service type in the Control Plane API and wires up config validation with sensible defaults for all fields.Changes
"postgrest"to theservice_typeenum in Goa DSL; regenerate API types and OpenAPI specs!= "mcp"check with an allowlist +switch-based config dispatch invalidateServiceSpecParsePostgRESTServiceConfig()with eight optional fields:db_schemas,db_anon_role,db_pool,max_rows,jwt_secret,jwt_aud,jwt_role_claim_key,server_cors_allowed_originsTesting
go test ./server/internal/database/... ./server/internal/api/apiv1/...
Checklist
Notes for Reviewers