Build container without account ID

This allows us to build containers in dry run mode. When we push the
container to the managed registry, we re-tag it to include the account
ID. We only ever push when not in dry run mode though, so when pushing
we can safely grab the account information.

Co-authored-by: Hasip Timurtaş <[email protected]>

Author: gpanders

.changeset/container-dry-run-fix.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+Build container images without the user's account ID. This allows containers to be built and verified in dry run mode (where we do not necessarily have the user's account info).
+
+When we push the image to the managed registry, we first re-tag the image to include the user's account ID so that the image has the full resolved image name.

packages/containers-shared/src/images.ts

@@ -1,5 +1,5 @@
 import { buildImage } from "./build";
-import { isCloudflareRegistryLink } from "./knobs";
+import { getCloudflareContainerRegistry, isCloudflareRegistryLink } from "./knobs";
 import { dockerLoginManagedRegistry } from "./login";
 import { ContainerDevOptions } from "./types";
 import {
@@ -61,3 +61,32 @@ export async function prepareContainerImagesForDev(
 		await checkExposedPorts(dockerPath, options);
 	}
 }
+
+/**
+ * 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.
+ */
+export async function resolveImageName(
+	accountId: string,
+	image: string
+): Promise<string> {
+	let url: URL;
+	try {
+		url = new URL(`http://${image}`);
+	} catch (_) {
+		return image;
+	}
+
+	if (url.hostname !== getCloudflareContainerRegistry()) {
+		return image;
+	}
+
+	if (url.pathname.startsWith(`/${accountId}`)) {
+		return image;
+	}
+
+	return `${url.hostname}/${accountId}${url.pathname}`;
+}
+

packages/wrangler/src/__tests__/cloudchamber/build.test.ts

