From d781df18437df3eb15149ce292158b0527f637ac Mon Sep 17 00:00:00 2001 From: Brian Fletcher Date: Tue, 26 Oct 2021 11:34:13 +0100 Subject: [PATCH] hide workflow runners behind createWorker Also renames legacy and default workflow runners. Signed-off-by: Brian Fletcher --- plugins/scaffolder-backend/api-report.md | 19 +---- .../sample-templates/all-templates.yaml | 1 - .../bitbucket-demo/template/catalog-info.yaml | 4 +- .../src/scaffolder/index.ts | 3 +- .../tasks/DefaultWorkflowRunner.test.ts | 6 +- ...wRunner.ts => HandlebarsWorkflowRunner.ts} | 2 +- .../tasks/LegacyWorkflowRunner.test.ts | 6 +- ...lowRunner.ts => NunjucksWorkflowRunner.ts} | 2 +- .../src/scaffolder/tasks/TaskWorker.test.ts | 74 +++++++++++++------ .../src/scaffolder/tasks/TaskWorker.ts | 12 +-- .../src/scaffolder/tasks/types.ts | 5 ++ .../scaffolder-backend/src/service/router.ts | 25 ++----- 12 files changed, 82 insertions(+), 77 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/tasks/{LegacyWorkflowRunner.ts => HandlebarsWorkflowRunner.ts} (99%) rename plugins/scaffolder-backend/src/scaffolder/tasks/{DefaultWorkflowRunner.ts => NunjucksWorkflowRunner.ts} (99%) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index a94fa8a2a0..d0f4cb36e8 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -412,6 +412,9 @@ export interface TaskBroker { vacuumTasks(timeoutS: { timeoutS: number }): Promise; } +// @public +export type TaskEventType = 'completion' | 'log'; + // @public export type TaskSecrets = { token: string | undefined; @@ -519,7 +522,6 @@ export type TaskStoreListEventsOptions = { // @public export class TaskWorker { - constructor(options: TaskWorkerOptions); // (undocumented) static createWorker(options: CreateWorkerOptions): TaskWorker; // (undocumented) @@ -528,15 +530,6 @@ export class TaskWorker { start(): void; } -// @public -export type TaskWorkerOptions = { - taskBroker: TaskBroker; - runners: { - legacyWorkflowRunner: LegacyWorkflowRunner; - workflowRunner: WorkflowRunner; - }; -}; - // Warning: (ae-missing-release-tag) "TemplateAction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -563,10 +556,4 @@ export class TemplateActionRegistry { action: TemplateAction, ): void; } - -// Warnings were encountered during analysis: -// -// src/scaffolder/tasks/TaskWorker.d.ts:14:9 - (ae-forgotten-export) The symbol "LegacyWorkflowRunner" needs to be exported by the entry point index.d.ts -// src/scaffolder/tasks/TaskWorker.d.ts:15:9 - (ae-forgotten-export) The symbol "WorkflowRunner" needs to be exported by the entry point index.d.ts -// src/scaffolder/tasks/types.d.ts:37:5 - (ae-forgotten-export) The symbol "TaskEventType" needs to be exported by the entry point index.d.ts ``` diff --git a/plugins/scaffolder-backend/sample-templates/all-templates.yaml b/plugins/scaffolder-backend/sample-templates/all-templates.yaml index 2d5adb8e3b..6637c9479b 100644 --- a/plugins/scaffolder-backend/sample-templates/all-templates.yaml +++ b/plugins/scaffolder-backend/sample-templates/all-templates.yaml @@ -6,7 +6,6 @@ metadata: spec: targets: - ./remote-templates.yaml - # For local development of a template, you can reference your local templates here. # Examples: # diff --git a/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template/catalog-info.yaml b/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template/catalog-info.yaml index 875664d2a8..3b685a57e0 100644 --- a/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template/catalog-info.yaml +++ b/plugins/scaffolder-backend/sample-templates/bitbucket-demo/template/catalog-info.yaml @@ -1,8 +1,8 @@ apiVersion: backstage.io/v1alpha1 kind: Component metadata: - name: {{cookiecutter.name | jsonify}} + name: { { cookiecutter.name | jsonify } } spec: type: website lifecycle: experimental - owner: {{cookiecutter.owner | jsonify}} + owner: { { cookiecutter.owner | jsonify } } diff --git a/plugins/scaffolder-backend/src/scaffolder/index.ts b/plugins/scaffolder-backend/src/scaffolder/index.ts index b2599f20ed..6b0e033b7d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/index.ts @@ -15,7 +15,7 @@ */ export * from './actions'; export { DatabaseTaskStore, TaskWorker } from './tasks'; -export type { TaskWorkerOptions, CreateWorkerOptions } from './tasks'; +export type { CreateWorkerOptions } from './tasks'; export type { TaskState } from './tasks'; export type { TaskSecrets, @@ -28,4 +28,5 @@ export type { TaskSpecV1beta2, TaskSpecV1beta3, Status, + TaskEventType, } from './tasks/types'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index d6d1b0cfd2..0a843f4ddf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -18,7 +18,7 @@ import mockFs from 'mock-fs'; import * as winston from 'winston'; import { getVoidLogger } from '@backstage/backend-common'; -import { DefaultWorkflowRunner } from './DefaultWorkflowRunner'; +import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { ConfigReader } from '@backstage/config'; @@ -27,7 +27,7 @@ import { Task, TaskSpec } from './types'; describe('DefaultWorkflowRunner', () => { const logger = getVoidLogger(); let actionRegistry = new TemplateActionRegistry(); - let runner: DefaultWorkflowRunner; + let runner: NunjucksWorkflowRunner; let fakeActionHandler: jest.Mock; const integrations = ScmIntegrations.fromConfig( @@ -88,7 +88,7 @@ describe('DefaultWorkflowRunner', () => { }, }); - runner = new DefaultWorkflowRunner({ + runner = new NunjucksWorkflowRunner({ actionRegistry, integrations, workingDirectory: '/tmp', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts similarity index 99% rename from plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts index 6d43ae978c..831ef98784 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts @@ -47,7 +47,7 @@ const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta2 => * This is the legacy workflow runner, which supports handlebars. This entire implementation will be replaced * with the default workflow runner interface in the future so this entire thing can go bye bye. */ -export class LegacyWorkflowRunner implements WorkflowRunner { +export class HandlebarsWorkflowRunner implements WorkflowRunner { private readonly handlebars: typeof Handlebars; constructor(private readonly options: Options) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts index 2714f1b80c..aa46747c68 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts @@ -20,12 +20,12 @@ import { createTemplateAction, TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { ConfigReader } from '@backstage/config'; import { getVoidLogger } from '@backstage/backend-common'; -import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; +import { HandlebarsWorkflowRunner } from './HandlebarsWorkflowRunner'; import { Task, TaskSpec } from './types'; import { RepoSpec } from '../actions/builtin/publish/util'; describe('LegacyWorkflowRunner', () => { - let runner: LegacyWorkflowRunner; + let runner: HandlebarsWorkflowRunner; const logger = getVoidLogger(); let actionRegistry = new TemplateActionRegistry(); @@ -60,7 +60,7 @@ describe('LegacyWorkflowRunner', () => { }, }); - runner = new LegacyWorkflowRunner({ + runner = new HandlebarsWorkflowRunner({ actionRegistry, integrations, workingDirectory: '/tmp', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts similarity index 99% rename from plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index e56befd939..5cceb16324 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -77,7 +77,7 @@ const createStepLogger = ({ task, step }: { task: Task; step: TaskStep }) => { return { taskLogger, streamLogger }; }; -export class DefaultWorkflowRunner implements WorkflowRunner { +export class NunjucksWorkflowRunner implements WorkflowRunner { private readonly nunjucks: nunjucks.Environment; private readonly nunjucksOptions: nunjucks.ConfigureOptions = { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index f6b332b6d3..3cb73abb85 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -19,8 +19,20 @@ import { ConfigReader } from '@backstage/config'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker } from './StorageTaskBroker'; import { TaskWorker } from './TaskWorker'; -import { WorkflowRunner } from './types'; -import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; +import { HandlebarsWorkflowRunner } from './HandlebarsWorkflowRunner'; +import { ScmIntegrations } from '@backstage/integration'; +import { TemplateActionRegistry } from '../actions'; +import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; + +jest.mock('./HandlebarsWorkflowRunner'); +const MockedHandlebarsWorkflowRunner = + HandlebarsWorkflowRunner as jest.Mock; +MockedHandlebarsWorkflowRunner.mockImplementation(); + +jest.mock('./NunjucksWorkflowRunner'); +const MockedNunjucksWorkflowRunner = + NunjucksWorkflowRunner as jest.Mock; +MockedNunjucksWorkflowRunner.mockImplementation(); async function createStore(): Promise { const manager = DatabaseManager.fromConfig( @@ -40,13 +52,19 @@ async function createStore(): Promise { describe('TaskWorker', () => { let storage: DatabaseTaskStore; - const workflowRunner: WorkflowRunner = { - execute: jest.fn(), - } as unknown as WorkflowRunner; - const legacyWorkflowRunner: LegacyWorkflowRunner = { + const integrations: ScmIntegrations = {} as ScmIntegrations; + + const actionRegistry: TemplateActionRegistry = {} as TemplateActionRegistry; + const workingDirectory = '/tmp/scaffolder'; + + const handlebarsWorkflowRunner: HandlebarsWorkflowRunner = { execute: jest.fn(), - } as unknown as LegacyWorkflowRunner; + } as unknown as HandlebarsWorkflowRunner; + + const workflowRunner: NunjucksWorkflowRunner = { + execute: jest.fn(), + } as unknown as NunjucksWorkflowRunner; beforeAll(async () => { storage = await createStore(); @@ -54,18 +72,22 @@ describe('TaskWorker', () => { beforeEach(() => { jest.resetAllMocks(); + MockedHandlebarsWorkflowRunner.mockImplementation( + () => handlebarsWorkflowRunner, + ); + MockedNunjucksWorkflowRunner.mockImplementation(() => workflowRunner); }); const logger = getVoidLogger(); it('should call the legacy workflow runner when the apiVersion is not beta3', async () => { const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ + const taskWorker = TaskWorker.createWorker({ + logger, + workingDirectory, + integrations, taskBroker: broker, - runners: { - legacyWorkflowRunner, - workflowRunner, - }, + actionRegistry, }); await broker.dispatch({ @@ -80,17 +102,23 @@ describe('TaskWorker', () => { const task = await broker.claim(); await taskWorker.runOneTask(task); - expect(legacyWorkflowRunner.execute).toHaveBeenCalled(); + expect(MockedHandlebarsWorkflowRunner).toBeCalledWith({ + actionRegistry, + integrations, + logger, + workingDirectory, + }); + expect(handlebarsWorkflowRunner.execute).toHaveBeenCalled(); }); it('should call the default workflow runner when the apiVersion is beta3', async () => { const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ + const taskWorker = TaskWorker.createWorker({ + logger, + workingDirectory, + integrations, taskBroker: broker, - runners: { - legacyWorkflowRunner, - workflowRunner, - }, + actionRegistry, }); await broker.dispatch({ @@ -114,12 +142,12 @@ describe('TaskWorker', () => { }); const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ + const taskWorker = TaskWorker.createWorker({ + logger, + workingDirectory, + integrations, taskBroker: broker, - runners: { - legacyWorkflowRunner, - workflowRunner, - }, + actionRegistry, }); const { taskId } = await broker.dispatch({ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 6e70ea06a6..fcc215be99 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -15,8 +15,8 @@ */ import { Task, TaskBroker, WorkflowRunner } from './types'; -import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; -import { DefaultWorkflowRunner } from './DefaultWorkflowRunner'; +import { HandlebarsWorkflowRunner } from './HandlebarsWorkflowRunner'; +import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { Logger } from 'winston'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; @@ -30,7 +30,7 @@ import { assertError } from '@backstage/errors'; export type TaskWorkerOptions = { taskBroker: TaskBroker; runners: { - legacyWorkflowRunner: LegacyWorkflowRunner; + legacyWorkflowRunner: HandlebarsWorkflowRunner; workflowRunner: WorkflowRunner; }; }; @@ -54,7 +54,7 @@ export type CreateWorkerOptions = { * @public */ export class TaskWorker { - constructor(private readonly options: TaskWorkerOptions) {} + private constructor(private readonly options: TaskWorkerOptions) {} static createWorker(options: CreateWorkerOptions) { const { @@ -65,14 +65,14 @@ export class TaskWorker { workingDirectory, } = options; - const legacyWorkflowRunner = new LegacyWorkflowRunner({ + const legacyWorkflowRunner = new HandlebarsWorkflowRunner({ logger, actionRegistry, integrations, workingDirectory, }); - const workflowRunner = new DefaultWorkflowRunner({ + const workflowRunner = new NunjucksWorkflowRunner({ actionRegistry, integrations, logger, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index cd4d291719..f2e13923ec 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -49,6 +49,11 @@ export type SerializedTask = { secrets?: TaskSecrets; }; +/** + * TaskEventType + * + * @public + */ export type TaskEventType = 'completion' | 'log'; /** diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 0bc00a2778..fcd5551be2 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -40,8 +40,6 @@ import { TemplateEntityV1beta3 } from '@backstage/plugin-scaffolder-common'; import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '../scaffolder/actions'; import { createBuiltinActions } from '../scaffolder/actions/builtin/createBuiltinActions'; -import { LegacyWorkflowRunner } from '../scaffolder/tasks/LegacyWorkflowRunner'; -import { DefaultWorkflowRunner } from '../scaffolder/tasks/DefaultWorkflowRunner'; import { TaskBroker, TaskSpec } from '../scaffolder/tasks/types'; /** @@ -98,28 +96,15 @@ export async function createRouter( const taskBroker = options.taskBroker || new StorageTaskBroker(databaseTaskStore, logger); const actionRegistry = new TemplateActionRegistry(); - const legacyWorkflowRunner = new LegacyWorkflowRunner({ - logger, - actionRegistry, - integrations, - workingDirectory, - }); - - const workflowRunner = new DefaultWorkflowRunner({ - actionRegistry, - integrations, - logger, - workingDirectory, - }); const workers = []; for (let i = 0; i < (taskWorkers || 1); i++) { - const worker = new TaskWorker({ + const worker = TaskWorker.createWorker({ taskBroker, - runners: { - legacyWorkflowRunner, - workflowRunner, - }, + actionRegistry, + integrations, + logger, + workingDirectory, }); workers.push(worker); }