From 6b59bd8c885a373f391ddf36c116e62503561771 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Jan 2023 21:13:32 +0100 Subject: [PATCH] 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; +}