Skip to content

Commit

Permalink
fix(wrangler): enforce Pages config validation around DOs (#5666)
Browse files Browse the repository at this point in the history
Today Pages cannot deploy Durable Objects itself. For this
reason it is mandatory that when declaring Durable Objects
bindings in the config file, the `script_name` is specified.
We are currently not failing validation if `script_name` is
not specified but we should. This commit fixes that.

Co-authored-by: Carmen Popoviciu <cpopoviciu@cloudflare.com>
  • Loading branch information
CarmenPopoviciu and Carmen Popoviciu committed Apr 23, 2024
1 parent 235c439 commit 81d9615
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 1 deletion.
8 changes: 8 additions & 0 deletions .changeset/new-dolphins-dress.md
@@ -0,0 +1,8 @@
---
"wrangler": minor
---

fix: Fix Pages config validation around Durable Objects

Today Pages cannot deploy Durable Objects itself. For this reason it is mandatory that when declaring Durable Objects bindings in the config file, the `script_name` is specified. We are currently not failing validation if
`script_name` is not specified but we should. These changes fix that.
56 changes: 55 additions & 1 deletion packages/wrangler/src/__tests__/config-validation-pages.test.ts
Expand Up @@ -139,7 +139,11 @@ describe("validatePagesConfig()", () => {
vars: { FOO: "foo" },
durable_objects: {
bindings: [
{ name: "TEST_DO_BINDING", class_name: "TEST_DO_CLASS" },
{
name: "TEST_DO_BINDING",
class_name: "TEST_DO_CLASS",
script_name: "TEST_DO_SCRIPT",
},
],
},
kv_namespaces: [{ binding: "TEST_KV_BINDING", id: "1" }],
Expand Down Expand Up @@ -268,6 +272,56 @@ describe("validatePagesConfig()", () => {
`);
});
});

describe("DO bindings validation", () => {
it("should pass if all Durable Objects bindings specify 'script_name'", () => {
const config = generateConfigurationWithDefaults();
config.pages_build_output_dir = "./public";
config.name = "pages-project";
config.durable_objects.bindings = [
{
name: "foo-DO",
class_name: "foo-class",
script_name: "foo-script",
},
{
name: "bar-DO",
class_name: "bar-class",
script_name: "bar-script",
},
];

const diagnostics = validatePagesConfig(config, [], "pages-project");
expect(diagnostics.hasWarnings()).toBeFalsy();
expect(diagnostics.hasErrors()).toBeFalsy();
});

it("should fail if any of the Durable Object bindings does not specify 'script_name'", () => {
const config = generateConfigurationWithDefaults();
config.pages_build_output_dir = "./public";
config.name = "pages-project";
config.durable_objects.bindings = [
{
name: "foo-DO",
class_name: "foo-class",
script_name: "foo-script",
},
{
name: "bar-DO",
class_name: "bar-class",
},
];

const diagnostics = validatePagesConfig(config, [], "pages-project");
expect(diagnostics.hasWarnings()).toBeFalsy();
expect(diagnostics.hasErrors()).toBeTruthy();
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Running configuration file validation for Pages:
- Durable Objects bindings should specify a \\"script_name\\".
Pages requires Durable Object bindings to specify the name of the Worker where the Durable Object is defined."
`);
});
});
});

function generateConfigurationWithDefaults() {
Expand Down
25 changes: 25 additions & 0 deletions packages/wrangler/src/config/validation-pages.ts
Expand Up @@ -10,6 +10,7 @@
import { FatalError } from "../errors";
import { defaultWranglerConfig } from "./config";
import { Diagnostics } from "./diagnostics";
import { isRequiredProperty } from "./validation-helpers";
import type { Config } from "./config";

const supportedPagesConfigFields = [
Expand Down Expand Up @@ -56,6 +57,7 @@ export function validatePagesConfig(
validateProjectName(projectName, diagnostics);
validatePagesEnvironmentNames(envNames, diagnostics);
validateUnsupportedFields(config, diagnostics);
validateDurableObjectBinding(config, diagnostics);

return diagnostics;
}
Expand Down Expand Up @@ -159,3 +161,26 @@ function validateUnsupportedFields(config: Config, diagnostics: Diagnostics) {
});
}
}

/**
* Validate the "script_name" field is specified for [[durable_objects.bindings]]
*
* This is necessary because Pages cannot define/deploy a DO itself today,
* and so this needs to be done with a Worker.
*/
function validateDurableObjectBinding(
config: Config,
diagnostics: Diagnostics
) {
if (config.durable_objects.bindings.length > 0) {
const invalidBindings = config.durable_objects.bindings.filter(
(binding) => !isRequiredProperty(binding, "script_name", "string")
);
if (invalidBindings.length > 0) {
diagnostics.errors.push(
`Durable Objects bindings should specify a "script_name".\n` +
`Pages requires Durable Object bindings to specify the name of the Worker where the Durable Object is defined.`
);
}
}
}

0 comments on commit 81d9615

Please sign in to comment.