Use digest-pinned images for Dockerfile container deploys#14343
Conversation
🦋 Changeset detectedLatest commit: 33d7240 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
Returns a digest-pinned image reference from pushed Dockerfile builds (with a docker manifest inspect fallback) and always deploys that digest instead of falling back to the previous app's tag.
The core logic looks sound and is well covered by the updated tests:
findRemoteDigest/imageRefWithDigestcorrectly resolve the repo and re-attach the digest, and theresolveImageNameidempotency means passing the already-namespaced tag in the post-push path is safe.- Dropping the
prevApp?.configuration.imagefallback is the right call given Cloudchamber now requires a digest-pinned reference, and the early-exit ("already exists remotely") path also returns a digest, so deploys stay consistent.
One actionable item:
-
(medium) Missing changeset. This is a user-facing behavior change to
wrangler(Dockerfile container deploys now publish a digest-pinned image reference instead of a mutable tag), so per the repo conventions it needs a changeset or it won't trigger a release. The changeset-bot also flagged this. Apatchchangeset describing the fix would do, e.g.:--- "wrangler": patch --- Use digest-pinned images for Dockerfile container deploys Snapshot-enabled container applications require `configuration.image` to be a registry reference pinned by digest. Dockerfile-backed containers now deploy the pushed image's digest reference instead of the mutable tag.
Alternatively, add the
no-changeset-requiredlabel if you intend to ship this without a release.
|
Review posted successfully to PR #14343. I reviewed PR #14343 ("Use digest-pinned images for Dockerfile container deploys"). The core logic is sound and well-tested:
The one actionable item is the missing changeset for a user-facing |
@cloudflare/autoconfig
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
fa5dd7a to
e5858a9
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
e5858a9 to
c933e52
Compare
|
✅ All changesets look good |
c933e52 to
6956e5b
Compare
|
@cloudflare/wrangler can you please take a look? Thank you |
6956e5b to
85a14fd
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
251111c to
9729a68
Compare
9729a68 to
33d7240
Compare
|
Thanks for enabling e2e CI tests @edmundhung CI is green! |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
Snapshot-enabled container applications are rejected by Cloudchamber unless configuration.image is a registry reference pinned by digest. Wrangler still lets users configure Dockerfile-backed containers with image = "./Dockerfile", but after building and pushing the image it deployed the mutable tag, which fails validation when experimental_enable_snapshots is enabled.
Return a digest-pinned image reference from pushed Dockerfile builds and use it in container application deploys. Preserve the local tag path for non-pushed builds, keep registry image URI deploys unchanged, and fall back to docker manifest inspect when Docker does not expose RepoDigests after push.
This lets Dockerfile-based Wrangler configs continue to work for snapshot-enabled container apps while giving Cloudchamber the immutable image reference it requires.
A picture of a cute animal (not mandatory, but encouraged)
