Skip to content

feat: remove ServiceUserRole#358

Merged
rshoemaker merged 1 commit intomainfrom
feat/PLAT-558/remove-service-user-role
Apr 22, 2026
Merged

feat: remove ServiceUserRole#358
rshoemaker merged 1 commit intomainfrom
feat/PLAT-558/remove-service-user-role

Conversation

@rshoemaker
Copy link
Copy Markdown
Contributor

Summary

  • Deletes the ServiceUserRole resource type and all associated dead code now that all service types use the connect_as credential model introduced in PLAT-539/552/553
  • Removes ServiceUser type and GenerateServiceUsername from the database package
  • Removes the Credentials field from all structs that carried it (ServiceInstance, ServiceInstanceSpec,
    ServiceInstanceSpecResource, ServiceContainerSpecOptions)
  • Cleans up a dead code path in generateMCPInstanceResources where a serviceType == "rag" guard would create
    ServiceUserRole resources that could never be reached (RAG dispatches to generateRAGInstanceResources before that block)

Test plan

PLAT-558

The connect_as credential model (PLAT-539/552/553) made
ServiceUserRole obsolete. This commit deletes it entirely.

- Delete `ServiceUserRole` resource type and all tests
- Remove `ServiceUser` type and `GenerateServiceUsername`
from the database package
- Remove `Credentials` field from `ServiceInstance`,
`ServiceInstanceSpec`, `ServiceInstanceSpecResource`,
and `ServiceContainerSpecOptions`
- Remove `populateCredentials()` from `ServiceInstanceSpecResource`
- Remove dead `serviceType == "rag"` ServiceUserRole
creation block from `generateMCPInstanceResources`
(RAG dispatches to its own function; this path was
never reached)
- Move `sanitizeIdentifier` to
`postgrest_authenticator_resource.go`, its only
remaining consumer
- Remove ServiceUserRole from the resource registry
@codacy-production
Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics -4 complexity · -4 duplication

Metric Results
Complexity -4
Duplication -4

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4496236e-f553-4c76-a48e-c1eacbdc46ee

📥 Commits

Reviewing files that changed from the base of the PR and between a682f3e and 458a9d2.

📒 Files selected for processing (12)
  • server/internal/database/service_instance.go
  • server/internal/database/service_instance_test.go
  • server/internal/orchestrator/swarm/orchestrator.go
  • server/internal/orchestrator/swarm/postgrest_authenticator_resource.go
  • server/internal/orchestrator/swarm/rag_instance_resources_test.go
  • server/internal/orchestrator/swarm/resources.go
  • server/internal/orchestrator/swarm/service_instance_spec.go
  • server/internal/orchestrator/swarm/service_spec.go
  • server/internal/orchestrator/swarm/service_spec_test.go
  • server/internal/orchestrator/swarm/service_user_role.go
  • server/internal/orchestrator/swarm/service_user_role_test.go
  • server/internal/workflows/plan_update.go
💤 Files with no reviewable changes (8)
  • server/internal/workflows/plan_update.go
  • server/internal/orchestrator/swarm/service_spec.go
  • server/internal/orchestrator/swarm/service_instance_spec.go
  • server/internal/orchestrator/swarm/resources.go
  • server/internal/orchestrator/swarm/rag_instance_resources_test.go
  • server/internal/orchestrator/swarm/service_user_role_test.go
  • server/internal/orchestrator/swarm/service_user_role.go
  • server/internal/database/service_instance_test.go

📝 Walkthrough

Walkthrough

This pull request removes service instance credential management by deleting the ServiceUser type, eliminating credentials fields from service instance and container spec structures, removing the ServiceUserRole resource implementation, and simplifying orchestrator resource generation logic.

Changes

