Do not update container apps image if the image is unchanged (#9565)

Previously we would build the image and retag even if the only change
was the build-related metadata. This changes that to detect if the build
has resulted in a new image, and if it has not remove the added tag and
ensures the applications image is not updated.

Author: IRCody

.changeset/rare-wasps-cough.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+Ensure that a container applications image configuration is not updated if there were not changes to the image.

packages/containers-shared/index.ts

@@ -6,3 +6,4 @@ export * from "./src/utils";
 export * from "./src/types";
 export * from "./src/inspect";
 export * from "./src/registry";
+export * from "./src/images";

packages/containers-shared/src/build.ts

@@ -7,7 +7,14 @@ export async function constructBuildCommand(
 	logger?: Logger
 ) {
 	const platform = options.platform ?? "linux/amd64";
-	const buildCmd = ["build", "-t", options.tag, "--platform", platform];
+	const buildCmd = [
+		"build",
+		"-t",
+		options.tag,
+		"--platform",
+		platform,
+		"--provenance=false",
+	];
 
 	if (options.args) {
 		for (const arg in options.args) {

packages/containers-shared/src/images.ts

@@ -0,0 +1,24 @@
+import { execFile } from "child_process";
+
+// Returns a list of docker image ids matching the provided repository:[tag]
+export async function getDockerImageDigest(
+	dockerPath: string,
+	imageTag: string
+): Promise<string> {
+	return new Promise((resolve, reject) => {
+		execFile(
+			dockerPath,
+			["images", "--digests", "--format", "{{.Digest}}", imageTag],
+			(error, stdout, stderr) => {
+				if (error) {
+					return reject(
+						new Error(
+							`Failed getting docker image digest for image: ${imageTag} with error: ${error}.`
+						)
+					);
+				}
+				return resolve(stdout.trim());
+			}
+		);
+	});
+}

packages/containers-shared/src/login.ts

@@ -15,7 +15,7 @@ export async function dockerLoginManagedRegistry(pathToDocker: string) {
 			getCloudflareContainerRegistry(),
 			{
 				expiration_minutes: expirationMinutes,
-				permissions: ["push"] as ImageRegistryPermissions[],
+				permissions: ["push", "pull"] as ImageRegistryPermissions[],
 			}
 		);
 

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

@@ -65,6 +65,7 @@ describe("buildAndMaybePush", () => {
 				`${getCloudflareContainerRegistry()}/test_account_id/test-app:tag`,
 				"--platform",
 				"linux/amd64",
+				"--provenance=false",
 				"-f",
 				"-",
 				"./container-context",
@@ -95,6 +96,7 @@ describe("buildAndMaybePush", () => {
 				`${getCloudflareContainerRegistry()}/test_account_id/test-app:tag`,
 				"--platform",
 				"linux/amd64",
+				"--provenance=false",
 				"-f",
 				"-",
 				"./container-context",
@@ -124,6 +126,7 @@ describe("buildAndMaybePush", () => {
 				`${getCloudflareContainerRegistry()}/test_account_id/test-app`,
 				"--platform",
 				"linux/amd64",
+				"--provenance=false",
 				"-f",
 				"-",
 				"./container-context",
@@ -145,6 +148,7 @@ describe("buildAndMaybePush", () => {
 				`${getCloudflareContainerRegistry()}/test_account_id/test-app`,
 				"--platform",
 				"linux/amd64",
+				"--provenance=false",
 				"--network",
 				"host",
 				"-f",

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

@@ -1,5 +1,5 @@
 import { Buffer } from "node:buffer";
-import { spawn, spawnSync } from "node:child_process";
+import { execFile, spawn, spawnSync } from "node:child_process";
 import { randomFillSync } from "node:crypto";
 import * as fs from "node:fs";
 import * as path from "node:path";
@@ -8708,6 +8708,28 @@ addEventListener('fetch', event => {});`
 						},
 					} as unknown as ChildProcess;
 				}
+				vi.mocked(execFile)
+					// docker images first call
+					.mockImplementationOnce((cmd, args, callback) => {
+						expect(cmd).toBe("/usr/bin/docker");
+						expect(args).toEqual([
+							"images",
+							"--digests",
+							"--format",
+							"{{.Digest}}",
+							getCloudflareContainerRegistry() +
+								"/test_account_id/my-container:Galaxy",
+						]);
+						if (callback) {
+							const back = callback as (
+								error: Error | null,
+								stdout: string,
+								stderr: string
+							) => void;
+							back(null, "three\n", "");
+						}
+						return {} as ChildProcess;
+					});
 
 				vi.mocked(spawn)
 					// 1. docker build
@@ -8720,6 +8742,7 @@ addEventListener('fetch', event => {});`
 								"/test_account_id/my-container:Galaxy",
 							"--platform",
 							"linux/amd64",
+							"--provenance=false",
 							"-f",
 							"-",
 							".",
@@ -8808,7 +8831,40 @@ addEventListener('fetch', event => {});`
 							},
 						} as unknown as ChildProcess;
 					})
-					// 4. docker push
+					// 4. docker manifest inspect
+					.mockImplementationOnce((cmd, args) => {
+						expect(cmd).toBe("/usr/bin/docker");
+						expect(args[0]).toBe("manifest");
+						expect(args[1]).toBe("inspect");
+						expect(args[2]).toEqual("my-container@three");
+						expect(args).toEqual([
+							"manifest",
+							"inspect",
+							`${getCloudflareContainerRegistry()}/test_account_id/my-container@three`,
+						]);
+						const readable = new Writable({
+							write() {},
+							final() {},
+						});
+						return {
+							stdout: Buffer.from(
+								"i promise I am an unsuccessful docker manifest call"
+							),
+							stdin: readable,
+							on: function (
+								reason: string,
+								cbPassed: (code: number) => unknown
+							) {
+								if (reason === "close") {
+									// We always fail this for this test because this is meant to stop
+									// us pushing if it succeeds and we want to go through the push workflow also
+									cbPassed(1);
+								}
+								return this;
+							},
+						} as unknown as ChildProcess;
+					})
+					// 5. docker push
 					.mockImplementationOnce((cmd, args) => {
 						expect(cmd).toBe("/usr/bin/docker");
 						expect(args).toEqual([
@@ -8868,7 +8924,7 @@ addEventListener('fetch', event => {});`
 							async ({ request }) => {
 								const json =
 									(await request.json()) as ImageRegistryCredentialsConfiguration;
-								expect(json.permissions).toEqual(["push"]);
+								expect(json.permissions).toEqual(["push", "pull"]);
 
 								return HttpResponse.json({
 									account_id: "test_account_id",
@@ -8928,6 +8984,7 @@ addEventListener('fetch', event => {});`
 
 					Uploaded test-name (TIMINGS)
 					Building image my-container:Galaxy
+					Image does not exist remotely, pushing: registry.cloudflare.com/test_account_id/my-container:Galaxy
 					Deployed test-name triggers (TIMINGS)
 					  https://test-name.test-sub-domain.workers.dev
 					Current Version ID: Galaxy-Class"

packages/wrangler/src/cloudchamber/apply.ts

@@ -377,12 +377,15 @@ export async function apply(
 	log(dim("Container application changes\n"));
 
 	for (const appConfigNoDefaults of config.containers) {
+		const application = applicationByNames[appConfigNoDefaults.name];
+		if (!appConfigNoDefaults.configuration.image && application) {
+			appConfigNoDefaults.configuration.image = application.configuration.image;
+		}
 		const appConfig = containerAppToCreateApplication(
 			appConfigNoDefaults,
 			args.skipDefaults
 		);
 
-		const application = applicationByNames[appConfig.name];
 		if (application !== undefined && application !== null) {
 			// we need to sort the objects (by key) because the diff algorithm works with
 			// lines

packages/wrangler/src/cloudchamber/build.ts

@@ -6,6 +6,7 @@ import {
 	dockerImageInspect,
 	dockerLoginManagedRegistry,
 	getCloudflareRegistryWithAccountNamespace,
+	getDockerImageDigest,
 	isDir,
 	runDockerCmd,
 } from "@cloudflare/containers-shared";
@@ -99,10 +100,12 @@ export async function buildAndMaybePush(
 			},
 			logger
 		);
+
 		await dockerBuild(pathToDocker, {
 			buildCmd,
 			dockerfile,
 		});
+
 		// ensure the account is not allowed to build anything that exceeds the current
 		// account's disk size limits
 		const inspectOutput = await dockerImageInspect(pathToDocker, {
@@ -126,7 +129,29 @@ export async function buildAndMaybePush(
 
 		if (push) {
 			await dockerLoginManagedRegistry(pathToDocker);
-			await runDockerCmd(pathToDocker, ["push", imageTag]);
+			try {
+				const repositoryOnly = imageTag.split(":")[0];
+				// if this succeeds it means this image already exists remotely
+				// if it fails it means it doesn't exist remotely and should be pushed.
+				const localDigest = await getDockerImageDigest(pathToDocker, imageTag);
+				const digest = repositoryOnly + "@" + localDigest;
+				await runDockerCmd(
+					pathToDocker,
+					["manifest", "inspect", digest],
+					"ignore"
+				);
+
+				logger.log("Image already exists remotely, skipping push");
+				logger.debug(
+					`Untagging built image: ${imageTag} since there was no change.`
+				);
+				await runDockerCmd(pathToDocker, ["image", "rm", imageTag]);
+				return "";
+			} catch (error) {
+				logger.log(`Image does not exist remotely, pushing: ${imageTag}`);
+
+				await runDockerCmd(pathToDocker, ["push", imageTag]);
+			}
 		}
 		return imageTag;
 	} catch (error) {