Skip to content

fix(cloudflare): prevent Docker pull for prebuilt Container images#1292

Open
adunne09 wants to merge 3 commits intoalchemy-run:mainfrom
adunne09:fix/container-prebuilt-image-pull
Open

fix(cloudflare): prevent Docker pull for prebuilt Container images#1292
adunne09 wants to merge 3 commits intoalchemy-run:mainfrom
adunne09:fix/container-prebuilt-image-pull

Conversation

@adunne09
Copy link

@adunne09 adunne09 commented Jan 4, 2026

Summary

Fixes 401 errors when passing a prebuilt Cloudflare registry image to the Container resource. Previously, the Container resource would attempt to pull the image via Docker even for images already in the CF registry, causing authentication failures.

Changes

alchemy/src/cloudflare/container.ts

  • Added resolveImageName() function that normalizes image references following wrangler's logic:
    • Short names like myapp:v1registry.cloudflare.com/{accountId}/myapp:v1
    • CF registry without accountId → adds accountId
    • External registries → pass through unchanged
  • Short-circuits Container for prebuilt images - constructs Image object directly without Docker operations
  • Added validation that throws if both build and image are specified
  • For dev mode with CF registry images, throws helpful error directing users to use dev: { remote: true }

alchemy/test/cloudflare/container.test.ts

  • Unit tests for resolveImageName() covering short names, CF registry with/without accountId, external registries
  • Integration test verifying prebuilt CF registry images skip Docker pull
  • Test for mutual exclusivity of build/image props

Behavior Matrix (matches wrangler)

Input Behavior
Dockerfile path Build locally, push to CF registry
Registry URI Pass directly to Cloudflare API (no pull/push)

Example Usage

// Deploy now works without attempting to pull the image first
const container = await Container('my-container', {
  className: 'MyContainer',
  image: `registry.cloudflare.com/my-image:v1`,
});

adunne09 and others added 2 commits January 4, 2026 11:32
When passing a prebuilt image to the Cloudflare Container resource, the
image was always being pulled locally, which resulted in a 401 error
when using Cloudflare registry images (registry.cloudflare.com).

This matches wrangler's behavior where:
- Dockerfile path → build locally, push to CF registry
- Registry URI → pass directly to Cloudflare API (no pull/push)

Changes:
- Add resolveImageName() to normalize image references following
  wrangler's logic (add accountId to CF registry images if missing)
- Short-circuit Container for prebuilt images - construct Image object
  directly without Docker operations
- Add validation to prevent both 'build' and 'image' being specified
- For dev mode with CF registry images, throw helpful error directing
  users to use dev: { remote: true }
- Add comprehensive tests for resolveImageName normalization
- Add tests for new Container behavior

Amp-Thread-ID: https://ampcode.com/threads/T-019b8a2c-f6e7-7309-bc1a-42ef33802f66
Co-authored-by: Amp <amp@ampcode.com>
const container = await Container(containerName, {
className: "TestContainer",
name: containerName,
image: `${cfRegistry}/${api.accountId}/test-image:v1`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in everyone's account?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we ever get an answer to this? I see it's still in the diff @adunne09

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as I can tell- yes. its used throughout the codebase

https://github.com/search?q=repo%3Aalchemy-run%2Falchemy%20api.accountId&type=code

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are asking about the test-image:v1 being on everyone's account, not the account id existing on API.

Copy link
Author

@adunne09 adunne09 Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 5, 2026

Open in StackBlitz

npm i https://pkg.pr.new/alchemy-run/alchemy@1292

commit: 0c21ce5

Copy link
Collaborator

@Mkassabov Mkassabov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little concerned with how big the diff is for what seems like a small change but it looks good. Assuming tests pass this is good to merge.

Curious about that test image

@adunne09
Copy link
Author

adunne09 commented Jan 5, 2026

A little concerned with how big the diff is for what seems like a small change but it looks good. Assuming tests pass this is good to merge.

Curious about that test image

yeah there's a lot of comments, i can clean those up if desired.

to address your feedback https://github.com/alchemy-run/alchemy/pull/1292/changes/BASE..4bcba262b94b15cbab75d6fe514c51c4875c7510#r2659859673 - I extended the changes past updating the container deployment logic to also mirror wrangler dev behavior so that overall parity is achieved (or at least closer).

@Mkassabov Mkassabov self-requested a review March 3, 2026 23:19
@Mkassabov Mkassabov self-assigned this Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants