feat: rework the TaskWorker to just be concerned with the task delegation

Signed-off-by: blam <ben@blam.sh>

plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts

@@ -23,6 +23,8 @@ import { DatabaseTaskStore } from './DatabaseTaskStore';
 import { StorageTaskBroker } from './StorageTaskBroker';
 import { TaskWorker } from './TaskWorker';
 import { ScmIntegrations } from '@backstage/integration';
+import { WorkflowRunner } from './types';
+import { LegacyWorkflowRunner } from './LegacyWorkflowRunner';
 
 async function createStore(): Promise<DatabaseTaskStore> {
   const manager = DatabaseManager.fromConfig(
@@ -40,71 +42,91 @@ async function createStore(): Promise<DatabaseTaskStore> {
 
 describe('TaskWorker', () => {
   let storage: DatabaseTaskStore;
-  let actionRegistry = new TemplateActionRegistry();
+  const workflowRunner: WorkflowRunner = {
+    execute: jest.fn(),
+  } as unknown as WorkflowRunner;
 
-  const integrations = ScmIntegrations.fromConfig(
-    new ConfigReader({
-      integrations: {
-        github: [{ host: 'github.com', token: 'token' }],
-      },
-    }),
-  );
+  const legacyWorkflowRunner: LegacyWorkflowRunner = {
+    execute: jest.fn(),
+  } as unknown as LegacyWorkflowRunner;
 
   beforeAll(async () => {
     storage = await createStore();
   });
 
   beforeEach(() => {
-    actionRegistry = new TemplateActionRegistry();
-    actionRegistry.register({
-      id: 'test-action',
-      handler: async ctx => {
-        ctx.output('testOutput', 'winning');
-        ctx.output('badOutput', false);
-      },
-    });
+    jest.resetAllMocks();
   });
 
   const logger = getVoidLogger();
 
-  it('should fail when action does not exist', async () => {
+  it('should call the legacy workflow runner when the apiVersion is not beta3', async () => {
     const broker = new StorageTaskBroker(storage, logger);
     const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
       taskBroker: broker,
-      integrations,
+      runners: {
+        legacyWorkflowRunner,
+        workflowRunner,
+      },
     });
-    const { taskId } = await broker.dispatch({
+
+    await broker.dispatch({
+      apiVersion: 'backstage.io/v1beta2',
       steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
       output: {
         result: '{{ steps.test.output.testOutput }}',
       },
       values: {},
     });
+
     const task = await broker.claim();
     await taskWorker.runOneTask(task);
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
 
-    expect((event?.body?.error as JsonObject)?.message).toBe(
-      "Template action with ID 'not-found-action' is not registered.",
-    );
+    expect(legacyWorkflowRunner.execute).toHaveBeenCalled();
   });
 
-  it('should template output', async () => {
+  it('should call the default workflow runner when the apiVersion is beta3', async () => {
     const broker = new StorageTaskBroker(storage, logger);
     const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
       taskBroker: broker,
-      integrations,
+      runners: {
+        legacyWorkflowRunner,
+        workflowRunner,
+      },
+    });
+
+    await broker.dispatch({
+      apiVersion: 'backstage.io/v1beta3',
+      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+      output: {
+        result: '{{ steps.test.output.testOutput }}',
+      },
+      values: {},
+    });
+
+    const task = await broker.claim();
+    await taskWorker.runOneTask(task);
+
+    expect(workflowRunner.execute).toHaveBeenCalled();
+  });
+
+  it('should save the output to the task', async () => {
+    (workflowRunner.execute as jest.Mock).mockResolvedValue({
+      output: { testOutput: 'testmockoutput' },
+    });
+
+    const broker = new StorageTaskBroker(storage, logger);
+    const taskWorker = new TaskWorker({
+      taskBroker: broker,
+      runners: {
+        legacyWorkflowRunner,
+        workflowRunner,
+      },
     });
 
     const { taskId } = await broker.dispatch({
-      steps: [{ id: 'test', name: 'test', action: 'test-action' }],
+      apiVersion: 'backstage.io/v1beta3',
+      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
       output: {
         result: '{{ steps.test.output.testOutput }}',
       },
@@ -116,375 +138,6 @@ describe('TaskWorker', () => {
 
     const { events } = await storage.listEvents({ taskId });
     const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toBe('winning');
-  });
-
-  it('should template input', async () => {
-    const inputAction = createTemplateAction<{
-      name: string;
-    }>({
-      id: 'test-input',
-      schema: {
-        input: {
-          type: 'object',
-          required: ['name'],
-          properties: {
-            name: {
-              title: 'name',
-              description: 'Enter name',
-              type: 'string',
-            },
-          },
-        },
-      },
-      async handler(ctx) {
-        if (ctx.input.name !== 'winning') {
-          throw new Error(
-            `expected name to be "winning" got ${ctx.input.name}`,
-          );
-        }
-      },
-    });
-    actionRegistry.register(inputAction);
-
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        { id: 'test', name: 'test', action: 'test-action' },
-        {
-          id: 'test-input',
-          name: 'test-input',
-          action: 'test-input',
-          input: {
-            name: '{{ steps.test.output.testOutput }}',
-          },
-        },
-      ],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
-      },
-      values: {},
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toBe('winning');
-  });
-
-  it('should execute steps conditionally', async () => {
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        { id: 'test', name: 'test', action: 'test-action' },
-        {
-          id: 'conditional',
-          name: 'conditional',
-          action: 'test-action',
-          if: '{{ steps.test.output.testOutput }}',
-        },
-      ],
-      output: {
-        result: '{{ steps.conditional.output.testOutput }}',
-      },
-      values: {},
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toBe('winning');
-  });
-
-  it('should execute steps conditionally with eq helper', async () => {
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        { id: 'test', name: 'test', action: 'test-action' },
-        {
-          id: 'conditional',
-          name: 'conditional',
-          action: 'test-action',
-          if: '{{ eq steps.test.output.testOutput "winning" }}',
-        },
-      ],
-      output: {
-        result: '{{ steps.conditional.output.testOutput }}',
-      },
-      values: {},
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toBe('winning');
-  });
-
-  it('should skip steps conditionally', async () => {
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        { id: 'test', name: 'test', action: 'test-action' },
-        {
-          id: 'conditional',
-          name: 'conditional',
-          action: 'test-action',
-          if: '{{ steps.test.output.badOutput }}',
-        },
-      ],
-      output: {
-        result: '{{ steps.conditional.output.testOutput }}',
-      },
-      values: {},
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-    expect((event?.body?.output as JsonObject).result).toBeUndefined();
-  });
-
-  it('should parse strings as objects if possible', async () => {
-    const inputAction = createTemplateAction<{
-      address: { line1: string };
-      list: string[];
-      address2: string;
-    }>({
-      id: 'test-input',
-      schema: {
-        input: {
-          type: 'object',
-          required: ['address'],
-          properties: {
-            address: {
-              title: 'address',
-              description: 'Enter name',
-              type: 'object',
-              properties: {
-                line1: {
-                  type: 'string',
-                },
-              },
-            },
-            address2: {
-              type: 'string',
-            },
-            list: {
-              type: 'array',
-              items: {
-                type: 'string',
-              },
-            },
-          },
-        },
-      },
-      async handler(ctx) {
-        if (ctx.input.list.length !== 1) {
-          throw new Error(
-            `expected list to have length "1" got ${ctx.input.list.length}`,
-          );
-        }
-        if (ctx.input.address.line1 !== 'line 1') {
-          throw new Error(
-            `expected address.line1 to be "line 1" got ${ctx.input.address.line1}`,
-          );
-        }
-
-        if (ctx.input.address2 !== '{"not valid"}') {
-          throw new Error(
-            `expected address2 to be "{"not valid"}" got ${ctx.input.address2}`,
-          );
-        }
-        ctx.output('address', ctx.input.address.line1);
-      },
-    });
-    actionRegistry.register(inputAction);
-
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        {
-          id: 'test-input',
-          name: 'test-input',
-          action: 'test-input',
-          input: {
-            address: JSON.stringify({ line1: 'line 1' }),
-            list: JSON.stringify(['hey!']),
-            address2: '{"not valid"}',
-          },
-        },
-      ],
-      output: {
-        result: '{{ steps.test-input.output.address }}',
-      },
-      values: {},
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-
-    expect((event?.body?.output as JsonObject).result).toBe('line 1');
-  });
-
-  // TODO(blam): Can delete this test when we make the helpers a public API
-  it('should provide a repoUrlParse helper for the templates', async () => {
-    const inputAction = createTemplateAction<{
-      destination: RepoSpec;
-    }>({
-      id: 'test-input',
-      schema: {
-        input: {
-          type: 'object',
-          required: ['destination'],
-          properties: {
-            destination: {
-              title: 'destination',
-              type: 'object',
-              properties: {
-                repo: {
-                  type: 'string',
-                },
-                host: {
-                  type: 'string',
-                },
-                owner: {
-                  type: 'string',
-                },
-                organization: {
-                  type: 'string',
-                },
-                workspace: {
-                  type: 'string',
-                },
-                project: {
-                  type: 'string',
-                },
-              },
-            },
-          },
-        },
-      },
-      async handler(ctx) {
-        ctx.output('host', ctx.input.destination.host);
-        ctx.output('repo', ctx.input.destination.repo);
-
-        if (ctx.input.destination.owner) {
-          ctx.output('owner', ctx.input.destination.owner);
-        }
-
-        if (ctx.input.destination.host !== 'github.com') {
-          throw new Error(
-            `expected host to be "github.com" got ${ctx.input.destination.host}`,
-          );
-        }
-
-        if (ctx.input.destination.repo !== 'repo') {
-          throw new Error(
-            `expected repo to be "repo" got ${ctx.input.destination.repo}`,
-          );
-        }
-
-        if (
-          ctx.input.destination.owner &&
-          ctx.input.destination.owner !== 'owner'
-        ) {
-          throw new Error(
-            `expected repo to be "owner" got ${ctx.input.destination.owner}`,
-          );
-        }
-      },
-    });
-    actionRegistry.register(inputAction);
-
-    const broker = new StorageTaskBroker(storage, logger);
-    const taskWorker = new TaskWorker({
-      logger,
-      workingDirectory: os.tmpdir(),
-      actionRegistry,
-      taskBroker: broker,
-      integrations,
-    });
-
-    const { taskId } = await broker.dispatch({
-      steps: [
-        {
-          id: 'test-input',
-          name: 'test-input',
-          action: 'test-input',
-          input: {
-            destination: '{{ parseRepoUrl parameters.repoUrl }}',
-          },
-        },
-      ],
-      output: {
-        host: '{{ steps.test-input.output.host }}',
-        repo: '{{ steps.test-input.output.repo }}',
-        owner: '{{ steps.test-input.output.owner }}',
-      },
-      values: {
-        repoUrl: 'github.com?repo=repo&owner=owner',
-      },
-    });
-
-    const task = await broker.claim();
-    await taskWorker.runOneTask(task);
-
-    const { events } = await storage.listEvents({ taskId });
-    const event = events.find(e => e.type === 'completion');
-
-    expect((event?.body?.output as JsonObject).host).toBe('github.com');
-    expect((event?.body?.output as JsonObject).repo).toBe('repo');
-    expect((event?.body?.output as JsonObject).owner).toBe('owner');
+    expect(event?.body.output).toEqual({ testOutput: 'testmockoutput' });
   });
 });

plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts

@@ -13,39 +13,19 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-import { JsonObject, JsonValue } from '@backstage/config';
-import { InputError } from '@backstage/errors';
-import fs from 'fs-extra';
-
-import path from 'path';
-
-import { Logger } from 'winston';
-import { parseRepoUrl } from '../actions/builtin/publish/util';
-import { TemplateActionRegistry } from '../actions/TemplateActionRegistry';
-import { isTruthy } from './helper';
 import { Task, TaskBroker, WorkflowRunner } from './types';
-import { ScmIntegrations } from '@backstage/integration';
 import { LegacyWorkflowRunner } from './LegacyWorkflowRunner';
-import { DefaultWorkflowRunner } from './DefaultWorkflowRunner';
 
 type Options = {
-  logger: Logger;
   taskBroker: TaskBroker;
-  workingDirectory: string;
-  actionRegistry: TemplateActionRegistry;
-  integrations: ScmIntegrations;
+  runners: {
+    legacyWorkflowRunner: LegacyWorkflowRunner;
+    workflowRunner: WorkflowRunner;
+  };
 };
 
 export class TaskWorker {
-  private readonly legacyWorkflowRunner: LegacyWorkflowRunner;
-  private readonly workflowRunner: WorkflowRunner;
-
-  constructor(private readonly options: Options) {
-    this.legacyWorkflowRunner = new LegacyWorkflowRunner(options);
-    this.workflowRunner = new DefaultWorkflowRunner(options);
-  }
-
+  constructor(private readonly options: Options) {}
   start() {
     (async () => {
       for (;;) {
@@ -59,8 +39,8 @@ export class TaskWorker {
     try {
       const { output } =
         task.spec.apiVersion === 'backstage.io/v1beta3'
-          ? await this.workflowRunner.execute(task)
-          : await this.legacyWorkflowRunner.execute(task);
+          ? await this.options.runners.workflowRunner.execute(task)
+          : await this.options.runners.legacyWorkflowRunner.execute(task);
 
       await task.complete('completed', { output });
     } catch (error) {