fix(delete-user-data): validate EXT_SELECTED_EVENTS and EVENTARC_CHANNEL at deploy

kits/delete-user-data/CHANGELOG.md

@@ -1,3 +1,5 @@
+- fix: `EXT_SELECTED_EVENTS` is validated at deploy, as the extension's install did. firebase-admin splits the list on commas without trimming, so `a, b, c` published only `a` and silently dropped the rest, and a type with different case or a typo was dropped without a warning. The deploy now fails if an entry has spaces or different case, isn't one of the three event types, or is listed twice. It also fails if event types are listed without `EVENTARC_CHANNEL`, or the channel isn't a full `projects/<project id>/locations/<location>/channels/<channel id>` name in this project and a location the extension supported (`us-central1`, `us-west1`, `europe-west4` or `asia-northeast1`); before, events were silently turned off. Empty entries are still skipped, and an unset or empty value still publishes nothing.
+
 - fix: `clearData`, `handleSearch` and `handleDeletion` can scale to 3000 instances each again, as the extension's 1st gen functions could. The kit set no `maxInstances`, so each function took the 2nd gen default of 100, and with one event per instance it handled at most 100 deletions or searches at once. `setGlobalOptions({ maxInstances })` still overrides it. Existing kit deployments switch on their next deploy.
 
 - fix: the functions deploy to `us-central1` by default again, as the extension did. The kit set no region, so the Firebase CLI deployed them to `us-east1`. The region is now read from `FUNCTION_DEFAULT_REGION` (default `us-central1`), the name `firebase ext:migrate` writes to `.env`, so each instance can deploy to its own region. `setGlobalOptions({ region })` no longer moves the functions; set `FUNCTION_DEFAULT_REGION` instead.

kits/delete-user-data/README.md

@@ -174,9 +174,16 @@ The functions can publish deletion events for each backend under
 | `EVENTARC_CHANNEL` | Full channel name, e.g. `projects/<project id>/locations/us-central1/channels/firebase` |
 | `EXT_SELECTED_EVENTS` | Comma-separated event types to publish, e.g. `firebase.extensions.delete-user-data.v1.firestore,firebase.extensions.delete-user-data.v1.storage` |
 
-Only the listed types are published. If `EXT_SELECTED_EVENTS` is unset or
-empty, nothing is published, which matches an extension instance installed with
-no event types selected.
+Only the listed types are published. List the types exactly as shown,
+separated by commas with no spaces. Empty entries are skipped. The deploy fails
+if an entry has spaces or different case, isn't one of the three types, or is
+listed twice, or if types are listed but `EVENTARC_CHANNEL` is unset, as the
+extension's install did. It also fails if `EVENTARC_CHANNEL` isn't a full
+channel name, is in another project, or is in a location other than
+`us-central1`, `us-west1`, `europe-west4` or `asia-northeast1`, the locations
+the extension supported. If `EXT_SELECTED_EVENTS` is unset or empty, nothing
+is published, which matches an extension instance installed with no event types
+selected.
 
 The channel must already exist. Deploying the kit does not create it, and
 publishing to a missing channel logs a warning without failing the function.

kits/delete-user-data/src/events.ts

@@ -19,6 +19,91 @@ import { logger } from "firebase-functions";
 
 let eventChannel: eventArc.Channel | undefined;
 
