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 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 3b22320025a8..01f8f3a7ae9a 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2421,6 +2421,108 @@ 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\\"." + `); + }); + + 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." + `); + }); + + 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: \\"memory\\",\\"invalid_field\\",\\"another_invalid\\"" + `); + }); }); describe("[kv_namespaces]", () => { diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 3fbcd097cf75..33965f72e376 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 @@ -2469,19 +2473,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,14 +2481,99 @@ 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" + ); + 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, + "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"] + ); + + // 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, + 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", + "durable_objects", + ] + ); + if ("configuration" in containerAppOptional) { + validateAdditionalProperties( + diagnostics, + `${field}.configuration`, + Object.keys(containerAppOptional.configuration), + ["image", "secrets", "labels", "disk", "vcpu", "memory_mib"] ); } }