Skip to content

Commit f125331

Browse files
IRCodyIRCody
committed
containers: update build to resolve the correct uri
Before it was possible to get incorrect/confusing uris for images built with containers build -p. Example: containers build -p -t registry.cloudflare.com/image:tag . would become registry.cloudflare.com/<accountid>/registry.cloudflare.com/image:tag. This change modifies build to use resolveImageName which handles these cases.
1 parent 93c733e commit f125331

File tree

2 files changed

+145
-3
lines changed

2 files changed

+145
-3
lines changed

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

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ describe("buildAndMaybePush", () => {
3131
runInTempDir();
3232
mockApiToken();
3333
mockAccountId();
34-
3534
beforeEach(() => {
3635
vi.clearAllMocks();
3736
vi.mocked(dockerImageInspect)
@@ -52,6 +51,150 @@ describe("buildAndMaybePush", () => {
5251
vi.clearAllMocks();
5352
});
5453

54+
it("should be able to build image and push with test-app:tag", async () => {
55+
await runWrangler(
56+
"containers build ./container-context -t test-app:tag -p"
57+
);
58+
expect(dockerBuild).toHaveBeenCalledWith("docker", {
59+
buildCmd: [
60+
"build",
61+
"-t",
62+
`${getCloudflareContainerRegistry()}/test-app:tag`,
63+
"--platform",
64+
"linux/amd64",
65+
"--provenance=false",
66+
"-f",
67+
"-",
68+
// turn this into a relative path so that this works across different OSes
69+
"./container-context",
70+
],
71+
dockerfile,
72+
});
73+
74+
// 3 calls: docker tag + docker push + docker rm
75+
expect(runDockerCmd).toHaveBeenCalledTimes(3);
76+
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
77+
"tag",
78+
`${getCloudflareContainerRegistry()}/test-app:tag`,
79+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
80+
]);
81+
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
82+
"push",
83+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
84+
]);
85+
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
86+
"image",
87+
"rm",
88+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
89+
]);
90+
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
91+
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
92+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
93+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
94+
});
95+
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
96+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
97+
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
98+
});
99+
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
100+
});
101+
102+
it("should be able to build image and push with registry.cloudflare.com/test-app:tag", async () => {
103+
await runWrangler(
104+
"containers build ./container-context -t test-app:tag -p"
105+
);
106+
expect(dockerBuild).toHaveBeenCalledWith("docker", {
107+
buildCmd: [
108+
"build",
109+
"-t",
110+
`${getCloudflareContainerRegistry()}/test-app:tag`,
111+
"--platform",
112+
"linux/amd64",
113+
"--provenance=false",
114+
"-f",
115+
"-",
116+
// turn this into a relative path so that this works across different OSes
117+
"./container-context",
118+
],
119+
dockerfile,
120+
});
121+
122+
// 3 calls: docker tag + docker push + docker rm
123+
expect(runDockerCmd).toHaveBeenCalledTimes(3);
124+
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
125+
"tag",
126+
`${getCloudflareContainerRegistry()}/test-app:tag`,
127+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
128+
]);
129+
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
130+
"push",
131+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
132+
]);
133+
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
134+
"image",
135+
"rm",
136+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
137+
]);
138+
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
139+
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
140+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
141+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
142+
});
143+
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
144+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
145+
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
146+
});
147+
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
148+
});
149+
150+
it("should be able to build image and push with registry.cloudflare.com/some-account-id/test-app:tag", async () => {
151+
await runWrangler(
152+
"containers build ./container-context -t test-app:tag -p"
153+
);
154+
expect(dockerBuild).toHaveBeenCalledWith("docker", {
155+
buildCmd: [
156+
"build",
157+
"-t",
158+
`${getCloudflareContainerRegistry()}/test-app:tag`,
159+
"--platform",
160+
"linux/amd64",
161+
"--provenance=false",
162+
"-f",
163+
"-",
164+
// turn this into a relative path so that this works across different OSes
165+
"./container-context",
166+
],
167+
dockerfile,
168+
});
169+
170+
// 3 calls: docker tag + docker push + docker rm
171+
expect(runDockerCmd).toHaveBeenCalledTimes(3);
172+
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
173+
"tag",
174+
`${getCloudflareContainerRegistry()}/test-app:tag`,
175+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
176+
]);
177+
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
178+
"push",
179+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
180+
]);
181+
expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
182+
"image",
183+
"rm",
184+
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
185+
]);
186+
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
187+
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
188+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
189+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
190+
});
191+
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
192+
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
193+
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
194+
});
195+
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
196+
});
197+
55198
it("should use a custom docker path if provided", async () => {
56199
vi.stubEnv("WRANGLER_DOCKER_BIN", "/custom/docker/path");
57200
await runWrangler(

‎packages/wrangler/src/cloudchamber/build.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
dockerImageInspect,
77
dockerLoginManagedRegistry,
88
getCloudflareContainerRegistry,
9-
getCloudflareRegistryWithAccountNamespace,
109
isDir,
1110
resolveImageName,
1211
runDockerCmd,
@@ -220,7 +219,7 @@ export async function buildAndMaybePush(
220219
}
221220
}
222221
// Re-tag the image to include the account ID
223-
const namespacedImageTag = getCloudflareRegistryWithAccountNamespace(
222+
const namespacedImageTag = resolveImageName(
224223
account.external_account_id,
225224
args.tag
226225
);

0 commit comments

Comments
 (0)