Skip to content

Commit cc15136

Browse files
committed
fix(cli): harden the image dependency-layer split
Create node_modules after post-install commands so a command that prunes it can't fail the final-stage copy, keep the original install instruction so existing layer caches still hit, and make the code stage's node_modules removal work as a non-root user when a directory is read-only.
1 parent ccb2569 commit cc15136

3 files changed

Lines changed: 39 additions & 10 deletions

File tree

.changeset/split-image-dependency-layers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"trigger.dev": patch
33
---
44

5-
Deployed images now ship dependencies and bundled task code as separate layers. Repeat deploys with unchanged dependencies push and pull far less data, making deploys and worker image pulls faster.
5+
Deployed images now ship dependencies and bundled task code as separate layers. Repeat deploys with unchanged dependencies typically push and pull far less data, making deploys and worker image pulls faster.

packages/cli-v3/src/deploy/buildImage.test.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,40 @@ describe("generateContainerfile", () => {
4040
const user = runtime === "bun" ? "bun:bun" : "node:node";
4141

4242
expect(containerfile).toContain("FROM build AS code");
43-
expect(containerfile).toContain("RUN rm -rf node_modules");
4443
expect(containerfile).toContain(
4544
`COPY --from=build --chown=${user} /app/node_modules ./node_modules`
4645
);
4746
expect(containerfile).toContain(`COPY --from=code --chown=${user} /app ./`);
4847
// The final stage must not copy all of /app from the build stage anymore,
4948
// or node_modules would be duplicated across two layers
5049
expect(containerfile).not.toContain(`COPY --from=build --chown=${user} /app ./`);
50+
}
51+
);
52+
53+
it.each(["node", "bun"] as BuildRuntime[])(
54+
"orders post-install commands, the node_modules guard, and the code stage for %s",
55+
async (runtime) => {
56+
const containerfile = await generateContainerfile({
57+
runtime,
58+
build: { commands: ["echo post-install"] },
59+
image: undefined,
60+
indexScript: "index.js",
61+
entrypoint: "entrypoint.js",
62+
});
63+
64+
const postInstall = containerfile.indexOf("RUN echo post-install");
65+
// The guard must run after post-install commands so a command that prunes
66+
// node_modules can't break the final-stage COPY of /app/node_modules
67+
const mkdirGuard = containerfile.indexOf("RUN mkdir -p node_modules");
68+
const codeStage = containerfile.indexOf("FROM build AS code");
69+
const rmNodeModules = containerfile.indexOf(
70+
"RUN chmod -R u+w node_modules && rm -rf node_modules"
71+
);
5172

52-
// node_modules must exist even for projects with zero external dependencies
53-
expect(containerfile).toContain("mkdir -p node_modules");
73+
expect(postInstall).toBeGreaterThan(-1);
74+
expect(mkdirGuard).toBeGreaterThan(postInstall);
75+
expect(codeStage).toBeGreaterThan(mkdirGuard);
76+
expect(rmNodeModules).toBeGreaterThan(codeStage);
5477
}
5578
);
5679
});

packages/cli-v3/src/deploy/buildImage.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -772,19 +772,22 @@ ${buildArgs}
772772
${buildEnvVars}
773773
774774
COPY --chown=bun:bun package.json ./
775-
# mkdir guards against bun not creating node_modules when there are no dependencies
776-
RUN bun install --production --no-save && mkdir -p node_modules
775+
RUN bun install --production --no-save
777776
778777
# Now copy all the files
779778
# IMPORTANT: Do this after running npm install because npm i will wipe out the node_modules directory
780779
COPY --chown=bun:bun . .
781780
782781
${postInstallCommands}
783782
783+
# node_modules may not exist when there are no dependencies to install
784+
RUN mkdir -p node_modules
785+
784786
# App files without node_modules, so the final stage can layer them separately
785787
FROM build AS code
786788
787-
RUN rm -rf node_modules
789+
# u+w first: rm as a non-root user fails on read-only directories
790+
RUN chmod -R u+w node_modules && rm -rf node_modules
788791
789792
FROM build AS indexer
790793
@@ -887,8 +890,7 @@ ENV NODE_ENV=production
887890
ENV NPM_CONFIG_UPDATE_NOTIFIER=false
888891
889892
COPY --chown=node:node package.json ./
890-
# mkdir guards against npm not creating node_modules when there are no dependencies
891-
RUN npm i --no-audit --no-fund --no-save --no-package-lock && mkdir -p node_modules
893+
RUN npm i --no-audit --no-fund --no-save --no-package-lock
892894
893895
# Now copy all the files
894896
# IMPORTANT: Do this after running npm install because npm i will wipe out the node_modules directory
@@ -899,10 +901,14 @@ ${postInstallCommands}
899901
# IMPORTANT: Doing this again to fix an issue with prisma generate removing the files in node_modules/trigger.dev for some reason...
900902
COPY --chown=node:node . .
901903
904+
# node_modules may not exist when there are no dependencies to install
905+
RUN mkdir -p node_modules
906+
902907
# App files without node_modules, so the final stage can layer them separately
903908
FROM build AS code
904909
905-
RUN rm -rf node_modules
910+
# u+w first: rm as a non-root user fails on read-only directories
911+
RUN chmod -R u+w node_modules && rm -rf node_modules
906912
907913
FROM build AS indexer
908914

0 commit comments

Comments
 (0)