containers: update push to resolve the correct uri

Before it was possible to get incorrect/confusing uris for images pushed
with containers push.

Example:

containers push registry.cloudflare.com/image:tag would become
registry.cloudflare.com/<accountid>/registry.cloudflare.com/image:tag.

This change modifies resolveImageName to behave like:

image:tag -> prepend registry.cloudflare.com/accountid/
registry.cloudflare.com/image:tag -> registry.cloudlfare.com/accountid/image:tag
registry.cloudflare.com/accountid/image:tag -> no change
anyother-registry.com/anything -> no change

Author: IRCody

.changeset/common-beers-push.md

@@ -0,0 +1,6 @@
+---
+"@cloudflare/containers-shared": minor
+"wrangler": patch
+---
+
+Handle more cases for correctly resolving the full uri for an image when using containers push.

packages/containers-shared/src/images.ts

@@ -4,6 +4,7 @@ import {
 	isCloudflareRegistryLink,
 } from "./knobs";
 import { dockerLoginManagedRegistry } from "./login";
+import { getCloudflareRegistryWithAccountNamespace } from "./registry";
 import {
 	checkExposedPorts,
 	cleanupDuplicateImageTags,
@@ -124,24 +125,29 @@ export async function prepareContainerImagesForDev(args: {
 /**
  * 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.
+ * image:tag -> prepend registry.cloudflare.com/accountid/
+ * registry.cloudflare.com/image:tag -> registry.cloudlfare.com/accountid/image:tag
+ * registry.cloudflare.com/accountid/image:tag -> no change
+ * anyother-registry.com/anything -> no change
  */
 export function resolveImageName(accountId: string, image: string): string {
 	let url: URL;
 	try {
 		url = new URL(`http://${image}`);
 	} catch {
-		return image;
+		// not a valid url so assume it is in the format image:tag and pre-pend the registry
+		return getCloudflareRegistryWithAccountNamespace(accountId, image);
 	}
-
+	// hostname not the managed registry, passthrough
 	if (url.hostname !== getCloudflareContainerRegistry()) {
 		return image;
 	}
 
+	// is managed registry and has the account id, passthrough
 	if (url.pathname.startsWith(`/${accountId}`)) {
 		return image;
 	}
 
+	// is managed registry and doesn't have the account id,add it to the path
 	return `${url.hostname}/${accountId}${url.pathname}`;
 }

packages/wrangler/src/__tests__/containers/push.test.ts

@@ -79,4 +79,66 @@ describe("containers push", () => {
 			"Unsupported platform"
 		);
 	});
+
+	it("should tag image with the correct uri if given an <image>:<tag> argument", async () => {
+		setIsTTY(false);
+		setWranglerConfig({});
+		vi.mocked(dockerImageInspect).mockResolvedValue("linux/amd64");
+		expect(std.err).toMatchInlineSnapshot(`""`);
+
+		await runWrangler("containers push test-app:tag");
+
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
+			"tag",
+			`test-app:tag`,
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
+			"push",
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+	});
+
+	it("should tag image with the correct uri if given an registry.cloudflare.com/<image>:<tag> argument", async () => {
+		setIsTTY(false);
+		setWranglerConfig({});
+		vi.mocked(dockerImageInspect).mockResolvedValue("linux/amd64");
+		expect(std.err).toMatchInlineSnapshot(`""`);
+
+		await runWrangler("containers push registry.cloudflare.com/test-app:tag");
+
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
+			"tag",
+			`registry.cloudflare.com/test-app:tag`,
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
+			"push",
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+	});
+
+	it("should tag image with the correct uri if given an registry.cloudflare.com/some-account-id/<image>:<tag> argument", async () => {
+		setIsTTY(false);
+		setWranglerConfig({});
+		vi.mocked(dockerImageInspect).mockResolvedValue("linux/amd64");
+		expect(std.err).toMatchInlineSnapshot(`""`);
+
+		await runWrangler(
+			"containers push registry.cloudflare.com/some-account-id/test-app:tag"
+		);
+
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
+			"tag",
+			`registry.cloudflare.com/some-account-id/test-app:tag`,
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+		expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
+			"push",
+			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
+		]);
+	});
 });

packages/wrangler/src/cloudchamber/build.ts

@@ -280,11 +280,9 @@ export async function pushCommand(
 ) {
 	try {
 		await dockerLoginManagedRegistry(args.pathToDocker);
+
 		const accountId = await getAccountId(config);
-		const newTag = getCloudflareRegistryWithAccountNamespace(
-			accountId,
-			args.TAG
-		);
+		const newTag = resolveImageName(accountId, args.TAG);
 		const dockerPath = args.pathToDocker ?? getDockerPath();
 		await checkImagePlatform(dockerPath, args.TAG);
 		await runDockerCmd(dockerPath, ["tag", args.TAG, newTag]);