Skip to content

fix(functions): clean up managed service accounts on declarative opt-out during filtered multi-codebase deploys - #11052

Merged
ajperel merged 1 commit into
mainfrom
ajp/fix-declartive-partial-deployment
Sep 10, 2026
Merged

fix(functions): clean up managed service accounts on declarative opt-out during filtered multi-codebase deploys#11052
ajperel merged 1 commit into
mainfrom
ajp/fix-declartive-partial-deployment

Conversation

@ajperel

@ajperel ajperel commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Description

When deploying multiple codebases where one codebase opts out of declarative security while another codebase uses a partial function filter (e.g. --only functions:codebaseA,functions:codebaseB:funcB), prepare.ts scoped its partial-filter check to codebaseA, discovering the unenrollment and updating functions to remove declarative labels and IAM bindings.

However, planner.ts checked function filters globally across all codebases. This marked codebaseA as partially filtered, suppressing deletion of the managed service account and permanently orphaning it in GCP IAM.

This change:

  • Extracts isCodebasePartiallyFiltered(codebase, filters) into functionsDeployHelper.ts to share consistent codebase-scoped partial-filter evaluation across discovery (prepare.ts) and planning (planner.ts).
  • Updates the deployment planner to scope partial-filter checks to the target codebase, ensuring the managed service account is deleted during opt-out unless that specific codebase is partially filtered.

Scenarios Tested

  • Unit tests:
    • src/deploy/functions/functionsDeployHelper.spec.ts: Added tests for isCodebasePartiallyFiltered covering undefined/empty filters, full codebase filters, foreign codebase partial filters, matching partial filters, and wildcard partial filters.
    • src/deploy/functions/release/planner.spec.ts: Added regression test verifying that opting out of a whole codebase alongside a filtered codebase deploy schedules the managed service account for deletion.
  • npm test

Sample Commands

firebase deploy --only functions:codebaseA,functions:codebaseB:funcB
firebase deploy --only functions:codebaseA

@ajperel
ajperel requested a review from Berlioz September 7, 2026 22:46

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a fix to clean up managed service accounts when opting out of declarative security alongside a filtered codebase deploy. It extracts the logic to check if a codebase is partially filtered into a helper function isCodebasePartiallyFiltered in functionsDeployHelper.ts, which is then utilized in both prepare.ts and planner.ts. Unit tests have been added to verify this helper function and the updated planner behavior. There are no review comments, and I have no additional feedback to provide.

…out during filtered multi-codebase deploys

When deploying multiple codebases where one codebase opts out of declarative security while another codebase uses a partial function filter (e.g. `--only functions:codebaseA,functions:codebaseB:funcB`), `prepare.ts` scoped its partial-filter check to `codebaseA`, discovering the unenrollment and updating functions to remove declarative labels and IAM bindings.

However, `planner.ts` checked function filters globally across all codebases. This marked `codebaseA` as partially filtered, suppressing deletion of the managed service account and permanently orphaning it in GCP IAM.

This change:
- Extracts `isCodebasePartiallyFiltered(codebase, filters)` into `functionsDeployHelper.ts` to share consistent codebase-scoped partial-filter evaluation across discovery (`prepare.ts`) and planning (`planner.ts`).
- Updates the deployment planner to scope partial-filter checks to the target codebase, ensuring the managed service account is deleted during opt-out unless that specific codebase is partially filtered.

- **Unit tests**:
  - `src/deploy/functions/functionsDeployHelper.spec.ts`: Added tests for `isCodebasePartiallyFiltered` covering undefined/empty filters, full codebase filters, foreign codebase partial filters, matching partial filters, and wildcard partial filters.
  - `src/deploy/functions/release/planner.spec.ts`: Added regression test verifying that opting out of a whole codebase alongside a filtered codebase deploy schedules the managed service account for deletion.
- `npm test`

```bash
firebase deploy --only functions:codebaseA,functions:codebaseB:funcB
firebase deploy --only functions:codebaseA
```
@ajperel
ajperel force-pushed the ajp/fix-declartive-partial-deployment branch from fe188b7 to 433a491 Compare September 9, 2026 23:49
@ajperel
ajperel enabled auto-merge (squash) September 9, 2026 23:50
@ajperel
ajperel merged commit 7b6fe96 into main Sep 10, 2026
57 of 59 checks passed
@ajperel
ajperel deleted the ajp/fix-declartive-partial-deployment branch September 10, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants