From e277c5dfa587134672ac3ba81351dee03deb8022 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Thu, 1 Oct 2020 17:38:12 +0800 Subject: [PATCH 1/9] Expose types to handle Database management This implements several types to own and manage databases on a database server. The current SimpleDatabase* classes preserve the present behaviour; future implementations can segregate databases to be owned by different roles. --- .../src/database/SingleDatabase.ts | 96 +++++++++++++++++++ packages/backend-common/src/database/index.ts | 2 + .../backend-common/src/database/postgres.ts | 3 +- packages/backend-common/src/database/types.ts | 59 ++++++++++++ packages/backend/src/index.ts | 32 +++---- packages/backend/src/plugins/catalog.ts | 5 +- packages/backend/src/types.ts | 9 +- .../default-app/packages/backend/src/index.ts | 26 +++-- plugins/auth-backend/src/service/router.ts | 8 +- .../src/service/standaloneServer.ts | 6 +- 10 files changed, 203 insertions(+), 43 deletions(-) create mode 100644 packages/backend-common/src/database/SingleDatabase.ts create mode 100644 packages/backend-common/src/database/types.ts diff --git a/packages/backend-common/src/database/SingleDatabase.ts b/packages/backend-common/src/database/SingleDatabase.ts new file mode 100644 index 0000000000..b599232ee8 --- /dev/null +++ b/packages/backend-common/src/database/SingleDatabase.ts @@ -0,0 +1,96 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import Knex from 'knex'; +import { Config } from '@backstage/config'; +import { createDatabaseClient, ensureDatabaseExists } from './connection'; +import { + DatabaseConfiguration, + PluginDatabaseFactory, + PluginDatabaseManager, +} from './types'; + +export class SingleDatabaseConfiguration implements DatabaseConfiguration { + private readonly config: Config; + + constructor(config: Config) { + this.config = config; + } + + getDatabaseConfig(_pluginId: string): Config { + return this.config; + } + + getAdminConfig(): Config { + return this.config; + } +} + +export class SingleDatabaseManager implements PluginDatabaseManager { + private readonly config: DatabaseConfiguration; + + constructor(config: DatabaseConfiguration) { + this.config = config; + } + + getDatabaseFactory(pluginId: string): PluginDatabaseFactory { + // eslint-disable-next-line no-use-before-define + return new SinglePluginDatabaseFactory(this, pluginId); + } + + async getDatabase(pluginId: string, suffix?: string): Promise { + const config = this.config.getDatabaseConfig(pluginId); + const overrides = SingleDatabaseManager.getDatabaseOverrides( + pluginId, + suffix, + ); + const overrideConfig = overrides.connection as Knex.ConnectionConfig; + await this.ensureDatabase(overrideConfig.database); + + return createDatabaseClient(config, overrides); + } + + private static getDatabaseOverrides( + pluginId: string, + suffix?: string, + ): Knex.Config { + const dbSuffix = suffix ? `_${suffix}` : ''; + return { + connection: { + database: `backstage_plugin_${pluginId}${dbSuffix}`, + }, + }; + } + + private async ensureDatabase(database: string) { + const config = this.config.getAdminConfig(); + await ensureDatabaseExists(config, database); + } +} + +export class SinglePluginDatabaseFactory implements PluginDatabaseFactory { + private manager: SingleDatabaseManager; + private readonly pluginId: string; + + constructor(manager: SingleDatabaseManager, pluginId: string) { + this.manager = manager; + this.pluginId = pluginId; + } + + getDatabase(database?: string): Promise { + return this.manager.getDatabase(this.pluginId, database); + } +} diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index 38d3d6224b..c667edc659 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -15,3 +15,5 @@ */ export * from './connection'; +export * from './types'; +export * from './SingleDatabase'; diff --git a/packages/backend-common/src/database/postgres.ts b/packages/backend-common/src/database/postgres.ts index 82e971f081..9972377de8 100644 --- a/packages/backend-common/src/database/postgres.ts +++ b/packages/backend-common/src/database/postgres.ts @@ -15,6 +15,7 @@ */ import knex, { PgConnectionConfig } from 'knex'; +import { cloneDeep } from 'lodash'; import { Config } from '@backstage/config'; import { mergeDatabaseConfig } from './config'; @@ -44,7 +45,7 @@ export function buildPgDatabaseConfig( overrides?: knex.Config, ) { return mergeDatabaseConfig( - dbConfig.get(), + cloneDeep(dbConfig.get()), { connection: getPgConnectionConfig(dbConfig, !!overrides), useNullAsDefault: true, diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts new file mode 100644 index 0000000000..00ee82f22a --- /dev/null +++ b/packages/backend-common/src/database/types.ts @@ -0,0 +1,59 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import knex from 'knex'; +import { Config } from '@backstage/config'; + +/** + * DatabaseConfiguration exposes the Application configuration to PluginDatabaseManager instances. + */ +export interface DatabaseConfiguration { + /** + * Returns the database configuration for the given Plugin. + */ + getDatabaseConfig(pluginId: string): Config; + + /** + * Returns the database configuration for administrating the database server. + */ + getAdminConfig(): Config; +} + +/** + * The PluginDatabaseManager produces PluginDatabaseFactory objects for plugins to consume and store their data. + */ +export type PluginDatabaseManager = { + /** + * getDatabaseFactory returns the database factory object for plugins to obtain database connections. + */ + getDatabaseFactory(pluginId: string): PluginDatabaseFactory; +}; + +/** + * The PluginDatabaseFactory is used to provide a mechanism for backend plugins to obtain database connections for + * itself. + * + * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from database integration. + */ +export type PluginDatabaseFactory = { + /** + * Returns a database connection. Plugins can omit the `database` parameter to get the default plugin database, or + * provide an identifier that will be used to identify a separate database from the default. + * + * Plugins completely own and manage the state of the database (including schema/migrations/data.) + */ + getDatabase(database?: String): Promise; +}; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 959af71427..192ceb95a2 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -24,17 +24,17 @@ import Router from 'express-promise-router'; import { - ensureDatabaseExists, - createDatabaseClient, createServiceBuilder, loadBackendConfig, getRootLogger, useHotMemoize, notFoundHandler, + SingleDatabaseConfiguration, + SingleDatabaseManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; -import { ConfigReader, AppConfig } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; import healthcheck from './plugins/healthcheck'; import auth from './plugins/auth'; import catalog from './plugins/catalog'; @@ -48,37 +48,29 @@ import graphql from './plugins/graphql'; import app from './plugins/app'; import { PluginEnvironment } from './types'; -function makeCreateEnv(loadedConfigs: AppConfig[]) { - const config = ConfigReader.fromConfigs(loadedConfigs); +function makeCreateEnv(config: ConfigReader) { const root = getRootLogger(); const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); root.info(`Created UrlReader ${reader}`); + const databaseConfig = new SingleDatabaseConfiguration( + config.getConfig('backend.database'), + ); + const databaseManager = new SingleDatabaseManager(databaseConfig); + return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const database = createDatabaseClient( - config.getConfig('backend.database'), - { - connection: { - database: `backstage_plugin_${plugin}`, - }, - }, - ); - return { logger, database, config, reader, discovery }; + const databaseFactory = databaseManager.getDatabaseFactory(plugin); + return { logger, database: databaseFactory, config, reader, discovery }; }; } async function main() { const configs = await loadBackendConfig(); const configReader = ConfigReader.fromConfigs(configs); - const createEnv = makeCreateEnv(configs); - await ensureDatabaseExists( - configReader.getConfig('backend.database'), - 'backstage_plugin_catalog', - 'backstage_plugin_auth', - ); + const createEnv = makeCreateEnv(configReader); const healthcheckEnv = useHotMemoize(module, () => createEnv('healthcheck')); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index c1d390f8cc..9ab2203ca2 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -34,7 +34,10 @@ export default async function createPlugin({ }: PluginEnvironment) { const locationReader = new LocationReaders({ logger, reader, config }); - const db = await DatabaseManager.createDatabase(database, { logger }); + const db = await DatabaseManager.createDatabase( + await database.getDatabase(), + { logger }, + ); const entitiesCatalog = new DatabaseEntitiesCatalog(db); const locationsCatalog = new DatabaseLocationsCatalog(db); const higherOrderOperation = new HigherOrderOperations( diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 9257fcc9cf..98797eddf9 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -14,14 +14,17 @@ * limitations under the License. */ -import Knex from 'knex'; import { Logger } from 'winston'; import { Config } from '@backstage/config'; -import { PluginEndpointDiscovery, UrlReader } from '@backstage/backend-common'; +import { + PluginDatabaseFactory, + PluginEndpointDiscovery, + UrlReader +} from '@backstage/backend-common'; export type PluginEnvironment = { logger: Logger; - database: Knex; + database: PluginDatabaseFactory; config: Config; reader: UrlReader; discovery: PluginEndpointDiscovery; 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 684db8aab8..437ef5afcd 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 @@ -9,16 +9,17 @@ import Router from 'express-promise-router'; import { ensureDatabaseExists, - createDatabaseClient, createServiceBuilder, loadBackendConfig, getRootLogger, useHotMemoize, notFoundHandler, + SingleDatabaseConfiguration, + SingleDatabaseManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; -import { ConfigReader, AppConfig } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; import auth from './plugins/auth'; import catalog from './plugins/catalog'; import scaffolder from './plugins/scaffolder'; @@ -26,32 +27,29 @@ import proxy from './plugins/proxy'; import techdocs from './plugins/techdocs'; import { PluginEnvironment } from './types'; -function makeCreateEnv(loadedConfigs: AppConfig[]) { - const config = ConfigReader.fromConfigs(loadedConfigs); +function makeCreateEnv(config: ConfigReader) { const root = getRootLogger(); const reader = UrlReaders.default({ logger: root, config }); const discovery = SingleHostDiscovery.fromConfig(config); root.info(`Created UrlReader ${reader}`); + const databaseConfig = new SingleDatabaseConfiguration( + config.getConfig('backend.database'), + ) + const databaseManager = new SingleDatabaseManager(databaseConfig); + return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const database = createDatabaseClient( - config.getConfig('backend.database'), - { - connection: { - database: `backstage_plugin_${plugin}`, - }, - }, - ); - return { logger, database, config, reader, discovery }; + const databaseFactory = databaseManager.getDatabaseFactory(plugin); + return { logger, database: databaseFactory, config, reader, discovery }; }; } async function main() { const configs = await loadBackendConfig(); const configReader = ConfigReader.fromConfigs(configs); - const createEnv = makeCreateEnv(configs); + const createEnv = makeCreateEnv(configReader); await ensureDatabaseExists( configReader.getConfig('backend.database'), 'backstage_plugin_catalog', diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index bcd1fc1794..d514586ec9 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -17,7 +17,6 @@ import express from 'express'; import Router from 'express-promise-router'; import cookieParser from 'cookie-parser'; -import Knex from 'knex'; import { Logger } from 'winston'; import { createAuthProvider } from '../providers'; import { Config } from '@backstage/config'; @@ -25,11 +24,12 @@ import { DatabaseKeyStore, TokenFactory, createOidcRouter } from '../identity'; import { NotFoundError, PluginEndpointDiscovery, + PluginDatabaseFactory, } from '@backstage/backend-common'; export interface RouterOptions { logger: Logger; - database: Knex; + database: PluginDatabaseFactory; config: Config; discovery: PluginEndpointDiscovery; } @@ -47,7 +47,9 @@ export async function createRouter({ const keyDurationSeconds = 3600; - const keyStore = await DatabaseKeyStore.create({ database }); + const keyStore = await DatabaseKeyStore.create({ + database: await database.getDatabase(), + }); const tokenIssuer = new TokenFactory({ issuer: authUrl, keyStore, diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index 71771340a5..e51b959188 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -53,7 +53,11 @@ export async function startStandaloneServer( const router = await createRouter({ logger, config, - database, + database: { + async getDatabase(_database?: string) { + return database; + }, + }, discovery, }); From 8c78b4e9dd78b7525372fe9584fa7e8db5e91b99 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Fri, 2 Oct 2020 12:35:40 +0800 Subject: [PATCH 2/9] Fix review comments - Combine the various exported types; the only API surface is now the `PluginDatabaseClientFactory` which is a free function. - Rename to the SingleConnectionManager class. - Refactor the PluginEnvironment type to use databaseClientFactory for clarity. - Fix mergeDatabaseConfig to not override the input dictionary. --- ...{SingleDatabase.ts => SingleConnection.ts} | 56 +++++++------------ .../backend-common/src/database/config.ts | 2 +- packages/backend-common/src/database/index.ts | 2 +- .../backend-common/src/database/postgres.ts | 3 +- packages/backend-common/src/database/types.ts | 44 ++------------- packages/backend/src/index.ts | 10 ++-- packages/backend/src/plugins/auth.ts | 9 ++- packages/backend/src/plugins/catalog.ts | 4 +- packages/backend/src/types.ts | 4 +- .../default-app/packages/backend/src/index.ts | 26 ++++----- .../packages/backend/src/plugins/auth.ts | 9 ++- .../packages/backend/src/plugins/catalog.ts | 7 ++- .../default-app/packages/backend/src/types.ts | 9 ++- plugins/auth-backend/src/service/router.ts | 6 +- .../src/service/standaloneServer.ts | 6 +- 15 files changed, 78 insertions(+), 119 deletions(-) rename packages/backend-common/src/database/{SingleDatabase.ts => SingleConnection.ts} (53%) diff --git a/packages/backend-common/src/database/SingleDatabase.ts b/packages/backend-common/src/database/SingleConnection.ts similarity index 53% rename from packages/backend-common/src/database/SingleDatabase.ts rename to packages/backend-common/src/database/SingleConnection.ts index b599232ee8..8ed93166cf 100644 --- a/packages/backend-common/src/database/SingleDatabase.ts +++ b/packages/backend-common/src/database/SingleConnection.ts @@ -17,43 +17,39 @@ import Knex from 'knex'; import { Config } from '@backstage/config'; import { createDatabaseClient, ensureDatabaseExists } from './connection'; -import { - DatabaseConfiguration, - PluginDatabaseFactory, - PluginDatabaseManager, -} from './types'; +import { PluginDatabaseClientFactory } from './types'; -export class SingleDatabaseConfiguration implements DatabaseConfiguration { +/** + * Implements a Database Manager which will automatically create new databases for plugins when requested. All + * requested databases are created with the credentials provided. + */ +export class SingleConnectionManager { private readonly config: Config; constructor(config: Config) { this.config = config; } - getDatabaseConfig(_pluginId: string): Config { + private getDatabaseConfig(): Config { return this.config; } - getAdminConfig(): Config { + private getAdminConfig(): Config { return this.config; } -} -export class SingleDatabaseManager implements PluginDatabaseManager { - private readonly config: DatabaseConfiguration; - - constructor(config: DatabaseConfiguration) { - this.config = config; + /** + * Generates a PluginDatabaseClientFactory for consumption by plugins. + */ + getDatabaseClientFactory(pluginId: string): PluginDatabaseClientFactory { + return (database?: string): Promise => { + return this.getDatabase(pluginId, database); + }; } - getDatabaseFactory(pluginId: string): PluginDatabaseFactory { - // eslint-disable-next-line no-use-before-define - return new SinglePluginDatabaseFactory(this, pluginId); - } - - async getDatabase(pluginId: string, suffix?: string): Promise { - const config = this.config.getDatabaseConfig(pluginId); - const overrides = SingleDatabaseManager.getDatabaseOverrides( + private async getDatabase(pluginId: string, suffix?: string): Promise { + const config = this.getDatabaseConfig(); + const overrides = SingleConnectionManager.getDatabaseOverrides( pluginId, suffix, ); @@ -76,21 +72,7 @@ export class SingleDatabaseManager implements PluginDatabaseManager { } private async ensureDatabase(database: string) { - const config = this.config.getAdminConfig(); + const config = this.getAdminConfig(); await ensureDatabaseExists(config, database); } } - -export class SinglePluginDatabaseFactory implements PluginDatabaseFactory { - private manager: SingleDatabaseManager; - private readonly pluginId: string; - - constructor(manager: SingleDatabaseManager, pluginId: string) { - this.manager = manager; - this.pluginId = pluginId; - } - - getDatabase(database?: string): Promise { - return this.manager.getDatabase(this.pluginId, database); - } -} diff --git a/packages/backend-common/src/database/config.ts b/packages/backend-common/src/database/config.ts index 80abc06aa2..a16cf6cbee 100644 --- a/packages/backend-common/src/database/config.ts +++ b/packages/backend-common/src/database/config.ts @@ -23,5 +23,5 @@ import { merge } from 'lodash'; * @param overrides Any additional overrides */ export function mergeDatabaseConfig(config: any, ...overrides: any[]) { - return merge(config, ...overrides); + return merge({}, config, ...overrides); } diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index c667edc659..c81153aa62 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -16,4 +16,4 @@ export * from './connection'; export * from './types'; -export * from './SingleDatabase'; +export * from './SingleConnection'; diff --git a/packages/backend-common/src/database/postgres.ts b/packages/backend-common/src/database/postgres.ts index 9972377de8..82e971f081 100644 --- a/packages/backend-common/src/database/postgres.ts +++ b/packages/backend-common/src/database/postgres.ts @@ -15,7 +15,6 @@ */ import knex, { PgConnectionConfig } from 'knex'; -import { cloneDeep } from 'lodash'; import { Config } from '@backstage/config'; import { mergeDatabaseConfig } from './config'; @@ -45,7 +44,7 @@ export function buildPgDatabaseConfig( overrides?: knex.Config, ) { return mergeDatabaseConfig( - cloneDeep(dbConfig.get()), + dbConfig.get(), { connection: getPgConnectionConfig(dbConfig, !!overrides), useNullAsDefault: true, diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 00ee82f22a..b616ca848a 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -15,45 +15,13 @@ */ import knex from 'knex'; -import { Config } from '@backstage/config'; /** - * DatabaseConfiguration exposes the Application configuration to PluginDatabaseManager instances. - */ -export interface DatabaseConfiguration { - /** - * Returns the database configuration for the given Plugin. - */ - getDatabaseConfig(pluginId: string): Config; - - /** - * Returns the database configuration for administrating the database server. - */ - getAdminConfig(): Config; -} - -/** - * The PluginDatabaseManager produces PluginDatabaseFactory objects for plugins to consume and store their data. - */ -export type PluginDatabaseManager = { - /** - * getDatabaseFactory returns the database factory object for plugins to obtain database connections. - */ - getDatabaseFactory(pluginId: string): PluginDatabaseFactory; -}; - -/** - * The PluginDatabaseFactory is used to provide a mechanism for backend plugins to obtain database connections for - * itself. + * The PluginDatabaseClientFactory is used to provide a mechanism for backend plugins to obtain database connections + * for itself. * - * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from database integration. + * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from + * database integration. Plugins can omit the `database` parameter to get the default plugin database, or + * provide an identifier that will be used to identify a separate database from the default. */ -export type PluginDatabaseFactory = { - /** - * Returns a database connection. Plugins can omit the `database` parameter to get the default plugin database, or - * provide an identifier that will be used to identify a separate database from the default. - * - * Plugins completely own and manage the state of the database (including schema/migrations/data.) - */ - getDatabase(database?: String): Promise; -}; +export type PluginDatabaseClientFactory = (database?: string) => Promise; diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index 192ceb95a2..a3b700de46 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -29,8 +29,7 @@ import { getRootLogger, useHotMemoize, notFoundHandler, - SingleDatabaseConfiguration, - SingleDatabaseManager, + SingleConnectionManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; @@ -55,15 +54,14 @@ function makeCreateEnv(config: ConfigReader) { root.info(`Created UrlReader ${reader}`); - const databaseConfig = new SingleDatabaseConfiguration( + const databaseManager = new SingleConnectionManager( config.getConfig('backend.database'), ); - const databaseManager = new SingleDatabaseManager(databaseConfig); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const databaseFactory = databaseManager.getDatabaseFactory(plugin); - return { logger, database: databaseFactory, config, reader, discovery }; + const databaseFactory = databaseManager.getDatabaseClientFactory(plugin); + return { logger, databaseClientFactory: databaseFactory, config, reader, discovery }; }; } diff --git a/packages/backend/src/plugins/auth.ts b/packages/backend/src/plugins/auth.ts index 913c8b783d..1e933fb398 100644 --- a/packages/backend/src/plugins/auth.ts +++ b/packages/backend/src/plugins/auth.ts @@ -19,9 +19,14 @@ import { PluginEnvironment } from '../types'; export default async function createPlugin({ logger, - database, + databaseClientFactory, config, discovery, }: PluginEnvironment) { - return await createRouter({ logger, config, database, discovery }); + return await createRouter({ + logger, + config, + database: databaseClientFactory, + discovery, + }); } diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index 9ab2203ca2..04e3304f0c 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -30,12 +30,12 @@ export default async function createPlugin({ logger, config, reader, - database, + databaseClientFactory, }: PluginEnvironment) { const locationReader = new LocationReaders({ logger, reader, config }); const db = await DatabaseManager.createDatabase( - await database.getDatabase(), + await databaseClientFactory(), { logger }, ); const entitiesCatalog = new DatabaseEntitiesCatalog(db); diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index 98797eddf9..b032917f1d 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -17,14 +17,14 @@ import { Logger } from 'winston'; import { Config } from '@backstage/config'; import { - PluginDatabaseFactory, + PluginDatabaseClientFactory, PluginEndpointDiscovery, UrlReader } from '@backstage/backend-common'; export type PluginEnvironment = { logger: Logger; - database: PluginDatabaseFactory; + databaseClientFactory: PluginDatabaseClientFactory; config: Config; reader: UrlReader; discovery: PluginEndpointDiscovery; 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 437ef5afcd..b35f21c5e9 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 @@ -8,14 +8,12 @@ import Router from 'express-promise-router'; import { - ensureDatabaseExists, createServiceBuilder, loadBackendConfig, getRootLogger, useHotMemoize, notFoundHandler, - SingleDatabaseConfiguration, - SingleDatabaseManager, + SingleConnectionManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; @@ -34,15 +32,20 @@ function makeCreateEnv(config: ConfigReader) { root.info(`Created UrlReader ${reader}`); - const databaseConfig = new SingleDatabaseConfiguration( - config.getConfig('backend.database'), - ) - const databaseManager = new SingleDatabaseManager(databaseConfig); + const databaseManager = new SingleConnectionManager( + config.getConfig('backend.database') + ); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const databaseFactory = databaseManager.getDatabaseFactory(plugin); - return { logger, database: databaseFactory, config, reader, discovery }; + const databaseFactory = databaseManager.getDatabaseClientFactory(plugin); + return { + logger, + databaseClientFactory: databaseFactory, + config, + reader, + discovery + }; }; } @@ -50,11 +53,6 @@ async function main() { const configs = await loadBackendConfig(); const configReader = ConfigReader.fromConfigs(configs); const createEnv = makeCreateEnv(configReader); - await ensureDatabaseExists( - configReader.getConfig('backend.database'), - 'backstage_plugin_catalog', - 'backstage_plugin_auth', - ); const catalogEnv = useHotMemoize(module, () => createEnv('catalog')); const scaffolderEnv = useHotMemoize(module, () => createEnv('scaffolder')); diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts index fe19855d5d..0f8cc21dec 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts @@ -3,9 +3,14 @@ import { PluginEnvironment } from '../types'; export default async function createPlugin({ logger, - database, + databaseClientFactory, config, discovery, }: PluginEnvironment) { - return await createRouter({ logger, config, database, discovery }); + return await createRouter({ + logger, + config, + database: databaseClientFactory, + discovery + }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts index 345ac4c23d..a0ff556ed0 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts @@ -14,11 +14,14 @@ export default async function createPlugin({ logger, config, reader, - database, + databaseClientFactory, }: PluginEnvironment) { const locationReader = new LocationReaders({ logger, reader, config }); - const db = await DatabaseManager.createDatabase(database, { logger }); + const db = await DatabaseManager.createDatabase( + await databaseClientFactory(), + { logger }, + ); const entitiesCatalog = new DatabaseEntitiesCatalog(db); const locationsCatalog = new DatabaseLocationsCatalog(db); const higherOrderOperation = new HigherOrderOperations( diff --git a/packages/create-app/templates/default-app/packages/backend/src/types.ts b/packages/create-app/templates/default-app/packages/backend/src/types.ts index aa003f63a5..c2b7b5fa8e 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/types.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/types.ts @@ -1,11 +1,14 @@ -import Knex from 'knex'; import { Logger } from 'winston'; import { Config } from '@backstage/config'; -import { PluginEndpointDiscovery, UrlReader } from '@backstage/backend-common'; +import { + PluginDatabaseClientFactory, + PluginEndpointDiscovery, + UrlReader +} from '@backstage/backend-common'; export type PluginEnvironment = { logger: Logger; - database: Knex; + databaseClientFactory: PluginDatabaseClientFactory; config: Config; reader: UrlReader discovery: PluginEndpointDiscovery; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index d514586ec9..f21b2d58fc 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -24,12 +24,12 @@ import { DatabaseKeyStore, TokenFactory, createOidcRouter } from '../identity'; import { NotFoundError, PluginEndpointDiscovery, - PluginDatabaseFactory, + PluginDatabaseClientFactory, } from '@backstage/backend-common'; export interface RouterOptions { logger: Logger; - database: PluginDatabaseFactory; + database: PluginDatabaseClientFactory; config: Config; discovery: PluginEndpointDiscovery; } @@ -48,7 +48,7 @@ export async function createRouter({ const keyDurationSeconds = 3600; const keyStore = await DatabaseKeyStore.create({ - database: await database.getDatabase(), + database: await database(), }); const tokenIssuer = new TokenFactory({ issuer: authUrl, diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index e51b959188..b5c15b3704 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -53,10 +53,8 @@ export async function startStandaloneServer( const router = await createRouter({ logger, config, - database: { - async getDatabase(_database?: string) { - return database; - }, + database: async () => { + return database; }, discovery, }); From b083c97f8459ab9275091e24f26646df4de4f830 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Mon, 5 Oct 2020 10:03:10 +0800 Subject: [PATCH 3/9] Replace PluginDatabaseClientFactory with PluginDatabaseManager --- CHANGELOG.md | 1 + .../src/database/SingleConnection.ts | 36 +++++++++++++------ packages/backend-common/src/database/types.ts | 20 +++++++---- packages/backend/src/index.ts | 10 +++--- packages/backend/src/plugins/auth.ts | 9 ++--- packages/backend/src/plugins/catalog.ts | 9 +++-- packages/backend/src/types.ts | 6 ++-- .../default-app/packages/backend/src/index.ts | 16 +++------ .../packages/backend/src/plugins/auth.ts | 9 ++--- .../packages/backend/src/plugins/catalog.ts | 5 ++- .../default-app/packages/backend/src/types.ts | 6 ++-- plugins/auth-backend/src/service/router.ts | 6 ++-- .../src/service/standaloneServer.ts | 6 ++-- 13 files changed, 71 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1860a4de6f..c5229a6794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ If you encounter issues while upgrading to a newer version, don't hesitate to re - The default mount point for backend plugins have been changed to `/api`. These changes are done in the backend package itself, so it is recommended that you sync up existing backend packages with this new pattern. [#2562](https://github.com/spotify/backstage/pull/2562) - A service discovery mechanism for backend plugins has been added, and is now a requirement for several backend plugins. See [packages/backend/src/index.ts](./packages/backend/src/index.ts) for how to set it up using `SingleHostDiscovery` from `@backstage/backend-common`. Note that the default base path for plugins is set to `/api` to that change, but it can be set to use the old behavior via the `basePath` option. [#2600](https://github.com/spotify/backstage/pull/2600) +- A plugin database manager has been created, and plugins can now accept that interface as an argument during initialisation. Notably, the `auth` plugin has a [`createRouter` signature change](./plugins/auth-backend/src/service/router.ts). See [packages/backend/src/index.ts](./packages/backend/src/index.ts) on how to set it up. [#2697](https://github.com/spotify/backstage/pull/2697) ### @backstage/auth-backend diff --git a/packages/backend-common/src/database/SingleConnection.ts b/packages/backend-common/src/database/SingleConnection.ts index 8ed93166cf..c34c3a8310 100644 --- a/packages/backend-common/src/database/SingleConnection.ts +++ b/packages/backend-common/src/database/SingleConnection.ts @@ -17,16 +17,28 @@ import Knex from 'knex'; import { Config } from '@backstage/config'; import { createDatabaseClient, ensureDatabaseExists } from './connection'; -import { PluginDatabaseClientFactory } from './types'; +import { PluginDatabaseManager } from './types'; /** - * Implements a Database Manager which will automatically create new databases for plugins when requested. All - * requested databases are created with the credentials provided. + * Implements a Database Manager which will automatically create new databases + * for plugins when requested. All requested databases are created with the + * credentials provided. */ -export class SingleConnectionManager { +export class SingleConnectionDatabaseManager { private readonly config: Config; - constructor(config: Config) { + /** + * Creates a new SingleConnectionDatabaseManager instance by reading from the `backend` + * config section, specifically the `.database` key for discovering the management + * database configuration. + */ + static fromConfig(config: Config): SingleConnectionDatabaseManager { + return new SingleConnectionDatabaseManager( + config.getConfig('backend.database'), + ); + } + + private constructor(config: Config) { this.config = config; } @@ -39,17 +51,21 @@ export class SingleConnectionManager { } /** - * Generates a PluginDatabaseClientFactory for consumption by plugins. + * Generates a PluginDatabaseManager for consumption by plugins. */ - getDatabaseClientFactory(pluginId: string): PluginDatabaseClientFactory { - return (database?: string): Promise => { - return this.getDatabase(pluginId, database); + forPlugin(pluginId: string): PluginDatabaseManager { + const _this = this; + + return { + getClient(database?: string): Promise { + return _this.getDatabase(pluginId, database); + }, }; } private async getDatabase(pluginId: string, suffix?: string): Promise { const config = this.getDatabaseConfig(); - const overrides = SingleConnectionManager.getDatabaseOverrides( + const overrides = SingleConnectionDatabaseManager.getDatabaseOverrides( pluginId, suffix, ); diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index b616ca848a..915a12a24e 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -17,11 +17,17 @@ import knex from 'knex'; /** - * The PluginDatabaseClientFactory is used to provide a mechanism for backend plugins to obtain database connections - * for itself. - * - * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from - * database integration. Plugins can omit the `database` parameter to get the default plugin database, or - * provide an identifier that will be used to identify a separate database from the default. + * The PluginDatabaseManager manages access to databases that Plugins get. */ -export type PluginDatabaseClientFactory = (database?: string) => Promise; +export interface PluginDatabaseManager { + /** + * getClient provides backend plugins database connections for itself. + * + * The purpose of this method is to allow plugins to get isolated data + * stores so that plugins are discouraged from database integration. Plugins + * can omit the `database` parameter to get the default plugin database, or + * provide an identifier that will be used to identify a separate database + * from the default. + */ + getClient(database?: string): Promise; +} diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts index a3b700de46..18fd44f089 100644 --- a/packages/backend/src/index.ts +++ b/packages/backend/src/index.ts @@ -29,7 +29,7 @@ import { getRootLogger, useHotMemoize, notFoundHandler, - SingleConnectionManager, + SingleConnectionDatabaseManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; @@ -54,14 +54,12 @@ function makeCreateEnv(config: ConfigReader) { root.info(`Created UrlReader ${reader}`); - const databaseManager = new SingleConnectionManager( - config.getConfig('backend.database'), - ); + const databaseManager = SingleConnectionDatabaseManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const databaseFactory = databaseManager.getDatabaseClientFactory(plugin); - return { logger, databaseClientFactory: databaseFactory, config, reader, discovery }; + const database = databaseManager.forPlugin(plugin); + return { logger, database, config, reader, discovery }; }; } diff --git a/packages/backend/src/plugins/auth.ts b/packages/backend/src/plugins/auth.ts index 1e933fb398..913c8b783d 100644 --- a/packages/backend/src/plugins/auth.ts +++ b/packages/backend/src/plugins/auth.ts @@ -19,14 +19,9 @@ import { PluginEnvironment } from '../types'; export default async function createPlugin({ logger, - databaseClientFactory, + database, config, discovery, }: PluginEnvironment) { - return await createRouter({ - logger, - config, - database: databaseClientFactory, - discovery, - }); + return await createRouter({ logger, config, database, discovery }); } diff --git a/packages/backend/src/plugins/catalog.ts b/packages/backend/src/plugins/catalog.ts index 04e3304f0c..b8e99e6398 100644 --- a/packages/backend/src/plugins/catalog.ts +++ b/packages/backend/src/plugins/catalog.ts @@ -30,14 +30,13 @@ export default async function createPlugin({ logger, config, reader, - databaseClientFactory, + database, }: PluginEnvironment) { const locationReader = new LocationReaders({ logger, reader, config }); - const db = await DatabaseManager.createDatabase( - await databaseClientFactory(), - { logger }, - ); + const db = await DatabaseManager.createDatabase(await database.getClient(), { + logger, + }); const entitiesCatalog = new DatabaseEntitiesCatalog(db); const locationsCatalog = new DatabaseLocationsCatalog(db); const higherOrderOperation = new HigherOrderOperations( diff --git a/packages/backend/src/types.ts b/packages/backend/src/types.ts index b032917f1d..f63b9dd780 100644 --- a/packages/backend/src/types.ts +++ b/packages/backend/src/types.ts @@ -17,14 +17,14 @@ import { Logger } from 'winston'; import { Config } from '@backstage/config'; import { - PluginDatabaseClientFactory, + PluginDatabaseManager, PluginEndpointDiscovery, - UrlReader + UrlReader, } from '@backstage/backend-common'; export type PluginEnvironment = { logger: Logger; - databaseClientFactory: PluginDatabaseClientFactory; + database: PluginDatabaseManager; config: Config; reader: UrlReader; discovery: PluginEndpointDiscovery; 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 b35f21c5e9..d58ebe4e42 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 @@ -13,7 +13,7 @@ import { getRootLogger, useHotMemoize, notFoundHandler, - SingleConnectionManager, + SingleConnectionDatabaseManager, SingleHostDiscovery, UrlReaders, } from '@backstage/backend-common'; @@ -32,20 +32,12 @@ function makeCreateEnv(config: ConfigReader) { root.info(`Created UrlReader ${reader}`); - const databaseManager = new SingleConnectionManager( - config.getConfig('backend.database') - ); + const databaseManager = SingleConnectionDatabaseManager.fromConfig(config); return (plugin: string): PluginEnvironment => { const logger = root.child({ type: 'plugin', plugin }); - const databaseFactory = databaseManager.getDatabaseClientFactory(plugin); - return { - logger, - databaseClientFactory: databaseFactory, - config, - reader, - discovery - }; + const database = databaseManager.forPlugin(plugin); + return { logger, database, config, reader, discovery }; }; } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts index 0f8cc21dec..fe19855d5d 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts @@ -3,14 +3,9 @@ import { PluginEnvironment } from '../types'; export default async function createPlugin({ logger, - databaseClientFactory, + database, config, discovery, }: PluginEnvironment) { - return await createRouter({ - logger, - config, - database: databaseClientFactory, - discovery - }); + return await createRouter({ logger, config, database, discovery }); } diff --git a/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts b/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts index a0ff556ed0..aee38a5fa0 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts @@ -14,12 +14,11 @@ export default async function createPlugin({ logger, config, reader, - databaseClientFactory, + database, }: PluginEnvironment) { const locationReader = new LocationReaders({ logger, reader, config }); - const db = await DatabaseManager.createDatabase( - await databaseClientFactory(), + const db = await DatabaseManager.createDatabase(await database.getClient(), { logger }, ); const entitiesCatalog = new DatabaseEntitiesCatalog(db); diff --git a/packages/create-app/templates/default-app/packages/backend/src/types.ts b/packages/create-app/templates/default-app/packages/backend/src/types.ts index c2b7b5fa8e..757a0e5acf 100644 --- a/packages/create-app/templates/default-app/packages/backend/src/types.ts +++ b/packages/create-app/templates/default-app/packages/backend/src/types.ts @@ -1,14 +1,14 @@ import { Logger } from 'winston'; import { Config } from '@backstage/config'; import { - PluginDatabaseClientFactory, + PluginDatabaseManager, PluginEndpointDiscovery, - UrlReader + UrlReader, } from '@backstage/backend-common'; export type PluginEnvironment = { logger: Logger; - databaseClientFactory: PluginDatabaseClientFactory; + database: PluginDatabaseManager; config: Config; reader: UrlReader discovery: PluginEndpointDiscovery; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index f21b2d58fc..6bb100de6b 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -24,12 +24,12 @@ import { DatabaseKeyStore, TokenFactory, createOidcRouter } from '../identity'; import { NotFoundError, PluginEndpointDiscovery, - PluginDatabaseClientFactory, + PluginDatabaseManager, } from '@backstage/backend-common'; export interface RouterOptions { logger: Logger; - database: PluginDatabaseClientFactory; + database: PluginDatabaseManager; config: Config; discovery: PluginEndpointDiscovery; } @@ -48,7 +48,7 @@ export async function createRouter({ const keyDurationSeconds = 3600; const keyStore = await DatabaseKeyStore.create({ - database: await database(), + database: await database.getClient(), }); const tokenIssuer = new TokenFactory({ issuer: authUrl, diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index b5c15b3704..d00e7d523d 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -53,8 +53,10 @@ export async function startStandaloneServer( const router = await createRouter({ logger, config, - database: async () => { - return database; + database: { + async getClient() { + return database; + }, }, discovery, }); From 906e593a54b2c103e4b90ac4ba0f94cba7c96ddd Mon Sep 17 00:00:00 2001 From: Joel Low Date: Mon, 5 Oct 2020 11:58:34 +0800 Subject: [PATCH 4/9] Implement tests --- .../src/database/SingleConnection.test.ts | 116 ++++++++++++++++++ .../src/database/config.test.ts | 12 ++ 2 files changed, 128 insertions(+) create mode 100644 packages/backend-common/src/database/SingleConnection.test.ts diff --git a/packages/backend-common/src/database/SingleConnection.test.ts b/packages/backend-common/src/database/SingleConnection.test.ts new file mode 100644 index 0000000000..9da256a2c3 --- /dev/null +++ b/packages/backend-common/src/database/SingleConnection.test.ts @@ -0,0 +1,116 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConfigReader } from '@backstage/config'; +import { createDatabaseClient } from './connection'; +import { SingleConnectionDatabaseManager } from './SingleConnection'; + +jest.mock('./connection'); + +describe('SingleConnectionDatabaseManager', () => { + const createConfig = (data: any) => + ConfigReader.fromConfigs([ + { + context: '', + data, + }, + ]); + + const defaultConfigOptions = { + backend: { + database: { + client: 'pg', + connection: { + host: 'localhost', + user: 'foo', + password: 'bar', + database: 'foodb', + }, + }, + }, + }; + const defaultConfig = () => createConfig(defaultConfigOptions); + + // This is similar to the ts-jest `mocked` helper. + const mocked = (f: Function) => f as jest.Mock; + + describe('SingleConnectionDatabaseManager.fromConfig', () => { + it('accesses the backend.database key', () => { + const getConfig = jest.fn(); + const config = defaultConfig(); + config.getConfig = getConfig; + + SingleConnectionDatabaseManager.fromConfig(config); + + expect(getConfig.mock.calls[0][0]).toEqual('backend.database'); + }); + }); + + describe('SingleConnectionDatabaseManager.forPlugin', () => { + const manager = SingleConnectionDatabaseManager.fromConfig(defaultConfig()); + + it('connects to a database scoped to the plugin', async () => { + const pluginId = 'test1'; + await manager.forPlugin(pluginId).getClient(); + + expect(mocked(createDatabaseClient).mock.calls).toHaveLength(1); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + + const callArgs = mockCalls[0]; + expect(callArgs[0].get()).toEqual(defaultConfigOptions.backend.database); + expect(callArgs[1].connection.database).toEqual( + `backstage_plugin_${pluginId}`, + ); + }); + + it('allows plugins to get alternative databases', async () => { + const pluginId = 'test1'; + const pluginManager = manager.forPlugin(pluginId); + await pluginManager.getClient(); + + const secondaryDatabase = 'extra'; + await pluginManager.getClient(secondaryDatabase); + + expect(mocked(createDatabaseClient).mock.calls).toHaveLength(2); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-2); + + const mainCallArgs = mockCalls[0]; + const secondaryCallArgs = mockCalls[1]; + expect(secondaryCallArgs[1].connection.database).toEqual( + `backstage_plugin_${pluginId}_${secondaryDatabase}`, + ); + expect(mainCallArgs[1].connection.database).not.toEqual( + secondaryCallArgs[1].connection.database, + ); + }); + + it('provides different plugins different databases', async () => { + const plugin1Id = 'test1'; + const plugin2Id = 'test2'; + await manager.forPlugin(plugin1Id).getClient(); + await manager.forPlugin(plugin2Id).getClient(); + + expect(mocked(createDatabaseClient).mock.calls).toHaveLength(2); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-2); + + const plugin1CallArgs = mockCalls[0]; + const plugin2CallArgs = mockCalls[1]; + expect(plugin1CallArgs[1].connection.database).not.toEqual( + plugin2CallArgs[1].connection.database, + ); + }); + }); +}); diff --git a/packages/backend-common/src/database/config.test.ts b/packages/backend-common/src/database/config.test.ts index ab29bc19ba..c9a4107686 100644 --- a/packages/backend-common/src/database/config.test.ts +++ b/packages/backend-common/src/database/config.test.ts @@ -18,6 +18,18 @@ import { mergeDatabaseConfig } from './config'; describe('config', () => { describe(mergeDatabaseConfig, () => { + it('does not mutate the input object', () => { + const input = { + original: 'key', + }; + const override = { + added: 'value', + }; + + mergeDatabaseConfig(input, override); + expect(input).not.toHaveProperty('added'); + }); + it('does not require overrides', () => { expect( mergeDatabaseConfig({ From f88cc8846b2f25a66335a346a8aa330c69fa120f Mon Sep 17 00:00:00 2001 From: Joel Low Date: Tue, 6 Oct 2020 09:04:05 +0800 Subject: [PATCH 5/9] Add missing @param declarations in API docs --- .../backend-common/src/database/SingleConnection.ts | 4 ++++ packages/backend-common/src/database/config.ts | 2 +- packages/backend-common/src/database/types.ts | 10 ++++++---- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/backend-common/src/database/SingleConnection.ts b/packages/backend-common/src/database/SingleConnection.ts index c34c3a8310..aa2e2186b5 100644 --- a/packages/backend-common/src/database/SingleConnection.ts +++ b/packages/backend-common/src/database/SingleConnection.ts @@ -31,6 +31,8 @@ export class SingleConnectionDatabaseManager { * Creates a new SingleConnectionDatabaseManager instance by reading from the `backend` * config section, specifically the `.database` key for discovering the management * database configuration. + * + * @param config The loaded application configuration. */ static fromConfig(config: Config): SingleConnectionDatabaseManager { return new SingleConnectionDatabaseManager( @@ -52,6 +54,8 @@ export class SingleConnectionDatabaseManager { /** * Generates a PluginDatabaseManager for consumption by plugins. + * + * @param pluginId The plugin that the database manager should be created for. Plugin names should be unique. */ forPlugin(pluginId: string): PluginDatabaseManager { const _this = this; diff --git a/packages/backend-common/src/database/config.ts b/packages/backend-common/src/database/config.ts index a16cf6cbee..af32556c28 100644 --- a/packages/backend-common/src/database/config.ts +++ b/packages/backend-common/src/database/config.ts @@ -19,7 +19,7 @@ import { merge } from 'lodash'; /** * Merges database objects together * - * @param config The base config + * @param config The base config. The input is not modified * @param overrides Any additional overrides */ export function mergeDatabaseConfig(config: any, ...overrides: any[]) { diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 915a12a24e..5705d8ae5f 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -24,10 +24,12 @@ export interface PluginDatabaseManager { * getClient provides backend plugins database connections for itself. * * The purpose of this method is to allow plugins to get isolated data - * stores so that plugins are discouraged from database integration. Plugins - * can omit the `database` parameter to get the default plugin database, or - * provide an identifier that will be used to identify a separate database - * from the default. + * stores so that plugins are discouraged from database integration. + * + * @param database This parameter can be omitted to get the default plugin + * database, or provide an identifier that will be used to identify a + * separate database from the default to connect to. This can be used for + * application-level sharding. */ getClient(database?: string): Promise; } From c990535157f1ccfa6222c00a5dc14a0b64656801 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Tue, 6 Oct 2020 18:16:13 +0800 Subject: [PATCH 6/9] Remove support for alternative plugin databases --- .../src/database/SingleConnection.test.ts | 21 ------------------- .../src/database/SingleConnection.ts | 15 +++++-------- packages/backend-common/src/database/types.ts | 7 +------ 3 files changed, 6 insertions(+), 37 deletions(-) diff --git a/packages/backend-common/src/database/SingleConnection.test.ts b/packages/backend-common/src/database/SingleConnection.test.ts index 9da256a2c3..bc68e02bf4 100644 --- a/packages/backend-common/src/database/SingleConnection.test.ts +++ b/packages/backend-common/src/database/SingleConnection.test.ts @@ -76,27 +76,6 @@ describe('SingleConnectionDatabaseManager', () => { ); }); - it('allows plugins to get alternative databases', async () => { - const pluginId = 'test1'; - const pluginManager = manager.forPlugin(pluginId); - await pluginManager.getClient(); - - const secondaryDatabase = 'extra'; - await pluginManager.getClient(secondaryDatabase); - - expect(mocked(createDatabaseClient).mock.calls).toHaveLength(2); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-2); - - const mainCallArgs = mockCalls[0]; - const secondaryCallArgs = mockCalls[1]; - expect(secondaryCallArgs[1].connection.database).toEqual( - `backstage_plugin_${pluginId}_${secondaryDatabase}`, - ); - expect(mainCallArgs[1].connection.database).not.toEqual( - secondaryCallArgs[1].connection.database, - ); - }); - it('provides different plugins different databases', async () => { const plugin1Id = 'test1'; const plugin2Id = 'test2'; diff --git a/packages/backend-common/src/database/SingleConnection.ts b/packages/backend-common/src/database/SingleConnection.ts index aa2e2186b5..f8c86838a1 100644 --- a/packages/backend-common/src/database/SingleConnection.ts +++ b/packages/backend-common/src/database/SingleConnection.ts @@ -61,17 +61,16 @@ export class SingleConnectionDatabaseManager { const _this = this; return { - getClient(database?: string): Promise { - return _this.getDatabase(pluginId, database); + getClient(): Promise { + return _this.getDatabase(pluginId); }, }; } - private async getDatabase(pluginId: string, suffix?: string): Promise { + private async getDatabase(pluginId: string): Promise { const config = this.getDatabaseConfig(); const overrides = SingleConnectionDatabaseManager.getDatabaseOverrides( pluginId, - suffix, ); const overrideConfig = overrides.connection as Knex.ConnectionConfig; await this.ensureDatabase(overrideConfig.database); @@ -79,14 +78,10 @@ export class SingleConnectionDatabaseManager { return createDatabaseClient(config, overrides); } - private static getDatabaseOverrides( - pluginId: string, - suffix?: string, - ): Knex.Config { - const dbSuffix = suffix ? `_${suffix}` : ''; + private static getDatabaseOverrides(pluginId: string): Knex.Config { return { connection: { - database: `backstage_plugin_${pluginId}${dbSuffix}`, + database: `backstage_plugin_${pluginId}`, }, }; } diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 5705d8ae5f..7eaa05f173 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -25,11 +25,6 @@ export interface PluginDatabaseManager { * * The purpose of this method is to allow plugins to get isolated data * stores so that plugins are discouraged from database integration. - * - * @param database This parameter can be omitted to get the default plugin - * database, or provide an identifier that will be used to identify a - * separate database from the default to connect to. This can be used for - * application-level sharding. */ - getClient(database?: string): Promise; + getClient(): Promise; } From 7151da64e050b0d5f3607888a978dde81458f7b0 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Tue, 6 Oct 2020 18:18:45 +0800 Subject: [PATCH 7/9] No longer any need to create databases, but drop them for consistency --- packages/e2e-test/src/e2e-test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/e2e-test/src/e2e-test.ts b/packages/e2e-test/src/e2e-test.ts index c7f6137ff1..c839454426 100644 --- a/packages/e2e-test/src/e2e-test.ts +++ b/packages/e2e-test/src/e2e-test.ts @@ -328,8 +328,8 @@ async function testAppServe(pluginName: string, appDir: string) { } } -/** Creates PG databases (drops if exists before) */ -async function createDB(database: string) { +/** Drops PG databases */ +async function dropDB(database: string) { const config = { host: process.env.POSTGRES_HOST, port: process.env.POSTGRES_PORT, @@ -342,7 +342,6 @@ async function createDB(database: string) { } catch (_) { /* do nothing*/ } - return pgtools.createdb(config, database); } /** @@ -350,7 +349,7 @@ async function createDB(database: string) { */ async function testBackendStart(appDir: string, isPostgres: boolean) { if (isPostgres) { - print('Creating DBs'); + print('Dropping old DBs'); await Promise.all( [ 'catalog', @@ -359,7 +358,7 @@ async function testBackendStart(appDir: string, isPostgres: boolean) { 'identity', 'proxy', 'techdocs', - ].map(name => createDB(`backstage_plugin_${name}`)), + ].map(name => dropDB(`backstage_plugin_${name}`)), ); print('Created DBs'); } From 0bc45e0d3a4133ce6d6a4bda2d4625d961921c41 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Wed, 7 Oct 2020 09:36:25 +0800 Subject: [PATCH 8/9] Fix additional review comments --- CHANGELOG.md | 5 ++++- .../src/database/SingleConnection.ts | 21 +++++-------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5229a6794..487b94a39d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,13 +8,16 @@ If you encounter issues while upgrading to a newer version, don't hesitate to re > Collect changes for the next release below +### Backend (example-backend, or backends created with @backstage/create-app) + +- A plugin database manager has been created, and plugins can now accept that interface as an argument during initialisation. Notably, the `auth` plugin has a [`createRouter` signature change](./plugins/auth-backend/src/service/router.ts). See [packages/backend/src/index.ts](./packages/backend/src/index.ts) on how to set it up. [#2697](https://github.com/spotify/backstage/pull/2697) + ## v0.1.1-alpha.24 ### Backend (example-backend, or backends created with @backstage/create-app) - The default mount point for backend plugins have been changed to `/api`. These changes are done in the backend package itself, so it is recommended that you sync up existing backend packages with this new pattern. [#2562](https://github.com/spotify/backstage/pull/2562) - A service discovery mechanism for backend plugins has been added, and is now a requirement for several backend plugins. See [packages/backend/src/index.ts](./packages/backend/src/index.ts) for how to set it up using `SingleHostDiscovery` from `@backstage/backend-common`. Note that the default base path for plugins is set to `/api` to that change, but it can be set to use the old behavior via the `basePath` option. [#2600](https://github.com/spotify/backstage/pull/2600) -- A plugin database manager has been created, and plugins can now accept that interface as an argument during initialisation. Notably, the `auth` plugin has a [`createRouter` signature change](./plugins/auth-backend/src/service/router.ts). See [packages/backend/src/index.ts](./packages/backend/src/index.ts) on how to set it up. [#2697](https://github.com/spotify/backstage/pull/2697) ### @backstage/auth-backend diff --git a/packages/backend-common/src/database/SingleConnection.ts b/packages/backend-common/src/database/SingleConnection.ts index f8c86838a1..777ced54a0 100644 --- a/packages/backend-common/src/database/SingleConnection.ts +++ b/packages/backend-common/src/database/SingleConnection.ts @@ -22,11 +22,10 @@ import { PluginDatabaseManager } from './types'; /** * Implements a Database Manager which will automatically create new databases * for plugins when requested. All requested databases are created with the - * credentials provided. + * credentials provided; if the database already exists no attempt to create + * the database will be made. */ export class SingleConnectionDatabaseManager { - private readonly config: Config; - /** * Creates a new SingleConnectionDatabaseManager instance by reading from the `backend` * config section, specifically the `.database` key for discovering the management @@ -40,17 +39,7 @@ export class SingleConnectionDatabaseManager { ); } - private constructor(config: Config) { - this.config = config; - } - - private getDatabaseConfig(): Config { - return this.config; - } - - private getAdminConfig(): Config { - return this.config; - } + private constructor(private readonly config: Config) {} /** * Generates a PluginDatabaseManager for consumption by plugins. @@ -68,7 +57,7 @@ export class SingleConnectionDatabaseManager { } private async getDatabase(pluginId: string): Promise { - const config = this.getDatabaseConfig(); + const config = this.config; const overrides = SingleConnectionDatabaseManager.getDatabaseOverrides( pluginId, ); @@ -87,7 +76,7 @@ export class SingleConnectionDatabaseManager { } private async ensureDatabase(database: string) { - const config = this.getAdminConfig(); + const config = this.config; await ensureDatabaseExists(config, database); } } From 7471a46d270004355fb87afdd0f10c83651b8a0d Mon Sep 17 00:00:00 2001 From: Joel Low Date: Wed, 7 Oct 2020 14:59:09 +0800 Subject: [PATCH 9/9] Use `resetAllMocks` and `toHaveBeenCalledTimes` --- .../src/database/SingleConnection.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/database/SingleConnection.test.ts b/packages/backend-common/src/database/SingleConnection.test.ts index bc68e02bf4..6278e4ecad 100644 --- a/packages/backend-common/src/database/SingleConnection.test.ts +++ b/packages/backend-common/src/database/SingleConnection.test.ts @@ -47,6 +47,8 @@ describe('SingleConnectionDatabaseManager', () => { // This is similar to the ts-jest `mocked` helper. const mocked = (f: Function) => f as jest.Mock; + afterEach(() => jest.resetAllMocks()); + describe('SingleConnectionDatabaseManager.fromConfig', () => { it('accesses the backend.database key', () => { const getConfig = jest.fn(); @@ -66,9 +68,9 @@ describe('SingleConnectionDatabaseManager', () => { const pluginId = 'test1'; await manager.forPlugin(pluginId).getClient(); - expect(mocked(createDatabaseClient).mock.calls).toHaveLength(1); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + expect(mocked(createDatabaseClient)).toHaveBeenCalledTimes(1); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); const callArgs = mockCalls[0]; expect(callArgs[0].get()).toEqual(defaultConfigOptions.backend.database); expect(callArgs[1].connection.database).toEqual( @@ -82,9 +84,9 @@ describe('SingleConnectionDatabaseManager', () => { await manager.forPlugin(plugin1Id).getClient(); await manager.forPlugin(plugin2Id).getClient(); - expect(mocked(createDatabaseClient).mock.calls).toHaveLength(2); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-2); + expect(mocked(createDatabaseClient)).toHaveBeenCalledTimes(2); + const mockCalls = mocked(createDatabaseClient).mock.calls; const plugin1CallArgs = mockCalls[0]; const plugin2CallArgs = mockCalls[1]; expect(plugin1CallArgs[1].connection.database).not.toEqual(