From a4aa244fb778b0a0441c82dbd889750b85000ae3 Mon Sep 17 00:00:00 2001 From: Paul Schultz Date: Mon, 13 Jan 2025 12:30:16 -0600 Subject: [PATCH] apply requested changes Signed-off-by: Paul Schultz --- .../olive-boxes-hide-backend-defaults.md | 7 ++ .../olive-boxes-hide-backend-plugin-api.md | 7 ++ .../olive-boxes-hide-backend-test-utils.md | 7 ++ .../olive-boxes-hide-catalog-backend.md | 7 ++ .../olive-boxes-hide-scaffolder-backend.md | 7 ++ .../olive-boxes-hide-scaffolder-node.md | 7 ++ .changeset/olive-boxes-hide.md | 13 --- docs/backend-system/core-services/auditor.md | 17 --- packages/backend-defaults/config.d.ts | 12 -- .../backend-defaults/report-auditor.api.md | 54 +++++---- .../src/entrypoints/auditor/Auditor.test.ts | 41 ------- .../src/entrypoints/auditor/Auditor.ts | 103 +++++++++++------- .../auditor/auditorServiceFactory.ts | 37 +------ .../entrypoints/rootLogger/WinstonLogger.ts | 94 ++++++++++++++-- .../rootLogger/rootLoggerServiceFactory.ts | 9 +- .../backend-defaults/src/lib/colorFormat.ts | 57 ---------- .../src/lib/defaultConsoleTransport.ts | 19 ---- .../src/lib/redacterFormat.ts | 69 ------------ packages/backend-plugin-api/report.api.md | 53 ++++----- .../services/definitions/AuditorService.ts | 34 +++--- .../src/services/definitions/index.ts | 7 +- packages/backend-test-utils/report.api.md | 13 +-- .../next/services/MockAuditorService.test.ts | 35 +----- .../src/next/services/MockAuditorService.ts | 49 +++++---- .../src/next/services/mockServices.ts | 22 +--- .../src/next/wiring/TestBackend.ts | 9 +- .../src/service/CatalogBuilder.ts | 1 - .../src/service/createRouter.ts | 20 +++- plugins/scaffolder-backend/report.api.md | 1 - .../src/scaffolder/tasks/StorageTaskBroker.ts | 24 ---- .../src/scaffolder/tasks/TaskWorker.ts | 6 +- .../scaffolder-backend/src/service/router.ts | 14 ++- 32 files changed, 344 insertions(+), 511 deletions(-) create mode 100644 .changeset/olive-boxes-hide-backend-defaults.md create mode 100644 .changeset/olive-boxes-hide-backend-plugin-api.md create mode 100644 .changeset/olive-boxes-hide-backend-test-utils.md create mode 100644 .changeset/olive-boxes-hide-catalog-backend.md create mode 100644 .changeset/olive-boxes-hide-scaffolder-backend.md create mode 100644 .changeset/olive-boxes-hide-scaffolder-node.md delete mode 100644 .changeset/olive-boxes-hide.md delete mode 100644 packages/backend-defaults/src/lib/colorFormat.ts delete mode 100644 packages/backend-defaults/src/lib/defaultConsoleTransport.ts delete mode 100644 packages/backend-defaults/src/lib/redacterFormat.ts diff --git a/.changeset/olive-boxes-hide-backend-defaults.md b/.changeset/olive-boxes-hide-backend-defaults.md new file mode 100644 index 0000000000..64c828feca --- /dev/null +++ b/.changeset/olive-boxes-hide-backend-defaults.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-defaults': minor +--- + +feat: add auditor to `coreServices` + +This change introduces the `auditor` service implementation details. diff --git a/.changeset/olive-boxes-hide-backend-plugin-api.md b/.changeset/olive-boxes-hide-backend-plugin-api.md new file mode 100644 index 0000000000..5595cfb338 --- /dev/null +++ b/.changeset/olive-boxes-hide-backend-plugin-api.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-plugin-api': minor +--- + +feat: add auditor to `coreServices` + +This change introduces the `auditor` service definition. diff --git a/.changeset/olive-boxes-hide-backend-test-utils.md b/.changeset/olive-boxes-hide-backend-test-utils.md new file mode 100644 index 0000000000..04ae80b24d --- /dev/null +++ b/.changeset/olive-boxes-hide-backend-test-utils.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-test-utils': minor +--- + +feat: add auditor to `coreServices` + +This change introduces mocks for the `auditor` service. diff --git a/.changeset/olive-boxes-hide-catalog-backend.md b/.changeset/olive-boxes-hide-catalog-backend.md new file mode 100644 index 0000000000..bbfbb315dd --- /dev/null +++ b/.changeset/olive-boxes-hide-catalog-backend.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +feat: add auditor to `coreServices` + +This change integrates the `auditor` service into the Catalog plugin. diff --git a/.changeset/olive-boxes-hide-scaffolder-backend.md b/.changeset/olive-boxes-hide-scaffolder-backend.md new file mode 100644 index 0000000000..b19ef1993b --- /dev/null +++ b/.changeset/olive-boxes-hide-scaffolder-backend.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +feat: add auditor to `coreServices` + +This change integrates the `auditor` service into the Scaffolder plugin. diff --git a/.changeset/olive-boxes-hide-scaffolder-node.md b/.changeset/olive-boxes-hide-scaffolder-node.md new file mode 100644 index 0000000000..7c3d3cdedc --- /dev/null +++ b/.changeset/olive-boxes-hide-scaffolder-node.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-node': minor +--- + +feat: add auditor to `coreServices` + +This change introduces an optional `taskId` property to `TaskContext`. diff --git a/.changeset/olive-boxes-hide.md b/.changeset/olive-boxes-hide.md deleted file mode 100644 index 878dc8394a..0000000000 --- a/.changeset/olive-boxes-hide.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -'@backstage/backend-plugin-api': minor -'@backstage/backend-test-utils': minor -'@backstage/plugin-scaffolder-backend': minor -'@backstage/backend-defaults': minor -'@backstage/plugin-catalog-backend': minor -'@backstage/plugin-scaffolder-node': minor ---- - -feat: add auditor to `coreServices` - -This change introduces a new `auditor` service to the `coreServices` in Backstage. -The auditor service enables plugins to emit audit events for security-relevant actions. diff --git a/docs/backend-system/core-services/auditor.md b/docs/backend-system/core-services/auditor.md index c356e43bd2..d4cbf70325 100644 --- a/docs/backend-system/core-services/auditor.md +++ b/docs/backend-system/core-services/auditor.md @@ -85,20 +85,3 @@ When defining `eventId` and `subEventId` for your audit events, follow these gui - Use `subEventId` to further categorize events within a logical group. For example, if the `eventId` is "fetch", the `subEventId` could be "by-id" or "by-location" to specify the method used for fetching. - Avoid redundant prefixes related to the plugin ID, as that context is already provided. - Choose names that clearly and concisely describe the event being audited. - -## Configuring the service - -The Auditor Service can be configured using the `backend.auditor` section in your `app-config.yaml` file. - -### Console Logging - -Console logging allows you to see audit events directly in your terminal output. This is useful for development and debugging purposes. To enable console logging, set the `enabled` flag to `true` within the `console` section: - -```yaml -backend: - auditor: - console: - enabled: true -``` - -By default, console logging is enabled. You can disable it by setting the `enabled` flag to `false`. diff --git a/packages/backend-defaults/config.d.ts b/packages/backend-defaults/config.d.ts index baf0bde2e6..aa97313921 100644 --- a/packages/backend-defaults/config.d.ts +++ b/packages/backend-defaults/config.d.ts @@ -632,18 +632,6 @@ export interface Config { paths?: string[]; }>; }; - auditor?: { - /** - * Configuration for the auditing to the console - */ - console: { - /** - * Enables auditing to console - * @default true - */ - enabled: boolean; - }; - }; }; /** diff --git a/packages/backend-defaults/report-auditor.api.md b/packages/backend-defaults/report-auditor.api.md index e74ca5b11f..57bb8949fd 100644 --- a/packages/backend-defaults/report-auditor.api.md +++ b/packages/backend-defaults/report-auditor.api.md @@ -3,15 +3,17 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts -import type { AuditorCreateEvent } from '@backstage/backend-plugin-api'; -import type { AuditorEventSeverityLevel } from '@backstage/backend-plugin-api'; import { AuditorService } from '@backstage/backend-plugin-api'; +import type { AuditorServiceCreateEventOptions } from '@backstage/backend-plugin-api'; +import type { AuditorServiceEvent } from '@backstage/backend-plugin-api'; +import type { AuditorServiceEventSeverityLevel } from '@backstage/backend-plugin-api'; import type { AuthService } from '@backstage/backend-plugin-api'; import type { Format } from 'logform'; import type { HttpAuthService } from '@backstage/backend-plugin-api'; import type { JsonObject } from '@backstage/types'; import type { PluginMetadataService } from '@backstage/backend-plugin-api'; import type { Request as Request_2 } from 'express'; +import type { RootLoggerService } from '@backstage/backend-plugin-api'; import { ServiceFactory } from '@backstage/backend-plugin-api'; import * as winston from 'winston'; @@ -19,7 +21,7 @@ import * as winston from 'winston'; export type AuditorEvent = [ eventId: string, meta: { - severityLevel: AuditorEventSeverityLevel; + severityLevel: AuditorServiceEventSeverityLevel; actor: AuditorEventActorDetails; meta?: JsonObject; request?: AuditorEventRequest; @@ -37,7 +39,7 @@ export type AuditorEventActorDetails = { // @public export type AuditorEventOptions = { eventId: string; - severityLevel?: AuditorEventSeverityLevel; + severityLevel?: AuditorServiceEventSeverityLevel; request?: Request_2; meta?: TMeta; } & AuditorEventStatus; @@ -49,23 +51,17 @@ export type AuditorEventRequest = { }; // @public (undocumented) -export type AuditorEventStatus = +export type AuditorEventStatus = | { status: 'initiated'; } | { status: 'succeeded'; } - | ({ + | { status: 'failed'; - } & ( - | { - error: TError; - } - | { - errors: TError[]; - } - )); + error: Error; + }; // @public export const auditorFieldFormat: Format; @@ -88,17 +84,16 @@ export class DefaultAuditorService implements AuditorService { }, ): DefaultAuditorService; // (undocumented) - createEvent( - options: Parameters>[0], - ): ReturnType>; + createEvent( + options: AuditorServiceCreateEventOptions, + ): Promise; } // @public (undocumented) -export const defaultProdFormat: Format; +export const defaultFormatter: Format; // @public (undocumented) export class DefaultRootAuditorService { - static colorFormat(): Format; static create(options?: RootAuditorOptions): DefaultRootAuditorService; // (undocumented) forPlugin(deps: { @@ -107,18 +102,19 @@ export class DefaultRootAuditorService { plugin: PluginMetadataService; }): AuditorService; // (undocumented) - log(auditEvent: AuditorEvent): Promise; + log(auditorEvent: AuditorEvent): Promise; } -// @public (undocumented) -export interface RootAuditorOptions { - // (undocumented) - format?: Format; - // (undocumented) - meta?: JsonObject; - // (undocumented) - transports?: winston.transport[]; -} +// @public +export type RootAuditorOptions = + | { + meta?: JsonObject; + format?: Format; + transports?: winston.transport[]; + } + | { + rootLogger: RootLoggerService; + }; // (No @packageDocumentation comment for this package) ``` diff --git a/packages/backend-defaults/src/entrypoints/auditor/Auditor.test.ts b/packages/backend-defaults/src/entrypoints/auditor/Auditor.test.ts index 4324b4aa6b..854fe94c8b 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/Auditor.test.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/Auditor.test.ts @@ -15,9 +15,6 @@ */ import { mockServices } from '@backstage/backend-test-utils'; -import { format } from 'logform'; -import { MESSAGE } from 'triple-beam'; -import Transport from 'winston-transport'; import { DefaultAuditorService, DefaultRootAuditorService } from './Auditor'; describe('Auditor', () => { @@ -38,44 +35,6 @@ describe('Auditor', () => { expect(childLogger).toBeInstanceOf(DefaultAuditorService); }); - it('should log', async () => { - const mockTransport = new Transport({ - log: jest.fn(), - logv: jest.fn(), - }); - - const pluginId = 'test-plugin'; - - const auditor = DefaultRootAuditorService.create({ - format: format.json(), - transports: [mockTransport], - }).forPlugin({ - auth: mockServices.auth.mock(), - httpAuth: mockServices.httpAuth.mock(), - plugin: { - getId: () => pluginId, - }, - }); - - await auditor.createEvent({ - eventId: 'test-event', - }); - - expect(mockTransport.log).toHaveBeenCalledWith( - expect.objectContaining({ - [MESSAGE]: JSON.stringify({ - actor: {}, - isAuditorEvent: true, - level: 'info', - message: 'test-plugin.test-event', - severityLevel: 'low', - status: 'initiated', - }), - }), - expect.any(Function), - ); - }); - it('should log a status "initiated" using createEvent', async () => { const pluginId = 'test-plugin'; diff --git a/packages/backend-defaults/src/entrypoints/auditor/Auditor.ts b/packages/backend-defaults/src/entrypoints/auditor/Auditor.ts index 68eb5035ea..4a62b02a03 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/Auditor.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/Auditor.ts @@ -15,21 +15,22 @@ */ import type { - AuditorCreateEvent, - AuditorEventSeverityLevel, AuditorService, + AuditorServiceCreateEventOptions, + AuditorServiceEvent, + AuditorServiceEventSeverityLevel, AuthService, BackstageCredentials, HttpAuthService, PluginMetadataService, + RootLoggerService, } from '@backstage/backend-plugin-api'; import { ForwardedError } from '@backstage/errors'; import type { JsonObject } from '@backstage/types'; import type { Request } from 'express'; import type { Format } from 'logform'; import * as winston from 'winston'; -import { colorFormat } from '../../lib/colorFormat'; -import { defaultConsoleTransport } from '../../lib/defaultConsoleTransport'; +import { WinstonLogger } from '../rootLogger'; /** @public */ export type AuditorEventActorDetails = { @@ -46,12 +47,13 @@ export type AuditorEventRequest = { }; /** @public */ -export type AuditorEventStatus = +export type AuditorEventStatus = | { status: 'initiated' } | { status: 'succeeded' } - | ({ + | { status: 'failed'; - } & ({ error: TError } | { errors: TError[] })); + error: Error; + }; /** * Options for creating an auditor event. @@ -66,7 +68,7 @@ export type AuditorEventOptions = { */ eventId: string; - severityLevel?: AuditorEventSeverityLevel; + severityLevel?: AuditorServiceEventSeverityLevel; /** (Optional) The associated HTTP request, if applicable. */ request?: Request; @@ -83,7 +85,7 @@ export type AuditorEventOptions = { export type AuditorEvent = [ eventId: string, meta: { - severityLevel: AuditorEventSeverityLevel; + severityLevel: AuditorServiceEventSeverityLevel; actor: AuditorEventActorDetails; meta?: JsonObject; request?: AuditorEventRequest; @@ -91,7 +93,7 @@ export type AuditorEvent = [ ]; /** @public */ -export const defaultProdFormat = winston.format.combine( +export const defaultFormatter = winston.format.combine( winston.format.timestamp({ format: 'YYYY-MM-DD HH:mm:ss', }), @@ -109,13 +111,6 @@ export const auditorFieldFormat = winston.format(info => { return { ...info, isAuditorEvent: true }; })(); -/** @public */ -export interface RootAuditorOptions { - meta?: JsonObject; - format?: Format; - transports?: winston.transport[]; -} - /** * A {@link @backstage/backend-plugin-api#AuditorService} implementation based on winston. * @@ -162,12 +157,10 @@ export class DefaultAuditorService implements AuditorService { this.impl.log(auditEvent); } - async createEvent( - options: Parameters>[0], - ): ReturnType> { - if (!options.suppressInitialEvent) { - await this.log({ ...options, status: 'initiated' }); - } + async createEvent( + options: AuditorServiceCreateEventOptions, + ): Promise { + await this.log({ ...options, status: 'initiated' }); return { success: async params => { @@ -256,11 +249,28 @@ export class DefaultAuditorService implements AuditorService { } } +/** + * Options for creating a root auditor. + * If `rootLogger` is provided, the root auditor will default to using it. + * Otherwise, a new logger will be created using the provided `meta`, `format`, and `transports`. + * + * @public + */ +export type RootAuditorOptions = + | { + meta?: JsonObject; + format?: Format; + transports?: winston.transport[]; + } + | { + rootLogger: RootLoggerService; + }; + /** @public */ export class DefaultRootAuditorService { - private readonly impl: winston.Logger; + private readonly impl: WinstonLogger; - private constructor(impl: winston.Logger) { + private constructor(impl: WinstonLogger) { this.impl = impl; } @@ -268,35 +278,44 @@ export class DefaultRootAuditorService { * Creates a {@link DefaultRootAuditorService} instance. */ static create(options?: RootAuditorOptions): DefaultRootAuditorService { - const defaultFormatter = - process.env.NODE_ENV === 'production' - ? defaultProdFormat - : DefaultRootAuditorService.colorFormat(); + if (options && 'rootLogger' in options) { + return new DefaultRootAuditorService( + options.rootLogger.child({ isAuditorEvent: true }) as WinstonLogger, + ); + } - let auditor = winston.createLogger({ + let auditor = WinstonLogger.create({ + meta: { + service: 'backstage', + }, level: 'info', format: winston.format.combine( auditorFieldFormat, options?.format ?? defaultFormatter, ), - transports: options?.transports ?? defaultConsoleTransport, + transports: options?.transports, }); if (options?.meta) { - auditor = auditor.child(options.meta); + auditor = auditor.child(options.meta) as WinstonLogger; } + return new DefaultRootAuditorService(auditor); } - /** - * Creates a pretty printed winston log formatter. - */ - static colorFormat(): Format { - return colorFormat(); - } + async log(auditorEvent: AuditorEvent): Promise { + const [eventId, meta] = auditorEvent; - async log(auditEvent: AuditorEvent): Promise { - this.impl.info(...auditEvent); + // change `error` type to a string for logging purposes + let fields: Omit & { error?: string }; + + if ('error' in meta) { + fields = { ...meta, error: meta.error.toString() }; + } else { + fields = meta; + } + + this.impl.info(eventId, fields); } forPlugin(deps: { @@ -304,7 +323,9 @@ export class DefaultRootAuditorService { httpAuth: HttpAuthService; plugin: PluginMetadataService; }): AuditorService { - const impl = new DefaultRootAuditorService(this.impl.child({})); + const impl = new DefaultRootAuditorService( + this.impl.child({}) as WinstonLogger, + ); return DefaultAuditorService.create(impl, deps); } } diff --git a/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts b/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts index 3a467c81b2..cba8823c92 100644 --- a/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/auditor/auditorServiceFactory.ts @@ -18,23 +18,7 @@ import { coreServices, createServiceFactory, } from '@backstage/backend-plugin-api'; -import type { Config } from '@backstage/config'; -import * as winston from 'winston'; -import { defaultConsoleTransport } from '../../lib/defaultConsoleTransport'; -import { - DefaultRootAuditorService, - auditorFieldFormat, - defaultProdFormat, -} from './Auditor'; - -const transports = { - auditorConsole: (config?: Config) => { - if (!config?.getOptionalBoolean('console.enabled')) { - return []; - } - return [defaultConsoleTransport]; - }, -}; +import { DefaultRootAuditorService } from './Auditor'; /** * Plugin-level auditing. @@ -48,26 +32,13 @@ const transports = { export const auditorServiceFactory = createServiceFactory({ service: coreServices.auditor, deps: { - config: coreServices.rootConfig, + rootLogger: coreServices.rootLogger, auth: coreServices.auth, httpAuth: coreServices.httpAuth, plugin: coreServices.pluginMetadata, }, - async createRootContext({ config }) { - const auditorConfig = config.getOptionalConfig('backend.auditor'); - - const auditor = DefaultRootAuditorService.create({ - meta: { - service: 'backstage', - }, - format: winston.format.combine( - auditorFieldFormat, - process.env.NODE_ENV === 'production' - ? defaultProdFormat - : DefaultRootAuditorService.colorFormat(), - ), - transports: [...transports.auditorConsole(auditorConfig)], - }); + async createRootContext({ rootLogger }) { + const auditor = DefaultRootAuditorService.create({ rootLogger }); return auditor; }, diff --git a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts index 2e142456ad..9f7532e66b 100644 --- a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts +++ b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts @@ -19,11 +19,16 @@ import { RootLoggerService, } from '@backstage/backend-plugin-api'; import { JsonObject } from '@backstage/types'; -import { Format } from 'logform'; -import { Logger, transport as Transport, createLogger, format } from 'winston'; -import { colorFormat } from '../../lib/colorFormat'; -import { defaultConsoleTransport } from '../../lib/defaultConsoleTransport'; -import { redacterFormat } from '../../lib/redacterFormat'; +import { Format, TransformableInfo } from 'logform'; +import { + Logger, + format, + createLogger, + transports, + transport as Transport, +} from 'winston'; +import { MESSAGE } from 'triple-beam'; +import { escapeRegExp } from '../../lib/escapeRegExp'; /** * @public @@ -60,7 +65,7 @@ export class WinstonLogger implements RootLoggerService { options.format ?? defaultFormatter, redacter.format, ), - transports: options.transports ?? defaultConsoleTransport, + transports: options.transports ?? new transports.Console(), }); if (options.meta) { @@ -77,14 +82,87 @@ export class WinstonLogger implements RootLoggerService { format: Format; add: (redactions: Iterable) => void; } { - return redacterFormat(); + const redactionSet = new Set(); + + let redactionPattern: RegExp | undefined = undefined; + + return { + format: format((obj: TransformableInfo) => { + if (!redactionPattern || !obj) { + return obj; + } + + obj[MESSAGE] = obj[MESSAGE]?.replace?.(redactionPattern, '***'); + + return obj; + })(), + 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 { - return colorFormat(); + 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]) => { + let stringValue = ''; + + try { + stringValue = `${value}`; + } catch (e) { + stringValue = '[field value not castable to string]'; + } + + return `${colorizer.colorize('field', `${key}`)}=${stringValue}`; + }) + .join(' '); + + return `${timestampColor} ${prefixColor} ${level} ${message} ${extraFields}`; + }), + ); } private constructor( diff --git a/packages/backend-defaults/src/entrypoints/rootLogger/rootLoggerServiceFactory.ts b/packages/backend-defaults/src/entrypoints/rootLogger/rootLoggerServiceFactory.ts index cb2c9eb9c9..5a4427cf5f 100644 --- a/packages/backend-defaults/src/entrypoints/rootLogger/rootLoggerServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/rootLogger/rootLoggerServiceFactory.ts @@ -15,13 +15,12 @@ */ import { - coreServices, createServiceFactory, + coreServices, } from '@backstage/backend-plugin-api'; -import { format } from 'winston'; -import { defaultConsoleTransport } from '../../lib/defaultConsoleTransport'; -import { createConfigSecretEnumerator } from '../rootConfig/createConfigSecretEnumerator'; +import { transports, format } from 'winston'; import { WinstonLogger } from '../rootLogger/WinstonLogger'; +import { createConfigSecretEnumerator } from '../rootConfig/createConfigSecretEnumerator'; /** * Root-level logging. @@ -47,7 +46,7 @@ export const rootLoggerServiceFactory = createServiceFactory({ process.env.NODE_ENV === 'production' ? format.json() : WinstonLogger.colorFormat(), - transports: [defaultConsoleTransport], + transports: [new transports.Console()], }); const secretEnumerator = await createConfigSecretEnumerator({ logger }); diff --git a/packages/backend-defaults/src/lib/colorFormat.ts b/packages/backend-defaults/src/lib/colorFormat.ts deleted file mode 100644 index 9dd5438f4a..0000000000 --- a/packages/backend-defaults/src/lib/colorFormat.ts +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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 { Format, TransformableInfo } from 'logform'; -import { format } from 'winston'; - -/** - * Creates a pretty printed winston log format. - */ -export function 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]) => { - let stringValue = ''; - try { - stringValue = `${value}`; - } catch (e) { - stringValue = '[field value not castable to string]'; - } - return `${colorizer.colorize('field', `${key}`)}=${stringValue}`; - }) - .join(' '); - - return `${timestampColor} ${prefixColor} ${level} ${message} ${extraFields}`; - }), - ); -} diff --git a/packages/backend-defaults/src/lib/defaultConsoleTransport.ts b/packages/backend-defaults/src/lib/defaultConsoleTransport.ts deleted file mode 100644 index 009eb32436..0000000000 --- a/packages/backend-defaults/src/lib/defaultConsoleTransport.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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 { transports } from 'winston'; - -export const defaultConsoleTransport = new transports.Console(); diff --git a/packages/backend-defaults/src/lib/redacterFormat.ts b/packages/backend-defaults/src/lib/redacterFormat.ts deleted file mode 100644 index 83030d1af0..0000000000 --- a/packages/backend-defaults/src/lib/redacterFormat.ts +++ /dev/null @@ -1,69 +0,0 @@ -/* - * 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 { Format, TransformableInfo } from 'logform'; -import { MESSAGE } from 'triple-beam'; -import { format } from 'winston'; -import { escapeRegExp } from './escapeRegExp'; - -/** - * Creates a winston log format for redacting secrets. - */ -export function redacterFormat(): { - format: Format; - add: (redactions: Iterable) => void; -} { - const redactionSet = new Set(); - - let redactionPattern: RegExp | undefined = undefined; - - return { - format: format((obj: TransformableInfo) => { - if (!redactionPattern || !obj) { - return obj; - } - - obj[MESSAGE] = obj[MESSAGE]?.replace?.(redactionPattern, '***'); - - return obj; - })(), - 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'); - } - }, - }; -} diff --git a/packages/backend-plugin-api/report.api.md b/packages/backend-plugin-api/report.api.md index 84a8de8243..a4f9b9fa1d 100644 --- a/packages/backend-plugin-api/report.api.md +++ b/packages/backend-plugin-api/report.api.md @@ -26,40 +26,35 @@ import { Readable } from 'stream'; import type { Request as Request_2 } from 'express'; import type { Response as Response_2 } from 'express'; -// @public (undocumented) -export type AuditorCreateEvent = (options: { - eventId: string; - severityLevel?: AuditorEventSeverityLevel; - request?: Request_2; - meta?: TRootMeta; - suppressInitialEvent?: boolean; -}) => Promise<{ - success(options?: { meta?: TMeta }): Promise; - fail( - options: { - meta?: TMeta; - } & ( - | { - error: TError; - } - | { - errors: TError[]; - } - ), - ): Promise; -}>; - -// @public -export type AuditorEventSeverityLevel = 'low' | 'medium' | 'high' | 'critical'; - // @public export interface AuditorService { // (undocumented) - createEvent( - options: Parameters>[0], - ): ReturnType>; + createEvent( + options: AuditorServiceCreateEventOptions, + ): Promise; } +// @public (undocumented) +export type AuditorServiceCreateEventOptions = { + eventId: string; + severityLevel?: AuditorServiceEventSeverityLevel; + request?: Request_2; + meta?: JsonObject; +}; + +// @public (undocumented) +export type AuditorServiceEvent = { + success(options?: { meta?: JsonObject }): Promise; + fail(options: { meta?: JsonObject; error: Error }): Promise; +}; + +// @public +export type AuditorServiceEventSeverityLevel = + | 'low' + | 'medium' + | 'high' + | 'critical'; + // @public export interface AuthService { authenticate( diff --git a/packages/backend-plugin-api/src/services/definitions/AuditorService.ts b/packages/backend-plugin-api/src/services/definitions/AuditorService.ts index f7e62b6c1a..c98fe65df3 100644 --- a/packages/backend-plugin-api/src/services/definitions/AuditorService.ts +++ b/packages/backend-plugin-api/src/services/definitions/AuditorService.ts @@ -24,10 +24,14 @@ import type { Request } from 'express'; * critical: root permission changes * @public */ -export type AuditorEventSeverityLevel = 'low' | 'medium' | 'high' | 'critical'; +export type AuditorServiceEventSeverityLevel = + | 'low' + | 'medium' + | 'high' + | 'critical'; /** @public */ -export type AuditorCreateEvent = (options: { +export type AuditorServiceCreateEventOptions = { /** * Use kebab-case to name audit events (e.g., "user-login", "file-download", "fetch"). Represents a logical group of similar events or operations. For example, "fetch" could be used as an eventId encompassing various fetch methods like "by-id" or "by-location". * @@ -36,7 +40,7 @@ export type AuditorCreateEvent = (options: { eventId: string; /** (Optional) The severity level for the audit event. */ - severityLevel?: AuditorEventSeverityLevel; + severityLevel?: AuditorServiceEventSeverityLevel; /** (Optional) The associated HTTP request, if applicable. */ request?: Request; @@ -46,18 +50,14 @@ export type AuditorCreateEvent = (options: { * This could include a `queryType` field, using kebab-case, for variations within the main event (e.g., "by-id", "by-user"). * For example, if the `eventId` is "fetch", the `queryType` in `meta` could be "by-id" or "by-location". */ - meta?: TRootMeta; + meta?: JsonObject; +}; - /** (Optional) Suppresses the automatic initial event. */ - suppressInitialEvent?: boolean; -}) => Promise<{ - success(options?: { meta?: TMeta }): Promise; - fail( - options: { - meta?: TMeta; - } & ({ error: TError } | { errors: TError[] }), - ): Promise; -}>; +/** @public */ +export type AuditorServiceEvent = { + success(options?: { meta?: JsonObject }): Promise; + fail(options: { meta?: JsonObject; error: Error }): Promise; +}; /** * A service that provides an auditor facility. @@ -67,7 +67,7 @@ export type AuditorCreateEvent = (options: { * @public */ export interface AuditorService { - createEvent( - options: Parameters>[0], - ): ReturnType>; + createEvent( + options: AuditorServiceCreateEventOptions, + ): Promise; } diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index ea3d412bd1..d6346cbe4e 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -15,9 +15,10 @@ */ export type { - AuditorCreateEvent, - AuditorEventSeverityLevel, AuditorService, + AuditorServiceCreateEventOptions, + AuditorServiceEvent, + AuditorServiceEventSeverityLevel, } from './AuditorService'; export type { AuthService, @@ -33,7 +34,6 @@ export type { CacheServiceOptions, CacheServiceSetOptions, } from './CacheService'; -export { coreServices } from './coreServices'; export type { DatabaseService } from './DatabaseService'; export type { DiscoveryService } from './DiscoveryService'; export type { HttpAuthService } from './HttpAuthService'; @@ -86,3 +86,4 @@ export type { UrlReaderServiceSearchResponseFile, } from './UrlReaderService'; export type { BackstageUserInfo, UserInfoService } from './UserInfoService'; +export { coreServices } from './coreServices'; diff --git a/packages/backend-test-utils/report.api.md b/packages/backend-test-utils/report.api.md index fb7645eb7a..8c5601cd09 100644 --- a/packages/backend-test-utils/report.api.md +++ b/packages/backend-test-utils/report.api.md @@ -36,7 +36,6 @@ import { ParamsDictionary } from 'express-serve-static-core'; import { ParsedQs } from 'qs'; import { PermissionsRegistryService } from '@backstage/backend-plugin-api'; import { PermissionsService } from '@backstage/backend-plugin-api'; -import type { RootAuditorOptions } from '@backstage/backend-defaults/auditor'; import { RootConfigService } from '@backstage/backend-plugin-api'; import { RootHealthService } from '@backstage/backend-plugin-api'; import { RootHttpRouterService } from '@backstage/backend-plugin-api'; @@ -155,19 +154,11 @@ export function mockErrorHandler(): ErrorRequestHandler< // @public export namespace mockServices { - export function auditor( - options?: auditor.Options & { - pluginId?: string; - }, - ): AuditorService; + export function auditor(options?: { pluginId?: string }): AuditorService; // (undocumented) export namespace auditor { - // (undocumented) - export type Options = RootAuditorOptions; const // (undocumented) - factory: ( - options?: auditor.Options, - ) => ServiceFactory; + factory: () => ServiceFactory; const // (undocumented) mock: ( partialImpl?: Partial | undefined, diff --git a/packages/backend-test-utils/src/next/services/MockAuditorService.test.ts b/packages/backend-test-utils/src/next/services/MockAuditorService.test.ts index b11f633ea8..594301579b 100644 --- a/packages/backend-test-utils/src/next/services/MockAuditorService.test.ts +++ b/packages/backend-test-utils/src/next/services/MockAuditorService.test.ts @@ -25,31 +25,8 @@ describe('MockAuditorService', () => { jest.resetAllMocks(); }); - it('should log', async () => { - jest.spyOn(console, 'log').mockImplementation(() => {}); - - const pluginId = 'test-plugin'; - - const auditor = MockRootAuditorService.create().forPlugin({ - plugin: { - getId: () => pluginId, - }, - auth: new MockAuthService({ - pluginId, - disableDefaultAuthPolicy: false, - }), - httpAuth: new MockHttpAuthService(pluginId, mockCredentials.user()), - }); - - await auditor.createEvent({ - eventId: 'test-event', - }); - - expect(console.log).toHaveBeenCalled(); - }); - it('should send initiated log with createEvent', async () => { - jest.spyOn(console, 'log').mockImplementation(() => {}); + const spy = jest.spyOn(MockRootAuditorService.prototype, 'log'); const pluginId = 'test-plugin'; @@ -68,11 +45,11 @@ describe('MockAuditorService', () => { eventId: 'test-event', }); - expect(console.log).toHaveBeenCalled(); + expect(spy).toHaveBeenCalled(); }); it('should send succeeded log with createEvent', async () => { - jest.spyOn(console, 'log').mockImplementation(() => {}); + const spy = jest.spyOn(MockRootAuditorService.prototype, 'log'); const pluginId = 'test-plugin'; @@ -93,11 +70,11 @@ describe('MockAuditorService', () => { await auditorEvent.success(); - expect(console.log).toHaveBeenCalledTimes(2); + expect(spy).toHaveBeenCalledTimes(2); }); it('should send failed log with createEvent', async () => { - jest.spyOn(console, 'log').mockImplementation(() => {}); + const spy = jest.spyOn(MockRootAuditorService.prototype, 'log'); const pluginId = 'test-plugin'; @@ -118,6 +95,6 @@ describe('MockAuditorService', () => { await auditorEvent.fail({ error: new Error('error') as ErrorLike }); - expect(console.log).toHaveBeenCalledTimes(2); + expect(spy).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/backend-test-utils/src/next/services/MockAuditorService.ts b/packages/backend-test-utils/src/next/services/MockAuditorService.ts index 2d34ffae9e..0a734275ec 100644 --- a/packages/backend-test-utils/src/next/services/MockAuditorService.ts +++ b/packages/backend-test-utils/src/next/services/MockAuditorService.ts @@ -19,17 +19,19 @@ import type { AuditorEventOptions, } from '@backstage/backend-defaults/auditor'; import type { - AuditorCreateEvent, AuditorService, + AuditorServiceCreateEventOptions, + AuditorServiceEvent, AuthService, BackstageCredentials, HttpAuthService, PluginMetadataService, + RootLoggerService, } from '@backstage/backend-plugin-api'; import { ForwardedError } from '@backstage/errors'; import type { JsonObject } from '@backstage/types'; import type { Request } from 'express'; -import type { mockServices } from './mockServices'; +import { mockServices } from './mockServices'; export class MockAuditorService implements AuditorService { private readonly impl: MockRootAuditorService; @@ -69,12 +71,10 @@ export class MockAuditorService implements AuditorService { this.impl.log(auditEvent); } - async createEvent( - options: Parameters>[0], - ): ReturnType> { - if (!options.suppressInitialEvent) { - await this.log({ ...options, status: 'initiated' }); - } + async createEvent( + options: AuditorServiceCreateEventOptions, + ): Promise { + await this.log({ ...options, status: 'initiated' }); return { success: async params => { @@ -164,20 +164,29 @@ export class MockAuditorService implements AuditorService { } export class MockRootAuditorService { - private readonly options: mockServices.auditor.Options; + private readonly impl: RootLoggerService; - private constructor(options?: mockServices.auditor.Options) { - this.options = options ?? {}; + private constructor() { + this.impl = mockServices.rootLogger(); } - static create( - options?: mockServices.auditor.Options, - ): MockRootAuditorService { - return new MockRootAuditorService(options); + static create(): MockRootAuditorService { + return new MockRootAuditorService(); } - async log(auditEvent: AuditorEvent): Promise { - this.#log(...auditEvent); + async log(auditorEvent: AuditorEvent): Promise { + const [eventId, meta] = auditorEvent; + + // change `error` type to a string for logging purposes + let fields: Omit & { error?: string }; + + if ('error' in meta) { + fields = { ...meta, error: meta.error.toString() }; + } else { + fields = meta; + } + + this.impl.info(eventId, fields); } forPlugin(deps: { @@ -185,11 +194,7 @@ export class MockRootAuditorService { httpAuth: HttpAuthService; plugin: PluginMetadataService; }): AuditorService { - const impl = new MockRootAuditorService(this.options); + const impl = new MockRootAuditorService(); return MockAuditorService.create(impl, deps); } - - #log(message: string, meta?: AuditorEvent[1]) { - console.log(message, JSON.stringify(meta)); - } } diff --git a/packages/backend-test-utils/src/next/services/mockServices.ts b/packages/backend-test-utils/src/next/services/mockServices.ts index e5c8db185b..5aff29032c 100644 --- a/packages/backend-test-utils/src/next/services/mockServices.ts +++ b/packages/backend-test-utils/src/next/services/mockServices.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import type { RootAuditorOptions } from '@backstage/backend-defaults/auditor'; import { cacheServiceFactory } from '@backstage/backend-defaults/cache'; import { databaseServiceFactory } from '@backstage/backend-defaults/database'; import { HostDiscovery } from '@backstage/backend-defaults/discovery'; @@ -227,12 +226,7 @@ export namespace mockServices { /** * Creates a mock implementation of the `AuditorService`. */ - export function auditor( - options?: auditor.Options & { - pluginId?: string; - }, - ): AuditorService { - const service = 'backstage'; + export function auditor(options?: { pluginId?: string }): AuditorService { const pluginId = options?.pluginId ?? 'test'; const mockAuth = new MockAuthService({ pluginId, @@ -247,9 +241,7 @@ export namespace mockServices { getId: () => pluginId, }; - const auditorMock = MockRootAuditorService.create({ - meta: options?.meta ? { ...options.meta, service } : { service }, - }); + const auditorMock = MockRootAuditorService.create(); return auditorMock.forPlugin({ auth: mockAuth, @@ -259,9 +251,7 @@ export namespace mockServices { } export namespace auditor { - export type Options = RootAuditorOptions; - - export const factory = (options?: auditor.Options) => + export const factory = () => createServiceFactory({ service: coreServices.auditor, deps: { @@ -270,11 +260,7 @@ export namespace mockServices { plugin: coreServices.pluginMetadata, }, createRootContext() { - const service = 'backstage'; - - return MockRootAuditorService.create({ - meta: options?.meta ? { ...options.meta, service } : { service }, - }); + return MockRootAuditorService.create(); }, factory( { auth: mockAuth, httpAuth: mockHttpAuth, plugin: mockPlugin }, diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index dfad3d2f9a..20d02cf880 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -16,24 +16,22 @@ import { Backend, createSpecializedBackend } from '@backstage/backend-app-api'; import { + createServiceFactory, BackendFeature, ExtensionPoint, coreServices, createBackendModule, createBackendPlugin, - createServiceFactory, } from '@backstage/backend-plugin-api'; +import { mockServices } from '../services'; import { ConfigReader } from '@backstage/config'; import express from 'express'; -import { mockServices } from '../services'; // Direct internal import to avoid duplication // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { InternalBackendFeature, InternalBackendRegistrations, } from '../../../../backend-plugin-api/src/wiring/types'; -// eslint-disable-next-line @backstage/no-forbidden-package-imports -import { HostDiscovery } from '@backstage/backend-defaults/discovery'; import { DefaultRootHttpRouter, ExtendedHttpServer, @@ -41,8 +39,7 @@ import { createHealthRouter, createHttpServer, } from '@backstage/backend-defaults/rootHttpRouter'; -// Direct internal import to avoid duplication -// eslint-disable-next-line @backstage/no-forbidden-package-imports +import { HostDiscovery } from '@backstage/backend-defaults/discovery'; /** @public */ export interface TestBackendOptions { diff --git a/plugins/catalog-backend/src/service/CatalogBuilder.ts b/plugins/catalog-backend/src/service/CatalogBuilder.ts index 3a1b4d6650..bf7f31919c 100644 --- a/plugins/catalog-backend/src/service/CatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/CatalogBuilder.ts @@ -137,7 +137,6 @@ export type CatalogEnvironment = { database: DatabaseService; config: RootConfigService; reader: UrlReaderService; - // TODO: Require all services once `backend-legacy` is removed permissions: PermissionsService | PermissionAuthorizer; permissionsRegistry?: PermissionsRegistryService; scheduler?: SchedulerService; diff --git a/plugins/catalog-backend/src/service/createRouter.ts b/plugins/catalog-backend/src/service/createRouter.ts index 06f319fd7a..08e98d0548 100644 --- a/plugins/catalog-backend/src/service/createRouter.ts +++ b/plugins/catalog-backend/src/service/createRouter.ts @@ -330,10 +330,23 @@ export async function createRouter( }); writeSingleEntityResponse(res, entities, `No entity with uid ${uid}`); - + await auditorEvent?.success({ meta: { - entities: entities, + // stringify to entity refs + entities: entities.entities.reduce((arr, element) => { + if (!element) { + return arr; + } + + if (typeof element === 'string') { + arr.push(element); + return arr; + } + + arr.push(stringifyEntityRef(element)); + return arr; + }, [] as string[]), }, }); } catch (err) { @@ -793,7 +806,8 @@ export async function createRouter( const errors = processingResult.errors.map(e => serializeError(e)); await auditorEvent?.fail({ - errors: errors, + // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet + error: (AggregateError as any)(errors, 'Could not validate entity'), }); res.status(400).json({ diff --git a/plugins/scaffolder-backend/report.api.md b/plugins/scaffolder-backend/report.api.md index 9cf2a6a142..b542cb1c94 100644 --- a/plugins/scaffolder-backend/report.api.md +++ b/plugins/scaffolder-backend/report.api.md @@ -634,7 +634,6 @@ export class TaskManager implements TaskContext_2 { auth?: AuthService, config?: Config, additionalWorkspaceProviders?: Record, - auditor?: AuditorService, ): TaskManager; // (undocumented) get createdBy(): string | undefined; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 863f06c586..0fd18a9ee5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -75,7 +75,6 @@ export class TaskManager implements TaskContext { auth?: AuthService, config?: Config, additionalWorkspaceProviders?: Record, - auditor?: AuditorService, ) { const workspaceService = DefaultWorkspaceService.create( task, @@ -91,7 +90,6 @@ export class TaskManager implements TaskContext { logger, workspaceService, auth, - auditor, ); agent.startTimeout(); return agent; @@ -105,7 +103,6 @@ export class TaskManager implements TaskContext { private readonly logger: Logger, private readonly workspaceService: WorkspaceService, private readonly auth?: AuthService, - private readonly auditor?: AuditorService, ) {} get spec() { @@ -204,26 +201,6 @@ export class TaskManager implements TaskContext { if (this.heartbeatTimeoutId) { clearTimeout(this.heartbeatTimeoutId); } - - const auditorEvent = await this.auditor?.createEvent({ - eventId: 'task', - severityLevel: 'medium', - meta: { - actionType: 'execution', - taskId: this.task.taskId, - taskParameters: this.task.spec.parameters, - }, - // The initial event is created in TaskWorker - suppressInitialEvent: true, - }); - - if (result === 'failed') { - await auditorEvent?.fail({ - error: metadata?.error as any, - }); - } else { - await auditorEvent?.success(); - } } private startTimeout() { @@ -396,7 +373,6 @@ export class StorageTaskBroker implements TaskBroker { this.auth, this.config, this.additionalWorkspaceProviders, - this.auditor, ); } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 492d8f10b1..6c77ef3413 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -174,7 +174,7 @@ export class TaskWorker { } async runOneTask(task: TaskContext) { - await this.auditor?.createEvent({ + const auditorEvent = await this.auditor?.createEvent({ eventId: 'task', severityLevel: 'medium', meta: { @@ -197,8 +197,12 @@ export class TaskWorker { ); await task.complete('completed', { output }); + await auditorEvent?.success(); } catch (error) { assertError(error); + await auditorEvent?.fail({ + error, + }); await task.complete('failed', { error: { name: error.name, message: error.message }, }); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index c947ad77ff..a12e43615e 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -589,7 +589,13 @@ export async function createRouter( const result = validate(values, parameters); if (!result.valid) { - await auditorEvent?.fail({ errors: result.errors }); + await auditorEvent?.fail({ + // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet + error: (AggregateError as any)( + result.errors, + 'Could not create entity', + ), + }); res.status(400).json({ errors: result.errors }); return; @@ -987,7 +993,11 @@ export async function createRouter( const result = validate(body.values, parameters); if (!result.valid) { await auditorEvent?.fail({ - errors: result.errors, + // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet + error: (AggregateError as any)( + result.errors, + 'Could not execute dry run', + ), meta: { templateRef: templateRef, parameters: template.spec.parameters,