Fix Firebase Function deploy bugs. (#3668)

This is embarrassing. I must have removed the preview check while refactoring the code after PR feedback. Also improving the logging logic to not log anything if no local dotenv files are found. Also addresses bug where wrong `FIREBASE_CONFIG` value was set #3667
firebase · Aug 12, 2021 · 592a4ee · 592a4ee
1 parent b22e912
commit 592a4ee
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 35 deletions.
diff --git a/scripts/emulator-tests/functionsEmulatorRuntime.spec.ts b/scripts/emulator-tests/functionsEmulatorRuntime.spec.ts
@@ -384,7 +384,7 @@ describe("FunctionsEmulator-Runtime", () => {
       }).timeout(TIMEOUT_MED);
     });
 
-    describe.only("environment variables", () => {
+    describe("environment variables", () => {
       before(() => {
         fs.writeFileSync(path.join(MODULE_ROOT, ".env"), "SOURCE=env\nFOO=foo");
         fs.writeFileSync(path.join(MODULE_ROOT, ".env.local"), "SOURCE=env.local");
@@ -471,30 +471,32 @@ describe("FunctionsEmulator-Runtime", () => {
         expect(res.var).to.eql("localhost:9099");
       }).timeout(TIMEOUT_MED);
 
-      it("should inject user environment variables when preview is enabled", async () => {
-        const frb = _.cloneDeep(FunctionRuntimeBundles.onRequest);
-        frb.emulators = {
-          auth: {
-            host: "localhost",
-            port: 9099,
-          },
-        };
-
-        const worker = invokeRuntimeWithFunctions(frb, () => {
-          return {
-            function_id: require("firebase-functions").https.onRequest((req: any, res: any) => {
-              res.json({
-                SOURCE: process.env.SOURCE,
-                FOO: process.env.FOO,
-              });
-            }),
-          };
-        });
-
-        const data = await callHTTPSFunction(worker, frb);
-        const res = JSON.parse(data);
-        expect(res).to.deep.equal({ SOURCE: "env.local", FOO: "foo" });
-      }).timeout(TIMEOUT_MED);
+      // TODO(danielylee): Therer isn't a good way to temporarily enable previews on functions runtime
+      // because it runs on a separate process. Re-enable this test once the preview is done.
+      // it("should inject user environment variables when preview is enabled", async () => {
+      //   const frb = _.cloneDeep(FunctionRuntimeBundles.onRequest);
+      //   frb.emulators = {
+      //     auth: {
+      //       host: "localhost",
+      //       port: 9099,
+      //     },
+      //   };
+
+      //   const worker = invokeRuntimeWithFunctions(frb, () => {
+      //     return {
+      //       function_id: require("firebase-functions").https.onRequest((req: any, res: any) => {
+      //         res.json({
+      //           SOURCE: process.env.SOURCE,
+      //           FOO: process.env.FOO,
+      //         });
+      //       }),
+      //     };
+      //   });
+
+      //   const data = await callHTTPSFunction(worker, frb);
+      //   const res = JSON.parse(data);
+      //   expect(res).to.deep.equal({ SOURCE: "env.local", FOO: "foo" });
+      // }).timeout(TIMEOUT_MED);
     });
   });
 

diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts
@@ -57,7 +57,7 @@ export async function prepare(
     "Error: 'functions.source' is not defined"
   );
   const source = options.config.src.functions.source;
-  const firebaseEnvs = functionsEnv.loadFirebaseEnvs(runtimeConfig, projectId);
+  const firebaseEnvs = functionsEnv.loadFirebaseEnvs(firebaseConfig, projectId);
   const userEnvs = functionsEnv.loadUserEnvs({
     functionsSource: options.config.path(source),
     projectId: projectId,

diff --git a/src/deploy/functions/prepareFunctionsUpload.ts b/src/deploy/functions/prepareFunctionsUpload.ts
@@ -47,15 +47,6 @@ export async function getFunctionsConfig(context: args.Context): Promise<{ [key:
   return config;
 }
 
-// TODO(inlined): move to a file that's not about uploading source code
-export async function getEnvs(context: args.Context): Promise<{ [key: string]: string }> {
-  const envs = {
-    FIREBASE_CONFIG: JSON.stringify(context.firebaseConfig),
-    GCLOUD_PROJECT: context.projectId,
-  };
-  return Promise.resolve(envs);
-}
-
 async function pipeAsync(from: archiver.Archiver, to: fs.WriteStream) {
   return new Promise((resolve, reject) => {
     to.on("finish", resolve);

diff --git a/src/functions/env.ts b/src/functions/env.ts
@@ -228,7 +228,14 @@ export function loadUserEnvs({
   projectId: string;
   projectAlias?: string;
 }): Record<string, string> {
+  if (!previews.dotenv) {
+    return {};
+  }
+
   const envFiles = findEnvfiles(functionsSource, projectId, projectAlias);
+  if (envFiles.length == 0) {
+    return {};
+  }
 
   // Disallow setting both .env.<projectId> and .env.<projectAlias>
   if (projectAlias) {

diff --git a/src/test/functions/env.spec.ts b/src/test/functions/env.spec.ts
@@ -5,6 +5,7 @@ import { sync as rimraf } from "rimraf";
 import { expect } from "chai";
 
 import * as env from "../../functions/env";
+import { previews } from "../../previews";
 
 describe("functions/env", () => {
   describe("parse", () => {
@@ -210,6 +211,14 @@ FOO=foo
     const projectInfo = { projectId: "my-project", projectAlias: "dev" };
     let tmpdir: string;
 
+    before(() => {
+      previews.dotenv = true;
+    });
+
+    after(() => {
+      previews.dotenv = false;
+    });
+
     beforeEach(() => {
       tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), "test"));
     });