Skip to content

Commit

Permalink
fix: fix hyperdrive bindings not getting proxied by miniflare
Browse files Browse the repository at this point in the history
  • Loading branch information
dario-piotrowicz committed Sep 7, 2024
1 parent 5936282 commit 3cd66fe
Show file tree
Hide file tree
Showing 15 changed files with 118 additions and 29 deletions.
30 changes: 29 additions & 1 deletion fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { readdir } from "fs/promises";
import * as nodeNet from "node:net";
import path from "path";
import {
D1Database,
Expand All @@ -18,7 +19,12 @@ import {
import { unstable_dev } from "wrangler";
import { getPlatformProxy } from "./shared";
import type { NamedEntrypoint } from "../workers/rpc-worker";
import type { KVNamespace, Rpc, Service } from "@cloudflare/workers-types";
import type {
Hyperdrive,
KVNamespace,
Rpc,
Service,
} from "@cloudflare/workers-types";
import type { UnstableDevWorker } from "wrangler";

type Env = {
Expand All @@ -35,6 +41,7 @@ type Env = {
MY_DO_B: DurableObjectNamespace;
MY_BUCKET: R2Bucket;
MY_D1: D1Database;
MY_HYPERDRIVE: Hyperdrive;
};

const wranglerTomlFilePath = path.join(__dirname, "..", "wrangler.toml");
Expand Down Expand Up @@ -303,6 +310,27 @@ describe("getPlatformProxy - env", () => {
}
});

// Important: the hyperdrive values are passthrough ones since the workerd specific hyperdrive values only make sense inside
// workerd itself and would simply not work in a node.js process
it("correctly obtains passthrough Hyperdrive bindings", async () => {
const { env, dispose } = await getPlatformProxy<Env>({
configPath: wranglerTomlFilePath,
});
try {
const { MY_HYPERDRIVE } = env;
expect(MY_HYPERDRIVE.connectionString).toEqual(
"postgres://user:pass@127.0.0.1:1234/db"
);
expect(MY_HYPERDRIVE.database).toEqual("db");
expect(MY_HYPERDRIVE.host).toEqual("127.0.0.1");
expect(MY_HYPERDRIVE.user).toEqual("user");
expect(MY_HYPERDRIVE.password).toEqual("pass");
expect(MY_HYPERDRIVE.port).toEqual(1234);
} finally {
await dispose();
}
});

describe("with a target environment", () => {
it("should provide bindings targeting a specified environment and also inherit top-level ones", async () => {
const { env, dispose } = await getPlatformProxy<Env>({
Expand Down
5 changes: 5 additions & 0 deletions fixtures/get-platform-proxy/wrangler.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ bindings = [
{ name = "MY_DO_B", script_name = "do-worker-b", class_name = "DurableObjectClass" }
]

[[hyperdrive]]
binding = "MY_HYPERDRIVE"
id = "000000000000000000000000000000000"
localConnectionString = "postgres://user:pass@127.0.0.1:1234/db"

[[d1_databases]]
binding = "MY_D1"
database_name = "test-db"
Expand Down
9 changes: 6 additions & 3 deletions packages/miniflare/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ import {
getDirectSocketName,
getGlobalServices,
HOST_CAPNP_CONNECT,
kProxyNodeBinding,
KV_PLUGIN_NAME,
normaliseDurableObject,
PLUGIN_ENTRIES,
Plugins,
PluginServicesOptions,
ProxyClient,
ProxyNodeBinding,
QueueConsumers,
QueueProducers,
QUEUES_PLUGIN_NAME,
Expand Down Expand Up @@ -1668,13 +1668,16 @@ export class Miniflare {
// missing in other plugins' options.
const pluginBindings = await plugin.getNodeBindings(workerOpts[key]);
for (const [name, binding] of Object.entries(pluginBindings)) {
if (binding === kProxyNodeBinding) {
if (binding instanceof ProxyNodeBinding) {
const proxyBindingName = getProxyBindingName(key, workerName, name);
const proxy = proxyClient.env[proxyBindingName];
let proxy = proxyClient.env[proxyBindingName];
assert(
proxy !== undefined,
`Expected ${proxyBindingName} to be bound`
);
if (binding.proxyOverrideHandler) {
proxy = new Proxy(proxy, binding.proxyOverrideHandler);
}
bindings[name] = proxy;
} else {
bindings[name] = binding;
Expand Down
4 changes: 2 additions & 2 deletions packages/miniflare/src/plugins/assets/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { z } from "zod";
import { Service } from "../../runtime";
import { SharedBindings } from "../../workers";
import { getUserServiceName } from "../core";
import { kProxyNodeBinding, Plugin } from "../shared";
import { Plugin, ProxyNodeBinding } from "../shared";
import {
ASSETS_KV_SERVICE_NAME,
ASSETS_PLUGIN_NAME,
Expand Down Expand Up @@ -39,7 +39,7 @@ export const ASSETS_PLUGIN: Plugin<typeof AssetsOptionsSchema> = {
return {};
}
return {
[options.assets.bindingName]: kProxyNodeBinding,
[options.assets.bindingName]: new ProxyNodeBinding(),
};
},

Expand Down
6 changes: 3 additions & 3 deletions packages/miniflare/src/plugins/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ import {
import { getCacheServiceName } from "../cache";
import { DURABLE_OBJECTS_STORAGE_SERVICE_NAME } from "../do";
import {
kProxyNodeBinding,
kUnsafeEphemeralUniqueKey,
parseRoutes,
Plugin,
ProxyNodeBinding,
SERVICE_LOOPBACK,
WORKER_BINDING_SERVICE_LOOPBACK,
} from "../shared";
Expand Down Expand Up @@ -473,15 +473,15 @@ export const CORE_PLUGIN: Plugin<
bindingEntries.push(
...Object.keys(options.serviceBindings).map((name) => [
name,
kProxyNodeBinding,
new ProxyNodeBinding(),
])
);
}
if (options.wrappedBindings !== undefined) {
bindingEntries.push(
...Object.keys(options.wrappedBindings).map((name) => [
name,
kProxyNodeBinding,
new ProxyNodeBinding(),
])
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/miniflare/src/plugins/d1/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import { SharedBindings } from "../../workers";
import {
getMiniflareObjectBindings,
getPersistPath,
kProxyNodeBinding,
migrateDatabase,
namespaceEntries,
namespaceKeys,
objectEntryWorker,
PersistenceSchema,
Plugin,
ProxyNodeBinding,
SERVICE_LOOPBACK,
} from "../shared";

Expand Down Expand Up @@ -69,7 +69,7 @@ export const D1_PLUGIN: Plugin<
getNodeBindings(options) {
const databases = namespaceKeys(options.d1Databases);
return Object.fromEntries(
databases.map((name) => [name, kProxyNodeBinding])
databases.map((name) => [name, new ProxyNodeBinding()])
);
},
async getServices({
Expand Down
6 changes: 4 additions & 2 deletions packages/miniflare/src/plugins/do/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { Worker_Binding } from "../../runtime";
import { getUserServiceName } from "../core";
import {
getPersistPath,
kProxyNodeBinding,
kUnsafeEphemeralUniqueKey,
PersistenceSchema,
Plugin,
ProxyNodeBinding,
UnsafeUniqueKey,
} from "../shared";

Expand Down Expand Up @@ -83,7 +83,9 @@ export const DURABLE_OBJECTS_PLUGIN: Plugin<
},
getNodeBindings(options) {
const objects = Object.keys(options.durableObjects ?? {});
return Object.fromEntries(objects.map((name) => [name, kProxyNodeBinding]));
return Object.fromEntries(
objects.map((name) => [name, new ProxyNodeBinding()])
);
},
async getServices({
sharedOptions,
Expand Down
22 changes: 19 additions & 3 deletions packages/miniflare/src/plugins/hyperdrive/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import assert from "node:assert";
import { z } from "zod";
import { Service, Worker_Binding } from "../../runtime";
import { Plugin } from "../shared";
import { Plugin, ProxyNodeBinding } from "../shared";

export const HYPERDRIVE_PLUGIN_NAME = "hyperdrive";

Expand Down Expand Up @@ -90,8 +90,24 @@ export const HYPERDRIVE_PLUGIN: Plugin<typeof HyperdriveInputOptionsSchema> = {
}
);
},
getNodeBindings() {
return {};
getNodeBindings(options) {
return Object.fromEntries(
Object.entries(options.hyperdrives ?? {}).map(([name, url]) => {
const connectionOverrides: Record<string | symbol, string | number> = {
connectionString: `${url}`,
port: Number.parseInt(url.port),
host: url.hostname,
};
const proxyNodeBinding = new ProxyNodeBinding({
get(target, prop) {
return prop in connectionOverrides
? connectionOverrides[prop]
: target[prop];
},
});
return [name, proxyNodeBinding];
})
);
},
async getServices({ options }) {
return Object.entries(options.hyperdrives ?? {}).map<Service>(
Expand Down
4 changes: 2 additions & 2 deletions packages/miniflare/src/plugins/kv/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import { SharedBindings } from "../../workers";
import {
getMiniflareObjectBindings,
getPersistPath,
kProxyNodeBinding,
migrateDatabase,
namespaceEntries,
namespaceKeys,
objectEntryWorker,
PersistenceSchema,
Plugin,
ProxyNodeBinding,
SERVICE_LOOPBACK,
} from "../shared";
import { KV_PLUGIN_NAME } from "./constants";
Expand Down Expand Up @@ -77,7 +77,7 @@ export const KV_PLUGIN: Plugin<
async getNodeBindings(options) {
const namespaces = namespaceKeys(options.kvNamespaces);
const bindings = Object.fromEntries(
namespaces.map((name) => [name, kProxyNodeBinding])
namespaces.map((name) => [name, new ProxyNodeBinding()])
);

if (isWorkersSitesEnabled(options)) {
Expand Down
4 changes: 2 additions & 2 deletions packages/miniflare/src/plugins/kv/sites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
SiteMatcherRegExps,
testSiteRegExps,
} from "../../workers";
import { kProxyNodeBinding } from "../shared";
import { ProxyNodeBinding } from "../shared";
import { KV_PLUGIN_NAME } from "./constants";

async function* listKeysInDirectoryInner(
Expand Down Expand Up @@ -97,7 +97,7 @@ export async function getSitesNodeBindings(
siteRegExps
);
return {
[SiteBindings.KV_NAMESPACE_SITE]: kProxyNodeBinding,
[SiteBindings.KV_NAMESPACE_SITE]: new ProxyNodeBinding(),
[SiteBindings.JSON_SITE_MANIFEST]: __STATIC_CONTENT_MANIFEST,
};
}
Expand Down
6 changes: 4 additions & 2 deletions packages/miniflare/src/plugins/queues/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import {
import { getUserServiceName } from "../core";
import {
getMiniflareObjectBindings,
kProxyNodeBinding,
objectEntryWorker,
Plugin,
ProxyNodeBinding,
SERVICE_LOOPBACK,
} from "../shared";

Expand Down Expand Up @@ -53,7 +53,9 @@ export const QUEUES_PLUGIN: Plugin<typeof QueuesOptionsSchema> = {
},
getNodeBindings(options) {
const queues = bindingKeys(options.queueProducers);
return Object.fromEntries(queues.map((name) => [name, kProxyNodeBinding]));
return Object.fromEntries(
queues.map((name) => [name, new ProxyNodeBinding()])
);
},
async getServices({
options,
Expand Down
6 changes: 4 additions & 2 deletions packages/miniflare/src/plugins/r2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import { SharedBindings } from "../../workers";
import {
getMiniflareObjectBindings,
getPersistPath,
kProxyNodeBinding,
migrateDatabase,
namespaceEntries,
namespaceKeys,
objectEntryWorker,
PersistenceSchema,
Plugin,
ProxyNodeBinding,
SERVICE_LOOPBACK,
} from "../shared";

Expand Down Expand Up @@ -51,7 +51,9 @@ export const R2_PLUGIN: Plugin<
},
getNodeBindings(options) {
const buckets = namespaceKeys(options.r2Buckets);
return Object.fromEntries(buckets.map((name) => [name, kProxyNodeBinding]));
return Object.fromEntries(
buckets.map((name) => [name, new ProxyNodeBinding()])
);
},
async getServices({
options,
Expand Down
7 changes: 5 additions & 2 deletions packages/miniflare/src/plugins/ratelimit/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import SCRIPT_RATELIMIT_OBJECT from "worker:ratelimit/ratelimit";
import { z } from "zod";
import { Worker_Binding } from "../../runtime";
import { kProxyNodeBinding, Plugin } from "../shared";
import { Plugin, ProxyNodeBinding } from "../shared";

export enum PeriodType {
TENSECONDS = 10,
Expand Down Expand Up @@ -57,7 +57,10 @@ export const RATELIMIT_PLUGIN: Plugin<typeof RatelimitOptionsSchema> = {
return {};
}
return Object.fromEntries(
Object.keys(options.ratelimits).map((name) => [name, kProxyNodeBinding])
Object.keys(options.ratelimits).map((name) => [
name,
new ProxyNodeBinding(),
])
);
},
async getServices({ options }) {
Expand Down
9 changes: 6 additions & 3 deletions packages/miniflare/src/plugins/shared/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,12 @@ export type Plugin<
? { sharedOptions?: undefined }
: { sharedOptions: SharedOptions });

// When this is returned as the binding from `PluginBase#getNodeBindings()`,
// Miniflare will replace it with a proxy to the binding in `workerd`
export const kProxyNodeBinding = Symbol("kProxyNodeBinding");
// When an instance of this class is returned as the binding from `PluginBase#getNodeBindings()`,
// Miniflare will replace it with a proxy to the binding in `workerd`, alongside applying the
// specified overrides (if there is any)
export class ProxyNodeBinding {
constructor(public proxyOverrideHandler?: ProxyHandler<any>) {}
}

export function namespaceKeys(
namespaces?: Record<string, string> | string[]
Expand Down
25 changes: 25 additions & 0 deletions packages/miniflare/test/plugins/hyperdrive/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Hyperdrive } from "@cloudflare/workers-types/experimental";
import test from "ava";
import { Miniflare, MiniflareOptions } from "miniflare";

Expand Down Expand Up @@ -34,6 +35,30 @@ test("fields match expected", async (t) => {
t.is(hyperdrive.port, 5432);
});

test("fields in binding proxy match expected", async (t) => {
const connectionString = `postgresql://user:password@localhost:5432/database`;
const mf = new Miniflare({
modules: true,
script: `export default { fetch() {} }`,
hyperdrives: {
HYPERDRIVE: connectionString,
},
});
t.teardown(() => mf.dispose());
const { HYPERDRIVE } = await mf.getBindings<{ HYPERDRIVE: Hyperdrive }>();
t.is(HYPERDRIVE.user, "user");
t.is(HYPERDRIVE.password, "password");
t.is(HYPERDRIVE.database, "database");
t.is(HYPERDRIVE.port, 5432);

// Important: the checks below differ from what the worker code would get inside workerd, this is necessary since getting the binding via `getBindings` implies that
// the binding is going to be used inside node.js and not within workerd where the hyperdrive connection is actually set, so the values need need to remain
// the exact same making the hyperdrive binding work as a simple no-op/passthrough (returning the workerd hyperdrive values wouldn't work as those would not
// work/have any meaning in a node.js process)
t.is(HYPERDRIVE.connectionString, connectionString);
t.is(HYPERDRIVE.host, "localhost");
});

test("validates config", async (t) => {
const opts: MiniflareOptions = { modules: true, script: "" };
const mf = new Miniflare(opts);
Expand Down

0 comments on commit 3cd66fe

Please sign in to comment.