+const EVENT_TYPES: ReadonlyArray<string> = [
+  "firestore",
+  "database",
+  "storage",
+].map((t) => `firebase.extensions.delete-user-data.v1.${t}`);
+
+const CHANNEL_NAME = /^projects\/([^/]+)\/locations\/([^/]+)\/channels\/[^/]+$/;
+
+// The channel locations the extension's install accepted, as in the Firebase
+// CLI's ALLOWED_EVENT_ARC_REGIONS.
+const CHANNEL_LOCATIONS: ReadonlyArray<string> = [
+  "us-central1",
+  "us-west1",
+  "europe-west4",
+  "asia-northeast1",
+];
+
+/**
+ * Reads `EXT_SELECTED_EVENTS`, failing on a value the extension's install
+ * rejected.
+ *
+ * Splits on "," and skips "" (as the CLI did); rejects anything that isn't an
+ * exact declared type, or is listed twice (as the platform did). Also rejects
+ * event types without `EVENTARC_CHANNEL`, and a channel that is malformed, in
+ * another project or in an unsupported location, as the install did. Called at
+ * module load so a bad value fails the deploy.
+ */
+export function selectedEventTypes(): string[] {
+  const types = (process.env.EXT_SELECTED_EVENTS ?? "")
+    .split(",")
+    .filter((t) => t !== "");
+  // A repeat is reported as a duplicate even if it isn't a declared type, as
+  // the platform did.
+  const errors = types.flatMap((t, i) =>
+    types.indexOf(t) !== i
+      ? [`'${t}' is listed more than once`]
+      : !EVENT_TYPES.includes(t)
+      ? [`'${t}' is not one of ${EVENT_TYPES.join(", ")}`]
+      : []
+  );
+  if (errors.length) {
+    throw new Error(
+      `EXT_SELECTED_EVENTS is invalid: ${errors.join("; ")}. ` +
+        "List event types separated by commas, without spaces."
+    );
+  }
+
+  const channel = process.env.EVENTARC_CHANNEL;
+  if (types.length > 0 && !channel) {
+    throw new Error(
+      "EXT_SELECTED_EVENTS is set but EVENTARC_CHANNEL is not. " +
+        "Must specify eventarc channel if allowing some events to be emitted."
+    );
+  }
+  if (channel) {
+    const match = CHANNEL_NAME.exec(channel);
+    if (!match) {
+      throw new Error(
+        `EVENTARC_CHANNEL '${channel}' is invalid. Use the full channel name, ` +
+          "e.g. projects/<project id>/locations/<location>/channels/<channel id>."
+      );
+    }
+    const [, project, location] = match;
+    // Set by the CLI when it loads the kit, and at runtime.
+    const currentProject = process.env.GCLOUD_PROJECT;
+    const channelErrors = [
+      ...(CHANNEL_LOCATIONS.includes(location)
+        ? []
+        : [
+            `location '${location}' is not supported, use one of ` +
+              CHANNEL_LOCATIONS.join(", "),
+          ]),
+      ...(currentProject && project !== currentProject
+        ? [`it must be in the same project, '${currentProject}'`]
+        : []),
+    ];
+    if (channelErrors.length) {
+      throw new Error(
+        `EVENTARC_CHANNEL '${channel}' is invalid: ${channelErrors.join("; ")}.`
+      );
+    }
+  }
+  return types;
+}
+
 /**
  * Opens the Eventarc channel only when event types are selected.
  *
@@ -29,10 +114,10 @@ let eventChannel: eventArc.Channel | undefined;
  */
 export const setupEventChannel = (): void => {
   const channel = process.env.EVENTARC_CHANNEL;
-  const selectedEvents = process.env.EXT_SELECTED_EVENTS;
+  const selectedEvents = selectedEventTypes();
 
   eventChannel =
-    channel && selectedEvents
+    channel && selectedEvents.length > 0
       ? eventArc.getEventarc().channel(channel, {
           allowedEventTypes: selectedEvents,
         })

kits/delete-user-data/src/index.ts

@@ -33,6 +33,7 @@ import {
 import * as logs from "./logs";
 
 assertParams();
+events.selectedEventTypes();
 export * from "./lib";
 
 const REQUIRED_ROLES: ReadonlyArray<Role> = [

kits/delete-user-data/tests/events.test.ts

@@ -24,13 +24,14 @@ vi.mock("firebase-admin/eventarc", () => ({
   getEventarc: () => ({ channel }),
 }));
 
-const CHANNEL = "projects/p/locations/l/channels/firebase";
+const CHANNEL = "projects/p/locations/us-central1/channels/firebase";
 
-const ALL_EVENTS = [
+const ALL_EVENT_TYPES = [
   "firebase.extensions.delete-user-data.v1.firestore",
   "firebase.extensions.delete-user-data.v1.database",
   "firebase.extensions.delete-user-data.v1.storage",
-].join(",");
+];
+const ALL_EVENTS = ALL_EVENT_TYPES.join(",");
 
 // `null` unsets the variable; `undefined` would trip importEvents' default.
 function setEnv(name: string, value: string | null | undefined) {
@@ -55,14 +56,17 @@ async function importEvents(
 
 const ORIGINAL_CHANNEL = process.env.EVENTARC_CHANNEL;
 const ORIGINAL_SELECTED_EVENTS = process.env.EXT_SELECTED_EVENTS;
+const ORIGINAL_PROJECT = process.env.GCLOUD_PROJECT;
 
 beforeEach(() => {
   vi.clearAllMocks();
+  setEnv("GCLOUD_PROJECT", "p");
 });
 
 afterEach(() => {
   setEnv("EVENTARC_CHANNEL", ORIGINAL_CHANNEL);
   setEnv("EXT_SELECTED_EVENTS", ORIGINAL_SELECTED_EVENTS);
+  setEnv("GCLOUD_PROJECT", ORIGINAL_PROJECT);
 });
 
 describe("setupEventChannel", () => {
@@ -73,10 +77,48 @@ describe("setupEventChannel", () => {
     );
 
     expect(channel).toHaveBeenCalledWith(CHANNEL, {
-      allowedEventTypes: "firebase.extensions.delete-user-data.v1.firestore",
+      allowedEventTypes: ["firebase.extensions.delete-user-data.v1.firestore"],
     });
   });
 
+  // The CLI dropped empty entries before the platform checked the list.
+  test("skips empty entries", async () => {
+    await importEvents(CHANNEL, `,${ALL_EVENT_TYPES.join(",,")},`);
+
+    expect(channel).toHaveBeenCalledWith(CHANNEL, {
+      allowedEventTypes: ALL_EVENT_TYPES,
+    });
+  });
+
+  // The extension's install rejected each of these, so the deploy fails
+  // instead of trimming or dropping entries.
+  test.each([
+    ["spaces after commas", ALL_EVENT_TYPES.join(", ")],
+    ["surrounding whitespace", `  ${ALL_EVENT_TYPES.join(" ,  ")}  `],
+    ["whitespace only", "   "],
+    ["only commas and spaces", " , ,"],
+    ["different case", "firebase.extensions.delete-user-data.v1.Firestore"],
+    [
+      "an unknown type",
+      "firebase.extensions.delete-user-data.v1.bogus,firebase.extensions.delete-user-data.v1.database",
+    ],
+    ["a duplicate type", `${ALL_EVENT_TYPES[0]},${ALL_EVENT_TYPES[0]}`],
+    ["a ';' separator", ALL_EVENT_TYPES.join(";")],
+  ])("rejects EXT_SELECTED_EVENTS with %s", async (_label, selectedEvents) => {
+    await expect(importEvents(CHANNEL, selectedEvents)).rejects.toThrow(
+      "EXT_SELECTED_EVENTS is invalid"
+    );
+    expect(channel).not.toHaveBeenCalled();
+  });
+
+  // The platform reported a repeat as a duplicate before checking the type.
+  test("reports a repeated unknown entry as listed more than once", async () => {
+    const load = importEvents(CHANNEL, " , ,");
+
+    await expect(load).rejects.toThrow("' ' is not one of");
+    await expect(load).rejects.toThrow("' ' is listed more than once");
+  });
+
   // The extension cleared its channel when no event types were selected, so
   // it published nothing. firebase-admin reads a missing allowlist as "allow
   // every type", which would publish all three.
@@ -96,13 +138,76 @@ describe("setupEventChannel", () => {
     }
   );
 
-  test("stays disabled when no channel is configured", async () => {
-    const events = await importEvents();
+  test("stays disabled when neither is configured", async () => {
+    const events = await importEvents(undefined, null);
 
     expect(channel).not.toHaveBeenCalled();
     await events.publishDeletionEvent("firestore", { uid: "u1" });
     expect(publish).not.toHaveBeenCalled();
   });
+
+  // The extension's install rejected selected events without a channel.
+  test("rejects selected event types without EVENTARC_CHANNEL", async () => {
+    await expect(importEvents(undefined, ALL_EVENTS)).rejects.toThrow(
+      "Must specify eventarc channel"
+    );
+    expect(channel).not.toHaveBeenCalled();
+  });
+
+  test.each([
+    ["a bare channel id", "firebase"],
+    ["a missing location", "projects/p/channels/firebase"],
+    ["a trailing slash", `${CHANNEL}/`],
+  ])("rejects an EVENTARC_CHANNEL with %s", async (_label, channelName) => {
+    await expect(importEvents(channelName, ALL_EVENTS)).rejects.toThrow(
+      "EVENTARC_CHANNEL"
+    );
+    expect(channel).not.toHaveBeenCalled();
+  });
+
+  // The extension's install accepted only a channel in its own project, in one
+  // of the CLI's ALLOWED_EVENT_ARC_REGIONS, even with no event types selected.
+  test.each(["us-central1", "us-west1", "europe-west4", "asia-northeast1"])(
+    "accepts a channel in %s",
+    async (location) => {
+      const name = `projects/p/locations/${location}/channels/c`;
+      await importEvents(name, ALL_EVENTS);
+
+      expect(channel).toHaveBeenCalledWith(name, {
+        allowedEventTypes: ALL_EVENT_TYPES,
+      });
+    }
+  );
+
+  test.each([
+    [
+      "an unsupported location",
+      "projects/p/locations/europe-west1/channels/c",
+      "location 'europe-west1' is not supported",
+    ],
+    [
+      "the global location",
+      "projects/p/locations/global/channels/c",
+      "location 'global' is not supported",
+    ],
+    [
+      "another project",
+      "projects/other/locations/us-central1/channels/c",
+      "it must be in the same project, 'p'",
+    ],
+    [
+      "the project number",
+      "projects/123456/locations/us-central1/channels/c",
+      "it must be in the same project, 'p'",
+    ],
+  ])("rejects a channel in %s", async (_label, channelName, message) => {
+    for (const selected of [ALL_EVENTS, null]) {
+      await expect(importEvents(channelName, selected)).rejects.toThrow(
+        message
+      );
+    }
+    expect(channel).not.toHaveBeenCalled();
+  });
 });
 
 describe("publish failures", () => {

kits/delete-user-data/tests/index.test.ts

@@ -70,11 +70,25 @@ describe("clearData", () => {
   });
 });
 
+// The CLI loads the kit to read its manifest, so a throw at import fails the
+// deploy, as the extension's install failed on the same value.
 describe("module load", () => {
   afterEach(() => {
     vi.unstubAllEnvs();
   });
 
+  test("fails on an invalid EXT_SELECTED_EVENTS", async () => {
+    vi.resetModules();
+    vi.stubEnv(
+      "EXT_SELECTED_EVENTS",
+      "firebase.extensions.delete-user-data.v1.firestore, firebase.extensions.delete-user-data.v1.storage"
+    );
+
+    await expect(import("../src/index")).rejects.toThrow(
+      "EXT_SELECTED_EVENTS is invalid"
+    );
+  });
+
   // Deploy-time discovery imports the kit, so this is what fails the deploy.
   test("throws when a param value is invalid", async () => {
     vi.resetModules();