Re-guard AR behind an experiment (#3935)

* Re-guard AR behind an experiment

* Changelog

CHANGELOG.md

@@ -2,3 +2,4 @@
 - Fixes issue where providing the `--project` flag during `init` would not be recognized with a default project already set. (#3870)
 - Fixes issue with setting memory limits for some functions (#3924)
 - New HTTPS functions only allow secure traffic. (#3923)
+- No longer default-enable AR and don't send builds to AR unless an experiment is enabled (#3935)

src/deploy/functions/args.ts

@@ -21,6 +21,7 @@ export interface Context {
   functionsSourceV1?: string;
   functionsSourceV2?: string;
   runtimeConfigEnabled?: boolean;
+  artifactRegistryEnabled?: boolean;
   firebaseConfig?: FirebaseConfig;
 
   // Filled in the "deploy" phase.

src/deploy/functions/containerCleaner.ts

@@ -170,6 +170,17 @@ export class ArtifactRegistryCleaner {
   }
 }
 
+// Temporary class to turn off AR cleaning if AR isn't enabled yet
+export class NoopArtifactRegistryCleaner extends ArtifactRegistryCleaner {
+  cleanupFunction(): Promise<void> {
+    return Promise.resolve();
+  }
+
+  cleanupFunctionCache(): Promise<void> {
+    return Promise.resolve();
+  }
+}
+
 export class ContainerRegistryCleaner {
   readonly helpers: Record<string, DockerHelper> = {};
 

src/deploy/functions/prepare.ts

@@ -20,6 +20,7 @@ import * as utils from "../../utils";
 import { logger } from "../../logger";
 import { ensureTriggerRegions } from "./triggerRegionHelper";
 import { ensureServiceAgentRoles } from "./checkIam";
+import e from "express";
 
 function hasUserConfig(config: Record<string, unknown>): boolean {
   // "firebase" key is always going to exist in runtime config.
@@ -31,6 +32,22 @@ function hasDotenv(opts: functionsEnv.UserEnvsOpts): boolean {
   return previews.dotenv && functionsEnv.hasUserEnvs(opts);
 }
 
+// We previously force-enabled AR. We want to wait on this to see if we can give
+// an upgrade warning in the future. If it already is enabled though we want to
+// remember this and still use the cleaner if necessary.
+async function maybeEnableAR(projectId: string): Promise<boolean> {
+  if (previews.artifactregistry) {
+    return ensureApiEnabled.check(
+      projectId,
+      "artifactregistry.googleapis.com",
+      "functions",
+      /* silent= */ true
+    );
+  }
+  await ensureApiEnabled.ensure(projectId, "artifactregistry.googleapis.com", "functions");
+  return true;
+}
+
 export async function prepare(
   context: args.Context,
   options: Options,
@@ -58,9 +75,10 @@ export async function prepare(
       /* silent=*/ true
     ),
     ensureCloudBuildEnabled(projectId),
-    ensureApiEnabled.ensure(projectId, "artifactregistry.googleapis.com", "functions"),
+    maybeEnableAR(projectId),
   ]);
   context.runtimeConfigEnabled = checkAPIsEnabled[1];
+  context.artifactRegistryEnabled = checkAPIsEnabled[3];
 
   // Get the Firebase Config, and set it on each function in the deployment.
   const firebaseConfig = await functionsConfig.getFirebaseConfig(options);

src/gcp/cloudfunctions.ts

@@ -205,8 +205,12 @@ export async function createFunction(
   const endpoint = `/${API_VERSION}/${apiPath}`;
 
   try {
+    const headers: Record<string, string> = {};
+    if (previews.artifactregistry) {
+      headers["X-Firebase-Artifact-Registry"] = "optin";
+    }
     const res = await api.request("POST", endpoint, {
-      headers: { "X-Firebase-Artifact-Registry": "optin" },
+      headers,
       auth: true,
       data: cloudFunction,
       origin: api.functionsOrigin,
@@ -370,8 +374,12 @@ export async function updateFunction(
   // Failure policy is always an explicit policy and is only signified by the presence or absence of
   // a protobuf.Empty value, so we have to manually add it in the missing case.
   try {
+    const headers: Record<string, string> = {};
+    if (previews.artifactregistry) {
+      headers["X-Firebase-Artifact-Registry"] = "optin";
+    }
     const res = await api.request("PATCH", endpoint, {
-      headers: { "X-Firebase-Artifact-Registry": "optin" },
+      headers,
       qs: {
         updateMask: fieldMasks.join(","),
       },

src/previews.ts

@@ -10,6 +10,7 @@ interface PreviewFlags {
   golang: boolean;
   deletegcfartifacts: boolean;
   dotenv: boolean;
+  artifactregistry: boolean;
 }
 
 export const previews: PreviewFlags = {
@@ -22,8 +23,9 @@ export const previews: PreviewFlags = {
   golang: false,
   deletegcfartifacts: false,
   dotenv: false,
+  artifactregistry: false,
 
-  ...configstore.get("previews"),
+  ...(configstore.get("previews") as Partial<PreviewFlags>),
 };
 
 if (process.env.FIREBASE_CLI_PREVIEWS) {