fix: auto-detect host SA from metadata + accept default Compute SA - #1482
Conversation
On the single-node Cloud Run tier the co-located broker registration never populated gcpHostServiceAccountEmail, causing passthrough identity to fail with a configuration error. Additionally, the SA email validator rejected @developer.gserviceaccount.com (default Compute Engine SA format), so even manually setting the field would not work. Changes: 1. registerGlobalProjectAndBroker now queries the GCE metadata server for the default service account email and project ID, and sets GCPHostServiceAccountEmail / GCPHostProjectID on both the create and update paths. 2. isValidServiceAccountEmail now accepts @developer.gserviceaccount.com in addition to @<project>.iam.gserviceaccount.com. Tests cover the widened validator, the passthrough gate with a developer-domain SA, and the broker PATCH endpoint with the new email format.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces auto-detection of the GCP host service account and project ID from the GCE metadata server during broker registration, and expands service account email validation to support default Compute Engine service accounts (@developer.gserviceaccount.com). Feedback suggests ensuring GCPHostProjectID is explicitly validated when using developer service accounts, as the project ID cannot be parsed from the project number in the email. Additionally, a code suggestion was provided to also support App Engine default service accounts (@appspot.gserviceaccount.com).
| case domain == "developer.gserviceaccount.com": | ||
| // Default Compute Engine SA (e.g. <project-number>-compute@developer.gserviceaccount.com). | ||
| return true |
There was a problem hiding this comment.
For @developer.gserviceaccount.com service accounts, the project ID cannot be derived from the email address because the email contains the project number (e.g., 721899303052-compute), not the project ID (alphanumeric string), and the domain does not encode the project ID.
If GCPHostProjectID is left empty, the fallback projectIDFromServiceAccountEmail (called on line 158) will fail to extract a valid project ID, leading to failed GCP IAM checks.
Consider adding a validation check in the broker update/registration path to ensure GCPHostProjectID is explicitly provided when a @developer.gserviceaccount.com service account is used, or update projectIDFromServiceAccountEmail to explicitly handle or fail for this domain.
| switch { | ||
| case strings.HasSuffix(domain, ".iam.gserviceaccount.com"): | ||
| // Custom IAM SA: project ID portion must be non-empty. | ||
| projectID := domain[:len(domain)-len(".iam.gserviceaccount.com")] | ||
| return len(projectID) > 0 | ||
| case domain == "developer.gserviceaccount.com": | ||
| // Default Compute Engine SA (e.g. <project-number>-compute@developer.gserviceaccount.com). | ||
| return true | ||
| default: | ||
| return false | ||
| } |
There was a problem hiding this comment.
While adding support for @developer.gserviceaccount.com is great for default Compute Engine service accounts, another very common default GCP service account domain is @appspot.gserviceaccount.com (used as the App Engine default service account, which has the format {project-id}@appspot.gserviceaccount.com).
Consider also accepting appspot.gserviceaccount.com in this switch to support environments using the App Engine default service account.
switch {
case strings.HasSuffix(domain, ".iam.gserviceaccount.com"):
// Custom IAM SA: project ID portion must be non-empty.
projectID := domain[:len(domain)-len(".iam.gserviceaccount.com")]
return len(projectID) > 0
case domain == "developer.gserviceaccount.com":
// Default Compute Engine SA (e.g. <project-number>-compute@developer.gserviceaccount.com).
return true
case domain == "appspot.gserviceaccount.com":
// App Engine default SA (e.g. <project-id>@appspot.gserviceaccount.com).
return true
default:
return false
}- Use context-aware metadata API (EmailWithContext, ProjectIDWithContext) to fix staticcheck SA1019 lint failures - Add @appspot.gserviceaccount.com to isValidServiceAccountEmail() for App Engine default service accounts - Document that project ID for default Compute/App Engine SAs comes from the broker record (metadata server), not from email parsing - Add test cases for appspot SA validation, passthrough gate, and broker update
* docs: add npm registry/proxy section to custom-images guide Document the NPM_REGISTRY build arg and NPM_CONFIG_FILE BuildKit secret introduced in #1476, which enable image builds behind corporate proxies where registry.npmjs.org is blocked. Changelog: 2026-09-06 Other changelog items reviewed — no further docs impact: - #1478, #1480, #1483, #1481, #1479: internal fixes - #1482: host SA auto-detection (docs had no manual step to remove) - #1475: Azure DevOps URL parsing fix (no interface change) - #1474: UI fix; #1473: demo script fix * docs: add weekly release notes for Aug 31 - Sep 6, 2026 --------- Co-authored-by: Scion Agent (du-0906) <agent@scion.dev>
Summary
Changes
Test plan
Tracking: ptone#1450