refactor dev to use normalised config

emily-shen · emily-shen · commit a162735d06d5 · 2025-07-15T15:53:24.000+01:00
diff --git a/packages/containers-shared/src/build.ts b/packages/containers-shared/src/build.ts
@@ -1,12 +1,9 @@
 import { spawn } from "child_process";
 import { readFileSync } from "fs";
-import path from "path";
-import { BuildArgs, ContainerDevOptions, Logger } from "./types";
+import { BuildArgs, DockerfileConfig, Logger } from "./types";
 
 export async function constructBuildCommand(
 	options: BuildArgs,
-	/** wrangler config path. used to resolve relative dockerfile path */
-	configPath: string | undefined,
 	logger?: Logger
 ) {
 	const platform = options.platform ?? "linux/amd64";
@@ -27,12 +24,11 @@ export async function constructBuildCommand(
 	if (options.setNetworkToHost) {
 		buildCmd.push("--network", "host");
 	}
-	const baseDir = configPath ? path.dirname(configPath) : process.cwd();
-	const absDockerfilePath = path.resolve(baseDir, options.pathToDockerfile);
-	const dockerfile = readFileSync(absDockerfilePath, "utf-8");
+
+	const dockerfile = readFileSync(options.pathToDockerfile, "utf-8");
 	// pipe in the dockerfile
 	buildCmd.push("-f", "-");
-	buildCmd.push(options.buildContext ?? path.dirname(absDockerfilePath));
+	buildCmd.push(options.buildContext);
 	logger?.debug(`Building image with command: ${buildCmd.join(" ")}`);
 	return { buildCmd, dockerfile };
 }
@@ -92,20 +88,16 @@ export function dockerBuild(
 
 export async function buildImage(
 	dockerPath: string,
-	options: ContainerDevOptions,
-	configPath: string | undefined
+	options: DockerfileConfig,
+	imageTag: string
 ) {
-	// just let the tag default to latest
-	const { buildCmd, dockerfile } = await constructBuildCommand(
-		{
-			tag: options.imageTag,
-			pathToDockerfile: options.image,
-			buildContext: options.imageBuildContext,
-			args: options.args,
-			platform: "linux/amd64",
-		},
-		configPath
-	);
+	const { buildCmd, dockerfile } = await constructBuildCommand({
+		tag: imageTag,
+		pathToDockerfile: options.dockerfile,
+		buildContext: options.image_build_context,
+		args: options.image_vars,
+		platform: "linux/amd64",
+	});
 
 	return dockerBuild(dockerPath, { buildCmd, dockerfile });
 }
diff --git a/packages/containers-shared/src/images.ts b/packages/containers-shared/src/images.ts
@@ -1,33 +1,34 @@
 import { buildImage } from "./build";
 import {
 	getCloudflareContainerRegistry,
+	getDevContainerImageName,
 	isCloudflareRegistryLink,
 } from "./knobs";
 import { dockerLoginManagedRegistry } from "./login";
-import { ContainerDevOptions } from "./types";
+import { ContainerNormalisedConfig, RegistryLinkConfig } from "./types";
 import {
 	checkExposedPorts,
-	isDockerfile,
 	runDockerCmd,
 	verifyDockerInstalled,
 } from "./utils";
 
 export async function pullImage(
 	dockerPath: string,
-	options: ContainerDevOptions
+	options: RegistryLinkConfig,
+	tag: string
 ): Promise<{ abort: () => void; ready: Promise<void> }> {
 	await dockerLoginManagedRegistry(dockerPath);
 	const pull = runDockerCmd(dockerPath, [
 		"pull",
-		options.image,
+		options.registry_link,
 		// All containers running on our platform need to be built for amd64 architecture, but by default docker pull seems to look for an image matching the host system, so we need to specify this here
 		"--platform",
 		"linux/amd64",
 	]);
 	const ready = pull.ready.then(async ({ aborted }: { aborted: boolean }) => {
 		if (!aborted) {
 			// re-tag image with the expected dev-formatted image tag for consistency
-			await runDockerCmd(dockerPath, ["tag", options.image, options.imageTag]);
+			await runDockerCmd(dockerPath, ["tag", options.registry_link, tag]);
 		}
 	});
 
@@ -51,14 +52,14 @@ export async function pullImage(
  */
 export async function prepareContainerImagesForDev(
 	dockerPath: string,
-	containerOptions: ContainerDevOptions[],
-	configPath: string | undefined,
+	containerOptions: ContainerNormalisedConfig[],
+	containerBuildId: string,
 	onContainerImagePreparationStart: (args: {
-		containerOptions: ContainerDevOptions;
+		containerOptions: ContainerNormalisedConfig;
 		abort: () => void;
 	}) => void,
 	onContainerImagePreparationEnd: (args: {
-		containerOptions: ContainerDevOptions;
+		containerOptions: ContainerNormalisedConfig;
 	}) => void
 ) {
 	let aborted = false;
@@ -69,8 +70,9 @@ export async function prepareContainerImagesForDev(
 	}
 	await verifyDockerInstalled(dockerPath);
 	for (const options of containerOptions) {
-		if (isDockerfile(options.image, configPath)) {
-			const build = await buildImage(dockerPath, options, configPath);
+		const tag = getDevContainerImageName(options.class_name, containerBuildId);
+		if ("dockerfile" in options) {
+			const build = await buildImage(dockerPath, options, tag);
 			onContainerImagePreparationStart({
 				containerOptions: options,
 				abort: () => {
@@ -83,13 +85,13 @@ export async function prepareContainerImagesForDev(
 				containerOptions: options,
 			});
 		} else {
-			if (!isCloudflareRegistryLink(options.image)) {
+			if (!isCloudflareRegistryLink(options.registry_link)) {
 				throw new Error(
-					`Image "${options.image}" is a registry link but does not point to the Cloudflare container registry.\n` +
+					`Image "${options.registry_link}" is a registry link but does not point to the Cloudflare container registry.\n` +
 						`To use an existing image from another repository, see https://developers.cloudflare.com/containers/image-management/#using-existing-images`
 				);
 			}
-			const pull = await pullImage(dockerPath, options);
+			const pull = await pullImage(dockerPath, options, tag);
 			onContainerImagePreparationStart({
 				containerOptions: options,
 				abort: () => {
@@ -103,7 +105,7 @@ export async function prepareContainerImagesForDev(
 			});
 		}
 		if (!aborted) {
-			await checkExposedPorts(dockerPath, options);
+			await checkExposedPorts(dockerPath, options, tag);
 		}
 	}
 }
diff --git a/packages/containers-shared/src/types.ts b/packages/containers-shared/src/types.ts
@@ -17,7 +17,7 @@ export type BuildArgs = {
 	tag: string;
 	pathToDockerfile: string;
 	/** image_build_context or args.PATH. if not provided, defaults to the dockerfile directory */
-	buildContext?: string;
+	buildContext: string;
 	/** any env vars that should be passed in at build time */
 	args?: Record<string, string>;
 	/** platform to build for. defaults to linux/amd64 */
@@ -50,19 +50,6 @@ export type InstanceTypeOrLimits =
 			instance_type: InstanceType;
 	  };
 
-/** build/pull agnostic container options */
-export type ContainerDevOptions = {
-	/** may be dockerfile or registry link */
-	image: string;
-	/** formatted as cloudflare-dev/workername-DOclassname:build-id */
-	imageTag: string;
-	/** container's DO class name */
-	class_name: string;
-	imageBuildContext?: string;
-	/** build time args */
-	args?: Record<string, string>;
-};
-
 /**
  * Shared container config that is used regardless of whether the image is from a dockerfile or a registry link.
  */
diff --git a/packages/containers-shared/src/utils.ts b/packages/containers-shared/src/utils.ts
@@ -2,7 +2,7 @@ import { execFile, spawn, StdioOptions } from "child_process";
 import { existsSync, statSync } from "fs";
 import path from "path";
 import { dockerImageInspect } from "./inspect";
-import { ContainerDevOptions } from "./types";
+import { ContainerNormalisedConfig } from "./types";
 
 /** helper for simple docker command call that don't require any io handling */
 export const runDockerCmd = (
@@ -201,10 +201,11 @@ const getContainerIdsFromImage = async (
  */
 export async function checkExposedPorts(
 	dockerPath: string,
-	options: ContainerDevOptions
+	options: ContainerNormalisedConfig,
+	imageTag: string
 ) {
 	const output = await dockerImageInspect(dockerPath, {
-		imageTag: options.imageTag,
+		imageTag: imageTag,
 		formatString: "{{ len .Config.ExposedPorts }}",
 	});
 	if (output === "0") {
diff --git a/packages/containers-shared/tests/utils.test.ts b/packages/containers-shared/tests/utils.test.ts
@@ -1,4 +1,5 @@
 import { mkdirSync, writeFileSync } from "fs";
+import { ContainerNormalisedConfig } from "../src/types";
 import { checkExposedPorts, isDockerfile } from "./../src/utils";
 import { runInTempDir } from "./helpers/run-in-tmp-dir";
 
@@ -71,6 +72,10 @@ vi.mock("../src/inspect", async (importOriginal) => {
 	};
 });
 
+const containerConfig = {
+	dockerfile: "",
+	class_name: "MyContainer",
+} as ContainerNormalisedConfig;
 describe("checkExposedPorts", () => {
 	beforeEach(() => {
 		docketImageInspectResult = "1";
@@ -79,23 +84,14 @@ describe("checkExposedPorts", () => {
 	it("should not error when some ports are exported", async () => {
 		docketImageInspectResult = "1";
 		await expect(
-			checkExposedPorts("./container-context/Dockerfile", {
-				image: "",
-				imageTag: "",
-				class_name: "MyContainer",
-			})
+			checkExposedPorts("docker", containerConfig, "image:tag")
 		).resolves.toBeUndefined();
 	});
 
 	it("should error, with an appropriate message when no ports are exported", async () => {
 		docketImageInspectResult = "0";
-		expect(
-			checkExposedPorts("./container-context/Dockerfile", {
-				image: "",
-				imageTag: "",
-				class_name: "MyContainer",
-			})
-		).rejects.toThrowErrorMatchingInlineSnapshot(`
+		expect(checkExposedPorts("docker", containerConfig, "image:tag")).rejects
+			.toThrowErrorMatchingInlineSnapshot(`
 				[Error: The container "MyContainer" does not expose any ports. In your Dockerfile, please expose any ports you intend to connect to.
 				For additional information please see: https://developers.cloudflare.com/containers/local-dev/#exposing-ports.
 				]
diff --git a/packages/wrangler/src/api/startDevWorker/ConfigController.ts b/packages/wrangler/src/api/startDevWorker/ConfigController.ts
@@ -1,11 +1,11 @@
 import assert from "node:assert";
 import path from "node:path";
-import { isDockerfile } from "@cloudflare/containers-shared";
 import { watch } from "chokidar";
 import { getAssetsOptions, validateAssetsArgsAndConfig } from "../../assets";
 import { fillOpenAPIConfiguration } from "../../cloudchamber/common";
 import { readConfig } from "../../config";
 import { containersScope } from "../../containers";
+import { getNormalizedContainerOptions } from "../../containers/config";
 import { getEntry } from "../../deployment-bundle/entry";
 import {
 	getBindings,
@@ -347,7 +347,7 @@ async function resolveConfig(
 			tsconfig: input.build?.tsconfig ?? config.tsconfig,
 			exports: entry.exports,
 		},
-		containers: config.containers,
+		containers: await getNormalizedContainerOptions(config),
 		dev: await resolveDevConfig(config, input),
 		legacy: {
 			site: legacySite,
@@ -399,10 +399,10 @@ async function resolveConfig(
 	// for pulling containers, we need to make sure the OpenAPI config for the
 	// container API client is properly set so that we can get the correct permissions
 	// from the cloudchamber API to pull from the repository.
-	const needsPulling = resolved.containers?.some(
-		(c) => !isDockerfile(c.image ?? c.configuration?.image, config.configPath)
+	const needsPulling = resolved.containers.some(
+		(c) => "registry_link" in c && c.registry_link
 	);
-	if (needsPulling && !resolved.dev.remote) {
+	if (resolved.containers.length && needsPulling && !resolved.dev.remote) {
 		await fillOpenAPIConfiguration(config, containersScope);
 	}
 
diff --git a/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts
@@ -27,7 +27,7 @@ import type {
 	ReloadStartEvent,
 } from "./events";
 import type { Binding, File, StartDevWorkerOptions } from "./types";
-import type { ContainerDevOptions } from "@cloudflare/containers-shared";
+import type { ContainerNormalisedConfig } from "@cloudflare/containers-shared";
 
 async function getBinaryFileContents(file: File<string | Uint8Array>) {
 	if ("contents" in file) {
@@ -183,7 +183,7 @@ export class LocalRuntimeController extends RuntimeController {
 	// Used to store the information and abort handle for the
 	// current container that is being built
 	containerBeingBuilt?: {
-		containerOptions: ContainerDevOptions;
+		containerOptions: ContainerNormalisedConfig;
 		abort: () => void;
 		abortRequested: boolean;
 	};
@@ -219,21 +219,30 @@ export class LocalRuntimeController extends RuntimeController {
 			}
 
 			// Assemble container options and build if necessary
-			const containerOptions = await getContainerOptions(data.config);
-			this.#dockerPath = data.config.dev?.dockerPath ?? getDockerPath();
-			// keep track of them so we can clean up later
-			for (const container of containerOptions ?? []) {
-				this.#containerImageTagsSeen.add(container.imageTag);
-			}
+
 			if (
-				containerOptions &&
+				data.config.containers?.length &&
+				data.config.dev.enableContainers &&
 				this.#currentContainerBuildId !== data.config.dev.containerBuildId
 			) {
+				this.#dockerPath = data.config.dev?.dockerPath ?? getDockerPath();
+				assert(
+					data.config.dev.containerBuildId,
+					"Build ID should be set if containers are enabled and defined"
+				);
+				for (const container of data.config.containers) {
+					this.#containerImageTagsSeen.add(
+						getDevContainerImageName(
+							container.class_name,
+							data.config.dev.containerBuildId
+						)
+					);
+				}
 				logger.log(chalk.dim("⎔ Preparing container image(s)..."));
 				await prepareContainerImagesForDev(
 					this.#dockerPath,
-					containerOptions,
-					data.config.config,
+					data.config.containers,
+					data.config.dev.containerBuildId,
 					(buildStartEvent) => {
 						this.containerBeingBuilt = {
 							...buildStartEvent,
@@ -421,35 +430,3 @@ export class LocalRuntimeController extends RuntimeController {
 		this.emit("reloadComplete", data);
 	}
 }
-
-/**
- * @returns Container options suitable for building or pulling images,
- * with image tag set to well-known dev format.
- * Undefined if containers are not enabled or not configured.
- */
-export async function getContainerOptions(
-	config: BundleCompleteEvent["config"]
-) {
-	if (!config.containers?.length || config.dev.enableContainers === false) {
-		return undefined;
-	}
-	// should be defined if containers are enabled
-	assert(
-		config.dev.containerBuildId,
-		"Build ID should be set if containers are enabled and defined"
-	);
-	const containers: ContainerDevOptions[] = [];
-	for (const container of config.containers) {
-		containers.push({
-			image: container.image ?? container.configuration?.image,
-			imageTag: getDevContainerImageName(
-				container.class_name,
-				config.dev.containerBuildId
-			),
-			args: container.image_vars,
-			imageBuildContext: container.image_build_context,
-			class_name: container.class_name,
-		});
-	}
-	return containers;
-}
diff --git a/packages/wrangler/src/api/startDevWorker/types.ts b/packages/wrangler/src/api/startDevWorker/types.ts
diff --git a/packages/wrangler/src/dev/miniflare.ts b/packages/wrangler/src/dev/miniflare.ts
