fix(deploy): grant logging.viewer and document TokenCreator for single-node tier - #1324
Open
ptone wants to merge 3 commits into
Open
fix(deploy): grant logging.viewer and document TokenCreator for single-node tier#1324ptone wants to merge 3 commits into
ptone wants to merge 3 commits into
Conversation
added 3 commits
August 28, 2026 19:12
…e-node tier (#111) The single-node deploy.sh granted NO IAM roles to the instance service account. The hub auto-detects GCP features via the metadata server and enables them, but without the corresponding permissions the features fail at runtime — not at startup. This commit: - Resolves the instance SA email (from --service-account or the Compute Engine default) and grants roles/logging.viewer at the project level. The hub's log query UI (pkg/hub/logquery.go) is auto-enabled when ResolveProjectID() succeeds, which it always does on Cloud Run via the metadata server. Without this role, the UI returns errors. - Documents but does NOT grant roles/iam.serviceAccountTokenCreator. The hub needs this role to impersonate target SAs for agent GCP identity (pkg/hub/gcp_token_iam.go), but the IAM API checks the permission on the TARGET SA resource, not at the project level. A project-level grant would let the hub SA impersonate any SA in the project, which is too broad for a one-command deploy. Instead, the script prints a copy-pasteable post-deploy instruction with the hub SA email already filled in and only TARGET_SA as a placeholder. - Adds comment blocks for two declined roles (serviceAccountAdmin for SA minting, monitoring.viewer for metrics dashboard) explaining why each is not granted and providing the manual grant command. The verification gate on unverified SAs is well-built: registration returns verificationFailed with both emails and the exact missing role; assignment returns 400 at every surface (create, patch, project-default). An unverified SA cannot reach agent startup. This makes the "document, don't grant" approach viable — the operator always sees why and what to do about it. Does NOT fix ptone's live crash on sn-harness-lab — he worked around that by granting TokenCreator manually. This fixes the gap that made the manual grant necessary.
Two problems found by architect review:
1. The logging.viewer grant was fatal to the deploy under set -euo
pipefail. An operator who lacks resourcemanager.projects.setIamPolicy
(common in shared/corporate GCP projects) could no longer deploy at
all — converting "a UI panel is unprovisioned" into "the product does
not install." The grant is now non-fatal but loud: on failure it emits
a WARNING naming the role, what it enables, states the deploy is
otherwise complete, and gives the exact manual command. This is the
same treatment TokenCreator already gets in this PR.
2. No test reached Step 5b — the grant had never executed in any test.
Added two di_main-level tests that run the FULL deploy flow through
Step 5b:
- TestScriptStep5bGrantSucceeds: verifies the gcloud call is made
with the correct args, the success message is printed, and the
deploy completes.
- TestScriptStep5bGrantFailsNonFatally: verifies the deploy completes
(exit 0) even when the grant fails, that a WARNING is emitted
naming the role, and that the manual command is printed.
Both tests override di_wait_for_iap and di_assert_perimeter (already
independently tested) to avoid curl to the real instance URL, and use
a full gcloud stub that handles all steps through Step 5b.
…mitted When no --service-account is provided, the instance runs as the Compute Engine default SA, which commonly holds project Editor. The script now tells the operator this: names the SA as the Compute Engine default, states it commonly holds broad project permissions, notes that the hub and every agent use it, and points at --service-account for a scoped alternative. This is disclosure, not enforcement — the operator is told what they have. The question of whether this tier should mint a dedicated, minimally-scoped SA is a design decision tracked separately (#116).
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
1 participant
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
roles/logging.viewerto the instance SA — non-fatal: on failure, emits a WARNING naming the role, states the deploy is otherwise complete, and prints the exact manual grant commandroles/iam.serviceAccountTokenCreatoras a post-deploy instruction with a copy-pasteable gcloud command (hub SA email pre-filled, only TARGET_SA as placeholder)iam.serviceAccountAdmin,monitoring.viewer) with stated reasons and manual grant commands--service-accountfor operators who want a scoped identitydi_resolve_instance_sapure function with tests, plus two di_main-level tests that run the full deploy flow through Step 5bWhy not copy the multi-node script
The IAM matrix (#111) identified four roles the multi-node
scripts/cloudrun/deploy.shgrants that would be over-granted on the single-node tier:roles/run.adminroles/cloudsql.clientroles/iap.tunnelResourceAccessorroles/storage.objectAdminWhy TokenCreator is documented, not granted
The IAM Credentials API checks
iam.serviceAccounts.getAccessTokenpermission on the target SA (projects/-/serviceAccounts/{email}atgcp_token_iam.go:66), not at the project level. A project-level serviceAccountTokenCreator grant would let the hub SA impersonate every SA in the project. The least-privilege shape is a per-target-SA binding, but the script runs before any target SA exists.The verification gate makes this viable: the API returns
verificationFailed: truewith both emails at SA registration time, and every assignment surface (create, patch, project-default) returns 400 with "not verified; verify it before assigning to agents". An unverified SA cannot reach agent startup.Note: The API returns everything the operator needs at registration time. Whether the UI renders verificationFailed clearly is unverified and should be checked separately.
Why the logging.viewer grant is non-fatal
An operator who lacks
resourcemanager.projects.setIamPolicyon their project (common in shared/corporate GCP projects) must not be prevented from deploying. The logging.viewer role backs a log-viewer UI panel that is not on the S1 path.Testability note
The Step 5b tests override
di_wait_for_iapanddi_assert_perimeterbecause those functions use curl against the real instance URL and cannot be stubbed through the_DI_API_BASE/_DI_TOKENINFO_URLseams. This is a testability defect in the script, not in the test -- any future test that needs to exercise a step after Step 4 will hit the same wall. The two functions are independently tested via stub HTTP servers; what is missing is a seam that lets di_main-level tests reach the late steps without overriding the functions.Test plan