Trigger parser can create callable triggers

CHANGELOG.md

@@ -0,0 +1 @@
+- Fixes bug where deploying callable functions failed (#4310).

src/deploy/functions/runtimes/node/parseTriggers.ts

@@ -195,12 +195,17 @@ export function addResourcesToBackend(
         reason: "Needed for task queue functions.",
       });
     } else if (annotation.httpsTrigger) {
-      const trigger: backend.HttpsTrigger = {};
-      if (annotation.failurePolicy) {
-        logger.warn(`Ignoring retry policy for HTTPS function ${annotation.name}`);
+      if (annotation.labels?.["deployment-callable"]) {
+        delete annotation.labels["deployment-callable"];
+        triggered = { callableTrigger: {} };
+      } else {
+        const trigger: backend.HttpsTrigger = {};
+        if (annotation.failurePolicy) {
+          logger.warn(`Ignoring retry policy for HTTPS function ${annotation.name}`);
+        }
+        proto.copyIfPresent(trigger, annotation.httpsTrigger, "invoker");
+        triggered = { httpsTrigger: trigger };
       }
-      proto.copyIfPresent(trigger, annotation.httpsTrigger, "invoker");
-      triggered = { httpsTrigger: trigger };
     } else if (annotation.schedule) {
       want.requiredAPIs.push({
         api: "cloudscheduler.googleapis.com",

src/gcp/cloudfunctions.ts

@@ -474,7 +474,18 @@ export function endpointFromFunction(gcfFunction: CloudFunction): backend.Endpoi
     trigger = {
       taskQueueTrigger: {},
     };
-  } else if (gcfFunction.labels?.["deployment-callable"]) {
+  } else if (
+    gcfFunction.labels?.["deployment-callable"] ||
+    // NOTE: "deployment-callabled" is a typo we introduced in https://github.com/firebase/firebase-tools/pull/4124.
+    // More than a month passed before we caught this typo, and we expect many callable functions in production
+    // to have this typo. It is convenient for users for us to treat the typo-ed label as a valid marker for callable
+    // function, so we do that here.
+    //
+    // The typo will be overwritten as callable functions are re-deployed. Eventually, there may be no callable
+    // functions with the typo-ed label, but we can't ever be sure. Sadly, we may have to carry this scar for a very long
+    // time.
+    gcfFunction.labels?.["deployment-callabled"]
+  ) {
     trigger = {
       callableTrigger: {},
     };
@@ -600,7 +611,7 @@ export function functionFromEndpoint(
   } else {
     gcfFunction.httpsTrigger = {};
     if (backend.isCallableTriggered(endpoint)) {
-      gcfFunction.labels = { ...gcfFunction.labels, "deployment-callabled": "true" };
+      gcfFunction.labels = { ...gcfFunction.labels, "deployment-callable": "true" };
     }
     if (endpoint.securityLevel) {
       gcfFunction.httpsTrigger.securityLevel = endpoint.securityLevel;

src/test/deploy/functions/runtimes/node/parseTriggers.spec.ts

@@ -65,6 +65,26 @@ describe("addResourcesToBackend", () => {
     expect(result).to.deep.equal(expected);
   });
 
+  it("should handle a callable trigger", () => {
+    const trigger: parseTriggers.TriggerAnnotation = {
+      ...BASIC_TRIGGER,
+      httpsTrigger: {},
+      labels: {
+        "deployment-callable": "true",
+      },
+    };
+
+    const result = backend.empty();
+    parseTriggers.addResourcesToBackend("project", "nodejs16", trigger, result);
+
+    const expected: backend.Backend = backend.of({
+      ...BASIC_ENDPOINT,
+      callableTrigger: {},
+      labels: {},
+    });
+    expect(result).to.deep.equal(expected);
+  });
+
   it("should handle a minimal task queue trigger", () => {
     const trigger: parseTriggers.TriggerAnnotation = {
       ...BASIC_TRIGGER,