-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[wrangler] Warn when compliance region environment variable overrides config #15058
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.
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| "@cloudflare/workers-utils": patch | ||
| "wrangler": patch | ||
| --- | ||
|
|
||
| Warn when `CLOUDFLARE_COMPLIANCE_REGION` overrides a conflicting configured compliance region | ||
|
|
||
| Wrangler now explains that the environment variable takes precedence and continues using its value instead of rejecting the command. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,4 @@ | ||
| import path from "node:path"; | ||
| import { dedent } from "ts-dedent"; | ||
| import { UserError } from "../errors"; | ||
| import { getGlobalConfigPath } from "../global-wrangler-config-path"; | ||
| import { | ||
| getBooleanEnvironmentVariableFactory, | ||
|
|
@@ -101,20 +99,6 @@ export const getCloudflareComplianceRegion = ( | |
| complianceConfig: ComplianceConfig | ||
| ) => { | ||
| const complianceRegionFromEnv = getCloudflareComplianceRegionFromEnv(); | ||
| if ( | ||
| complianceRegionFromEnv !== undefined && | ||
| complianceConfig?.compliance_region !== undefined && | ||
| complianceRegionFromEnv !== complianceConfig.compliance_region | ||
| ) { | ||
| throw new UserError( | ||
| dedent` | ||
| The compliance region has been set to different values in two places: | ||
| - \`CLOUDFLARE_COMPLIANCE_REGION\` environment variable: \`${complianceRegionFromEnv}\` | ||
| - \`compliance_region\` configuration property: \`${complianceConfig.compliance_region}\` | ||
| `, | ||
| { telemetryMessage: false } | ||
| ); | ||
| } | ||
| return ( | ||
| complianceRegionFromEnv || complianceConfig?.compliance_region || "public" | ||
| ); | ||
|
Comment on lines
102
to
104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Commands that only work in the public region can now silently talk to the FedRAMP endpoint Commands that declare they only support the public region now have that declaration silently overridden by the environment variable ( Why the public-only marker is no longer honouredMany call sites pass the Previously, when The new warning added in Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines
100
to
104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Account info command reports the wrong source for the compliance region The compliance region source shown by the account info output is chosen purely from the configured value ( How removing the conflict error exposes the mislabelling in printComplianceRegionBefore this PR, A fix would be to compare the resolved region against the configured one (as (Refers to lines 100-105) Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
100
to
104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟨 Conflicting compliance region settings no longer block the command, allowing FedRAMP-configured projects to target the public region The hard error that previously stopped Wrangler when (Refers to lines 100-105) Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { | |
| configFileName, | ||
| experimental_readRawConfig, | ||
| FatalError, | ||
| getCloudflareComplianceRegion, | ||
| isPagesConfig, | ||
| normalizeAndValidateConfig, | ||
| UserError, | ||
|
|
@@ -68,6 +69,25 @@ async function logWarningsWithUpgradeHint( | |
| } | ||
| } | ||
|
|
||
| function logWarningsIfComplianceRegionIsOverridden( | ||
| config: Config, | ||
| hideWarnings: boolean | undefined | ||
| ): void { | ||
| const configuredRegion = config.compliance_region; | ||
| if (hideWarnings || configuredRegion === undefined) { | ||
| return; | ||
| } | ||
|
|
||
| const resolvedRegion = getCloudflareComplianceRegion(config); | ||
| if (resolvedRegion === configuredRegion) { | ||
| return; | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: |
||
|
|
||
| logger.warn( | ||
| `The compliance region was resolved to "${resolvedRegion}" from the \`CLOUDFLARE_COMPLIANCE_REGION\` environment variable, which takes precedence over the configured value "${configuredRegion}".` | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Carries the validated `Config` alongside the | ||
| * watcher dependency set and the normalised type-generation settings. | ||
|
|
@@ -123,6 +143,7 @@ export async function readNewConfig( | |
| ); | ||
|
|
||
| void logWarningsWithUpgradeHint(diagnostics, options.hideWarnings); | ||
| logWarningsIfComplianceRegionIsOverridden(config, options.hideWarnings); | ||
| if (diagnostics.hasErrors()) { | ||
| throw new UserError(diagnostics.renderErrors(), { | ||
| telemetryMessage: "new-config worker validation failed", | ||
|
|
@@ -175,6 +196,7 @@ export function readConfig( | |
| ); | ||
|
|
||
| void logWarningsWithUpgradeHint(diagnostics, options?.hideWarnings); | ||
| logWarningsIfComplianceRegionIsOverridden(config, options.hideWarnings); | ||
| if (diagnostics.hasErrors()) { | ||
| throw new UserError(diagnostics.renderErrors(), { | ||
| telemetryMessage: "config wrangler validation failed", | ||
|
|
@@ -238,6 +260,7 @@ export function readPagesConfig( | |
| ); | ||
|
|
||
| void logWarningsWithUpgradeHint(diagnostics, options.hideWarnings); | ||
| logWarningsIfComplianceRegionIsOverridden(config, options.hideWarnings); | ||
| if (diagnostics.hasErrors()) { | ||
| throw new UserError(diagnostics.renderErrors(), { | ||
| telemetryMessage: "config pages validation failed", | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.