Improve FAH create & delete

src/commands/apphosting-backends-delete.ts

@@ -5,10 +5,11 @@
 import { promptOnce } from "../prompt";
 import { logger } from "../logger";
 import { DEFAULT_REGION } from "../init/features/apphosting/constants";
+import { last } from "../utils";
 import * as utils from "../utils";
 import * as apphosting from "../gcp/apphosting";
 
 const Table = require("cli-table");
 
 const COLUMN_LENGTH = 20;
 const TABLE_HEAD = [
@@ -29,16 +30,13 @@
   .action(async (options: Options) => {
     const projectId = needProjectId(options);
     let location = options.location as string;
-    const backendId = options.backend as string;
-    if (!backendId) {
-      throw new FirebaseError("Backend id can't be empty.");
-    }
+    let backendId = options.backend as string;
 
     if (!location) {
       const allowedLocations = (await apphosting.listLocations(projectId)).map(
         (loc) => loc.locationId,
       );
       location = await promptOnce({
         name: "region",
         type: "list",
         default: DEFAULT_REGION,
@@ -47,24 +45,30 @@
       });
     }
 
+    let backend: apphosting.Backend;
+    if (backendId) {
+      try {
+        backend = await apphosting.getBackend(projectId, location, backendId);
+      } catch (err: any) {
+        throw new FirebaseError(`No backends found with given parameters. Command aborted.`, {
+          original: err,
+        });
+      }
+    } else {
+      backend = await pickBackend(projectId, location);
+      backendId = last(backend.name.split("/"));
+    }
+
     const table = new Table({
       head: TABLE_HEAD,
       style: { head: ["green"] },
     });
     table.colWidths = COLUMN_LENGTH;
 
-    let backend;
-    try {
-      backend = await apphosting.getBackend(projectId, location, backendId);
-      populateTable(backend, table);
-    } catch (err: any) {
-      throw new FirebaseError(`No backends found with given parameters. Command aborted.`, {
-        original: err,
-      });
-    }
+    populateTable(backend, table);
 
     utils.logWarning("You are about to permanently delete the backend:");
     logger.info(table.toString());
 
     const confirmDeletion = await promptOnce(
       {
@@ -112,3 +116,21 @@
   });
   table.push(newRow);
 }
+
+async function pickBackend(projectId: string, location: string): Promise<apphosting.Backend> {
+  const backendList = await apphosting.listBackends(projectId, location);
+  if (!backendList.backends.length) {
+    throw new FirebaseError(`No backends found in location ${location}`);
+  }
+  if (backendList.backends.length === 1) {
+    return backendList.backends[0];
+  }
+  const backendIds = backendList.backends.map((backend) => last(backend.name.split("/")));
+  const backendId = await promptOnce({
+    name: "backend",
+    type: "list",
+    message: "Please select the backend you'd like to delete:",
+    choices: backendIds,
+  });
+  return backendList.backends.find((backend) => last(backend.name.split("/")) === backendId)!;
+}

src/gcp/apphosting.ts

@@ -67,7 +67,7 @@ export type BuildOutputOnlyFields =
   | "error"
   | "image"
   | "sourceRef"
-  | "buildLogUri"
+  | "buildLogsUri"
   | "reconciling"
   | "uuid"
   | "etag"
@@ -196,10 +196,20 @@ export interface RolloutPolicy {
   // end oneof trigger
   stages?: RolloutStage[];
   disabled?: boolean;
+
+  // TODO: This will be undefined if disabled is not true, right?
   disabledTime: string;
 }
 
-export type RolloutPolicyOutputOnlyFields = "disabledtime";
+export type RolloutPolicyOutputOnlyFields = "disabledTime";
+
+export type RolloutPolicyInput = Omit<RolloutPolicy, RolloutPolicyOutputOnlyFields | "stages"> & {
+  stages: Omit<RolloutStage, "startTime" | "endTime">[];
+};
+
+export type TrafficInput = Omit<Traffic, TrafficOutputOnlyFields | "rolloutPolicy"> & {
+  rolloutPolicy: RolloutPolicyInput;
+};
 
 export type RolloutProgression =
   | "PROGRESSION_UNSPECIFIED"
@@ -297,8 +307,8 @@ export async function deleteBackend(
   location: string,
   backendId: string,
 ): Promise<Operation> {
-  const name = `projects/${projectId}/locations/${location}/backends/${backendId}?force=true`;
-  const res = await client.delete<Operation>(name);
+  const name = `projects/${projectId}/locations/${location}/backends/${backendId}`;
+  const res = await client.delete<Operation>(name, { queryParams: { force: "true" } });
 
   return res.body;
 }
@@ -374,14 +384,21 @@ export async function updateTraffic(
   projectId: string,
   location: string,
   backendId: string,
-  traffic: Omit<Traffic, TrafficOutputOnlyFields | "name">,
+  traffic: Omit<TrafficInput, "name">,
 ): Promise<Operation> {
-  const fieldMasks = proto.fieldMasks(traffic);
+  // BUG(b/322891558): setting deep fields on rolloutPolicy doesn't work for some
+  // reason. Create a copy without deep fields to force the updateMask to be
+  // correct.
+  const trafficCopy = { ...traffic };
+  if ("rolloutPolicy" in traffic) {
+    trafficCopy.rolloutPolicy = {} as any;
+  }
+  const fieldMasks = proto.fieldMasks(trafficCopy);
   const queryParams = {
     updateMask: fieldMasks.join(","),
   };
   const name = `projects/${projectId}/locations/${location}/backends/${backendId}/traffic`;
-  const res = await client.patch<Omit<Traffic, TrafficOutputOnlyFields>, Operation>(
+  const res = await client.patch<TrafficInput, Operation>(
     name,
     { ...traffic, name },
     {

src/init/features/apphosting/index.ts

@@ -17,6 +17,7 @@ import { FirebaseError } from "../../../error";
 import { promptOnce } from "../../../prompt";
 import { DEFAULT_REGION } from "./constants";
 import { ensure } from "../../../ensureApiEnabled";
+import * as deploymentTool from "../../../deploymentTool";
 
 const apphostingPollerOptions: Omit<poller.OperationPollerOptions, "operationResourceName"> = {
   apiOrigin: apphostingOrigin,
@@ -76,6 +77,33 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {
 
   const backend = await onboardBackend(projectId, location, backendId);
 
+  // TODO: Once tag patterns are implemented, prompt which method the user
+  // prefers. We could reduce the nubmer of questions asked by letting people
+  // enter tag:<pattern>?
+  const branch = await promptOnce({
+    name: "branch",
+    type: "input",
+    default: "main",
+    message: "Pick a branch for continuous deployment",
+  });
+  const traffic: Omit<apphosting.TrafficInput, "name"> = {
+    rolloutPolicy: {
+      codebaseBranch: branch,
+      stages: [
+        {
+          progression: "IMMEDIATE",
+          targetPercent: 100,
+        },
+      ],
+    },
+  };
+  const op = await apphosting.updateTraffic(projectId, location, backendId, traffic);
+  await poller.pollOperation<apphosting.Traffic>({
+    ...apphostingPollerOptions,
+    pollerName: `updateTraffic-${projectId}-${location}-${backendId}`,
+    operationResourceName: op.name,
+  });
+
   const confirmRollout = await promptOnce({
     type: "confirm",
     name: "rollout",
@@ -89,13 +117,6 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {
     return;
   }
 
-  const branch = await promptOnce({
-    name: "branch",
-    type: "input",
-    default: "main",
-    message: "Which branch do you want to deploy?",
-  });
-
   const { build } = await onboardRollout(projectId, location, backendId, {
     source: {
       codebase: {
@@ -105,6 +126,12 @@ export async function doSetup(setup: any, projectId: string): Promise<void> {
   });
 
   if (build.state !== "READY") {
+    if (!build.buildLogsUri) {
+      throw new FirebaseError(
+        "Failed to build your app, but failed to get build logs as well. " +
+          "This is an internal error and should be reported",
+      );
+    }
     throw new FirebaseError(
       `Failed to build your app. Please inspect the build logs at ${build.buildLogsUri}.`,
       { children: [build.error] },
@@ -137,7 +164,7 @@ function toBackend(cloudBuildConnRepo: Repository): Omit<Backend, BackendOutputO
       repository: `${cloudBuildConnRepo.name}`,
       rootDirectory: "/",
     },
-    labels: {},
+    labels: deploymentTool.labels(),
   };
 }
 

src/test/init/apphosting/index.spec.ts

@@ -7,6 +7,7 @@ import * as poller from "../../../operation-poller";
 import * as prompt from "../../../prompt";
 import { createBackend, onboardBackend } from "../../../init/features/apphosting/index";
 import { FirebaseError } from "../../../error";
+import * as deploymentTool from "../../../deploymentTool";
 
 describe("operationsConverter", () => {
   const sandbox: sinon.SinonSandbox = sinon.createSandbox();
@@ -66,10 +67,10 @@ describe("operationsConverter", () => {
         repository: cloudBuildConnRepo.name,
         rootDirectory: "/",
       },
-      labels: {},
+      labels: deploymentTool.labels(),
     };
 
-    it("should createBackend", async () => {
+    it("should createBackend with rollout policy", async () => {
       createBackendStub.resolves(op);
       pollOperationStub.resolves(completeBackend);