fix(gcp): narrow runtime SA recommender access to a single-permission custom role

[IAmJSD]

Root cause

terraform/modules/compute/gcp/cloud-run/main.tf granted the Cloud Run runtime SA the predefined roles/recommender.viewer. That role carries get/list on every recommender and insight type in the project. The runtime needs one of them. terraform/modules/compute/gcp/gke/main.tf had no Recommender grant, even though the GKE deployment runs the same collector (google_cloud_scheduler_job.recommendations → /api/scheduled/collect_recommendations). A GCP account in application_default auth mode (internal/credentials/resolver.go ResolveGCPTokenSourceWithOpts) collects with the runtime SA, which on GKE is the workload SA through Workload Identity.

Fix

• Cloud Run: removed google_project_iam_member.recommender_viewer. Added google_project_iam_custom_role.cudly_recommendation_reader (role_id = "cudlyRecommendationReader", permissions = exactly recommender.usageCommitmentRecommendations.list, GA) and google_project_iam_member.recommendation_reader, which binds it to google_service_account.cloud_run at project scope.
• GKE: added the same custom role and binding for google_service_account.workload. var.compute_platform selects either cloud-run or gke, never both, so the two modules can't both create the role in one environment. The role ID and permission match the CLI setup wizard's cudlyRecommendationReader (cloud-commitments-cli#2124 / #2158). This follows the existing practice of keeping identical cudlyCommitmentWriter definitions so the modules converge.
• scripts/gcp-import-dev-state.sh: removed the import of the deleted recommender_viewer address. Importing it now would fail.
• New static test terraform/modules/compute/gcp/runtime_recommender_iam_test.go. It checks that both modules have no predefined roles/recommender.* on any code line, that each defines cudlyRecommendationReader once with exactly ["recommender.usageCommitmentRecommendations.list"], and that each has exactly one google_project_iam_member binding it to that module's runtime SA with project = var.project_id.

Recommender API calls made by the runtime → permission

Source: pinned github.com/LeanerCloud/cloud-commitments-go/providers/gcp@v0.0.0-20261015143912-740dfbd72456. Every Recommender call is ListRecommendations. Nothing in the module calls GetRecommendation, ListInsights/GetInsight or Mark*.

Call (code ref)	Parent	Permission	Covered after this PR
services/computeengine/client.go:437-444 ListRecommendations	projects/P/locations/R/recommenders/google.compute.commitment.UsageCommitmentRecommender	recommender.usageCommitmentRecommendations.list	yes (the only commitment recommender)
services/cloudsql/client.go:194-202 ListRecommendations	.../recommenders/google.cloudsql.instance.PerformanceRecommender	recommender.cloudsqlInstancePerformanceRecommendations.list	no, on purpose (see below)
services/memorystore/client.go:189-197 ListRecommendations	.../recommenders/google.memorystore.redis.PerformanceRecommender	none. Google's recommender list does not include this ID, so the call fails whatever role is held	n/a
services/cloudstorage	none. Deprecated; it only returns not-supported	none	n/a

recommendations.go collectRegion handles per-(region, service) errors by logging a WARN without failing the region. The all-attempted-failed guard only trips when every call fails, and the Compute call succeeds.

Behavior change to review: the runtime SA can no longer list Cloud SQL Performance recommendations. That recommender gives performance advice (open tables, transaction ID use), not commitment recommendations. CLI#2158 made the same call and leaves it unprovisioned. Fixing the Cloud SQL and Memorystore recommender IDs is tracked as a cloud-commitments-go follow-up. Until then, expect one extra per-region WARN for Cloud SQL where roles/recommender.viewer used to let that call succeed. If you'd rather keep Cloud SQL perf recs flowing for now, the alternative is adding recommender.cloudsqlInstancePerformanceRecommendations.list to this role. I haven't done that because it would cement the wrong recommender ID.

Bootstrap requirement

Creating google_project_iam_custom_role needs iam.roles.{get,create,update,delete} on the deployer SA. terraform/environments/gcp/ci-cd-permissions/service_account.tf already grants roles/iam.roleAdmin for cudlyCommitmentWriter, so the bootstrap doesn't change. If an environment's deploy SA was bootstrapped without it, the apply 403s on iam.roles.get/create. That's the intended signal to re-apply ci-cd-permissions, and there's deliberately no fallback to roles/recommender.viewer. If someone already ran the CLI wizard against the host project, cudlyRecommendationReader exists and create returns 409. Import it with terraform import 'module.compute_cloud_run[0].google_project_iam_custom_role.cudly_recommendation_reader' projects/P/roles/cudlyRecommendationReader. The definitions are identical.

Verification (static only, nothing applied)

• New test fails on origin/main: Cloud Run reports the predefined role and the missing custom role, and GKE reports the missing custom role. It passes after the fix.
• Mutation probes, each caught by the test: an extra ...get permission, the GKE binding reverted to roles/recommender.viewer, a binding to the wrong SA (scheduler[0]), a binding with a non-var.project_id project, and a renamed role ID.
• go test ./terraform/... ./iac/... ./internal/iacfiles/... passes (with AWS_CONFIG_FILE=/dev/null). golangci-lint v2.10.1 on the package reports 0 issues.
• terraform fmt -check -recursive terraform/ is clean. terraform init -backend=false + validate pass on modules/compute/gcp/cloud-run, modules/compute/gcp/gke and environments/gcp, using a temp data dir that was deleted afterwards. Lockfiles are unchanged.
• scripts/check-gcp-secret-scope.sh passes. scripts/test-gcp-secret-scope.sh gives 40/40.
• git-secrets --scan of the changed files in an isolated throwaway repo is clean. scripts/test-git-secrets-allowlist.sh gives 42/42.
• git merge-tree against the sibling fix(gcp): exclude uninitialized replicas from GKE and Cloud Run readiness #1024 branch (fix/issue-648, probe edits in the same modules) is clean.
• Not verified: no live GCP project. I haven't confirmed live that a project custom role accepts recommender.usageCommitmentRecommendations.list (Google's IAM docs list it under project-level CUD viewer roles), or that an SA with only this role plus roles/compute.viewer gets recommendations back. Operators: make sure the deploy SA holds roles/iam.roleAdmin (re-apply ci-cd-permissions if not), then re-apply the GCP environment.

