Skip to content

Commit eaf4f4b

Browse files
emily-shenemily-shen
committed
validate and normalise name, constraints and observability too
1 parent 7a2b396 commit eaf4f4b

File tree

5 files changed

+152
-165
lines changed

5 files changed

+152
-165
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,10 @@ export type SharedContainerConfig = {
6363
rollout_step_percentage: number;
6464
/** if undefined in config, defaults to "full_auto" */
6565
rollout_kind: "full_auto" | "full_manual" | "none";
66-
constraints?: {
66+
constraints: {
6767
regions?: string[];
6868
cities?: string[];
6969
tier?: number;
7070
};
71+
observability: { logsEnabled: boolean };
7172
} & InstanceTypeOrLimits;

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

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { runInTempDir } from "../helpers/run-in-tmp";
88
import type { Config } from "../../config";
99

1010
describe("getNormalizedContainerOptions", () => {
11+
mockApiToken();
12+
mockAccountId();
1113
runInTempDir();
1214
beforeEach(() => {
1315
vi.clearAllMocks();
@@ -143,7 +145,10 @@ describe("getNormalizedContainerOptions", () => {
143145
dockerfile: expect.stringMatching(/[/\\]Dockerfile$/),
144146
image_build_context: expect.stringMatching(/[/\\][^/\\]*$/),
145147
image_vars: undefined,
146-
constraints: undefined,
148+
constraints: { tier: 1 },
149+
observability: {
150+
logsEnabled: false,
151+
},
147152
});
148153
});
149154

@@ -155,7 +160,7 @@ describe("getNormalizedContainerOptions", () => {
155160
containers: [
156161
{
157162
class_name: "TestContainer",
158-
image: "registry.example.com/test:latest",
163+
image: `${getCloudflareContainerRegistry()}/test:latest`,
159164
name: "test-container",
160165
},
161166
],
@@ -180,8 +185,11 @@ describe("getNormalizedContainerOptions", () => {
180185
rollout_step_percentage: 25,
181186
rollout_kind: "full_auto",
182187
instance_type: "dev",
183-
constraints: undefined,
184188
image_uri: "registry.cloudflare.com/some-account-id/test:latest",
189+
constraints: { tier: 1 },
190+
observability: {
191+
logsEnabled: false,
192+
},
185193
});
186194
});
187195

@@ -225,8 +233,8 @@ describe("getNormalizedContainerOptions", () => {
225233
disk_bytes: 5_000_000_000, // 5000 MB in bytes
226234
memory_mib: 1024,
227235
vcpu: 2,
228-
constraints: undefined,
229236
image_uri: "registry.example.com/test:latest",
237+
constraints: { tier: 1 },
230238
});
231239
});
232240

@@ -264,8 +272,8 @@ describe("getNormalizedContainerOptions", () => {
264272
rollout_step_percentage: 25,
265273
rollout_kind: "full_auto",
266274
instance_type: "standard",
267-
constraints: undefined,
268275
image_uri: "registry.example.com/test:latest",
276+
constraints: { tier: 1 },
269277
});
270278
});
271279

@@ -275,6 +283,9 @@ describe("getNormalizedContainerOptions", () => {
275283
configPath: "/test/wrangler.toml",
276284
userConfigPath: "/test/wrangler.toml",
277285
topLevelName: "test-worker",
286+
observability: {
287+
logs: { enabled: true },
288+
},
278289
containers: [
279290
{
280291
name: "custom-name",
@@ -288,7 +299,6 @@ describe("getNormalizedContainerOptions", () => {
288299
constraints: {
289300
regions: ["us-east-1", "us-west-2"],
290301
cities: ["NYC", "SF"],
291-
tier: 1,
292302
},
293303
},
294304
],
@@ -314,9 +324,11 @@ describe("getNormalizedContainerOptions", () => {
314324
instance_type: "basic",
315325
image_uri: "registry.example.com/test:latest",
316326
constraints: {
317-
regions: ["us-east-1", "us-west-2"],
318-
cities: ["NYC", "SF"],
319-
tier: 1,
327+
regions: ["US-EAST-1", "US-WEST-2"],
328+
cities: ["nyc", "sf"],
329+
},
330+
observability: {
331+
logsEnabled: true,
320332
},
321333
});
322334
});
@@ -359,7 +371,10 @@ describe("getNormalizedContainerOptions", () => {
359371
dockerfile: expect.stringMatching(/[/\\]nested[/\\]Dockerfile$/),
360372
image_build_context: expect.stringMatching(/[/\\]nested$/),
361373
image_vars: undefined,
362-
constraints: undefined,
374+
constraints: { tier: 1 },
375+
observability: {
376+
logsEnabled: false,
377+
},
363378
});
364379
});
365380

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

