fix(ingestion): make the role grants runnable and drop the CREATE ROLE - #844
Merged
Conversation
Two things were wrong with this script. It told the operator to create the role. Registering the service account as a Cloud SQL IAM user already creates the Postgres role, which Terraform does via google_sql_user -- `gcloud sql users list` shows ocotillo-ingestion@waterdatainitiative-271000.iam as CLOUD_IAM_SERVICE_ACCOUNT on the instance. A CREATE ROLE would fail, and wanting one is a sign the Terraform half has not been applied. It also hardcoded the role name via \set while referencing an unset db_name, so it could not run as written. Both are now required parameters and the script stops with a readable message if either is missing. That matters more than convenience here: the instance hosts both `ocotillo` and `ocotillo-staging`, and granting against the wrong one would succeed silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
.terraform.tfstate.lock.info was committed with the us-west4 change. It is written while a plan or apply holds the lock and left behind when a run is interrupted -- which is how it got picked up. It is machine-specific, and a stale one in a fresh checkout is actively misleading, since Terraform reports it as another user holding the lock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.27% total — gate is 75%. |
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
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
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.
ingestion_role.sqlcould not be run as written, and it instructed the operatorto do something that would have failed. Found while trying to execute it against
dataservices.The CREATE ROLE was wrong
The script told you to uncomment a
CREATE ROLEfor the IAM principal. Thatrole already exists — registering a service account as a Cloud SQL IAM user
creates the Postgres role automatically, which Terraform does via
google_sql_user.ingestion:So the script now only grants, and says so. Reaching for a
CREATE ROLEis asignal the Terraform half has not been applied, not something to work around.
It could not run
role_namewas hardcoded with\setwhiledb_namewas referenced and neverset, so every
GRANTwould have failed on an undefined variable.Both are now required parameters, with a readable abort if either is missing:
psql "host=... dbname=ocotillo user=postgres" \ -v db_name=ocotillo \ -v role_name=ocotillo-ingestion@waterdatainitiative-271000.iam \ -f automated_ingestion/sql/ingestion_role.sqlRequiring them is the point, not a formality. The instance hosts both
ocotilloandocotillo-staging. Granting against the wrong one succeedssilently and leaves you debugging the database you did not touch.
The role name contains
@and dots, so every reference uses:"role_name",which quotes it as an identifier — unquoted is a syntax error.
Also
Stops tracking
automated_ingestion/iac/.terraform.tfstate.lock.info, which wascommitted with the us-west4 change. It is written while a run holds the state
lock and left behind when one is interrupted. A stale copy in a fresh checkout
is worse than useless: Terraform reports it as another user holding the lock.
Not verified against a live database
I could not run this — no
psqlhere, the superuser is a built-in account whosepassword I will not handle, and the account I am authenticated as has no
database login on the instance. The grants themselves are unchanged from the
reviewed version; what changed is that the script now runs and targets what you
tell it to.
🤖 Generated with Claude Code