diff --git a/.changeset/khaki-steaks-sniff.md b/.changeset/khaki-steaks-sniff.md new file mode 100644 index 000000000000..e24422604da6 --- /dev/null +++ b/.changeset/khaki-steaks-sniff.md @@ -0,0 +1,10 @@ +--- +"wrangler": patch +--- + +fix: ensure that non-inherited fields are not removed when using an inferred named environment + +It is an error for the the user to provide an environment name that doesn't match any of the named environments in the Wrangler configuration. +But if there are no named environments defined at all in the Wrangler configuration, we special case the top-level environment as though it was a named environment. +Previously, when this happens, we would remove all the nonInheritable fields from the configuration (essentially all the bindings) leaving an incorrect configuration. +Now we correctly generate a flattened named environment that has the nonInheritable fields, plus correctly applies any transformFn on inheritable fields. diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts index 9ab76328d5d3..f20b25fbb28e 100644 --- a/packages/wrangler/src/__tests__/configuration.test.ts +++ b/packages/wrangler/src/__tests__/configuration.test.ts @@ -3854,22 +3854,35 @@ describe("normalizeAndValidateConfig()", () => { describe("named environments", () => { it("should warn if we specify an environment but there are no named environments", () => { - const rawConfig: RawConfig = {}; - const { diagnostics } = normalizeAndValidateConfig(rawConfig, undefined, { - env: "DEV", - }); + const rawConfig: RawConfig = { + name: "my-worker", + kv_namespaces: [{ binding: "KV", id: "xxxx-xxxx-xxxx-xxxx" }], + }; + const { diagnostics, config } = normalizeAndValidateConfig( + rawConfig, + undefined, + { + env: "dev", + } + ); + expect(config).toEqual( + expect.objectContaining({ + name: "my-worker-dev", + kv_namespaces: [{ binding: "KV", id: "xxxx-xxxx-xxxx-xxxx" }], + }) + ); expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` "Processing wrangler configuration: " `); expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` "Processing wrangler configuration: - - No environment found in configuration with name \\"DEV\\". - Before using \`--env=DEV\` there should be an equivalent environment section in the configuration. + - No environment found in configuration with name \\"dev\\". + Before using \`--env=dev\` there should be an equivalent environment section in the configuration. Consider adding an environment configuration section to the Wrangler configuration file: \`\`\` - [env.DEV] + [env.dev] \`\`\` " `); @@ -4111,70 +4124,70 @@ describe("normalizeAndValidateConfig()", () => { Please remove the field from this environment." `); }); - }); - it("should error if named environment contains a `account_id` field, even if there is no top-level name", () => { - const rawConfig: RawConfig = { - legacy_env: false, - env: { - DEV: { - account_id: "some_account_id", + it("should error if named environment contains a `account_id` field, even if there is no top-level name", () => { + const rawConfig: RawConfig = { + legacy_env: false, + env: { + DEV: { + account_id: "some_account_id", + }, }, - }, - }; + }; - const { config, diagnostics } = normalizeAndValidateConfig( - rawConfig, - undefined, - { env: "DEV" } - ); + const { config, diagnostics } = normalizeAndValidateConfig( + rawConfig, + undefined, + { env: "DEV" } + ); - expect(diagnostics.hasWarnings()).toBe(true); - expect(config.account_id).toBeUndefined(); - expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` - "Processing wrangler configuration: - - Experimental: Service environments are in beta, and their behaviour is guaranteed to change in the future. DO NOT USE IN PRODUCTION." - `); - expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` - "Processing wrangler configuration: + expect(diagnostics.hasWarnings()).toBe(true); + expect(config.account_id).toBeUndefined(); + expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - Experimental: Service environments are in beta, and their behaviour is guaranteed to change in the future. DO NOT USE IN PRODUCTION." + `); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: - - \\"env.DEV\\" environment configuration - - The \\"account_id\\" field is not allowed in named service environments. - Please remove the field from this environment." - `); - }); + - \\"env.DEV\\" environment configuration + - The \\"account_id\\" field is not allowed in named service environments. + Please remove the field from this environment." + `); + }); - it("should error if top-level config and a named environment both contain a `account_id` field", () => { - const rawConfig: RawConfig = { - account_id: "ACCOUNT_ID", - legacy_env: false, - env: { - DEV: { - account_id: "ENV_ACCOUNT_ID", + it("should error if top-level config and a named environment both contain a `account_id` field", () => { + const rawConfig: RawConfig = { + account_id: "ACCOUNT_ID", + legacy_env: false, + env: { + DEV: { + account_id: "ENV_ACCOUNT_ID", + }, }, - }, - }; + }; - const { config, diagnostics } = normalizeAndValidateConfig( - rawConfig, - undefined, - { env: "DEV" } - ); + const { config, diagnostics } = normalizeAndValidateConfig( + rawConfig, + undefined, + { env: "DEV" } + ); - expect(config.account_id).toEqual("ACCOUNT_ID"); - expect(diagnostics.hasErrors()).toBe(true); - expect(diagnostics.hasWarnings()).toBe(true); - expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` - "Processing wrangler configuration: - - Experimental: Service environments are in beta, and their behaviour is guaranteed to change in the future. DO NOT USE IN PRODUCTION." - `); - expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` - "Processing wrangler configuration: + expect(config.account_id).toEqual("ACCOUNT_ID"); + expect(diagnostics.hasErrors()).toBe(true); + expect(diagnostics.hasWarnings()).toBe(true); + expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(` + "Processing wrangler configuration: + - Experimental: Service environments are in beta, and their behaviour is guaranteed to change in the future. DO NOT USE IN PRODUCTION." + `); + expect(diagnostics.renderErrors()).toMatchInlineSnapshot(` + "Processing wrangler configuration: - - \\"env.DEV\\" environment configuration - - The \\"account_id\\" field is not allowed in named service environments. - Please remove the field from this environment." - `); + - \\"env.DEV\\" environment configuration + - The \\"account_id\\" field is not allowed in named service environments. + Please remove the field from this environment." + `); + }); }); it("should warn for non-inherited fields that are missing in environments", () => { diff --git a/packages/wrangler/src/config/validation-helpers.ts b/packages/wrangler/src/config/validation-helpers.ts index 9d38a7ea9377..fd109f095be6 100644 --- a/packages/wrangler/src/config/validation-helpers.ts +++ b/packages/wrangler/src/config/validation-helpers.ts @@ -69,7 +69,11 @@ export function inheritable( ): Environment[K] { validate(diagnostics, field, rawEnv[field], topLevelEnv); return ( - (rawEnv[field] as Environment[K]) ?? + // `rawEnv === topLevelEnv` is a special case where the user has provided an environment name + // but that named environment is not actually defined in the configuration. + // In that case we have reused the topLevelEnv as the rawEnv, + // and so we need to process the `transformFn()` anyway rather than just using the field in the `rawEnv`. + (rawEnv !== topLevelEnv ? (rawEnv[field] as Environment[K]) : undefined) ?? transformFn(topLevelEnv?.[field]) ?? defaultValue ); diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index b2837f7b42c7..b4f2f8343b1b 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -224,7 +224,7 @@ export function normalizeAndValidateConfig( activeEnv = normalizeAndValidateEnvironment( envDiagnostics, configPath, - {}, + topLevelEnv, // in this case reuse the topLevelEnv to ensure that nonInherited fields are not removed isDispatchNamespace, envName, topLevelEnv,