From 5424ff774268a3548a23b2b0b7b960866cba801c Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 10:45:36 +0100 Subject: [PATCH 1/7] chore: support cloudsql-connector Signed-off-by: blam --- packages/backend-defaults/package.json | 8 ++ .../entrypoints/database/DatabaseManager.ts | 1 + .../database/connectors/postgres.test.ts | 114 ++++++++++++++---- .../database/connectors/postgres.ts | 41 +++++-- yarn.lock | 12 +- 5 files changed, 140 insertions(+), 36 deletions(-) diff --git a/packages/backend-defaults/package.json b/packages/backend-defaults/package.json index 689688630e..4612cac52b 100644 --- a/packages/backend-defaults/package.json +++ b/packages/backend-defaults/package.json @@ -185,6 +185,14 @@ "yn": "^4.0.0", "zod": "^3.22.4" }, + "peerDependencies": { + "@google-cloud/cloud-sql-connector": "^1.4.0" + }, + "peerDependenciesMeta": { + "@google-cloud/cloud-sql-connector": { + "optional": true + } + }, "devDependencies": { "@aws-sdk/util-stream-node": "^3.350.0", "@backstage/backend-plugin-api": "workspace:^", diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts index 75e7d35eee..4ea66de2cc 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts +++ b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts @@ -252,6 +252,7 @@ export class DatabaseManager { databaseConfig, { pg: new PgConnector(databaseConfig, prefix), + 'pg+cloudsql': new PgConnector(databaseConfig, prefix), sqlite3: new Sqlite3Connector(databaseConfig), 'better-sqlite3': new Sqlite3Connector(databaseConfig), mysql: new MysqlConnector(databaseConfig, prefix), diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts index 1e07984967..55568bc816 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts @@ -22,6 +22,8 @@ import { parsePgConnectionString, } from './postgres'; +jest.mock('@google-cloud/cloud-sql-connector'); + describe('postgres', () => { const createMockConnection = () => ({ host: 'acme', @@ -37,33 +39,35 @@ describe('postgres', () => { new ConfigReader({ client: 'pg', connection }); describe('buildPgDatabaseConfig', () => { - it('builds a postgres config', () => { + it('builds a postgres config', async () => { const mockConnection = createMockConnection(); - expect(buildPgDatabaseConfig(createConfig(mockConnection))).toEqual({ - client: 'pg', - connection: mockConnection, - useNullAsDefault: true, - }); - }); - - it('builds a connection string config', () => { - const mockConnectionString = createMockConnectionString(); - - expect(buildPgDatabaseConfig(createConfig(mockConnectionString))).toEqual( + expect(await buildPgDatabaseConfig(createConfig(mockConnection))).toEqual( { client: 'pg', - connection: mockConnectionString, + connection: mockConnection, useNullAsDefault: true, }, ); }); - it('overrides the database name', () => { + it('builds a connection string config', async () => { + const mockConnectionString = createMockConnectionString(); + + expect( + await buildPgDatabaseConfig(createConfig(mockConnectionString)), + ).toEqual({ + client: 'pg', + connection: mockConnectionString, + useNullAsDefault: true, + }); + }); + + it('overrides the database name', async () => { const mockConnection = createMockConnection(); expect( - buildPgDatabaseConfig(createConfig(mockConnection), { + await buildPgDatabaseConfig(createConfig(mockConnection), { connection: { database: 'other_db' }, }), ).toEqual({ @@ -76,14 +80,14 @@ describe('postgres', () => { }); }); - it('overrides the schema name', () => { + it('overrides the schema name', async () => { const mockConnection = { ...createMockConnection(), schema: 'schemaName', }; expect( - buildPgDatabaseConfig(createConfig(mockConnection), { + await buildPgDatabaseConfig(createConfig(mockConnection), { searchPath: ['schemaName'], }), ).toEqual({ @@ -94,11 +98,11 @@ describe('postgres', () => { }); }); - it('adds additional config settings', () => { + it('adds additional config settings', async () => { const mockConnection = createMockConnection(); expect( - buildPgDatabaseConfig(createConfig(mockConnection), { + await buildPgDatabaseConfig(createConfig(mockConnection), { connection: { database: 'other_db' }, pool: { min: 0, max: 7 }, debug: true, @@ -115,12 +119,12 @@ describe('postgres', () => { }); }); - it('overrides the database from connection string', () => { + it('overrides the database from connection string', async () => { const mockConnectionString = createMockConnectionString(); const mockConnection = createMockConnection(); expect( - buildPgDatabaseConfig(createConfig(mockConnectionString), { + await buildPgDatabaseConfig(createConfig(mockConnectionString), { connection: { database: 'other_db' }, }), ).toEqual({ @@ -133,6 +137,64 @@ describe('postgres', () => { useNullAsDefault: true, }); }); + + it('uses the correct config when using pg+google-cloud-sql', async () => { + const mockConnectionString = createMockConnectionString(); + const mockConnection = createMockConnection(); + + expect( + await buildPgDatabaseConfig( + new ConfigReader({ + client: 'pg+google-cloud-sql', + connection: mockConnectionString, + instanceConnectionName: 'project:region:instance', + }), + { connection: { database: 'other_db' } }, + ), + ).toEqual({ + client: 'pg', + connection: { + ...mockConnection, + port: '5432', + database: 'other_db', + }, + instanceConnectionName: 'project:region:instance', + useNullAsDefault: true, + }); + }); + + it('adds the settings from cloud-sql-connector', async () => { + const { Connector } = jest.requireMock( + '@google-cloud/cloud-sql-connector', + ) as jest.Mocked; + + const mockStream = (): any => {}; + Connector.prototype.getOptions.mockResolvedValue({ stream: mockStream }); + + const mockConnectionString = createMockConnectionString(); + const mockConnection = createMockConnection(); + + expect( + await buildPgDatabaseConfig( + new ConfigReader({ + client: 'pg+google-cloud-sql', + connection: mockConnectionString, + instanceConnectionName: 'project:region:instance', + }), + { connection: { database: 'other_db' } }, + ), + ).toEqual({ + client: 'pg', + connection: { + ...mockConnection, + port: '5432', + database: 'other_db', + stream: mockStream, + }, + instanceConnectionName: 'project:region:instance', + useNullAsDefault: true, + }); + }); }); describe('getPgConnectionConfig', () => { @@ -174,9 +236,9 @@ describe('postgres', () => { }); describe('createPgDatabaseClient', () => { - it('creates a postgres knex instance', () => { + it('creates a postgres knex instance', async () => { expect( - createPgDatabaseClient( + await createPgDatabaseClient( createConfig({ host: 'acme', user: 'foo', @@ -187,14 +249,14 @@ describe('postgres', () => { ).toBeTruthy(); }); - it('attempts to read an ssl cert', () => { - expect(() => + it('attempts to read an ssl cert', async () => { + await expect(() => createPgDatabaseClient( createConfig( 'postgresql://postgres:pass@localhost:5432/dbname?sslrootcert=/path/to/file', ), ), - ).toThrow(/no such file or directory/); + ).rejects.toThrow(/no such file or directory/); }); }); diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index b5cfffd1f4..c81d352b74 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -37,11 +37,11 @@ const ddlLimiter = limiterFactory(1); * @param dbConfig - The database config * @param overrides - Additional options to merge with the config */ -export function createPgDatabaseClient( +export async function createPgDatabaseClient( dbConfig: Config, overrides?: Knex.Config, ) { - const knexConfig = buildPgDatabaseConfig(dbConfig, overrides); + const knexConfig = await buildPgDatabaseConfig(dbConfig, overrides); const database = knexFactory(knexConfig); const role = dbConfig.getOptionalString('role'); @@ -64,11 +64,11 @@ export function createPgDatabaseClient( * @param dbConfig - The database config * @param overrides - Additional options to merge with the config */ -export function buildPgDatabaseConfig( +export async function buildPgDatabaseConfig( dbConfig: Config, overrides?: Knex.Config, ) { - return mergeDatabaseConfig( + const config = mergeDatabaseConfig( dbConfig.get(), { connection: getPgConnectionConfig(dbConfig, !!overrides), @@ -76,6 +76,33 @@ export function buildPgDatabaseConfig( }, overrides, ); + + if (config.client === 'pg+google-cloud-sql') { + const { + Connector: CloudSqlConnector, + IpAddressTypes, + AuthTypes, + } = await import('@google-cloud/cloud-sql-connector'); + // override the config to be pg for backwards compat with other code + config.client = 'pg'; + + const connector = new CloudSqlConnector(); + const clientOpts = await connector.getOptions({ + instanceConnectionName: dbConfig.getString('instanceConnectionName'), + ipType: IpAddressTypes.PUBLIC, + authType: AuthTypes.IAM, + }); + + return { + ...config, + connection: { + ...config.connection, + ...clientOpts, + }, + }; + } + + return config; } /** @@ -130,7 +157,7 @@ export async function ensurePgDatabaseExists( dbConfig: Config, ...databases: Array ) { - const admin = createPgDatabaseClient(dbConfig, { + const admin = await createPgDatabaseClient(dbConfig, { connection: { database: 'postgres', }, @@ -186,7 +213,7 @@ export async function ensurePgSchemaExists( dbConfig: Config, ...schemas: Array ): Promise { - const admin = createPgDatabaseClient(dbConfig); + const admin = await createPgDatabaseClient(dbConfig); const role = dbConfig.getOptionalString('role'); try { @@ -219,7 +246,7 @@ export async function dropPgDatabase( dbConfig: Config, ...databases: Array ) { - const admin = createPgDatabaseClient(dbConfig); + const admin = await createPgDatabaseClient(dbConfig); try { await Promise.all( databases.map(async database => { diff --git a/yarn.lock b/yarn.lock index 3075fcd983..915e711688 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3662,6 +3662,11 @@ __metadata: yauzl: ^3.0.0 yn: ^4.0.0 zod: ^3.22.4 + peerDependencies: + "@google-cloud/cloud-sql-connector": ^1.4.0 + peerDependenciesMeta: + "@google-cloud/cloud-sql-connector": + optional: true languageName: unknown linkType: soft @@ -30112,14 +30117,15 @@ __metadata: linkType: hard "gaxios@npm:^6.0.0, gaxios@npm:^6.0.2, gaxios@npm:^6.1.1": - version: 6.3.0 - resolution: "gaxios@npm:6.3.0" + version: 6.7.1 + resolution: "gaxios@npm:6.7.1" dependencies: extend: ^3.0.2 https-proxy-agent: ^7.0.1 is-stream: ^2.0.0 node-fetch: ^2.6.9 - checksum: 4d4a8db32d833f8012435e2016cb0c919cac288e821bf81f877504e4284ef12b444cd903448e738c4031cd5219adf1e8d68e7df2b3dba774db9fde27f71109d4 + uuid: ^9.0.1 + checksum: ed5952655339918e0868c6f4e079d6e9e55b20a242ddb1be25076cdfb0dd1ca5a2cb233da7352baa972c19f898a78fa6ba67e7d848717c9ca9877c269b5b02f7 languageName: node linkType: hard From 1ac6b721ec725095945953e6135e47423bc87063 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 10:47:38 +0100 Subject: [PATCH 2/7] chore: add dev dep and changeset Signed-off-by: blam Signed-off-by: blam --- .changeset/pink-pets-jump.md | 5 +++ packages/backend-defaults/package.json | 1 + yarn.lock | 56 ++++++++++++++++++++++++-- 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 .changeset/pink-pets-jump.md diff --git a/.changeset/pink-pets-jump.md b/.changeset/pink-pets-jump.md new file mode 100644 index 0000000000..1d65169349 --- /dev/null +++ b/.changeset/pink-pets-jump.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-defaults': patch +--- + +Support `client: pg+google-cloud-sql` in database client for usage with `@google-cloud/cloud-sql-connector` and `iam` auth diff --git a/packages/backend-defaults/package.json b/packages/backend-defaults/package.json index 4612cac52b..b0f77d5c44 100644 --- a/packages/backend-defaults/package.json +++ b/packages/backend-defaults/package.json @@ -198,6 +198,7 @@ "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", + "@google-cloud/cloud-sql-connector": "^1.4.0", "@types/archiver": "^6.0.0", "@types/base64-stream": "^1.0.2", "@types/concat-stream": "^2.0.0", diff --git a/yarn.lock b/yarn.lock index 915e711688..1dfd85032c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3599,6 +3599,7 @@ __metadata: "@backstage/plugin-events-node": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" "@backstage/types": "workspace:^" + "@google-cloud/cloud-sql-connector": ^1.4.0 "@google-cloud/storage": ^7.0.0 "@keyv/memcache": ^2.0.1 "@keyv/redis": ^4.0.1 @@ -10343,6 +10344,18 @@ __metadata: languageName: node linkType: hard +"@google-cloud/cloud-sql-connector@npm:^1.4.0": + version: 1.4.0 + resolution: "@google-cloud/cloud-sql-connector@npm:1.4.0" + dependencies: + "@googleapis/sqladmin": ^24.0.0 + gaxios: ^6.1.1 + google-auth-library: ^9.2.0 + p-throttle: ^5.1.0 + checksum: c4ac2cc8cc24cc5966f146fe5686493fdb0bb4fc43a27ec086bd116f5ce475ca88f5e5e5fb79d17e6c31fb1a752d2f7e94433db17eea787696556c45415a31a1 + languageName: node + linkType: hard + "@google-cloud/container@npm:^5.0.0": version: 5.19.0 resolution: "@google-cloud/container@npm:5.19.0" @@ -10412,6 +10425,15 @@ __metadata: languageName: node linkType: hard +"@googleapis/sqladmin@npm:^24.0.0": + version: 24.0.0 + resolution: "@googleapis/sqladmin@npm:24.0.0" + dependencies: + googleapis-common: ^7.0.0 + checksum: b1c93d18b800ecd5af79dc6728e6b030f6918a1c5e2094d2dd7b460e3b2033cb9ce4f4ffe5b6155ecc2f699b8e4bdf0a94999786671b8fa05e1d8f3ce90cf466 + languageName: node + linkType: hard + "@graphiql/react@npm:^0.20.3": version: 0.20.3 resolution: "@graphiql/react@npm:0.20.3" @@ -30116,7 +30138,7 @@ __metadata: languageName: node linkType: hard -"gaxios@npm:^6.0.0, gaxios@npm:^6.0.2, gaxios@npm:^6.1.1": +"gaxios@npm:^6.0.0, gaxios@npm:^6.0.2, gaxios@npm:^6.0.3, gaxios@npm:^6.1.1": version: 6.7.1 resolution: "gaxios@npm:6.7.1" dependencies: @@ -30598,7 +30620,7 @@ __metadata: languageName: node linkType: hard -"google-auth-library@npm:^9.0.0, google-auth-library@npm:^9.3.0, google-auth-library@npm:^9.6.3": +"google-auth-library@npm:^9.0.0, google-auth-library@npm:^9.2.0, google-auth-library@npm:^9.3.0, google-auth-library@npm:^9.6.3, google-auth-library@npm:^9.7.0": version: 9.15.0 resolution: "google-auth-library@npm:9.15.0" dependencies: @@ -30639,6 +30661,20 @@ __metadata: languageName: node linkType: hard +"googleapis-common@npm:^7.0.0": + version: 7.2.0 + resolution: "googleapis-common@npm:7.2.0" + dependencies: + extend: ^3.0.2 + gaxios: ^6.0.3 + google-auth-library: ^9.7.0 + qs: ^6.7.0 + url-template: ^2.0.8 + uuid: ^9.0.0 + checksum: 58f520134c9d6f439ef81919471689f0278ef0ffdbd50c693a59282d95141be74df3a5614c25347c140fc44201e0ef998300389f4cbf51502f2351e67c758ab6 + languageName: node + linkType: hard + "gopd@npm:^1.0.1": version: 1.0.1 resolution: "gopd@npm:1.0.1" @@ -38309,6 +38345,13 @@ __metadata: languageName: node linkType: hard +"p-throttle@npm:^5.1.0": + version: 5.1.0 + resolution: "p-throttle@npm:5.1.0" + checksum: c412429cbb759a0772a083200a556e5866d4a7cb5383a9cc85880f47f07f4b1787fc33d2c824ab659fff0fc4a34a56e1d0712e88808f0c0ae0ffbe93b54f46c1 + languageName: node + linkType: hard + "p-timeout@npm:^3.2.0": version: 3.2.0 resolution: "p-timeout@npm:3.2.0" @@ -40271,7 +40314,7 @@ __metadata: languageName: node linkType: hard -"qs@npm:^6.10.1, qs@npm:^6.10.3, qs@npm:^6.11.0, qs@npm:^6.12.2, qs@npm:^6.9.4": +"qs@npm:^6.10.1, qs@npm:^6.10.3, qs@npm:^6.11.0, qs@npm:^6.12.2, qs@npm:^6.7.0, qs@npm:^6.9.4": version: 6.13.1 resolution: "qs@npm:6.13.1" dependencies: @@ -46116,6 +46159,13 @@ __metadata: languageName: node linkType: hard +"url-template@npm:^2.0.8": + version: 2.0.8 + resolution: "url-template@npm:2.0.8" + checksum: 4183fccd74e3591e4154134d4443dccecba9c455c15c7df774f1f1e3fa340fd9bffb903b5beec347196d15ce49c34edf6dec0634a95d170ad6e78c0467d6e13e + languageName: node + linkType: hard + "url-value-parser@npm:^2.0.0": version: 2.0.3 resolution: "url-value-parser@npm:2.0.3" From bc27c79e45cf5ad22298880dc3072ba910565a81 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 10:49:43 +0100 Subject: [PATCH 3/7] chore: rename to `google-cloudsql` Signed-off-by: blam Signed-off-by: blam --- .changeset/pink-pets-jump.md | 2 +- .../src/entrypoints/database/DatabaseManager.ts | 2 +- .../src/entrypoints/database/connectors/postgres.test.ts | 6 +++--- .../src/entrypoints/database/connectors/postgres.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.changeset/pink-pets-jump.md b/.changeset/pink-pets-jump.md index 1d65169349..eb01eb27e1 100644 --- a/.changeset/pink-pets-jump.md +++ b/.changeset/pink-pets-jump.md @@ -2,4 +2,4 @@ '@backstage/backend-defaults': patch --- -Support `client: pg+google-cloud-sql` in database client for usage with `@google-cloud/cloud-sql-connector` and `iam` auth +Support `client: pg+google-cloudsql` in database client for usage with `@google-cloud/cloud-sql-connector` and `iam` auth diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts index 4ea66de2cc..d94b462794 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts +++ b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts @@ -252,7 +252,7 @@ export class DatabaseManager { databaseConfig, { pg: new PgConnector(databaseConfig, prefix), - 'pg+cloudsql': new PgConnector(databaseConfig, prefix), + 'pg+google-cloudsql': new PgConnector(databaseConfig, prefix), sqlite3: new Sqlite3Connector(databaseConfig), 'better-sqlite3': new Sqlite3Connector(databaseConfig), mysql: new MysqlConnector(databaseConfig, prefix), diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts index 55568bc816..9d820510fd 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts @@ -138,14 +138,14 @@ describe('postgres', () => { }); }); - it('uses the correct config when using pg+google-cloud-sql', async () => { + it('uses the correct config when using pg+google-cloudsql', async () => { const mockConnectionString = createMockConnectionString(); const mockConnection = createMockConnection(); expect( await buildPgDatabaseConfig( new ConfigReader({ - client: 'pg+google-cloud-sql', + client: 'pg+google-cloudsql', connection: mockConnectionString, instanceConnectionName: 'project:region:instance', }), @@ -177,7 +177,7 @@ describe('postgres', () => { expect( await buildPgDatabaseConfig( new ConfigReader({ - client: 'pg+google-cloud-sql', + client: 'pg+google-cloudsql', connection: mockConnectionString, instanceConnectionName: 'project:region:instance', }), diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index c81d352b74..a7cb173972 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -77,7 +77,7 @@ export async function buildPgDatabaseConfig( overrides, ); - if (config.client === 'pg+google-cloud-sql') { + if (config.client === 'pg+google-cloudsql') { const { Connector: CloudSqlConnector, IpAddressTypes, From 4bdfcf05ba22c56cd813ea3b004ee9dff46955a1 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 11:43:52 +0100 Subject: [PATCH 4/7] chore: adjust config a little bit Signed-off-by: blam --- packages/backend-defaults/config.d.ts | 26 ++++++++++++-- .../database/connectors/postgres.test.ts | 34 +++++++++---------- .../database/connectors/postgres.ts | 10 +++--- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/packages/backend-defaults/config.d.ts b/packages/backend-defaults/config.d.ts index c15fe45139..b88671748e 100644 --- a/packages/backend-defaults/config.d.ts +++ b/packages/backend-defaults/config.d.ts @@ -377,7 +377,7 @@ export interface Config { /** Database connection configuration, select base database type using the `client` field */ database: { /** Default database client to use */ - client: 'better-sqlite3' | 'sqlite3' | 'pg'; + client: 'better-sqlite3' | 'sqlite3' | 'pg' | 'pg+google-cloudsql'; /** * Base database connection string, or object with individual connection properties * @visibility secret @@ -390,6 +390,10 @@ export interface Config { * @visibility secret */ password?: string; + /** + * The instance connection name to use for google cloudsql connector. Should be format of `project:region:instance` + */ + instance?: string; /** * Other connection settings */ @@ -436,12 +440,28 @@ export interface Config { plugin?: { [pluginId: string]: { /** Database client override */ - client?: 'better-sqlite3' | 'sqlite3' | 'pg'; + client?: 'better-sqlite3' | 'sqlite3' | 'pg' | 'pg+google-cloudsql'; /** * Database connection string or Knex object override * @visibility secret */ - connection?: string | object; + connection?: + | string + | { + /** + * Password that belongs to the client User + * @visibility secret + */ + password?: string; + /** + * The instance connection name to use for google cloudsql connector. Should be format of `project:region:instance` + */ + instance?: string; + /** + * Other connection settings + */ + [key: string]: unknown; + }; /** * Whether to ensure the given database exists by creating it if it does not. * Defaults to base config if unspecified. diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts index 9d820510fd..71c6b882c7 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts @@ -139,26 +139,26 @@ describe('postgres', () => { }); it('uses the correct config when using pg+google-cloudsql', async () => { - const mockConnectionString = createMockConnectionString(); - const mockConnection = createMockConnection(); - expect( await buildPgDatabaseConfig( new ConfigReader({ client: 'pg+google-cloudsql', - connection: mockConnectionString, - instanceConnectionName: 'project:region:instance', + connection: { + user: 'ben@gke.com', + instance: 'project:region:instance', + port: 5423, + }, }), { connection: { database: 'other_db' } }, ), ).toEqual({ client: 'pg', connection: { - ...mockConnection, - port: '5432', + user: 'ben@gke.com', + instance: 'project:region:instance', + port: 5423, database: 'other_db', }, - instanceConnectionName: 'project:region:instance', useNullAsDefault: true, }); }); @@ -171,27 +171,27 @@ describe('postgres', () => { const mockStream = (): any => {}; Connector.prototype.getOptions.mockResolvedValue({ stream: mockStream }); - const mockConnectionString = createMockConnectionString(); - const mockConnection = createMockConnection(); - expect( await buildPgDatabaseConfig( new ConfigReader({ client: 'pg+google-cloudsql', - connection: mockConnectionString, - instanceConnectionName: 'project:region:instance', + connection: { + user: 'ben@gke.com', + instance: 'project:region:instance', + port: 5423, + }, }), { connection: { database: 'other_db' } }, ), ).toEqual({ client: 'pg', connection: { - ...mockConnection, - port: '5432', - database: 'other_db', + user: 'ben@gke.com', + instance: 'project:region:instance', + port: 5423, stream: mockStream, + database: 'other_db', }, - instanceConnectionName: 'project:region:instance', useNullAsDefault: true, }); }); diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index a7cb173972..66c1c0ffb2 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -78,23 +78,25 @@ export async function buildPgDatabaseConfig( ); if (config.client === 'pg+google-cloudsql') { + if (!config.connection.instance) { + throw new Error('Missing instance connection name for Cloud SQL'); + } + const { Connector: CloudSqlConnector, IpAddressTypes, AuthTypes, } = await import('@google-cloud/cloud-sql-connector'); - // override the config to be pg for backwards compat with other code - config.client = 'pg'; - const connector = new CloudSqlConnector(); const clientOpts = await connector.getOptions({ - instanceConnectionName: dbConfig.getString('instanceConnectionName'), + instanceConnectionName: config.connection.instance, ipType: IpAddressTypes.PUBLIC, authType: AuthTypes.IAM, }); return { ...config, + client: 'pg', connection: { ...config.connection, ...clientOpts, From 35d626380380bc57ec039a82f2dedc7b8728aa54 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 13:49:53 +0100 Subject: [PATCH 5/7] chore: rework solution Signed-off-by: blam --- packages/backend-defaults/config.d.ts | 33 +++++++++++++------ .../database/connectors/postgres.test.ts | 10 +++--- .../database/connectors/postgres.ts | 10 ++++-- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/packages/backend-defaults/config.d.ts b/packages/backend-defaults/config.d.ts index b88671748e..8272bc5ca8 100644 --- a/packages/backend-defaults/config.d.ts +++ b/packages/backend-defaults/config.d.ts @@ -377,23 +377,29 @@ export interface Config { /** Database connection configuration, select base database type using the `client` field */ database: { /** Default database client to use */ - client: 'better-sqlite3' | 'sqlite3' | 'pg' | 'pg+google-cloudsql'; + client: 'better-sqlite3' | 'sqlite3' | 'pg'; /** * Base database connection string, or object with individual connection properties * @visibility secret */ connection: | string + | { + /** + * The specific config for cloudsql connections + */ + type: 'cloudsql'; + /** + * The instance connection name for the cloudsql instance, e.g. `project:region:instance` + */ + instance: string; + } | { /** * Password that belongs to the client User * @visibility secret */ password?: string; - /** - * The instance connection name to use for google cloudsql connector. Should be format of `project:region:instance` - */ - instance?: string; /** * Other connection settings */ @@ -440,28 +446,35 @@ export interface Config { plugin?: { [pluginId: string]: { /** Database client override */ - client?: 'better-sqlite3' | 'sqlite3' | 'pg' | 'pg+google-cloudsql'; + client?: 'better-sqlite3' | 'sqlite3' | 'pg'; /** * Database connection string or Knex object override * @visibility secret */ connection?: | string + | { + /** + * The specific config for cloudsql connections + */ + type: 'cloudsql'; + /** + * The instance connection name for the cloudsql instance, e.g. `project:region:instance` + */ + instance: string; + } | { /** * Password that belongs to the client User * @visibility secret */ password?: string; - /** - * The instance connection name to use for google cloudsql connector. Should be format of `project:region:instance` - */ - instance?: string; /** * Other connection settings */ [key: string]: unknown; }; + /** * Whether to ensure the given database exists by creating it if it does not. * Defaults to base config if unspecified. diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts index 71c6b882c7..2481004131 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts @@ -138,12 +138,13 @@ describe('postgres', () => { }); }); - it('uses the correct config when using pg+google-cloudsql', async () => { + it('uses the correct config when using cloudsql', async () => { expect( await buildPgDatabaseConfig( new ConfigReader({ - client: 'pg+google-cloudsql', + client: 'pg', connection: { + type: 'cloudsql', user: 'ben@gke.com', instance: 'project:region:instance', port: 5423, @@ -155,7 +156,6 @@ describe('postgres', () => { client: 'pg', connection: { user: 'ben@gke.com', - instance: 'project:region:instance', port: 5423, database: 'other_db', }, @@ -174,8 +174,9 @@ describe('postgres', () => { expect( await buildPgDatabaseConfig( new ConfigReader({ - client: 'pg+google-cloudsql', + client: 'pg', connection: { + type: 'cloudsql', user: 'ben@gke.com', instance: 'project:region:instance', port: 5423, @@ -187,7 +188,6 @@ describe('postgres', () => { client: 'pg', connection: { user: 'ben@gke.com', - instance: 'project:region:instance', port: 5423, stream: mockStream, database: 'other_db', diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index 66c1c0ffb2..b938d657a2 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -77,7 +77,7 @@ export async function buildPgDatabaseConfig( overrides, ); - if (config.client === 'pg+google-cloudsql') { + if (config.connection?.type === 'cloudsql') { if (!config.connection.instance) { throw new Error('Missing instance connection name for Cloud SQL'); } @@ -94,7 +94,7 @@ export async function buildPgDatabaseConfig( authType: AuthTypes.IAM, }); - return { + const cloudsqlConfig = { ...config, client: 'pg', connection: { @@ -102,6 +102,12 @@ export async function buildPgDatabaseConfig( ...clientOpts, }, }; + + // Trim additional properties from the connection object passed to knex + delete cloudsqlConfig.connection.type; + delete cloudsqlConfig.connection.instance; + + return cloudsqlConfig; } return config; From bc4ae198efceb58d60238aca03a95e6afa493beb Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Nov 2024 13:50:40 +0100 Subject: [PATCH 6/7] chore: update changeset and implementation Signed-off-by: blam --- .changeset/pink-pets-jump.md | 2 +- .../src/entrypoints/database/DatabaseManager.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.changeset/pink-pets-jump.md b/.changeset/pink-pets-jump.md index eb01eb27e1..d9bb97e36e 100644 --- a/.changeset/pink-pets-jump.md +++ b/.changeset/pink-pets-jump.md @@ -2,4 +2,4 @@ '@backstage/backend-defaults': patch --- -Support `client: pg+google-cloudsql` in database client for usage with `@google-cloud/cloud-sql-connector` and `iam` auth +Support `connection.type: cloudsql` in database client for usage with `@google-cloud/cloud-sql-connector` and `iam` auth diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts index d94b462794..75e7d35eee 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts +++ b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts @@ -252,7 +252,6 @@ export class DatabaseManager { databaseConfig, { pg: new PgConnector(databaseConfig, prefix), - 'pg+google-cloudsql': new PgConnector(databaseConfig, prefix), sqlite3: new Sqlite3Connector(databaseConfig), 'better-sqlite3': new Sqlite3Connector(databaseConfig), mysql: new MysqlConnector(databaseConfig, prefix), From ed5abc4340dbd8c2bb869b7a0a3df4f21e16ba21 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 29 Nov 2024 09:40:58 +0100 Subject: [PATCH 7/7] chore: code review Signed-off-by: blam Signed-off-by: blam --- .../database/connectors/postgres.test.ts | 25 +++++++++++++++++++ .../database/connectors/postgres.ts | 4 +++ 2 files changed, 29 insertions(+) diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts index 2481004131..6f66f98f15 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.test.ts @@ -163,6 +163,31 @@ describe('postgres', () => { }); }); + it('should throw with incorrect config', async () => { + await expect( + buildPgDatabaseConfig( + new ConfigReader({ + client: 'pg', + connection: { + type: 'cloudsql', + }, + }), + ), + ).rejects.toThrow(/Missing instance connection name for Cloud SQL/); + + await expect( + buildPgDatabaseConfig( + new ConfigReader({ + client: 'not-pg', + connection: { + type: 'cloudsql', + instance: 'asd:asd:asd', + }, + }), + ), + ).rejects.toThrow(/Cloud SQL only supports the pg client/); + }); + it('adds the settings from cloud-sql-connector', async () => { const { Connector } = jest.requireMock( '@google-cloud/cloud-sql-connector', diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index b938d657a2..5f21d278e7 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -78,6 +78,10 @@ export async function buildPgDatabaseConfig( ); if (config.connection?.type === 'cloudsql') { + if (config.client !== 'pg') { + throw new Error('Cloud SQL only supports the pg client'); + } + if (!config.connection.instance) { throw new Error('Missing instance connection name for Cloud SQL'); }