Skip to content

Validate environments for unexpected fields, not just top level env #11969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
emily-shen wants to merge 1 commit into
base: main
Choose a base branch
from
+129 −96
Open

Validate environments for unexpected fields, not just top level env #11969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tidy-plants-thank.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@cloudflare/workers-utils": patch
---

Validate environments for unexpected fields in Wrangler config

Previously, this check only applied to the top-level environment.
9 changes: 9 additions & 0 deletions packages/workers-utils/src/config/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,15 @@ function normalizeAndValidateEnvironment(
configPath
);

// top level 'rawEnv' includes inheritable keys and is validated elsewhere
if (envName !== "top level") {
validateAdditionalProperties(
diagnostics,
"env." + envName,
Object.keys(rawEnv),
Object.keys(environment)
);
}
return environment;
}

Expand Down
209 changes: 113 additions & 96 deletions packages/workers-utils/tests/config/validation/normalize-and-validate-config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,13 +438,13 @@ describe("normalizeAndValidateConfig()", () => {

expect(normalizeString(diagnostics.renderWarnings()))
.toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"my-site/index.js\\"
\`\`\`"
`);
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"my-site/index.js\\"
\`\`\`"
`);
});

it("should error if `site` config is missing `bucket`", () => {
Expand All @@ -471,13 +471,13 @@ describe("normalizeAndValidateConfig()", () => {

expect(normalizeString(diagnostics.renderWarnings()))
.toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"workers-site/index.js\\"
\`\`\`"
`);
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"workers-site/index.js\\"
\`\`\`"
`);

expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
Expand Down Expand Up @@ -516,13 +516,13 @@ describe("normalizeAndValidateConfig()", () => {

expect(normalizeString(diagnostics.renderWarnings()))
.toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"111/index.js\\"
\`\`\`"
`);
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"111/index.js\\"
\`\`\`"
`);
});

it("should log a deprecation warning if entry-point is defined", async () => {
Expand Down Expand Up @@ -552,13 +552,13 @@ describe("normalizeAndValidateConfig()", () => {

expect(normalizeString(diagnostics.renderWarnings()))
.toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"some/other/script.js\\"
\`\`\`"
`);
"Processing wrangler configuration:
- Deprecation: \\"site.entry-point\\":
Delete the \`site.entry-point\` field, then add the top level \`main\` field to your configuration file:
\`\`\`
main = \\"some/other/script.js\\"
\`\`\`"
`);
});
});

Expand Down Expand Up @@ -5182,9 +5182,9 @@ describe("normalizeAndValidateConfig()", () => {
"
`);
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"
`);
"Processing wrangler configuration:
"
`);
});

it("should use top-level values for inheritable config fields", () => {
Expand Down Expand Up @@ -5678,6 +5678,23 @@ describe("normalizeAndValidateConfig()", () => {
`);
});

it("should warn on unexpected fields", () => {
const { diagnostics } = normalizeAndValidateConfig(
// @ts-expect-error purposely using an invalid field
{ env: { ENV1: { bla: "haj" } } },
undefined,
undefined,
{ env: "ENV1" }
);
expect(diagnostics.hasWarnings()).toBe(true);
expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- Unexpected fields found in env.ENV1 field: \\"bla\\""
`);
});

describe("[define]", () => {
it("should accept valid values for config.define", () => {
const rawConfig: RawConfig = {
Expand Down Expand Up @@ -5846,16 +5863,16 @@ describe("normalizeAndValidateConfig()", () => {
expect(diagnostics.hasErrors()).toBe(false);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"define.ghi\\" exists at the top level, but not on \\"env.ENV1.define\\".
This is not what you probably want, since \\"define\\" configuration is not inherited by environments.
Please add \\"define.ghi\\" to \\"env.ENV1\\".
- \\"xyz\\" exists on \\"env.ENV1\\", but not on the top level.
This is not what you probably want, since \\"define\\" configuration within environments can only override existing top level \\"define\\" configuration
Please remove \\"env.ENV1.define.xyz\\", or add \\"define.xyz\\"."
`);
- \\"env.ENV1\\" environment configuration
- \\"define.ghi\\" exists at the top level, but not on \\"env.ENV1.define\\".
This is not what you probably want, since \\"define\\" configuration is not inherited by environments.
Please add \\"define.ghi\\" to \\"env.ENV1\\".
- \\"xyz\\" exists on \\"env.ENV1\\", but not on the top level.
This is not what you probably want, since \\"define\\" configuration within environments can only override existing top level \\"define\\" configuration
Please remove \\"env.ENV1.define.xyz\\", or add \\"define.xyz\\"."
`);
});

it("should error if the values on config.define in an environment are not strings", () => {
Expand Down Expand Up @@ -6572,11 +6589,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6594,11 +6611,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6616,11 +6633,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6638,11 +6655,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand Down Expand Up @@ -6682,11 +6699,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.hasErrors()).toBe(false);
});

Expand Down Expand Up @@ -6724,11 +6741,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6748,11 +6765,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6772,11 +6789,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6796,11 +6813,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand Down Expand Up @@ -6835,11 +6852,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand Down Expand Up @@ -6869,11 +6886,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6893,11 +6910,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6917,11 +6934,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand All @@ -6941,11 +6958,11 @@ describe("normalizeAndValidateConfig()", () => {
);

expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
"Processing wrangler configuration:

- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
- \\"env.ENV1\\" environment configuration
- \\"unsafe\\" fields are experimental and may change or break at any time."
`);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:

Expand Down
Loading