diff --git a/.changeset/gentle-ghosts-love.md b/.changeset/gentle-ghosts-love.md new file mode 100644 index 0000000000..34efebad84 --- /dev/null +++ b/.changeset/gentle-ghosts-love.md @@ -0,0 +1,19 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +- **BREAKING** - `DatabaseTaskStore()` constructor is now removed. Please use the `DatabaseTaskStore.create()` method instead. + +- **BREAKING** - `TaskStore.createTask()` method now only takes one argument of type `TaskStoreCreateTaskOptions` which encapsulates the `spec` and `secrets` + +```diff +- TaskStore.createTask(spec, secrets) ++ TaskStore.createTask({ spec, secrets}) +``` + +- **BREAKING** - `TaskBroker.dispatch()` method now only takes one argument of type `TaskBrokerDispatchOptions` which encapsulates the `spec` and `secrets` + +```diff +- TaskBroker.dispatch(spec, secrets) ++ TaskBroker.dispatch({ spec, secrets}) +``` diff --git a/.changeset/tender-apricots-fetch.md b/.changeset/tender-apricots-fetch.md new file mode 100644 index 0000000000..5fe8bda4ef --- /dev/null +++ b/.changeset/tender-apricots-fetch.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +- **DEPRECATED** - `Status` has been deprecated in favour of `TaskStatus` +- **DEPRECATED** - `CompletedTaskState` has been deprecated in favour of `TaskCompletionState` +- **DEPRECATED** - `DispatchResult` has been deprecated in favour of `TaskBrokerDispatchResult` diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 14b422048d..c0ff3b3f2b 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -50,8 +50,8 @@ export type ActionContext = { templateInfo?: TemplateInfo; }; -// @public -export type CompletedTaskState = 'failed' | 'completed'; +// @public @deprecated +export type CompletedTaskState = TaskCompletionState; // Warning: (ae-missing-release-tag) "createBuiltinActions" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -315,7 +315,6 @@ export type CreateWorkerOptions = { // @public export class DatabaseTaskStore implements TaskStore { - constructor(options: DatabaseTaskStoreOptions); // (undocumented) claimTask(): Promise; // (undocumented) @@ -325,7 +324,7 @@ export class DatabaseTaskStore implements TaskStore { eventBody, }: { taskId: string; - status: Status; + status: TaskStatus; eventBody: JsonObject; }): Promise; // Warning: (ae-forgotten-export) The symbol "DatabaseTaskStoreOptions" needs to be exported by the entry point index.d.ts @@ -334,11 +333,8 @@ export class DatabaseTaskStore implements TaskStore { static create(options: DatabaseTaskStoreOptions): Promise; // (undocumented) createTask( - spec: TaskSpec, - secrets?: TaskSecrets, - ): Promise<{ - taskId: string; - }>; + options: TaskStoreCreateTaskOptions, + ): Promise; // (undocumented) emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise; // (undocumented) @@ -357,10 +353,8 @@ export class DatabaseTaskStore implements TaskStore { }>; } -// @public -export type DispatchResult = { - taskId: string; -}; +// @public @deprecated +export type DispatchResult = TaskBrokerDispatchResult; // Warning: (ae-missing-release-tag) "fetchContents" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // @@ -449,7 +443,7 @@ export class ScaffolderEntitiesProcessor implements CatalogProcessor { export type SerializedTask = { id: string; spec: TaskSpec; - status: Status; + status: TaskStatus; createdAt: string; lastHeartbeatAt?: string; secrets?: TaskSecrets; @@ -464,20 +458,17 @@ export type SerializedTaskEvent = { createdAt: string; }; -// @public -export type Status = - | 'open' - | 'processing' - | 'failed' - | 'cancelled' - | 'completed'; +// @public @deprecated +export type Status = TaskStatus; // @public export interface TaskBroker { // (undocumented) claim(): Promise; // (undocumented) - dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise; + dispatch( + options: TaskBrokerDispatchOptions, + ): Promise; // (undocumented) get(taskId: string): Promise; // (undocumented) @@ -499,10 +490,24 @@ export interface TaskBroker { vacuumTasks(timeoutS: { timeoutS: number }): Promise; } +// @public +export type TaskBrokerDispatchOptions = { + spec: TaskSpec; + secrets?: TaskSecrets; +}; + +// @public +export type TaskBrokerDispatchResult = { + taskId: string; +}; + +// @public +export type TaskCompletionState = 'failed' | 'completed'; + // @public export interface TaskContext { // (undocumented) - complete(result: CompletedTaskState, metadata?: JsonValue): Promise; + complete(result: TaskCompletionState, metadata?: JsonValue): Promise; // (undocumented) done: boolean; // (undocumented) @@ -521,7 +526,7 @@ export type TaskEventType = 'completion' | 'log'; // @public export class TaskManager implements TaskContext { // (undocumented) - complete(result: CompletedTaskState, metadata?: JsonObject): Promise; + complete(result: TaskCompletionState, metadata?: JsonObject): Promise; // (undocumented) static create( state: TaskState, @@ -561,6 +566,14 @@ export interface TaskState { taskId: string; } +// @public +export type TaskStatus = + | 'open' + | 'processing' + | 'failed' + | 'cancelled' + | 'completed'; + // @public export interface TaskStore { // (undocumented) @@ -568,16 +581,13 @@ export interface TaskStore { // (undocumented) completeTask(options: { taskId: string; - status: Status; + status: TaskStatus; eventBody: JsonObject; }): Promise; // (undocumented) createTask( - task: TaskSpec, - secrets?: TaskSecrets, - ): Promise<{ - taskId: string; - }>; + options: TaskStoreCreateTaskOptions, + ): Promise; // (undocumented) emitLogEvent({ taskId, body }: TaskStoreEmitOptions): Promise; // (undocumented) @@ -596,6 +606,17 @@ export interface TaskStore { }>; } +// @public +export type TaskStoreCreateTaskOptions = { + spec: TaskSpec; + secrets?: TaskSecrets; +}; + +// @public +export type TaskStoreCreateTaskResult = { + taskId: string; +}; + // @public export type TaskStoreEmitOptions = { taskId: string; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index dcda3b921e..389ecae5c6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -22,13 +22,13 @@ import { v4 as uuid } from 'uuid'; import { SerializedTaskEvent, SerializedTask, - Status, + TaskStatus, TaskEventType, - TaskSecrets, - TaskSpec, TaskStore, TaskStoreEmitOptions, TaskStoreListEventsOptions, + TaskStoreCreateTaskOptions, + TaskStoreCreateTaskResult, } from './types'; import { DateTime } from 'luxon'; @@ -40,7 +40,7 @@ const migrationsDir = resolvePackagePath( export type RawDbTaskRow = { id: string; spec: string; - status: Status; + status: TaskStatus; last_heartbeat_at?: string; created_at: string; secrets?: string | null; @@ -80,7 +80,7 @@ export class DatabaseTaskStore implements TaskStore { return new DatabaseTaskStore(options); } - constructor(options: DatabaseTaskStoreOptions) { + private constructor(options: DatabaseTaskStoreOptions) { this.db = options.database; } @@ -108,14 +108,13 @@ export class DatabaseTaskStore implements TaskStore { } async createTask( - spec: TaskSpec, - secrets?: TaskSecrets, - ): Promise<{ taskId: string }> { + options: TaskStoreCreateTaskOptions, + ): Promise { const taskId = uuid(); await this.db('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 }; @@ -202,7 +201,7 @@ export class DatabaseTaskStore implements TaskStore { eventBody, }: { taskId: string; - status: Status; + status: TaskStatus; eventBody: JsonObject; }): Promise { let oldStatus: string; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index 93285a4ab3..daf444cdd8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/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(resolve => { const observedEvents = new Array(); @@ -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 diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 7e7ebe5436..49c1056727 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -17,16 +17,16 @@ import { JsonObject } from '@backstage/types'; import { assertError } from '@backstage/errors'; import { Logger } from 'winston'; import { - CompletedTaskState, + TaskCompletionState, TaskContext, TaskSecrets, TaskSpec, TaskStore, TaskBroker, - DispatchResult, SerializedTaskEvent, SerializedTask, } from './types'; +import { TaskBrokerDispatchOptions } from '.'; /** * TaskManager @@ -75,7 +75,7 @@ export class TaskManager implements TaskContext { } async complete( - result: CompletedTaskState, + result: TaskCompletionState, metadata?: JsonObject, ): Promise { await this.storage.completeTask({ @@ -155,10 +155,9 @@ export class StorageTaskBroker implements TaskBroker { } async dispatch( - spec: TaskSpec, - secrets?: TaskSecrets, - ): Promise { - 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, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 9b6baf3b84..9ff6b9c8a1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/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(); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts index f563a08df6..89404d3390 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -21,6 +21,7 @@ export type { CreateWorkerOptions } from './TaskWorker'; export type { TaskSecrets, TaskSpec, + TaskCompletionState, CompletedTaskState, TaskStoreEmitOptions, TaskStoreListEventsOptions, @@ -29,10 +30,15 @@ export type { TaskSpecV1beta2, TaskSpecV1beta3, Status, + TaskStatus, TaskEventType, TaskBroker, TaskContext, TaskStore, DispatchResult, + TaskBrokerDispatchResult, + TaskBrokerDispatchOptions, + TaskStoreCreateTaskOptions, + TaskStoreCreateTaskResult, TemplateMetadata, } from './types'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 8b28fe562c..c75b85f7b5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -32,11 +32,11 @@ export type { }; /** - * Status + * The status of each step of the Task * * @public */ -export type Status = +export type TaskStatus = | 'open' | 'processing' | 'failed' @@ -44,11 +44,27 @@ export type Status = | 'completed'; /** - * CompletedTaskState + * The status of each step of the Task + * + * @public + * @deprecated use TaskStatus instead + */ +export type Status = TaskStatus; + +/** + * The state of a completed task. * * @public */ -export type CompletedTaskState = 'failed' | 'completed'; +export type TaskCompletionState = 'failed' | 'completed'; + +/** + * The state of a completed task. + * + * @public + * @deprecated use TaskCompletionState instead + */ +export type CompletedTaskState = TaskCompletionState; /** * SerializedTask @@ -58,7 +74,7 @@ export type CompletedTaskState = 'failed' | 'completed'; export type SerializedTask = { id: string; spec: TaskSpec; - status: Status; + status: TaskStatus; createdAt: string; lastHeartbeatAt?: string; secrets?: TaskSecrets; @@ -94,14 +110,33 @@ export type TaskSecrets = Record & { }; /** - * DispatchResult + * The result of {@link TaskBroker.dispatch} * * @public */ -export type DispatchResult = { +export type TaskBrokerDispatchResult = { taskId: string; }; +/** + * The options passed to {@link TaskBroker.dispatch} + * Currently a spec and optional secrets + * + * @public + */ +export type TaskBrokerDispatchOptions = { + spec: TaskSpec; + secrets?: TaskSecrets; +}; + +/** + * DispatchResult + * + * @public + * @deprecated use TaskBrokerDispatchResult instead + */ +export type DispatchResult = TaskBrokerDispatchResult; + /** * Task * @@ -112,7 +147,7 @@ export interface TaskContext { secrets?: TaskSecrets; done: boolean; emitLog(message: string, metadata?: JsonValue): Promise; - complete(result: CompletedTaskState, metadata?: JsonValue): Promise; + complete(result: TaskCompletionState, metadata?: JsonValue): Promise; getWorkspaceName(): Promise; } @@ -123,7 +158,9 @@ export interface TaskContext { */ export interface TaskBroker { claim(): Promise; - dispatch(spec: TaskSpec, secrets?: TaskSecrets): Promise; + dispatch( + options: TaskBrokerDispatchOptions, + ): Promise; vacuumTasks(timeoutS: { timeoutS: number }): Promise; observe( options: { @@ -158,6 +195,23 @@ export type TaskStoreListEventsOptions = { after?: number | undefined; }; +/** + * The options passed to {@link TaskStore.createTask} + * @public + */ +export type TaskStoreCreateTaskOptions = { + spec: TaskSpec; + secrets?: TaskSecrets; +}; + +/** + * The response from {@link TaskStore.createTask} + * @public + */ +export type TaskStoreCreateTaskResult = { + taskId: string; +}; + /** * TaskStore * @@ -165,14 +219,13 @@ export type TaskStoreListEventsOptions = { */ export interface TaskStore { createTask( - task: TaskSpec, - secrets?: TaskSecrets, - ): Promise<{ taskId: string }>; + options: TaskStoreCreateTaskOptions, + ): Promise; getTask(taskId: string): Promise; claimTask(): Promise; completeTask(options: { taskId: string; - status: Status; + status: TaskStatus; eventBody: JsonObject; }): Promise; heartbeatTask(taskId: string): Promise; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 8a3645cf31..371fc47dc2 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/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 });