Add support for secrets in gcfv2 endpoints (#4451)

We now read `secretEnvironmentVariables` property from the endpoint manifest and include the property when creating/updating v1 and v2 functions. Side note - if secretEnvVar in the manifest is missing `secret` property (i.e. doesn't specify the secret resource name), we'll assume that resource name matches the `key` property. This will be the case for all CF3 functions.
firebase · May 3, 2022 · c0f19a3 · c0f19a3
1 parent c8cb86b
commit c0f19a3
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1 @@
+- Add support for secrets to v2 functions (#4451).
diff --git a/src/deploy/functions/release/index.ts b/src/deploy/functions/release/index.ts
@@ -106,7 +106,10 @@ export async function release(
       const projectId = needProjectId(options);
       const projectNumber = await needProjectNumber(options);
       // Re-load backend with all endpoints, not just the ones deployed.
-      const reloadedBackend = await backend.existingBackend({ projectId } as args.Context);
+      const reloadedBackend = await backend.existingBackend(
+        { projectId } as args.Context,
+        /* forceRefresh= */ true
+      );
       const prunedResult = await secrets.pruneAndDestroySecrets(
         { projectId, projectNumber },
         backend.allEndpoints(reloadedBackend)

diff --git a/src/deploy/functions/runtimes/discovery/v1alpha1.ts b/src/deploy/functions/runtimes/discovery/v1alpha1.ts
@@ -244,6 +244,24 @@ function parseEndpoints(
       "environmentVariables",
       "cpu"
     );
+    renameIfPresent(
+      parsed,
+      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,
+            });
+          }
+        }
+      }
+    );
     allParsed.push(parsed);
   }
 

diff --git a/src/deploy/functions/validate.ts b/src/deploy/functions/validate.ts
@@ -136,17 +136,17 @@ export async function secretsAreValid(projectId: string, wantBackend: backend.Ba
   await validateSecretVersions(projectId, endpoints);
 }
 
+const secretsSupportedPlatforms = ["gcfv1", "gcfv2"];
 /**
  * Ensures that all endpoints specifying secret environment variables target platform that supports the feature.
  */
 function validatePlatformTargets(endpoints: backend.Endpoint[]) {
-  const supportedPlatforms = ["gcfv1"];
-  const unsupported = endpoints.filter((e) => !supportedPlatforms.includes(e.platform));
+  const unsupported = endpoints.filter((e) => !secretsSupportedPlatforms.includes(e.platform));
   if (unsupported.length > 0) {
     const errs = unsupported.map((e) => `${e.id}[platform=${e.platform}]`);
     throw new FirebaseError(
       `Tried to set secret environment variables on ${errs.join(", ")}. ` +
-        `Only ${supportedPlatforms.join(", ")} support secret environments.`
+        `Only ${secretsSupportedPlatforms.join(", ")} support secret environments.`
     );
   }
 }

