From 1d7c8401f03d7e21090b2f51e8f22891e7bf329a Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 30 Jul 2025 12:38:54 +0100 Subject: [PATCH] fix specified environments being ignored for remote bindings in some cases --- .changeset/cruel-ears-heal.md | 26 ++++++++ .changeset/giant-laws-play.md | 5 ++ .changeset/stale-dogs-throw.md | 5 ++ .../remote-worker.staging.js | 7 +++ .../tests/index.test.ts | 61 +++++++++++++++++++ .../env.d.ts | 7 +++ .../remote-worker.staging.js | 7 +++ .../run-tests.mjs | 51 ++++++++++++++-- .../test-staging/index.spec.ts | 29 +++++++++ .../test/index.spec.ts | 15 ++--- .../vitest.workers.config.staging.ts | 49 +++++++++++++++ .../vitest.workers.config.ts | 2 +- .../wrangler.json | 13 +++- .../vitest-pool-workers/src/pool/config.ts | 5 +- .../src/api/integrations/platform/index.ts | 5 +- .../wrangler/src/api/remoteBindings/index.ts | 47 ++++++++------ 16 files changed, 299 insertions(+), 35 deletions(-) create mode 100644 .changeset/cruel-ears-heal.md create mode 100644 .changeset/giant-laws-play.md create mode 100644 .changeset/stale-dogs-throw.md create mode 100644 fixtures/get-platform-proxy-remote-bindings/remote-worker.staging.js create mode 100644 fixtures/vitest-pool-workers-remote-bindings/env.d.ts create mode 100644 fixtures/vitest-pool-workers-remote-bindings/remote-worker.staging.js create mode 100644 fixtures/vitest-pool-workers-remote-bindings/test-staging/index.spec.ts create mode 100644 fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.staging.ts diff --git a/.changeset/cruel-ears-heal.md b/.changeset/cruel-ears-heal.md new file mode 100644 index 000000000000..88288cec0713 --- /dev/null +++ b/.changeset/cruel-ears-heal.md @@ -0,0 +1,26 @@ +--- +"wrangler": patch +--- + +update `maybeStartOrUpdateRemoteProxySession` config argument (to allow callers to specify an environment) + +Before this change `maybeStartOrUpdateRemoteProxySession` could be called with either the path to a wrangler config file or the configuration of a worker. The former override however did not allow the caller to specify an environment, so the `maybeStartOrUpdateRemoteProxySession` API has been updated so that in the wrangler config case an object (with the path and a potential environment) needs to be passed instead. + +For example, before callers could invoke the function in the following way + +```ts +await maybeStartOrUpdateRemoteProxySession(configPath); +``` + +note that there is no way to tell the function what environment to use when parsing the wrangle configuration. + +Now callers will instead call the function in the following way: + +```ts +await maybeStartOrUpdateRemoteProxySession({ + path: configPath, + environment: targetEnvironment, +}); +``` + +note that now a target environment can be specified. diff --git a/.changeset/giant-laws-play.md b/.changeset/giant-laws-play.md new file mode 100644 index 000000000000..b4494abfa143 --- /dev/null +++ b/.changeset/giant-laws-play.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +fix `getPlatformProxy` not taking into account the potentially specified environment for remote bindings diff --git a/.changeset/stale-dogs-throw.md b/.changeset/stale-dogs-throw.md new file mode 100644 index 000000000000..1ac6dc7b736f --- /dev/null +++ b/.changeset/stale-dogs-throw.md @@ -0,0 +1,5 @@ +--- +"@cloudflare/vitest-pool-workers": patch +--- + +fix the potentially specified environment not being taken into account for remote bindings diff --git a/fixtures/get-platform-proxy-remote-bindings/remote-worker.staging.js b/fixtures/get-platform-proxy-remote-bindings/remote-worker.staging.js new file mode 100644 index 000000000000..94457a6694a0 --- /dev/null +++ b/fixtures/get-platform-proxy-remote-bindings/remote-worker.staging.js @@ -0,0 +1,7 @@ +export default { + fetch() { + return new Response( + "Hello from a remote Worker, defined for the staging environment, part of the getPlatformProxy remote bindings fixture!" + ); + }, +}; diff --git a/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts b/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts index 20c9f2f28b58..385ac2d727c5 100644 --- a/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts +++ b/fixtures/get-platform-proxy-remote-bindings/tests/index.test.ts @@ -15,6 +15,7 @@ const execOptions = { env: { ...process.env, ...auth }, } as const; const remoteWorkerName = `tmp-e2e-worker-test-remote-bindings-${randomUUID().split("-")[0]}`; +const remoteStagingWorkerName = `tmp-e2e-staging-worker-test-remote-bindings-${randomUUID().split("-")[0]}`; const remoteKvName = `tmp-e2e-remote-kv-test-remote-bindings-${randomUUID().split("-")[0]}`; if (auth) { @@ -32,6 +33,19 @@ if (auth) { throw new Error(`Failed to deploy ${remoteWorkerName}`); } + const stagingDeployOut = execSync( + `pnpm wrangler deploy remote-worker.staging.js --name ${remoteStagingWorkerName} --compatibility-date 2025-06-19`, + execOptions + ); + + if ( + !new RegExp(`Deployed\\s+${remoteStagingWorkerName}\\b`).test( + stagingDeployOut + ) + ) { + throw new Error(`Failed to deploy ${remoteStagingWorkerName}`); + } + const kvAddOut = execSync( `pnpm wrangler kv namespace create ${remoteKvName}`, execOptions @@ -71,6 +85,24 @@ if (auth) { experimental_remote: true, }, ], + env: { + staging: { + services: [ + { + binding: "MY_WORKER", + service: remoteStagingWorkerName, + experimental_remote: true, + }, + ], + kv_namespaces: [ + { + binding: "MY_KV", + id: remoteKvId, + experimental_remote: true, + }, + ], + }, + }, }, undefined, 2 @@ -84,6 +116,10 @@ if (auth) { `pnpm wrangler delete --name ${remoteWorkerName}`, execOptions ); + execSync( + `pnpm wrangler delete --name ${remoteStagingWorkerName}`, + execOptions + ); execSync( `pnpm wrangler kv namespace delete --namespace-id=${remoteKvId}`, execOptions @@ -115,6 +151,31 @@ if (auth) { await dispose(); }); + test("getPlatformProxy works with remote bindings specified in an environment", async () => { + vi.stubEnv("CLOUDFLARE_ACCOUNT_ID", auth.CLOUDFLARE_ACCOUNT_ID); + vi.stubEnv("CLOUDFLARE_API_TOKEN", auth.CLOUDFLARE_API_TOKEN); + const { env, dispose } = await getPlatformProxy<{ + MY_WORKER: Fetcher; + MY_KV: KVNamespace; + }>({ + configPath: "./.tmp/wrangler.json", + experimental: { remoteBindings: true }, + environment: "staging", + }); + + const workerText = await ( + await env.MY_WORKER.fetch("http://example.com") + ).text(); + expect(workerText).toEqual( + "Hello from a remote Worker, defined for the staging environment, part of the getPlatformProxy remote bindings fixture!" + ); + + const kvValue = await env.MY_KV.get("test-key"); + expect(kvValue).toEqual("remote-kv-value"); + + await dispose(); + }); + test("getPlatformProxy does not work with remote bindings if the experimental remoteBindings flag is not turned on", async () => { const { env, dispose } = await getPlatformProxy<{ MY_WORKER: Fetcher; diff --git a/fixtures/vitest-pool-workers-remote-bindings/env.d.ts b/fixtures/vitest-pool-workers-remote-bindings/env.d.ts new file mode 100644 index 000000000000..5ca5d483e6ef --- /dev/null +++ b/fixtures/vitest-pool-workers-remote-bindings/env.d.ts @@ -0,0 +1,7 @@ +import { Fetcher } from "@cloudflare/workers-types/experimental"; + +declare module "cloudflare:test" { + interface ProvidedEnv { + MY_WORKER: Fetcher; + } +} diff --git a/fixtures/vitest-pool-workers-remote-bindings/remote-worker.staging.js b/fixtures/vitest-pool-workers-remote-bindings/remote-worker.staging.js new file mode 100644 index 000000000000..2857097be871 --- /dev/null +++ b/fixtures/vitest-pool-workers-remote-bindings/remote-worker.staging.js @@ -0,0 +1,7 @@ +export default { + fetch() { + return new Response( + "Hello from a remote Worker, defined for the staging environment, part of the vitest-pool-workers remote bindings fixture!" + ); + }, +}; diff --git a/fixtures/vitest-pool-workers-remote-bindings/run-tests.mjs b/fixtures/vitest-pool-workers-remote-bindings/run-tests.mjs index 8dc1fc491cb0..3b6356620f68 100644 --- a/fixtures/vitest-pool-workers-remote-bindings/run-tests.mjs +++ b/fixtures/vitest-pool-workers-remote-bindings/run-tests.mjs @@ -1,11 +1,9 @@ /** * This fixture is particular since it needs to communicate with remote resources, namely - * a remote worker. + * two remote workers. * - * This script is used to deploy a remote worker and run the fixture using said worker. - * - * Alternatively you can simply deploy, using your account, the `./remote-worker.js` file as - * a worker named `my-worker-test` and directly run the fixture using vitest. + * This script is used to deploy the remote workers and run the fixture using said workers + * with the appropriate vitest configurations. */ import { execSync } from "child_process"; import { randomUUID } from "crypto"; @@ -22,11 +20,17 @@ rmSync("./.tmp", { recursive: true, force: true }); cpSync("./src", "./.tmp/src", { recursive: true }); cpSync("./test", "./.tmp/test", { recursive: true }); cpSync("./vitest.workers.config.ts", "./.tmp/vitest.workers.config.ts"); +cpSync( + "./vitest.workers.config.staging.ts", + "./.tmp/vitest.workers.config.staging.ts" +); const remoteWorkerName = `tmp-e2e-worker-test-remote-bindings-${randomUUID().split("-")[0]}`; +const remoteStagingWorkerName = `tmp-e2e-staging-worker-test-remote-bindings-${randomUUID().split("-")[0]}`; const wranglerJson = JSON.parse(readFileSync("./wrangler.json", "utf8")); wranglerJson.services[0].service = remoteWorkerName; +wranglerJson.env.staging.services[0].service = remoteStagingWorkerName; writeFileSync( "./.tmp/wrangler.json", @@ -60,12 +64,49 @@ if (!new RegExp(`Deployed\\s+${remoteWorkerName}\\b`).test(`${deployOut}`)) { throw new Error(`Failed to deploy ${remoteWorkerName}`); } +writeFileSync( + "./.tmp/remote-wrangler.staging.json", + JSON.stringify( + { + name: remoteStagingWorkerName, + main: "../remote-worker.staging.js", + compatibility_date: "2025-06-01", + }, + undefined, + 2 + ), + "utf8" +); + +const deployStagingOut = execSync( + "pnpm wrangler deploy -c .tmp/remote-wrangler.staging.json", + { + stdio: "pipe", + cwd: "./.tmp", + env, + } +); +if ( + !new RegExp(`Deployed\\s+${remoteStagingWorkerName}\\b`).test( + `${deployStagingOut}` + ) +) { + throw new Error(`Failed to deploy ${remoteStagingWorkerName}`); +} + try { execSync("pnpm test:vitest --config ./.tmp/vitest.workers.config.ts", { env, }); + execSync( + "pnpm test:vitest --config ./.tmp/vitest.workers.config.staging.ts", + { + env, + } + ); } finally { execSync(`pnpm wrangler delete --name ${remoteWorkerName}`, { env }); + execSync(`pnpm wrangler delete --name ${remoteStagingWorkerName}`, { env }); rmSync("./.tmp", { recursive: true, force: true }); } diff --git a/fixtures/vitest-pool-workers-remote-bindings/test-staging/index.spec.ts b/fixtures/vitest-pool-workers-remote-bindings/test-staging/index.spec.ts new file mode 100644 index 000000000000..5297f2d8bbde --- /dev/null +++ b/fixtures/vitest-pool-workers-remote-bindings/test-staging/index.spec.ts @@ -0,0 +1,29 @@ +import { + createExecutionContext, + env, + SELF, + waitOnExecutionContext, +} from "cloudflare:test"; +import { describe, expect, test } from "vitest"; + +describe("Vitest pool workers remote bindings with a staging environment", () => { + test( + "fetching unit-style from a remote service binding", + { timeout: 50_000 }, + async () => { + const response = await env.MY_WORKER.fetch("http://example.com"); + const ctx = createExecutionContext(); + await waitOnExecutionContext(ctx); + expect(await response.text()).toMatchInlineSnapshot( + `"Hello from a remote Worker, defined for the staging environment, part of the vitest-pool-workers remote bindings fixture!"` + ); + } + ); + + test("fetching integration-style from the local worker (which uses remote bindings)", async () => { + const response = await SELF.fetch("https://example.com"); + expect(await response.text()).toMatchInlineSnapshot( + `"Response from remote worker: Hello from a remote Worker, defined for the staging environment, part of the vitest-pool-workers remote bindings fixture!"` + ); + }); +}); diff --git a/fixtures/vitest-pool-workers-remote-bindings/test/index.spec.ts b/fixtures/vitest-pool-workers-remote-bindings/test/index.spec.ts index 384df4de30cd..35963d0ad2d5 100644 --- a/fixtures/vitest-pool-workers-remote-bindings/test/index.spec.ts +++ b/fixtures/vitest-pool-workers-remote-bindings/test/index.spec.ts @@ -3,19 +3,16 @@ import { env, SELF, waitOnExecutionContext, - // @ts-ignore } from "cloudflare:test"; -import { describe, expect, it } from "vitest"; +import { describe, expect, test } from "vitest"; -describe("Hello World worker", () => { - it( - "responds with Hello World! (unit style)", +describe("Vitest pool workers remote bindings", () => { + test( + "fetching unit-style from a remote service binding", { timeout: 50_000 }, async () => { - const request = new Request("http://example.com"); + const response = await env.MY_WORKER.fetch("http://example.com"); const ctx = createExecutionContext(); - debugger; - const response = await env.MY_WORKER.fetch(request, env, ctx); await waitOnExecutionContext(ctx); expect(await response.text()).toMatchInlineSnapshot( `"Hello from a remote Worker part of the vitest-pool-workers remote bindings fixture!"` @@ -23,7 +20,7 @@ describe("Hello World worker", () => { } ); - it("responds with Hello World! (integration style)", async () => { + test("fetching integration-style from the local worker (which uses remote bindings)", async () => { const response = await SELF.fetch("https://example.com"); expect(await response.text()).toMatchInlineSnapshot( `"Response from remote worker: Hello from a remote Worker part of the vitest-pool-workers remote bindings fixture!"` diff --git a/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.staging.ts b/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.staging.ts new file mode 100644 index 000000000000..3f2c76ec754a --- /dev/null +++ b/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.staging.ts @@ -0,0 +1,49 @@ +import { defineWorkersConfig } from "@cloudflare/vitest-pool-workers/config"; + +class FilteredPushArray extends Array { + constructor(private readonly predicate: (item: T) => boolean) { + super(); + } + + push(...items: T[]) { + return super.push(...items.filter(this.predicate)); + } +} + +export default defineWorkersConfig({ + test: { + include: ["test-staging/**/*.spec.ts"], + poolOptions: { + workers: { + experimental_remoteBindings: true, + wrangler: { configPath: "./wrangler.json", environment: "staging" }, + }, + }, + + // Configure the `vite-node` server used by Vitest code to import configs, + // custom pools and tests. By default, Vitest effectively applies Vite + // transforms to all files outside `node_modules`. This means by default, + // our custom pool code is transformed by Vite during development, but not + // when published, leading to possible behaviour mismatches. To fix this, + // we ensure file paths containing `packages/vitest-pool-workers/dist` are + // always "externalised", meaning they're imported directly by Node. + server: { + deps: { + // Vitest automatically adds `/^(?!.*node_modules).*\.mjs$/` as an + // `inline` RegExp: https://github.com/vitest-dev/vitest/blob/v2.1.1/packages/vitest/src/constants.ts#L9 + // We'd like `packages/vitest-pool-workers/dist/pool/index.mjs` to be + // externalised though. Unfortunately, `inline`s are checked before + // `external`s, so there's no nice way we can override this. Instead, + // we prevent the extra `inline` being added in the first place. + inline: new FilteredPushArray((item: any) => { + const str = item.toString(); + return str !== "/^(?!.*node_modules).*\\.mjs$/"; + }), + external: [ + /packages\/vitest-pool-workers\/dist/, + /packages\/wrangler\//, + ], + }, + }, + }, +}); diff --git a/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.ts b/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.ts index 6a476d79b5e1..efd74027518f 100644 --- a/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.ts +++ b/fixtures/vitest-pool-workers-remote-bindings/vitest.workers.config.ts @@ -10,9 +10,9 @@ class FilteredPushArray extends Array { } } -debugger; export default defineWorkersConfig({ test: { + include: ["test/**/*.spec.ts"], poolOptions: { workers: { experimental_remoteBindings: true, diff --git a/fixtures/vitest-pool-workers-remote-bindings/wrangler.json b/fixtures/vitest-pool-workers-remote-bindings/wrangler.json index 75d57aee81ab..f74d6f974e4f 100644 --- a/fixtures/vitest-pool-workers-remote-bindings/wrangler.json +++ b/fixtures/vitest-pool-workers-remote-bindings/wrangler.json @@ -8,5 +8,16 @@ "service": "my-worker-test", "experimental_remote": true } - ] + ], + "env": { + "staging": { + "services": [ + { + "binding": "MY_WORKER", + "service": "my-staging-worker-test", + "experimental_remote": true + } + ] + } + } } diff --git a/packages/vitest-pool-workers/src/pool/config.ts b/packages/vitest-pool-workers/src/pool/config.ts index ad0fed9dde55..798f6f7d043f 100644 --- a/packages/vitest-pool-workers/src/pool/config.ts +++ b/packages/vitest-pool-workers/src/pool/config.ts @@ -259,7 +259,10 @@ async function parseCustomPoolOptions( const remoteProxySessionData = options.experimental_remoteBindings ? await wrangler.experimental_maybeStartOrUpdateRemoteProxySession( - configPath, + { + path: options.wrangler.configPath, + environment: options.wrangler.environment, + }, preExistingRemoteProxySessionData ?? null ) : null; diff --git a/packages/wrangler/src/api/integrations/platform/index.ts b/packages/wrangler/src/api/integrations/platform/index.ts index 7df3a2b2a074..e22dba0c15c8 100644 --- a/packages/wrangler/src/api/integrations/platform/index.ts +++ b/packages/wrangler/src/api/integrations/platform/index.ts @@ -144,7 +144,10 @@ export async function getPlatformProxy< let remoteProxySession: RemoteProxySession | undefined = undefined; if (experimentalRemoteBindings && config.configPath) { remoteProxySession = ( - (await maybeStartOrUpdateRemoteProxySession(config.configPath)) ?? {} + (await maybeStartOrUpdateRemoteProxySession({ + path: config.configPath, + environment: env, + })) ?? {} ).session; } diff --git a/packages/wrangler/src/api/remoteBindings/index.ts b/packages/wrangler/src/api/remoteBindings/index.ts index a8759508a17a..8a58b015cc19 100644 --- a/packages/wrangler/src/api/remoteBindings/index.ts +++ b/packages/wrangler/src/api/remoteBindings/index.ts @@ -99,13 +99,29 @@ export function pickRemoteBindings( ); } +type WranglerConfigObject = { + /** The path to the wrangler config file */ + path: string; + /** The target environment */ + environment?: string; +}; + +type WorkerConfigObject = { + /** The name of the worker */ + name?: string; + /** The Worker's bindings */ + bindings: NonNullable; + /** If running in a non-public compliance region, set this here. */ + complianceRegion?: Config["compliance_region"]; +}; + /** * Utility for potentially starting or updating a remote proxy session. * * It uses an internal map for storing existing remote proxy session indexed by worker names. If no worker name is provided * the remote proxy session won't be retrieved nor saved to/from the internal map. * - * @param configPathOrWorkerConfig either a file path to a wrangler configuration file or an object containing the name of + * @param wranglerOrWorkerConfigObject either a file path to a wrangler configuration file or an object containing the name of * the target worker alongside its bindings. * @param preExistingRemoteProxySessionData the optional data of a pre-existing remote proxy session if there was one, this * argument can be omitted or set to null if there is no pre-existing remote proxy session @@ -113,14 +129,7 @@ export function pickRemoteBindings( * defining any remote bindings), the data associated to the created/updated remote proxy session otherwise. */ export async function maybeStartOrUpdateRemoteProxySession( - configPathOrWorkerConfig: - | string - | { - name?: string; - /** If running in a non-public compliance region, set this here. */ - complianceRegion?: Config["compliance_region"]; - bindings: NonNullable; - }, + wranglerOrWorkerConfigObject: WranglerConfigObject | WorkerConfigObject, preExistingRemoteProxySessionData?: { session: RemoteProxySession; remoteBindings: Record; @@ -129,21 +138,25 @@ export async function maybeStartOrUpdateRemoteProxySession( session: RemoteProxySession; remoteBindings: Record; } | null> { - if (typeof configPathOrWorkerConfig === "string") { - const configPath = configPathOrWorkerConfig; - const config = readConfig({ config: configPath }); + if ("path" in wranglerOrWorkerConfigObject) { + const wranglerConfigObject = wranglerOrWorkerConfigObject; + const config = readConfig({ + config: wranglerConfigObject.path, + env: wranglerConfigObject.environment, + }); assert(config.name); - configPathOrWorkerConfig = { + wranglerOrWorkerConfigObject = { name: config.name, complianceRegion: getCloudflareComplianceRegion(config), bindings: convertConfigBindingsToStartWorkerBindings(config) ?? {}, }; } - const workerConfigs = configPathOrWorkerConfig; - const remoteBindings = pickRemoteBindings(workerConfigs.bindings); + const workerConfigObject = wranglerOrWorkerConfigObject; + + const remoteBindings = pickRemoteBindings(workerConfigObject.bindings); let remoteProxySession = preExistingRemoteProxySessionData?.session; @@ -157,8 +170,8 @@ export async function maybeStartOrUpdateRemoteProxySession( if (!remoteProxySession) { if (Object.keys(remoteBindings).length > 0) { remoteProxySession = await startRemoteProxySession(remoteBindings, { - workerName: configPathOrWorkerConfig.name, - complianceRegion: configPathOrWorkerConfig.complianceRegion, + workerName: wranglerOrWorkerConfigObject.name, + complianceRegion: wranglerOrWorkerConfigObject.complianceRegion, }); } } else {