feat: add connect_as field for service database credentials#338
Merged
rshoemaker merged 5 commits intomainfrom Apr 14, 2026
Merged
feat: add connect_as field for service database credentials#338rshoemaker merged 5 commits intomainfrom
rshoemaker merged 5 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds 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)
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
|
| Category | Results |
|---|---|
| Complexity | 1 medium |
🟢 Metrics 13 complexity · -2 duplication
Metric Results Complexity 13 Duplication -2
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/orchestrator/swarm/mcp_config_resource.go`:
- Around line 55-60: Remove the connect_as credential paths from
MCPConfigResource.DiffIgnore so changes to "/connect_as_username" and
"/connect_as_password" will be detected and trigger Update; additionally, adjust
Refresh logic (related to MCPConfigResource.Refresh) to only check for the
existence of "config.yaml" and avoid rewriting runtime-owned files
("tokens.yaml" and "users.yaml") so those files remain runtime-managed while
config.yaml presence is used as the sole refresh existence check.
In `@server/internal/orchestrator/swarm/service_instance_spec.go`:
- Around line 93-96: The MCP branch in populateCredentials returns early without
clearing existing credentials, which can leave stale secrets in s.Credentials;
modify populateCredentials so that when s.ServiceSpec.ServiceType == "mcp" you
explicitly reset/clear s.Credentials (e.g., set to nil or an empty credentials
struct) before returning, ensuring old credentials are not carried forward.
In `@server/internal/resource/migrations/2_0_0.go`:
- Around line 19-35: The migration currently deletes all swarm.service_user_role
entries; instead restrict deletions to MCP-owned role state only: in
Version_2_0_0.Run, when iterating state.Resources for serviceUserRoleType and
when filtering data.Dependencies, check each resource's ownership/metadata
(e.g., an owner/managedBy field on the resource or data.Metadata) and only
remove resources and dependency edges where that ownership indicates MCP (e.g.,
managedBy == "mcp"); leave non-MCP service_user_role resources and their
dependency edges untouched so ServiceUserRole resolution in ServiceInstanceSpec
and rag_config_resource continues to work.
In `@server/internal/workflows/plan_update.go`:
- Around line 126-138: The planner currently copies the referenced user's
ConnectAsPassword from spec.DatabaseUsers into the service without validating
it; if the referenced database.User has an empty password the plan will succeed
but service DB auth will fail. Update the lookup block that handles
serviceSpec.ConnectAs in plan_update.go (where connectAsUser is resolved from
spec.DatabaseUsers) to reject empty secrets: after finding database.User (type
database.User) check that its Password (or ConnectAsPassword field used when
assigning to the service) is non-empty and return an error like "connect_as user
%q has empty password" if empty; apply the same validation for the additional
occurrence noted around lines 154-156 so the planner never copies an empty
ConnectAsPassword into the service spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab39b018-ec33-43a9-9704-06e8bac3b037
⛔ 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 (26)
api/apiv1/design/database.goe2e/service_provisioning_test.goserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/validate.goserver/internal/api/apiv1/validate_test.goserver/internal/database/operations/golden_test/TestUpdateDatabase/add_service_to_existing_database.jsonserver/internal/database/operations/golden_test/TestUpdateDatabase/remove_service_from_existing_database.jsonserver/internal/database/operations/golden_test/TestUpdateDatabase/single_node_with_service_from_empty.jsonserver/internal/database/operations/helpers_test.goserver/internal/database/service_instance.goserver/internal/database/spec.goserver/internal/orchestrator/swarm/mcp_config_resource.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/swarm/service_instance.goserver/internal/orchestrator/swarm/service_instance_spec.goserver/internal/resource/migrations/2_0_0.goserver/internal/resource/migrations/2_0_0_test.goserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/empty.jsonserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/no_nodes.jsonserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/populate_n3_with_n1_source.jsonserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/single_node_with_replicas.jsonserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/three_nodes.jsonserver/internal/resource/migrations/golden_test/TestVersion_1_0_0/with_restore_config.jsonserver/internal/resource/migrations/provide.goserver/internal/resource/state.goserver/internal/workflows/plan_update.go
💤 Files with no reviewable changes (4)
- server/internal/database/operations/golden_test/TestUpdateDatabase/remove_service_from_existing_database.json
- server/internal/database/operations/golden_test/TestUpdateDatabase/add_service_to_existing_database.json
- server/internal/database/operations/golden_test/TestUpdateDatabase/single_node_with_service_from_empty.json
- server/internal/database/operations/helpers_test.go
rshoemaker
force-pushed
the
feat/PLAT-539/service_connect_as
branch
from
1f90726 to
81e9b63
Compare
tsivaprasad
reviewed
Apr 14, 2026
Services now specify which database_users entry to connect as via the new required connect_as field on ServiceSpec, replacing the auto-created svc_*_ro and svc_*_rw service accounts for MCP. - Add connect_as to Goa DSL, domain model, and API convert layer - Validate connect_as references an existing database_users entry - Validate allow_writes requires db_owner on the connect_as user - Wire connect_as credentials through ServiceInstanceSpec into MCPConfigResource, bypassing ServiceUserRole for MCP - Disconnect ServiceUserRole creation for MCP services (PostgREST and RAG still use it until they adopt connect_as) - Add state migration (v2.0.0) to remove swarm.service_user_role resources and dependency references from existing deployments - Update E2E test fixtures with connect_as
…nore so credential changes trigger an Update
rshoemaker
force-pushed
the
feat/PLAT-539/service_connect_as
branch
from
c6931ba to
d4e8033
Compare
tsivaprasad
reviewed
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
database_usersentry to connect as via the new requiredconnect_asfield onServiceSpec, replacing the auto-createdsvc_*_ro/svc_*_rwservice accounts for MCPconnect_asreferences an existingdatabase_usersentry, and thatallow_writes: truerequiresdb_owner: trueServiceUserRolestateServiceUserRolecreation is disconnected for MCP (PostgREST/RAG still use it until they adoptconnect_as)swarm.service_user_roleresources and dependency references from existing deploymentsconnect_asTest plan