diff --git a/src/gcp/cloudfunctions.ts b/src/gcp/cloudfunctions.ts
@@ -370,12 +370,13 @@ export async function updateFunction(
   cloudFunction: Omit<CloudFunction, OutputOnlyFields>
 ): Promise<Operation> {
   const endpoint = `/${cloudFunction.name}`;
-  // Keys in labels and environmentVariables are user defined, so we don't recurse
+  // Keys in labels and environmentVariables and secretEnvironmentVariables are user defined, so we don't recurse
   // for field masks.
   const fieldMasks = proto.fieldMasks(
     cloudFunction,
     /* doNotRecurseIn...=*/ "labels",
-    "environmentVariables"
+    "environmentVariables",
+    "secretEnvironmentVariables"
   );
 
   // Failure policy is always an explicit policy and is only signified by the presence or absence of

diff --git a/src/gcp/cloudfunctionsv2.ts b/src/gcp/cloudfunctionsv2.ts
@@ -92,6 +92,20 @@ export interface EventFilter {
   value: string;
 }
 
+/**
+ * Configurations for secret environment variables attached to a cloud functions resource.
+ */
+export interface SecretEnvVar {
+  /* Name of the environment variable. */
+  key: string;
+  /* Project identifier (or project number) of the project that contains the secret. */
+  projectId: string;
+  /* Name of the secret in secret manager. e.g. MY_SECRET, NOT projects/abc/secrets/MY_SECRET */
+  secret: string;
+  /* Version of the secret (version number or the string 'latest') */
+  version?: string;
+}
+
 /** The Cloud Run service that underlies a Cloud Function. */
 export interface ServiceConfig {
   // Output only
@@ -104,6 +118,7 @@ export interface ServiceConfig {
   timeoutSeconds?: number;
   availableMemory?: string;
   environmentVariables?: Record<string, string>;
+  secretEnvironmentVariables?: SecretEnvVar[];
   maxInstanceCount?: number;
   minInstanceCount?: number;
   vpcConnector?: string;
@@ -351,12 +366,13 @@ async function listFunctionsInternal(
 export async function updateFunction(
   cloudFunction: Omit<CloudFunction, OutputOnlyFields>
 ): Promise<Operation> {
-  // Keys in labels and environmentVariables are user defined, so we don't recurse
+  // Keys in labels and environmentVariables and secretEnvironmentVariables are user defined, so we don't recurse
   // for field masks.
   const fieldMasks = proto.fieldMasks(
     cloudFunction,
     /* doNotRecurseIn...=*/ "labels",
-    "serviceConfig.environmentVariables"
+    "serviceConfig.environmentVariables",
+    "serviceConfig.secretEnvironmentVariables"
   );
   try {
     const queryParams = {
@@ -422,6 +438,7 @@ export function functionFromEndpoint(endpoint: backend.Endpoint, source: Storage
     gcfFunction.serviceConfig,
     endpoint,
     "environmentVariables",
+    "secretEnvironmentVariables",
     "serviceAccountEmail",
     "ingressSettings",
     "timeoutSeconds"
@@ -586,6 +603,7 @@ export function endpointFromFunction(gcfFunction: CloudFunction): backend.Endpoi
     "serviceAccountEmail",
     "ingressSettings",
     "environmentVariables",
+    "secretEnvironmentVariables",
     "timeoutSeconds"
   );
   proto.renameIfPresent(

diff --git a/src/test/deploy/functions/validate.spec.ts b/src/test/deploy/functions/validate.spec.ts
@@ -420,10 +420,12 @@ describe("validate", () => {
       expect(validate.secretsAreValid(project, b)).to.not.be.rejected;
     });
 
-    it("fails validation given endpoint with secrets targeting unsupported platform", () => {
+    it("fails validation given non-existent secret version", () => {
+      secretVersionStub.rejects({ reason: "Secret version does not exist" });
+
       const b = backend.of({
         ...ENDPOINT,
-        platform: "gcfv2",
+        platform: "gcfv1",
         secretEnvironmentVariables: [
           {
             projectId: project,
@@ -432,8 +434,10 @@ describe("validate", () => {
           },
         ],
       });
-
-      expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(FirebaseError);
+      expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
+        FirebaseError,
+        /Failed to validate secret version/
+      );
     });
 
     it("fails validation given non-existent secret version", () => {
@@ -450,7 +454,10 @@ describe("validate", () => {
           },
         ],
       });
-      expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(FirebaseError);
+      expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
+        FirebaseError,
+        /Failed to validate secret versions/
+      );
     });
 
     it("fails validation given disabled secret version", () => {
@@ -471,7 +478,10 @@ describe("validate", () => {
           },
         ],
       });
-      expect(validate.secretsAreValid(project, b)).to.be.rejected;
+      expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
+        FirebaseError,
+        /Failed to validate secret versions/
+      );
     });
 
     it("passes validation and resolves latest version given valid secret config", async () => {
@@ -481,20 +491,22 @@ describe("validate", () => {
         state: "ENABLED",
       });
 
-      const b = backend.of({
-        ...ENDPOINT,
-        platform: "gcfv1",
-        secretEnvironmentVariables: [
-          {
-            projectId: project,
-            secret: "MY_SECRET",
-            key: "MY_SECRET",
-          },
-        ],
-      });
-
-      await validate.secretsAreValid(project, b);
-      expect(backend.allEndpoints(b)[0].secretEnvironmentVariables![0].version).to.equal("2");
+      for (const platform of ["gcfv1" as const, "gcfv2" as const]) {
+        const b = backend.of({
+          ...ENDPOINT,
+          platform,
+          secretEnvironmentVariables: [
+            {
+              projectId: project,
+              secret: "MY_SECRET",
+              key: "MY_SECRET",
+            },
+          ],
+        });
+
+        await validate.secretsAreValid(project, b);
+        expect(backend.allEndpoints(b)[0].secretEnvironmentVariables![0].version).to.equal("2");
+      }
     });
   });
 });
diff --git a/src/test/gcp/cloudfunctionsv2.spec.ts b/src/test/gcp/cloudfunctionsv2.spec.ts
@@ -211,6 +211,13 @@ describe("cloudfunctionsv2", () => {
         environmentVariables: {
           FOO: "bar",
         },
+        secretEnvironmentVariables: [
+          {
+            secret: "MY_SECRET",
+            key: "MY_SECRET",
+            projectId: "project",
+          },
+        ],
       };
 
       const fullGcfFunction: Omit<
@@ -227,6 +234,13 @@ describe("cloudfunctionsv2", () => {
           environmentVariables: {
             FOO: "bar",
           },
+          secretEnvironmentVariables: [
+            {
+              secret: "MY_SECRET",
+              key: "MY_SECRET",
+              projectId: "project",
+            },
+          ],
           vpcConnector: "connector",
           vpcConnectorEgressSettings: "ALL_TRAFFIC",
           ingressSettings: "ALLOW_ALL",