Repository navigation
feat(relay-infra): the relay's own Cloud SQL instance and move-job identity (step 4, staging only) - #29564
feat(relay-infra): the relay's own Cloud SQL instance and move-job identity (step 4, staging only)#29564Jinwoo-H wants to merge 3 commits into
Conversation
…, move logins and job identity (step 4) Staging only (relay_dedicated_database_enabled); production needs its own go.
… on Enterprise after an Enterprise Plus stockout
…loud SQL refused it under a resource condition
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
66394219-5b01-4a80-8e7c-59b603865dd9
📒 Files selected for processing: 3
cloud/infra/terraform/environments/staging.tfvarscloud/infra/terraform/relay-dedicated-database.tfcloud/infra/terraform/variables.tf
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| count = local.relay_dedicated_database_count | ||
|
|
||
| project = var.project_id | ||
| name = "orca_relay_move" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Give the shared-instance move login a scoped database role.
Cloud SQL automatically grants this built-in login cloudsqlsuperuser. That role permits broad operations, including creating roles and databases, on the shared auth instance. The move job needs access to relay data, not those instance-wide privileges. Create a scoped PostgreSQL role and assign it to orca_relay_move; grant the move operations explicitly. (docs.cloud.google.com)
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the move tool and repository-owned SQL for grants and ownership changes.
fd -i 'database-move-cli\.ts' -x rg -n -C 4 -i \
'relay-move|GRANT|SET ROLE|ALTER (TABLE|SCHEMA|DEFAULT PRIVILEGES)|CREATE (TABLE|SCHEMA)'
rg -n -C 3 -i 'orca_relay_move|GRANT .*orca_relay|ALTER DEFAULT PRIVILEGES' cloudRepository: stablyai/orca
Length of output: 2069
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- move tool candidates ---'
rg --files -g '*database*move*' -g '*move*database*' -g '*.ts' -g '*.sql' | sort
printf '%s\n' '--- move workflow references ---'
rg -n -i -F -- 'database-move-cli' . || test "$?" -eq 1
rg -n -i -e 'relay[-_ ]move' -e 'orca_relay_move' -e 'SET ROLE' -e 'GRANT ' -e 'ALTER DEFAULT PRIVILEGES' -e 'CREATE TABLE' -e 'CREATE SCHEMA' --glob '!node_modules/**' --glob '!vendor/**' --glob '!dist/**' --glob '!build/**' . || test "$?" -eq 1Repository: stablyai/orca
Length of output: 45660
🏁 Script executed:
set -euo pipefail
log=/tmp/coderabbit-shell-logs/shell-output-ujdmPa
printf '%s\n' '--- exact move-tool matches in prior output ---'
rg -n -i -e 'database-move-cli' -e 'orca_relay_move' -e 'relay-move' -e 'ALTER DEFAULT PRIVILEGES' -e 'SET ROLE' "$log" | head -n 200 || test "$?" -eq 1
printf '%s\n' '--- candidate path lines from prior output ---'
rg -n -i -e 'database.*move' -e 'move.*database' "$log" | head -n 200 || test "$?" -eq 1Repository: stablyai/orca
Length of output: 40991
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- orca_relay references ---'
rg -n -F -- 'orca_relay' cloud/infra cloud/apps/relay cloud/packages 2>/dev/null || test "$?" -eq 1
printf '%s\n' '--- relay schema ownership and privilege statements ---'
rg -n -i -e 'CREATE SCHEMA' -e 'ALTER .*OWNER' -e 'GRANT ' -e 'REVOKE ' -e 'DEFAULT PRIVILEGES' -e 'role' cloud/apps/relay cloud/packages/postgres-schema cloud/infra 2>/dev/null || test "$?" -eq 1
printf '%s\n' '--- database move candidate files ---'
rg --files cloud | rg -i 'database.*move|move.*database|relay.*move|move.*relay' || trueRepository: stablyai/orca
Length of output: 41039
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- shared database Terraform ---'
nl -ba cloud/infra/terraform/relay-database.tf | sed -n '1,180p'
printf '%s\n' '--- dedicated database Terraform ---'
nl -ba cloud/infra/terraform/relay-dedicated-database.tf | sed -n '70,145p'
printf '%s\n' '--- schema package entrypoints ---'
nl -ba cloud/packages/postgres-schema/src/apply-postgres-schema.ts | sed -n '1,260p'
printf '%s\n' '--- schema application call sites ---'
rg -n -F -e 'applyPostgresSchema' -e 'apply-postgres-schema' -e 'POSTGRES_URL' -e 'DATABASE_URL' cloud/apps/relay cloud/packages/postgres-schema cloud/infra/terraform | head -n 200 || test "$?" -eq 1Repository: stablyai/orca
Length of output: 32651
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- relay database startup and schema call ---'
nl -ba cloud/apps/relay/src/database.ts | sed -n '1440,1525p'
printf '%s\n' '--- relay runtime database URL bindings ---'
nl -ba cloud/infra/terraform/relay.tf | sed -n '190,235p'
nl -ba cloud/infra/terraform/relay.tf | sed -n '520,565p'
printf '%s\n' '--- relay schema DDL sources ---'
rg -n -i -e 'CREATE TABLE' -e 'CREATE SEQUENCE' -e 'CREATE SCHEMA' -e 'ALTER TABLE' -e 'CREATE INDEX' cloud/apps/relay/src/database.ts cloud/apps/relay/src --glob '*.ts' | head -n 160 || test "$?" -eq 1Repository: stablyai/orca
Length of output: 24575
Grant orca_relay_move access to relay objects before using it.
The relay application connects as orca_relay, and schema startup applies PostgreSQL DDL through that connection. Terraform creates a separate orca_relay_move user but defines no table, sequence, schema, or default-privilege grants. cloudsqlsuperuser is not PostgreSQL SUPERUSER, so it does not bypass object ownership. The move can therefore fail when it reads or updates existing relay tables. Objects created by the move login on the dedicated instance may also be inaccessible to orca_relay after cutover.
Add the required grants or transfer ownership. Ensure the move workflow preserves application access to objects created during the move.
|
Closing because #31792 ( This PR creates the dedicated relay database, move-job identity/IAM and secrets, with staging-only enablement. Its Terraform root, variables and staging tfvars now belong to that private repository. The public PR cannot reconcile the active staging state or grant the move job its required database roles. I also read the permission/ownership findings in review; closure does not resolve those findings or assert that staging state is safe to apply. I reviewed the complete patch and discussion. The private repository is inaccessible to this audit, so any still-needed work must be carried forward there; this is a repository-ownership closure, not a claim that the operational issue is fixed. |
ELI5
The relay is getting its own database server, so heavy relay traffic can no longer slow down sign-in. This PR adds the Terraform for that server and for the identity of the one-off job that moves the data. It only turns them on in staging. Production stays off until Jinwoo gives the go.
What Changed
relay-dedicated-database.tf(new) adds the relay's own Cloud SQL instance:db-perf-optimized-N-2.max_connectionsis set explicitly to 500.orca_relay_movelogin on each instance. This is the only change on the shared auth instance: one new user, no restart, no flag change.cloudsql.client;database-target.json.staging.tfvarsthey are on, using Enterprisedb-custom-2-7680under the name...-relay-db3(reasons under Testing).Why
cloudsql.logical_decoding, and that flag restarts the shared instance, which is a sign-in outage. The move tool tracks changes with triggers instead, and this PR only provides what that tool needs.Testing
All staging work used targeted applies and created resources only. I ran a full rehearsal on staging on 10-16, then put staging back to sleep the way I found it. Findings that shaped this PR:
relay_dedicated_database_edition). The move mechanism does not depend on the edition.db-custom-4-15360.-db3.cloudsql.users.updatereturned 403 under an IAMresource.namecondition. The move tool's rollback refused cleanly, before changing anything. The grant is now project-wide, on a role with three permissions.terraform validatepasses, and the targeted staging plan only adds resources.I manually tested these changes locally
Automated tests added/updated, or explained why not below: infrastructure only, proven by the staging apply and rehearsal above.
Review
Stack: #29405 (relay switching), #29462 and #29463 (move tool). This Terraform PR is independent of them. Do not merge until Jinwoo's go. Until it merges, a full staging plan from main would show these staging resources as to-be-destroyed, and
prevent_destroywould stop that plan. Targeted applies are not affected.