Make it more explicitly clear that we skip local builds if the apphostinglocalbuilds flag is not enabled.

Also make the createTarArchive implementation closer to the one from the zip-deploy branch

Author: falahat

src/deploy/apphosting/deploy.spec.ts

@@ -56,6 +56,7 @@ describe("apphosting", () => {
   let createReadStreamStub: sinon.SinonStub;
   let getProjectNumberStub: sinon.SinonStub;
   let isEnabledStub: sinon.SinonStub;
+  let assertEnabledStub: sinon.SinonStub;
 
   beforeEach(() => {
     getProjectNumberStub = sinon
@@ -71,6 +72,11 @@ describe("apphosting", () => {
       .stub(fs, "createReadStream")
       .throws("Unexpected createReadStream call");
     isEnabledStub = sinon.stub(experiments, "isEnabled").returns(false);
+    assertEnabledStub = sinon.stub(experiments, "assertEnabled").callsFake((name, task) => {
+      if (!experiments.isEnabled(name)) {
+        throw new Error(`Cannot ${task} because the experiment ${name} is not enabled.`);
+      }
+    });
   });
 
   afterEach(() => {
@@ -107,105 +113,45 @@ describe("apphosting", () => {
       getProjectNumberStub.resolves(projectNumber);
       upsertBucketStub.resolves(bucketName);
       createArchiveStub.onFirstCall().resolves("path/to/foo-1234.zip");
-      createArchiveStub.onSecondCall().resolves("path/to/foo-local-build-1234.zip");
+      createTarArchiveStub.onFirstCall().resolves("path/to/foo-local-build-1234.tar.gz");
 
       uploadObjectStub.onFirstCall().resolves({
         bucket: bucketName,
         object: "foo-1234",
       });
       uploadObjectStub.onSecondCall().resolves({
         bucket: bucketName,
-        object: "foo-local-build-1234",
+        object: "foo-local-build-1234.tar.gz",
       });
 
       createReadStreamStub.returns("stream" as any);
+      // For standard source deploy, experiment is not required.
+      // But fooLocalBuild will trigger assertEnabled.
+      isEnabledStub.withArgs("apphostinglocalbuilds").returns(true);
 
       await deploy(context, opts);
 
-      // assert backend foo calls
-
-      expect(upsertBucketStub).to.be.calledWith({
-        product: "apphosting",
-        createMessage: `Creating Cloud Storage bucket in ${location} to store App Hosting source code uploads at ${bucketName}...`,
-        projectId: "my-project",
-        req: {
-          baseName: bucketName,
-          purposeLabel: `apphosting-source-${location}`,
-          location: location,
-          lifecycle: {
-            rule: [
-              {
-                action: { type: "Delete" },
-                condition: { age: 30 },
-              },
-            ],
-          },
-        },
-      });
-
-      // assert backend fooLocalBuild calls
-      expect(upsertBucketStub).to.be.calledWith({
-        product: "apphosting",
-        createMessage:
-          "Creating Cloud Storage bucket in us-central1 to store App Hosting source code uploads at firebaseapphosting-sources-000000000000-us-central1...",
-        projectId: "my-project",
-        req: {
-          baseName: "firebaseapphosting-sources-000000000000-us-central1",
-          purposeLabel: `apphosting-source-${location}`,
-          location: "us-central1",
-          lifecycle: {
-            rule: [
-              {
-                action: { type: "Delete" },
-                condition: { age: 30 },
-              },
-            ],
-          },
-        },
-      });
-      expect(createArchiveStub).to.be.calledWithExactly(
-        context.backendConfigs["fooLocalBuild"],
-        process.cwd(),
-        "./nextjs/standalone",
-      );
-      expect(uploadObjectStub).to.be.calledWithMatch(
-        sinon.match.any,
-        "firebaseapphosting-sources-000000000000-us-central1",
-      );
+      expect(upsertBucketStub).to.be.calledWithMatch({ projectId: "my-project" });
     });
 
-    it("correctly creates and sets storage URIs", async () => {
+    it("fails for local builds when experiment is disabled", async () => {
       const context = initializeContext();
-      const projectNumber = "000000000000";
-      const location = "us-central1";
-      const bucketName = `firebaseapphosting-sources-${projectNumber}-${location}`;
-      getProjectNumberStub.resolves(projectNumber);
-      upsertBucketStub.resolves(bucketName);
-      createArchiveStub.onFirstCall().resolves("path/to/foo-1234.zip");
-      createArchiveStub.onSecondCall().resolves("path/to/foo-local-build-1234.zip");
-
-      uploadObjectStub.onFirstCall().resolves({
-        bucket: bucketName,
-        object: "foo-1234",
-      });
-
-      uploadObjectStub.onSecondCall().resolves({
-        bucket: bucketName,
-        object: "foo-local-build-1234",
-      });
-      createReadStreamStub.returns("stream" as any);
+      delete context.backendConfigs.foo;
+      delete context.backendLocations.foo;
+      
+      getProjectNumberStub.resolves("000000");
+      upsertBucketStub.resolves("bucket");
+      isEnabledStub.withArgs("apphostinglocalbuilds").returns(false);
 
-      await deploy(context, opts);
+      const localOpts = { ...opts, config: new Config({ apphosting: { backendId: "fooLocalBuild", localBuild: true } }) };
+      await deploy(context, localOpts);
 
-      expect(context.backendStorageUris["foo"]).to.equal(`gs://${bucketName}/foo-1234.zip`);
-      expect(context.backendStorageUris["fooLocalBuild"]).to.equal(
-        `gs://${bucketName}/foo-local-build-1234.zip`,
-      );
+      expect(createTarArchiveStub).to.not.be.called;
+      expect(createArchiveStub).to.not.be.called;
     });
 
     it("uses createTarArchive for local builds when experiment is enabled", async () => {
       const context = initializeContext();
-      // Remove the non-local build backend for this test for simplicity
       delete context.backendConfigs.foo;
       delete context.backendLocations.foo;
       const projectNumber = "000000000000";
@@ -222,7 +168,8 @@ describe("apphosting", () => {
       });
       createReadStreamStub.returns("stream" as any);
 
-      await deploy(context, { ...opts, config: new Config({ apphosting: { backendId: "fooLocalBuild", localBuild: true } }) });
+      const localOpts = { ...opts, config: new Config({ apphosting: { backendId: "fooLocalBuild", localBuild: true } }) };
+      await deploy(context, localOpts);
 
       expect(createTarArchiveStub).to.be.calledOnce;
       expect(createArchiveStub).to.not.be.called;

src/deploy/apphosting/deploy.ts

@@ -5,7 +5,7 @@ import * as gcs from "../../gcp/storage";
 import { getProjectNumber } from "../../getProjectNumber";
 import { Options } from "../../options";
 import { needProjectId } from "../../projectUtils";
-import { logLabeledBullet } from "../../utils";
+import { logLabeledBullet, logLabeledWarning } from "../../utils";
 import { Context } from "./args";
 import { createArchive, createTarArchive } from "./util";
 import { isEnabled } from "../../experiments";
@@ -67,6 +67,13 @@ export default async function (context: Context, options: Options): Promise<void
       const rootDir = options.projectRoot ?? process.cwd();
       let builtAppDir;
       if (cfg.localBuild) {
+        if (!isEnabled("apphostinglocalbuilds")) {
+          logLabeledWarning(
+            "apphosting",
+            `Skipping local build for backend ${cfg.backendId} because the experiment apphostinglocalbuilds is not enabled.`,
+          );
+          return;
+        }
         builtAppDir = context.backendLocalBuilds[cfg.backendId].buildDir;
         if (!builtAppDir) {
           throw new FirebaseError(`No local build dir found for ${cfg.backendId}`);

src/deploy/apphosting/release.spec.ts

@@ -3,12 +3,10 @@ import * as rollout from "../../apphosting/rollout";
 import * as backend from "../../apphosting/backend";
 import { Config } from "../../config";
 import { RC } from "../../rc";
-import { needProjectId } from "../../projectUtils";
 import { Context } from "./args";
 import release from "./release";
 import { expect } from "chai";
 import * as experiments from "../../experiments";
-import { isEnabled } from "../../experiments";
 
 const BASE_OPTS = {
   cwd: "/",

src/deploy/apphosting/release.ts

@@ -31,13 +31,11 @@ export default async function (context: Context, options: Options): Promise<void
     backendIds = backendIds.filter((id) => !missingBackends.includes(id));
   }
 
-  const localBuildBackends = backendIds.filter(
-    (id) => context.backendLocalBuilds[id] && !isEnabled("apphostinglocalbuilds"),
-  );
-  if (localBuildBackends.length > 0) {
+  const localBuildBackends = backendIds.filter((id) => context.backendLocalBuilds[id]);
+  if (localBuildBackends.length > 0 && !isEnabled("apphostinglocalbuilds")) {
     logLabeledWarning(
       "apphosting",
-      `Skipping backend(s) ${localBuildBackends.join(", ")}. Local Builds are not supported yet.`,
+      `Skipping local build backend(s) ${localBuildBackends.join(", ")} because the experiment apphostinglocalbuilds is not enabled.`,
     );
     backendIds = backendIds.filter((id) => !localBuildBackends.includes(id));
   }
@@ -49,7 +47,7 @@ export default async function (context: Context, options: Options): Promise<void
   const projectId = needProjectId(options);
   const rollouts = backendIds.map((backendId) => {
     const cfg = context.backendConfigs[backendId];
-    const isLocalBuild = cfg.localBuild && isEnabled("apphostinglocalbuilds");
+    const isLocalBuild = cfg.localBuild;
     let source: any;
     if (isLocalBuild) {
       source = {

src/deploy/apphosting/util.ts

@@ -1,10 +1,12 @@
 import * as archiver from "archiver";
 import * as fs from "fs";
 import * as path from "path";
+import * as tar from "tar";
 import * as tmp from "tmp";
 import { FirebaseError } from "../../error";
 import { AppHostingSingle } from "../../firebaseConfig";
 import * as fsAsync from "../../fsAsync";
+import { APPHOSTING_YAML_FILE_REGEX } from "../../apphosting/config";
 
 /**
  * Locates the source code for a backend and creates an archive to eventually upload to GCS.
@@ -52,56 +54,71 @@ export async function createArchive(
 }
 
 /**
- * Locates the source code for a backend and creates a tar archive to eventually upload to GCS.
+ * Locates the built artifacts for a backend and creates a tar archive to eventually upload to GCS.
  */
 export async function createTarArchive(
   config: AppHostingSingle,
   rootDir: string,
   targetSubDir?: string,
 ): Promise<string> {
   const tmpFile = tmp.fileSync({ prefix: `${config.backendId}-`, postfix: ".tar.gz" }).name;
-  const fileStream = fs.createWriteStream(tmpFile, {
-    flags: "w",
-    encoding: "binary",
-  });
-  const archive = archiver("tar", {
-    gzip: true,
-    gzipOptions: {
-      level: 9,
-    },
-  });
 
   const targetDir = targetSubDir ? path.join(rootDir, targetSubDir) : rootDir;
-  const ignore = config.ignore || ["node_modules", ".git"];
-  ignore.push("firebase-debug.log", "firebase-debug.*.log");
-  const gitIgnorePatterns = parseGitIgnorePatterns(targetDir);
-  ignore.push(...gitIgnorePatterns);
+  // For built artifacts, we typically want to include everything except debug logs and git metadata.
+  // We do NOT ignore node_modules here because local builds (like Next.js standalone) often require them.
+  const ignore = ["firebase-debug.log", "firebase-debug.*.log", ".git"];
+
   try {
-    const files = await fsAsync.readdirRecursive({
+    const rdrFiles = await fsAsync.readdirRecursive({
       path: targetDir,
       ignore: ignore,
       isGitIgnore: true,
     });
-    for (const file of files) {
-      const name = path.relative(rootDir, file.name);
-      archive.file(file.name, {
-        name,
-        mode: file.mode,
-        // Set fixed metadata for deterministic hashing
-        stats: {
-          uid: 0,
-          gid: 0,
-          mtime: new Date(0),
-        },
-      } as archiver.EntryData);
+
+    const allFiles: string[] = rdrFiles.map((rdrf) => path.relative(rootDir, rdrf.name));
+
+    // If we're archiving a built app directory, we must also include the apphosting.yaml
+    // from the root directory so the backend knows the configuration.
+    if (targetSubDir) {
+      const rootFiles = fs.readdirSync(rootDir).filter((file) =>
+        APPHOSTING_YAML_FILE_REGEX.test(file),
+      );
+      for (const file of rootFiles) {
+        if (!allFiles.includes(file)) {
+          allFiles.push(file);
+        }
+      }
     }
-    await pipeAsync(archive, fileStream);
+
+    if (!allFiles.length) {
+      throw new FirebaseError(
+        `Cannot create a tar archive with 0 files from directory "${targetDir}"`,
+      );
+    }
+
+    // Sort files to ensure deterministic hashing (order matters in tarballs)
+    allFiles.sort();
+
+    await tar.create(
+      {
+        gzip: true,
+        file: tmpFile,
+        cwd: rootDir,
+        portable: true, // Strips host-specific UID/GID
+        mtime: new Date(0), // Set fixed timestamp for deterministic hashing
+      },
+      allFiles,
+    );
   } catch (err: unknown) {
+    if (err instanceof FirebaseError) {
+      throw err;
+    }
     throw new FirebaseError(
       "Could not read source directory. Remove links and shortcuts and try again.",
       { original: err as Error, exit: 1 },
     );
   }
+
   return tmpFile;
 }