Skip to content

containers: update push and build to resolve the correct uri #10623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 26, 2025
+244 −31
Merged

containers: update push and build to resolve the correct uri #10623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5fd6701
containers: update push to resolve the correct uri
IRCody Sep 11, 2025
ec0e7d5
containers: update build to resolve the correct uri
IRCody Sep 11, 2025
2035805
Containers: modify build to use the users provided tag
IRCody Sep 12, 2025
809c5b5
check + test updates
IRCody Sep 12, 2025
19e753d
Deploy test updates
IRCody Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/common-beers-push.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@cloudflare/containers-shared": minor
"wrangler": patch
---

Handle more cases for correctly resolving the full uri for an image when using containers push.
14 changes: 10 additions & 4 deletions packages/containers-shared/src/images.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
isCloudflareRegistryLink,
} from "./knobs";
import { dockerLoginManagedRegistry } from "./login";
import { getCloudflareRegistryWithAccountNamespace } from "./registry";
import {
checkExposedPorts,
cleanupDuplicateImageTags,
Expand Down Expand Up @@ -124,24 +125,29 @@ export async function prepareContainerImagesForDev(args: {
/**
* Resolve an image name to the full unambiguous name.
*
* For now, this only converts images stored in the managed registry to contain
* the user's account ID in the path.
* image:tag -> prepend registry.cloudflare.com/accountid/
* registry.cloudflare.com/image:tag -> registry.cloudlfare.com/accountid/image:tag
* registry.cloudflare.com/accountid/image:tag -> no change
* anyother-registry.com/anything -> no change
*/
export function resolveImageName(accountId: string, image: string): string {
let url: URL;
try {
url = new URL(`http://${image}`);
} catch {
return image;
// not a valid url so assume it is in the format image:tag and pre-pend the registry
return getCloudflareRegistryWithAccountNamespace(accountId, image);
Copy link
Member

@gpanders gpanders Sep 11, 2025

Choose a reason for hiding this comment

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

What if image is a file path, like ./Dockerfile?

Copy link
Contributor Author

@IRCody IRCody Sep 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It really should never be since this is only called before pushing an image.

I'm not sure if the function should be changed to deal with that or just change the name to be more reflective of what it is used for. WDYT?

Copy link
Member

@gpanders gpanders Sep 12, 2025

Choose a reason for hiding this comment

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

I think this is fine as is then, thanks for clarifying.

}

// hostname not the managed registry, passthrough
if (url.hostname !== getCloudflareContainerRegistry()) {
return image;
}

// is managed registry and has the account id, passthrough
if (url.pathname.startsWith(`/${accountId}`)) {
return image;
}

// is managed registry and doesn't have the account id,add it to the path
return `${url.hostname}/${accountId}${url.pathname}`;
}
173 changes: 158 additions & 15 deletions packages/wrangler/src/__tests__/cloudchamber/build.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ describe("buildAndMaybePush", () => {
runInTempDir();
mockApiToken();
mockAccountId();

beforeEach(() => {
vi.clearAllMocks();
vi.mocked(dockerImageInspect)
Expand All @@ -52,6 +51,150 @@ describe("buildAndMaybePush", () => {
vi.clearAllMocks();
});

it("should be able to build image and push with test-app:tag", async () => {
await runWrangler(
"containers build ./container-context -t test-app:tag -p"
);
expect(dockerBuild).toHaveBeenCalledWith("docker", {
buildCmd: [
"build",
"-t",
`test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
"-f",
"-",
// turn this into a relative path so that this works across different OSes
"./container-context",
],
dockerfile,
});

// 3 calls: docker tag + docker push + docker rm
expect(runDockerCmd).toHaveBeenCalledTimes(3);
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
"tag",
`test-app:tag`,
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
"push",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
"image",
"rm",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
imageTag: `test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
});
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
imageTag: `test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
});
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
});

it("should be able to build image and push with registry.cloudflare.com/test-app:tag", async () => {
await runWrangler(
"containers build ./container-context -t registry.cloudflare.com/test-app:tag -p"
);
expect(dockerBuild).toHaveBeenCalledWith("docker", {
buildCmd: [
"build",
"-t",
`registry.cloudflare.com/test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
"-f",
"-",
// turn this into a relative path so that this works across different OSes
"./container-context",
],
dockerfile,
});

// 3 calls: docker tag + docker push + docker rm
expect(runDockerCmd).toHaveBeenCalledTimes(3);
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
"tag",
`registry.cloudflare.com/test-app:tag`,
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
"push",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
"image",
"rm",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
});
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
});
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
});

it("should be able to build image and push with registry.cloudflare.com/some-account-id/test-app:tag", async () => {
await runWrangler(
"containers build ./container-context -t registry.cloudflare.com/some-account-id/test-app:tag -p"
);
expect(dockerBuild).toHaveBeenCalledWith("docker", {
buildCmd: [
"build",
"-t",
`registry.cloudflare.com/some-account-id/test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
"-f",
"-",
// turn this into a relative path so that this works across different OSes
"./container-context",
],
dockerfile,
});

// 3 calls: docker tag + docker push + docker rm
expect(runDockerCmd).toHaveBeenCalledTimes(3);
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
"tag",
`registry.cloudflare.com/some-account-id/test-app:tag`,
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
"push",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
"image",
"rm",
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
imageTag: `registry.cloudflare.com/some-account-id/test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
});
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
imageTag: `registry.cloudflare.com/some-account-id/test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
});
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
});

it("should use a custom docker path if provided", async () => {
vi.stubEnv("WRANGLER_DOCKER_BIN", "/custom/docker/path");
await runWrangler(
Expand All @@ -61,7 +204,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand All @@ -76,15 +219,15 @@ describe("buildAndMaybePush", () => {
1,
"/custom/docker/path",
{
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
}
);
expect(dockerImageInspect).toHaveBeenNthCalledWith(
2,
"/custom/docker/path",
{
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
}
);
Expand All @@ -105,7 +248,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand All @@ -121,7 +264,7 @@ describe("buildAndMaybePush", () => {
expect(runDockerCmd).toHaveBeenCalledTimes(3);
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
"tag",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
]);
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
Expand All @@ -135,11 +278,11 @@ describe("buildAndMaybePush", () => {
]);
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
});
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
});
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
Expand Down Expand Up @@ -168,7 +311,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand All @@ -189,15 +332,15 @@ describe("buildAndMaybePush", () => {
expect(runDockerCmd).toHaveBeenCalledWith("docker", [
"image",
"rm",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
]);
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ json .RepoDigests }} {{ .Id }}",
});
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
imageTag: `test-app:tag`,
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
});
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
Expand All @@ -210,7 +353,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app`,
`test-app`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand All @@ -232,7 +375,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app`,
`test-app`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand All @@ -254,7 +397,7 @@ describe("buildAndMaybePush", () => {
buildCmd: [
"build",
"-t",
`${getCloudflareContainerRegistry()}/test-app:tag`,
`test-app:tag`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand Down
8 changes: 4 additions & 4 deletions packages/wrangler/src/__tests__/containers/deploy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1957,7 +1957,7 @@ function mockDockerBuild(
expect(args).toEqual([
"build",
"-t",
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
`${containerName}:${tag}`,
"--platform",
"linux/amd64",
"--provenance=false",
Expand Down Expand Up @@ -1998,7 +1998,7 @@ function mockDockerImageInspectDigests(containerName: string, tag: string) {
expect(args).toEqual([
"image",
"inspect",
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
`${containerName}:${tag}`,
"--format",
"{{ json .RepoDigests }} {{ .Id }}",
]);
Expand Down Expand Up @@ -2034,7 +2034,7 @@ function mockDockerImageInspectSize(containerName: string, tag: string) {
expect(args).toEqual([
"image",
"inspect",
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
`${containerName}:${tag}`,
"--format",
"{{ .Size }} {{ len .RootFS.Layers }}",
]);
Expand Down Expand Up @@ -2113,7 +2113,7 @@ function mockDockerTag(from: string, to: string, tag: string) {
expect(cmd).toBe("/usr/bin/docker");
expect(args).toEqual([
"tag",
`${getCloudflareContainerRegistry()}/${from}:${tag}`,
`${from}:${tag}`,
`${getCloudflareContainerRegistry()}/${to}:${tag}`,
]);
return defaultChildProcess();
Expand Down
Loading
Loading