Correctly copy over secret environment variables (#4543)

* Correctly copy over secret environment variables * Changelog * Run formatter * Add unit tests
firebase · May 11, 2022 · e8ea6c9 · e8ea6c9
1 parent 002bcc5
commit e8ea6c9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1 @@
+- Fixes bug where secret environment variables were not set on functions (#4543)
diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts
@@ -13,7 +13,14 @@ const CHANNEL_NAME_REGEX = new RegExp(
     "(?<channel>[A-Za-z\\d\\-_]+)"
 );
 
-export type ManifestEndpoint = backend.ServiceConfiguration &
+export interface ManifestSecretEnv {
+  key: string;
+  secret?: string;
+  projectId: string;
+}
+
+type Base = Omit<backend.ServiceConfiguration, "secretEnvironmentVariables">;
+export type ManifestEndpoint = Base &
   backend.Triggered &
   Partial<backend.HttpsTriggered> &
   Partial<backend.CallableTriggered> &
@@ -24,6 +31,7 @@ export type ManifestEndpoint = backend.ServiceConfiguration &
     region?: string[];
     entryPoint: string;
     platform?: backend.FunctionsPlatform;
+    secretEnvironmentVariables?: Array<ManifestSecretEnv>;
   };
 
 export interface Manifest {
@@ -249,17 +257,16 @@ function parseEndpoints(
       ep,
       "secretEnvironmentVariables",
       "secretEnvironmentVariables",
-      (senvs: ManifestEndpoint["secretEnvironmentVariables"]) => {
-        if (senvs && senvs.length > 0) {
-          ep.secretEnvironmentVariables = [];
-          for (const { key, secret } of senvs) {
-            ep.secretEnvironmentVariables.push({
-              key,
-              secret: secret || key, // if secret is undefined, assume env var key == secret name
-              projectId: project,
-            });
-          }
+      (senvs: Array<ManifestSecretEnv>) => {
+        const secretEnvironmentVariables: backend.SecretEnvVar[] = [];
+        for (const { key, secret } of senvs) {
+          secretEnvironmentVariables.push({
+            key,
+            secret: secret || key, // if secret is undefined, assume env var key == secret name
+            projectId: project,
+          });
         }
+        return secretEnvironmentVariables;
       }
     );
     allParsed.push(parsed);

diff --git a/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts b/src/test/deploy/functions/runtimes/discovery/v1alpha1.spec.ts
@@ -294,7 +294,10 @@ describe("backendFromV1Alpha1", () => {
   }); // Parser errors;
 
   describe("allows valid backends", () => {
-    const DEFAULTED_ENDPOINT: Omit<backend.Endpoint, "httpsTrigger"> = {
+    const DEFAULTED_ENDPOINT: Omit<
+      backend.Endpoint,
+      "httpsTrigger" | "secretEnvironmentVariables"
+    > = {
       ...MIN_ENDPOINT,
       platform: "gcfv2",
       id: "id",
@@ -551,6 +554,13 @@ describe("backendFromV1Alpha1", () => {
         },
         ingressSettings: "ALLOW_INTERNAL_ONLY",
         serviceAccountEmail: "sa@",
+        secretEnvironmentVariables: [
+          {
+            key: "SECRET",
+            secret: "SECRET",
+            projectId: "project",
+          },
+        ],
       };
 
       const yaml: v1alpha1.Manifest = {
@@ -560,6 +570,13 @@ describe("backendFromV1Alpha1", () => {
             ...MIN_ENDPOINT,
             httpsTrigger: {},
             ...fields,
+            secretEnvironmentVariables: [
+              {
+                key: "SECRET",
+                // Missing "secret"
+                projectId: "project",
+              },
+            ],
           },
         },
       };