diff --git a/packages/backend-common/src/database/connection.test.ts b/packages/backend-common/src/database/connection.test.ts index 814503fda1..2cc3a54e06 100644 --- a/packages/backend-common/src/database/connection.test.ts +++ b/packages/backend-common/src/database/connection.test.ts @@ -15,7 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; -import { createDatabase } from './connection'; +import { createDatabaseClient } from './connection'; describe('database connection', () => { const createConfig = (data: any) => @@ -26,10 +26,10 @@ describe('database connection', () => { }, ]); - describe(createDatabase, () => { + describe(createDatabaseClient, () => { it('returns a postgres connection', () => { expect( - createDatabase( + createDatabaseClient( createConfig({ client: 'pg', connection: { @@ -45,7 +45,7 @@ describe('database connection', () => { it('returns an sqlite connection', () => { expect( - createDatabase( + createDatabaseClient( createConfig({ client: 'sqlite3', connection: ':memory:', @@ -56,7 +56,7 @@ describe('database connection', () => { it('tries to create a mysql connection as a passthrough', () => { expect(() => - createDatabase( + createDatabaseClient( createConfig({ client: 'mysql', connection: { @@ -72,7 +72,7 @@ describe('database connection', () => { it('accepts overrides', () => { expect( - createDatabase( + createDatabaseClient( createConfig({ client: 'pg', connection: { @@ -93,7 +93,7 @@ describe('database connection', () => { it('throws an error without a client', () => { expect(() => - createDatabase( + createDatabaseClient( createConfig({ connection: '', }), @@ -103,7 +103,7 @@ describe('database connection', () => { it('throws an error without a connection', () => { expect(() => - createDatabase( + createDatabaseClient( createConfig({ client: 'pg', }), diff --git a/packages/backend-common/src/database/connection.ts b/packages/backend-common/src/database/connection.ts index d968275482..009a85239f 100644 --- a/packages/backend-common/src/database/connection.ts +++ b/packages/backend-common/src/database/connection.ts @@ -15,30 +15,36 @@ */ import knex from 'knex'; -import { ConfigReader } from '@backstage/config'; +import { Config } from '@backstage/config'; import { mergeDatabaseConfig } from './config'; -import { createPgDatabase } from './postgres'; -import { createSqlite3Database } from './sqlite3'; +import { createPgDatabaseClient } from './postgres'; +import { createSqliteDatabaseClient } from './sqlite3'; type DatabaseClient = 'pg' | 'sqlite3' | string; /** * Creates a knex database connection * - * @param config The database config + * @param dbConfig The database config * @param overrides Additional options to merge with the config */ -export function createDatabase( - config: ConfigReader, +export function createDatabaseClient( + dbConfig: Config, overrides?: Partial, ) { - const client: DatabaseClient = config.getString('client'); + const client: DatabaseClient = dbConfig.getString('client'); if (client === 'pg') { - return createPgDatabase(config, overrides); + return createPgDatabaseClient(dbConfig, overrides); } else if (client === 'sqlite3') { - return createSqlite3Database(config); + return createSqliteDatabaseClient(dbConfig); } - return knex(mergeDatabaseConfig(config.get(), overrides)); + return knex(mergeDatabaseConfig(dbConfig.get(), overrides)); } + +/** + * Alias for createDatabaseClient + * @deprecated Use createDatabaseClient instead + */ +export const createDatabase = createDatabaseClient; diff --git a/packages/backend-common/src/database/postgres.test.ts b/packages/backend-common/src/database/postgres.test.ts index 8edfc2c15d..82161dc53c 100644 --- a/packages/backend-common/src/database/postgres.test.ts +++ b/packages/backend-common/src/database/postgres.test.ts @@ -14,15 +14,26 @@ * limitations under the License. */ -import { ConfigReader } from '@backstage/config'; +import { Config, ConfigReader } from '@backstage/config'; import { - parsePgConnectionString, + createPgDatabaseClient, buildPgDatabaseConfig, - createPgDatabase, + getPgConnectionConfig, + parsePgConnectionString, } from './postgres'; describe('postgres', () => { - const createConfig = (connection: any) => + const createMockConnection = () => ({ + host: 'acme', + user: 'foo', + password: 'bar', + database: 'foodb', + }); + + const createMockConnectionString = () => + 'postgresql://foo:bar@acme:5432/foodb'; + + const createConfig = (connection: any): Config => ConfigReader.fromConfigs([ { context: '', @@ -35,86 +46,58 @@ describe('postgres', () => { describe(buildPgDatabaseConfig, () => { it('builds a postgres config', () => { - expect( - buildPgDatabaseConfig( - createConfig({ - host: 'acme', - user: 'foo', - password: 'bar', - port: '5432', - database: 'foodb', - }), - ), - ).toEqual({ + const mockConnection = createMockConnection(); + + expect(buildPgDatabaseConfig(createConfig(mockConnection))).toEqual({ client: 'pg', - connection: { - host: 'acme', - user: 'foo', - password: 'bar', - port: '5432', - database: 'foodb', - }, + connection: mockConnection, useNullAsDefault: true, }); }); it('builds a connection string config', () => { - expect( - buildPgDatabaseConfig( - createConfig('postgresql://foo:bar@acme:5432/foodb'), - ), - ).toEqual({ - client: 'pg', - connection: 'postgresql://foo:bar@acme:5432/foodb', - useNullAsDefault: true, - }); + const mockConnectionString = createMockConnectionString(); + + expect(buildPgDatabaseConfig(createConfig(mockConnectionString))).toEqual( + { + client: 'pg', + connection: mockConnectionString, + useNullAsDefault: true, + }, + ); }); it('overrides the database name', () => { + const mockConnection = createMockConnection(); + expect( - buildPgDatabaseConfig( - createConfig({ - host: 'somehost', - user: 'postgres', - password: 'pass', - database: 'foo', - }), - { connection: { database: 'foodb' } }, - ), + buildPgDatabaseConfig(createConfig(mockConnection), { + connection: { database: 'other_db' }, + }), ).toEqual({ client: 'pg', connection: { - host: 'somehost', - user: 'postgres', - password: 'pass', - database: 'foodb', + ...mockConnection, + database: 'other_db', }, useNullAsDefault: true, }); }); it('adds additional config settings', () => { + const mockConnection = createMockConnection(); + expect( - buildPgDatabaseConfig( - createConfig({ - host: 'somehost', - user: 'postgres', - password: 'pass', - database: 'foo', - }), - { - connection: { database: 'foodb' }, - pool: { min: 0, max: 7 }, - debug: true, - }, - ), + buildPgDatabaseConfig(createConfig(mockConnection), { + connection: { database: 'other_db' }, + pool: { min: 0, max: 7 }, + debug: true, + }), ).toEqual({ client: 'pg', connection: { - host: 'somehost', - user: 'postgres', - password: 'pass', - database: 'foodb', + ...mockConnection, + database: 'other_db', }, useNullAsDefault: true, pool: { min: 0, max: 7 }, @@ -123,37 +106,72 @@ describe('postgres', () => { }); it('overrides the database from connection string', () => { + const mockConnectionString = createMockConnectionString(); + const mockConnection = createMockConnection(); + expect( - buildPgDatabaseConfig( - createConfig('postgresql://postgres:pass@localhost:5432/dbname'), - { connection: { database: 'foodb' } }, - ), + buildPgDatabaseConfig(createConfig(mockConnectionString), { + connection: { database: 'other_db' }, + }), ).toEqual({ client: 'pg', connection: { - host: 'localhost', - user: 'postgres', - password: 'pass', + ...mockConnection, port: '5432', - database: 'foodb', + database: 'other_db', }, useNullAsDefault: true, }); }); }); - describe(createPgDatabase, () => { + describe(getPgConnectionConfig, () => { + it('returns the connection object back', () => { + const mockConnection = createMockConnection(); + const config = createConfig(mockConnection); + + expect(getPgConnectionConfig(config)).toEqual(mockConnection); + }); + + it('does not parse the connection string', () => { + const mockConnection = createMockConnection(); + const config = createConfig(mockConnection); + + expect(getPgConnectionConfig(config, true)).toEqual(mockConnection); + }); + + it('automatically parses the connection string', () => { + const mockConnection = createMockConnection(); + const mockConnectionString = createMockConnectionString(); + const config = createConfig(mockConnectionString); + + expect(getPgConnectionConfig(config)).toEqual({ + ...mockConnection, + port: '5432', + }); + }); + + it('parses the connection string', () => { + const mockConnection = createMockConnection(); + const mockConnectionString = createMockConnectionString(); + const config = createConfig(mockConnectionString); + + expect(getPgConnectionConfig(config, true)).toEqual({ + ...mockConnection, + port: '5432', + }); + }); + }); + + describe(createPgDatabaseClient, () => { it('creates a postgres knex instance', () => { expect( - createPgDatabase( + createPgDatabaseClient( createConfig({ - client: 'pg', - connection: { - host: 'acme', - user: 'foo', - password: 'bar', - database: 'foodb', - }, + host: 'acme', + user: 'foo', + password: 'bar', + database: 'foodb', }), ), ).toBeTruthy(); @@ -161,7 +179,7 @@ describe('postgres', () => { it('attempts to read an ssl cert', () => { expect(() => - createPgDatabase( + createPgDatabaseClient( createConfig( 'postgresql://postgres:pass@localhost:5432/dbname?sslrootcert=/path/to/file', ), diff --git a/packages/backend-common/src/database/postgres.ts b/packages/backend-common/src/database/postgres.ts index 09b6d0653b..03a74156c7 100644 --- a/packages/backend-common/src/database/postgres.ts +++ b/packages/backend-common/src/database/postgres.ts @@ -14,18 +14,18 @@ * limitations under the License. */ -import knex from 'knex'; -import { ConfigReader } from '@backstage/config'; +import knex, { PgConnectionConfig } from 'knex'; +import { Config } from '@backstage/config'; import { mergeDatabaseConfig } from './config'; /** - * Creates a knex sqlite3 database connection + * Creates a knex postgres database connection * * @param dbConfig The database config * @param overrides Additional options to merge with the config */ -export function createPgDatabase( - dbConfig: ConfigReader, +export function createPgDatabaseClient( + dbConfig: Config, overrides?: knex.Config, ) { const knexConfig = buildPgDatabaseConfig(dbConfig, overrides); @@ -40,26 +40,43 @@ export function createPgDatabase( * @param overrides Additional options to merge with the config */ export function buildPgDatabaseConfig( - dbConfig: ConfigReader, + dbConfig: Config, overrides?: knex.Config, ) { - const connection = dbConfig.get('connection') as any; - return mergeDatabaseConfig( dbConfig.get(), { - // Only parse the connection string when overrides are provided - connection: - overrides && - (typeof connection === 'string' || connection instanceof String) - ? parsePgConnectionString(connection as string) - : connection, + connection: getPgConnectionConfig(dbConfig, !!overrides), useNullAsDefault: true, }, overrides, ); } +/** + * Gets the postgres connection config + * + * @param dbConfig The database config + * @param parseConnectionString Flag to explictly control connection string parsing + */ +export function getPgConnectionConfig( + dbConfig: Config, + parseConnectionString?: boolean, +): PgConnectionConfig | string { + const connection = dbConfig.get('connection') as any; + const isConnectionString = + typeof connection === 'string' || connection instanceof String; + const autoParse = typeof parseConnectionString !== 'boolean'; + + const shouldParseConnectionString = autoParse + ? isConnectionString + : parseConnectionString && isConnectionString; + + return shouldParseConnectionString + ? parsePgConnectionString(connection as string) + : connection; +} + /** * Parses a connection string using pg-connection-string * diff --git a/packages/backend-common/src/database/sqlite3.test.ts b/packages/backend-common/src/database/sqlite3.test.ts index a25720b81c..dbff5ee354 100644 --- a/packages/backend-common/src/database/sqlite3.test.ts +++ b/packages/backend-common/src/database/sqlite3.test.ts @@ -15,7 +15,10 @@ */ import { ConfigReader } from '@backstage/config'; -import { buildSqlite3DatabaseConfig, createSqlite3Database } from './sqlite3'; +import { + buildSqliteDatabaseConfig, + createSqliteDatabaseClient, +} from './sqlite3'; describe('sqlite3', () => { const createConfig = (connection: any) => @@ -29,9 +32,9 @@ describe('sqlite3', () => { }, ]); - describe(buildSqlite3DatabaseConfig, () => { + describe(buildSqliteDatabaseConfig, () => { it('buidls a string connection', () => { - expect(buildSqlite3DatabaseConfig(createConfig(':memory:'))).toEqual({ + expect(buildSqliteDatabaseConfig(createConfig(':memory:'))).toEqual({ client: 'sqlite3', connection: ':memory:', useNullAsDefault: true, @@ -40,7 +43,7 @@ describe('sqlite3', () => { it('builds a filename connection', () => { expect( - buildSqlite3DatabaseConfig( + buildSqliteDatabaseConfig( createConfig({ filename: '/path/to/foo', }), @@ -56,7 +59,7 @@ describe('sqlite3', () => { it('replaces the connection with an override', () => { expect( - buildSqlite3DatabaseConfig(createConfig(':memory:'), { + buildSqliteDatabaseConfig(createConfig(':memory:'), { connection: { filename: '/path/to/foo' }, }), ).toEqual({ @@ -69,10 +72,10 @@ describe('sqlite3', () => { }); }); - describe(createSqlite3Database, () => { + describe(createSqliteDatabaseClient, () => { it('creates an in memory knex instance', () => { expect( - createSqlite3Database( + createSqliteDatabaseClient( createConfig({ client: 'sqlite3', connection: ':memory:', diff --git a/packages/backend-common/src/database/sqlite3.ts b/packages/backend-common/src/database/sqlite3.ts index 6249394d05..7bdc4380a1 100644 --- a/packages/backend-common/src/database/sqlite3.ts +++ b/packages/backend-common/src/database/sqlite3.ts @@ -15,7 +15,7 @@ */ import knex from 'knex'; -import { ConfigReader } from '@backstage/config'; +import { Config } from '@backstage/config'; import { mergeDatabaseConfig } from './config'; /** @@ -24,11 +24,11 @@ import { mergeDatabaseConfig } from './config'; * @param dbConfig The database config * @param overrides Additional options to merge with the config */ -export function createSqlite3Database( - dbConfig: ConfigReader, +export function createSqliteDatabaseClient( + dbConfig: Config, overrides?: knex.Config, ) { - const knexConfig = buildSqlite3DatabaseConfig(dbConfig, overrides); + const knexConfig = buildSqliteDatabaseConfig(dbConfig, overrides); const database = knex(knexConfig); database.client.pool.on('createSuccess', (_eventId: any, resource: any) => { @@ -44,8 +44,8 @@ export function createSqlite3Database( * @param dbConfig The database config * @param overrides Additional options to merge with the config */ -export function buildSqlite3DatabaseConfig( - dbConfig: ConfigReader, +export function buildSqliteDatabaseConfig( + dbConfig: Config, overrides?: knex.Config, ) { return mergeDatabaseConfig( diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 6df593182d..0423047cf7 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -23,7 +23,7 @@ */ import { - createDatabase, + createDatabaseClient, createServiceBuilder, loadBackendConfig, getRootLogger, @@ -48,11 +48,14 @@ function makeCreateEnv(loadedConfigs: AppConfig[]) { return (plugin: string): PluginEnvironment => { const logger = getRootLogger().child({ type: 'plugin', plugin }); - const database = createDatabase(config.getConfig('backend.database'), { - connection: { - database: `backstage_plugin_${plugin}`, + const database = createDatabaseClient( + config.getConfig('backend.database'), + { + connection: { + database: `backstage_plugin_${plugin}`, + }, }, - }); + ); return { logger, database, config }; }; } diff --git a/packages/create-app/templates/default-app/packages/backend/src/index.ts b/packages/create-app/templates/default-app/packages/backend/src/index.ts index d6fba2478a..6a014727e9 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/index.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/index.ts @@ -7,7 +7,7 @@ */ import { - createDatabase, + createDatabaseClient, createServiceBuilder, loadBackendConfig, getRootLogger, @@ -27,11 +27,14 @@ function makeCreateEnv(loadedConfigs: AppConfig[]) { return (plugin: string): PluginEnvironment => { const logger = getRootLogger().child({ type: 'plugin', plugin }); - const database = createDatabase(config.getConfig('backend.database'), { - connection: { - database: `backstage_plugin_${plugin}`, + const database = createDatabaseClient( + config.getConfig('backend.database'), + { + connection: { + database: `backstage_plugin_${plugin}`, + }, }, - }); + ); return { logger, database, config }; }; }