Skip to content

chore(api): transfer Alembic object ownership#1671

Merged
Jacky-Pham merged 1 commit into
mainfrom
Jacky/alembic-service-account-ownership-pr
May 28, 2026
Merged

chore(api): transfer Alembic object ownership#1671
Jacky-Pham merged 1 commit into
mainfrom
Jacky/alembic-service-account-ownership-pr

Conversation

@Jacky-Pham
Copy link
Copy Markdown
Collaborator

@Jacky-Pham Jacky-Pham commented May 27, 2026

Summary

This is an SRE initiative that came from the security team around Cloud SQL IAM/service account ownership.

I lined this up with Andriy's Alembic IAM migration flow from the other apps. The important bit is the same pattern used in PPR/Search: read DATABASE_OWNER_ROLE, escape quotes, run SET ROLE before Alembic runs, then commit so migrations create objects as that DB role.

So the short explanation is: I basically copied the migration ownership approach Andriy used there and adapted it to STRR's standalone env.py.

What changed

  • added DATABASE_OWNER_ROLE support in STRR Alembic env.py
  • runs SET ROLE before online migrations when DATABASE_OWNER_ROLE is set
  • escapes double quotes in the configured role name before building the SET ROLE statement
  • documents that the migration user must be able to SET ROLE and the owner role needs schema create privileges
  • added an integration test that runs Alembic against disposable Postgres with an IAM-style role name and verifies object/type ownership

Testing

  • python3 -m py_compile strr-api/migrations/env.py strr-api/tests/integration/test_alembic_ownership.py
  • poetry run bandit -r migrations/env.py
  • poetry run pytest tests/integration/test_alembic_ownership.py --no-cov
  • targeted coverage check for migrations/env.py: 83%

Note: I also reran the full integration suite. The Alembic test passed, but two existing NOC tests failed on nocSentDate being null. I reran those exact two tests and they still failed, so I’m treating that as unrelated to this migration-role change.

@Jacky-Pham Jacky-Pham force-pushed the Jacky/alembic-service-account-ownership-pr branch from bea58d6 to b13547d Compare May 28, 2026 20:21
@Jacky-Pham Jacky-Pham force-pushed the Jacky/alembic-service-account-ownership-pr branch from b13547d to 46ca976 Compare May 28, 2026 20:23
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Collaborator

@dimak1 dimak1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Jacky-Pham Jacky-Pham merged commit 6cfe739 into main May 28, 2026
16 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