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