From fdc1d3d8ef7a4d8b82fa670a540a7e3485d3a944 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 16:43:28 +0100 Subject: [PATCH 01/18] backend-app-api: forklift config implementation from backend-common Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 6 ++++++ .../src => backend-app-api/src/config}/config.test.ts | 0 .../src => backend-app-api/src/config}/config.ts | 0 yarn.lock | 6 ++++++ 4 files changed, 12 insertions(+) rename packages/{backend-common/src => backend-app-api/src/config}/config.test.ts (100%) rename packages/{backend-common/src => backend-app-api/src/config}/config.ts (100%) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 01eaeaecaf..943db41972 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -36,10 +36,14 @@ "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-tasks": "workspace:^", + "@backstage/cli-common": "workspace:^", "@backstage/config": "workspace:^", + "@backstage/config-loader": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", "@backstage/plugin-permission-node": "workspace:^", + "@backstage/types": "workspace:^", + "@manypkg/get-packages": "^1.1.3", "@types/cors": "^2.8.6", "@types/express": "^4.17.6", "compression": "^1.7.4", @@ -50,6 +54,7 @@ "helmet": "^6.0.0", "lodash": "^4.17.21", "minimatch": "^5.0.0", + "minimist": "^1.2.5", "morgan": "^1.10.0", "node-forge": "^1.3.1", "selfsigned": "^2.0.0", @@ -61,6 +66,7 @@ "@types/compression": "^1.7.0", "@types/fs-extra": "^9.0.3", "@types/http-errors": "^2.0.0", + "@types/minimist": "^1.2.0", "@types/morgan": "^1.9.0", "@types/node-forge": "^1.3.0", "@types/stoppable": "^1.1.0", diff --git a/packages/backend-common/src/config.test.ts b/packages/backend-app-api/src/config/config.test.ts similarity index 100% rename from packages/backend-common/src/config.test.ts rename to packages/backend-app-api/src/config/config.test.ts diff --git a/packages/backend-common/src/config.ts b/packages/backend-app-api/src/config/config.ts similarity index 100% rename from packages/backend-common/src/config.ts rename to packages/backend-app-api/src/config/config.ts diff --git a/yarn.lock b/yarn.lock index 282b7b24bc..40d2189edc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3381,15 +3381,20 @@ __metadata: "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-tasks": "workspace:^" "@backstage/cli": "workspace:^" + "@backstage/cli-common": "workspace:^" "@backstage/config": "workspace:^" + "@backstage/config-loader": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/plugin-auth-node": "workspace:^" "@backstage/plugin-permission-node": "workspace:^" + "@backstage/types": "workspace:^" + "@manypkg/get-packages": ^1.1.3 "@types/compression": ^1.7.0 "@types/cors": ^2.8.6 "@types/express": ^4.17.6 "@types/fs-extra": ^9.0.3 "@types/http-errors": ^2.0.0 + "@types/minimist": ^1.2.0 "@types/morgan": ^1.9.0 "@types/node-forge": ^1.3.0 "@types/stoppable": ^1.1.0 @@ -3402,6 +3407,7 @@ __metadata: http-errors: ^2.0.0 lodash: ^4.17.21 minimatch: ^5.0.0 + minimist: ^1.2.5 morgan: ^1.10.0 node-forge: ^1.3.1 selfsigned: ^2.0.0 From 6420b7dc512caf111b25374c133c9ff6af792a5f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 16:44:50 +0100 Subject: [PATCH 02/18] backend-app-api: split out ObservableConfigProxy Signed-off-by: Patrik Oldsberg --- .../src/config/ObservableConfigProxy.ts | 130 ++++++++++++++++++ packages/backend-app-api/src/config/config.ts | 112 --------------- 2 files changed, 130 insertions(+), 112 deletions(-) create mode 100644 packages/backend-app-api/src/config/ObservableConfigProxy.ts diff --git a/packages/backend-app-api/src/config/ObservableConfigProxy.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.ts new file mode 100644 index 0000000000..8fcc3f060f --- /dev/null +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.ts @@ -0,0 +1,130 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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 { ConfigService, LoggerService } from '@backstage/backend-plugin-api'; +import { ConfigReader } from '@backstage/config'; +import { JsonValue } from '@backstage/types'; + +export class ObservableConfigProxy implements ConfigService { + private config: ConfigService = new ConfigReader({}); + + private readonly subscribers: (() => void)[] = []; + + constructor( + private readonly logger: LoggerService, + private readonly parent?: ObservableConfigProxy, + private parentKey?: string, + ) { + if (parent && !parentKey) { + throw new Error('parentKey is required if parent is set'); + } + } + + setConfig(config: ConfigService) { + if (this.parent) { + throw new Error('immutable'); + } + this.config = config; + for (const subscriber of this.subscribers) { + try { + subscriber(); + } catch (error) { + this.logger.error(`Config subscriber threw error, ${error}`); + } + } + } + + subscribe(onChange: () => void): { unsubscribe: () => void } { + if (this.parent) { + return this.parent.subscribe(onChange); + } + + this.subscribers.push(onChange); + return { + unsubscribe: () => { + const index = this.subscribers.indexOf(onChange); + if (index >= 0) { + this.subscribers.splice(index, 1); + } + }, + }; + } + + private select(required: true): ConfigService; + private select(required: false): ConfigService | undefined; + private select(required: boolean): ConfigService | undefined { + if (this.parent && this.parentKey) { + if (required) { + return this.parent.select(true).getConfig(this.parentKey); + } + return this.parent.select(false)?.getOptionalConfig(this.parentKey); + } + + return this.config; + } + + has(key: string): boolean { + return this.select(false)?.has(key) ?? false; + } + keys(): string[] { + return this.select(false)?.keys() ?? []; + } + get(key?: string): T { + return this.select(true).get(key); + } + getOptional(key?: string): T | undefined { + return this.select(false)?.getOptional(key); + } + getConfig(key: string): ConfigService { + return new ObservableConfigProxy(this.logger, this, key); + } + getOptionalConfig(key: string): ConfigService | undefined { + if (this.select(false)?.has(key)) { + return new ObservableConfigProxy(this.logger, this, key); + } + return undefined; + } + getConfigArray(key: string): ConfigService[] { + return this.select(true).getConfigArray(key); + } + getOptionalConfigArray(key: string): ConfigService[] | undefined { + return this.select(false)?.getOptionalConfigArray(key); + } + getNumber(key: string): number { + return this.select(true).getNumber(key); + } + getOptionalNumber(key: string): number | undefined { + return this.select(false)?.getOptionalNumber(key); + } + getBoolean(key: string): boolean { + return this.select(true).getBoolean(key); + } + getOptionalBoolean(key: string): boolean | undefined { + return this.select(false)?.getOptionalBoolean(key); + } + getString(key: string): string { + return this.select(true).getString(key); + } + getOptionalString(key: string): string | undefined { + return this.select(false)?.getOptionalString(key); + } + getStringArray(key: string): string[] { + return this.select(true).getStringArray(key); + } + getOptionalStringArray(key: string): string[] | undefined { + return this.select(false)?.getOptionalStringArray(key); + } +} diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index a09557b22d..0a8854c17f 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -26,7 +26,6 @@ import { LoadConfigOptionsRemote, } from '@backstage/config-loader'; import { AppConfig, Config, ConfigReader } from '@backstage/config'; -import { JsonValue } from '@backstage/types'; import { getPackages } from '@manypkg/get-packages'; import { isValidUrl } from './urls'; @@ -61,117 +60,6 @@ const updateRedactionList = ( setRootLoggerRedactionList(Array.from(values)); }; -export class ObservableConfigProxy implements Config { - private config: Config = new ConfigReader({}); - - private readonly subscribers: (() => void)[] = []; - - constructor( - private readonly logger: LoggerService, - private readonly parent?: ObservableConfigProxy, - private parentKey?: string, - ) { - if (parent && !parentKey) { - throw new Error('parentKey is required if parent is set'); - } - } - - setConfig(config: Config) { - if (this.parent) { - throw new Error('immutable'); - } - this.config = config; - for (const subscriber of this.subscribers) { - try { - subscriber(); - } catch (error) { - this.logger.error(`Config subscriber threw error, ${error}`); - } - } - } - - subscribe(onChange: () => void): { unsubscribe: () => void } { - if (this.parent) { - return this.parent.subscribe(onChange); - } - - this.subscribers.push(onChange); - return { - unsubscribe: () => { - const index = this.subscribers.indexOf(onChange); - if (index >= 0) { - this.subscribers.splice(index, 1); - } - }, - }; - } - - private select(required: true): Config; - private select(required: false): Config | undefined; - private select(required: boolean): Config | undefined { - if (this.parent && this.parentKey) { - if (required) { - return this.parent.select(true).getConfig(this.parentKey); - } - return this.parent.select(false)?.getOptionalConfig(this.parentKey); - } - - return this.config; - } - - has(key: string): boolean { - return this.select(false)?.has(key) ?? false; - } - keys(): string[] { - return this.select(false)?.keys() ?? []; - } - get(key?: string): T { - return this.select(true).get(key); - } - getOptional(key?: string): T | undefined { - return this.select(false)?.getOptional(key); - } - getConfig(key: string): Config { - return new ObservableConfigProxy(this.logger, this, key); - } - getOptionalConfig(key: string): Config | undefined { - if (this.select(false)?.has(key)) { - return new ObservableConfigProxy(this.logger, this, key); - } - return undefined; - } - getConfigArray(key: string): Config[] { - return this.select(true).getConfigArray(key); - } - getOptionalConfigArray(key: string): Config[] | undefined { - return this.select(false)?.getOptionalConfigArray(key); - } - getNumber(key: string): number { - return this.select(true).getNumber(key); - } - getOptionalNumber(key: string): number | undefined { - return this.select(false)?.getOptionalNumber(key); - } - getBoolean(key: string): boolean { - return this.select(true).getBoolean(key); - } - getOptionalBoolean(key: string): boolean | undefined { - return this.select(false)?.getOptionalBoolean(key); - } - getString(key: string): string { - return this.select(true).getString(key); - } - getOptionalString(key: string): string | undefined { - return this.select(false)?.getOptionalString(key); - } - getStringArray(key: string): string[] { - return this.select(true).getStringArray(key); - } - getOptionalStringArray(key: string): string[] | undefined { - return this.select(false)?.getOptionalStringArray(key); - } -} - // A global used to ensure that only a single file watcher is active at a time. let currentCancelFunc: () => void; From 240514363f7128e9d7e47ed5140811e0c17ce8bb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 16:49:02 +0100 Subject: [PATCH 03/18] backend-app-api: lift in url utils from backend-common Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/config/config.ts | 4 +-- packages/backend-app-api/src/lib/urls.test.ts | 34 +++++++++++++++++++ packages/backend-app-api/src/lib/urls.ts | 25 ++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 packages/backend-app-api/src/lib/urls.test.ts create mode 100644 packages/backend-app-api/src/lib/urls.ts diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index 0a8854c17f..0f07be85d2 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -27,8 +27,8 @@ import { } from '@backstage/config-loader'; import { AppConfig, Config, ConfigReader } from '@backstage/config'; import { getPackages } from '@manypkg/get-packages'; - -import { isValidUrl } from './urls'; +import { ObservableConfigProxy } from './ObservableConfigProxy'; +import { isValidUrl } from '../lib/urls'; import { setRootLoggerRedactionList } from './logging/rootLogger'; diff --git a/packages/backend-app-api/src/lib/urls.test.ts b/packages/backend-app-api/src/lib/urls.test.ts new file mode 100644 index 0000000000..c2a67fb849 --- /dev/null +++ b/packages/backend-app-api/src/lib/urls.test.ts @@ -0,0 +1,34 @@ +/* + * Copyright 2021 The Backstage Authors + * + * 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 { isValidUrl } from './urls'; + +describe('isValidUrl', () => { + it('should return true for url', () => { + const validUrl = isValidUrl('http://some.valid.url'); + expect(validUrl).toBe(true); + }); + + it('should return false for absolute path', () => { + const validUrl = isValidUrl('/some/absolute/path'); + expect(validUrl).toBe(false); + }); + + it('should return false for relative path', () => { + const validUrl = isValidUrl('../some/relative/path'); + expect(validUrl).toBe(false); + }); +}); diff --git a/packages/backend-app-api/src/lib/urls.ts b/packages/backend-app-api/src/lib/urls.ts new file mode 100644 index 0000000000..848cea25d9 --- /dev/null +++ b/packages/backend-app-api/src/lib/urls.ts @@ -0,0 +1,25 @@ +/* + * Copyright 2021 The Backstage Authors + * + * 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. + */ + +export function isValidUrl(url: string): boolean { + try { + // eslint-disable-next-line no-new + new URL(url); + return true; + } catch { + return false; + } +} From 3d5b5f89daeec92f912c89f148c07fd4ed9e1478 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 16:56:03 +0100 Subject: [PATCH 04/18] backend-common: forklift logging implementation to backend-app-api Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/package.json | 4 +++- .../src/logging/formats.ts | 0 .../{backend-common => backend-app-api}/src/logging/index.ts | 0 .../src/logging/loggerToWinstonLogger.ts | 0 .../src/logging/rootLogger.test.ts | 0 .../src/logging/rootLogger.ts | 0 .../src/logging/voidLogger.ts | 0 yarn.lock | 2 ++ 8 files changed, 5 insertions(+), 1 deletion(-) rename packages/{backend-common => backend-app-api}/src/logging/formats.ts (100%) rename packages/{backend-common => backend-app-api}/src/logging/index.ts (100%) rename packages/{backend-common => backend-app-api}/src/logging/loggerToWinstonLogger.ts (100%) rename packages/{backend-common => backend-app-api}/src/logging/rootLogger.test.ts (100%) rename packages/{backend-common => backend-app-api}/src/logging/rootLogger.ts (100%) rename packages/{backend-common => backend-app-api}/src/logging/voidLogger.ts (100%) diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 943db41972..e7551a1130 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -53,13 +53,15 @@ "fs-extra": "10.1.0", "helmet": "^6.0.0", "lodash": "^4.17.21", + "logform": "^2.3.2", "minimatch": "^5.0.0", "minimist": "^1.2.5", "morgan": "^1.10.0", "node-forge": "^1.3.1", "selfsigned": "^2.0.0", "stoppable": "^1.1.0", - "winston": "^3.2.1" + "winston": "^3.2.1", + "winston-transport": "^4.5.0" }, "devDependencies": { "@backstage/cli": "workspace:^", diff --git a/packages/backend-common/src/logging/formats.ts b/packages/backend-app-api/src/logging/formats.ts similarity index 100% rename from packages/backend-common/src/logging/formats.ts rename to packages/backend-app-api/src/logging/formats.ts diff --git a/packages/backend-common/src/logging/index.ts b/packages/backend-app-api/src/logging/index.ts similarity index 100% rename from packages/backend-common/src/logging/index.ts rename to packages/backend-app-api/src/logging/index.ts diff --git a/packages/backend-common/src/logging/loggerToWinstonLogger.ts b/packages/backend-app-api/src/logging/loggerToWinstonLogger.ts similarity index 100% rename from packages/backend-common/src/logging/loggerToWinstonLogger.ts rename to packages/backend-app-api/src/logging/loggerToWinstonLogger.ts diff --git a/packages/backend-common/src/logging/rootLogger.test.ts b/packages/backend-app-api/src/logging/rootLogger.test.ts similarity index 100% rename from packages/backend-common/src/logging/rootLogger.test.ts rename to packages/backend-app-api/src/logging/rootLogger.test.ts diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-app-api/src/logging/rootLogger.ts similarity index 100% rename from packages/backend-common/src/logging/rootLogger.ts rename to packages/backend-app-api/src/logging/rootLogger.ts diff --git a/packages/backend-common/src/logging/voidLogger.ts b/packages/backend-app-api/src/logging/voidLogger.ts similarity index 100% rename from packages/backend-common/src/logging/voidLogger.ts rename to packages/backend-app-api/src/logging/voidLogger.ts diff --git a/yarn.lock b/yarn.lock index 40d2189edc..d6822cad9d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3406,6 +3406,7 @@ __metadata: helmet: ^6.0.0 http-errors: ^2.0.0 lodash: ^4.17.21 + logform: ^2.3.2 minimatch: ^5.0.0 minimist: ^1.2.5 morgan: ^1.10.0 @@ -3414,6 +3415,7 @@ __metadata: stoppable: ^1.1.0 supertest: ^6.1.3 winston: ^3.2.1 + winston-transport: ^4.5.0 languageName: unknown linkType: soft From 669d811da8cba3a14fac000098fb8d651a9eab4c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 17:05:31 +0100 Subject: [PATCH 05/18] backend-app-api: copy over regex util from backend-common Signed-off-by: Patrik Oldsberg --- .../src/lib/escapeRegExp.test.ts | 84 +++++++++++++++++++ .../backend-app-api/src/lib/escapeRegExp.ts | 24 ++++++ .../backend-app-api/src/logging/rootLogger.ts | 2 +- 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 packages/backend-app-api/src/lib/escapeRegExp.test.ts create mode 100644 packages/backend-app-api/src/lib/escapeRegExp.ts diff --git a/packages/backend-app-api/src/lib/escapeRegExp.test.ts b/packages/backend-app-api/src/lib/escapeRegExp.test.ts new file mode 100644 index 0000000000..13c6ae4a5a --- /dev/null +++ b/packages/backend-app-api/src/lib/escapeRegExp.test.ts @@ -0,0 +1,84 @@ +/* + * Copyright 2021 The Backstage Authors + * + * 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 { escapeRegExp } from './escapeRegExp'; + +describe('escapeRegExp', () => { + test('does not escape non-regex characters', () => { + expect(escapeRegExp('Backstage Backstage')).toBe('Backstage Backstage'); + }); + + test('all the characters', () => { + expect(escapeRegExp('^$\\.*+?()[]{}|')).toBe( + '\\^\\$\\\\\\.\\*\\+\\?\\(\\)\\[\\]\\{\\}\\|', + ); + }); + + test('character: ^', () => { + expect(escapeRegExp('^')).toBe('\\^'); + }); + + test('character: $', () => { + expect(escapeRegExp('$')).toBe('\\$'); + }); + + test('character: \\', () => { + expect(escapeRegExp('\\')).toBe('\\\\'); + }); + + test('character: .', () => { + expect(escapeRegExp('.')).toBe('\\.'); + }); + + test('character: *', () => { + expect(escapeRegExp('*')).toBe('\\*'); + }); + + test('character: +', () => { + expect(escapeRegExp('+')).toBe('\\+'); + }); + + test('character: ?', () => { + expect(escapeRegExp('?')).toBe('\\?'); + }); + + test('character: (', () => { + expect(escapeRegExp('(')).toBe('\\('); + }); + + test('character: )', () => { + expect(escapeRegExp(')')).toBe('\\)'); + }); + + test('character: [', () => { + expect(escapeRegExp('[')).toBe('\\['); + }); + + test('character: ]', () => { + expect(escapeRegExp(']')).toBe('\\]'); + }); + + test('character: {', () => { + expect(escapeRegExp('{')).toBe('\\{'); + }); + + test('character: }', () => { + expect(escapeRegExp('}')).toBe('\\}'); + }); + + test('character: |', () => { + expect(escapeRegExp('|')).toBe('\\|'); + }); +}); diff --git a/packages/backend-app-api/src/lib/escapeRegExp.ts b/packages/backend-app-api/src/lib/escapeRegExp.ts new file mode 100644 index 0000000000..bc78967ebe --- /dev/null +++ b/packages/backend-app-api/src/lib/escapeRegExp.ts @@ -0,0 +1,24 @@ +/* + * Copyright 2021 The Backstage Authors + * + * 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. + */ + +/** + * Escapes a given string to be used inside a RegExp. + * + * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions + */ +export const escapeRegExp = (text: string) => { + return text.replace(/[.*+?^${}(\)|[\]\\]/g, '\\$&'); +}; diff --git a/packages/backend-app-api/src/logging/rootLogger.ts b/packages/backend-app-api/src/logging/rootLogger.ts index 6f6eff6289..ab7643b660 100644 --- a/packages/backend-app-api/src/logging/rootLogger.ts +++ b/packages/backend-app-api/src/logging/rootLogger.ts @@ -18,7 +18,7 @@ import { merge } from 'lodash'; import * as winston from 'winston'; import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; -import { escapeRegExp } from '../util/escapeRegExp'; +import { escapeRegExp } from '../lib/escapeRegExp'; let rootLogger: winston.Logger; let redactionRegExp: RegExp | undefined; From d54cd2b29850cc8d659cbbd6f7fd0b1037559158 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 17:26:02 +0100 Subject: [PATCH 06/18] backend-app-api: move global loggers and winston helper back to backend-common Signed-off-by: Patrik Oldsberg --- .../backend-app-api/src/logging/rootLogger.ts | 30 ------------------- .../src/logging/globalLoggers.ts} | 29 ++++++++++++++++++ .../src/logging/index.ts | 9 +----- .../src/logging/loggerToWinstonLogger.ts | 0 .../src/tokens/ServerTokenManager.test.ts | 2 +- 5 files changed, 31 insertions(+), 39 deletions(-) rename packages/{backend-app-api/src/logging/voidLogger.ts => backend-common/src/logging/globalLoggers.ts} (55%) rename packages/{backend-app-api => backend-common}/src/logging/index.ts (80%) rename packages/{backend-app-api => backend-common}/src/logging/loggerToWinstonLogger.ts (100%) diff --git a/packages/backend-app-api/src/logging/rootLogger.ts b/packages/backend-app-api/src/logging/rootLogger.ts index ab7643b660..37122754ba 100644 --- a/packages/backend-app-api/src/logging/rootLogger.ts +++ b/packages/backend-app-api/src/logging/rootLogger.ts @@ -20,36 +20,8 @@ import { LoggerOptions } from 'winston'; import { coloredFormat } from './formats'; import { escapeRegExp } from '../lib/escapeRegExp'; -let rootLogger: winston.Logger; let redactionRegExp: RegExp | undefined; -/** - * Gets the current root logger. - * - * @public - */ -export function getRootLogger(): winston.Logger { - return rootLogger; -} - -/** - * Sets a completely custom default "root" logger. - * - * @remarks - * - * This is the logger instance that will be the foundation for all other logger - * instances passed to plugins etc, in a given backend. - * - * Only use this if you absolutely need to make a completely custom logger. - * Normally if you want to make light adaptations to the default logger - * behavior, you would instead call {@link createRootLogger}. - * - * @public - */ -export function setRootLogger(newLogger: winston.Logger) { - rootLogger = newLogger; -} - export function setRootLoggerRedactionList(redactionList: string[]) { // Exclude secrets that are empty or just one character in length. These // typically mean that you are running local dev or tests, or using the @@ -127,5 +99,3 @@ export function createRootLogger( return logger; } - -rootLogger = createRootLogger(); diff --git a/packages/backend-app-api/src/logging/voidLogger.ts b/packages/backend-common/src/logging/globalLoggers.ts similarity index 55% rename from packages/backend-app-api/src/logging/voidLogger.ts rename to packages/backend-common/src/logging/globalLoggers.ts index 79077f5c96..b9ec04b654 100644 --- a/packages/backend-app-api/src/logging/voidLogger.ts +++ b/packages/backend-common/src/logging/globalLoggers.ts @@ -26,3 +26,32 @@ export function getVoidLogger(): winston.Logger { transports: [new winston.transports.Console({ silent: true })], }); } + +let rootLogger: winston.Logger = createRootLogger(); + +/** + * Gets the current root logger. + * + * @public + */ +export function getRootLogger(): winston.Logger { + return rootLogger; +} + +/** + * Sets a completely custom default "root" logger. + * + * @remarks + * + * This is the logger instance that will be the foundation for all other logger + * instances passed to plugins etc, in a given backend. + * + * Only use this if you absolutely need to make a completely custom logger. + * Normally if you want to make light adaptations to the default logger + * behavior, you would instead call {@link createRootLogger}. + * + * @public + */ +export function setRootLogger(newLogger: winston.Logger) { + rootLogger = newLogger; +} diff --git a/packages/backend-app-api/src/logging/index.ts b/packages/backend-common/src/logging/index.ts similarity index 80% rename from packages/backend-app-api/src/logging/index.ts rename to packages/backend-common/src/logging/index.ts index ff2315c51b..ed49b25b55 100644 --- a/packages/backend-app-api/src/logging/index.ts +++ b/packages/backend-common/src/logging/index.ts @@ -14,12 +14,5 @@ * limitations under the License. */ -export * from './formats'; -export { - createRootLogger, - getRootLogger, - setRootLogger, - redactWinstonLogLine, -} from './rootLogger'; -export * from './voidLogger'; +export { getRootLogger, getVoidLogger, setRootLogger } from './globalLoggers'; export { loggerToWinstonLogger } from './loggerToWinstonLogger'; diff --git a/packages/backend-app-api/src/logging/loggerToWinstonLogger.ts b/packages/backend-common/src/logging/loggerToWinstonLogger.ts similarity index 100% rename from packages/backend-app-api/src/logging/loggerToWinstonLogger.ts rename to packages/backend-common/src/logging/loggerToWinstonLogger.ts diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 3e5f0ba69a..3c487e9b9b 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -16,7 +16,7 @@ import { ConfigReader } from '@backstage/config'; import * as jose from 'jose'; -import { getVoidLogger } from '../logging/voidLogger'; +import { getVoidLogger } from '../logging'; import { ServerTokenManager } from './ServerTokenManager'; import { TokenManager } from './types'; From 851e6639b4c6b272fd6532855b106cee8821c462 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 17:39:05 +0100 Subject: [PATCH 07/18] backend-app-api: added WinstonLogger with color format Signed-off-by: Patrik Oldsberg --- .../src/logging/WinstonLogger.ts | 89 +++++++++++++++++++ .../backend-app-api/src/logging/formats.ts | 45 ---------- packages/backend-app-api/src/logging/index.ts | 17 ++++ 3 files changed, 106 insertions(+), 45 deletions(-) create mode 100644 packages/backend-app-api/src/logging/WinstonLogger.ts delete mode 100644 packages/backend-app-api/src/logging/formats.ts create mode 100644 packages/backend-app-api/src/logging/index.ts diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts new file mode 100644 index 0000000000..99c120f3ed --- /dev/null +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -0,0 +1,89 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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 { + LoggerService, + LogMeta, + RootLoggerService, +} from '@backstage/backend-plugin-api'; +import { Format, TransformableInfo } from 'logform'; +import { Logger, format } from 'winston'; + +/** + * @public + */ +export class WinstonLogger implements RootLoggerService { + #winston: Logger; + + static fromWinston(logger: Logger): WinstonLogger { + return new WinstonLogger(logger); + } + + static colorFormat(): Format { + const colorizer = format.colorize(); + + return format.combine( + format.timestamp(), + format.colorize({ + colors: { + timestamp: 'dim', + prefix: 'blue', + field: 'cyan', + debug: 'grey', + }, + }), + format.printf((info: TransformableInfo) => { + const { timestamp, level, message, plugin, service, ...fields } = info; + const prefix = plugin || service; + const timestampColor = colorizer.colorize('timestamp', timestamp); + const prefixColor = colorizer.colorize('prefix', prefix); + + const extraFields = Object.entries(fields) + .map( + ([key, value]) => + `${colorizer.colorize('field', `${key}`)}=${value}`, + ) + .join(' '); + + return `${timestampColor} ${prefixColor} ${level} ${message} ${extraFields}`; + }), + ); + } + + constructor(winston: Logger) { + this.#winston = winston; + } + + error(message: string, meta?: LogMeta): void { + this.#winston.error(message, meta); + } + + warn(message: string, meta?: LogMeta): void { + this.#winston.warn(message, meta); + } + + info(message: string, meta?: LogMeta): void { + this.#winston.info(message, meta); + } + + debug(message: string, meta?: LogMeta): void { + this.#winston.debug(message, meta); + } + + child(meta: LogMeta): LoggerService { + return new WinstonLogger(this.#winston.child(meta)); + } +} diff --git a/packages/backend-app-api/src/logging/formats.ts b/packages/backend-app-api/src/logging/formats.ts deleted file mode 100644 index 53eb55e790..0000000000 --- a/packages/backend-app-api/src/logging/formats.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * 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 * as winston from 'winston'; -import { TransformableInfo } from 'logform'; - -const coloredTemplate = (info: TransformableInfo) => { - const { timestamp, level, message, plugin, service, ...fields } = info; - const colorizer = winston.format.colorize(); - const prefix = plugin || service; - const timestampColor = colorizer.colorize('timestamp', timestamp); - const prefixColor = colorizer.colorize('prefix', prefix); - - const extraFields = Object.entries(fields) - .map(([key, value]) => `${colorizer.colorize('field', `${key}`)}=${value}`) - .join(' '); - - return `${timestampColor} ${prefixColor} ${level} ${message} ${extraFields}`; -}; - -/** - * A logging format that adds coloring to console output. - * - * @public - */ -export const coloredFormat = winston.format.combine( - winston.format.timestamp(), - winston.format.colorize({ - colors: { timestamp: 'dim', prefix: 'blue', field: 'cyan', debug: 'grey' }, - }), - winston.format.printf(coloredTemplate), -); diff --git a/packages/backend-app-api/src/logging/index.ts b/packages/backend-app-api/src/logging/index.ts new file mode 100644 index 0000000000..3e7d43ff50 --- /dev/null +++ b/packages/backend-app-api/src/logging/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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. + */ + +export { WinstonLogger } from './WinstonLogger'; From 6b59bd8c885a373f391ddf36c116e62503561771 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 21:13:32 +0100 Subject: [PATCH 08/18] backend-app-api: add creation with redaction for WinstonLogger Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/index.ts | 1 + .../src/logging/WinstonLogger.ts | 95 +++++++++- packages/backend-app-api/src/logging/index.ts | 1 + .../src/logging/rootLogger.test.ts | 164 ------------------ .../backend-app-api/src/logging/rootLogger.ts | 101 ----------- .../rootLogger/rootLoggerFactory.ts | 46 ++--- .../services/definitions/RootLoggerService.ts | 4 +- 7 files changed, 109 insertions(+), 303 deletions(-) delete mode 100644 packages/backend-app-api/src/logging/rootLogger.test.ts delete mode 100644 packages/backend-app-api/src/logging/rootLogger.ts diff --git a/packages/backend-app-api/src/index.ts b/packages/backend-app-api/src/index.ts index 9527e35919..14e7859818 100644 --- a/packages/backend-app-api/src/index.ts +++ b/packages/backend-app-api/src/index.ts @@ -21,5 +21,6 @@ */ export * from './http'; +export * from './logging'; export * from './wiring'; export * from './services/implementations'; diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts index 99c120f3ed..2557ca6366 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -20,18 +20,97 @@ import { RootLoggerService, } from '@backstage/backend-plugin-api'; import { Format, TransformableInfo } from 'logform'; -import { Logger, format } from 'winston'; +import { + Logger, + format, + createLogger, + transports, + transport as Transport, +} from 'winston'; /** * @public */ +export interface WinstonLoggerOptions { + meta?: LogMeta; + level: string; + format: Format; + transports: Transport[]; +} + +/** + * A {@link @backstage/backend-plugin-api#LoggerService} implementation based on winston. + * + * @public + */ export class WinstonLogger implements RootLoggerService { #winston: Logger; + #addRedactions?: RootLoggerService['addRedactions']; - static fromWinston(logger: Logger): WinstonLogger { - return new WinstonLogger(logger); + /** + * Creates a {@link WinstonLogger} instance. + */ + static create(options: WinstonLoggerOptions): WinstonLogger { + const redacter = WinstonLogger.redacter(); + + let logger = createLogger({ + level: options.level, + format: format.combine(redacter.format, options.format), + transports: options.transports ?? new transports.Console(), + }); + if (options.meta) { + logger = logger.child(options.meta); + } + + return new WinstonLogger(logger, redacter.add); } + /** + * Creates a winston log formatter for redacting secrets. + */ + static redacter(): { format: Format; add: (redactions: string[]) => void } { + const redactionSet = new Set(); + + let redactionPattern: RegExp | undefined = undefined; + + return { + format: format((info: TransformableInfo) => { + if (redactionPattern && typeof info.message === 'string') { + info.message = info.message.replace(redactionPattern, '[REDACTED]'); + } + return info; + })(), + add(newRedactions: string[]) { + let changed = false; + for (const redaction of newRedactions) { + // Exclude secrets that are empty or just one character in length. These + // typically mean that you are running local dev or tests, or using the + // --lax flag which sets things to just 'x'. + if (redaction.length <= 1) { + continue; + } + if (!redactionSet.has(redaction)) { + redactionSet.add(redaction); + changed = true; + } + } + if (changed) { + if (redactionSet.size > 0) { + redactionPattern = new RegExp( + `(${Array.from(redactionSet).join('|')})`, + 'g', + ); + } else { + redactionPattern = undefined; + } + } + }, + }; + } + + /** + * Creates a pretty printed winston log formatter. + */ static colorFormat(): Format { const colorizer = format.colorize(); @@ -63,8 +142,12 @@ export class WinstonLogger implements RootLoggerService { ); } - constructor(winston: Logger) { + private constructor( + winston: Logger, + addRedactions?: RootLoggerService['addRedactions'], + ) { this.#winston = winston; + this.#addRedactions = addRedactions; } error(message: string, meta?: LogMeta): void { @@ -86,4 +169,8 @@ export class WinstonLogger implements RootLoggerService { child(meta: LogMeta): LoggerService { return new WinstonLogger(this.#winston.child(meta)); } + + addRedactions(redactions: string[]): void { + this.#addRedactions?.(redactions); + } } diff --git a/packages/backend-app-api/src/logging/index.ts b/packages/backend-app-api/src/logging/index.ts index 3e7d43ff50..14fe33f898 100644 --- a/packages/backend-app-api/src/logging/index.ts +++ b/packages/backend-app-api/src/logging/index.ts @@ -15,3 +15,4 @@ */ export { WinstonLogger } from './WinstonLogger'; +export type { WinstonLoggerOptions } from './WinstonLogger'; diff --git a/packages/backend-app-api/src/logging/rootLogger.test.ts b/packages/backend-app-api/src/logging/rootLogger.test.ts deleted file mode 100644 index 0f2f581569..0000000000 --- a/packages/backend-app-api/src/logging/rootLogger.test.ts +++ /dev/null @@ -1,164 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * 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 * as winston from 'winston'; -import { - createRootLogger, - getRootLogger, - setRootLogger, - setRootLoggerRedactionList, -} from './rootLogger'; - -describe('rootLogger', () => { - it('can replace the default logger', () => { - const logger = winston.createLogger(); - jest.spyOn(logger, 'info').mockReturnValue(logger); - - setRootLogger(logger); - getRootLogger().info('testing'); - - expect(logger.info).toHaveBeenCalledWith( - expect.stringContaining('testing'), - ); - }); - - it('redacts given secrets', () => { - const transport = new winston.transports.Console(); - const logger = createRootLogger({ transports: [transport] }); - jest.spyOn(transport, 'write'); - setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', 'SECRET.3']); - logger.info('Logging SECRET-1 and SECRET_2 and SECRET.3'); - - expect(transport.write).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Logging [REDACTED] and [REDACTED] and [REDACTED]', - }), - ); - }); - - it('redacts but ignores empty and one-character secrets', () => { - const transport = new winston.transports.Console(); - const logger = createRootLogger({ transports: [transport] }); - jest.spyOn(transport, 'write'); - setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', 'Q', '']); - logger.info('Logging SECRET-1 and SECRET_2 and Q'); - - expect(transport.write).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Logging [REDACTED] and [REDACTED] and Q', - }), - ); - }); - - describe('createRootLogger', () => { - it('creates a new logger', () => { - const oldLogger = getRootLogger(); - const newLogger = createRootLogger(); - - expect(oldLogger).not.toBe(newLogger); - }); - - it('replaces the existing root logger', () => { - const oldLogger = getRootLogger(); - createRootLogger(); - const newLogger = getRootLogger(); - expect(oldLogger).not.toBe(newLogger); - }); - - it('can append additional default metadata', () => { - const format = winston.format.json(); - const logger = createRootLogger({ - format, - defaultMeta: { - appName: 'backstage', - appEnv: 'prod', - containerId: 'abc', - }, - }); - jest.spyOn(format, 'transform'); - - logger.info('testing'); - - expect(format.transform).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'testing', - service: 'backstage', - appName: 'backstage', - appEnv: 'prod', - containerId: 'abc', - }), - {}, - ); - }); - - it('can add override existing transports', () => { - const transport = new winston.transports.Console({ level: 'debug' }); - const logger = createRootLogger({ transports: [transport] }); - expect(logger.transports.length).toBe(1); - expect(logger.transports[0]).toBe(transport); - }); - - it('can append an additional transport', () => { - const logger = createRootLogger(); - const transport = new winston.transports.Console({ level: 'debug' }); - logger.add(transport); - expect(logger.transports.length).toBe(2); - expect(logger.transports[1]).toBe(transport); - expect(logger.transports[1].level).toBe('debug'); - }); - - it('can override default format', () => { - const format = winston.format(() => false)(); - const logger = createRootLogger({ format }); - expect( - logger.format.transform({ message: 'hello', level: 'info' }), - ).toBeFalsy(); - }); - - it('can override the service label', () => { - const transport = new winston.transports.Console(); - const logger = createRootLogger({ transports: [transport] }); - const writeSpy = jest - .spyOn(transport, 'write') - .mockImplementation((_c, _e) => true); - - logger.info('msg-a'); - logger.child({ service: 'b' }).info('msg-b'); - logger.info('msg-c', { service: 'c' }); - - expect(writeSpy.mock.calls).toEqual([ - [ - expect.objectContaining({ - message: 'msg-a', - service: 'backstage', - }), - ], - [ - expect.objectContaining({ - message: 'msg-b', - service: 'b', - }), - ], - [ - expect.objectContaining({ - message: 'msg-c', - service: 'c', - }), - ], - ]); - }); - }); -}); diff --git a/packages/backend-app-api/src/logging/rootLogger.ts b/packages/backend-app-api/src/logging/rootLogger.ts deleted file mode 100644 index 37122754ba..0000000000 --- a/packages/backend-app-api/src/logging/rootLogger.ts +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * 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 { merge } from 'lodash'; -import * as winston from 'winston'; -import { LoggerOptions } from 'winston'; -import { coloredFormat } from './formats'; -import { escapeRegExp } from '../lib/escapeRegExp'; - -let redactionRegExp: RegExp | undefined; - -export function setRootLoggerRedactionList(redactionList: string[]) { - // Exclude secrets that are empty or just one character in length. These - // typically mean that you are running local dev or tests, or using the - // --lax flag which sets things to just 'x'. So exclude those. - const filtered = redactionList.filter(r => r.length > 1); - - if (filtered.length) { - redactionRegExp = new RegExp( - `(${filtered.map(escapeRegExp).join('|')})`, - 'g', - ); - } else { - redactionRegExp = undefined; - } -} - -/** - * A winston formatting function that finds occurrences of filteredKeys - * and replaces them with the corresponding identifier. - * - * @public - */ -export function redactWinstonLogLine(info: winston.Logform.TransformableInfo) { - // TODO(hhogg): The logger is created before the config is loaded, because the - // logger is needed in the config loader. There is a risk of a secret being - // logged out during the config loading stage. - // TODO(freben): Added a check that info.message actually was a string, - // because it turned out that this was not necessarily guaranteed. - // https://github.com/backstage/backstage/issues/8306 - if (redactionRegExp && typeof info.message === 'string') { - info.message = info.message.replace(redactionRegExp, '[REDACTED]'); - } - - return info; -} - -/** - * Creates a default "root" logger. This also calls {@link setRootLogger} under - * the hood. - * - * @remarks - * - * This is the logger instance that will be the foundation for all other logger - * instances passed to plugins etc, in a given backend. - * - * @public - */ -export function createRootLogger( - options: winston.LoggerOptions = {}, - env = process.env, -): winston.Logger { - const logger = winston - .createLogger( - merge( - { - level: env.LOG_LEVEL || 'info', - format: winston.format.combine( - winston.format(redactWinstonLogLine)(), - env.NODE_ENV === 'production' - ? winston.format.json() - : coloredFormat, - ), - transports: [ - new winston.transports.Console({ - silent: env.JEST_WORKER_ID !== undefined && !env.LOG_LEVEL, - }), - ], - }, - options, - ), - ) - .child({ service: 'backstage' }); - - setRootLogger(logger); - - return logger; -} diff --git a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts index 54d40b6a12..33443613b6 100644 --- a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts @@ -14,48 +14,28 @@ * limitations under the License. */ -import { createRootLogger } from '@backstage/backend-common'; import { createServiceFactory, - LoggerService, coreServices, } from '@backstage/backend-plugin-api'; -import { LogMeta } from '@backstage/backend-plugin-api'; -import { Logger as WinstonLogger } from 'winston'; - -class BackstageLogger implements LoggerService { - static fromWinston(logger: WinstonLogger): BackstageLogger { - return new BackstageLogger(logger); - } - - private constructor(private readonly winston: WinstonLogger) {} - - error(message: string, meta?: LogMeta): void { - this.winston.error(message, meta); - } - - warn(message: string, meta?: LogMeta): void { - this.winston.warn(message, meta); - } - - info(message: string, meta?: LogMeta): void { - this.winston.info(message, meta); - } - - debug(message: string, meta?: LogMeta): void { - this.winston.debug(message, meta); - } - - child(meta: LogMeta): LoggerService { - return new BackstageLogger(this.winston.child(meta)); - } -} +import { WinstonLogger } from '../../../logging'; +import { transports, format } from 'winston'; /** @public */ export const rootLoggerFactory = createServiceFactory({ service: coreServices.rootLogger, deps: {}, async factory() { - return BackstageLogger.fromWinston(createRootLogger()); + return WinstonLogger.create({ + meta: { + service: 'backstage', + }, + level: process.env.LOG_LEVEL || 'info', + format: + process.env.NODE_ENV === 'production' + ? format.json() + : WinstonLogger.colorFormat(), + transports: [new transports.Console()], + }); }, }); diff --git a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts index ad318ef53f..11a12b4387 100644 --- a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts +++ b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts @@ -17,4 +17,6 @@ import { LoggerService } from './LoggerService'; /** @public */ -export interface RootLoggerService extends LoggerService {} +export interface RootLoggerService extends LoggerService { + addRedactions(redactions: string[]): void; +} From e4c3c82ecc056d4e9ee53ff017ab506ed33c7286 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 21:14:04 +0100 Subject: [PATCH 09/18] backend-common: reimplement createRootLogger using WinstonLogger Signed-off-by: Patrik Oldsberg --- .../src/logging/createRootLogger.ts | 79 +++++++++++++++++++ .../src/logging/globalLoggers.ts | 2 +- packages/backend-common/src/logging/index.ts | 1 + 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 packages/backend-common/src/logging/createRootLogger.ts diff --git a/packages/backend-common/src/logging/createRootLogger.ts b/packages/backend-common/src/logging/createRootLogger.ts new file mode 100644 index 0000000000..630a3015e1 --- /dev/null +++ b/packages/backend-common/src/logging/createRootLogger.ts @@ -0,0 +1,79 @@ +/* + * Copyright 2020 The Backstage Authors + * + * 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 { WinstonLogger } from '@backstage/backend-app-api'; +import { merge } from 'lodash'; +import * as winston from 'winston'; +import { LoggerOptions } from 'winston'; +import { setRootLogger } from './globalLoggers'; + +const redacter = WinstonLogger.redacter(); + +export const setRootLoggerRedactionList = redacter.add; + +/** + * A winston formatting function that finds occurrences of filteredKeys + * and replaces them with the corresponding identifier. + * + * @public + */ +export const redactWinstonLogLine: ( + info: winston.Logform.TransformableInfo, +) => winston.Logform.TransformableInfo | boolean = redacter.format.transform; + +/** + * Creates a default "root" logger. This also calls {@link setRootLogger} under + * the hood. + * + * @remarks + * + * This is the logger instance that will be the foundation for all other logger + * instances passed to plugins etc, in a given backend. + * + * @public + */ +export function createRootLogger( + options: winston.LoggerOptions = {}, + env = process.env, +): winston.Logger { + const logger = winston + .createLogger( + merge( + { + level: env.LOG_LEVEL || 'info', + format: winston.format.combine( + redacter.format, + env.NODE_ENV === 'production' + ? winston.format.json() + : WinstonLogger.colorFormat(), + ), + transports: [ + new winston.transports.Console({ + silent: env.JEST_WORKER_ID !== undefined && !env.LOG_LEVEL, + }), + ], + }, + options, + ), + ) + .child({ service: 'backstage' }); + + setRootLogger(logger); + + return logger; +} + +setRootLogger(createRootLogger()); diff --git a/packages/backend-common/src/logging/globalLoggers.ts b/packages/backend-common/src/logging/globalLoggers.ts index b9ec04b654..7567deb646 100644 --- a/packages/backend-common/src/logging/globalLoggers.ts +++ b/packages/backend-common/src/logging/globalLoggers.ts @@ -27,7 +27,7 @@ export function getVoidLogger(): winston.Logger { }); } -let rootLogger: winston.Logger = createRootLogger(); +let rootLogger: winston.Logger; /** * Gets the current root logger. diff --git a/packages/backend-common/src/logging/index.ts b/packages/backend-common/src/logging/index.ts index ed49b25b55..9a32885371 100644 --- a/packages/backend-common/src/logging/index.ts +++ b/packages/backend-common/src/logging/index.ts @@ -15,4 +15,5 @@ */ export { getRootLogger, getVoidLogger, setRootLogger } from './globalLoggers'; +export { createRootLogger } from './createRootLogger'; export { loggerToWinstonLogger } from './loggerToWinstonLogger'; From d2d17f0fab32aae799fd93c981664d106a611293 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:07:49 +0100 Subject: [PATCH 10/18] backend-app-api: hook up logger redactions and tweak surface a bit Signed-off-by: Patrik Oldsberg --- ....test.ts => ObservableConfigProxy.test.ts} | 2 +- packages/backend-app-api/src/config/config.ts | 82 ++++++++----------- packages/backend-app-api/src/config/index.ts | 17 ++++ packages/backend-app-api/src/index.ts | 1 + .../src/logging/WinstonLogger.ts | 29 ++++--- .../implementations/config/configFactory.ts | 38 +++++++-- .../services/implementations/config/index.ts | 1 + .../services/definitions/RootLoggerService.ts | 2 +- 8 files changed, 99 insertions(+), 73 deletions(-) rename packages/backend-app-api/src/config/{config.test.ts => ObservableConfigProxy.test.ts} (98%) create mode 100644 packages/backend-app-api/src/config/index.ts diff --git a/packages/backend-app-api/src/config/config.test.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts similarity index 98% rename from packages/backend-app-api/src/config/config.test.ts rename to packages/backend-app-api/src/config/ObservableConfigProxy.test.ts index 67db6996cd..e90225db1a 100644 --- a/packages/backend-app-api/src/config/config.test.ts +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts @@ -16,7 +16,7 @@ import { LoggerService } from '@backstage/backend-plugin-api'; import { ConfigReader } from '@backstage/config'; -import { ObservableConfigProxy } from './config'; +import { ObservableConfigProxy } from './ObservableConfigProxy'; describe('ObservableConfigProxy', () => { const errLogger = { diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index 0f07be85d2..6adebbdf52 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -21,47 +21,44 @@ import { findPaths } from '@backstage/cli-common'; import { loadConfigSchema, loadConfig, - ConfigSchema, ConfigTarget, LoadConfigOptionsRemote, } from '@backstage/config-loader'; -import { AppConfig, Config, ConfigReader } from '@backstage/config'; +import { Config, ConfigReader } from '@backstage/config'; import { getPackages } from '@manypkg/get-packages'; import { ObservableConfigProxy } from './ObservableConfigProxy'; import { isValidUrl } from '../lib/urls'; -import { setRootLoggerRedactionList } from './logging/rootLogger'; - -// Fetch the schema and get all the secrets to pass to the rootLogger for redaction -const updateRedactionList = ( - schema: ConfigSchema, - configs: AppConfig[], - logger: LoggerService, -) => { - const secretAppConfigs = schema.process(configs, { - visibility: ['secret'], - ignoreSchemaErrors: true, +/** @public */ +export async function createConfigSecretEnumerator(options: { + logger: LoggerService; + dir?: string; +}): Promise<(config: Config) => Iterable> { + const { logger, dir = process.cwd() } = options; + const { packages } = await getPackages(dir); + const schema = await loadConfigSchema({ + dependencies: packages.map(p => p.packageJson.name), }); - const secretConfig = ConfigReader.fromConfigs(secretAppConfigs); - const values = new Set(); - const data = secretConfig.get(); - JSON.parse( - JSON.stringify(data), - (_, v) => typeof v === 'string' && values.add(v), - ); - - logger.info( - `${values.size} secret${ - values.size > 1 ? 's' : '' - } found in the config which will be redacted`, - ); - - setRootLoggerRedactionList(Array.from(values)); -}; - -// A global used to ensure that only a single file watcher is active at a time. -let currentCancelFunc: () => void; + return (config: Config) => { + const [secretsData] = schema.process( + [{ data: config.get(), context: 'schema-enumerator' }], + { + visibility: ['secret'], + ignoreSchemaErrors: true, + }, + ); + const secrets = new Set(); + JSON.parse( + JSON.stringify(secretsData), + (_, v) => typeof v === 'string' && secrets.add(v), + ); + logger.info( + `Found ${secrets.size} new secrets in config that will be redacted`, + ); + return secrets; + }; +} /** * Load configuration for a Backend. @@ -75,7 +72,7 @@ export async function loadBackendConfig(options: { // process.argv or any other overrides remote?: LoadConfigOptionsRemote; argv: string[]; -}): Promise { +}): Promise<{ config: Config }> { const args = parseArgs(options.argv); const configTargets: ConfigTarget[] = [args.config ?? []] @@ -85,13 +82,7 @@ export async function loadBackendConfig(options: { /* eslint-disable-next-line no-restricted-syntax */ const paths = findPaths(__dirname); - // TODO(hhogg): This is fetching _all_ of the packages of the monorepo - // in order to find the secrets for redactions, however we only care about - // the backend ones, we need to find a way to exclude the frontend packages. - const { packages } = await getPackages(paths.targetDir); - const schema = await loadConfigSchema({ - dependencies: packages.map(p => p.packageJson.name), - }); + let currentCancelFunc: (() => void) | undefined = undefined; const config = new ObservableConfigProxy(options.logger); const { appConfigs } = await loadConfig({ @@ -112,7 +103,8 @@ export async function loadBackendConfig(options: { } currentCancelFunc = resolve; - // For reloads of this module we need to use a dispose handler rather than the global. + // TODO(Rugvip): We keep this here for now to avoid breaking the old system + // since this is re-used in backend-common if (module.hot) { module.hot.addDisposeHandler(resolve); } @@ -126,11 +118,5 @@ export async function loadBackendConfig(options: { config.setConfig(ConfigReader.fromConfigs(appConfigs)); - // Subscribe to config changes and update the redaction list for logging - updateRedactionList(schema, appConfigs, options.logger); - config.subscribe(() => - updateRedactionList(schema, appConfigs, options.logger), - ); - - return config; + return { config }; } diff --git a/packages/backend-app-api/src/config/index.ts b/packages/backend-app-api/src/config/index.ts new file mode 100644 index 0000000000..0ed8d2bc93 --- /dev/null +++ b/packages/backend-app-api/src/config/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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. + */ + +export { loadBackendConfig, createConfigSecretEnumerator } from './config'; diff --git a/packages/backend-app-api/src/index.ts b/packages/backend-app-api/src/index.ts index 14e7859818..13864a3470 100644 --- a/packages/backend-app-api/src/index.ts +++ b/packages/backend-app-api/src/index.ts @@ -20,6 +20,7 @@ * @packageDocumentation */ +export * from './config'; export * from './http'; export * from './logging'; export * from './wiring'; diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts index 2557ca6366..706e3cf7a7 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -68,20 +68,23 @@ export class WinstonLogger implements RootLoggerService { /** * Creates a winston log formatter for redacting secrets. */ - static redacter(): { format: Format; add: (redactions: string[]) => void } { + static redacter(): { + format: Format; + add: (redactions: Iterable) => void; + } { const redactionSet = new Set(); let redactionPattern: RegExp | undefined = undefined; return { - format: format((info: TransformableInfo) => { + format: format(info => { if (redactionPattern && typeof info.message === 'string') { info.message = info.message.replace(redactionPattern, '[REDACTED]'); } return info; })(), - add(newRedactions: string[]) { - let changed = false; + add(newRedactions) { + let added = 0; for (const redaction of newRedactions) { // Exclude secrets that are empty or just one character in length. These // typically mean that you are running local dev or tests, or using the @@ -91,18 +94,14 @@ export class WinstonLogger implements RootLoggerService { } if (!redactionSet.has(redaction)) { redactionSet.add(redaction); - changed = true; + added += 1; } } - if (changed) { - if (redactionSet.size > 0) { - redactionPattern = new RegExp( - `(${Array.from(redactionSet).join('|')})`, - 'g', - ); - } else { - redactionPattern = undefined; - } + if (added > 0) { + redactionPattern = new RegExp( + `(${Array.from(redactionSet).join('|')})`, + 'g', + ); } }, }; @@ -170,7 +169,7 @@ export class WinstonLogger implements RootLoggerService { return new WinstonLogger(this.#winston.child(meta)); } - addRedactions(redactions: string[]): void { + addRedactions(redactions: string[]) { this.#addRedactions?.(redactions); } } diff --git a/packages/backend-app-api/src/services/implementations/config/configFactory.ts b/packages/backend-app-api/src/services/implementations/config/configFactory.ts index 6bb84aa5a1..b0f5a17f3f 100644 --- a/packages/backend-app-api/src/services/implementations/config/configFactory.ts +++ b/packages/backend-app-api/src/services/implementations/config/configFactory.ts @@ -14,14 +14,28 @@ * limitations under the License. */ -import { - loadBackendConfig, - loggerToWinstonLogger, -} from '@backstage/backend-common'; import { coreServices, createServiceFactory, } from '@backstage/backend-plugin-api'; +import { LoadConfigOptionsRemote } from '@backstage/config-loader'; +import { + createConfigSecretEnumerator, + loadBackendConfig, +} from '../../../config'; + +/** @public */ +export interface ConfigFactoryOptions { + /** + * Process arguments to use instead of the default `process.argv()`. + */ + argv?: string[]; + + /** + * Enables and sets options for remote configuration loading. + */ + remote?: LoadConfigOptionsRemote; +} /** @public */ export const configFactory = createServiceFactory({ @@ -29,11 +43,19 @@ export const configFactory = createServiceFactory({ deps: { logger: coreServices.rootLogger, }, - async factory({ logger }) { - const config = await loadBackendConfig({ - argv: process.argv, - logger: loggerToWinstonLogger(logger), + async factory({ logger }, options?: ConfigFactoryOptions) { + const argv = options?.argv ?? process.argv; + const secretEnumerator = await createConfigSecretEnumerator({ logger }); + + const { config } = await loadBackendConfig({ + argv, + logger, + remote: options?.remote, }); + + logger.addRedactions(secretEnumerator(config)); + config.subscribe?.(() => logger.addRedactions(secretEnumerator(config))); + return config; }, }); diff --git a/packages/backend-app-api/src/services/implementations/config/index.ts b/packages/backend-app-api/src/services/implementations/config/index.ts index a9019b5b1c..b23c47ce0a 100644 --- a/packages/backend-app-api/src/services/implementations/config/index.ts +++ b/packages/backend-app-api/src/services/implementations/config/index.ts @@ -15,3 +15,4 @@ */ export { configFactory } from './configFactory'; +export type { ConfigFactoryOptions } from './configFactory'; diff --git a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts index 11a12b4387..a6840f48c0 100644 --- a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts +++ b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts @@ -18,5 +18,5 @@ import { LoggerService } from './LoggerService'; /** @public */ export interface RootLoggerService extends LoggerService { - addRedactions(redactions: string[]): void; + addRedactions(redactions: Iterable): void; } From 2041e78c9ccfaabb098d53b23ecca05c99f8492a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:10:50 +0100 Subject: [PATCH 11/18] backend-common: add back config api wrapper Signed-off-by: Patrik Oldsberg --- packages/backend-common/src/config.ts | 50 +++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 packages/backend-common/src/config.ts diff --git a/packages/backend-common/src/config.ts b/packages/backend-common/src/config.ts new file mode 100644 index 0000000000..dd96a6b46a --- /dev/null +++ b/packages/backend-common/src/config.ts @@ -0,0 +1,50 @@ +/* + * Copyright 2020 The Backstage Authors + * + * 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 { + createConfigSecretEnumerator, + loadBackendConfig as newLoadBackendConfig, +} from '@backstage/backend-app-api'; +import { LoggerService } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; +import { LoadConfigOptionsRemote } from '@backstage/config-loader'; +import { setRootLoggerRedactionList } from './logging/createRootLogger'; + +/** + * Load configuration for a Backend. + * + * This function should only be called once, during the initialization of the backend. + * + * @public + */ +export async function loadBackendConfig(options: { + logger: LoggerService; + // process.argv or any other overrides + remote?: LoadConfigOptionsRemote; + argv: string[]; +}): Promise { + const secretEnumerator = await createConfigSecretEnumerator({ + logger: options.logger, + }); + const { config } = await newLoadBackendConfig(options); + + setRootLoggerRedactionList(secretEnumerator(config)); + config.subscribe?.(() => + setRootLoggerRedactionList(secretEnumerator(config)), + ); + + return config; +} From 0fb7cc0130199b79176521e3fc0ffda9c04b7ce7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:14:20 +0100 Subject: [PATCH 12/18] backend-common: update exports for backwards compatibility Signed-off-by: Patrik Oldsberg --- .../backend-common/src/logging/createRootLogger.ts | 13 +++++++++++-- packages/backend-common/src/logging/index.ts | 6 +++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/backend-common/src/logging/createRootLogger.ts b/packages/backend-common/src/logging/createRootLogger.ts index 630a3015e1..2e6fe20525 100644 --- a/packages/backend-common/src/logging/createRootLogger.ts +++ b/packages/backend-common/src/logging/createRootLogger.ts @@ -30,9 +30,18 @@ export const setRootLoggerRedactionList = redacter.add; * * @public */ -export const redactWinstonLogLine: ( +export function redactWinstonLogLine( info: winston.Logform.TransformableInfo, -) => winston.Logform.TransformableInfo | boolean = redacter.format.transform; +): winston.Logform.TransformableInfo { + return redacter.format.transform(info) as winston.Logform.TransformableInfo; +} + +/** + * Creates a pretty printed winston log formatter. + * + * @public + */ +export const coloredFormat = WinstonLogger.colorFormat(); /** * Creates a default "root" logger. This also calls {@link setRootLogger} under diff --git a/packages/backend-common/src/logging/index.ts b/packages/backend-common/src/logging/index.ts index 9a32885371..e6b29afa15 100644 --- a/packages/backend-common/src/logging/index.ts +++ b/packages/backend-common/src/logging/index.ts @@ -15,5 +15,9 @@ */ export { getRootLogger, getVoidLogger, setRootLogger } from './globalLoggers'; -export { createRootLogger } from './createRootLogger'; +export { + createRootLogger, + redactWinstonLogLine, + coloredFormat, +} from './createRootLogger'; export { loggerToWinstonLogger } from './loggerToWinstonLogger'; From d3c89d2ec9f4be39ec6cd2202ac04a540daf742b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:14:49 +0100 Subject: [PATCH 13/18] updated API reports for config and logger move to backend-app-api Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 59 +++++++++++++++++++++++ packages/backend-plugin-api/api-report.md | 5 +- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 199b0863de..03d1ecc2be 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -12,13 +12,16 @@ import { CorsOptions } from 'cors'; import { ErrorRequestHandler } from 'express'; import { Express as Express_2 } from 'express'; import { ExtensionPoint } from '@backstage/backend-plugin-api'; +import { Format } from 'logform'; import { Handler } from 'express'; import { HelmetOptions } from 'helmet'; import * as http from 'http'; import { HttpRouterService } from '@backstage/backend-plugin-api'; import { IdentityService } from '@backstage/backend-plugin-api'; import { LifecycleService } from '@backstage/backend-plugin-api'; +import { LoadConfigOptionsRemote } from '@backstage/config-loader'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { LogMeta } from '@backstage/backend-plugin-api'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { PluginCacheManager } from '@backstage/backend-common'; import { PluginDatabaseManager } from '@backstage/backend-common'; @@ -33,6 +36,7 @@ import { ServiceFactory } from '@backstage/backend-plugin-api'; import { ServiceFactoryOrFunction } from '@backstage/backend-plugin-api'; import { ServiceRef } from '@backstage/backend-plugin-api'; import { TokenManagerService } from '@backstage/backend-plugin-api'; +import { transport } from 'winston'; import { UrlReader } from '@backstage/backend-common'; // @public (undocumented) @@ -51,6 +55,18 @@ export const cacheFactory: () => ServiceFactory; // @public (undocumented) export const configFactory: () => ServiceFactory; +// @public (undocumented) +export interface ConfigFactoryOptions { + argv?: string[]; + remote?: LoadConfigOptionsRemote; +} + +// @public (undocumented) +export function createConfigSecretEnumerator(options: { + logger: LoggerService; + dir?: string; +}): Promise<(config: Config) => Iterable>; + // @public export function createHttpServer( listener: RequestListener, @@ -147,6 +163,15 @@ export type IdentityFactoryOptions = { // @public export const lifecycleFactory: () => ServiceFactory; +// @public +export function loadBackendConfig(options: { + logger: LoggerService; + remote?: LoadConfigOptionsRemote; + argv: string[]; +}): Promise<{ + config: Config; +}>; + // @public (undocumented) export const loggerFactory: () => ServiceFactory; @@ -231,4 +256,38 @@ export const tokenManagerFactory: () => ServiceFactory; // @public (undocumented) export const urlReaderFactory: () => ServiceFactory; + +// @public +export class WinstonLogger implements RootLoggerService { + // (undocumented) + addRedactions(redactions: string[]): void; + // (undocumented) + child(meta: LogMeta): LoggerService; + static colorFormat(): Format; + static create(options: WinstonLoggerOptions): WinstonLogger; + // (undocumented) + debug(message: string, meta?: LogMeta): void; + // (undocumented) + error(message: string, meta?: LogMeta): void; + // (undocumented) + info(message: string, meta?: LogMeta): void; + static redacter(): { + format: Format; + add: (redactions: Iterable) => void; + }; + // (undocumented) + warn(message: string, meta?: LogMeta): void; +} + +// @public (undocumented) +export interface WinstonLoggerOptions { + // (undocumented) + format: Format; + // (undocumented) + level: string; + // (undocumented) + meta?: LogMeta; + // (undocumented) + transports: transport[]; +} ``` diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 33190277c9..408908ca71 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -282,7 +282,10 @@ export interface RootHttpRouterService { export interface RootLifecycleService extends LifecycleService {} // @public (undocumented) -export interface RootLoggerService extends LoggerService {} +export interface RootLoggerService extends LoggerService { + // (undocumented) + addRedactions(redactions: Iterable): void; +} // @public (undocumented) export interface SchedulerService extends PluginTaskScheduler {} From 0e63aab3110ee3503651a8f9f7f1a3aaa7151ea8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:18:26 +0100 Subject: [PATCH 14/18] added changesets for config and logger move to backend-app-api Signed-off-by: Patrik Oldsberg --- .changeset/brave-chicken-thank.md | 5 +++++ .changeset/shaggy-buses-rescue.md | 5 +++++ .changeset/twelve-fans-own.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/brave-chicken-thank.md create mode 100644 .changeset/shaggy-buses-rescue.md create mode 100644 .changeset/twelve-fans-own.md diff --git a/.changeset/brave-chicken-thank.md b/.changeset/brave-chicken-thank.md new file mode 100644 index 0000000000..1278e38068 --- /dev/null +++ b/.changeset/brave-chicken-thank.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Internal refactor of the logger and configuration loading implementations. diff --git a/.changeset/shaggy-buses-rescue.md b/.changeset/shaggy-buses-rescue.md new file mode 100644 index 0000000000..ed038c6c12 --- /dev/null +++ b/.changeset/shaggy-buses-rescue.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Moved over logging and configuration loading implementations from `@backstage/backend-common`. There is a now `WinstonLogger` which implements the `RootLoggerService` through Winston with accompanying utilities. For configuration the `loadBackendConfig` function has been moved over, but it now instead returns an object with a `config` property. diff --git a/.changeset/twelve-fans-own.md b/.changeset/twelve-fans-own.md new file mode 100644 index 0000000000..71139f73d7 --- /dev/null +++ b/.changeset/twelve-fans-own.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': patch +--- + +Updated the `RootLoggerService` to also have an `addRedactions` method. From 4d2b87c44b05db3d4a0cf26e24f3b70886f9009b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 11 Jan 2023 10:47:56 +0100 Subject: [PATCH 15/18] backend-app-api: flip around config and logger dependency Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/api-report.md | 3 +-- .../src/config/ObservableConfigProxy.test.ts | 13 +++------- .../src/config/ObservableConfigProxy.ts | 9 ++++--- packages/backend-app-api/src/config/config.ts | 8 +++---- .../src/logging/WinstonLogger.ts | 6 ++--- .../implementations/config/configFactory.ts | 24 ++++--------------- .../rootLogger/rootLoggerFactory.ts | 15 +++++++++--- packages/backend-plugin-api/api-report.md | 5 +--- .../services/definitions/RootLoggerService.ts | 4 +--- 9 files changed, 33 insertions(+), 54 deletions(-) diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 03d1ecc2be..bcf746aa5a 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -165,7 +165,6 @@ export const lifecycleFactory: () => ServiceFactory; // @public export function loadBackendConfig(options: { - logger: LoggerService; remote?: LoadConfigOptionsRemote; argv: string[]; }): Promise<{ @@ -260,7 +259,7 @@ export const urlReaderFactory: () => ServiceFactory; // @public export class WinstonLogger implements RootLoggerService { // (undocumented) - addRedactions(redactions: string[]): void; + addRedactions(redactions: Iterable): void; // (undocumented) child(meta: LogMeta): LoggerService; static colorFormat(): Format; diff --git a/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts index e90225db1a..876f5ac7d3 100644 --- a/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.test.ts @@ -14,19 +14,12 @@ * limitations under the License. */ -import { LoggerService } from '@backstage/backend-plugin-api'; import { ConfigReader } from '@backstage/config'; import { ObservableConfigProxy } from './ObservableConfigProxy'; describe('ObservableConfigProxy', () => { - const errLogger = { - error: (message: string) => { - throw new Error(message); - }, - } as unknown as LoggerService; - it('should notify subscribers', () => { - const config = new ObservableConfigProxy(errLogger); + const config = new ObservableConfigProxy(); const fn = jest.fn(); const sub = config.subscribe(fn); @@ -51,7 +44,7 @@ describe('ObservableConfigProxy', () => { }); it('should forward subscriptions', () => { - const config1 = new ObservableConfigProxy(errLogger); + const config1 = new ObservableConfigProxy(); const fn1 = jest.fn(); const fn2 = jest.fn(); @@ -119,7 +112,7 @@ describe('ObservableConfigProxy', () => { }); it('should make sub configs available as expected', () => { - const config = new ObservableConfigProxy(errLogger); + const config = new ObservableConfigProxy(); config.setConfig(new ConfigReader({ a: { x: 1 } })); diff --git a/packages/backend-app-api/src/config/ObservableConfigProxy.ts b/packages/backend-app-api/src/config/ObservableConfigProxy.ts index 8fcc3f060f..dd3334c9a5 100644 --- a/packages/backend-app-api/src/config/ObservableConfigProxy.ts +++ b/packages/backend-app-api/src/config/ObservableConfigProxy.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ConfigService, LoggerService } from '@backstage/backend-plugin-api'; +import { ConfigService } from '@backstage/backend-plugin-api'; import { ConfigReader } from '@backstage/config'; import { JsonValue } from '@backstage/types'; @@ -24,7 +24,6 @@ export class ObservableConfigProxy implements ConfigService { private readonly subscribers: (() => void)[] = []; constructor( - private readonly logger: LoggerService, private readonly parent?: ObservableConfigProxy, private parentKey?: string, ) { @@ -42,7 +41,7 @@ export class ObservableConfigProxy implements ConfigService { try { subscriber(); } catch (error) { - this.logger.error(`Config subscriber threw error, ${error}`); + console.error(`Config subscriber threw error, ${error}`); } } } @@ -89,11 +88,11 @@ export class ObservableConfigProxy implements ConfigService { return this.select(false)?.getOptional(key); } getConfig(key: string): ConfigService { - return new ObservableConfigProxy(this.logger, this, key); + return new ObservableConfigProxy(this, key); } getOptionalConfig(key: string): ConfigService | undefined { if (this.select(false)?.has(key)) { - return new ObservableConfigProxy(this.logger, this, key); + return new ObservableConfigProxy(this, key); } return undefined; } diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index 6adebbdf52..b1da4fb85b 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -68,8 +68,6 @@ export async function createConfigSecretEnumerator(options: { * @public */ export async function loadBackendConfig(options: { - logger: LoggerService; - // process.argv or any other overrides remote?: LoadConfigOptionsRemote; argv: string[]; }): Promise<{ config: Config }> { @@ -84,14 +82,14 @@ export async function loadBackendConfig(options: { let currentCancelFunc: (() => void) | undefined = undefined; - const config = new ObservableConfigProxy(options.logger); + const config = new ObservableConfigProxy(); const { appConfigs } = await loadConfig({ configRoot: paths.targetRoot, configTargets: configTargets, remote: options.remote, watch: { onChange(newConfigs) { - options.logger.info( + console.info( `Reloaded config from ${newConfigs.map(c => c.context).join(', ')}`, ); @@ -112,7 +110,7 @@ export async function loadBackendConfig(options: { }, }); - options.logger.info( + console.info( `Loaded config from ${appConfigs.map(c => c.context).join(', ')}`, ); diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts index 706e3cf7a7..cfee65d053 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -45,7 +45,7 @@ export interface WinstonLoggerOptions { */ export class WinstonLogger implements RootLoggerService { #winston: Logger; - #addRedactions?: RootLoggerService['addRedactions']; + #addRedactions?: (redactions: Iterable) => void; /** * Creates a {@link WinstonLogger} instance. @@ -143,7 +143,7 @@ export class WinstonLogger implements RootLoggerService { private constructor( winston: Logger, - addRedactions?: RootLoggerService['addRedactions'], + addRedactions?: (redactions: Iterable) => void, ) { this.#winston = winston; this.#addRedactions = addRedactions; @@ -169,7 +169,7 @@ export class WinstonLogger implements RootLoggerService { return new WinstonLogger(this.#winston.child(meta)); } - addRedactions(redactions: string[]) { + addRedactions(redactions: Iterable) { this.#addRedactions?.(redactions); } } diff --git a/packages/backend-app-api/src/services/implementations/config/configFactory.ts b/packages/backend-app-api/src/services/implementations/config/configFactory.ts index b0f5a17f3f..0ec15c29fe 100644 --- a/packages/backend-app-api/src/services/implementations/config/configFactory.ts +++ b/packages/backend-app-api/src/services/implementations/config/configFactory.ts @@ -19,10 +19,7 @@ import { createServiceFactory, } from '@backstage/backend-plugin-api'; import { LoadConfigOptionsRemote } from '@backstage/config-loader'; -import { - createConfigSecretEnumerator, - loadBackendConfig, -} from '../../../config'; +import { loadBackendConfig } from '../../../config'; /** @public */ export interface ConfigFactoryOptions { @@ -40,22 +37,11 @@ export interface ConfigFactoryOptions { /** @public */ export const configFactory = createServiceFactory({ service: coreServices.config, - deps: { - logger: coreServices.rootLogger, - }, - async factory({ logger }, options?: ConfigFactoryOptions) { - const argv = options?.argv ?? process.argv; - const secretEnumerator = await createConfigSecretEnumerator({ logger }); - - const { config } = await loadBackendConfig({ - argv, - logger, - remote: options?.remote, - }); - - logger.addRedactions(secretEnumerator(config)); - config.subscribe?.(() => logger.addRedactions(secretEnumerator(config))); + deps: {}, + async factory({}, options?: ConfigFactoryOptions) { + const { argv = process.argv, remote } = options ?? {}; + const { config } = await loadBackendConfig({ argv, remote }); return config; }, }); diff --git a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts index 33443613b6..1c2214b98c 100644 --- a/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts +++ b/packages/backend-app-api/src/services/implementations/rootLogger/rootLoggerFactory.ts @@ -20,13 +20,16 @@ import { } from '@backstage/backend-plugin-api'; import { WinstonLogger } from '../../../logging'; import { transports, format } from 'winston'; +import { createConfigSecretEnumerator } from '../../../config'; /** @public */ export const rootLoggerFactory = createServiceFactory({ service: coreServices.rootLogger, - deps: {}, - async factory() { - return WinstonLogger.create({ + deps: { + config: coreServices.config, + }, + async factory({ config }) { + const logger = WinstonLogger.create({ meta: { service: 'backstage', }, @@ -37,5 +40,11 @@ export const rootLoggerFactory = createServiceFactory({ : WinstonLogger.colorFormat(), transports: [new transports.Console()], }); + + const secretEnumerator = await createConfigSecretEnumerator({ logger }); + logger.addRedactions(secretEnumerator(config)); + config.subscribe?.(() => logger.addRedactions(secretEnumerator(config))); + + return logger; }, }); diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 408908ca71..33190277c9 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -282,10 +282,7 @@ export interface RootHttpRouterService { export interface RootLifecycleService extends LifecycleService {} // @public (undocumented) -export interface RootLoggerService extends LoggerService { - // (undocumented) - addRedactions(redactions: Iterable): void; -} +export interface RootLoggerService extends LoggerService {} // @public (undocumented) export interface SchedulerService extends PluginTaskScheduler {} diff --git a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts index a6840f48c0..ad318ef53f 100644 --- a/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts +++ b/packages/backend-plugin-api/src/services/definitions/RootLoggerService.ts @@ -17,6 +17,4 @@ import { LoggerService } from './LoggerService'; /** @public */ -export interface RootLoggerService extends LoggerService { - addRedactions(redactions: Iterable): void; -} +export interface RootLoggerService extends LoggerService {} From f4444b1dfa4bc62fe363d141054a323e43612e01 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 12 Jan 2023 19:58:35 +0100 Subject: [PATCH 16/18] backend-app-api: avoid requiring config data for secrets enumerator Signed-off-by: Patrik Oldsberg --- packages/backend-app-api/src/config/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-app-api/src/config/config.ts b/packages/backend-app-api/src/config/config.ts index b1da4fb85b..d3349b41ef 100644 --- a/packages/backend-app-api/src/config/config.ts +++ b/packages/backend-app-api/src/config/config.ts @@ -42,7 +42,7 @@ export async function createConfigSecretEnumerator(options: { return (config: Config) => { const [secretsData] = schema.process( - [{ data: config.get(), context: 'schema-enumerator' }], + [{ data: config.getOptional() ?? {}, context: 'schema-enumerator' }], { visibility: ['secret'], ignoreSchemaErrors: true, From 08121b62c54afeba28e1379fa1c9525f7bde5bfc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Jan 2023 00:58:47 +0100 Subject: [PATCH 17/18] backend-test-utils: add mock root logger service Signed-off-by: Patrik Oldsberg --- .../implementations/mockRootLoggerService.ts | 90 +++++++++++++++++++ .../src/next/wiring/TestBackend.ts | 4 +- 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 packages/backend-test-utils/src/next/implementations/mockRootLoggerService.ts diff --git a/packages/backend-test-utils/src/next/implementations/mockRootLoggerService.ts b/packages/backend-test-utils/src/next/implementations/mockRootLoggerService.ts new file mode 100644 index 0000000000..f5421e273b --- /dev/null +++ b/packages/backend-test-utils/src/next/implementations/mockRootLoggerService.ts @@ -0,0 +1,90 @@ +/* + * Copyright 2023 The Backstage Authors + * + * 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 { + coreServices, + createServiceFactory, + LoggerService, + LogMeta, + RootLoggerService, +} from '@backstage/backend-plugin-api'; + +interface MockLoggerOptions { + levels: + | boolean + | { error: boolean; warn: boolean; info: boolean; debug: boolean }; +} + +class MockLogger implements RootLoggerService { + #levels: Exclude; + #meta: LogMeta; + + error(message: string, meta?: LogMeta | Error | undefined): void { + this.#log('error', message, meta); + } + + warn(message: string, meta?: LogMeta | Error | undefined): void { + this.#log('warn', message, meta); + } + + info(message: string, meta?: LogMeta | Error | undefined): void { + this.#log('info', message, meta); + } + + debug(message: string, meta?: LogMeta | Error | undefined): void { + this.#log('debug', message, meta); + } + + child(meta: LogMeta): LoggerService { + return new MockLogger(this.#levels, { ...this.#meta, ...meta }); + } + + constructor(levels: MockLoggerOptions['levels'], meta: LogMeta) { + if (typeof levels === 'boolean') { + this.#levels = { + error: levels, + debug: levels, + info: levels, + warn: levels, + }; + } else { + this.#levels = levels; + } + this.#meta = meta; + } + + #log( + level: 'error' | 'warn' | 'info' | 'debug', + message: string, + meta?: LogMeta | Error | undefined, + ) { + if (this.#levels[level]) { + const labels = Object.entries(this.#meta) + .map(([key, value]) => `${key}=${value}`) + .join(','); + console[level](`${labels} ${message}`, meta); + } + } +} + +/** @public */ +export const mockRootLoggerService = createServiceFactory({ + service: coreServices.rootLogger, + deps: {}, + async factory(_deps) { + return new MockLogger(false, {}); + }, +}); diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 8fe489750a..21650125d3 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -20,7 +20,6 @@ import { lifecycleFactory, rootLifecycleFactory, loggerFactory, - rootLoggerFactory, cacheFactory, permissionsFactory, schedulerFactory, @@ -43,6 +42,7 @@ import { } from '@backstage/backend-plugin-api'; import { mockConfigFactory } from '../implementations/mockConfigService'; +import { mockRootLoggerService } from '../implementations/mockRootLoggerService'; import { mockTokenManagerFactory } from '../implementations/mockTokenManagerService'; import { ConfigReader } from '@backstage/config'; import express from 'express'; @@ -89,10 +89,10 @@ const defaultServiceFactories = [ lifecycleFactory(), loggerFactory(), mockConfigFactory(), + mockRootLoggerService(), mockTokenManagerFactory(), permissionsFactory(), rootLifecycleFactory(), - rootLoggerFactory(), schedulerFactory(), urlReaderFactory(), ]; From d9fe3f68be40782b69d19dbe6a27b3b9dee2713c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 13 Jan 2023 00:59:15 +0100 Subject: [PATCH 18/18] app-backend: avoid installing real services that break tests Signed-off-by: Patrik Oldsberg --- plugins/app-backend/src/service/appPlugin.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/plugins/app-backend/src/service/appPlugin.test.ts b/plugins/app-backend/src/service/appPlugin.test.ts index 7c57a5d685..518f56b7f9 100644 --- a/plugins/app-backend/src/service/appPlugin.test.ts +++ b/plugins/app-backend/src/service/appPlugin.test.ts @@ -19,12 +19,6 @@ import { resolve as resolvePath } from 'path'; import fetch from 'node-fetch'; import { startTestBackend } from '@backstage/backend-test-utils'; import { appPlugin } from './appPlugin'; -import { - databaseFactory, - httpRouterFactory, - loggerFactory, - rootLoggerFactory, -} from '@backstage/backend-app-api'; describe('appPlugin', () => { beforeEach(() => { @@ -45,12 +39,6 @@ describe('appPlugin', () => { it('boots', async () => { const { server } = await startTestBackend({ - services: [ - loggerFactory(), - rootLoggerFactory(), - databaseFactory(), - httpRouterFactory(), - ], features: [ appPlugin({ appPackageName: 'app',