Resolve container image name when modifying (#9923)

The container image name should be resolved into the full (namespaced)
image name both when creating an application and when modifying an
application.

Move the image name resolution out of the create path and into the
shared code path taken for both creation and modification.

Author: gpanders

.changeset/quiet-mangos-wave.md

@@ -0,0 +1,6 @@
+---
+"@cloudflare/containers-shared": patch
+"wrangler": patch
+---
+
+Fix image name resolution when modifying a container application

packages/containers-shared/src/images.ts

@@ -114,10 +114,7 @@ export async function prepareContainerImagesForDev(
  * 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> {
+export function resolveImageName(accountId: string, image: string): string {
 	let url: URL;
 	try {
 		url = new URL(`http://${image}`);

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

@@ -1858,7 +1858,7 @@ describe("cloudchamber apply", () => {
 		expect(app.configuration?.instance_type).toEqual("dev");
 	});
 
-	test("expands image names from managed registry", async () => {
+	test("expands image names from managed registry when creating an application", async () => {
 		setIsTTY(false);
 		const registry = getCloudflareContainerRegistry();
 		writeWranglerConfig({
@@ -1904,7 +1904,7 @@ describe("cloudchamber apply", () => {
 			│   tier = 2
 			│
 			│   [containers.configuration]
-			│   image = \\"${registry}/hello:1.0\\"
+			│   image = \\"${registry}/some-account-id/hello:1.0\\"
 			│   instance_type = \\"dev\\"
 			│
 			│
@@ -1915,4 +1915,79 @@ describe("cloudchamber apply", () => {
 			"
 		`);
 	});
+
+	test("expands image names from managed registry when modifying an application", async () => {
+		setIsTTY(false);
+		const registry = getCloudflareContainerRegistry();
+		writeWranglerConfig({
+			name: "my-container",
+			containers: [
+				{
+					name: "my-container-app",
+					instances: 3,
+					class_name: "DurableObjectClass",
+					image: `${registry}/hello:1.0`,
+					instance_type: "standard",
+					constraints: {
+						tier: 2,
+					},
+				},
+			],
+		});
+
+		mockGetApplications([
+			{
+				id: "abc",
+				name: "my-container-app",
+				instances: 3,
+				created_at: new Date().toString(),
+				version: 1,
+				account_id: "1",
+				scheduling_policy: SchedulingPolicy.REGIONAL,
+				configuration: {
+					image: `${registry}/some-account-id/hello:1.0`,
+					disk: {
+						size: "2GB",
+						size_mb: 2000,
+					},
+					vcpu: 0.0625,
+					memory: "256MB",
+					memory_mib: 256,
+				},
+				constraints: {
+					tier: 3,
+				},
+			},
+		]);
+
+		const applicationReqBodyPromise = mockModifyApplication();
+		await runWrangler("cloudchamber apply");
+		expect(std.stdout).toMatchInlineSnapshot(`
+			"╭ Deploy a container application deploy changes to your application
+			│
+			│ Container application changes
+			│
+			├ EDIT my-container-app
+			│
+			│   [containers.configuration]
+			│   image = \\"${registry}/some-account-id/hello:1.0\\"
+			│ - instance_type = \\"dev\\"
+			│ + instance_type = \\"standard\\"
+			│
+			│   [containers.constraints]
+			│   ...
+			│ - tier = 3
+			│ + tier = 2
+			│
+			│
+			│  SUCCESS  Modified application my-container-app
+			│
+			╰ Applied changes
+
+			"
+		`);
+		expect(std.stderr).toMatchInlineSnapshot(`""`);
+		const app = await applicationReqBodyPromise;
+		expect(app.configuration?.instance_type).toEqual("standard");
+	});
 });

packages/wrangler/src/cloudchamber/apply.ts

@@ -93,10 +93,14 @@ function createApplicationToModifyApplication(
 }
 
 function applicationToCreateApplication(
+	accountId: string,
 	application: Application
 ): CreateApplicationRequest {
 	const app: CreateApplicationRequest = {
-		configuration: application.configuration,
+		configuration: {
+			...application.configuration,
+			image: resolveImageName(accountId, application.configuration.image),
+		},
 		constraints: application.constraints,
 		max_instances: application.max_instances,
 		name: application.name,
@@ -194,6 +198,7 @@ function containerAppToInstanceType(
 }
 
 function containerAppToCreateApplication(
+	accountId: string,
 	containerApp: ContainerApp,
 	observability: Observability | undefined,
 	existingApp: Application | undefined,
@@ -216,10 +221,15 @@ function containerAppToCreateApplication(
 			telemetryMessage: true,
 		});
 	}
+
 	const app: CreateApplicationRequest = {
 		...containerApp,
 		name: containerApp.name,
-		configuration,
+		configuration: {
+			...configuration,
+			// De-sugar image name
+			image: resolveImageName(accountId, configuration.image),
+		},
 		instances: containerApp.instances ?? 0,
 		scheduling_policy:
 			(containerApp.scheduling_policy as SchedulingPolicy) ??
@@ -506,7 +516,9 @@ export async function apply(
 			appConfigNoDefaults.configuration.image = application.configuration.image;
 		}
 
+		const accountId = config.account_id || (await getAccountId(config));
 		const appConfig = containerAppToCreateApplication(
+			accountId,
 			appConfigNoDefaults,
 			config.observability,
 			application,
@@ -517,7 +529,7 @@ export async function apply(
 			// we need to sort the objects (by key) because the diff algorithm works with
 			// lines
 			const prevApp = sortObjectRecursive<CreateApplicationRequest>(
-				stripUndefined(applicationToCreateApplication(application))
+				stripUndefined(applicationToCreateApplication(accountId, application))
 			);
 
 			// fill up fields that their defaults were changed over-time,
@@ -688,18 +700,7 @@ export async function apply(
 				printLine(el, "  ");
 			});
 
-		const accountId = config.account_id || (await getAccountId(config));
-		const configToPush = {
-			...appConfig,
-
-			configuration: {
-				...appConfig.configuration,
-
-				// De-sugar image name. We do it here so that the user
-				// sees the simplified image name in diffs.
-				image: await resolveImageName(accountId, appConfig.configuration.image),
-			},
-		};
+		const configToPush = { ...appConfig };
 
 		// add to the actions array to create the app later
 		actions.push({