Resolve dockerfile to absolute path in redirected config in vite plugin (#10038)

* resolve dockerfile to absolute path in redirected config

* changeset

* resolve buildContext to absolute path in wrangler too

* resolve buildContext to absolute path in vite plugin

Author: emily-shen

.changeset/rich-bottles-remain.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/vite-plugin": patch
+---
+
+Resolve `containers.image` (if it is a path to a Dockerfile) to an absolute path in the deploy config.

packages/containers-shared/src/build.ts

@@ -30,9 +30,13 @@ export async function constructBuildCommand(
 	const baseDir = configPath ? path.dirname(configPath) : process.cwd();
 	const absDockerfilePath = path.resolve(baseDir, options.pathToDockerfile);
 	const dockerfile = readFileSync(absDockerfilePath, "utf-8");
+
+	const absBuildContext = options.buildContext
+		? path.resolve(baseDir, options.buildContext)
+		: path.dirname(absDockerfilePath);
 	// pipe in the dockerfile
 	buildCmd.push("-f", "-");
-	buildCmd.push(options.buildContext ?? path.dirname(absDockerfilePath));
+	buildCmd.push(absBuildContext);
 	logger?.debug(`Building image with command: ${buildCmd.join(" ")}`);
 	return { buildCmd, dockerfile };
 }

packages/vite-plugin-cloudflare/src/index.ts

@@ -6,6 +6,7 @@ import { getDevContainerImageName } from "@cloudflare/containers-shared/src/knob
 import {
 	generateContainerBuildId,
 	getContainerIdsByImageTags,
+	isDockerfile,
 } from "@cloudflare/containers-shared/src/utils";
 import { generateStaticRoutingRuleMatcher } from "@cloudflare/workers-shared/asset-worker/src/utils/rules-engine";
 import replace from "@rollup/plugin-replace";
@@ -263,6 +264,45 @@ if (import.meta.hot) {
 								path.resolve(resolvedViteConfig.root, clientOutputDirectory)
 							),
 						};
+
+						if (workerConfig.containers?.length) {
+							workerConfig.containers = workerConfig.containers.map(
+								(container) => {
+									const baseDir = workerConfig.configPath
+										? path.dirname(workerConfig.configPath)
+										: resolvedViteConfig.root;
+									// Wrangler's config validation resolves to container.configuration.image, even though it is deprecated
+									const image =
+										container.configuration?.image ?? container.image;
+									if (isDockerfile(image, workerConfig.configPath)) {
+										const output = {
+											...container,
+											image: path.resolve(baseDir, image),
+											...(container.image_build_context
+												? {
+														image_build_context: path.resolve(
+															baseDir,
+															container.image_build_context
+														),
+													}
+												: {}),
+										};
+										// however, wrangler deploy will re-resolve the config, so we should
+										// deduplicate the container.configuration.image field in favour of
+										// the non-deprecated one when we write out the deploy config
+										delete output.configuration?.image;
+										// if we don't do this, we get a warning that container.configuration is deprecated
+										if (
+											output.configuration &&
+											Object.keys(output.configuration).length === 0
+										) {
+											delete output.configuration;
+										}
+										return output;
+									} else return container;
+								}
+							);
+						}
 					} else {
 						workerConfig.assets = undefined;
 					}

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

@@ -1,4 +1,5 @@
 import { mkdirSync, writeFileSync } from "fs";
+import path from "path";
 import {
 	dockerBuild,
 	dockerImageInspect,
@@ -68,7 +69,7 @@ describe("buildAndMaybePush", () => {
 				"--provenance=false",
 				"-f",
 				"-",
-				"./container-context",
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});
@@ -100,7 +101,8 @@ describe("buildAndMaybePush", () => {
 				"--provenance=false",
 				"-f",
 				"-",
-				"./container-context",
+				// turn this into a relative path so that this works across different OSes
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});
@@ -151,7 +153,7 @@ describe("buildAndMaybePush", () => {
 				"--provenance=false",
 				"-f",
 				"-",
-				"./container-context",
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});
@@ -193,7 +195,7 @@ describe("buildAndMaybePush", () => {
 				"--provenance=false",
 				"-f",
 				"-",
-				"./container-context",
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});
@@ -217,7 +219,7 @@ describe("buildAndMaybePush", () => {
 				"host",
 				"-f",
 				"-",
-				"./container-context",
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});
@@ -237,7 +239,7 @@ describe("buildAndMaybePush", () => {
 				"--provenance=false",
 				"-f",
 				"-",
-				"./container-context",
+				path.resolve(process.cwd(), "./container-context"),
 			],
 			dockerfile,
 		});