From 3c7b9ae0b9584abdff0ba059a827836018b8434e Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Thu, 10 Jul 2025 23:08:31 +0100 Subject: [PATCH 1/7] add some extra validation --- packages/wrangler/src/config/validation.ts | 86 ++++++++++++++++------ 1 file changed, 64 insertions(+), 22 deletions(-) diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 3fbcd097cf75..0d7a0b00b5eb 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2469,19 +2469,6 @@ function validateContainerApp( `"containers.rollout_step_percentage" field should be a number between 25 and 100, but got ${containerAppOptional.rollout_step_percentage}` ); } - - if ( - !isOptionalProperty(containerAppOptional, "rollout_kind", "string") && - "rollout_kind" in containerAppOptional && - !["full_auto", "full_manual", "none"].includes( - containerAppOptional.rollout_kind - ) - ) { - diagnostics.errors.push( - `"containers.rollout_kind" field should be either 'full_auto', 'full_manual' or 'none', but got ${containerAppOptional.rollout_kind}` - ); - } - // Leaving for legacy reasons // TODO: When cleaning up container.configuration usage in other places clean this up // as well. @@ -2490,16 +2477,71 @@ function validateContainerApp( `"containers.configuration" is defined as an array, it should be an object` ); } - if ("instance_type" in containerAppOptional) { - validateOptionalProperty( - diagnostics, - field, + validateOptionalProperty( + diagnostics, + field, + "rollout_kind", + containerAppOptional.rollout_kind, + "string", + ["full_auto", "full_manual", "none"] + ); + validateOptionalProperty( + diagnostics, + field, + "instance_type", + containerAppOptional.instance_type, + "string", + ["dev", "basic", "standard"] + ); + validateOptionalProperty( + diagnostics, + field, + "max_instances", + containerAppOptional.max_instances, + "number" + ); + validateOptionalProperty( + diagnostics, + field, + "image_build_context", + containerAppOptional.image_build_context, + "string" + ); + validateOptionalProperty( + diagnostics, + field, + "image_vars", + containerAppOptional.image_vars, + "object" + ); + validateOptionalProperty( + diagnostics, + field, + "scheduling_policy", + containerAppOptional.scheduling_policy, + "string", + ["regional", "moon", "default"] + ); + validateAdditionalProperties( + diagnostics, + field, + Object.keys(containerAppOptional), + [ + "name", + "instances", + "max_instances", + "image", + "image_build_context", + "image_vars", + "class_name", + "scheduling_policy", "instance_type", - containerAppOptional.instance_type, - "string", - ["dev", "basic", "standard"] - ); - } + "configuration", + "constraints", + "rollout_step_percentage", + "rollout_kind", + ] + ); } if (diagnostics.errors.length > 0) { From 8195d2df36e4ac97789bcca2dc8cb2318cb9e460 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 11 Jul 2025 11:07:22 +0100 Subject: [PATCH 2/7] add test --- .../__tests__/config/configuration.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 3b22320025a8..9d316c6b6f55 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2421,6 +2421,44 @@ describe("normalizeAndValidateConfig()", () => { ); } }); + + it("should error for invalid container app fields", () => { + const { diagnostics } = normalizeAndValidateConfig( + { + name: "test-worker", + containers: [ + { + image: "something", + class_name: "test-class", + rollout_kind: "invalid", + instance_type: "invalid", + max_instances: "invalid", + image_build_context: 123, + image_vars: "invalid", + scheduling_policy: "invalid", + unknown_field: "value", + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - Unexpected fields found in containers field: \\"unknown_field\\"" + `); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - Expected \\"containers.rollout_kind\\" field to be one of [\\"full_auto\\",\\"full_manual\\",\\"none\\"] but got \\"invalid\\". + - Expected \\"containers.instance_type\\" field to be one of [\\"dev\\",\\"basic\\",\\"standard\\"] but got \\"invalid\\". + - Expected \\"containers.max_instances\\" to be of type number but got \\"invalid\\". + - Expected \\"containers.image_build_context\\" to be of type string but got 123. + - Expected \\"containers.image_vars\\" to be of type object but got \\"invalid\\". + - Expected \\"containers.scheduling_policy\\" field to be one of [\\"regional\\",\\"moon\\",\\"default\\"] but got \\"invalid\\"." + `); + }); }); describe("[kv_namespaces]", () => { From 24e4c955a4ecee78e490bef10cfa01f81f28c577 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 11 Jul 2025 11:29:02 +0100 Subject: [PATCH 3/7] add deprecation warnings --- .../src/__tests__/cloudchamber/deploy.test.ts | 22 +++++++------- .../__tests__/config/configuration.test.ts | 30 +++++++++++++++++++ packages/wrangler/src/config/validation.ts | 20 ++++++++++++- 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts b/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts index 0229f045f5ac..68f24f1f2542 100644 --- a/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts +++ b/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts @@ -96,7 +96,7 @@ describe("wrangler deploy with containers", () => { containers: [ { name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", image: "./Dockerfile", }, @@ -147,7 +147,7 @@ describe("wrangler deploy with containers", () => { containers: [ { name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", image: "./Dockerfile", }, @@ -161,7 +161,7 @@ describe("wrangler deploy with containers", () => { mockCreateApplication({ name: "my-container", - instances: 10, + max_instances: 10, durable_objects: { namespace_id: "1" }, configuration: { image: @@ -206,7 +206,7 @@ describe("wrangler deploy with containers", () => { { image: "docker.io/hello:world", name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", }, ], @@ -217,7 +217,7 @@ describe("wrangler deploy with containers", () => { mockCreateApplication({ name: "my-container", - instances: 10, + max_instances: 10, durable_objects: { namespace_id: "1" }, scheduling_policy: SchedulingPolicy.DEFAULT, }); @@ -287,7 +287,7 @@ describe("wrangler deploy with containers", () => { containers: [ { name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", image: "../Dockerfile", }, @@ -311,7 +311,7 @@ describe("wrangler deploy with containers", () => { mockCreateApplication({ name: "my-container", - instances: 10, + max_instances: 10, durable_objects: { namespace_id: "1" }, configuration: { image: @@ -391,7 +391,7 @@ describe("wrangler deploy with containers", () => { containers: [ { name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", image: "../Dockerfile", }, @@ -408,7 +408,7 @@ describe("wrangler deploy with containers", () => { mockCreateApplication({ name: "my-container", - instances: 10, + max_instances: 10, durable_objects: { namespace_id: "1" }, configuration: { image: @@ -453,7 +453,7 @@ describe("wrangler deploy with containers", () => { { image: "docker.io/hello:world", name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", }, ], @@ -515,7 +515,7 @@ describe("wrangler deploy with containers dry run", () => { { image: "./Dockerfile", name: "my-container", - instances: 10, + max_instances: 10, class_name: "ExampleDurableObject", }, ], diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 9d316c6b6f55..668c3df9866c 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2459,6 +2459,36 @@ describe("normalizeAndValidateConfig()", () => { - Expected \\"containers.scheduling_policy\\" field to be one of [\\"regional\\",\\"moon\\",\\"default\\"] but got \\"invalid\\"." `); }); + + it("should warn for deprecated container fields", () => { + const { diagnostics } = normalizeAndValidateConfig( + { + name: "test-worker", + containers: [ + { + class_name: "test-class", + instances: 10, + configuration: { + image: "config-image" + }, + durable_objects: { + namespace_id: "test-namespace" + } + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - \\"containers.configuration\\" is deprecated. Use top level \\"containers\\" fields instead. \\"configuration.image\\" should be \\"image\\", \\"configuration.disk\\" should be set via \\"instance_type\\". + - \\"containers.instances\\" is deprecated. Use \\"containers.max_instances\\" instead. + - \\"containers.durable_objects\\" is deprecated. Use the \\"class_name\\" field instead." + `); + }); }); describe("[kv_namespaces]", () => { diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 0d7a0b00b5eb..6a33cf812719 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2421,7 +2421,6 @@ function validateContainerApp( name += config === undefined ? "" : `-${envName}`; containerAppOptional.name = name.toLowerCase().replace(/ /g, "-"); } - if ( !containerAppOptional.configuration?.image && !containerAppOptional.image @@ -2430,6 +2429,11 @@ function validateContainerApp( `"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.` ); } + if ("configuration" in containerAppOptional) { + diagnostics.warnings.push( + `"containers.configuration" is deprecated. Use top level "containers" fields instead. "configuration.image" should be "image", "configuration.disk" should be set via "instance_type".` + ); + } // Validate that we have an image configuration for this container app. // For legacy reasons we have to check both at containerAppOptional.image and @@ -2522,6 +2526,19 @@ function validateContainerApp( "string", ["regional", "moon", "default"] ); + + // Add deprecation warnings for legacy fields + if ("instances" in containerAppOptional) { + diagnostics.warnings.push( + `"containers.instances" is deprecated. Use "containers.max_instances" instead.` + ); + } + if ("durable_objects" in containerAppOptional) { + diagnostics.warnings.push( + `"containers.durable_objects" is deprecated. Use the "class_name" field instead.` + ); + } + validateAdditionalProperties( diagnostics, field, @@ -2540,6 +2557,7 @@ function validateContainerApp( "constraints", "rollout_step_percentage", "rollout_kind", + "durable_objects", ] ); } From 2e505f8616e2480c0e7d4d3fbeb94aaef67467e1 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 11 Jul 2025 11:35:52 +0100 Subject: [PATCH 4/7] changeset --- .changeset/clean-colts-vanish.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/clean-colts-vanish.md diff --git a/.changeset/clean-colts-vanish.md b/.changeset/clean-colts-vanish.md new file mode 100644 index 000000000000..76d11b893d86 --- /dev/null +++ b/.changeset/clean-colts-vanish.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Add more thorough validation to containers configuration From af4dbde032ab4b647a0790d8baa7cc822c0630f9 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Fri, 11 Jul 2025 13:17:09 +0100 Subject: [PATCH 5/7] lint --- .../wrangler/src/__tests__/config/configuration.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 668c3df9866c..7aef842a7ced 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2469,11 +2469,11 @@ describe("normalizeAndValidateConfig()", () => { class_name: "test-class", instances: 10, configuration: { - image: "config-image" + image: "config-image", }, durable_objects: { - namespace_id: "test-namespace" - } + namespace_id: "test-namespace", + }, }, ], } as unknown as RawConfig, From 65c2c190f4021d211ca0101f2a13cfef862b9a87 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 14 Jul 2025 10:17:25 +0100 Subject: [PATCH 6/7] validate nested configuration --- .../__tests__/config/configuration.test.ts | 34 +++++++++++++++++++ packages/wrangler/src/config/validation.ts | 8 +++++ 2 files changed, 42 insertions(+) diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 7aef842a7ced..851d00fb7a51 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2489,6 +2489,40 @@ describe("normalizeAndValidateConfig()", () => { - \\"containers.durable_objects\\" is deprecated. Use the \\"class_name\\" field instead." `); }); + + it("should error for invalid containers.configuration fields", () => { + const { diagnostics } = normalizeAndValidateConfig( + { + name: "test-worker", + containers: [ + { + class_name: "test-class", + configuration: { + image: "config-image", + secrets: [], + labels: [], + disk: { size: "2GB" }, + memory: "256MB", + vcpu: 0.5, + memory_mib: 256, + invalid_field: "should not be here", + another_invalid: 123, + }, + }, + ], + } as unknown as RawConfig, + undefined, + undefined, + { env: undefined } + ); + + console.dir(diagnostics.warnings); + expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - \\"containers.configuration\\" is deprecated. Use top level \\"containers\\" fields instead. \\"configuration.image\\" should be \\"image\\", \\"configuration.disk\\" should be set via \\"instance_type\\". + - Unexpected fields found in containers.configuration field: \\"invalid_field\\",\\"another_invalid\\"" + `); + }); }); describe("[kv_namespaces]", () => { diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 6a33cf812719..d7c097510855 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2560,6 +2560,14 @@ function validateContainerApp( "durable_objects", ] ); + if ("configuration" in containerAppOptional) { + validateAdditionalProperties( + diagnostics, + `${field}.configuration`, + Object.keys(containerAppOptional.configuration), + ["image", "secrets", "labels", "disk", "memory", "vcpu", "memory_mib"] + ); + } } if (diagnostics.errors.length > 0) { From ddbc4dcbab41b277bc49e9f2434300f0ae7c6aef Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 10:02:01 +0100 Subject: [PATCH 7/7] pr feedback --- .../src/__tests__/config/configuration.test.ts | 2 +- packages/wrangler/src/config/validation.ts | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 851d00fb7a51..01f8f3a7ae9a 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2520,7 +2520,7 @@ describe("normalizeAndValidateConfig()", () => { expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` "Processing wrangler configuration: - \\"containers.configuration\\" is deprecated. Use top level \\"containers\\" fields instead. \\"configuration.image\\" should be \\"image\\", \\"configuration.disk\\" should be set via \\"instance_type\\". - - Unexpected fields found in containers.configuration field: \\"invalid_field\\",\\"another_invalid\\"" + - Unexpected fields found in containers.configuration field: \\"memory\\",\\"invalid_field\\",\\"another_invalid\\"" `); }); }); diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index d7c097510855..33965f72e376 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2504,6 +2504,14 @@ function validateContainerApp( containerAppOptional.max_instances, "number" ); + if ( + containerAppOptional.max_instances !== undefined && + containerAppOptional.max_instances < 0 + ) { + diagnostics.errors.push( + `"containers.max_instances" field should be a positive number, but got ${containerAppOptional.max_instances}` + ); + } validateOptionalProperty( diagnostics, field, @@ -2565,7 +2573,7 @@ function validateContainerApp( diagnostics, `${field}.configuration`, Object.keys(containerAppOptional.configuration), - ["image", "secrets", "labels", "disk", "memory", "vcpu", "memory_mib"] + ["image", "secrets", "labels", "disk", "vcpu", "memory_mib"] ); } }