From f398ea06be95d3f58b3b19662efb99b4ebd48aa6 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 22 Dec 2021 19:44:24 +0000 Subject: [PATCH 1/6] Refactor how we pass bindings from config to the worker upload api. There are 3 types of 'bindings' that we upload with a worker definition: `vars`, `kv_namespaces`, and `durable_objects`. These are configured in `wrangler.toml`, and get passed through to the api that uploads a worker definition (in `form_data.ts`), when using `dev` and `publish` commands. We currently serialise it as a big `variables` object, where the key of the objects defines the binding "name", and the value contains information to be bound. (https://github.com/cloudflare/wrangler2/blob/0330ecf1b54c92dfe86cb3f38394f453ed418381/packages/wrangler/src/index.tsx#L507-L530) This code sucks for many reasons: it loses some type information, hard to read/grok, hard to add new types of bindings, and mostly it's unnecessary. This PR refactors this code to instead mostly preserve the configuration style structure all the way until we actually serialise it when uploading a worker definition. I also added some more types along the way, and cleared some dead code. I will follow up this PR with 2 PRs soon: one, which introduces service bindings (as a fresh take on https://github.com/cloudflare/wrangler2/pull/156), and another one that introduces tests for configuration. --- packages/wrangler/src/api/form_data.ts | 6 +++--- packages/wrangler/src/api/worker.ts | 1 - packages/wrangler/src/config.ts | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index 3adf347c3756..ab6931408c0c 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -36,11 +36,11 @@ interface WorkerMetadata { usage_model?: "bundled" | "unbound"; migrations?: CfDurableObjectMigrations; bindings: ( - | { type: "kv_namespace"; name: string; namespace_id: string } + | { type: "kv_namespace"; binding: string; namespace_id: string } | { type: "plain_text"; name: string; text: string } | { - type: "durable_object_namespace"; name: string; + type: "durable_object_namespace"; class_name: string; script_name?: string; } @@ -67,7 +67,7 @@ export function toFormData(worker: CfWorkerInit): FormData { bindings.kv_namespaces?.forEach(({ id, binding }) => { metadataBindings.push({ - name: binding, + binding, type: "kv_namespace", namespace_id: id, }); diff --git a/packages/wrangler/src/api/worker.ts b/packages/wrangler/src/api/worker.ts index ec101e07b28c..85ba56adee03 100644 --- a/packages/wrangler/src/api/worker.ts +++ b/packages/wrangler/src/api/worker.ts @@ -61,7 +61,6 @@ export interface CfModule { */ type?: CfModuleType; } - /** * A map of variable names to values. */ diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index 1af53f8114a8..3c24417da5b9 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -35,7 +35,7 @@ type KVNamespace = { id: string; }; -type DurableObject = { +export type DurableObject = { name: string; class_name: string; script_name?: string; From 9d566210fab3c2396f8411a34fe05d36229facfb Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 23 Dec 2021 11:49:16 +0000 Subject: [PATCH 2/6] correctly pass the name of a kv namespace when binding oops. --- packages/wrangler/src/api/form_data.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index ab6931408c0c..3adf347c3756 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -36,11 +36,11 @@ interface WorkerMetadata { usage_model?: "bundled" | "unbound"; migrations?: CfDurableObjectMigrations; bindings: ( - | { type: "kv_namespace"; binding: string; namespace_id: string } + | { type: "kv_namespace"; name: string; namespace_id: string } | { type: "plain_text"; name: string; text: string } | { - name: string; type: "durable_object_namespace"; + name: string; class_name: string; script_name?: string; } @@ -67,7 +67,7 @@ export function toFormData(worker: CfWorkerInit): FormData { bindings.kv_namespaces?.forEach(({ id, binding }) => { metadataBindings.push({ - binding, + name: binding, type: "kv_namespace", namespace_id: id, }); From 1bf4d095b44aac57dfd593cd4eb7955ed0ad1219 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 23 Dec 2021 13:15:20 +0000 Subject: [PATCH 3/6] Pass a couple of lint warnings, un-export some unused exports --- packages/wrangler/src/api/worker.ts | 1 + packages/wrangler/src/config.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/wrangler/src/api/worker.ts b/packages/wrangler/src/api/worker.ts index 85ba56adee03..ec101e07b28c 100644 --- a/packages/wrangler/src/api/worker.ts +++ b/packages/wrangler/src/api/worker.ts @@ -61,6 +61,7 @@ export interface CfModule { */ type?: CfModuleType; } + /** * A map of variable names to values. */ diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index 3c24417da5b9..1af53f8114a8 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -35,7 +35,7 @@ type KVNamespace = { id: string; }; -export type DurableObject = { +type DurableObject = { name: string; class_name: string; script_name?: string; From b3af0a960a2f816700b18a3c93c8deb09273be4f Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 23 Dec 2021 14:18:25 +0000 Subject: [PATCH 4/6] Experimental worker-to-worker bindings support Adds support for experimental worker-to-worker bindings, letting you call a worker from another worker. The Wrangler config field looks like follows: ```toml services = [{ name = "foo", script_name = "foo-service", environment = "production" }] ``` Where: - `name` is the name of the binding in your worker code - `script_name` is the script to reference - `environment` is the environment of the script to reference (e.g. `production`, `staging`, etc) ### What doesn't work? - The API still doesn't support worker-to-worker bindings for previews, so `wrangler dev` will fail for now ### Differences from #156 This PR is based in #156, with a few changes - - the configuration field `env` is renamed to `environment` (we want to move away from short forms like `env`) - the configuration field `environment` is not an optional field (because we want to explicitly bind to environments, or folks could accidentally mess with production data) - the configuration field services is called `experimental_services` so it's clear on usage that this is not ready for production usage yet, and is only for our internal testing. We also log a warning when we see the field. --- packages/wrangler/src/api/form_data.ts | 15 +++++++++++++++ packages/wrangler/src/api/worker.ts | 10 ++++++++++ packages/wrangler/src/config.ts | 11 ++++++++++- packages/wrangler/src/index.tsx | 14 +++++++++++++- packages/wrangler/src/publish.ts | 1 + 5 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index 3adf347c3756..e3d015c42ac5 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -44,6 +44,12 @@ interface WorkerMetadata { class_name: string; script_name?: string; } + | { + type: "service"; + name: string; + script_name: string; + environment: string; + } )[]; } @@ -88,6 +94,15 @@ export function toFormData(worker: CfWorkerInit): FormData { metadataBindings.push({ name: key, type: "plain_text", text: value }); }); + bindings.services?.forEach(({ name, script_name, environment }) => { + metadataBindings.push({ + name, + type: "service", + script_name, + environment, + }); + }); + const metadata: WorkerMetadata = { ...(mainType !== "commonjs" ? { main_module: name } : { body_part: name }), bindings: metadataBindings, diff --git a/packages/wrangler/src/api/worker.ts b/packages/wrangler/src/api/worker.ts index ec101e07b28c..8b42ad4bc6c2 100644 --- a/packages/wrangler/src/api/worker.ts +++ b/packages/wrangler/src/api/worker.ts @@ -86,6 +86,15 @@ interface CfDurableObject { script_name?: string; } +/** + * A Service. + */ +interface CfService { + name: string; + script_name: string; + environment: string; +} + export interface CfDurableObjectMigrations { old_tag?: string; new_tag: string; @@ -119,6 +128,7 @@ export interface CfWorkerInit { kv_namespaces?: CfKvNamespace[]; durable_objects?: { bindings: CfDurableObject[] }; vars?: CfVars; + services?: CfService[]; }; migrations: void | CfDurableObjectMigrations; compatibility_date: string | void; diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index 1af53f8114a8..424e2e493b15 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -41,6 +41,12 @@ type DurableObject = { script_name?: string; }; +type Service = { + name: string; + script_name: string; + environment: string; +}; + type Build = { command?: string; cwd?: string; @@ -86,6 +92,8 @@ type Env = { vars?: Vars; durable_objects?: { bindings: DurableObject[] }; kv_namespaces?: KVNamespace[]; + experimental_services?: Service[]; + migrations?: DurableObjectMigration[]; usage_model?: UsageModel; // inherited }; @@ -108,9 +116,10 @@ export type Config = { jsx_factory?: string; // inherited jsx_fragment?: string; // inherited vars?: Vars; - migrations?: DurableObjectMigration[]; durable_objects?: { bindings: DurableObject[] }; kv_namespaces?: KVNamespace[]; + experimental_services?: Service[]; + migrations?: DurableObjectMigration[]; site?: Site; // inherited // we should use typescript to parse cron patterns triggers?: { crons: Cron[] }; // inherited diff --git a/packages/wrangler/src/index.tsx b/packages/wrangler/src/index.tsx index cc707993d018..b3e31f390b4b 100644 --- a/packages/wrangler/src/index.tsx +++ b/packages/wrangler/src/index.tsx @@ -79,7 +79,12 @@ async function readConfig(path?: string): Promise { }); }); - const mirroredFields = ["vars", "kv_namespaces", "durable_objects"]; + const mirroredFields = [ + "vars", + "kv_namespaces", + "durable_objects", + "experimental_services", + ]; Object.keys(config.env || {}).forEach((env) => { mirroredFields.forEach((field) => { // if it exists on top level, it should exist on env defns @@ -93,6 +98,12 @@ async function readConfig(path?: string): Promise { }); }); + if ("experimental_services" in config) { + console.warn( + "The experimental_services field is only for cloudflare internal usage right now, and is subject to change. Please do not use this on production projects" + ); + } + // todo: validate, add defaults // let's just do some basics for now @@ -528,6 +539,7 @@ export async function main(argv: string[]): Promise { ), vars: envRootObj.vars, durable_objects: envRootObj.durable_objects, + services: envRootObj.experimental_services, }} /> ); diff --git a/packages/wrangler/src/publish.ts b/packages/wrangler/src/publish.ts index 81e2daecd24c..b16f1d2f5163 100644 --- a/packages/wrangler/src/publish.ts +++ b/packages/wrangler/src/publish.ts @@ -209,6 +209,7 @@ export default async function publish(props: Props): Promise { ), vars: envRootObj.vars, durable_objects: envRootObj.durable_objects, + services: envRootObj.experimental_services, }; const worker: CfWorkerInit = { From dd9c4e441f515dc22d366269291b47cdd3e81017 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 23 Dec 2021 14:22:05 +0000 Subject: [PATCH 5/6] Add a changeset --- .changeset/shiny-crews-warn.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shiny-crews-warn.md diff --git a/.changeset/shiny-crews-warn.md b/.changeset/shiny-crews-warn.md new file mode 100644 index 000000000000..f4572e6c6eb5 --- /dev/null +++ b/.changeset/shiny-crews-warn.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Add experimental support for worker-to-worker service bindings. This introduces a new field in configuration `experimental_services`, and serialises it when creating and uploading a worker definition. This is highly experimental, and doesn't work with `wrangler dev` yet. From c6408f9345517ab4bf1a22dce47d1d65a4b225a1 Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Thu, 23 Dec 2021 20:22:10 +0000 Subject: [PATCH 6/6] s/script_name/service --- packages/wrangler/src/api/form_data.ts | 6 +++--- packages/wrangler/src/api/worker.ts | 2 +- packages/wrangler/src/config.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/wrangler/src/api/form_data.ts b/packages/wrangler/src/api/form_data.ts index e3d015c42ac5..df7b921d93b4 100644 --- a/packages/wrangler/src/api/form_data.ts +++ b/packages/wrangler/src/api/form_data.ts @@ -47,7 +47,7 @@ interface WorkerMetadata { | { type: "service"; name: string; - script_name: string; + service: string; environment: string; } )[]; @@ -94,11 +94,11 @@ export function toFormData(worker: CfWorkerInit): FormData { metadataBindings.push({ name: key, type: "plain_text", text: value }); }); - bindings.services?.forEach(({ name, script_name, environment }) => { + bindings.services?.forEach(({ name, service, environment }) => { metadataBindings.push({ name, type: "service", - script_name, + service, environment, }); }); diff --git a/packages/wrangler/src/api/worker.ts b/packages/wrangler/src/api/worker.ts index 8b42ad4bc6c2..382cad8487dd 100644 --- a/packages/wrangler/src/api/worker.ts +++ b/packages/wrangler/src/api/worker.ts @@ -91,7 +91,7 @@ interface CfDurableObject { */ interface CfService { name: string; - script_name: string; + service: string; environment: string; } diff --git a/packages/wrangler/src/config.ts b/packages/wrangler/src/config.ts index 424e2e493b15..6992fba7c74a 100644 --- a/packages/wrangler/src/config.ts +++ b/packages/wrangler/src/config.ts @@ -43,7 +43,7 @@ type DurableObject = { type Service = { name: string; - script_name: string; + service: string; environment: string; };