Skip to content

Commit 6cc24c0

Browse files
emily-shenemily-shen
authored
add more validation to containers config (#9934)
* add some extra validation * add test * add deprecation warnings * changeset * lint * validate nested configuration * pr feedback
1 parent cda9e64 commit 6cc24c0

File tree

4 files changed

+215
-32
lines changed

4 files changed

+215
-32
lines changed

‎.changeset/clean-colts-vanish.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
Add more thorough validation to containers configuration

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ describe("wrangler deploy with containers", () => {
9696
containers: [
9797
{
9898
name: "my-container",
99-
instances: 10,
99+
max_instances: 10,
100100
class_name: "ExampleDurableObject",
101101
image: "./Dockerfile",
102102
},
@@ -147,7 +147,7 @@ describe("wrangler deploy with containers", () => {
147147
containers: [
148148
{
149149
name: "my-container",
150-
instances: 10,
150+
max_instances: 10,
151151
class_name: "ExampleDurableObject",
152152
image: "./Dockerfile",
153153
},
@@ -161,7 +161,7 @@ describe("wrangler deploy with containers", () => {
161161

162162
mockCreateApplication({
163163
name: "my-container",
164-
instances: 10,
164+
max_instances: 10,
165165
durable_objects: { namespace_id: "1" },
166166
configuration: {
167167
image:
@@ -206,7 +206,7 @@ describe("wrangler deploy with containers", () => {
206206
{
207207
image: "docker.io/hello:world",
208208
name: "my-container",
209-
instances: 10,
209+
max_instances: 10,
210210
class_name: "ExampleDurableObject",
211211
},
212212
],
@@ -217,7 +217,7 @@ describe("wrangler deploy with containers", () => {
217217

218218
mockCreateApplication({
219219
name: "my-container",
220-
instances: 10,
220+
max_instances: 10,
221221
durable_objects: { namespace_id: "1" },
222222
scheduling_policy: SchedulingPolicy.DEFAULT,
223223
});
@@ -287,7 +287,7 @@ describe("wrangler deploy with containers", () => {
287287
containers: [
288288
{
289289
name: "my-container",
290-
instances: 10,
290+
max_instances: 10,
291291
class_name: "ExampleDurableObject",
292292
image: "../Dockerfile",
293293
},
@@ -311,7 +311,7 @@ describe("wrangler deploy with containers", () => {
311311

312312
mockCreateApplication({
313313
name: "my-container",
314-
instances: 10,
314+
max_instances: 10,
315315
durable_objects: { namespace_id: "1" },
316316
configuration: {
317317
image:
@@ -391,7 +391,7 @@ describe("wrangler deploy with containers", () => {
391391
containers: [
392392
{
393393
name: "my-container",
394-
instances: 10,
394+
max_instances: 10,
395395
class_name: "ExampleDurableObject",
396396
image: "../Dockerfile",
397397
},
@@ -408,7 +408,7 @@ describe("wrangler deploy with containers", () => {
408408

409409
mockCreateApplication({
410410
name: "my-container",
411-
instances: 10,
411+
max_instances: 10,
412412
durable_objects: { namespace_id: "1" },
413413
configuration: {
414414
image:
@@ -453,7 +453,7 @@ describe("wrangler deploy with containers", () => {
453453
{
454454
image: "docker.io/hello:world",
455455
name: "my-container",
456-
instances: 10,
456+
max_instances: 10,
457457
class_name: "ExampleDurableObject",
458458
},
459459
],
@@ -515,7 +515,7 @@ describe("wrangler deploy with containers dry run", () => {
515515
{
516516
image: "./Dockerfile",
517517
name: "my-container",
518-
instances: 10,
518+
max_instances: 10,
519519
class_name: "ExampleDurableObject",
520520
},
521521
],

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,6 +2421,108 @@ describe("normalizeAndValidateConfig()", () => {
24212421
);
24222422
}
24232423
});
2424+
2425+
it("should error for invalid container app fields", () => {
2426+
const { diagnostics } = normalizeAndValidateConfig(
2427+
{
2428+
name: "test-worker",
2429+
containers: [
2430+
{
2431+
image: "something",
2432+
class_name: "test-class",
2433+
rollout_kind: "invalid",
2434+
instance_type: "invalid",
2435+
max_instances: "invalid",
2436+
image_build_context: 123,
2437+
image_vars: "invalid",
2438+
scheduling_policy: "invalid",
2439+
unknown_field: "value",
2440+
},
2441+
],
2442+
} as unknown as RawConfig,
2443+
undefined,
2444+
undefined,
2445+
{ env: undefined }
2446+
);
2447+
2448+
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
2449+
"Processing wrangler configuration:
2450+
- Unexpected fields found in containers field: \\"unknown_field\\""
2451+
`);
2452+
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
2453+
"Processing wrangler configuration:
2454+
- Expected \\"containers.rollout_kind\\" field to be one of [\\"full_auto\\",\\"full_manual\\",\\"none\\"] but got \\"invalid\\".
2455+
- Expected \\"containers.instance_type\\" field to be one of [\\"dev\\",\\"basic\\",\\"standard\\"] but got \\"invalid\\".
2456+
- Expected \\"containers.max_instances\\" to be of type number but got \\"invalid\\".
2457+
- Expected \\"containers.image_build_context\\" to be of type string but got 123.
2458+
- Expected \\"containers.image_vars\\" to be of type object but got \\"invalid\\".
2459+
- Expected \\"containers.scheduling_policy\\" field to be one of [\\"regional\\",\\"moon\\",\\"default\\"] but got \\"invalid\\"."
2460+
`);
2461+
});
2462+
2463+
it("should warn for deprecated container fields", () => {
2464+
const { diagnostics } = normalizeAndValidateConfig(
2465+
{
2466+
name: "test-worker",
2467+
containers: [
2468+
{
2469+
class_name: "test-class",
2470+
instances: 10,
2471+
configuration: {
2472+
image: "config-image",
2473+
},
2474+
durable_objects: {
2475+
namespace_id: "test-namespace",
2476+
},
2477+
},
2478+
],
2479+
} as unknown as RawConfig,
2480+
undefined,
2481+
undefined,
2482+
{ env: undefined }
2483+
);
2484+
2485+
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
2486+
"Processing wrangler configuration:
2487+
- \\"containers.configuration\\" is deprecated. Use top level \\"containers\\" fields instead. \\"configuration.image\\" should be \\"image\\", \\"configuration.disk\\" should be set via \\"instance_type\\".
2488+
- \\"containers.instances\\" is deprecated. Use \\"containers.max_instances\\" instead.
2489+
- \\"containers.durable_objects\\" is deprecated. Use the \\"class_name\\" field instead."
2490+
`);
2491+
});
2492+
2493+
it("should error for invalid containers.configuration fields", () => {
2494+
const { diagnostics } = normalizeAndValidateConfig(
2495+
{
2496+
name: "test-worker",
2497+
containers: [
2498+
{
2499+
class_name: "test-class",
2500+
configuration: {
2501+
image: "config-image",
2502+
secrets: [],
2503+
labels: [],
2504+
disk: { size: "2GB" },
2505+
memory: "256MB",
2506+
vcpu: 0.5,
2507+
memory_mib: 256,
2508+
invalid_field: "should not be here",
2509+
another_invalid: 123,
2510+
},
2511+
},
2512+
],
2513+
} as unknown as RawConfig,
2514+
undefined,
2515+
undefined,
2516+
{ env: undefined }
2517+
);
2518+
2519+
console.dir(diagnostics.warnings);
2520+
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
2521+
"Processing wrangler configuration:
2522+
- \\"containers.configuration\\" is deprecated. Use top level \\"containers\\" fields instead. \\"configuration.image\\" should be \\"image\\", \\"configuration.disk\\" should be set via \\"instance_type\\".
2523+
- Unexpected fields found in containers.configuration field: \\"memory\\",\\"invalid_field\\",\\"another_invalid\\""
2524+
`);
2525+
});
24242526
});
24252527

24262528
describe("[kv_namespaces]", () => {

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

Lines changed: 97 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,7 +2421,6 @@ function validateContainerApp(
24212421
name += config === undefined ? "" : `-${envName}`;
24222422
containerAppOptional.name = name.toLowerCase().replace(/ /g, "-");
24232423
}
2424-
24252424
if (
24262425
!containerAppOptional.configuration?.image &&
24272426
!containerAppOptional.image
@@ -2430,6 +2429,11 @@ function validateContainerApp(
24302429
`"containers.image" field must be defined for each container app. This should be the path to your Dockerfile or a image URI pointing to the Cloudflare registry.`
24312430
);
24322431
}
2432+
if ("configuration" in containerAppOptional) {
2433+
diagnostics.warnings.push(
2434+
`"containers.configuration" is deprecated. Use top level "containers" fields instead. "configuration.image" should be "image", "configuration.disk" should be set via "instance_type".`
2435+
);
2436+
}
24332437

24342438
// Validate that we have an image configuration for this container app.
24352439
// For legacy reasons we have to check both at containerAppOptional.image and
@@ -2469,19 +2473,6 @@ function validateContainerApp(
24692473
`"containers.rollout_step_percentage" field should be a number between 25 and 100, but got ${containerAppOptional.rollout_step_percentage}`
24702474
);
24712475
}
2472-
2473-
if (
2474-
!isOptionalProperty(containerAppOptional, "rollout_kind", "string") &&
2475-
"rollout_kind" in containerAppOptional &&
2476-
!["full_auto", "full_manual", "none"].includes(
2477-
containerAppOptional.rollout_kind
2478-
)
2479-
) {
2480-
diagnostics.errors.push(
2481-
`"containers.rollout_kind" field should be either 'full_auto', 'full_manual' or 'none', but got ${containerAppOptional.rollout_kind}`
2482-
);
2483-
}
2484-
24852476
// Leaving for legacy reasons
24862477
// TODO: When cleaning up container.configuration usage in other places clean this up
24872478
// as well.
@@ -2490,14 +2481,99 @@ function validateContainerApp(
24902481
`"containers.configuration" is defined as an array, it should be an object`
24912482
);
24922483
}
2493-
if ("instance_type" in containerAppOptional) {
2494-
validateOptionalProperty(
2495-
diagnostics,
2496-
field,
2484+
validateOptionalProperty(
2485+
diagnostics,
2486+
field,
2487+
"rollout_kind",
2488+
containerAppOptional.rollout_kind,
2489+
"string",
2490+
["full_auto", "full_manual", "none"]
2491+
);
2492+
validateOptionalProperty(
2493+
diagnostics,
2494+
field,
2495+
"instance_type",
2496+
containerAppOptional.instance_type,
2497+
"string",
2498+
["dev", "basic", "standard"]
2499+
);
2500+
validateOptionalProperty(
2501+
diagnostics,
2502+
field,
2503+
"max_instances",
2504+
containerAppOptional.max_instances,
2505+
"number"
2506+
);
2507+
if (
2508+
containerAppOptional.max_instances !== undefined &&
2509+
containerAppOptional.max_instances < 0
2510+
) {
2511+
diagnostics.errors.push(
2512+
`"containers.max_instances" field should be a positive number, but got ${containerAppOptional.max_instances}`
2513+
);
2514+
}
2515+
validateOptionalProperty(
2516+
diagnostics,
2517+
field,
2518+
"image_build_context",
2519+
containerAppOptional.image_build_context,
2520+
"string"
2521+
);
2522+
validateOptionalProperty(
2523+
diagnostics,
2524+
field,
2525+
"image_vars",
2526+
containerAppOptional.image_vars,
2527+
"object"
2528+
);
2529+
validateOptionalProperty(
2530+
diagnostics,
2531+
field,
2532+
"scheduling_policy",
2533+
containerAppOptional.scheduling_policy,
2534+
"string",
2535+
["regional", "moon", "default"]
2536+
);
2537+
2538+
// Add deprecation warnings for legacy fields
2539+
if ("instances" in containerAppOptional) {
2540+
diagnostics.warnings.push(
2541+
`"containers.instances" is deprecated. Use "containers.max_instances" instead.`
2542+
);
2543+
}
2544+
if ("durable_objects" in containerAppOptional) {
2545+
diagnostics.warnings.push(
2546+
`"containers.durable_objects" is deprecated. Use the "class_name" field instead.`
2547+
);
2548+
}
2549+
2550+
validateAdditionalProperties(
2551+
diagnostics,
2552+
field,
2553+
Object.keys(containerAppOptional),
2554+
[
2555+
"name",
2556+
"instances",
2557+
"max_instances",
2558+
"image",
2559+
"image_build_context",
2560+
"image_vars",
2561+
"class_name",
2562+
"scheduling_policy",
24972563
"instance_type",
2498-
containerAppOptional.instance_type,
2499-
"string",
2500-
["dev", "basic", "standard"]
2564+
"configuration",
2565+
"constraints",
2566+
"rollout_step_percentage",
2567+
"rollout_kind",
2568+
"durable_objects",
2569+
]
2570+
);
2571+
if ("configuration" in containerAppOptional) {
2572+
validateAdditionalProperties(
2573+
diagnostics,
2574+
`${field}.configuration`,
2575+
Object.keys(containerAppOptional.configuration),
2576+
["image", "secrets", "labels", "disk", "vcpu", "memory_mib"]
25012577
);
25022578
}
25032579
}

0 commit comments

Comments
 (0)