feat(cloudflare/container): build images from a prebuilt context directory#583
feat(cloudflare/container): build images from a prebuilt context directory#583Butch78 wants to merge 4 commits into
Conversation
The caller supplies a complete Docker build context (its own Dockerfile plus everything it COPYs) and the JS bundling pipeline is skipped: no main, no runtime shim, no appended ENTRYPOINT. The image hash folds in every file in the directory so content changes still trigger a rebuild and rollout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for deploying Cloudflare Containers from a fully self-contained, prebuilt Docker build context (skipping JS bundling and generated Dockerfile/entrypoint logic).
Changes:
- Introduces
prebuiltContextto allow building/pushing images directly from a caller-provided Docker context directory. - Adjusts validation so
mainis no longer required whenprebuiltContextis provided. - Updates the build pipeline to branch between “prebuilt context” and “bundled JS context” modes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/alchemy/src/Cloudflare/Container/ContainerApplication.ts | Implements prebuiltContext mode: hashes the directory contents for tagging and builds the provided context verbatim. |
| packages/alchemy/src/Cloudflare/Container/Container.ts | Makes main optional to support prebuiltContext-only containers and documents the new behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * JS entrypoint. Required unless `prebuiltContext` supplies a complete | ||
| * non-JS Docker build context. | ||
| */ | ||
| main?: string; |
| const entries = yield* fs.readDirectory(contextDir, { | ||
| recursive: true, | ||
| }); | ||
| const fileHashes: Record<string, string> = {}; | ||
| for (const entry of [...entries].sort()) { | ||
| const fullPath = `${contextDir}/${entry}`; | ||
| const info = yield* fs.stat(fullPath); | ||
| if (info.type === "File") { | ||
| fileHashes[entry] = yield* sha256(yield* fs.readFile(fullPath)); | ||
| } | ||
| } |
| }); | ||
| const fileHashes: Record<string, string> = {}; | ||
| for (const entry of [...entries].sort()) { | ||
| const fullPath = `${contextDir}/${entry}`; |
| const info = yield* fs.stat(fullPath); | ||
| if (info.type === "File") { | ||
| fileHashes[entry] = yield* sha256(yield* fs.readFile(fullPath)); | ||
| } |
| if (props.prebuiltContext) { | ||
| // Prebuilt-context mode: the caller supplies a complete Docker | ||
| // build context. Hash every regular file in it (sorted, so the | ||
| // digest is order-stable) in place of the bundle hash. |
| return { files: [], imageRef, imageHash }; | ||
| } |
| if (props.prebuiltContext) { | ||
| // Prebuilt-context mode: build the caller's directory verbatim, | ||
| // with its own Dockerfile, no bundle files, no appended ENTRYPOINT. | ||
| yield* dockerBuild({ | ||
| tag: imageRef, | ||
| context: props.prebuiltContext, | ||
| platform: "linux/amd64", | ||
| }); |
There was a problem hiding this comment.
I don't think prebuiltContext is an intuitive name for this property. I do agree it should be possible to skip bundling.
Why not just call it context?: string and make main?: string optional. Skip JS bundling if main and dockerfile are not provided?
I also wonder if dockerfile should be a path (or maybe even allow it to be both a path or dockerfile contents because we can detect that with path.resolve?
Cloudflare.Container("container", {
context: "./sql-container",
// dockerfile: "Dockerfile.custom" // <- defaults to "Dockerfile"
})There was a problem hiding this comment.
Good call, done in 9fbf01d:
- Renamed
prebuiltContext→context. JS bundling is skipped whenever there's nomain(the two are now mutually exclusive). dockerfileaccepts a path or inline contents, resolved by whether it points at an existing file (path.resolveagainst the context, then cwd). In context mode it defaults to<context>/Dockerfileand is passed viadocker build -f.
Cloudflare.Container("container", {
context: "./sql-container",
// dockerfile: "Dockerfile.custom" // path, or inline "FROM …" contents
})…dress review Respond to PR review (sam-goodwin + Copilot): - Rename `prebuiltContext` -> `context`; build context mode iff no `main` (the two are mutually exclusive). `main` stays optional. - `dockerfile` accepts a path OR inline contents, resolved by existence, in both modes. Context mode defaults to `<context>/Dockerfile` and passes `docker build -f`; `dockerBuild` gains a `dockerfile` option. - Single `resolveContextMode` helper decides mode + Dockerfile location for both the hash and build steps, so they can't drift. - Context hash: honor `.dockerignore` + always skip `.git`, fold in unix mode, and stream each file via `sha256File` (no whole-file buffering). - Cross-platform `path.join` instead of string concatenation. - Tests: streaming `sha256File`, and `docker build -f` custom Dockerfile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @copilot — addressed in 9fbf01d (alongside the
Added unit coverage for streaming |
| export const sha256File = Effect.fn(function* (filePath: string) { | ||
| const fs = yield* FileSystem.FileSystem; | ||
| const hash = createHash("sha256"); | ||
| yield* fs | ||
| .stream(filePath) | ||
| .pipe(Stream.runForEach((chunk) => Effect.sync(() => hash.update(chunk)))); | ||
| return hash.digest("hex"); | ||
| }); |
There was a problem hiding this comment.
Do we already have a util for this in sha256.ts?
If not, add it to that file for re-usability.
There was a problem hiding this comment.
Good idea — moved it to Util/sha256.ts as sha256File (streams via the FileSystem service through node:crypto, alongside sha256/sha256Object) in 607f4ed. Tests moved to test/Util/sha256.test.ts.
Per review: keep the streaming file hash with the other SHA-256 helpers for reuse instead of in Bundle/Docker.ts. Tests move to test/Util/sha256. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the suggestions, @sam-goodwin — both are in: |
Build a Cloudflare Container straight from a self-contained Docker build context, skipping JS bundling entirely. Cloudflare Containers run any
linux/amd64image; only the resource assumed a JS entrypoint.context(a build-context dir) replaces the JS-bundling pipeline when nomainis set.mainandcontextare mutually exclusive;mainstays optional.dockerfileaccepts a path or inline contents (told apart by whether it resolves to a file), in both modes. Context mode passesdocker build -f;dockerBuildgains adockerfileoption.resolveContextModehelper decides mode + Dockerfile location for both the hash and the build, so they can't drift..dockerignoreand always skipping.git, so content changes trigger a rebuild and rollout.Used to deploy a Rust (DataFusion) container attached to a Durable Object: builds, pushes to
registry.cloudflare.comwith the content-addressed tag, and serves viactx.container.