From e4abed3e8f9c46a014a045885da0dea5c4ae8837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Assun=C3=A7=C3=A3o?= Date: Mon, 15 Jul 2024 12:35:21 -0500 Subject: [PATCH] Add support for creating Hyperdrive over Access configs (#6245) * Add support for creating Hyperdrive over Access configs * PR Feedback --- .changeset/soft-eagles-fly.md | 5 + .../wrangler/src/__tests__/hyperdrive.test.ts | 149 +++++++++++++++++- packages/wrangler/src/hyperdrive/client.ts | 13 +- packages/wrangler/src/hyperdrive/create.ts | 133 ++++++++++++++-- packages/wrangler/src/hyperdrive/update.ts | 54 +++++-- 5 files changed, 318 insertions(+), 36 deletions(-) create mode 100644 .changeset/soft-eagles-fly.md diff --git a/.changeset/soft-eagles-fly.md b/.changeset/soft-eagles-fly.md new file mode 100644 index 000000000000..4cf623d1c172 --- /dev/null +++ b/.changeset/soft-eagles-fly.md @@ -0,0 +1,5 @@ +--- +"wrangler": minor +--- + +feature: Add support for Hyperdrive over Access configs diff --git a/packages/wrangler/src/__tests__/hyperdrive.test.ts b/packages/wrangler/src/__tests__/hyperdrive.test.ts index 0465ac5ef34f..e1adc57814aa 100644 --- a/packages/wrangler/src/__tests__/hyperdrive.test.ts +++ b/packages/wrangler/src/__tests__/hyperdrive.test.ts @@ -239,6 +239,76 @@ describe("hyperdrive commands", () => { `); }); + it("should reject a create hyperdrive command if both connection string and individual origin params are provided", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive create test123 --connection-string='postgresql://test:password@example.com/neondb' --host=example.com --port=5432 --database=neondb --user=test" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Arguments host and connection-string are mutually exclusive + + " + `); + }); + + it("should create a hyperdrive over access config given the right params", async () => { + mockHyperdriveRequest(); + await runWrangler( + "hyperdrive create test123 --host=example.com --database=neondb --user=test --password=password --access-client-id=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access --access-client-secret=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ); + expect(std.out).toMatchInlineSnapshot(` + "🚧 Creating 'test123' + ✅ Created new Hyperdrive config + { + \\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\", + \\"name\\": \\"test123\\", + \\"origin\\": { + \\"host\\": \\"example.com\\", + \\"database\\": \\"neondb\\", + \\"user\\": \\"test\\", + \\"access_client_id\\": \\"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access\\" + }, + \\"caching\\": { + \\"disabled\\": false + } + }" + `); + }); + + it("should reject a create hyperdrive over access command if access client ID is set but not access client secret", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive create test123 --host=example.com --database=neondb --user=test --password=password --access-client-id='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access'" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Missing dependent arguments: + + access-client-id -> access-client-secret + + " + `); + }); + + it("should reject a create hyperdrive over access command if access client secret is set but not access client ID", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive create test123 --host=example.com --database=neondb --user=test --password=password --access-client-secret=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Missing dependent arguments: + + access-client-secret -> access-client-id + + " + `); + }); + it("should handle listing configs", async () => { mockHyperdriveRequest(); await runWrangler("hyperdrive list"); @@ -315,10 +385,10 @@ describe("hyperdrive commands", () => { ) ).rejects.toThrow(); expect(std.err).toMatchInlineSnapshot(` - "X [ERROR] When updating the origin, all of the following must be set: origin-host, origin-port, database, origin-user, origin-password + "X [ERROR] When updating the origin, all of the following must be set: origin-host, database, origin-user, origin-password - " - `); + " + `); expect(std.out).toMatchInlineSnapshot(`""`); }); @@ -395,6 +465,77 @@ describe("hyperdrive commands", () => { }" `); }); + + it("should handle updating a hyperdrive to a hyperdrive over access config given the right parameters", async () => { + mockHyperdriveRequest(); + await runWrangler( + "hyperdrive update xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --origin-host=example.com --database=mydb --origin-user=newuser --origin-password='passw0rd!' --access-client-id='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access' --access-client-secret='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'" + ); + expect(std.out).toMatchInlineSnapshot(` + "🚧 Updating 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx' + ✅ Updated xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx Hyperdrive config + { + \\"id\\": \\"xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx\\", + \\"name\\": \\"test123\\", + \\"origin\\": { + \\"host\\": \\"example.com\\", + \\"database\\": \\"mydb\\", + \\"user\\": \\"newuser\\", + \\"access_client_id\\": \\"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access\\" + }, + \\"caching\\": { + \\"disabled\\": false + } + }" + `); + }); + + it("should throw an exception when updating a hyperdrive config's origin but neither port nor access credentials are provided", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive update xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --origin-host=example.com --database=mydb --origin-user=newuser --origin-password='passw0rd!'" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] When updating the origin, either the port or the Access Client ID and Secret must be set + + " + `); + expect(std.out).toMatchInlineSnapshot(`""`); + }); + + it("should reject an update command if the access client ID is provided but not the access client secret", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive update xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --origin-host=example.com --database=mydb --origin-user=newuser --origin-password='passw0rd!' --access-client-id='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx.access'" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Missing dependent arguments: + + access-client-id -> access-client-secret + + " + `); + }); + + it("should reject an update command if the access client secret is provided but not the access client ID", async () => { + mockHyperdriveRequest(); + await expect(() => + runWrangler( + "hyperdrive update xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx --origin-host=example.com --database=mydb --origin-user=newuser --origin-password='passw0rd!' --access-client-secret='xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'" + ) + ).rejects.toThrow(); + expect(std.err).toMatchInlineSnapshot(` + "X [ERROR] Missing dependent arguments: + + access-client-secret -> access-client-id + + " + `); + }); }); const defaultConfig: HyperdriveConfig = { @@ -437,6 +578,7 @@ function mockHyperdriveRequest() { // @ts-expect-error This is a string scheme: reqBody.origin.protocol, user: reqBody.origin.user, + access_client_id: reqBody.origin.access_client_id, }, caching: reqBody.caching, }, @@ -462,6 +604,7 @@ function mockHyperdriveRequest() { port: reqBody.origin.port, database: reqBody.origin.database, user: reqBody.origin.user, + access_client_id: reqBody.origin.access_client_id, } : defaultConfig.origin, caching: reqBody.caching ?? defaultConfig.caching, diff --git a/packages/wrangler/src/hyperdrive/client.ts b/packages/wrangler/src/hyperdrive/client.ts index ec900513468d..5ec3946d74d9 100644 --- a/packages/wrangler/src/hyperdrive/client.ts +++ b/packages/wrangler/src/hyperdrive/client.ts @@ -9,19 +9,18 @@ export type HyperdriveConfig = { caching: CachingOptions; }; -export type Origin = { +export type PublicOrigin = { host?: string; port?: number; -}; - -export type PublicOrigin = Origin & { scheme?: string; database?: string; user?: string; + access_client_id?: string; }; -export type OriginWithPassword = PublicOrigin & { +export type OriginWithSecrets = PublicOrigin & { password?: string; + access_client_secret?: string; }; export type CachingOptions = { @@ -32,13 +31,13 @@ export type CachingOptions = { export type CreateUpdateHyperdriveBody = { name: string; - origin: OriginWithPassword; + origin: OriginWithSecrets; caching: CachingOptions; }; export type PatchHyperdriveBody = { name?: string; - origin?: OriginWithPassword; + origin?: OriginWithSecrets; caching?: CachingOptions; }; diff --git a/packages/wrangler/src/hyperdrive/create.ts b/packages/wrangler/src/hyperdrive/create.ts index 2ae29b2d5e0e..d793c6998895 100644 --- a/packages/wrangler/src/hyperdrive/create.ts +++ b/packages/wrangler/src/hyperdrive/create.ts @@ -1,4 +1,5 @@ import { readConfig } from "../config"; +import { UserError } from "../errors"; import { logger } from "../logger"; import { createConfig } from "./client"; import type { @@ -16,10 +17,58 @@ export function options(yargs: CommonYargsArgv) { .options({ "connection-string": { type: "string", - demandOption: true, describe: "The connection string for the database you want Hyperdrive to connect to - ex: protocol://user:password@host:port/database", }, + host: { + type: "string", + describe: "The host of the origin database", + conflicts: "connection-string", + }, + port: { + type: "number", + describe: "The port number of the origin database", + conflicts: [ + "connection-string", + "access-client-id", + "access-client-secret", + ], + }, + scheme: { + type: "string", + describe: + "The scheme used to connect to the origin database - e.g. postgresql or postgres", + default: "postgresql", + }, + database: { + type: "string", + describe: "The name of the database within the origin database", + conflicts: "connection-string", + }, + user: { + type: "string", + describe: "The username used to connect to the origin database", + conflicts: "connection-string", + }, + password: { + type: "string", + describe: "The password used to connect to the origin database", + conflicts: "connection-string", + }, + "access-client-id": { + type: "string", + describe: + "The Client ID of the Access token to use when connecting to the origin database, must be set with a Client Access Secret", + conflicts: ["connection-string", "port"], + implies: ["access-client-secret"], + }, + "access-client-secret": { + type: "string", + describe: + "The Client Secret of the Access token to use when connecting to the origin database, must be set with a Client Access ID", + conflicts: ["connection-string", "port"], + implies: ["access-client-id"], + }, "caching-disabled": { type: "boolean", describe: "Disables the caching of SQL responses", @@ -43,7 +92,16 @@ export async function handler( ) { const config = readConfig(args.config, args); - const url = new URL(args.connectionString); + const url = args.connectionString + ? new URL(args.connectionString) + : buildURLFromParts( + args.host, + args.port, + args.scheme, + args.database, + args.user, + args.password + ); if ( url.port === "" && @@ -53,43 +111,59 @@ export async function handler( } if (url.protocol === "") { - logger.log("You must specify the database protocol - e.g. 'postgresql'."); + throw new UserError( + "You must specify the database protocol - e.g. 'postgresql'." + ); } else if (url.protocol !== "postgresql:" && url.protocol !== "postgres:") { - logger.log( + throw new UserError( "Only PostgreSQL or PostgreSQL compatible databases are currently supported." ); } else if (url.host === "") { - logger.log( + throw new UserError( "You must provide a hostname or IP address in your connection string - e.g. 'user:password@database-hostname.example.com:5432/databasename" ); } else if (url.port === "") { - logger.log( + throw new UserError( "You must provide a port number - e.g. 'user:password@database.example.com:port/databasename" ); } else if (url.pathname === "") { - logger.log( + throw new UserError( "You must provide a database name as the path component - e.g. /postgres" ); } else if (url.username === "") { - logger.log( + throw new UserError( "You must provide a username - e.g. 'user:password@database.example.com:port/databasename'" ); } else if (url.password === "") { - logger.log( + throw new UserError( "You must provide a password - e.g. 'user:password@database.example.com:port/databasename' " ); } else { logger.log(`🚧 Creating '${args.name}'`); + + // if access client ID and client secret supplied in args, use them to construct origin without a port + const origin = + args.accessClientId && args.accessClientSecret + ? { + host: url.hostname, + scheme: url.protocol.replace(":", ""), + database: decodeURIComponent(url.pathname.replace("/", "")), + user: decodeURIComponent(url.username), + password: decodeURIComponent(url.password), + access_client_id: args.accessClientId, + access_client_secret: args.accessClientSecret, + } + : { + host: url.hostname, + port: parseInt(url.port), + scheme: url.protocol.replace(":", ""), + database: decodeURIComponent(url.pathname.replace("/", "")), + user: decodeURIComponent(url.username), + password: decodeURIComponent(url.password), + }; const database = await createConfig(config, { name: args.name, - origin: { - host: url.hostname, - port: parseInt(url.port), - scheme: url.protocol.replace(":", ""), - database: decodeURIComponent(url.pathname.replace("/", "")), - user: decodeURIComponent(url.username), - password: decodeURIComponent(url.password), - }, + origin, caching: { disabled: args.cachingDisabled, max_age: args.maxAge, @@ -102,3 +176,28 @@ export async function handler( ); } } + +function buildURLFromParts( + host: string | undefined, + port: number | undefined, + scheme: string, + database: string | undefined, + user: string | undefined, + password: string | undefined +): URL { + const url = new URL(database ? `/${database}` : "", `${scheme}://${host}`); + + if (port) { + url.port = port.toString(); + } + + if (user) { + url.username = user; + } + + if (password) { + url.password = password; + } + + return url; +} diff --git a/packages/wrangler/src/hyperdrive/update.ts b/packages/wrangler/src/hyperdrive/update.ts index b2a4d72a06f8..8ee66fd86303 100644 --- a/packages/wrangler/src/hyperdrive/update.ts +++ b/packages/wrangler/src/hyperdrive/update.ts @@ -42,6 +42,20 @@ export function options(yargs: CommonYargsArgv) { type: "string", describe: "The password used to connect to the origin database", }, + "access-client-id": { + type: "string", + describe: + "The Client ID of the Access token to use when connecting to the origin database", + conflicts: ["origin-port"], + implies: ["access-client-secret"], + }, + "access-client-secret": { + type: "string", + describe: + "The Client Secret of the Access token to use when connecting to the origin database", + conflicts: ["origin-port"], + implies: ["access-client-id"], + }, "caching-disabled": { type: "boolean", describe: "Disables the caching of SQL responses", @@ -62,7 +76,6 @@ export function options(yargs: CommonYargsArgv) { const requiredOriginOptions = [ "originHost", - "originPort", "database", "originUser", "originPassword", @@ -96,6 +109,17 @@ export async function handler( ); } + if ( + allOriginFieldsSet && + args.originPort === undefined && + args.accessClientId === undefined && + args.accessClientSecret === undefined + ) { + throw new UserError( + `When updating the origin, either the port or the Access Client ID and Secret must be set` + ); + } + const config = readConfig(args.config, args); logger.log(`🚧 Updating '${args.id}'`); @@ -107,14 +131,26 @@ export async function handler( } if (allOriginFieldsSet) { - database.origin = { - scheme: args.originScheme ?? "postgresql", - host: args.originHost, - port: args.originPort, - database: args.database, - user: args.originUser, - password: args.originPassword, - }; + if (args.accessClientId && args.accessClientSecret) { + database.origin = { + scheme: args.originScheme ?? "postgresql", + host: args.originHost, + database: args.database, + user: args.originUser, + password: args.originPassword, + access_client_id: args.accessClientId, + access_client_secret: args.accessClientSecret, + }; + } else { + database.origin = { + scheme: args.originScheme ?? "postgresql", + host: args.originHost, + port: args.originPort, + database: args.database, + user: args.originUser, + password: args.originPassword, + }; + } } database.caching = {