feat: make connect_as required for all service types#356
Conversation
Remove g.Default("") and omitempty from the connect_as Goa attribute,
add it to g.Required(), and remove the MCP-only guard on
validateConnectAs. All service types (MCP, RAG, PostgREST) now require
connect_as at both the API and validation layers.
📝 WalkthroughWalkthroughThe 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 docstrings
🧪 Generate unit tests (beta)
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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | -4 |
| Duplication | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/apiv1/design/database.go (1)
789-1085: Optional: add aconnect_asexample to at least oneCreateDatabaseRequestexample.None of the
g.Example(...)payloads onCreateDatabaseRequest(orUpdateDatabaseRequest) include aservicesblock, so the generated OpenAPI docs won't showcase the now-requiredconnect_asfield. Consider adding a services example that demonstrates a validconnect_as→database_users.usernamereference so API consumers discover the requirement from the docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/apiv1/design/database.go` around lines 789 - 1085, The CreateDatabaseRequest examples (and likewise UpdateDatabaseRequest) omit a services block demonstrating the required connect_as field, so OpenAPI docs won't show how connect_as maps to database_users.username; add a services entry to at least one g.Example in CreateDatabaseRequest that includes a service with a connect_as value referencing an existing database_users username (e.g., "admin" or "storefront") to illustrate the required mapping, and mirror the same addition in UpdateDatabaseRequest examples so consumers can see a valid connect_as → database_users.username reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/apiv1/design/database.go`:
- Around line 789-1085: The CreateDatabaseRequest examples (and likewise
UpdateDatabaseRequest) omit a services block demonstrating the required
connect_as field, so OpenAPI docs won't show how connect_as maps to
database_users.username; add a services entry to at least one g.Example in
CreateDatabaseRequest that includes a service with a connect_as value
referencing an existing database_users username (e.g., "admin" or "storefront")
to illustrate the required mapping, and mirror the same addition in
UpdateDatabaseRequest examples so consumers can see a valid connect_as →
database_users.username reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8b01e9c-b736-4149-9a7a-14eede64c69a
⛔ Files ignored due to path filters (9)
api/apiv1/gen/control_plane/service.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/encode_decode.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/encode_decode.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 (2)
api/apiv1/design/database.goserver/internal/api/apiv1/validate.go
Summary
connect_asa required field onServiceSpecfor all service typesg.Default("")workaround andomitemptyfrom the Goa DSLvalidateConnectAs— validation now runs unconditionallyallow_writes+db_ownercross-validation remains MCP-specificContext
Now that RAG (#351) and PostgREST (#352) have adopted
connect_as, the temporary workarounds to make it optional can be removed.Test plan
connect_asin the required arrayconnect_ason both create-database and update-databaseconnect_as)PLAT-554