Skip to content

Commit 75817eb

Browse files
committed
bring back dev specific options type
1 parent eaf4f4b commit 75817eb

File tree

10 files changed

+151
-85
lines changed

10 files changed

+151
-85
lines changed

packages/containers-shared/src/build.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { spawn } from "child_process";
22
import { readFileSync } from "fs";
3-
import type { BuildArgs, DockerfileConfig, Logger } from "./types";
3+
import type {
4+
BuildArgs,
5+
ContainerDevOptions,
6+
ImageURIConfig,
7+
Logger,
8+
} from "./types";
49

510
export async function constructBuildCommand(
611
options: BuildArgs,
@@ -88,11 +93,10 @@ export function dockerBuild(
8893

8994
export async function buildImage(
9095
dockerPath: string,
91-
options: DockerfileConfig,
92-
imageTag: string
96+
options: Exclude<ContainerDevOptions, ImageURIConfig>
9397
) {
9498
const { buildCmd, dockerfile } = await constructBuildCommand({
95-
tag: imageTag,
99+
tag: options.image_tag,
96100
pathToDockerfile: options.dockerfile,
97101
buildContext: options.image_build_context,
98102
args: options.image_vars,

packages/containers-shared/src/images.ts

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { buildImage } from "./build";
22
import {
33
getCloudflareContainerRegistry,
4-
getDevContainerImageName,
54
isCloudflareRegistryLink,
65
} from "./knobs";
76
import { dockerLoginManagedRegistry } from "./login";
@@ -10,12 +9,11 @@ import {
109
runDockerCmd,
1110
verifyDockerInstalled,
1211
} from "./utils";
13-
import type { ContainerNormalizedConfig, RegistryLinkConfig } from "./types";
12+
import type { ContainerDevOptions, DockerfileConfig } from "./types";
1413

1514
export async function pullImage(
1615
dockerPath: string,
17-
options: RegistryLinkConfig,
18-
tag: string
16+
options: Exclude<ContainerDevOptions, DockerfileConfig>
1917
): Promise<{ abort: () => void; ready: Promise<void> }> {
2018
await dockerLoginManagedRegistry(dockerPath);
2119
const pull = runDockerCmd(dockerPath, [
@@ -28,7 +26,11 @@ export async function pullImage(
2826
const ready = pull.ready.then(async ({ aborted }: { aborted: boolean }) => {
2927
if (!aborted) {
3028
// re-tag image with the expected dev-formatted image tag for consistency
31-
await runDockerCmd(dockerPath, ["tag", options.image_uri, tag]);
29+
await runDockerCmd(dockerPath, [
30+
"tag",
31+
options.image_uri,
32+
options.image_tag,
33+
]);
3234
}
3335
});
3436

@@ -50,23 +52,23 @@ export async function pullImage(
5052
* such as checking if the Docker CLI is installed, and if the container images
5153
* expose any ports.
5254
*/
53-
export async function prepareContainerImagesForDev(options: {
55+
export async function prepareContainerImagesForDev(args: {
5456
dockerPath: string;
55-
containerOptions: ContainerNormalizedConfig[];
57+
containerOptions: ContainerDevOptions[];
5658
onContainerImagePreparationStart: (args: {
57-
containerOptions: ContainerNormalizedConfig;
59+
containerOptions: ContainerDevOptions;
5860
abort: () => void;
5961
}) => void;
6062
onContainerImagePreparationEnd: (args: {
61-
containerOptions: ContainerNormalizedConfig[];
63+
containerOptions: ContainerDevOptions;
6264
}) => void;
6365
}) {
6466
const {
6567
dockerPath,
6668
containerOptions,
6769
onContainerImagePreparationStart,
6870
onContainerImagePreparationEnd,
69-
} = options;
71+
} = args;
7072
let aborted = false;
7173
if (process.platform === "win32") {
7274
throw new Error(
@@ -75,9 +77,8 @@ export async function prepareContainerImagesForDev(options: {
7577
}
7678
await verifyDockerInstalled(dockerPath);
7779
for (const options of containerOptions) {
78-
const tag = getDevContainerImageName(options.class_name, containerBuildId);
7980
if ("dockerfile" in options) {
80-
const build = await buildImage(dockerPath, options, tag);
81+
const build = await buildImage(dockerPath, options);
8182
onContainerImagePreparationStart({
8283
containerOptions: options,
8384
abort: () => {
@@ -96,7 +97,7 @@ export async function prepareContainerImagesForDev(options: {
9697
`To use an existing image from another repository, see https://developers.cloudflare.com/containers/image-management/#using-existing-images`
9798
);
9899
}
99-
const pull = await pullImage(dockerPath, options, tag);
100+
const pull = await pullImage(dockerPath, options);
100101
onContainerImagePreparationStart({
101102
containerOptions: options,
102103
abort: () => {
@@ -110,7 +111,7 @@ export async function prepareContainerImagesForDev(options: {
110111
});
111112
}
112113
if (!aborted) {
113-
await checkExposedPorts(dockerPath, options, tag);
114+
await checkExposedPorts(dockerPath, options);
114115
}
115116
}
116117
}

packages/containers-shared/src/types.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@ export type BuildArgs = {
2222
setNetworkToHost?: boolean;
2323
};
2424

25-
export type ContainerNormalizedConfig = RegistryLinkConfig | DockerfileConfig;
26-
export type DockerfileConfig = SharedContainerConfig & {
25+
export type ContainerNormalizedConfig = SharedContainerConfig &
26+
(ImageURIConfig | DockerfileConfig);
27+
export type DockerfileConfig = {
2728
/** absolute path, resolved relative to the wrangler config file */
2829
dockerfile: string;
2930
/** absolute path, resolved relative to the wrangler config file. defaults to the directory of the dockerfile */
3031
image_build_context: string;
3132
image_vars?: Record<string, string>;
3233
};
33-
export type RegistryLinkConfig = SharedContainerConfig & {
34+
export type ImageURIConfig = {
3435
image_uri: string;
3536
};
3637

@@ -70,3 +71,11 @@ export type SharedContainerConfig = {
7071
};
7172
observability: { logsEnabled: boolean };
7273
} & InstanceTypeOrLimits;
74+
75+
/** build/pull agnostic container options */
76+
export type ContainerDevOptions = {
77+
/** formatted as cloudflare-dev/workername-DOclassname:build-id */
78+
image_tag: string;
79+
/** container's DO class name */
80+
class_name: string;
81+
} & (DockerfileConfig | ImageURIConfig);

packages/containers-shared/src/utils.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { execFile, spawn } from "child_process";
2+
import { randomUUID } from "crypto";
23
import { existsSync, statSync } from "fs";
34
import path from "path";
45
import { dockerImageInspect } from "./inspect";
5-
import type { ContainerNormalizedConfig } from "./types";
6+
import type { ContainerDevOptions } from "./types";
67
import type { StdioOptions } from "child_process";
78

89
/** helper for simple docker command call that don't require any io handling */
@@ -229,11 +230,10 @@ export const getContainerIdsFromImage = async (
229230
*/
230231
export async function checkExposedPorts(
231232
dockerPath: string,
232-
options: ContainerNormalizedConfig,
233-
imageTag: string
233+
options: ContainerDevOptions
234234
) {
235235
const output = await dockerImageInspect(dockerPath, {
236-
imageTag: imageTag,
236+
imageTag: options.image_tag,
237237
formatString: "{{ len .Config.ExposedPorts }}",
238238
});
239239
if (output === "0") {

packages/containers-shared/tests/utils.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { mkdirSync, writeFileSync } from "fs";
22
import { checkExposedPorts, isDockerfile } from "./../src/utils";
33
import { runInTempDir } from "./helpers/run-in-tmp-dir";
4-
import type { ContainerNormalizedConfig } from "../src/types";
4+
import type { ContainerDevOptions } from "../src/types";
55

66
describe("isDockerfile", () => {
77
const dockerfile =
@@ -75,7 +75,7 @@ vi.mock("../src/inspect", async (importOriginal) => {
7575
const containerConfig = {
7676
dockerfile: "",
7777
class_name: "MyContainer",
78-
} as ContainerNormalizedConfig;
78+
} as ContainerDevOptions;
7979
describe("checkExposedPorts", () => {
8080
beforeEach(() => {
8181
docketImageInspectResult = "1";
@@ -84,14 +84,14 @@ describe("checkExposedPorts", () => {
8484
it("should not error when some ports are exported", async () => {
8585
docketImageInspectResult = "1";
8686
await expect(
87-
checkExposedPorts("docker", containerConfig, "image:tag")
87+
checkExposedPorts("docker", containerConfig)
8888
).resolves.toBeUndefined();
8989
});
9090

9191
it("should error, with an appropriate message when no ports are exported", async () => {
9292
docketImageInspectResult = "0";
93-
await expect(checkExposedPorts("docker", containerConfig, "image:tag"))
94-
.rejects.toThrowErrorMatchingInlineSnapshot(`
93+
await expect(checkExposedPorts("docker", containerConfig)).rejects
94+
.toThrowErrorMatchingInlineSnapshot(`
9595
[Error: The container "MyContainer" does not expose any ports. In your Dockerfile, please expose any ports you intend to connect to.
9696
For additional information please see: https://developers.cloudflare.com/containers/local-dev/#exposing-ports.
9797
]

packages/vite-plugin-cloudflare/src/index.ts

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import type {
7575
ResolvedPluginConfig,
7676
WorkerConfig,
7777
} from "./plugin-config";
78+
import type { ContainerDevOptions } from "@cloudflare/containers-shared";
7879
import type { StaticRouting } from "@cloudflare/workers-shared/utils/types";
7980
import type { Unstable_RawConfig } from "wrangler";
8081

@@ -488,7 +489,7 @@ if (import.meta.hot) {
488489
"Build ID should be set if containers are enabled and defined"
489490
);
490491
// Assemble container options and build if necessary
491-
const containerOptions = await getContainerOptions(
492+
const containerOptions = getContainerOptions(
492493
entryWorkerConfig,
493494
containerBuildId
494495
);
@@ -497,12 +498,11 @@ if (import.meta.hot) {
497498
if (containerOptions) {
498499
// keep track of them so we can clean up later
499500
for (const container of containerOptions) {
500-
containerImageTagsSeen.add(container.imageTag);
501+
containerImageTagsSeen.add(container.image_tag);
501502
}
502503

503504
await prepareContainerImagesForDev({
504505
dockerPath,
505-
configPath: entryWorkerConfig.configPath,
506506
containerOptions,
507507
onContainerImagePreparationStart: () => {},
508508
onContainerImagePreparationEnd: () => {},
@@ -1082,25 +1082,41 @@ if (import.meta.hot) {
10821082
* with image tag set to well-known dev format, or undefined if
10831083
* containers are not enabled or not configured.
10841084
*/
1085-
async function getContainerOptions(
1085+
function getContainerOptions(
10861086
config: WorkerConfig,
10871087
containerBuildId: string
1088-
) {
1088+
): ContainerDevOptions[] | undefined {
10891089
if (!config.containers?.length || config.dev.enable_containers === false) {
10901090
return undefined;
10911091
}
1092-
1092+
const baseDir = config.configPath
1093+
? path.dirname(config.configPath)
1094+
: resolvedViteConfig.root;
10931095
return config.containers.map((container) => {
1094-
return {
1095-
image: container.image ?? container.configuration?.image,
1096-
imageTag: getDevContainerImageName(
1097-
container.class_name,
1098-
containerBuildId
1099-
),
1100-
args: container.image_vars,
1101-
imageBuildContext: container.image_build_context,
1102-
class_name: container.class_name,
1103-
};
1096+
if (isDockerfile(container.image, config.configPath)) {
1097+
const absoluteDockerfilePath = path.resolve(baseDir, container.image);
1098+
return {
1099+
dockerfile: absoluteDockerfilePath,
1100+
image_build_context: container.image_build_context
1101+
? path.resolve(baseDir, container.image_build_context)
1102+
: path.dirname(absoluteDockerfilePath),
1103+
image_vars: container.image_vars,
1104+
class_name: container.class_name,
1105+
image_tag: getDevContainerImageName(
1106+
container.class_name,
1107+
containerBuildId
1108+
),
1109+
};
1110+
} else {
1111+
return {
1112+
image_uri: container.image,
1113+
class_name: container.class_name,
1114+
image_tag: getDevContainerImageName(
1115+
container.class_name,
1116+
containerBuildId
1117+
),
1118+
};
1119+
}
11041120
});
11051121
}
11061122
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { mkdirSync, writeFileSync } from "fs";
2-
import path from "path";
32
import {
43
dockerBuild,
54
dockerImageInspect,
@@ -62,7 +61,7 @@ describe("buildAndMaybePush", () => {
6261
"--provenance=false",
6362
"-f",
6463
"-",
65-
path.resolve(process.cwd(), "./container-context"),
64+
"./container-context",
6665
],
6766
dockerfile,
6867
});
@@ -95,7 +94,7 @@ describe("buildAndMaybePush", () => {
9594
"-f",
9695
"-",
9796
// turn this into a relative path so that this works across different OSes
98-
path.resolve(process.cwd(), "./container-context"),
97+
"./container-context",
9998
],
10099
dockerfile,
101100
});
@@ -146,7 +145,7 @@ describe("buildAndMaybePush", () => {
146145
"--provenance=false",
147146
"-f",
148147
"-",
149-
path.resolve(process.cwd(), "./container-context"),
148+
"./container-context",
150149
],
151150
dockerfile,
152151
});
@@ -188,7 +187,7 @@ describe("buildAndMaybePush", () => {
188187
"--provenance=false",
189188
"-f",
190189
"-",
191-
path.resolve(process.cwd(), "./container-context"),
190+
"./container-context",
192191
],
193192
dockerfile,
194193
});
@@ -212,7 +211,7 @@ describe("buildAndMaybePush", () => {
212211
"host",
213212
"-f",
214213
"-",
215-
path.resolve(process.cwd(), "./container-context"),
214+
"./container-context",
216215
],
217216
dockerfile,
218217
});
@@ -232,7 +231,7 @@ describe("buildAndMaybePush", () => {
232231
"--provenance=false",
233232
"-f",
234233
"-",
235-
path.resolve(process.cwd(), "./container-context"),
234+
"./container-context",
236235
],
237236
dockerfile,
238237
});

packages/wrangler/src/api/integrations/platform/index.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ async function getMiniflareOptionsFromConfig(args: {
219219
migrations: config.migrations,
220220
imagesLocalMode: true,
221221
tails: [],
222-
containers: undefined,
222+
containerDOClassNames: new Set(
223+
config.containers?.map((c) => c.class_name)
224+
),
223225
containerBuildId: undefined,
224226
},
225227
remoteProxyConnectionString,
@@ -369,6 +371,9 @@ export function unstable_getMiniflareWorkerOptions(
369371
fallthrough: rule.fallthrough,
370372
}));
371373

374+
const containerDOClassNames = new Set(
375+
config.containers?.map((c) => c.class_name)
376+
);
372377
const bindings = getBindings(config, env, true, {}, true);
373378
const { bindingOptions, externalWorkers } = buildMiniflareBindingOptions(
374379
{
@@ -382,7 +387,7 @@ export function unstable_getMiniflareWorkerOptions(
382387
migrations: config.migrations,
383388
imagesLocalMode: !!options?.imagesLocalMode,
384389
tails: config.tail_consumers,
385-
containers: config.containers,
390+
containerDOClassNames,
386391
containerBuildId: options?.containerBuildId,
387392
},
388393
options?.remoteProxyConnectionString,
@@ -435,7 +440,7 @@ export function unstable_getMiniflareWorkerOptions(
435440
useSQLite,
436441
container: getImageNameFromDOClassName({
437442
doClassName: binding.class_name,
438-
containers: config.containers,
443+
containerDOClassNames,
439444
containerBuildId: options?.containerBuildId,
440445
}),
441446
} satisfies DurableObjectDefinition,

0 commit comments

Comments
 (0)