Skip to content

Commit

Permalink
Prevents uploads with both cron triggers and smart placement enabled (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
shahsimpson authored Jun 3, 2023
1 parent a728876 commit b5b46b4
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/rich-tips-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Prevents uploads with both cron triggers and smart placement enabled
56 changes: 55 additions & 1 deletion packages/wrangler/src/__tests__/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ describe("normalizeAndValidateConfig()", () => {
first_party_worker: true,
logpush: true,
placement: {
mode: "smart",
mode: "off",
},
};

Expand Down Expand Up @@ -4547,6 +4547,60 @@ describe("normalizeAndValidateConfig()", () => {
});
});

describe("[placement]", () => {
it("should error if both placement and triggers are configured", () => {
const { diagnostics } = normalizeAndValidateConfig(
{
triggers: {
crons: [1111],
},
placement: { mode: "smart" },
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(diagnostics.hasWarnings()).toBe(false);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- You cannot configure both [triggers] and [placement] in your wrangler.toml. Placement is not supported with cron triggers."
`);
});
it("should not error if triggers are configured and placement is set off", () => {
const { diagnostics } = normalizeAndValidateConfig(
{
triggers: {
crons: [1111],
},
placement: { mode: "off" },
} as unknown as RawConfig,
undefined,
{ env: undefined }
);

expect(diagnostics.hasWarnings()).toBe(false);
expect(diagnostics.hasErrors()).toBe(false);
});
it("should not error if placement is configured and triggers is empty array", () => {
const expectedConfig: RawEnvironment = {
triggers: { crons: [] },
placement: {
mode: "smart",
},
};
const { config, diagnostics } = normalizeAndValidateConfig(
expectedConfig,
undefined,
{ env: undefined }
);

expect(config).toEqual(expect.objectContaining({ ...expectedConfig }));

expect(diagnostics.hasWarnings()).toBe(false);
expect(diagnostics.hasErrors()).toBe(false);
});
});

describe("(deprecated)", () => {
it("should remove and warn about deprecated properties", () => {
const environment: RawEnvironment = {
Expand Down
21 changes: 21 additions & 0 deletions packages/wrangler/src/config/validation-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,27 @@ export const validateAdditionalProperties = (
return true;
};

/**
* Add a diagnostic error for any configurations that include both cron triggers and smart placement
*/
export const validateSmartPlacementConfig = (
diagnostics: Diagnostics,
placement: { mode: "off" | "smart" } | undefined,
triggers:
| {
crons: string[];
}
| undefined
): boolean => {
if (placement?.mode === "smart" && !!triggers?.crons?.length) {
diagnostics.errors.push(
`You cannot configure both [triggers] and [placement] in your wrangler.toml. Placement is not supported with cron triggers.`
);
return false;
}
return true;
};

/**
* Get the names of the bindings collection in `value`.
*
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/src/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
appendEnvName,
getBindingNames,
isValidName,
validateSmartPlacementConfig,
} from "./validation-helpers";
import type { Config, DevConfig, RawConfig, RawDevConfig } from "./config";
import type {
Expand Down Expand Up @@ -237,6 +238,8 @@ export function normalizeAndValidateConfig(
[...Object.keys(config), "env"]
);

validateSmartPlacementConfig(diagnostics, config.placement, config.triggers);

experimental(diagnostics, rawConfig, "assets");

return { config, diagnostics };
Expand Down

0 comments on commit b5b46b4

Please sign in to comment.