Making onCall Dart functions Public when deployed (#10390)

* Making Http and Callable Dart functions Public when deployed

Author: shettyvarun268

src/deploy/functions/release/fabricator.spec.ts

@@ -910,6 +910,16 @@ describe("Fabricator", () => {
       expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["custom@"]);
     });
 
+    it("sets invoker to private on Node updates when explicitly configured as private", async () => {
+      gcfv2.updateFunction.resolves({ name: "op", done: false });
+      poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
+      run.setInvokerUpdate.resolves();
+      const ep = endpoint({ httpsTrigger: { invoker: ["private"] } }, { platform: "gcfv2" });
+
+      await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
+      expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["private"]);
+    });
+
     it("sets explicit invoker on dataConnectGraphqlTrigger", async () => {
       gcfv2.updateFunction.resolves({ name: "op", done: false });
       poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
@@ -974,6 +984,16 @@ describe("Fabricator", () => {
       expect(run.setInvokerUpdate).to.not.have.been.called;
     });
 
+    it("updates invoker to public on Node updates when explicitly null", async () => {
+      gcfv2.updateFunction.resolves({ name: "op", done: false });
+      poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
+      run.setInvokerUpdate.resolves();
+      const ep = endpoint({ httpsTrigger: { invoker: null } }, { platform: "gcfv2" });
+
+      await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
+      expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["public"]);
+    });
+
     it("doesn't set invoker on non-http functions", async () => {
       gcfv2.updateFunction.resolves({ name: "op", done: false });
       poller.pollOperation.resolves({ serviceConfig: { service: "service" } });
@@ -1767,7 +1787,7 @@ describe("Fabricator", () => {
   describe("createRunFunction", () => {
     it("creates a Cloud Run service with correct configuration", async () => {
       runv2.createService.resolves({ uri: "https://service", name: "service" } as any);
-      run.setInvokerUpdate.resolves();
+      run.setInvokerCreate.resolves();
 
       const ep = endpoint(
         { httpsTrigger: {} },
@@ -1805,6 +1825,44 @@ describe("Fabricator", () => {
         }),
       );
     });
+
+    it("always sets callable triggers to public on creation", async () => {
+      runv2.createService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerCreate.resolves();
+
+      const ep = endpoint(
+        { callableTrigger: {} },
+        {
+          platform: "run",
+          baseImageUri: "gcr.io/base",
+          command: ["cmd"],
+          args: ["arg"],
+        },
+      );
+      await fab.createRunFunction(ep);
+
+      expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, sinon.match.string, [
+        "public",
+      ]);
+    });
+
+    it("does not set invoker on creation when HTTPS configuration is private", async () => {
+      runv2.createService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerCreate.resolves();
+
+      const ep = endpoint(
+        { httpsTrigger: { invoker: ["private"] } },
+        {
+          platform: "run",
+          baseImageUri: "gcr.io/base",
+          command: ["cmd"],
+          args: ["arg"],
+        },
+      );
+      await fab.createRunFunction(ep);
+
+      expect(run.setInvokerCreate).to.not.have.been.called;
+    });
   });
 
   describe("updateRunFunction", () => {
@@ -1839,6 +1897,58 @@ describe("Fabricator", () => {
         }),
       );
     });
+
+    it("does not update invoker for callable functions", async () => {
+      runv2.updateService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerUpdate.resolves();
+
+      const ep = endpoint({ callableTrigger: {} }, { platform: "run" });
+      const update = { endpoint: ep };
+
+      await fab.updateRunFunction(update);
+
+      expect(run.setInvokerUpdate).to.not.have.been.called;
+    });
+
+    it("updates invoker to public for HTTPS functions when explicitly null", async () => {
+      runv2.updateService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerUpdate.resolves();
+
+      const ep = endpoint({ httpsTrigger: { invoker: null } }, { platform: "run" });
+      const update = { endpoint: ep };
+
+      await fab.updateRunFunction(update);
+
+      expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, sinon.match.string, [
+        "public",
+      ]);
+    });
+
+    it("does not update invoker for HTTPS functions when invoker is omitted (undefined)", async () => {
+      runv2.updateService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerUpdate.resolves();
+
+      const ep = endpoint({ httpsTrigger: {} }, { platform: "run" });
+      const update = { endpoint: ep };
+
+      await fab.updateRunFunction(update);
+
+      expect(run.setInvokerUpdate).to.not.have.been.called;
+    });
+
+    it("updates invoker for HTTPS functions to private when explicitly configured as private", async () => {
+      runv2.updateService.resolves({ uri: "https://service", name: "service" } as any);
+      run.setInvokerUpdate.resolves();
+
+      const ep = endpoint({ httpsTrigger: { invoker: ["private"] } }, { platform: "run" });
+      const update = { endpoint: ep };
+
+      await fab.updateRunFunction(update);
+
+      expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, sinon.match.string, [
+        "private",
+      ]);
+    });
   });
 
   describe("deleteRunFunction", () => {

src/deploy/functions/release/fabricator.ts

@@ -662,6 +662,8 @@ export class Fabricator {
       .catch(rethrowAs(endpoint, "delete"));
   }
 
+  // We use createRunFunction and updateRunFunction specifically to deploy Dart functions
+  // directly over to Cloud Run! This avoids cluttering normal Gen 2 Node function hooks.
   async createRunFunction(endpoint: backend.Endpoint): Promise<void> {
     const storageSource = this.sources[endpoint.codebase!]?.storage;
     if (!storageSource) {
@@ -692,7 +694,20 @@ export class Fabricator {
       })
       .catch(rethrowAs(endpoint, "create"));
 
-    await this.setInvoker(endpoint);
+    const serviceName = `projects/${endpoint.project}/locations/${endpoint.region}/services/${endpoint.runServiceId}`;
+    if (backend.isHttpsTriggered(endpoint)) {
+      const invoker = endpoint.httpsTrigger.invoker || ["public"];
+      if (!invoker.includes("private")) {
+        await this.executor
+          .run(() => run.setInvokerCreate(endpoint.project, serviceName, invoker))
+          .catch(rethrowAs(endpoint, "set invoker"));
+      }
+    } else if (backend.isCallableTriggered(endpoint)) {
+      // Callable functions should always be public
+      await this.executor
+        .run(() => run.setInvokerCreate(endpoint.project, serviceName, ["public"]))
+        .catch(rethrowAs(endpoint, "set invoker"));
+    }
   }
 
   async updateRunFunction(update: planner.EndpointUpdate): Promise<void> {
@@ -722,7 +737,19 @@ export class Fabricator {
       })
       .catch(rethrowAs(endpoint, "update"));
 
-    await this.setInvoker(endpoint);
+    const serviceName = `projects/${endpoint.project}/locations/${endpoint.region}/services/${endpoint.runServiceId}`;
+    // We check for null vs undefined to respect settings people make on the Google Console.
+    // If it's omitted (undefined), we don't touch policies. If it is explicitly null, we make it public.
+    let invoker: string[] | undefined;
+    if (backend.isHttpsTriggered(endpoint)) {
+      invoker = endpoint.httpsTrigger.invoker === null ? ["public"] : endpoint.httpsTrigger.invoker;
+    }
+
+    if (invoker) {
+      await this.executor
+        .run(() => run.setInvokerUpdate(endpoint.project, serviceName, invoker!))
+        .catch(rethrowAs(endpoint, "set invoker"));
+    }
   }
 
   async deleteRunFunction(endpoint: backend.Endpoint): Promise<void> {
@@ -740,23 +767,6 @@ export class Fabricator {
       .catch(rethrowAs(endpoint, "delete"));
   }
 
-  async setInvoker(endpoint: backend.Endpoint): Promise<void> {
-    if (backend.isHttpsTriggered(endpoint)) {
-      const invoker = endpoint.httpsTrigger.invoker || ["public"];
-      if (!invoker.includes("private")) {
-        await this.executor
-          .run(() =>
-            run.setInvokerUpdate(
-              endpoint.project,
-              `projects/${endpoint.project}/locations/${endpoint.region}/services/${endpoint.runServiceId}`,
-              invoker,
-            ),
-          )
-          .catch(rethrowAs(endpoint, "set invoker"));
-      }
-    }
-  }
-
   async setRunTraits(serviceName: string, endpoint: backend.Endpoint): Promise<void> {
     await this.functionExecutor
       .run(async () => {