make some breaking changes to TaskStore and TaskBroker

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

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

@@ -24,11 +24,11 @@ import {
   SerializedTask,
   TaskStatus,
   TaskEventType,
-  TaskSecrets,
-  TaskSpec,
   TaskStore,
   TaskStoreEmitOptions,
   TaskStoreListEventsOptions,
+  TaskStoreCreateTaskOptions,
+  TaskStoreCreateTaskResult,
 } from './types';
 import { DateTime } from 'luxon';
 
@@ -108,14 +108,13 @@ export class DatabaseTaskStore implements TaskStore {
   }
 
   async createTask(
-    spec: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{ taskId: string }> {
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult> {
     const taskId = uuid();
     await this.db<RawDbTaskRow>('tasks').insert({
       id: taskId,
-      spec: JSON.stringify(spec),
-      secrets: secrets ? JSON.stringify(secrets) : undefined,
+      spec: JSON.stringify(options.spec),
+      secrets: options.secrets ? JSON.stringify(options.secrets) : undefined,
       status: 'open',
     });
     return { taskId };

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

@@ -47,7 +47,7 @@ describe('StorageTaskBroker', () => {
   const logger = getVoidLogger();
   it('should claim a dispatched work item', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({} as TaskSpec);
+    await broker.dispatch({ spec: {} as TaskSpec });
     await expect(broker.claim()).resolves.toEqual(expect.any(TaskManager));
   });
 
@@ -57,15 +57,15 @@ describe('StorageTaskBroker', () => {
 
     await expect(Promise.race([promise, 'waiting'])).resolves.toBe('waiting');
 
-    await broker.dispatch({} as TaskSpec);
+    await broker.dispatch({ spec: {} as TaskSpec });
     await expect(promise).resolves.toEqual(expect.any(TaskManager));
   });
 
   it('should dispatch multiple items and claim them in order', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({ steps: [{ id: 'a' }] } as TaskSpec);
-    await broker.dispatch({ steps: [{ id: 'b' }] } as TaskSpec);
-    await broker.dispatch({ steps: [{ id: 'c' }] } as TaskSpec);
+    await broker.dispatch({ spec: { steps: [{ id: 'a' }] } as TaskSpec });
+    await broker.dispatch({ spec: { steps: [{ id: 'b' }] } as TaskSpec });
+    await broker.dispatch({ spec: { steps: [{ id: 'c' }] } as TaskSpec });
 
     const taskA = await broker.claim();
     const taskB = await broker.claim();
@@ -80,14 +80,14 @@ describe('StorageTaskBroker', () => {
 
   it('should store secrets', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    await broker.dispatch({} as TaskSpec, fakeSecrets);
+    await broker.dispatch({ spec: {} as TaskSpec, secrets: fakeSecrets });
     const task = await broker.claim();
     expect(task.secrets).toEqual(fakeSecrets);
   }, 10000);
 
   it('should complete a task', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec);
+    const dispatchResult = await broker.dispatch({ spec: {} as TaskSpec });
     const task = await broker.claim();
     await task.complete('completed');
     const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -96,7 +96,10 @@ describe('StorageTaskBroker', () => {
 
   it('should remove secrets after picking up a task', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets);
+    const dispatchResult = await broker.dispatch({
+      spec: {} as TaskSpec,
+      secrets: fakeSecrets,
+    });
     await broker.claim();
 
     const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -105,7 +108,7 @@ describe('StorageTaskBroker', () => {
 
   it('should fail a task', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    const dispatchResult = await broker.dispatch({} as TaskSpec);
+    const dispatchResult = await broker.dispatch({ spec: {} as TaskSpec });
     const task = await broker.claim();
     await task.complete('failed');
     const taskRow = await storage.getTask(dispatchResult.taskId);
@@ -116,7 +119,7 @@ describe('StorageTaskBroker', () => {
     const broker1 = new StorageTaskBroker(storage, logger);
     const broker2 = new StorageTaskBroker(storage, logger);
 
-    const { taskId } = await broker1.dispatch({} as TaskSpec);
+    const { taskId } = await broker1.dispatch({ spec: {} as TaskSpec });
 
     const logPromise = new Promise<SerializedTaskEvent[]>(resolve => {
       const observedEvents = new Array<SerializedTaskEvent>();
@@ -155,7 +158,7 @@ describe('StorageTaskBroker', () => {
 
   it('should heartbeat', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    const { taskId } = await broker.dispatch({} as TaskSpec);
+    const { taskId } = await broker.dispatch({ spec: {} as TaskSpec });
     const task = await broker.claim();
 
     const initialTask = await storage.getTask(taskId);
@@ -173,7 +176,7 @@ describe('StorageTaskBroker', () => {
 
   it('should be update the status to failed if heartbeat fails', async () => {
     const broker = new StorageTaskBroker(storage, logger);
-    const { taskId } = await broker.dispatch({} as TaskSpec);
+    const { taskId } = await broker.dispatch({ spec: {} as TaskSpec });
     const task = await broker.claim();
 
     jest

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

@@ -23,10 +23,10 @@ import {
   TaskSpec,
   TaskStore,
   TaskBroker,
-  DispatchResult,
   SerializedTaskEvent,
   SerializedTask,
 } from './types';
+import { TaskBrokerDispatchOptions } from '.';
 
 /**
  * TaskManager
@@ -155,10 +155,9 @@ export class StorageTaskBroker implements TaskBroker {
   }
 
   async dispatch(
-    spec: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<DispatchResult> {
-    const taskRow = await this.storage.createTask(spec, secrets);
+    options: TaskBrokerDispatchOptions,
+  ): Promise<{ taskId: string }> {
+    const taskRow = await this.storage.createTask(options);
     this.signalDispatch();
     return {
       taskId: taskRow.taskId,

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

@@ -91,12 +91,14 @@ describe('TaskWorker', () => {
     });
 
     await broker.dispatch({
-      apiVersion: 'backstage.io/v1beta2',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'backstage.io/v1beta2',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        values: {},
       },
-      values: {},
     });
 
     const task = await broker.claim();
@@ -122,12 +124,14 @@ describe('TaskWorker', () => {
     });
 
     await broker.dispatch({
-      apiVersion: 'scaffolder.backstage.io/v1beta3',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'scaffolder.backstage.io/v1beta3',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        parameters: {},
       },
-      parameters: {},
     });
 
     const task = await broker.claim();
@@ -151,12 +155,14 @@ describe('TaskWorker', () => {
     });
 
     const { taskId } = await broker.dispatch({
-      apiVersion: 'scaffolder.backstage.io/v1beta3',
-      steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
-      output: {
-        result: '{{ steps.test.output.testOutput }}',
+      spec: {
+        apiVersion: 'scaffolder.backstage.io/v1beta3',
+        steps: [{ id: 'test', name: 'test', action: 'not-found-action' }],
+        output: {
+          result: '{{ steps.test.output.testOutput }}',
+        },
+        parameters: {},
       },
-      parameters: {},
     });
 
     const task = await broker.claim();

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

@@ -36,5 +36,7 @@ export type {
   TaskContext,
   TaskStore,
   DispatchResult,
+  TaskBrokerDispatchResult,
+  TaskBrokerDispatchOptions,
   TemplateMetadata,
 } from './types';

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

@@ -74,7 +74,7 @@ export type CompletedTaskState = TaskCompletionState;
 export type SerializedTask = {
   id: string;
   spec: TaskSpec;
-  status: Status;
+  status: TaskStatus;
   createdAt: string;
   lastHeartbeatAt?: string;
   secrets?: TaskSecrets;
@@ -110,14 +110,31 @@ export type TaskSecrets = Record<string, string> & {
 };
 
 /**
- * DispatchResult
+ * The result of the TaskBroker Dispatch
  *
  * @public
  */
-export type DispatchResult = {
+export type TaskBrokerDispatchResult = {
   taskId: string;
 };
 
+/**
+ * The options passed to TaskBroker Dispatch
+ * Currently a spec and optional secrets
+ *
+ * @public
+ */
+export type TaskBrokerDispatchOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+/**
+ * DispatchResult
+ * @deprecated use TaskBrokerDispatchResult instead
+ */
+export type DispatchResult = TaskBrokerDispatchResult;
+
 /**
  * Task
  *
@@ -128,7 +145,7 @@ export interface TaskContext {
   secrets?: TaskSecrets;
   done: boolean;
   emitLog(message: string, metadata?: JsonValue): Promise<void>;
-  complete(result: CompletedTaskState, metadata?: JsonValue): Promise<void>;
+  complete(result: TaskCompletionState, metadata?: JsonValue): Promise<void>;
   getWorkspaceName(): Promise<string>;
 }
 
@@ -139,7 +156,9 @@ export interface TaskContext {
  */
 export interface TaskBroker {
   claim(): Promise<TaskContext>;
-  dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise<DispatchResult>;
+  dispatch(
+    options: TaskBrokerDispatchOptions,
+  ): Promise<TaskBrokerDispatchResult>;
   vacuumTasks(timeoutS: { timeoutS: number }): Promise<void>;
   observe(
     options: {
@@ -174,6 +193,23 @@ export type TaskStoreListEventsOptions = {
   after?: number | undefined;
 };
 
+/**
+ * The options passed to TaskStore create
+ * @public
+ */
+export type TaskStoreCreateTaskOptions = {
+  spec: TaskSpec;
+  secrets?: TaskSecrets;
+};
+
+/**
+ * The response from TaskStoreCreate
+ * @public
+ */
+export type TaskStoreCreateTaskResult = {
+  taskId: string;
+};
+
 /**
  * TaskStore
  *
@@ -181,14 +217,13 @@ export type TaskStoreListEventsOptions = {
  */
 export interface TaskStore {
   createTask(
-    task: TaskSpec,
-    secrets?: TaskSecrets,
-  ): Promise<{ taskId: string }>;
+    options: TaskStoreCreateTaskOptions,
+  ): Promise<TaskStoreCreateTaskResult>;
   getTask(taskId: string): Promise<SerializedTask>;
   claimTask(): Promise<SerializedTask | undefined>;
   completeTask(options: {
     taskId: string;
-    status: Status;
+    status: TaskStatus;
     eventBody: JsonObject;
   }): Promise<void>;
   heartbeatTask(taskId: string): Promise<void>;

plugins/scaffolder-backend/src/service/router.ts

@@ -266,9 +266,12 @@ export async function createRouter(
         );
       }
 
-      const result = await taskBroker.dispatch(taskSpec, {
-        ...req.body.secrets,
-        backstageToken: token,
+      const result = await taskBroker.dispatch({
+        spec: taskSpec,
+        secrets: {
+          ...req.body.secrets,
+          backstageToken: token,
+        },
       });
 
       res.status(201).json({ id: result.taskId });