@@ -62,7 +62,7 @@ describe("buildAndMaybePush", () => {
 			buildCmd: [
 				"build",
 				"-t",
-				`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+				`${getCloudflareContainerRegistry()}/test-app:tag`,
 				"--platform",
 				"linux/amd64",
 				"--provenance=false",
@@ -73,7 +73,7 @@ describe("buildAndMaybePush", () => {
 			dockerfile,
 		});
 		expect(dockerImageInspect).toHaveBeenCalledWith("/custom/docker/path", {
-			imageTag: `${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
 			formatString:
 				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
 		});
@@ -94,7 +94,7 @@ describe("buildAndMaybePush", () => {
 			buildCmd: [
 				"build",
 				"-t",
-				`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+				`${getCloudflareContainerRegistry()}/test-app:tag`,
 				"--platform",
 				"linux/amd64",
 				"--provenance=false",
@@ -104,14 +104,26 @@ describe("buildAndMaybePush", () => {
 			],
 			dockerfile,
 		});
-		expect(runDockerCmd).toHaveBeenCalledTimes(1);
-		expect(runDockerCmd).toHaveBeenCalledWith("docker", [
+
+		// 3 calls: docker tag + docker push + docker rm
+		expect(runDockerCmd).toHaveBeenCalledTimes(3);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
+			"tag",
+			`${getCloudflareContainerRegistry()}/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).toHaveBeenCalledOnce();
 		expect(dockerImageInspect).toHaveBeenCalledWith("docker", {
-			imageTag: `${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
 			formatString:
 				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
 		});
@@ -121,7 +133,7 @@ describe("buildAndMaybePush", () => {
 	it("should be able to build image and not push if it already exists in remote", async () => {
 		vi.mocked(runDockerCmd).mockResolvedValueOnce();
 		vi.mocked(dockerImageInspect).mockResolvedValue(
-			'53387881 2 ["registry.cloudflare.com/some-account-id/test-app@sha256:three"]'
+			'53387881 2 ["registry.cloudflare.com/test-app@sha256:three"]'
 		);
 		await runWrangler(
 			"containers build ./container-context -t test-app:tag -p"
@@ -130,7 +142,7 @@ describe("buildAndMaybePush", () => {
 			buildCmd: [
 				"build",
 				"-t",
-				`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+				`${getCloudflareContainerRegistry()}/test-app:tag`,
 				"--platform",
 				"linux/amd64",
 				"--provenance=false",
@@ -154,11 +166,11 @@ describe("buildAndMaybePush", () => {
 		expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
 			"image",
 			"rm",
-			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+			`${getCloudflareContainerRegistry()}/test-app:tag`,
 		]);
 		expect(dockerImageInspect).toHaveBeenCalledOnce();
 		expect(dockerImageInspect).toHaveBeenCalledWith("docker", {
-			imageTag: `${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
 			formatString:
 				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
 		});
@@ -172,7 +184,7 @@ describe("buildAndMaybePush", () => {
 			buildCmd: [
 				"build",
 				"-t",
-				`${getCloudflareContainerRegistry()}/some-account-id/test-app`,
+				`${getCloudflareContainerRegistry()}/test-app`,
 				"--platform",
 				"linux/amd64",
 				"--provenance=false",
@@ -194,7 +206,7 @@ describe("buildAndMaybePush", () => {
 			buildCmd: [
 				"build",
 				"-t",
-				`${getCloudflareContainerRegistry()}/some-account-id/test-app`,
+				`${getCloudflareContainerRegistry()}/test-app`,
 				"--platform",
 				"linux/amd64",
 				"--provenance=false",
@@ -270,27 +282,24 @@ describe("buildAndMaybePush", () => {
 	});
 
 	describe("resolveAppDiskSize", () => {
-		const accountBase = {
-			limits: { disk_mb_per_deployment: 2000 },
-		} as CompleteAccountCustomer;
 		it("should return parsed app disk size", () => {
-			const result = resolveAppDiskSize(accountBase, {
+			const result = resolveAppDiskSize({
 				...defaultConfiguration,
 				configuration: { image: "", disk: { size: "500MB" } },
 			});
 			expect(result).toBeCloseTo(500 * 1000 * 1000, -5);
 		});
 
 		it("should return default size when disk size not set", () => {
-			const result = resolveAppDiskSize(accountBase, {
+			const result = resolveAppDiskSize({
 				...defaultConfiguration,
 				configuration: { image: "" },
 			});
 			expect(result).toBeCloseTo(2 * 1000 * 1000 * 1000, -5);
 		});
 
 		it("should return undefined if app is not passed", () => {
-			expect(resolveAppDiskSize(accountBase, undefined)).toBeUndefined();
+			expect(resolveAppDiskSize(undefined)).toBeUndefined();
 		});
 	});
 });

packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts

@@ -7,6 +7,7 @@ import {
 } from "@cloudflare/containers-shared";
 import { http, HttpResponse } from "msw";
 import { maybeBuildContainer } from "../../cloudchamber/deploy";
+import { clearCachedAccount } from "../../cloudchamber/locations";
 import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
 import { mockConsoleMethods } from "../helpers/mock-console";
 import { mockLegacyScriptData } from "../helpers/mock-legacy-script";
@@ -33,7 +34,7 @@ vi.mock("node:child_process");
 describe("maybeBuildContainer", () => {
 	it("Should return imageUpdate: true if using an image URI", async () => {
 		const config = {
-			image: "registry.cloudflare.com/some-account-id/some-image:uri",
+			image: "registry.cloudflare.com/some-image:uri",
 			class_name: "Test",
 		};
 		const result = await maybeBuildContainer(
@@ -121,8 +122,10 @@ describe("wrangler deploy with containers", () => {
 			)
 			.mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy"))
 			.mockImplementationOnce(mockDockerLogin("mockpassword"))
-			.mockImplementationOnce(mockDockerManifestInspect("my-container", true))
-			.mockImplementationOnce(mockDockerPush("my-container", "Galaxy"));
+			.mockImplementationOnce(mockDockerManifestInspect("some-account-id/my-container", true))
+			.mockImplementationOnce(mockDockerTag("my-container", "some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerPush("some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerImageDelete("some-account-id/my-container", "Galaxy"));
 
 		writeWranglerConfig({
 			durable_objects: {
@@ -247,8 +250,10 @@ describe("wrangler deploy with containers", () => {
 			)
 			.mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy"))
 			.mockImplementationOnce(mockDockerLogin("mockpassword"))
-			.mockImplementationOnce(mockDockerManifestInspect("my-container", true))
-			.mockImplementationOnce(mockDockerPush("my-container", "Galaxy"));
+			.mockImplementationOnce(mockDockerManifestInspect("some-account-id/my-container", true))
+			.mockImplementationOnce(mockDockerTag("my-container", "some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerPush("some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerImageDelete("some-account-id/my-container", "Galaxy"));
 
 		mockContainersAccount();
 
@@ -336,8 +341,10 @@ describe("wrangler deploy with containers", () => {
 			)
 			.mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy"))
 			.mockImplementationOnce(mockDockerLogin("mockpassword"))
-			.mockImplementationOnce(mockDockerManifestInspect("my-container", true))
-			.mockImplementationOnce(mockDockerPush("my-container", "Galaxy"));
+			.mockImplementationOnce(mockDockerManifestInspect("some-account-id/my-container", true))
+			.mockImplementationOnce(mockDockerTag("my-container", "some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerPush("some-account-id/my-container", "Galaxy"))
+			.mockImplementationOnce(mockDockerImageDelete("some-account-id/my-container", "Galaxy"));
 
 		fs.mkdirSync("nested/src", { recursive: true });
 		fs.writeFileSync("Dockerfile", "FROM alpine");
@@ -437,6 +444,73 @@ describe("wrangler deploy with containers", () => {
 	});
 });
 
+// This is a separate describe block because we intentionally do not mock any
+// API tokens, account ID, or authorization. The purpose of these tests is to
+// ensure that --dry-run mode works without requiring any API access.
+describe("wrangler deploy with containers dry run", () => {
+	runInTempDir();
+	const std = mockConsoleMethods();
+
+	beforeEach(() => {
+		clearCachedAccount();
+	});
+
+	afterEach(() => {
+		vi.unstubAllEnvs();
+	});
+
+	it("builds the image without pushing", async () => {
+		vi.mocked(spawn)
+			.mockImplementationOnce(mockDockerInfo())
+			.mockImplementationOnce(
+				mockDockerBuild("my-container", "worker", "FROM scratch", process.cwd())
+			)
+			.mockImplementationOnce(mockDockerImageInspect("my-container", "worker"))
+			.mockImplementationOnce(mockDockerLogin("mockpassword"))
+			.mockImplementationOnce(mockDockerManifestInspect("my-container", true))
+			.mockImplementationOnce(mockDockerPush("my-container", "worker"));
+
+		vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker");
+
+		fs.writeFileSync("Dockerfile", "FROM scratch");
+		fs.writeFileSync(
+			"index.js",
+			`export class ExampleDurableObject {}; export default{};`
+		);
+
+		writeWranglerConfig({
+			durable_objects: {
+				bindings: [
+					{
+						name: "EXAMPLE_DO_BINDING",
+						class_name: "ExampleDurableObject",
+					},
+				],
+			},
+			containers: [
+				{
+					image: "./Dockerfile",
+					name: "my-container",
+					instances: 10,
+					class_name: "ExampleDurableObject",
+				},
+			],
+			migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }],
+		});
+
+		await runWrangler("deploy --dry-run index.js");
+		expect(std.out).toMatchInlineSnapshot(`
+			"Total Upload: xx KiB / gzip: xx KiB
+			Building image my-container:worker
+			Your Worker has access to the following bindings:
+			Binding                                            Resource
+			env.EXAMPLE_DO_BINDING (ExampleDurableObject)      Durable Object
+			
+			--dry-run: exiting now."
+		`);
+	});
+});
+
 function mockGetVersion(versionId: string) {
 	msw.use(
 		http.get(
@@ -549,7 +623,7 @@ function mockDockerBuild(
 		expect(args).toEqual([
 			"build",
 			"-t",
-			`${getCloudflareContainerRegistry()}/some-account-id/${containerName}:${tag}`,
+			`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
 			"--platform",
 			"linux/amd64",
 			"--provenance=false",
@@ -590,7 +664,7 @@ function mockDockerImageInspect(containerName: string, tag: string) {
 		expect(args).toEqual([
 			"image",
 			"inspect",
-			`${getCloudflareContainerRegistry()}/some-account-id/${containerName}:${tag}`,
+			`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
 			"--format",
 			"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
 		]);
@@ -612,14 +686,26 @@ function mockDockerImageInspect(containerName: string, tag: string) {
 		setImmediate(() => {
 			stdout.emit(
 				"data",
-				`123456 4 ["${getCloudflareContainerRegistry()}/some-account-id/${containerName}@sha256:three"]`
+				`123456 4 ["${getCloudflareContainerRegistry()}/${containerName}@sha256:three"]`
 			);
 		});
 
 		return child as unknown as ChildProcess;
 	};
 }
 
+function mockDockerImageDelete(containerName: string, tag: string) {
+	return (cmd: string, args: readonly string[]) => {
+		expect(cmd).toBe("/usr/bin/docker");
+		expect(args).toEqual([
+			"image",
+			"rm",
+			`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
+		]);
+		return defaultChildProcess();
+	};
+}
+
 function mockDockerLogin(expectedPassword: string) {
 	return (cmd: string, _args: readonly string[]) => {
 		expect(cmd).toBe("/usr/bin/docker");
@@ -653,7 +739,7 @@ function mockDockerManifestInspect(containerName: string, shouldFail = true) {
 		expect(args).toEqual([
 			"manifest",
 			"inspect",
-			`${getCloudflareContainerRegistry()}/some-account-id/${containerName}@three`,
+			`${getCloudflareContainerRegistry()}/${containerName}@three`,
 		]);
 		const readable = new Writable({
 			write() {},
@@ -679,7 +765,19 @@ function mockDockerPush(containerName: string, tag: string) {
 		expect(cmd).toBe("/usr/bin/docker");
 		expect(args).toEqual([
 			"push",
-			`${getCloudflareContainerRegistry()}/some-account-id/${containerName}:${tag}`,
+			`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
+		]);
+		return defaultChildProcess();
+	};
+}
+
+function mockDockerTag(from: string, to: string, tag: string) {
+	return (cmd: string, args: readonly string[]) => {
+		expect(cmd).toBe("/usr/bin/docker");
+		expect(args).toEqual([
+			"tag",
+			`${getCloudflareContainerRegistry()}/${from}:${tag}`,
+			`${getCloudflareContainerRegistry()}/${to}:${tag}`,
 		]);
 		return defaultChildProcess();
 	};