From 433a4912ad4ca085d81c988c76a65c727734a7ce Mon Sep 17 00:00:00 2001 From: Andy Perelson Date: Mon, 7 Sep 2026 22:46:01 +0000 Subject: [PATCH] fix(functions): clean up managed service accounts on declarative opt-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 ``` --- CHANGELOG.md | 1 + .../functions/functionsDeployHelper.spec.ts | 36 +++++++++++++++++++ src/deploy/functions/functionsDeployHelper.ts | 10 ++++++ src/deploy/functions/prepare.ts | 8 ++--- src/deploy/functions/release/planner.spec.ts | 18 ++++++++++ src/deploy/functions/release/planner.ts | 9 ++--- 6 files changed, 70 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..5352f64cb6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- [fixed] Clean up managed service accounts when opting out of declarative security alongside a filtered codebase deploy. diff --git a/src/deploy/functions/functionsDeployHelper.spec.ts b/src/deploy/functions/functionsDeployHelper.spec.ts index 456f42e9b55..72ccd8d008b 100644 --- a/src/deploy/functions/functionsDeployHelper.spec.ts +++ b/src/deploy/functions/functionsDeployHelper.spec.ts @@ -642,4 +642,40 @@ describe("functionsDeployHelper", () => { expect(collisions).to.be.empty; }); }); + + describe("isCodebasePartiallyFiltered", () => { + it("should return false when filters is undefined or empty", () => { + expect(helper.isCodebasePartiallyFiltered("codebaseA")).to.be.false; + expect(helper.isCodebasePartiallyFiltered("codebaseA", [])).to.be.false; + }); + + it("should return false when a filter targets the whole codebase without idChunks", () => { + expect(helper.isCodebasePartiallyFiltered("codebaseA", [{ codebase: "codebaseA" }])).to.be + .false; + expect( + helper.isCodebasePartiallyFiltered("codebaseA", [{ codebase: "codebaseA", idChunks: [] }]), + ).to.be.false; + }); + + it("should return false when filter with idChunks targets a different codebase", () => { + expect( + helper.isCodebasePartiallyFiltered("codebaseA", [ + { codebase: "codebaseA" }, + { codebase: "codebaseB", idChunks: ["funcB"] }, + ]), + ).to.be.false; + }); + + it("should return true when filter with idChunks targets this codebase", () => { + expect( + helper.isCodebasePartiallyFiltered("codebaseA", [ + { codebase: "codebaseA", idChunks: ["funcA"] }, + ]), + ).to.be.true; + }); + + it("should return true when filter with idChunks has no codebase (wildcard filter)", () => { + expect(helper.isCodebasePartiallyFiltered("codebaseA", [{ idChunks: ["funcA"] }])).to.be.true; + }); + }); }); diff --git a/src/deploy/functions/functionsDeployHelper.ts b/src/deploy/functions/functionsDeployHelper.ts index 8094bc131b6..dc4175e7219 100644 --- a/src/deploy/functions/functionsDeployHelper.ts +++ b/src/deploy/functions/functionsDeployHelper.ts @@ -249,6 +249,16 @@ export function isEndpointFiltered(endpoint: backend.Endpoint, filters: Endpoint return filters.some((filter) => endpointMatchesFilter(endpoint, filter)); } +/** Checks if a codebase has any intra-codebase (partial) filters targeting specific function IDs. */ +export function isCodebasePartiallyFiltered(codebase: string, filters?: EndpointFilter[]): boolean { + if (!filters) { + return false; + } + return filters.some( + (f) => (!f.codebase || f.codebase === codebase) && !!f.idChunks && f.idChunks.length > 0, + ); +} + /** * Parses raw CLI filter strings for functions:delete into EndpointFilter objects. * diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 343a3fb79c1..33f68d74dd4 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -28,6 +28,7 @@ import { endpointMatchesAnyFilter, getEndpointFilters, groupEndpointsByCodebase, + isCodebasePartiallyFiltered, targetCodebases, } from "./functionsDeployHelper"; import { logLabeledBullet, logLabeledWarning } from "../../utils"; @@ -107,12 +108,7 @@ export async function discoverSecurityDetails( (e) => !!e.labels?.[DECLARATIVE_SECURITY_ETAG_LABEL], )?.labels?.[DECLARATIVE_SECURITY_ETAG_LABEL]; - const isPartiallyFiltered = !!( - filters && - filters.some( - (f) => (!f.codebase || f.codebase === codebase) && f.idChunks && f.idChunks.length > 0, - ) - ); + const isPartiallyFiltered = isCodebasePartiallyFiltered(codebase, filters); const isEnrolling = !!requiredRoles && !existingManagedSA; const isUnenrolling = !requiredRoles && !!existingManagedSA && !!haveRolesEtag; diff --git a/src/deploy/functions/release/planner.spec.ts b/src/deploy/functions/release/planner.spec.ts index c5bb9ec65de..09df0cb5d1a 100644 --- a/src/deploy/functions/release/planner.spec.ts +++ b/src/deploy/functions/release/planner.spec.ts @@ -591,6 +591,24 @@ describe("planner", () => { ); }); + it("deletes the service account when opting out with a full codebase deploy alongside a filtered deploy in another codebase", async () => { + const wantBackend = backend.empty(); + const haveBackend = backend.of(func("id", "region")); + + const plan = await planner.createDeploymentPlan({ + wantBackend, + haveBackend, + codebase: "codebaseA", + projectId: "my-project", + filters: [{ codebase: "codebaseA" }, { codebase: "codebaseB", idChunks: ["funcB"] }], + existingManagedSA: "firebase-fn-123@my-project.iam.gserviceaccount.com", + }); + + expect(plan.serviceAccountToDelete).to.equal( + "firebase-fn-123@my-project.iam.gserviceaccount.com", + ); + }); + it("deletes the service account when opting out during an unfiltered deploy", async () => { const wantBackend = backend.empty(); const haveBackend = backend.of(func("id", "region")); diff --git a/src/deploy/functions/release/planner.ts b/src/deploy/functions/release/planner.ts index 950951da5f9..19de0d06d03 100644 --- a/src/deploy/functions/release/planner.ts +++ b/src/deploy/functions/release/planner.ts @@ -2,6 +2,7 @@ import { EndpointFilter, endpointMatchesAnyFilter, getFunctionLabel, + isCodebasePartiallyFiltered, } from "../functionsDeployHelper"; import { isFirebaseManaged } from "../../../deploymentTool"; import { FirebaseError } from "../../../error"; @@ -177,11 +178,7 @@ export async function createDeploymentPlan(args: PlanArgs): Promise f.idChunks && f.idChunks.length > 0) && - !deleteAll - ); + const isPartiallyFiltered = isCodebasePartiallyFiltered(codebase, filters); const hasWantEndpoints = backend.someEndpoint(wantBackend, () => true); @@ -191,7 +188,7 @@ export async function createDeploymentPlan(args: PlanArgs): Promise