fix: improve error message when deploying to a gcp project where fire…

CHANGELOG.md

@@ -1,2 +1,7 @@
+- [Fixed] Defer secret access permission granting to release phase to prevent service account 404 race conditions.
+- Fixed parsing and path resolution bugs in `ext:export` options, and reverted `--extension-instance` option back to `--instance`.
+- [Fixed] Increases default polling timeout for App Hosting operations and rollouts to 60 minutes.
+- Fixed an issue where App Hosting deploys failed when the deploying account lacked permission to create or grant roles to the default compute service account, even when that service account already existed. (#10806)
+- Improved the error shown when deploying to a Google Cloud project that does not have Firebase enabled (#10379)
 - [Added] Add -f, --force option to `firebase ext:migrate`.
 - [Fixed] Fix parameter type preservation and optional system parameter handling during extension updates in `firebase ext:migrate`.

src/functionsConfig.spec.ts

@@ -1,6 +1,11 @@
 import { expect } from "chai";
+import nock from "./test/helpers/nock";
 
 import * as functionsConfig from "./functionsConfig";
+import { firebaseApiOrigin } from "./api";
+import { FirebaseError } from "./error";
+
+const FAKE_PROJECT_ID = "my-project";
 
 describe("config.parseSetArgs", () => {
   it("should throw if a reserved namespace is used", () => {
@@ -42,3 +47,53 @@
     ]);
   });
 });
+
+describe("config.getFirebaseConfig", () => {
+  before(() => {
+    nock.disableNetConnect();
+  });
+
+  after(() => {
+    nock.enableNetConnect();
+  });
+
+  afterEach(() => {
+    expect(nock.isDone()).to.be.true;
+  });
+
+  it("should return the admin SDK config on success", async () => {
+    nock(firebaseApiOrigin())
+      .get(`/v1beta1/projects/${FAKE_PROJECT_ID}/adminSdkConfig`)
+      .reply(200, { projectId: FAKE_PROJECT_ID });
+
+    const config = await functionsConfig.getFirebaseConfig({ project: FAKE_PROJECT_ID });
+
+    expect(config).to.deep.eq({ projectId: FAKE_PROJECT_ID });
+  });
+
+  it("should throw a friendly error on 404 that points at projects:addfirebase", async () => {
+    nock(firebaseApiOrigin())
+      .get(`/v1beta1/projects/${FAKE_PROJECT_ID}/adminSdkConfig`)
+      .reply(404, { error: { message: "Requested entity was not found." } });
+
+    const err = await expect(functionsConfig.getFirebaseConfig({ project: FAKE_PROJECT_ID })).to.be
+      .rejected;
+
+    expect(err).to.be.instanceOf(FirebaseError);
+    expect(err.status).to.eq(404);
+    expect(err.message).to.contain(`firebase projects:addfirebase ${FAKE_PROJECT_ID}`);
+    // getFirebaseConfig also backs functions:delete, functions:export and ext:*,
+    // so the message must not be phrased as a deploy failure.
+    expect(err.message).to.not.contain("deploy");
+  });
+
+  it("should rethrow non-404 errors as-is", async () => {
+    nock(firebaseApiOrigin())
+      .get(`/v1beta1/projects/${FAKE_PROJECT_ID}/adminSdkConfig`)
+      .reply(500, { error: { message: "Internal error" } });
+
+    await expect(functionsConfig.getFirebaseConfig({ project: FAKE_PROJECT_ID }))
+      .to.be.rejectedWith(FirebaseError, "Internal error")
+      .and.eventually.have.property("status", 500);
+  });
+});

src/functionsConfig.ts

@@ -39,15 +39,26 @@
   firebase experiments:enable ${LEGACY_RUNTIME_CONFIG_EXPERIMENT}
 `;
 
+// adminSdkConfig 404s for any project this account can't see, so the message
+// must not assert a cause. The addfirebase hint covers the Cloud-project case.
+const projectNotFoundMessage = (projectId: string): string =>
+  `Firebase project ${clc.bold(projectId)} was not found.
+Make sure the project exists and that your account has access to it.
+
+If ${clc.bold(projectId)} is a Google Cloud project without Firebase, add Firebase to it:
+
+  firebase projects:addfirebase ${projectId}
+`;
+
 export function getFunctionsConfigDeprecationMessage(): string {
   return FUNCTIONS_CONFIG_DEPRECATION_MESSAGE;
 }
 
 export function logFunctionsConfigDeprecationWarning(): void {
   logWarningToStderr(FUNCTIONS_CONFIG_DEPRECATION_MESSAGE);
 }
 
 export function ensureLegacyRuntimeConfigCommandsEnabled(): void {
   if (experiments.isEnabled(LEGACY_RUNTIME_CONFIG_EXPERIMENT)) {
     return;
   }
@@ -72,7 +83,7 @@
   configId: string,
   varPath: string,
   val: string | object,
 ): Promise<any> {
   if (configId === "" || varPath === "") {
     const msg = "Invalid argument, each config value must have a 2-part key (e.g. foo.bar).";
     throw new FirebaseError(msg);
@@ -80,13 +91,13 @@
   return runtimeconfig.variables.set(projectId, configId, varPath, val);
 }
 
 function isReservedNamespace(id: Id) {
   return RESERVED_NAMESPACES.some((reserved) => {
     return id.config.toLowerCase().startsWith(reserved);
   });
 }
 
 export async function ensureApi(options: any): Promise<void> {
   const projectId = needProjectId(options);
   return ensureApiEnabled(projectId, "runtimeconfig.googleapis.com", "runtimeconfig", true);
 }
@@ -114,10 +125,21 @@
 
 export async function getFirebaseConfig(options: any): Promise<args.FirebaseConfig> {
   const projectId = needProjectId(options);
-  const response = await apiClient.get<args.FirebaseConfig>(
-    `/v1beta1/projects/${projectId}/adminSdkConfig`,
-  );
-  return response.body;
+  try {
+    const response = await apiClient.get<args.FirebaseConfig>(
+      `/v1beta1/projects/${projectId}/adminSdkConfig`,
+    );
+    return response.body;
+  } catch (err: unknown) {
+    if (err instanceof FirebaseError && err.status === 404) {
+      throw new FirebaseError(projectNotFoundMessage(projectId), {
+        original: err,
+        exit: 1,
+        status: 404,
+      });
+    }
+    throw err;
+  }
 }
 
 // If you make changes to this function, run "node scripts/test-functions-config.js"