Repository navigation
fix(gcp/gke): grant workload SA access to the admin password secret - #504
Merged
Merged
Conversation
The pod gets ADMIN_PASSWORD_SECRET but the module had no binding on it, so startup would hit PermissionDenied. Add secretAccessor and secretVersionAdder (the app reads the secret and adds a version on admin password change), mirroring the Cloud Run module and omitting project. Closes #480.
cristim
added
priority/p2
Contributor
|
Warning Review limit reached
This review includes 3 billable files and costs up to $0.75.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 59 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 50 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
Comment |
IAmJSD
added a commit
that referenced
this pull request
Oct 26, 2026
…rry (#1213) * sec(ci): refuse authoritative GCP IAM policies in the secret scope guard check-gcp-secret-scope.sh finds Secret Manager roles by their text, but an authoritative google_{project,folder,organization}_iam_policy can take its policy_data from file(), a variable with no in-tree default or a module output, so the role it grants never appears in a scanned file and the guard reported such configurations clean. Fail closed instead: any non-comment mention of those types, or of data "google_iam_policy", now exits 2, as the guard already does for the JSON encoding. CI's terraform fmt check now covers iac/ as well as terraform/, so every root the guard scans is held to the layout its block tracking was written for. The self-test reads SCAN_ROOTS and fails if a root has no fmt step. Closes #150 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * sec(ci): allowlist the roles a per-secret Secret Manager grant may carry check-gcp-secret-scope.sh exempted a per-secret binding's `role =` line for any roles/secretmanager.* name, so widening a per-secret grant to roles/secretmanager.admin (the #504 review's mutation) still exited 0. Inside a per-secret IAM resource every `role =` line must now name roles/secretmanager.secretAccessor or roles/secretmanager.secretVersionAdder as a literal, matched case-sensitively. Any other role there, including one from another service such as roles/owner or a reference the guard cannot resolve, is a finding unless waived with a reason. Those two are the only roles the repo grants per secret and the only ones the issue names. Inline policy_data role lines on a per-secret _iam_policy get the same check. One built from data "google_iam_policy" already exits 2 after #150; one fed from file() or a variable remains out of reach and is listed under LIMITATIONS. Closes #505 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
Grant the GKE workload service account access to the admin password secret it is already given as
ADMIN_PASSWORD_SECRET.Why
The module passed the secret name to the pod but had no IAM binding on it, so a GKE deployment would hit PermissionDenied resolving the admin password at startup (same failure as the Cloud Run dev outage, #476/#479). The app also writes to it:
buildAdminPasswordSyncCallbackcallsPutSecret, which isAddSecretVersionon GCP (internal/secrets/gcp_resolver.go), so both roles are needed.Change
admin_password_reader(roles/secretmanager.secretAccessor) andadmin_password_writer(roles/secretmanager.secretVersionAdder) forgoogle_service_account.workload, noprojectargument (fullprojects/<number>/secrets/<id>name; setting it force-replaces the binding each apply).enable_admin_password_writervariable (default false), same gate and semantics as Cloud Run (name is known after apply).environments/gcp/compute.tfsets it to true for the gke module call.Verification
terraform fmt -check,terraform init -backend=false && terraform validatepass for the gke module andterraform/environments/gcp.scripts/check-gcp-secret-scope.sh(CI "GCP Secret Manager grant scope") andscripts/test-gcp-secret-scope.shpass locally.Closes #480