Fix GCF V2 Artifact Registry cleanup

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)

src/deploy/functions/containerCleaner.spec.ts

@@ -9,15 +9,24 @@
 import * as poller from "../../operation-poller";
 import * as utils from "../../utils";
 
+function createTestEndpoint(overrides: Partial<backend.Endpoint> = {}): backend.Endpoint {
+  return {
+    id: "id",
+    region: "us-central1",
+    project: "project",
+    platform: "gcfv2",
+    entryPoint: "function",
+    runtime: "nodejs16",
+    httpsTrigger: {},
+    ...overrides,
+  };
+}
+
 describe("CleanupBuildImages", () => {
   let gcr: sinon.SinonStubbedInstance<containerCleaner.ContainerRegistryCleaner>;
   let ar: sinon.SinonStubbedInstance<containerCleaner.ArtifactRegistryCleaner>;
   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);
@@ -30,13 +39,13 @@
   });
 
   it("uses GCR and AR", async () => {
     await containerCleaner.cleanupBuildImages([TARGET], [], { gcr, ar } as any);
     expect(gcr.cleanupFunction).to.have.been.called;
   });
 
   it("reports failed domains from AR", async () => {
     ar.cleanupFunction.rejects(new Error("uh oh"));
     await containerCleaner.cleanupBuildImages([], [TARGET], { gcr, ar } as any);
     expect(logLabeledWarning).to.have.been.calledWithMatch(
       "functions",
       new RegExp(
@@ -47,7 +56,7 @@
 
   it("reports failed domains from GCR", async () => {
     gcr.cleanupFunction.rejects(new Error("uh oh"));
     await containerCleaner.cleanupBuildImages([], [TARGET], { gcr, ar } as any);
     expect(logLabeledWarning).to.have.been.calledWithMatch(
       "functions",
       new RegExp("https://console.cloud.google.com/gcr/images/project/us/gcf"),
@@ -76,8 +85,8 @@
     name: "foo/bar",
     tags: ["tag1", "tag2"],
     manifest: {
       "sha256:hash1": {} as any,
       "sha256:hash2": {} as any,
     },
     child: ["baz"],
   };
@@ -190,53 +199,118 @@
     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;
   });

src/deploy/functions/containerCleaner.ts

@@ -39,8 +39,8 @@ async function retry<Return>(func: () => Promise<Return>): Promise<Return> {
 }
 
 export async function cleanupBuildImages(
-  haveFunctions: backend.TargetIds[],
-  deletedFunctions: backend.TargetIds[],
+  haveFunctions: backend.Endpoint[],
+  deletedFunctions: backend.Endpoint[],
   cleaners: { gcr?: ContainerRegistryCleaner; ar?: ArtifactRegistryCleaner } = {},
 ): Promise<void> {
   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 <lower><dash><lower>
-    // * Other capital letters are replaced with <underscore><lower>
-    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 <lower><dash><lower>
+  // * Other capital letters are replaced with <underscore><lower>
+  // 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: <region>-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<void> {
+  async cleanupFunction(func: backend.Endpoint): Promise<void> {
     let op: artifactregistry.Operation;
     try {
       op = await artifactregistry.deletePackage(ArtifactRegistryCleaner.packagePath(func));