From f02e445cf52478e09c5742b2d3420dc16cc60732 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 6 Mar 2024 15:44:52 +0100 Subject: [PATCH 01/10] chore: use redactor Signed-off-by: blam --- .../tasks/NunjucksWorkflowRunner.ts | 28 +++++++++++-------- plugins/scaffolder-node/src/actions/types.ts | 1 + 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index f29fd2ce1f..4e5de6c2b7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -54,6 +54,8 @@ import { scaffolderActionRules } from '../../service/rules'; import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alpha'; import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { PermissionsService } from '@backstage/backend-plugin-api'; +import { loggerToWinstonLogger } from '@backstage/backend-common'; +import { WinstonLogger } from '@backstage/backend-app-api'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -101,16 +103,8 @@ const createStepLogger = ({ step: TaskStep; }) => { const metadata = { stepId: step.id }; - const taskLogger = winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: winston.format.combine( - winston.format.colorize(), - winston.format.simple(), - ), - defaultMeta: {}, - }); - const streamLogger = new PassThrough(); + streamLogger.on('data', async data => { const message = data.toString().trim(); if (message?.length > 1) { @@ -118,7 +112,19 @@ const createStepLogger = ({ } }); - taskLogger.add(new winston.transports.Stream({ stream: streamLogger })); + const taskLogger = WinstonLogger.create({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.simple(), + ), + transports: [ + new winston.transports.Console(), + new winston.transports.Stream({ stream: streamLogger }), + ], + }); + + taskLogger.addRedactions(Object.values(task.secrets ?? {})); return { taskLogger, streamLogger }; }; @@ -355,7 +361,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { await action.handler({ input: iteration.input, secrets: task.secrets ?? {}, - logger: taskLogger, + logger: loggerToWinstonLogger(taskLogger), logStream: streamLogger, workspacePath, async checkpoint( diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 6a6b28bdcb..121b9e66b4 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -32,6 +32,7 @@ export type ActionContext< TActionOutput extends JsonObject = JsonObject, > = { logger: Logger; + /** @deprecated - use the logger instead */ logStream: Writable; secrets?: TaskSecrets; workspacePath: string; From abdfbf1a5d9260cd90580edaf50a4a74d94e8d82 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 6 Mar 2024 15:45:43 +0100 Subject: [PATCH 02/10] chore: install dep Signed-off-by: blam Signed-off-by: blam --- plugins/scaffolder-backend/package.json | 1 + yarn.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index bdfb627e2b..9a9f9e0de7 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -46,6 +46,7 @@ "build:assets": "node scripts/build-nunjucks.js" }, "dependencies": { + "@backstage/backend-app-api": "workspace:^", "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-tasks": "workspace:^", diff --git a/yarn.lock b/yarn.lock index bef0ffa114..2d279f6620 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8723,6 +8723,7 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-scaffolder-backend@workspace:plugins/scaffolder-backend" dependencies: + "@backstage/backend-app-api": "workspace:^" "@backstage/backend-common": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-tasks": "workspace:^" From 4522e6ef02cbab0d8d0567dfe52573a32a2e0702 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 8 Mar 2024 14:09:32 +0100 Subject: [PATCH 03/10] chore: switch around how we setup the loggers Signed-off-by: blam --- .../tasks/NunjucksWorkflowRunner.ts | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 4e5de6c2b7..0178d946ac 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -102,13 +102,11 @@ const createStepLogger = ({ task: TaskContext; step: TaskStep; }) => { - const metadata = { stepId: step.id }; - const streamLogger = new PassThrough(); - - streamLogger.on('data', async data => { + const stepLogStream = new PassThrough(); + stepLogStream.on('data', async data => { const message = data.toString().trim(); if (message?.length > 1) { - await task.emitLog(message, metadata); + await task.emitLog(message, { stepId: step.id }); } }); @@ -120,12 +118,27 @@ const createStepLogger = ({ ), transports: [ new winston.transports.Console(), - new winston.transports.Stream({ stream: streamLogger }), + new winston.transports.Stream({ stream: stepLogStream }), ], }); taskLogger.addRedactions(Object.values(task.secrets ?? {})); + // This stream logger should be deprecated. We're going to replace it with + // just using the logger directly, as all those logs get written to step logs + // using the stepLogStream above. + // Initially this stream used to be the only way to write to the client logs, but that + // has changed over time, there's not really a need for this anymore. + // You can just create a simple wrapper like the below in your action to write to the main logger. + // This way we also get recactions for free. + const streamLogger = new PassThrough(); + streamLogger.on('data', async data => { + const message = data.toString().trim(); + if (message?.length > 1) { + taskLogger.info(message); + } + }); + return { taskLogger, streamLogger }; }; From 98e369c31686bf3afdb88a41c6d242949e018577 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 8 Mar 2024 14:18:09 +0100 Subject: [PATCH 04/10] chore: reworking redacting of secrets and updating deprecated use of `logStream` Signed-off-by: blam --- .../src/actions/fetch/cookiecutter.ts | 9 +++++++-- .../src/actions/fetch/rails/index.ts | 8 +++++++- .../src/scaffolder/actions/builtin/catalog/write.ts | 2 +- .../src/scaffolder/actions/builtin/debug/log.ts | 4 ++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts index 8b084e6377..1a49ab0855 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.ts @@ -25,7 +25,7 @@ import { ScmIntegrations } from '@backstage/integration'; import commandExists from 'command-exists'; import fs from 'fs-extra'; import path, { resolve as resolvePath } from 'path'; -import { Writable } from 'stream'; +import { PassThrough, Writable } from 'stream'; import { createTemplateAction, fetchContents, @@ -245,10 +245,15 @@ export function createFetchCookiecutterAction(options: { _extensions: ctx.input.extensions, }; + const logStream = new PassThrough(); + logStream.on('data', chunk => { + ctx.logger.info(chunk.toString()); + }); + // Will execute the template in ./template and put the result in ./result await cookiecutter.run({ workspacePath: workDir, - logStream: ctx.logStream, + logStream, values: values, imageName: ctx.input.imageName, templateDir: templateDir, diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts index dfc7bea418..394d0fb02c 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.ts @@ -26,6 +26,7 @@ import { import { resolve as resolvePath } from 'path'; import { RailsNewRunner } from './railsNewRunner'; +import { PassThrough } from 'stream'; /** * Creates the `fetch:rails` Scaffolder action. @@ -215,10 +216,15 @@ export function createFetchRailsAction(options: { throw new Error(`Image ${imageName} is not allowed`); } + const logStream = new PassThrough(); + logStream.on('data', chunk => { + ctx.logger.info(chunk.toString()); + }); + // Will execute the template in ./template and put the result in ./result await templateRunner.run({ workspacePath: workDir, - logStream: ctx.logStream, + logStream, values: { ...ctx.input.values, imageName }, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/write.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/write.ts index 7663ad15c8..3a11fcd946 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/write.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/write.ts @@ -49,7 +49,7 @@ export function createCatalogWriteAction() { examples, supportsDryRun: true, async handler(ctx) { - ctx.logStream.write(`Writing catalog-info.yaml`); + ctx.logger.info(`Writing catalog-info.yaml`); const { filePath, entity } = ctx.input; const path = filePath ?? 'catalog-info.yaml'; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts index 524e12b5c9..98ab02437e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts @@ -60,12 +60,12 @@ export function createDebugLogAction() { ctx.logger.info(JSON.stringify(ctx.input, null, 2)); if (ctx.input?.message) { - ctx.logStream.write(ctx.input.message); + ctx.logger.info(ctx.input.message); } if (ctx.input?.listWorkspace) { const files = await recursiveReadDir(ctx.workspacePath); - ctx.logStream.write( + ctx.logger.info( `Workspace:\n${files .map(f => ` - ${relative(ctx.workspacePath, f)}`) .join('\n')}`, From 02ee4665a0dcd918e05062f9619088e09fe8c545 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 8 Mar 2024 14:26:11 +0100 Subject: [PATCH 05/10] chore: added changeset Signed-off-by: blam --- .changeset/blue-plums-beam.md | 5 +++++ plugins/scaffolder-node/src/actions/types.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/blue-plums-beam.md diff --git a/.changeset/blue-plums-beam.md b/.changeset/blue-plums-beam.md new file mode 100644 index 0000000000..2405e2611c --- /dev/null +++ b/.changeset/blue-plums-beam.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-node': minor +--- + +**DEPRECATION** - Deprecated the `logStream` in the `ActionContext`. Please move to using `ctx.logger.x` instead. diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 121b9e66b4..72b9bb09c0 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -32,7 +32,7 @@ export type ActionContext< TActionOutput extends JsonObject = JsonObject, > = { logger: Logger; - /** @deprecated - use the logger instead */ + /** @deprecated - use `ctx.logger` instead */ logStream: Writable; secrets?: TaskSecrets; workspacePath: string; From e9663a9cde23170fb6ba2a1e3b7473569ae78072 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 8 Mar 2024 14:28:08 +0100 Subject: [PATCH 06/10] chore: added more changesets Signed-off-by: blam Signed-off-by: blam --- .changeset/bright-beers-dress.md | 7 +++++++ .changeset/real-cows-flow.md | 5 +++++ 2 files changed, 12 insertions(+) create mode 100644 .changeset/bright-beers-dress.md create mode 100644 .changeset/real-cows-flow.md diff --git a/.changeset/bright-beers-dress.md b/.changeset/bright-beers-dress.md new file mode 100644 index 0000000000..7de575d31a --- /dev/null +++ b/.changeset/bright-beers-dress.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend-module-cookiecutter': patch +'@backstage/plugin-scaffolder-backend-module-rails': patch +--- + +Move away from using `ctx.logStream` diff --git a/.changeset/real-cows-flow.md b/.changeset/real-cows-flow.md new file mode 100644 index 0000000000..ace6df6ed4 --- /dev/null +++ b/.changeset/real-cows-flow.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Enable the redaction of secrets using the redacting logger and the secrets from the `TaskSpec` From 9f03f049160b957f70489f9290587d8cc985c9e3 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Mar 2024 16:19:10 +0100 Subject: [PATCH 07/10] chore: duplicate some of the logging work Signed-off-by: blam --- plugins/scaffolder-backend/package.json | 52 ++--- .../tasks/NunjucksWorkflowRunner.ts | 3 +- .../src/scaffolder/tasks/logger.ts | 182 ++++++++++++++++++ plugins/scaffolder-node/src/actions/types.ts | 1 + 4 files changed, 211 insertions(+), 27 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 9a9f9e0de7..2c34e937fe 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -1,18 +1,30 @@ { "name": "@backstage/plugin-scaffolder-backend", - "description": "The Backstage backend plugin that helps you create new things", "version": "1.22.0-next.1", - "main": "src/index.ts", - "types": "src/index.ts", - "license": "Apache-2.0", + "description": "The Backstage backend plugin that helps you create new things", + "backstage": { + "role": "backend-plugin" + }, "publishConfig": { "access": "public" }, + "keywords": [ + "backstage" + ], + "homepage": "https://backstage.io", + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "plugins/scaffolder-backend" + }, + "license": "Apache-2.0", "exports": { ".": "./src/index.ts", "./alpha": "./src/alpha.ts", "./package.json": "./package.json" }, + "main": "src/index.ts", + "types": "src/index.ts", "typesVersions": { "*": { "alpha": [ @@ -23,30 +35,23 @@ ] } }, - "backstage": { - "role": "backend-plugin" - }, - "homepage": "https://backstage.io", - "repository": { - "type": "git", - "url": "https://github.com/backstage/backstage", - "directory": "plugins/scaffolder-backend" - }, - "keywords": [ - "backstage" + "files": [ + "dist", + "migrations", + "config.d.ts", + "assets" ], "scripts": { - "start": "backstage-cli package start", "build": "backstage-cli package build", + "build:assets": "node scripts/build-nunjucks.js", + "clean": "backstage-cli package clean", "lint": "backstage-cli package lint", - "test": "backstage-cli package test", "prepack": "backstage-cli package prepack", "postpack": "backstage-cli package postpack", - "clean": "backstage-cli package clean", - "build:assets": "node scripts/build-nunjucks.js" + "start": "backstage-cli package start", + "test": "backstage-cli package test" }, "dependencies": { - "@backstage/backend-app-api": "workspace:^", "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/backend-tasks": "workspace:^", @@ -82,6 +87,7 @@ "jsonschema": "^1.2.6", "knex": "^3.0.0", "lodash": "^4.17.21", + "logform": "^2.3.2", "luxon": "^3.0.0", "nunjucks": "^3.2.3", "p-limit": "^3.1.0", @@ -106,11 +112,5 @@ "supertest": "^6.1.3", "wait-for-expect": "^3.0.2" }, - "files": [ - "dist", - "migrations", - "config.d.ts", - "assets" - ], "configSchema": "config.d.ts" } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 0178d946ac..fbd3813163 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -55,7 +55,7 @@ import { actionExecutePermission } from '@backstage/plugin-scaffolder-common/alp import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { PermissionsService } from '@backstage/backend-plugin-api'; import { loggerToWinstonLogger } from '@backstage/backend-common'; -import { WinstonLogger } from '@backstage/backend-app-api'; +import { WinstonLogger } from './logger'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -374,6 +374,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { await action.handler({ input: iteration.input, secrets: task.secrets ?? {}, + // TODO(blam): move to LoggerService and away from Winston logger: loggerToWinstonLogger(taskLogger), logStream: streamLogger, workspacePath, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts new file mode 100644 index 0000000000..b96b747077 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts @@ -0,0 +1,182 @@ +/* + * Copyright 2024 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, + RootLoggerService, +} from '@backstage/backend-plugin-api'; +import { JsonObject } from '@backstage/types'; +import { Format, TransformableInfo } from 'logform'; +import { + Logger, + format, + createLogger, + transports, + transport as Transport, +} from 'winston'; + +/** + * Escapes a given string to be used inside a RegExp. + * + * Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions + */ +const escapeRegExp = (text: string) => { + return text.replace(/[.*+?^${}(\)|[\]\\]/g, '\\$&'); +}; + +interface WinstonLoggerOptions { + meta?: JsonObject; + level: string; + format: Format; + transports: Transport[]; +} + +export class WinstonLogger implements RootLoggerService { + #winston: Logger; + #addRedactions?: (redactions: Iterable) => void; + + /** + * 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: Iterable) => void; + } { + const redactionSet = new Set(); + + let redactionPattern: RegExp | undefined = undefined; + + return { + format: format(info => { + if (redactionPattern && typeof info.message === 'string') { + info.message = info.message.replace(redactionPattern, '[REDACTED]'); + } + if (redactionPattern && typeof info.stack === 'string') { + info.stack = info.stack.replace(redactionPattern, '[REDACTED]'); + } + return info; + })(), + add(newRedactions) { + let added = 0; + for (const redactionToTrim of newRedactions) { + // Trimming the string ensures that we don't accdentally get extra + // newlines or other whitespace interfering with the redaction; this + // can happen for example when using string literals in yaml + const redaction = redactionToTrim.trim(); + // 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); + added += 1; + } + } + if (added > 0) { + const redactions = Array.from(redactionSet) + .map(r => escapeRegExp(r)) + .join('|'); + redactionPattern = new RegExp(`(${redactions})`, 'g'); + } + }, + }; + } + + /** + * Creates a pretty printed winston log formatter. + */ + 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}`; + }), + ); + } + + private constructor( + winston: Logger, + addRedactions?: (redactions: Iterable) => void, + ) { + this.#winston = winston; + this.#addRedactions = addRedactions; + } + + error(message: string, meta?: JsonObject): void { + this.#winston.error(message, meta); + } + + warn(message: string, meta?: JsonObject): void { + this.#winston.warn(message, meta); + } + + info(message: string, meta?: JsonObject): void { + this.#winston.info(message, meta); + } + + debug(message: string, meta?: JsonObject): void { + this.#winston.debug(message, meta); + } + + child(meta: JsonObject): LoggerService { + return new WinstonLogger(this.#winston.child(meta)); + } + + addRedactions(redactions: Iterable) { + this.#addRedactions?.(redactions); + } +} diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 72b9bb09c0..f0e4c12869 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -31,6 +31,7 @@ export type ActionContext< TActionInput extends JsonObject, TActionOutput extends JsonObject = JsonObject, > = { + // TODO(blam): move this to LoggerService logger: Logger; /** @deprecated - use `ctx.logger` instead */ logStream: Writable; From 3a9979d6095d2166247d263a5ae6817c55f216c6 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Mar 2024 16:19:42 +0100 Subject: [PATCH 08/10] chore: fix yarn.lock Signed-off-by: blam Signed-off-by: blam --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 2d279f6620..0f498305c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8723,7 +8723,6 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-scaffolder-backend@workspace:plugins/scaffolder-backend" dependencies: - "@backstage/backend-app-api": "workspace:^" "@backstage/backend-common": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-tasks": "workspace:^" @@ -8767,6 +8766,7 @@ __metadata: jsonschema: ^1.2.6 knex: ^3.0.0 lodash: ^4.17.21 + logform: ^2.3.2 luxon: ^3.0.0 nunjucks: ^3.2.3 p-limit: ^3.1.0 From af97420444731c727a523432806bff8a85a37fc7 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Mar 2024 16:40:23 +0100 Subject: [PATCH 09/10] chore: fixing tests Signed-off-by: blam --- .../builtin/debug/log.examples.test.ts | 18 +++++------ .../actions/builtin/debug/log.test.ts | 32 +++++++------------ 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts index 06addba98d..0c37db9eb9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.examples.test.ts @@ -15,23 +15,23 @@ */ import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; -import { Writable } from 'stream'; import { createDebugLogAction } from './log'; import { join } from 'path'; import yaml from 'yaml'; import { examples } from './log.examples'; import { createMockDirectory } from '@backstage/backend-test-utils'; +import { Logger } from 'winston'; describe('debug:log examples', () => { - const logStream = { - write: jest.fn(), - } as jest.Mocked> as jest.Mocked; + const logger = { + info: jest.fn(), + } as unknown as jest.Mocked; const mockDir = createMockDirectory(); const workspacePath = mockDir.resolve('workspace'); const mockContext = createMockActionContext({ - logStream, + logger, workspacePath, }); @@ -51,8 +51,7 @@ describe('debug:log examples', () => { input: yaml.parse(examples[0].example).steps[0].input, }); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('Hello Backstage!'), ); }); @@ -63,11 +62,10 @@ describe('debug:log examples', () => { input: yaml.parse(examples[1].example).steps[0].input, }); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('README.md'), ); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining(join('a-directory', 'index.md')), ); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts index 09c090582a..9c626f1dfe 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts @@ -15,21 +15,21 @@ */ import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; -import { Writable } from 'stream'; +import { Logger } from 'winston'; import { createDebugLogAction } from './log'; import { join } from 'path'; import yaml from 'yaml'; import { createMockDirectory } from '@backstage/backend-test-utils'; describe('debug:log', () => { - const logStream = { - write: jest.fn(), - } as jest.Mocked> as jest.Mocked; + const logger = { + info: jest.fn(), + } as unknown as jest.Mocked; const mockDir = createMockDirectory(); const workspacePath = mockDir.resolve('workspace'); - const mockContext = createMockActionContext({ workspacePath, logStream }); + const mockContext = createMockActionContext({ workspacePath, logger }); const action = createDebugLogAction(); @@ -41,12 +41,6 @@ describe('debug:log', () => { jest.resetAllMocks(); }); - it('should do nothing', async () => { - await action.handler(mockContext); - - expect(logStream.write).toHaveBeenCalledTimes(0); - }); - it('should log the workspace content, if active', async () => { const context = { ...mockContext, @@ -57,11 +51,10 @@ describe('debug:log', () => { await action.handler(context); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('README.md'), ); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining(join('a-directory', 'index.md')), ); }); @@ -76,8 +69,7 @@ describe('debug:log', () => { await action.handler(context); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('Hello Backstage!'), ); }); @@ -94,11 +86,10 @@ describe('debug:log', () => { await action.handler(context); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('README.md'), ); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining(join('a-directory', 'index.md')), ); }); @@ -115,8 +106,7 @@ describe('debug:log', () => { await action.handler(context); - expect(logStream.write).toHaveBeenCalledTimes(1); - expect(logStream.write).toHaveBeenCalledWith( + expect(logger.info).toHaveBeenCalledWith( expect.stringContaining('Hello Backstage!'), ); }); From 0d256a4d3267f1051dacd083773ec4cda64eb709 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 12 Mar 2024 10:15:17 +0100 Subject: [PATCH 10/10] chore: fix tests Signed-off-by: blam --- .../src/actions/fetch/cookiecutter.test.ts | 5 +++-- .../src/actions/fetch/rails/index.test.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts index 92a636e58b..fad544488a 100644 --- a/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts +++ b/plugins/scaffolder-backend-module-cookiecutter/src/actions/fetch/cookiecutter.test.ts @@ -23,6 +23,7 @@ import { createFetchCookiecutterAction } from './cookiecutter'; import { join } from 'path'; import type { ActionContext } from '@backstage/plugin-scaffolder-node'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; +import { Writable } from 'stream'; const executeShellCommand = jest.fn(); const commandExists = jest.fn(); @@ -166,7 +167,7 @@ describe('fetch:cookiecutter', () => { join(mockTmpDir, 'template'), '--verbose', ], - logStream: mockContext.logStream, + logStream: expect.any(Writable), }), ); }); @@ -187,7 +188,7 @@ describe('fetch:cookiecutter', () => { }, workingDir: '/input', envVars: { HOME: '/tmp' }, - logStream: mockContext.logStream, + logStream: expect.any(Writable), }), ); }); diff --git a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts index b43ffcc95c..40514dc7c5 100644 --- a/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts +++ b/plugins/scaffolder-backend-module-rails/src/actions/fetch/rails/index.test.ts @@ -35,6 +35,7 @@ import { createFetchRailsAction } from './index'; import { fetchContents } from '@backstage/plugin-scaffolder-node'; import { createMockDirectory } from '@backstage/backend-test-utils'; import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test-utils'; +import { Writable } from 'stream'; describe('fetch:rails', () => { const mockDir = createMockDirectory(); @@ -104,7 +105,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logStream: mockContext.logStream, + logStream: expect.any(Writable), values: mockContext.input.values, }); }); @@ -120,7 +121,7 @@ describe('fetch:rails', () => { expect(mockRailsTemplater.run).toHaveBeenCalledWith({ workspacePath: mockContext.workspacePath, - logStream: mockContext.logStream, + logStream: expect.any(Writable), values: { ...mockContext.input.values, imageName: 'foo/rails-custom-image',