Out of scope

• The GKE workload SA still has no roles/compute.viewer or cudlyCommitmentWriter, unlike Cloud Run. So application_default collection on GKE still stops at the compute.regions.list 403 ("skipping recommendations") before it reaches the Recommender call. Achieving parity there is a separate change.
• Customer-side target templates still grant roles/recommender.viewer: iac/federation/gcp-target, iac/federation/gcp-sa-impersonation, internal/iacfiles/templates/gcp-sa-impersonation-cli.sh.tmpl.
• Cloud SQL and Memorystore recommender IDs in cloud-commitments-go.

Closes #788

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

Or wait 38 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: d4a25cc9-871d-493f-a680-fc2a217c11a0

📥 Commits

Reviewing files that changed from the base of the PR and between 6e80319 and 9ac4f37.

📒 Files selected for processing: 4

• scripts/gcp-import-dev-state.sh
• terraform/modules/compute/gcp/cloud-run/main.tf
• terraform/modules/compute/gcp/gke/main.tf
• terraform/modules/compute/gcp/runtime_recommender_iam_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review verdict: CLEAN at 9ac4f37, with one owner decision

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief, acting as a security reviewer. Read-only, with no credentials and no plan or apply. The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Recommender calls vs the new single permission (pinned cloud-commitments-go providers/gcp@740dfbd72456):

Call	Permission needed	Covered
services/computeengine/client.go:437-444 ListRecommendations (UsageCommitmentRecommender)	recommender.usageCommitmentRecommendations.list	Yes. It is the only permission in the role (custom role block in cloud-run/main.tf and gke/main.tf).
services/cloudsql/client.go:194-202 ListRecommendations (cloudsql PerformanceRecommender)	recommender.cloudsqlInstancePerformanceRecommendations.list	No. It will 403 after this merges.
services/memorystore/client.go:189-197 (memorystore PerformanceRecommender)	none (not a valid Google recommender ID)	n/a

Binding:

• It is project-level (project = var.project_id), which matches the projects/P/locations/R/recommenders/... query path. There is no billing-account-level query.
• The old recommender_viewer binding is removed and the new one added in the same apply, in dependency order.

Import script: it only drops the obsolete recommender_viewer import, with no secrets. A pre-existing custom role (from the CLI wizard) gives 409 and needs the manual import the PR body documents.

Deploy (merge held for an explicit owner OK):

• deploy-gcp.yml applies on every push to main.
• The deployer has roles/iam.roleAdmin (ci-cd-permissions/service_account.tf:36), so it can create the custom role. An older bootstrapped deployer SA may need ci-cd-permissions re-applied first.

⚠️ Decision for @cristim: the Cloud SQL Performance recommender call will start returning 403. Per the PR body it is logged as a per-region WARN, not a failure. If that recommender feeds anything you want, it needs its own permission added to the custom role. If it is unused for commitments, the narrowing is right and that call could be removed.

Not verified live: that usageCommitmentRecommendations.list is accepted in a custom role (it is the documented permission).

What was attacked and how:

• terraform fmt -check -recursive is clean.
• init and validate succeed in cloud-run and gke.
• go test ./terraform/modules/compute/gcp/... passes.
• shellcheck has only a pre-existing info note.
• Mutation probe: changing the permission to .get fails TestRuntimeSARecommenderGrantIsNarrow.

Gate:

• Labels are mirrored from gcp: narrow Cloud Run runtime SA roles/recommender.viewer; GKE module has no recommender grant #788.
• Needs the author to mark the draft ready, then green CI and CodeRabbit, then the owner's OK and the Cloud SQL decision.

[cristim]

Merge driver status (claude-cc-pr-merge, 2026-10-17T15:25:27Z): code gate met, merge BLOCKED on an owner decision from @cristim: the Cloud SQL Performance recommender call starts returning 403 under the narrowed role (add the permission or drop the call), plus the same dev-deploy OK.

[cristim]

Refreshing CI (claude-cc-pr-merge-2): main changed scripts/ after this PR's CI ran, so CI must re-run on the current merge ref before merging. Reopening now.

[cristim]

Gate record for 9ac4f37 (claude-cc-pr-merge-2):

• Independent adversarial review: CLEAN at this exact SHA (fix(gcp): narrow runtime SA recommender access to a single-permission custom role #1035 (comment); non-author, cold brief, coordinated by claude-cc-reviewer). The local verification evidence and its limits (fixture or mock level, no real-account apply) are in that verdict.
• Owner hold lifted 2026-10-17 (~/.claude/decisions/2026-10-17-platform-holds-lifted.md). Owner accepts that the Cloud SQL perf recommender call will 403 under the narrowed role; merging applies to dev.
• CI: required checks green on this SHA, re-checked by merge-gated.sh at merge time, including the guard against main changing the same packages since CI ran.
• Labels mirror the linked issue. No unresolved review threads. CodeRabbit: skipped (quota).
• Merging with squash and --match-head-commit; no --admin.