fix(functions): defer secret access granting to release phase to prevent SA 404 race condition - #10983
Merged
Conversation
…ent SA 404 race condition ### Description Refactors secret access permission granting during Cloud Functions deployment: - Refactors ensure.secretAccess into ensure.secretsAccessDelta (to calculate secret-to-service-account mappings), ensure.checkSecretAccess (for dry-run output), and ensure.grantSecretAccess (to grant IAM permissions per secret). - Attaches secretAccessPlan to CodebasePlan in planner.ts. - In prepare.ts, computes secretsAccessDelta and runs checkSecretAccess only during --dry-run. Added explicit comments noting that actual grants take place in release. - In fabricator.ts, enqueues grantSecretAccess execution after grantNewRoles() runs (ensuring service accounts are created first) with retryPredicates: [isTransientError, isServiceAccount404] to gracefully handle GCP IAM propagation delays. ### Scenarios Tested - Ran unit test suite covering executor.spec.ts, ensure.spec.ts, planner.spec.ts, fabricator.spec.ts, and lifecycle.spec.ts (182 passing). - Verified npm run lint:changed-files and npm run build. ### Sample Commands - firebase deploy --only functions - firebase deploy --only functions --dry-run
inlined
requested review from
Berlioz and
ajperel
and removed request for
ajperel
August 25, 2026 00:44
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request resolves a secret access permission race condition during Cloud Functions deployment by deferring secret grants to the release phase after service accounts are created. It refactors the executor's retry mechanism to use predicates, enabling retries on service account 404 errors, and adds automatic cleanup of newly created service accounts upon complete deployment failure. Feedback on these changes highlights a performance regression in secretAccess where secret grants are processed serially instead of in parallel.
ajperel
approved these changes
Aug 25, 2026
ajperel
left a comment
ajperel
left a comment
Contributor
There was a problem hiding this comment.
Nice. I realize dry-run could benefit from larger improvements but this seems a step in the right direction. Thanks for quickly fixing this!
inlined
force-pushed
the
stellar_wave_sails_13h41
branch
from
August 26, 2026 04:55
ba92705 to
6527959
Compare
inlined
enabled auto-merge (squash)
August 26, 2026 04:58
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
3 participants
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.
Description
Refactors secret access permission granting during Cloud Functions deployment:
Scenarios Tested
Sample Commands
Notes
This issue revealed that I didn't know about the --dry-run command and it might be worth revisiting this command. I left the existing code in prepare becasue dry-run skips release, but there is legitimate stuff happening in release. We should eventually do a tech debt pass where we maybe stop running parallel code in prepare and instead run --dry-run throughout the codebase. This can reveal things in deploy like "create X bucket" "upload to X artfiact registry" "Create X task queue" "create X service account with Y roles". There's simply too much right now to fix it all and I opted for preserving existing behavior rather than improving it.