Lines changed: 59 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -133,26 +133,21 @@ describe("wrangler deploy with containers", () => {
133133
│
134134
├ NEW my-container
135135
│
136-
│ {
137-
│ \\"containers\\": [
138-
│ {
139-
│ \\"name\\": \\"my-container\\",
140-
│ \\"scheduling_policy\\": \\"default\\",
141-
│ \\"configuration\\": {
142-
│ \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:Galaxy\\",
143-
│ \\"instance_type\\": \\"dev\\"
144-
│ },
145-
│ \\"instances\\": 0,
146-
│ \\"max_instances\\": 10,
147-
│ \\"constraints\\": {
148-
│ \\"tier\\": 1
149-
│ },
150-
│ \\"durable_objects\\": {
151-
│ \\"namespace_id\\": \\"1\\"
152-
│ }
153-
│ }
154-
│ ]
155-
│ }
136+
│ [[containers]]
137+
│ name = \\"my-container\\"
138+
│ scheduling_policy = \\"default\\"
139+
│ instances = 0
140+
│ max_instances = 10
141+
│
142+
│ [containers.configuration]
143+
│ image = \\"registry.cloudflare.com/some-account-id/my-container:Galaxy\\"
144+
│ instance_type = \\"dev\\"
145+
│
146+
│ [containers.constraints]
147+
│ tier = 1
148+
│
149+
│ [containers.durable_objects]
150+
│ namespace_id = \\"1\\"
156151
│
157152
│
158153
│ SUCCESS Created application my-container (Application ID: undefined)
@@ -201,26 +196,21 @@ describe("wrangler deploy with containers", () => {
201196
│
202197
├ NEW my-container
203198
│
204-
│ {
205-
│ \\"containers\\": [
206-
│ {
207-
│ \\"name\\": \\"my-container\\",
208-
│ \\"scheduling_policy\\": \\"default\\",
209-
│ \\"configuration\\": {
210-
│ \\"image\\": \\"docker.io/hello:world\\",
211-
│ \\"instance_type\\": \\"dev\\"
212-
│ },
213-
│ \\"instances\\": 0,
214-
│ \\"max_instances\\": 10,
215-
│ \\"constraints\\": {
216-
│ \\"tier\\": 1
217-
│ },
218-
│ \\"durable_objects\\": {
219-
│ \\"namespace_id\\": \\"1\\"
220-
│ }
221-
│ }
222-
│ ]
223-
│ }
199+
│ [[containers]]
200+
│ name = \\"my-container\\"
201+
│ scheduling_policy = \\"default\\"
202+
│ instances = 0
203+
│ max_instances = 10
204+
│
205+
│ [containers.configuration]
206+
│ image = \\"docker.io/hello:world\\"
207+
│ instance_type = \\"dev\\"
208+
│
209+
│ [containers.constraints]
210+
│ tier = 1
211+
│
212+
│ [containers.durable_objects]
213+
│ namespace_id = \\"1\\"
224214
│
225215
│
226216
│ SUCCESS Created application my-container (Application ID: undefined)
@@ -414,7 +404,6 @@ describe("wrangler deploy with containers", () => {
414404
├ EDIT my-container
415405
│
416406
│ [[containers]]
417-
│ instances = 0
418407
│ - max_instances = 2
419408
│ + max_instances = 10
420409
│ name = \\"my-container\\"
@@ -556,7 +545,6 @@ describe("wrangler deploy with containers", () => {
556545
├ EDIT my-container
557546
│
558547
│ [[containers]]
559-
│ instances = 0
560548
│ - max_instances = 2
561549
│ + max_instances = 10
562550
│ name = \\"my-container\\"
@@ -580,26 +568,21 @@ describe("wrangler deploy with containers", () => {
580568
│
581569
├ NEW my-container-app-2
582570
│
583-
│ {
584-
│ \\"containers\\": [
585-
│ {
586-
│ \\"name\\": \\"my-container-app-2\\",
587-
│ \\"scheduling_policy\\": \\"default\\",
588-
│ \\"configuration\\": {
589-
│ \\"image\\": \\"docker.io/hello:world\\",
590-
│ \\"instance_type\\": \\"dev\\"
591-
│ },
592-
│ \\"instances\\": 0,
593-
│ \\"max_instances\\": 3,
594-
│ \\"constraints\\": {
595-
│ \\"tier\\": 1
596-
│ },
597-
│ \\"durable_objects\\": {
598-
│ \\"namespace_id\\": \\"2\\"
599-
│ }
600-
│ }
601-
│ ]
602-
│ }
571+
│ [[containers]]
572+
│ name = \\"my-container-app-2\\"
573+
│ scheduling_policy = \\"default\\"
574+
│ instances = 0
575+
│ max_instances = 3
576+
│
577+
│ [containers.configuration]
578+
│ image = \\"docker.io/hello:world\\"
579+
│ instance_type = \\"dev\\"
580+
│
581+
│ [containers.constraints]
582+
│ tier = 1
583+
│
584+
│ [containers.durable_objects]
585+
│ namespace_id = \\"2\\"
603586
│
604587
│
605588
│ SUCCESS Created application my-container-app-2 (Application ID: undefined)
@@ -1149,26 +1132,20 @@ describe("wrangler deploy with containers", () => {
11491132
│
11501133
├ NEW my-container
11511134
│
1152-
│ {
1153-
│ \\"containers\\": [
1154-
│ {
1155-
│ \\"name\\": \\"my-container\\",
1156-
│ \\"scheduling_policy\\": \\"default\\",
1157-
│ \\"configuration\\": {
1158-
│ \\"image\\": \\"registry.cloudflare.com/some-account-id/hello:1.0\\",
1159-
│ \\"instance_type\\": \\"standard\\"
1160-
│ },
1161-
│ \\"instances\\": 0,
1162-
│ \\"max_instances\\": 10,
1163-
│ \\"constraints\\": {
1164-
│ \\"tier\\": 2
1165-
│ },
1166-
│ \\"durable_objects\\": {
1167-
│ \\"namespace_id\\": \\"1\\"
1168-
│ }
1169-
│ }
1170-
│ ]
1171-
│ }
1135+
│ [[containers]]
1136+
│ name = \\"my-container\\"
1137+
│ scheduling_policy = \\"default\\"
1138+
│ instances = 0
1139+
│ max_instances = 10
1140+
│
1141+
│ [containers.configuration]
1142+
│ image = \\"registry.cloudflare.com/some-account-id/hello:1.0\\"
1143+
│ instance_type = \\"standard\\"
1144+
│
1145+
│ [containers.constraints]
1146+
│
1147+
│ [containers.durable_objects]
1148+
│ namespace_id = \\"1\\"
11721149
│
11731150
│
11741151
│ SUCCESS Created application my-container (Application ID: undefined)

‎packages/wrangler/src/containers/config.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ import path, { dirname } from "node:path";
33
import {
44
InstanceType,
55
isDockerfile,
6+
resolveImageName,
67
SchedulingPolicy,
78
} from "@cloudflare/containers-shared";
89
import { UserError } from "../errors";
10+
import { getAccountId } from "../user";
911
import type { Config } from "../config";
1012
import type {
1113
ContainerNormalizedConfig,
@@ -56,9 +58,24 @@ export const getNormalizedContainerOptions = async (
5658
max_instances: container.max_instances ?? 0,
5759
scheduling_policy: (container.scheduling_policy ??
5860
SchedulingPolicy.DEFAULT) as SchedulingPolicy,
59-
constraints: container.constraints,
61+
constraints:
62+
"constraints" in container
63+
? {
64+
regions: container.constraints?.regions?.map((region) =>
65+
region.toUpperCase()
66+
),
67+
cities: container.constraints?.cities?.map((city) =>
68+
city.toLowerCase()
69+
),
70+
}
71+
: { tier: 1 },
6072
rollout_step_percentage: container.rollout_step_percentage ?? 25,
61-
rollout_kind: container.rollout_kind ?? "full_auto", // this is the default in the API, so we use it here too ,
73+
rollout_kind: container.rollout_kind ?? "full_auto",
74+
observability: {
75+
logsEnabled:
76+
config.observability?.logs?.enabled ??
77+
config.observability?.enabled === true,
78+
},
6279
};
6380

6481
let instanceTypeOrDisk: InstanceTypeOrLimits;
@@ -100,10 +117,11 @@ export const getNormalizedContainerOptions = async (
100117
image_vars: container.image_vars,
101118
});
102119
} else {
120+
const accountId = await getAccountId(config);
103121
normalizedContainers.push({
104122
...shared,
105123
...instanceTypeOrDisk,
106-
registry_link: container.image, // if it is not a dockerfile, it must be a registry link or have thrown an error
124+
image_uri: resolveImageName(accountId, container.image), // if it is not a dockerfile, it must be an image uri or have thrown an error
107125
});
108126
}
109127
}

0 commit comments

Comments
 (0)