diff --git a/CHANGELOG.md b/CHANGELOG.md index e6fc050cb03..55fe99e507c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,4 @@ - Fix webframeworks deployments when using multiple hosting sites in `firebase.json`. (#8314) - Add a new command to setup a cleanup policy for functions artifacts. (#8268) - Support 3rd party builders for Angular. (#7557) +- Fix GCF V2 artifact cleanup by correctly encoding artifact names to match GCF V2's format (#8318) diff --git a/src/deploy/functions/containerCleaner.spec.ts b/src/deploy/functions/containerCleaner.spec.ts index a5d4b3c79ce..043847a8256 100644 --- a/src/deploy/functions/containerCleaner.spec.ts +++ b/src/deploy/functions/containerCleaner.spec.ts @@ -9,15 +9,24 @@ import * as docker from "../../gcp/docker"; import * as poller from "../../operation-poller"; import * as utils from "../../utils"; +function createTestEndpoint(overrides: Partial = {}): backend.Endpoint { + return { + id: "id", + region: "us-central1", + project: "project", + platform: "gcfv2", + entryPoint: "function", + runtime: "nodejs16", + httpsTrigger: {}, + ...overrides, + }; +} + describe("CleanupBuildImages", () => { let gcr: sinon.SinonStubbedInstance; let ar: sinon.SinonStubbedInstance; let logLabeledWarning: sinon.SinonStub; - const TARGET: backend.TargetIds = { - project: "project", - region: "us-central1", - id: "id", - }; + const TARGET: backend.Endpoint = createTestEndpoint(); beforeEach(() => { gcr = sinon.createStubInstance(containerCleaner.ContainerRegistryCleaner); @@ -190,53 +199,118 @@ describe("ArtifactRegistryCleaner", () => { sinon.verifyAndRestore(); }); + describe("packagePath", () => { + it("uses V1 encoding for gcfv1 functions", () => { + const func = createTestEndpoint({ + id: "helloWorldV1", + region: "us-central1", + project: "my-project", + platform: "gcfv1", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/my-project/locations/us-central1/repositories/gcf-artifacts/packages/hello_world_v1", + ); + }); + + it("uses V2 encoding for gcfv2 functions", () => { + const func = createTestEndpoint({ + id: "helloWorldV2", + region: "us-central1", + project: "my-project", + platform: "gcfv2", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/my-project/locations/us-central1/repositories/gcf-artifacts/packages/my--project__us--central1__hello_world_v2", + ); + }); + + it("encodes V2 project IDs with dashes", () => { + const func = createTestEndpoint({ + id: "function", + region: "region", + project: "my-cool-project", + platform: "gcfv2", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/my-cool-project/locations/region/repositories/gcf-artifacts/packages/my--cool--project__region__function", + ); + }); + + it("encodes V2 project IDs with underscores", () => { + const func = createTestEndpoint({ + id: "function", + region: "region", + project: "my_cool_project", + platform: "gcfv2", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/my_cool_project/locations/region/repositories/gcf-artifacts/packages/my__cool__project__region__function", + ); + }); + + it("encodes V2 regions with dashes", () => { + const func = createTestEndpoint({ + id: "function", + region: "us-central1", + project: "project", + platform: "gcfv2", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/project/locations/us-central1/repositories/gcf-artifacts/packages/project__us--central1__function", + ); + }); + + it("encodes V2 function IDs with capital letters", () => { + const func = createTestEndpoint({ + id: "Strange-Casing_cases", + region: "region", + project: "project", + platform: "gcfv2", + }); + + expect(containerCleaner.ArtifactRegistryCleaner.packagePath(func)).to.equal( + "projects/project/locations/region/repositories/gcf-artifacts/packages/project__region__s-strange--_casing__cases", + ); + }); + }); + it("deletes artifacts", async () => { const cleaner = new containerCleaner.ArtifactRegistryCleaner(); - const func = { + const func = createTestEndpoint({ id: "function", region: "region", project: "project", - }; + platform: "gcfv2", + }); ar.deletePackage.returns(Promise.resolve({ name: "op" } as any)); await cleaner.cleanupFunction(func); expect(ar.deletePackage).to.have.been.calledWith( - "projects/project/locations/region/repositories/gcf-artifacts/packages/function", + "projects/project/locations/region/repositories/gcf-artifacts/packages/project__region__function", ); expect(poll.pollOperation).to.have.been.called; }); it("bypasses poller if the operation is completed", async () => { const cleaner = new containerCleaner.ArtifactRegistryCleaner(); - const func = { + const func = createTestEndpoint({ id: "function", region: "region", project: "project", - }; - - ar.deletePackage.returns(Promise.resolve({ name: "op", done: true })); - - await cleaner.cleanupFunction(func); - expect(ar.deletePackage).to.have.been.calledWith( - "projects/project/locations/region/repositories/gcf-artifacts/packages/function", - ); - expect(poll.pollOperation).to.not.have.been.called; - }); - - it("encodeds to avoid upper-case letters", async () => { - const cleaner = new containerCleaner.ArtifactRegistryCleaner(); - const func = { - id: "Strange-Casing_cases", - region: "region", - project: "project", - }; + platform: "gcfv2", + }); ar.deletePackage.returns(Promise.resolve({ name: "op", done: true })); await cleaner.cleanupFunction(func); expect(ar.deletePackage).to.have.been.calledWith( - "projects/project/locations/region/repositories/gcf-artifacts/packages/s-strange--_casing__cases", + "projects/project/locations/region/repositories/gcf-artifacts/packages/project__region__function", ); expect(poll.pollOperation).to.not.have.been.called; }); diff --git a/src/deploy/functions/containerCleaner.ts b/src/deploy/functions/containerCleaner.ts index 7ab2207d931..27045acbb9d 100644 --- a/src/deploy/functions/containerCleaner.ts +++ b/src/deploy/functions/containerCleaner.ts @@ -39,8 +39,8 @@ async function retry(func: () => Promise): Promise { } export async function cleanupBuildImages( - haveFunctions: backend.TargetIds[], - deletedFunctions: backend.TargetIds[], + haveFunctions: backend.Endpoint[], + deletedFunctions: backend.Endpoint[], cleaners: { gcr?: ContainerRegistryCleaner; ar?: ArtifactRegistryCleaner } = {}, ): Promise { utils.logBullet(clc.bold(clc.cyan("functions: ")) + "cleaning up build files..."); @@ -100,18 +100,12 @@ export async function cleanupBuildImages( // than the raw Docker API. If there are reports of any quota issues we may have to run these // requests through a ThrottlerQueue. export class ArtifactRegistryCleaner { - static packagePath(func: backend.TargetIds): string { - // GCFv1 names can include upper-case letters, but docker images cannot. - // to fix this, the artifact registry path for these images uses a custom encoding scheme. - // * Underscores are doubled - // * Dashes are doubled - // * A leading capital letter is replaced with - // * Other capital letters are replaced with - const encodedId = func.id - .replace(/_/g, "__") - .replace(/-/g, "--") - .replace(/^[A-Z]/, (first) => `${first.toLowerCase()}-${first.toLowerCase()}`) - .replace(/[A-Z]/g, (upper) => `_${upper.toLowerCase()}`); + static packagePath(func: backend.Endpoint): string { + const encodedId = + func.platform === "gcfv2" + ? ArtifactRegistryCleaner.encodePackageNameV2(func) + : ArtifactRegistryCleaner.encodePackageNameV1(func); + return `projects/${func.project}/locations/${func.region}/repositories/gcf-artifacts/packages/${encodedId}`; } @@ -121,6 +115,42 @@ export class ArtifactRegistryCleaner { masterTimeout: 5 * 60 * 1_000, }; + private static encodePart(part: string): string { + return part + .replace(/_/g, "__") + .replace(/-/g, "--") + .replace(/^[A-Z]/, (first) => `${first.toLowerCase()}-${first.toLowerCase()}`) + .replace(/[A-Z]/g, (upper) => `_${upper.toLowerCase()}`); + } + + // GCF V1: Simple underscore concatenation + // Example: "helloWorldV1" -> "hello_world_v1" + private static encodePackageNameV1(func: backend.TargetIds): string { + return ArtifactRegistryCleaner.encodePart(func.id); + } + + // GCF V2 artifact names follow this schema: + // {encoded_project}__{encoded_region}__{encoded_function} + // + // Each part is encoded separately with these rules: + // * Underscores are doubled ("_" -> "__") + // * Dashes are doubled ("-" -> "--") + // * A leading capital letter is replaced with + // * Other capital letters are replaced with + // Then the parts are joined with double underscores + // Example: + // - project "my-cool-project" -> "my--cool--project" + // - region "us-central1" -> "us--central1" + // - functionId "myFunction" -> "my_function" + // Final result: "my--cool--project__us--central1__my_function" + private static encodePackageNameV2(func: backend.TargetIds): string { + return [ + ArtifactRegistryCleaner.encodePart(func.project), + ArtifactRegistryCleaner.encodePart(func.region), + ArtifactRegistryCleaner.encodePart(func.id), + ].join("__"); + } + // GCFv1 for AR has the following directory structure // Hostname: -docker.pkg.dev // Directory structure: @@ -130,7 +160,7 @@ export class ArtifactRegistryCleaner { // We leave the cache directory alone because it only costs // a few MB and improves performance. We only delete the cache if // the function was deleted in its entirety. - async cleanupFunction(func: backend.TargetIds): Promise { + async cleanupFunction(func: backend.Endpoint): Promise { let op: artifactregistry.Operation; try { op = await artifactregistry.deletePackage(ArtifactRegistryCleaner.packagePath(func));