Skip to content

Commit d66ec89

Browse files
falahatfalahat
authored
make local builds respect "ignore" files when uploading the built output (#10438)
* make local builds respect "ignore" files when uploading the built output to GCS for zip deploys * Make it so we copy local build files into a separate temporary folder before we actually build it * fix the test cleanup * delete local_build folder * run the linters/formatters again * test and formatting fixes * Make sure we do not accidentally clobber existing data if a "local_build" folder already exists * remove legacy localbuild path and remove @apphosting/build dependency * get rid of universalMaker experiment * Update package lock * Remove unneccesary warning * format fix * fix: scope local build archives to local_build folder and include bundle.yaml * refactor: define and use LOCAL_BUILD_DIR_NAME constant for local build directory * Clarify and clean up how we handle ignore files and in what order. * For local builds, we apply all ignore filtering (user firebase.json ignores and .gitignore) * BEFORE the build runs, right here during the directory copying phase. This creates a clean, * isolated workspace in the `.local_build` folder that contains exactly the same source files * that would be uploaded to Cloud Build. * Allow parallel local builds by making the .local_build folder unique to each backend * improve wording * Fix format/linter errors * By default, ignore local builds folders. * fix glob pattern * remove prepare.ts ignore-listing
1 parent 7ce2bd5 commit d66ec89

10 files changed

Lines changed: 390 additions & 45 deletions

File tree

‎src/apphosting/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
export const DEFAULT_LOCATION = "us-east4";
22
export const DEFAULT_DEPLOY_METHOD = "github";
33
export const ALLOWED_DEPLOY_METHODS = [{ name: "Deploy using github", value: "github" }];
4+
export const LOCAL_BUILD_DIR_NAME = ".local_build";

‎src/apphosting/localbuilds.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ function executeUniversalMakerBinary(universalMakerBinary: string, projectRoot:
4646
universalMakerBinary,
4747
["-application_dir", projectRoot, "-output_dir", projectRoot, "-output_format", "json"],
4848
{
49+
cwd: projectRoot,
4950
env: {
5051
...process.env,
5152
X_GOOGLE_TARGET_PLATFORM: "fah",

‎src/deploy/apphosting/args.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export interface LocalBuild {
55
buildConfig: BuildConfig;
66
buildDir: string;
77
annotations: Record<string, string>;
8+
localBuildScratchDir: string;
89
}
910

1011
export interface Context {

‎src/deploy/apphosting/deploy.spec.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from "chai";
22
import * as sinon from "sinon";
3+
import * as path from "path";
34
import { Config } from "../../config";
45
import * as gcs from "../../gcp/storage";
56
import { RC } from "../../rc";
@@ -10,6 +11,7 @@ import * as fs from "fs";
1011
import * as getProjectNumber from "../../getProjectNumber";
1112
import * as experiments from "../../experiments";
1213
import { FirebaseError } from "../../error";
14+
import { LOCAL_BUILD_DIR_NAME } from "../../apphosting/constants";
1315

1416
const BASE_OPTS = {
1517
cwd: "/",
@@ -42,6 +44,7 @@ function initializeContext(): Context {
4244
backendLocalBuilds: {
4345
fooLocalBuild: {
4446
buildDir: "./nextjs/standalone",
47+
localBuildScratchDir: path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_fooLocalBuild`),
4548
buildConfig: {},
4649
annotations: {},
4750
},
@@ -172,7 +175,7 @@ describe("apphosting", () => {
172175
);
173176
expect(createTarArchiveStub).to.be.calledWithExactly(
174177
context.backendConfigs["fooLocalBuild"],
175-
process.cwd(),
178+
path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_fooLocalBuild`),
176179
"./nextjs/standalone",
177180
);
178181
expect(uploadObjectStub).to.be.calledWithMatch(
@@ -215,7 +218,7 @@ describe("apphosting", () => {
215218
);
216219
expect(createTarArchiveStub).to.be.calledWithExactly(
217220
context.backendConfigs["fooLocalBuild"],
218-
process.cwd(),
221+
path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_fooLocalBuild`),
219222
"./nextjs/standalone",
220223
);
221224
expect(uploadObjectStub).to.be.calledWithMatch(

‎src/deploy/apphosting/deploy.ts‎

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { logLabeledBullet } from "../../utils";
99
import { Context } from "./args";
1010
import * as util from "./util";
1111
import * as experiments from "../../experiments";
12+
import { logger } from "../../logger";
1213

1314
/**
1415
* Uploads App Hosting source code or local build output to Google Cloud Storage.
@@ -18,7 +19,6 @@ import * as experiments from "../../experiments";
1819
* method, while local build deployments are tar-balled using the "createTarArchive"
1920
* method. The resulting archive is uploaded to the bucket, and the URI is stored in
2021
* the context for the subsequent release phase.
21-
*
2222
* @param context - The deployment context containing backend configs and locations.
2323
* @param options - CLI options providing project ID and root directory.
2424
*/
@@ -73,47 +73,62 @@ export default async function (context: Context, options: Options): Promise<void
7373
await Promise.all(
7474
Object.values(context.backendConfigs).map(async (cfg) => {
7575
const rootDir = options.projectRoot ?? process.cwd();
76-
let builtAppDir;
77-
const isLocalBuild = !!cfg.localBuild;
78-
if (isLocalBuild) {
79-
experiments.assertEnabled("apphostinglocalbuilds", "App Hosting local builds");
80-
builtAppDir = context.backendLocalBuilds[cfg.backendId].buildDir;
81-
if (!builtAppDir) {
82-
throw new FirebaseError(`No local build dir found for ${cfg.backendId}`);
76+
let localBuildScratchDir: string | undefined;
77+
try {
78+
const isLocalBuild = cfg.localBuild;
79+
let builtAppDir: string | undefined;
80+
if (isLocalBuild) {
81+
experiments.assertEnabled("apphostinglocalbuilds", "App Hosting local builds");
82+
const localBuild = context.backendLocalBuilds[cfg.backendId];
83+
builtAppDir = localBuild?.buildDir;
84+
localBuildScratchDir = localBuild?.localBuildScratchDir;
85+
if (!builtAppDir || !localBuildScratchDir) {
86+
throw new FirebaseError(`No local build dir found for ${cfg.backendId}`);
87+
}
8388
}
84-
}
8589

86-
const zippedSourcePath = isLocalBuild
87-
? await util.createLocalBuildTarArchive(cfg, rootDir, builtAppDir)
88-
: await util.createSourceDeployArchive(cfg, rootDir);
90+
const zippedSourcePath = isLocalBuild
91+
? await util.createLocalBuildTarArchive(cfg, localBuildScratchDir!, builtAppDir)
92+
: await util.createSourceDeployArchive(cfg, rootDir);
8993

90-
logLabeledBullet(
91-
"apphosting",
92-
`Zipped ${isLocalBuild ? "built app" : "source"} for backend ${cfg.backendId}`,
93-
);
94+
logLabeledBullet(
95+
"apphosting",
96+
`Zipped ${isLocalBuild ? "built app" : "source"} for backend ${cfg.backendId}`,
97+
);
9498

95-
const backendLocation = context.backendLocations[cfg.backendId];
96-
if (!backendLocation) {
97-
throw new FirebaseError(
98-
`Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
99+
const backendLocation = context.backendLocations[cfg.backendId];
100+
if (!backendLocation) {
101+
throw new FirebaseError(
102+
`Failed to find location for backend ${cfg.backendId}. Please contact support with the contents of your firebase-debug.log to report your issue.`,
103+
);
104+
}
105+
logLabeledBullet(
106+
"apphosting",
107+
`Uploading ${isLocalBuild ? "built app" : "source"} for backend ${cfg.backendId}...`,
99108
);
109+
const bucketName = bucketsPerLocation[backendLocation]!;
110+
const { bucket, object } = await gcs.uploadObject(
111+
{
112+
file: zippedSourcePath,
113+
stream: fs.createReadStream(zippedSourcePath),
114+
},
115+
bucketName,
116+
isLocalBuild ? gcs.ContentType.TAR : gcs.ContentType.ZIP,
117+
);
118+
logLabeledBullet("apphosting", `Uploaded at gs://${bucket}/${object}`);
119+
context.backendStorageUris[cfg.backendId] =
120+
`gs://${bucketName}/${path.basename(zippedSourcePath)}`;
121+
} finally {
122+
if (localBuildScratchDir && fs.existsSync(localBuildScratchDir)) {
123+
try {
124+
fs.rmSync(localBuildScratchDir, { recursive: true, force: true });
125+
} catch (err) {
126+
logger.debug(
127+
`Failed to clean up local build directory ${localBuildScratchDir}: ${err}`,
128+
);
129+
}
130+
}
100131
}
101-
logLabeledBullet(
102-
"apphosting",
103-
`Uploading ${isLocalBuild ? "built app" : "source"} for backend ${cfg.backendId}...`,
104-
);
105-
const bucketName = bucketsPerLocation[backendLocation]!;
106-
const { bucket, object } = await gcs.uploadObject(
107-
{
108-
file: zippedSourcePath,
109-
stream: fs.createReadStream(zippedSourcePath),
110-
},
111-
bucketName,
112-
isLocalBuild ? gcs.ContentType.TAR : gcs.ContentType.ZIP,
113-
);
114-
logLabeledBullet("apphosting", `Uploaded at gs://${bucket}/${object}`);
115-
context.backendStorageUris[cfg.backendId] =
116-
`gs://${bucketName}/${path.basename(zippedSourcePath)}`;
117132
}),
118133
);
119134
}

‎src/deploy/apphosting/prepare.spec.ts‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from "chai";
22
import * as sinon from "sinon";
3+
import * as path from "path";
34
import * as backend from "../../apphosting/backend";
45
import { Config } from "../../config";
56
import * as apiEnabled from "../../ensureApiEnabled";
@@ -24,6 +25,9 @@ import * as apphostingUtils from "../../apphosting/utils";
2425
import { AppHostingYamlConfig, EnvMap } from "../../apphosting/yaml";
2526
import { Options } from "../../options";
2627
import { AppHostingSingle } from "../../firebaseConfig";
28+
import * as fs from "fs";
29+
import * as fsAsync from "../../fsAsync";
30+
import { LOCAL_BUILD_DIR_NAME } from "../../apphosting/constants";
2731

2832
const BASE_OPTS = {
2933
cwd: "/",
@@ -88,6 +92,12 @@ describe("apphosting", () => {
8892
addServiceAccountToRolesStub = sinon
8993
.stub(resourceManager, "addServiceAccountToRoles")
9094
.resolves();
95+
96+
sinon.stub(fs, "existsSync").returns(false);
97+
sinon.stub(fs, "mkdirSync").returns(undefined);
98+
sinon.stub(fs, "rmSync").returns(undefined);
99+
sinon.stub(fs, "copyFileSync").returns(undefined);
100+
sinon.stub(fsAsync, "readdirRecursive").resolves([]);
91101
});
92102

93103
afterEach(() => {
@@ -142,6 +152,7 @@ describe("apphosting", () => {
142152
});
143153
expect(context.backendLocalBuilds["foo"]).to.deep.equal({
144154
buildDir: "./next/standalone",
155+
localBuildScratchDir: path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_foo`),
145156
buildConfig,
146157
annotations,
147158
});
@@ -153,6 +164,94 @@ describe("apphosting", () => {
153164
);
154165
});
155166

167+
it("supports multiple parallel local builds without directory clobbering", async () => {
168+
const optsWithMultipleLocalBuilds = {
169+
...opts,
170+
config: new Config({
171+
apphosting: [
172+
{
173+
backendId: "backend-prod",
174+
rootDir: "/",
175+
ignore: [],
176+
localBuild: true,
177+
},
178+
{
179+
backendId: "backend-staging",
180+
rootDir: "/",
181+
ignore: [],
182+
localBuild: true,
183+
},
184+
],
185+
}),
186+
};
187+
const context = initializeContext();
188+
context.backendConfigs = {
189+
"backend-prod": {
190+
backendId: "backend-prod",
191+
rootDir: "/",
192+
ignore: [],
193+
localBuild: true,
194+
},
195+
"backend-staging": {
196+
backendId: "backend-staging",
197+
rootDir: "/",
198+
ignore: [],
199+
localBuild: true,
200+
},
201+
};
202+
context.backendLocations = {
203+
"backend-prod": "us-central1",
204+
"backend-staging": "us-central1",
205+
};
206+
207+
const localBuildStub = sinon.stub(localbuilds, "localBuild");
208+
localBuildStub
209+
.withArgs(
210+
sinon.match.any,
211+
sinon.match((p: string) => p.endsWith(`${LOCAL_BUILD_DIR_NAME}_backend-prod`)),
212+
sinon.match.any,
213+
)
214+
.resolves({
215+
outputFiles: ["./next/standalone-prod"],
216+
buildConfig: { runCommand: "npm run build:prod", env: [] },
217+
annotations: { framework: "nextjs" },
218+
});
219+
localBuildStub
220+
.withArgs(
221+
sinon.match.any,
222+
sinon.match((p: string) => p.endsWith(`${LOCAL_BUILD_DIR_NAME}_backend-staging`)),
223+
sinon.match.any,
224+
)
225+
.resolves({
226+
outputFiles: ["./next/standalone-staging"],
227+
buildConfig: { runCommand: "npm run build:staging", env: [] },
228+
annotations: { framework: "nextjs" },
229+
});
230+
231+
listBackendsStub.onFirstCall().resolves({
232+
backends: [
233+
{ name: "projects/my-project/locations/us-central1/backends/backend-prod" },
234+
{ name: "projects/my-project/locations/us-central1/backends/backend-staging" },
235+
],
236+
});
237+
238+
await prepare(context, optsWithMultipleLocalBuilds);
239+
240+
expect(context.backendLocalBuilds["backend-prod"].localBuildScratchDir).to.equal(
241+
path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_backend-prod`),
242+
);
243+
expect(context.backendLocalBuilds["backend-staging"].localBuildScratchDir).to.equal(
244+
path.join(process.cwd(), `${LOCAL_BUILD_DIR_NAME}_backend-staging`),
245+
);
246+
247+
expect(context.backendLocalBuilds["backend-prod"].buildDir).to.equal(
248+
"./next/standalone-prod",
249+
);
250+
expect(context.backendLocalBuilds["backend-staging"].buildDir).to.equal(
251+
"./next/standalone-staging",
252+
);
253+
});
254+
156255
it("injects Firebase configuration when appId is present", async () => {
157256
const optsWithLocalBuild = {
158257
...opts,
@@ -305,6 +404,41 @@ describe("apphosting", () => {
305404
}
306405
});
307406

407+
it("should fail if localBuild is specified and local build directory already exists", async () => {
408+
const optsWithLocalBuild = {
409+
...opts,
410+
config: new Config({
411+
apphosting: {
412+
backendId: "foo",
413+
rootDir: "/",
414+
ignore: [],
415+
localBuild: true,
416+
},
417+
}),
418+
};
419+
const context = initializeContext();
420+
421+
(fs.existsSync as sinon.SinonStub).callsFake((pathLike: fs.PathLike) => {
422+
if (typeof pathLike === "string" && pathLike.endsWith(`${LOCAL_BUILD_DIR_NAME}_foo`)) {
423+
return true;
424+
}
425+
return false;
426+
});
427+
428+
listBackendsStub.onFirstCall().resolves({
429+
backends: [
430+
{
431+
name: "projects/my-project/locations/us-central1/backends/foo",
432+
},
433+
],
434+
});
435+
436+
await expect(prepare(context, optsWithLocalBuild)).to.be.rejectedWith(
437+
FirebaseError,
438+
"The local build scratch directory",
439+
);
440+
});
441+
308442
it("links to existing backend if it already exists", async () => {
309443
const context = initializeContext();
310444
listBackendsStub.onFirstCall().resolves({

0 commit comments

Comments
 (0)