From dd7764675084ab6254065b18278899a6f3e9845c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Tue, 6 Oct 2020 15:34:34 +0200 Subject: [PATCH 01/33] emptystates: add transparent versions --- .../EmptyState/EmptyState.stories.tsx | 14 +----- .../src/components/EmptyState/EmptyState.tsx | 5 --- .../EmptyState/assets/createComponent.svg | 37 ++++++++++++++- .../EmptyState/assets/missingAnnotation.svg | 43 +++++++++++++++++- .../components/EmptyState/assets/noBuild.svg | 45 ++++++++++++++++++- .../EmptyState/assets/noInformation.svg | 11 ++++- 6 files changed, 134 insertions(+), 21 deletions(-) diff --git a/packages/core/src/components/EmptyState/EmptyState.stories.tsx b/packages/core/src/components/EmptyState/EmptyState.stories.tsx index 15d61f14c0..9d507091d2 100644 --- a/packages/core/src/components/EmptyState/EmptyState.stories.tsx +++ b/packages/core/src/components/EmptyState/EmptyState.stories.tsx @@ -26,13 +26,9 @@ export default { const containerStyle = { width: '100%', height: '100vh' }; -export const Field = () => ( +export const MissingAnnotation = () => (
- +
); @@ -80,9 +76,3 @@ export const WithAction = () => ( /> ); - -export const MissingAnnotation = () => ( -
- -
-); diff --git a/packages/core/src/components/EmptyState/EmptyState.tsx b/packages/core/src/components/EmptyState/EmptyState.tsx index 4469c3ce86..3a56fc6cb3 100644 --- a/packages/core/src/components/EmptyState/EmptyState.tsx +++ b/packages/core/src/components/EmptyState/EmptyState.tsx @@ -66,11 +66,6 @@ export const EmptyState = ({ title, description, missing, action }: Props) => { - background diff --git a/packages/core/src/components/EmptyState/assets/createComponent.svg b/packages/core/src/components/EmptyState/assets/createComponent.svg index bb8b50e1ef..2634bb7f23 100644 --- a/packages/core/src/components/EmptyState/assets/createComponent.svg +++ b/packages/core/src/components/EmptyState/assets/createComponent.svg @@ -1 +1,36 @@ - \ No newline at end of file + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/core/src/components/EmptyState/assets/missingAnnotation.svg b/packages/core/src/components/EmptyState/assets/missingAnnotation.svg index 39534656fa..4f69605e6a 100644 --- a/packages/core/src/components/EmptyState/assets/missingAnnotation.svg +++ b/packages/core/src/components/EmptyState/assets/missingAnnotation.svg @@ -1 +1,42 @@ - \ No newline at end of file + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/core/src/components/EmptyState/assets/noBuild.svg b/packages/core/src/components/EmptyState/assets/noBuild.svg index ef45e1c736..d84f36af30 100644 --- a/packages/core/src/components/EmptyState/assets/noBuild.svg +++ b/packages/core/src/components/EmptyState/assets/noBuild.svg @@ -1 +1,44 @@ - \ No newline at end of file + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/core/src/components/EmptyState/assets/noInformation.svg b/packages/core/src/components/EmptyState/assets/noInformation.svg index 9a1c230e97..a36ce59d32 100644 --- a/packages/core/src/components/EmptyState/assets/noInformation.svg +++ b/packages/core/src/components/EmptyState/assets/noInformation.svg @@ -1 +1,10 @@ - \ No newline at end of file + + + + + + + + + + From a385c27eeb8bf9befafcb3880b40a86f35686f4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Tue, 6 Oct 2020 15:35:29 +0200 Subject: [PATCH 02/33] rm background --- packages/core/src/components/EmptyState/EmptyState.tsx | 1 - packages/core/src/components/EmptyState/assets/Background.svg | 1 - 2 files changed, 2 deletions(-) delete mode 100644 packages/core/src/components/EmptyState/assets/Background.svg diff --git a/packages/core/src/components/EmptyState/EmptyState.tsx b/packages/core/src/components/EmptyState/EmptyState.tsx index 3a56fc6cb3..523f4b467d 100644 --- a/packages/core/src/components/EmptyState/EmptyState.tsx +++ b/packages/core/src/components/EmptyState/EmptyState.tsx @@ -17,7 +17,6 @@ import React from 'react'; import { makeStyles, Typography, Grid } from '@material-ui/core'; import { EmptyStateImage } from './EmptyStateImage'; -import Background from './assets/Background.svg'; const useStyles = makeStyles(theme => ({ root: { diff --git a/packages/core/src/components/EmptyState/assets/Background.svg b/packages/core/src/components/EmptyState/assets/Background.svg deleted file mode 100644 index ce9aae6739..0000000000 --- a/packages/core/src/components/EmptyState/assets/Background.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file From e277c5dfa587134672ac3ba81351dee03deb8022 Mon Sep 17 00:00:00 2001 From: Joel Low Date: Thu, 1 Oct 2020 17:38:12 +0800 Subject: [PATCH 03/33] 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 04/33] 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 05/33] 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 06/33] 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 07/33] 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 08/33] 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 09/33] 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 10/33] 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 04af2d7af4dd5507047e6f0a4efd929988eaa8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Wed, 7 Oct 2020 08:40:57 +0200 Subject: [PATCH 11/33] rm non-used css --- packages/core/src/components/EmptyState/EmptyState.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/components/EmptyState/EmptyState.tsx b/packages/core/src/components/EmptyState/EmptyState.tsx index 523f4b467d..1af5ff6dd5 100644 --- a/packages/core/src/components/EmptyState/EmptyState.tsx +++ b/packages/core/src/components/EmptyState/EmptyState.tsx @@ -29,10 +29,6 @@ const useStyles = makeStyles(theme => ({ imageContainer: { position: 'relative', }, - backgroundImage: { - position: 'absolute', - width: '100%', - }, })); type Props = { From 758729cc3d66a765195335ffc68e723c93893a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Wed, 7 Oct 2020 08:45:00 +0200 Subject: [PATCH 12/33] Update EmptyStateImage.tsx --- packages/core/src/components/EmptyState/EmptyStateImage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/EmptyState/EmptyStateImage.tsx b/packages/core/src/components/EmptyState/EmptyStateImage.tsx index 49598ae6db..d52a0f7dcf 100644 --- a/packages/core/src/components/EmptyState/EmptyStateImage.tsx +++ b/packages/core/src/components/EmptyState/EmptyStateImage.tsx @@ -27,7 +27,7 @@ type Props = { const useStyles = makeStyles({ generalImg: { - width: '75%', + width: '95%', zIndex: 2, position: 'absolute', left: '50%', From 7471a46d270004355fb87afdd0f10c83651b8a0d Mon Sep 17 00:00:00 2001 From: Joel Low Date: Wed, 7 Oct 2020 14:59:09 +0800 Subject: [PATCH 13/33] 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( From 8ca12579875e5bd3df21e8920e6e423ac4f02ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Wed, 7 Oct 2020 19:14:45 +0200 Subject: [PATCH 14/33] Update noInformation.svg --- .../components/EmptyState/assets/noInformation.svg | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/packages/core/src/components/EmptyState/assets/noInformation.svg b/packages/core/src/components/EmptyState/assets/noInformation.svg index a36ce59d32..c199cf8b13 100644 --- a/packages/core/src/components/EmptyState/assets/noInformation.svg +++ b/packages/core/src/components/EmptyState/assets/noInformation.svg @@ -1,10 +1 @@ - - - - - - - - - - + From 0181b365e4fe8a61a2636e2bb4fb785b518d46d2 Mon Sep 17 00:00:00 2001 From: Brenda Sukh Date: Wed, 7 Oct 2020 14:52:26 -0400 Subject: [PATCH 15/33] Add project field to product cost api calls --- .../ExampleCostInsightsClient.ts | 34 +++++++++++++++++-- .../cost-insights/src/api/CostInsightsApi.ts | 15 +++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/app/src/plugins/cost-insights/ExampleCostInsightsClient.ts b/packages/app/src/plugins/cost-insights/ExampleCostInsightsClient.ts index c01092dc51..e1ef77c537 100644 --- a/packages/app/src/plugins/cost-insights/ExampleCostInsightsClient.ts +++ b/packages/app/src/plugins/cost-insights/ExampleCostInsightsClient.ts @@ -23,6 +23,7 @@ import { Project, ProductCost, Group, + Maybe, } from '@backstage/plugin-cost-insights'; export class ExampleCostInsightsClient implements CostInsightsApi { @@ -122,9 +123,38 @@ export class ExampleCostInsightsClient implements CostInsightsApi { product: string, group: string, duration: Duration, + project: Maybe, ): Promise { + const projectProductInsights = await this.request( + { product, group, duration, project }, + { + aggregation: [80_000, 110_000], + change: { + ratio: 0.375, + amount: 30_000, + }, + entities: [ + { + id: null, // entities with null ids will be appear as "Unlabeled" in product panels + aggregation: [45_000, 50_000], + }, + { + id: 'entity-a', + aggregation: [15_000, 20_000], + }, + { + id: 'entity-b', + aggregation: [20_000, 30_000], + }, + { + id: 'entity-e', + aggregation: [0, 10_000], + }, + ], + }, + ); const productInsights: ProductCost = await this.request( - { product, group, duration }, + { product, group, duration, project }, { aggregation: [200_000, 250_000], change: { @@ -172,7 +202,7 @@ export class ExampleCostInsightsClient implements CostInsightsApi { }, ); - return productInsights; + return project ? projectProductInsights : productInsights; } async getAlerts(group: string): Promise { diff --git a/plugins/cost-insights/src/api/CostInsightsApi.ts b/plugins/cost-insights/src/api/CostInsightsApi.ts index b7628e2f9e..bdf673b7d1 100644 --- a/plugins/cost-insights/src/api/CostInsightsApi.ts +++ b/plugins/cost-insights/src/api/CostInsightsApi.ts @@ -15,7 +15,15 @@ */ import { createApiRef } from '@backstage/core'; -import { Alert, Cost, Duration, Group, Project, ProductCost } from '../types'; +import { + Alert, + Cost, + Duration, + Group, + Project, + ProductCost, + Maybe, +} from '../types'; export type CostInsightsApi = { /** @@ -87,6 +95,9 @@ export type CostInsightsApi = { * in this product. The type of entity depends on the product - it may be deployed services, * storage buckets, managed database instances, etc. * + * If project is supplied, this should only return product costs for the given billing entity + * (project in GCP). + * * The time period is supplied as a Duration rather than intervals, since this is always expected * to return data for two bucketed time period (e.g. month vs month, or quarter vs quarter). * @@ -94,11 +105,13 @@ export type CostInsightsApi = { * @param group * @param duration A time duration, such as P1M. See the Duration type for a detailed explanation * of how the durations are interpreted in Cost Insights. + * @param project (optional) The project id from getGroupProjects or query parameters */ getProductInsights( product: string, group: string, duration: Duration, + project: Maybe, ): Promise; /** From 58db835fbd5f4ddab0153ff7fe99062fb2ed2e30 Mon Sep 17 00:00:00 2001 From: Brenda Sukh Date: Wed, 7 Oct 2020 14:53:05 -0400 Subject: [PATCH 16/33] Remove project select from cost overview footer --- .../CostOverviewCard/CostOverviewCard.tsx | 12 ++----- .../CostOverviewCard/CostOverviewFooter.tsx | 36 ------------------- 2 files changed, 2 insertions(+), 46 deletions(-) delete mode 100644 plugins/cost-insights/src/components/CostOverviewCard/CostOverviewFooter.tsx diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx index ef174132a3..1d9543c8c0 100644 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx +++ b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewCard.tsx @@ -19,10 +19,8 @@ import { Box, Card, CardContent, Divider } from '@material-ui/core'; import CostOverviewChart from '../CostOverviewChart'; import CostOverviewChartLegend from '../CostOverviewChartLegend'; import CostOverviewHeader from './CostOverviewHeader'; -import CostOverviewFooter from './CostOverviewFooter'; import MetricSelect from '../MetricSelect'; import PeriodSelect from '../PeriodSelect'; -import ProjectSelect from '../ProjectSelect'; import { useScroll, useFilters, useConfig } from '../../hooks'; import { mapFiltersToProps } from './selector'; import { DefaultNavigation } from '../../utils/navigation'; @@ -45,7 +43,6 @@ const CostOverviewCard = ({ change, aggregation, trendline, - projects, }: CostOverviewCardProps) => { const { metrics } = useConfig(); const { ScrollAnchor } = useScroll(DefaultNavigation.CostOverviewCard); @@ -73,18 +70,13 @@ const CostOverviewCard = ({ trendline={trendline} /> - - + - + ); diff --git a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewFooter.tsx b/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewFooter.tsx deleted file mode 100644 index 03920887b8..0000000000 --- a/plugins/cost-insights/src/components/CostOverviewCard/CostOverviewFooter.tsx +++ /dev/null @@ -1,36 +0,0 @@ -/* - * 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 React from 'react'; -import { Box } from '@material-ui/core'; - -type CostOverviewFooterProps = { - children?: React.ReactNode; -}; - -const CostOverviewFooter = ({ children }: CostOverviewFooterProps) => ( - - {React.Children.map(children, child => ( - {child} - ))} - -); - -export default CostOverviewFooter; From d8f874d39ae5609fe339e3ca9ce7ce3bc3cc7914 Mon Sep 17 00:00:00 2001 From: Brenda Sukh Date: Wed, 7 Oct 2020 14:54:18 -0400 Subject: [PATCH 17/33] Add cost overview banner --- .../CostInsightsPage/CostInsightsPage.tsx | 62 +++++++++++++++---- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx index c426eff8bc..dfd90e428a 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/CostInsightsPage.tsx @@ -15,7 +15,7 @@ */ import React, { useCallback, useEffect, useState } from 'react'; -import { Box, Container, Divider, Grid } from '@material-ui/core'; +import { Box, Container, Divider, Grid, Typography } from '@material-ui/core'; import { Progress, useApi, featureFlagsApiRef } from '@backstage/core'; import { default as MaterialAlert } from '@material-ui/lab/Alert'; import { costInsightsApiRef } from '../../api'; @@ -41,6 +41,7 @@ import { } from '../../hooks'; import { Alert, Cost, intervalsOf, Maybe, Project } from '../../types'; import { mapLoadingToProps } from './selector'; +import ProjectSelect from '../ProjectSelect'; const CostInsightsPage = () => { const flags = useApi(featureFlagsApiRef).getFlags(); @@ -55,7 +56,7 @@ const CostInsightsPage = () => { const [alerts, setAlerts] = useState>(null); const [error, setError] = useState>(null); - const { pageFilters } = useFilters(p => p); + const { pageFilters, setPageFilters } = useFilters(p => p); const { loadingActions, loadingGroups, @@ -63,6 +64,7 @@ const CostInsightsPage = () => { dispatchInitial, dispatchInsights, dispatchNone, + dispatchReset, } = useLoading(mapLoadingToProps); /* eslint-disable react-hooks/exhaustive-deps */ @@ -75,8 +77,15 @@ const CostInsightsPage = () => { const dispatchLoadingInitial = useCallback(dispatchInitial, []); const dispatchLoadingInsights = useCallback(dispatchInsights, []); const dispatchLoadingNone = useCallback(dispatchNone, []); + const dispatchLoadingReset = useCallback(dispatchReset, []); /* eslint-enable react-hooks/exhaustive-deps */ + const setProject = (project: Maybe) => + setPageFilters({ + ...pageFilters, + project: project === 'all' ? null : project, + }); + useEffect(() => { async function getInsights() { setError(null); @@ -165,6 +174,43 @@ const CostInsightsPage = () => { ); } + const onProjectSelect = (project: Maybe) => { + setProject(project); + dispatchLoadingReset(loadingActions); + }; + + const CostOverviewBanner = () => ( + + + + Cost Overview + + + + {!!flags.get('cost-insights-currencies') && ( + + + + )} + + + + ); + return ( @@ -180,15 +226,6 @@ const CostInsightsPage = () => { justifyContent="flex-end" mb={2} > - {!!flags.get('cost-insights-currencies') && ( - - - - )} @@ -212,6 +249,9 @@ const CostInsightsPage = () => { )} + + + {!!dailyCost.aggregation.length && ( From d0241306fa1ed6736ca38f4c079828b169422d5b Mon Sep 17 00:00:00 2001 From: Brenda Sukh Date: Wed, 7 Oct 2020 14:54:37 -0400 Subject: [PATCH 18/33] Update cost insights select styles --- .../src/components/CostInsightsPage/selector.tsx | 9 ++++++++- .../ProductInsightsCard/ProductInsightsCard.tsx | 4 +++- .../src/components/ProjectSelect/ProjectSelect.tsx | 2 +- plugins/cost-insights/src/utils/styles.ts | 1 + 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/plugins/cost-insights/src/components/CostInsightsPage/selector.tsx b/plugins/cost-insights/src/components/CostInsightsPage/selector.tsx index 51db47c082..8a9cf021c9 100644 --- a/plugins/cost-insights/src/components/CostInsightsPage/selector.tsx +++ b/plugins/cost-insights/src/components/CostInsightsPage/selector.tsx @@ -14,7 +14,11 @@ * limitations under the License. */ import { MapLoadingToProps } from '../../hooks'; -import { getResetState, DefaultLoadingAction } from '../../types'; +import { + getResetState, + DefaultLoadingAction, + getResetStateWithoutInitial, +} from '../../types'; type CostInsightsPageLoadingProps = { loadingActions: Array; @@ -23,6 +27,7 @@ type CostInsightsPageLoadingProps = { dispatchInitial: (isLoading: boolean) => void; dispatchInsights: (isLoading: boolean) => void; dispatchNone: (loadingActions: string[]) => void; + dispatchReset: (loadingActions: string[]) => void; }; export const mapLoadingToProps: MapLoadingToProps = ({ @@ -39,4 +44,6 @@ export const mapLoadingToProps: MapLoadingToProps dispatch({ [DefaultLoadingAction.CostInsightsPage]: isLoading }), dispatchNone: (loadingActions: string[]) => dispatch(getResetState(loadingActions)), + dispatchReset: (loadingActions: string[]) => + dispatch(getResetStateWithoutInitial(loadingActions)), }); diff --git a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx index 1d24139c64..c9790a5bfa 100644 --- a/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx +++ b/plugins/cost-insights/src/components/ProductInsightsCard/ProductInsightsCard.tsx @@ -39,7 +39,7 @@ const ProductInsightsCard = ({ product }: ProductInsightsCardProps) => { const [resource, setResource] = useState>(null); const [error, setError] = useState>(null); - const { group, product: productFilter, setProduct } = useFilters( + const { group, product: productFilter, setProduct, project } = useFilters( mapFiltersToProps(product.kind), ); const { loadingProduct, dispatchLoading } = useLoading( @@ -68,6 +68,7 @@ const ProductInsightsCard = ({ product }: ProductInsightsCardProps) => { product.kind, group!, productFilter!.duration, + project, ); setResource(p); } catch (e) { @@ -87,6 +88,7 @@ const ProductInsightsCard = ({ product }: ProductInsightsCardProps) => { productFilter, group, product.kind, + project, ]); const onPeriodSelect = (duration: Duration) => { diff --git a/plugins/cost-insights/src/components/ProjectSelect/ProjectSelect.tsx b/plugins/cost-insights/src/components/ProjectSelect/ProjectSelect.tsx index cbcd8ede4e..934ad99f63 100644 --- a/plugins/cost-insights/src/components/ProjectSelect/ProjectSelect.tsx +++ b/plugins/cost-insights/src/components/ProjectSelect/ProjectSelect.tsx @@ -48,7 +48,7 @@ const ProjectSelect = ({ project, projects, onSelect }: ProjectSelectProps) => {