Cohort / File(s) Summary
Database credential types
server/internal/database/service_instance.go, server/internal/database/service_instance_test.go
Removed ServiceUser type, GenerateServiceUsername function, and Credentials fields from ServiceInstance and ServiceInstanceSpec. Deleted all associated unit tests for username/ID generation. Removed now-unused imports for hash and string operations.
ServiceUserRole resource removal
server/internal/orchestrator/swarm/service_user_role.go, server/internal/orchestrator/swarm/service_user_role_test.go
Deleted entire ServiceUserRole resource implementation (263 lines) including role creation, deletion, and dependency management logic. Removed all corresponding unit tests covering identifier generation, dependency computation, and role construction across multi-node clusters.
Orchestrator credential elimination
server/internal/orchestrator/swarm/orchestrator.go, server/internal/orchestrator/swarm/service_instance_spec.go, server/internal/orchestrator/swarm/service_spec.go
Removed ServiceUserRole allocations and credential assignments from MCP and RAG instance resource generation. Deleted Credentials field from ServiceInstanceSpecResource and ServiceContainerSpecOptions. Simplified per-node resource augmentation logic to PostgREST-only conditional handling.
Resource registry and container specs
server/internal/orchestrator/swarm/resources.go, server/internal/orchestrator/swarm/service_spec_test.go
Unregistered ServiceUserRole resource type. Removed credentials setup from test fixtures and makePostgRESTSpecOpts() helper. Updated test comment phrasing from "credentials and connection details" to "connection details".
PostgREST identifier sanitization
server/internal/orchestrator/swarm/postgrest_authenticator_resource.go
Added sanitizeIdentifier helper function that safely wraps PostgreSQL identifiers with double-quoting and escaping. Updated role SQL statements to use identifier sanitization for all identifier references. Added strings import.
Documentation updates
server/internal/orchestrator/swarm/rag_instance_resources_test.go, server/internal/workflows/plan_update.go
Removed comments referencing ServiceUserRole credential generation and canonical node selection. No executable test assertions or behavior changed.

Poem

🐰 Credentials bundled, packed away,
ServiceUser roles no longer stay,
Simplified specs, cleaner the flow,
Sanitized identifiers steal the show!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: remove ServiceUserRole' clearly and concisely summarizes the main change—deletion of the ServiceUserRole resource type and associated code—and follows Conventional Commits style as required.
Description check ✅ Passed The description provides a clear summary of changes, comprehensive bullet points, documented testing confirmation, and a completed checklist; all required sections are present and adequately detailed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/PLAT-558/remove-service-user-role

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@moizpgedge
Copy link
Copy Markdown
Contributor

LGTM!

Test 1 — PostgREST deployment: no svc_ roles, authenticator configured correctly

restish control-plane-local-1 create-database '{
  "spec": {
    "database_name": "test-postgrest",
    "database_users": [{ "username": "admin", "password": "testpass", "db_owner": true, "attributes": ["LOGIN", "SUPERUSER"] }],
    "port": 0,
    "nodes": [{ "name": "n1", "host_ids": ["host-1"] }],
    "services": [{ "service_id": "api", "service_type": "postgrest", "version": "latest", "host_ids": ["host-1"], "port": 0, "connect_as": "admin", "config": {} }]
  }
}'

SELECT COUNT(*) FROM pg_roles WHERE rolname LIKE 'svc_%';           -- 0 ✓
SELECT rolinherit FROM pg_roles WHERE rolname = 'admin';             -- f ✓
SELECT EXISTS (
  SELECT 1 FROM pg_auth_members m
  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'
);                                                                    -- t ✓
SELECT HAS_DATABASE_PRIVILEGE('admin', 'test-postgrest', 'CONNECT'); -- t ✓

Test 2 — Validation: connect_as enforced for PostgREST

Missing connect_as:

services block with no connect_as field

# → HTTP 400: "connect_as" is missing from body ✓
Non-existent connect_as:


# connect_as: "ghost_user" (not in database_users)
# → HTTP 400: connect_as "ghost_user" does not match any database_users entry ✓

Test 3 — Service removal: connect_as user not dropped

restish control-plane-local-1 update-database 2772edc2-9fba-46fc-b014-318915359a20 '{
  "spec": {
    "database_name": "test-postgrest",
    "database_users": [
      { "username": "admin", "password": "testpass", "db_owner": true, "attributes": ["LOGIN", "SUPERUSER"] }
    ],
    "port": 0,
    "nodes": [{ "name": "n1", "host_ids": ["host-1"] }],
    "services": []
  }
}'


SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'admin'); -- t ✓

docker service ls | grep api  # no output ✓

Test 4 — Preflight blocks deployment when anon role does not exist

restish control-plane-local-1 create-database '{
  "spec": {
    "database_name": "test-preflight2",
    "database_users": [
      { "username": "admin", "password": "testpass", "db_owner": true, "attributes": ["LOGIN", "SUPERUSER"] }
    ],
    "port": 0,
    "nodes": [{ "name": "n1", "host_ids": ["host-1"] }],
    "services": [{
      "service_id": "api",
      "service_type": "postgrest",
      "version": "latest",
      "host_ids": ["host-1"],
      "port": 0,
      "connect_as": "admin",
      "config": {
        "db_anon_role": "role_that_does_not_exist"
      }
    }]
  }
}'

docker service ls | grep api  # no output — container never started ✓

@rshoemaker rshoemaker merged commit 18e7678 into main Apr 22, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants