Skip to content

Commit 7a6381c

Browse files
IRCodyIRCody
authored
containers: update push and build to resolve the correct uri (#10623)
* 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 * 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. * Containers: modify build to use the users provided tag Except when we are pushing to our registry. * check + test updates * Deploy test updates
1 parent 2ff7e6d commit 7a6381c

File tree

6 files changed

+244
-31
lines changed

6 files changed

+244
-31
lines changed

‎.changeset/common-beers-push.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@cloudflare/containers-shared": minor
3+
"wrangler": patch
4+
---
5+
6+
Handle more cases for correctly resolving the full uri for an image when using containers push.

‎packages/containers-shared/src/images.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
isCloudflareRegistryLink,
55
} from "./knobs";
66
import { dockerLoginManagedRegistry } from "./login";
7+
import { getCloudflareRegistryWithAccountNamespace } from "./registry";
78
import {
89
checkExposedPorts,
910
cleanupDuplicateImageTags,
@@ -124,24 +125,29 @@ export async function prepareContainerImagesForDev(args: {
124125
/**
125126
* Resolve an image name to the full unambiguous name.
126127
*
127-
* For now, this only converts images stored in the managed registry to contain
128-
* the user's account ID in the path.
128+
* image:tag -> prepend registry.cloudflare.com/accountid/
129+
* registry.cloudflare.com/image:tag -> registry.cloudlfare.com/accountid/image:tag
130+
* registry.cloudflare.com/accountid/image:tag -> no change
131+
* anyother-registry.com/anything -> no change
129132
*/
130133
export function resolveImageName(accountId: string, image: string): string {
131134
let url: URL;
132135
try {
133136
url = new URL(`http://${image}`);
134137
} catch {
135-
return image;
138+
// not a valid url so assume it is in the format image:tag and pre-pend the registry
139+
return getCloudflareRegistryWithAccountNamespace(accountId, image);
136140
}
137-
141+
// hostname not the managed registry, passthrough
138142
if (url.hostname !== getCloudflareContainerRegistry()) {
139143
return image;
140144
}
141145

146+
// is managed registry and has the account id, passthrough
142147
if (url.pathname.startsWith(`/${accountId}`)) {
143148
return image;
144149
}
145150

151+
// is managed registry and doesn't have the account id,add it to the path
146152
return `${url.hostname}/${accountId}${url.pathname}`;
147153
}

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

Lines changed: 158 additions & 15 deletions
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+
`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+
`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: `test-app:tag`,
93+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
94+
});
95+
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
96+
imageTag: `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 registry.cloudflare.com/test-app:tag -p"
105+
);
106+
expect(dockerBuild).toHaveBeenCalledWith("docker", {
107+
buildCmd: [
108+
"build",
109+
"-t",
110+
`registry.cloudflare.com/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+
`registry.cloudflare.com/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 registry.cloudflare.com/some-account-id/test-app:tag -p"
153+
);
154+
expect(dockerBuild).toHaveBeenCalledWith("docker", {
155+
buildCmd: [
156+
"build",
157+
"-t",
158+
`registry.cloudflare.com/some-account-id/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+
`registry.cloudflare.com/some-account-id/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: `registry.cloudflare.com/some-account-id/test-app:tag`,
189+
formatString: "{{ json .RepoDigests }} {{ .Id }}",
190+
});
191+
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
192+
imageTag: `registry.cloudflare.com/some-account-id/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(
@@ -61,7 +204,7 @@ describe("buildAndMaybePush", () => {
61204
buildCmd: [
62205
"build",
63206
"-t",
64-
`${getCloudflareContainerRegistry()}/test-app:tag`,
207+
`test-app:tag`,
65208
"--platform",
66209
"linux/amd64",
67210
"--provenance=false",
@@ -76,15 +219,15 @@ describe("buildAndMaybePush", () => {
76219
1,
77220
"/custom/docker/path",
78221
{
79-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
222+
imageTag: `test-app:tag`,
80223
formatString: "{{ json .RepoDigests }} {{ .Id }}",
81224
}
82225
);
83226
expect(dockerImageInspect).toHaveBeenNthCalledWith(
84227
2,
85228
"/custom/docker/path",
86229
{
87-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
230+
imageTag: `test-app:tag`,
88231
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
89232
}
90233
);
@@ -105,7 +248,7 @@ describe("buildAndMaybePush", () => {
105248
buildCmd: [
106249
"build",
107250
"-t",
108-
`${getCloudflareContainerRegistry()}/test-app:tag`,
251+
`test-app:tag`,
109252
"--platform",
110253
"linux/amd64",
111254
"--provenance=false",
@@ -121,7 +264,7 @@ describe("buildAndMaybePush", () => {
121264
expect(runDockerCmd).toHaveBeenCalledTimes(3);
122265
expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
123266
"tag",
124-
`${getCloudflareContainerRegistry()}/test-app:tag`,
267+
`test-app:tag`,
125268
`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
126269
]);
127270
expect(runDockerCmd).toHaveBeenNthCalledWith(2, "docker", [
@@ -135,11 +278,11 @@ describe("buildAndMaybePush", () => {
135278
]);
136279
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
137280
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
138-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
281+
imageTag: `test-app:tag`,
139282
formatString: "{{ json .RepoDigests }} {{ .Id }}",
140283
});
141284
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
142-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
285+
imageTag: `test-app:tag`,
143286
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
144287
});
145288
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
@@ -168,7 +311,7 @@ describe("buildAndMaybePush", () => {
168311
buildCmd: [
169312
"build",
170313
"-t",
171-
`${getCloudflareContainerRegistry()}/test-app:tag`,
314+
`test-app:tag`,
172315
"--platform",
173316
"linux/amd64",
174317
"--provenance=false",
@@ -189,15 +332,15 @@ describe("buildAndMaybePush", () => {
189332
expect(runDockerCmd).toHaveBeenCalledWith("docker", [
190333
"image",
191334
"rm",
192-
`${getCloudflareContainerRegistry()}/test-app:tag`,
335+
`test-app:tag`,
193336
]);
194337
expect(dockerImageInspect).toHaveBeenCalledTimes(2);
195338
expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
196-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
339+
imageTag: `test-app:tag`,
197340
formatString: "{{ json .RepoDigests }} {{ .Id }}",
198341
});
199342
expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
200-
imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
343+
imageTag: `test-app:tag`,
201344
formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
202345
});
203346
expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
@@ -210,7 +353,7 @@ describe("buildAndMaybePush", () => {
210353
buildCmd: [
211354
"build",
212355
"-t",
213-
`${getCloudflareContainerRegistry()}/test-app`,
356+
`test-app`,
214357
"--platform",
215358
"linux/amd64",
216359
"--provenance=false",
@@ -232,7 +375,7 @@ describe("buildAndMaybePush", () => {
232375
buildCmd: [
233376
"build",
234377
"-t",
235-
`${getCloudflareContainerRegistry()}/test-app`,
378+
`test-app`,
236379
"--platform",
237380
"linux/amd64",
238381
"--provenance=false",
@@ -254,7 +397,7 @@ describe("buildAndMaybePush", () => {
254397
buildCmd: [
255398
"build",
256399
"-t",
257-
`${getCloudflareContainerRegistry()}/test-app:tag`,
400+
`test-app:tag`,
258401
"--platform",
259402
"linux/amd64",
260403
"--provenance=false",

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1957,7 +1957,7 @@ function mockDockerBuild(
19571957
expect(args).toEqual([
19581958
"build",
19591959
"-t",
1960-
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
1960+
`${containerName}:${tag}`,
19611961
"--platform",
19621962
"linux/amd64",
19631963
"--provenance=false",
@@ -1998,7 +1998,7 @@ function mockDockerImageInspectDigests(containerName: string, tag: string) {
19981998
expect(args).toEqual([
19991999
"image",
20002000
"inspect",
2001-
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
2001+
`${containerName}:${tag}`,
20022002
"--format",
20032003
"{{ json .RepoDigests }} {{ .Id }}",
20042004
]);
@@ -2034,7 +2034,7 @@ function mockDockerImageInspectSize(containerName: string, tag: string) {
20342034
expect(args).toEqual([
20352035
"image",
20362036
"inspect",
2037-
`${getCloudflareContainerRegistry()}/${containerName}:${tag}`,
2037+
`${containerName}:${tag}`,
20382038
"--format",
20392039
"{{ .Size }} {{ len .RootFS.Layers }}",
20402040
]);
@@ -2113,7 +2113,7 @@ function mockDockerTag(from: string, to: string, tag: string) {
21132113
expect(cmd).toBe("/usr/bin/docker");
21142114
expect(args).toEqual([
21152115
"tag",
2116-
`${getCloudflareContainerRegistry()}/${from}:${tag}`,
2116+
`${from}:${tag}`,
21172117
`${getCloudflareContainerRegistry()}/${to}:${tag}`,
21182118
]);
21192119
return defaultChildProcess();

0 commit comments

Comments
 (0)