diff --git a/CHANGELOG.md b/CHANGELOG.md index 09dca487cd2..f3bb2b35239 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/deploy/functions/args.ts b/src/deploy/functions/args.ts index b028e5e62db..6c52fffe86d 100644 --- a/src/deploy/functions/args.ts +++ b/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. diff --git a/src/deploy/functions/containerCleaner.ts b/src/deploy/functions/containerCleaner.ts index 76f90873f92..85d52c19f87 100644 --- a/src/deploy/functions/containerCleaner.ts +++ b/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 { + return Promise.resolve(); + } + + cleanupFunctionCache(): Promise { + return Promise.resolve(); + } +} + export class ContainerRegistryCleaner { readonly helpers: Record = {}; diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index 3d7a846d498..345a01bae97 100644 --- a/src/deploy/functions/prepare.ts +++ b/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): 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 { + 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); diff --git a/src/gcp/cloudfunctions.ts b/src/gcp/cloudfunctions.ts index bbdf4044022..e1fe8ecb6c4 100644 --- a/src/gcp/cloudfunctions.ts +++ b/src/gcp/cloudfunctions.ts @@ -205,8 +205,12 @@ export async function createFunction( const endpoint = `/${API_VERSION}/${apiPath}`; try { + const headers: Record = {}; + 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 = {}; + 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(","), }, diff --git a/src/previews.ts b/src/previews.ts index 2ab4575bccc..f135f667665 100644 --- a/src/previews.ts +++ b/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), }; if (process.env.FIREBASE_CLI_PREVIEWS) {