From acb684e1c63e96a17b7de89b01f5f196807c8bc4 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 4 Feb 2024 19:55:02 +0100 Subject: [PATCH 01/23] Checkpoint Signed-off-by: bnechyporenko --- .../package.json | 2 + .../src/actions/github.ts | 1 + .../src/actions/helpers.ts | 169 +++++++++++------- .../migrations/20240203232000_state.js | 35 ++++ plugins/scaffolder-backend/src/index.ts | 1 + .../src/scaffolder/tasks/DatabaseTaskStore.ts | 13 ++ .../tasks/NunjucksWorkflowRunner.ts | 8 + .../src/scaffolder/tasks/StorageTaskBroker.ts | 14 +- .../src/scaffolder/tasks/types.ts | 9 +- .../src/util/defineCheckpoint.ts | 34 ++++ plugins/scaffolder-node/src/actions/types.ts | 4 + plugins/scaffolder-node/src/tasks/index.ts | 1 + plugins/scaffolder-node/src/tasks/types.ts | 10 ++ yarn.lock | 2 + 14 files changed, 237 insertions(+), 66 deletions(-) create mode 100644 plugins/scaffolder-backend/migrations/20240203232000_state.js create mode 100644 plugins/scaffolder-backend/src/util/defineCheckpoint.ts diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index a0cccb536c..f48feaaae5 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -37,7 +37,9 @@ "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", + "@backstage/plugin-scaffolder-backend": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", + "@backstage/types": "workspace:^", "@octokit/webhooks": "^10.0.0", "libsodium-wrappers": "^0.7.11", "octokit": "^3.0.0", diff --git a/plugins/scaffolder-backend-module-github/src/actions/github.ts b/plugins/scaffolder-backend-module-github/src/actions/github.ts index 91d7a34aea..ed55714852 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/github.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/github.ts @@ -244,6 +244,7 @@ export function createPublishGithubAction(options: { repoVariables, secrets, ctx.logger, + ctx.checkpoint, ); const remoteUrl = newRepo.clone_url; diff --git a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts index 4d95a6494f..87c125eda2 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts @@ -36,6 +36,8 @@ import { enableBranchProtectionOnDefaultRepoBranch, entityRefToName, } from './gitHelpers'; +import { JsonObject } from '@backstage/types'; +import { defineCheckpoint } from '@backstage/plugin-scaffolder-backend'; const DEFAULT_TIMEOUT_MS = 60_000; @@ -138,6 +140,10 @@ export async function createGithubRepoWithCollaboratorsAndTopics( repoVariables: { [key: string]: string } | undefined, secrets: { [key: string]: string } | undefined, logger: Logger, + checkpoint?: ( + key: string, + fn: () => Promise, + ) => Promise, ) { // eslint-disable-next-line testing-library/no-await-sync-queries const user = await client.rest.users.getByUsername({ @@ -148,59 +154,70 @@ export async function createGithubRepoWithCollaboratorsAndTopics( await validateAccessTeam(client, access); } - const repoCreationPromise = - user.data.type === 'Organization' - ? client.rest.repos.createInOrg({ - name: repo, - org: owner, - private: repoVisibility === 'private', - // @ts-ignore https://github.com/octokit/types.ts/issues/522 - visibility: repoVisibility, - description: description, - delete_branch_on_merge: deleteBranchOnMerge, - allow_merge_commit: allowMergeCommit, - allow_squash_merge: allowSquashMerge, - squash_merge_commit_title: squashMergeCommitTitle, - squash_merge_commit_message: squashMergeCommitMessage, - allow_rebase_merge: allowRebaseMerge, - allow_auto_merge: allowAutoMerge, - homepage: homepage, - has_projects: hasProjects, - has_wiki: hasWiki, - has_issues: hasIssues, - }) - : client.rest.repos.createForAuthenticatedUser({ - name: repo, - private: repoVisibility === 'private', - description: description, - delete_branch_on_merge: deleteBranchOnMerge, - allow_merge_commit: allowMergeCommit, - allow_squash_merge: allowSquashMerge, - squash_merge_commit_title: squashMergeCommitTitle, - squash_merge_commit_message: squashMergeCommitMessage, - allow_rebase_merge: allowRebaseMerge, - allow_auto_merge: allowAutoMerge, - homepage: homepage, - has_projects: hasProjects, - has_wiki: hasWiki, - has_issues: hasIssues, - }); + const repoCreation = async () => { + const repoCreationPromise = + user.data.type === 'Organization' + ? client.rest.repos.createInOrg({ + name: repo, + org: owner, + private: repoVisibility === 'private', + // @ts-ignore https://github.com/octokit/types.ts/issues/522 + visibility: repoVisibility, + description: description, + delete_branch_on_merge: deleteBranchOnMerge, + allow_merge_commit: allowMergeCommit, + allow_squash_merge: allowSquashMerge, + squash_merge_commit_title: squashMergeCommitTitle, + squash_merge_commit_message: squashMergeCommitMessage, + allow_rebase_merge: allowRebaseMerge, + allow_auto_merge: allowAutoMerge, + homepage: homepage, + has_projects: hasProjects, + has_wiki: hasWiki, + has_issues: hasIssues, + }) + : client.rest.repos.createForAuthenticatedUser({ + name: repo, + private: repoVisibility === 'private', + description: description, + delete_branch_on_merge: deleteBranchOnMerge, + allow_merge_commit: allowMergeCommit, + allow_squash_merge: allowSquashMerge, + squash_merge_commit_title: squashMergeCommitTitle, + squash_merge_commit_message: squashMergeCommitMessage, + allow_rebase_merge: allowRebaseMerge, + allow_auto_merge: allowAutoMerge, + homepage: homepage, + has_projects: hasProjects, + has_wiki: hasWiki, + has_issues: hasIssues, + }); - let newRepo; + let newRepo; - try { - newRepo = (await repoCreationPromise).data; - } catch (e) { - assertError(e); - if (e.message === 'Resource not accessible by integration') { - logger.warn( - `The GitHub app or token provided may not have the required permissions to create the ${user.data.type} repository ${owner}/${repo}.`, + try { + newRepo = (await repoCreationPromise).data; + } catch (e) { + assertError(e); + if (e.message === 'Resource not accessible by integration') { + logger.warn( + `The GitHub app or token provided may not have the required permissions to create the ${user.data.type} repository ${owner}/${repo}.`, + ); + } + throw new Error( + `Failed to create the ${user.data.type} repository ${owner}/${repo}, ${e.message}`, ); } - throw new Error( - `Failed to create the ${user.data.type} repository ${owner}/${repo}, ${e.message}`, - ); - } + return { newRepo }; + }; + + const { newRepo } = await defineCheckpoint<{ + newRepo: { clone_url: string; html_url: string }; + }>({ + key: 'v1.task.checkpoint.repo.creation', + checkpoint, + fn: repoCreation, + }); if (access?.startsWith(`${owner}/`)) { const [, team] = access.split('/'); @@ -213,11 +230,19 @@ export async function createGithubRepoWithCollaboratorsAndTopics( }); // No need to add access if it's the person who owns the personal account } else if (access && access !== owner) { - await client.rest.repos.addCollaborator({ - owner, - repo, - username: access, - permission: 'admin', + const addCollaborator = async () => { + await client.rest.repos.addCollaborator({ + owner, + repo, + username: access, + permission: 'admin', + }); + return {}; + }; + await defineCheckpoint({ + key: 'v1.task.checkpoint.add.collaborator', + checkpoint, + fn: addCollaborator, }); } @@ -225,11 +250,19 @@ export async function createGithubRepoWithCollaboratorsAndTopics( for (const collaborator of collaborators) { try { if ('user' in collaborator) { - await client.rest.repos.addCollaborator({ - owner, - repo, - username: entityRefToName(collaborator.user), - permission: collaborator.access, + const addCollaborator = async () => { + await client.rest.repos.addCollaborator({ + owner, + repo, + username: entityRefToName(collaborator.user), + permission: collaborator.access, + }); + return {}; + }; + await defineCheckpoint({ + key: `v1.task.checkpoint.add.collaborator.${collaborator.user}`, + checkpoint, + fn: addCollaborator, }); } else if ('team' in collaborator) { await client.rest.teams.addOrUpdateRepoPermissionsInOrg({ @@ -264,11 +297,19 @@ export async function createGithubRepoWithCollaboratorsAndTopics( } for (const [key, value] of Object.entries(repoVariables ?? {})) { - await client.rest.actions.createRepoVariable({ - owner, - repo, - name: key, - value: value, + const createRepoVariable = async () => { + await client.rest.actions.createRepoVariable({ + owner, + repo, + name: key, + value: value, + }); + return {}; + }; + await defineCheckpoint({ + key: `v1.task.checkpoint.create.repo.variable.${key}`, + checkpoint, + fn: createRepoVariable, }); } diff --git a/plugins/scaffolder-backend/migrations/20240203232000_state.js b/plugins/scaffolder-backend/migrations/20240203232000_state.js new file mode 100644 index 0000000000..9ffea1716e --- /dev/null +++ b/plugins/scaffolder-backend/migrations/20240203232000_state.js @@ -0,0 +1,35 @@ +/* + * Copyright 2020 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. + */ + +// @ts-check + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + await knex.schema.alterTable('tasks', table => { + table.text('state').nullable().comment('A state of the checkpoints'); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.alterTable('tasks', table => { + table.dropColumn('state'); + }); +}; diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index 649a5df233..d59499739e 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -23,5 +23,6 @@ export * from './scaffolder'; export * from './service/router'; export * from './lib'; +export { defineCheckpoint } from './util/defineCheckpoint'; export * from './deprecated'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 766db4646e..e6083a469f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -30,6 +30,7 @@ import { TaskStoreCreateTaskResult, TaskStoreShutDownTaskOptions, TaskStoreRecoverTaskOptions, + TaskStoreStateOptions, } from './types'; import { SerializedTaskEvent, @@ -52,6 +53,7 @@ export type RawDbTaskRow = { id: string; spec: string; status: TaskStatus; + state?: string; last_heartbeat_at?: string; created_at: string; created_by: string | null; @@ -394,6 +396,17 @@ export class DatabaseTaskStore implements TaskStore { }); } + async saveCheckpoint?(options: TaskStoreStateOptions): Promise { + if (options.state) { + const serializedState = JSON.stringify(options.state); + await this.db('tasks') + .where({ id: options.taskId }) + .update({ + state: serializedState, + }); + } + } + async listEvents( options: TaskStoreListEventsOptions, ): Promise<{ events: SerializedTaskEvent[] }> { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index ce1ad71c33..190c880280 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -349,6 +349,14 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { logger: taskLogger, logStream: streamLogger, workspacePath, + async checkpoint( + key: string, + fn: () => Promise, + ) { + const value = await fn(); + task.updateCheckpoint?.(key, value); + return value; + }, createTemporaryDirectory: async () => { const tmpDir = await fs.mkdtemp( `${workspacePath}_step-${step.id}-`, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 8b49f492e8..8762253436 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -16,7 +16,7 @@ import { Config } from '@backstage/config'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; -import { TaskSecrets } from '@backstage/plugin-scaffolder-node'; +import { TaskSecrets, TaskState } from '@backstage/plugin-scaffolder-node'; import { JsonObject, Observable } from '@backstage/types'; import { Logger } from 'winston'; import ObservableImpl from 'zen-observable'; @@ -91,6 +91,14 @@ export class TaskManager implements TaskContext { }); } + async updateCheckpoint?(key: string, value: JsonObject): Promise { + this.task.state = { [key]: value }; + await this.storage.saveCheckpoint?.({ + taskId: this.task.taskId, + state: this.task.state, + }); + } + async complete( result: TaskCompletionState, metadata?: JsonObject, @@ -144,6 +152,10 @@ export interface CurrentClaimedTask { * The secrets that are stored with the task. */ secrets?: TaskSecrets; + /** + * The state of checkpoints of the task. + */ + state?: TaskState; /** * The creator of the task. */ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index c5783ccb49..b6225d46a9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -16,7 +16,7 @@ import { JsonValue, JsonObject, HumanDuration } from '@backstage/types'; import { TaskSpec, TaskStep } from '@backstage/plugin-scaffolder-common'; -import { TaskSecrets } from '@backstage/plugin-scaffolder-node'; +import { TaskSecrets, TaskState } from '@backstage/plugin-scaffolder-node'; import { TemplateAction, TaskStatus as _TaskStatus, @@ -113,6 +113,11 @@ export type TaskStoreEmitOptions = { body: TBody; }; +export type TaskStoreStateOptions = { + taskId: string; + state?: TaskState; +}; + /** * TaskStoreListEventsOptions * @@ -194,6 +199,8 @@ export interface TaskStore { emitLogEvent(options: TaskStoreEmitOptions): Promise; + saveCheckpoint?(options: TaskStoreStateOptions): Promise; + listEvents( options: TaskStoreListEventsOptions, ): Promise<{ events: SerializedTaskEvent[] }>; diff --git a/plugins/scaffolder-backend/src/util/defineCheckpoint.ts b/plugins/scaffolder-backend/src/util/defineCheckpoint.ts new file mode 100644 index 0000000000..6656005684 --- /dev/null +++ b/plugins/scaffolder-backend/src/util/defineCheckpoint.ts @@ -0,0 +1,34 @@ +/* + * Copyright 2021 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 { JsonObject } from '@backstage/types'; + +export type DefineCheckpointProps = { + checkpoint?: (key: string, fn: () => Promise) => Promise; + key: string; + fn: () => Promise; +}; + +export const defineCheckpoint = async ( + props: DefineCheckpointProps, +): Promise => { + const { checkpoint, fn, key } = props; + return checkpoint + ? checkpoint?.(key, async () => { + return await fn(); + }) + : fn(); +}; diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 7e8c2b4f5f..a7f44c4730 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -35,6 +35,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; + checkpoint?( + key: string, + fn: () => Promise, + ): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 930de95237..99638e48af 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -24,5 +24,6 @@ export type { TaskCompletionState, TaskContext, TaskEventType, + TaskState, TaskStatus, } from './types'; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 7cdc044bdf..7136211671 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -26,6 +26,13 @@ export type TaskSecrets = Record & { backstageToken?: string; }; +/** + * TaskState + * + * @public + */ +export type TaskState = Record; + /** * The status of each step of the Task * @@ -110,6 +117,7 @@ export interface TaskContext { cancelSignal: AbortSignal; spec: TaskSpec; secrets?: TaskSecrets; + state?: TaskState; createdBy?: string; done: boolean; isDryRun?: boolean; @@ -118,6 +126,8 @@ export interface TaskContext { emitLog(message: string, logMetadata?: JsonObject): Promise; + updateCheckpoint?(key: string, value: JsonObject): Promise; + getWorkspaceName(): Promise; } diff --git a/yarn.lock b/yarn.lock index 8c0ddcabc2..4a18d8c628 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8354,7 +8354,9 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/integration": "workspace:^" + "@backstage/plugin-scaffolder-backend": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" + "@backstage/types": "workspace:^" "@octokit/webhooks": ^10.0.0 "@types/libsodium-wrappers": ^0.7.10 fs-extra: 10.1.0 From 245617991c9d43cf3bdeb76c5ca3da45007beaec Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 4 Feb 2024 20:43:59 +0100 Subject: [PATCH 02/23] Checkpoint Signed-off-by: bnechyporenko --- .../tasks/NunjucksWorkflowRunner.ts | 21 +++++++++++--- .../src/scaffolder/tasks/StorageTaskBroker.ts | 14 +++++++-- .../src/util/defineCheckpoint.ts | 8 ++--- plugins/scaffolder-node/src/tasks/index.ts | 1 + plugins/scaffolder-node/src/tasks/types.ts | 29 +++++++++++++++++-- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 190c880280..d746cf859f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -26,7 +26,7 @@ import fs from 'fs-extra'; import path from 'path'; import nunjucks from 'nunjucks'; import { JsonArray, JsonObject, JsonValue } from '@backstage/types'; -import { InputError, NotAllowedError } from '@backstage/errors'; +import { InputError, NotAllowedError, stringifyError } from '@backstage/errors'; import { PassThrough } from 'stream'; import { generateExampleOutput, isTruthy } from './helper'; import { validate as validateJsonSchema } from 'jsonschema'; @@ -353,9 +353,22 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { key: string, fn: () => Promise, ) { - const value = await fn(); - task.updateCheckpoint?.(key, value); - return value; + try { + const value = await fn(); + task.updateCheckpoint?.({ + key, + status: 'success', + value, + }); + return value; + } catch (err) { + task.updateCheckpoint?.({ + key, + status: 'failed', + reason: stringifyError(err), + }); + throw err; + } }, createTemporaryDirectory: async () => { const tmpDir = await fs.mkdtemp( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 8762253436..01722edba9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -16,7 +16,11 @@ import { Config } from '@backstage/config'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; -import { TaskSecrets, TaskState } from '@backstage/plugin-scaffolder-node'; +import { + TaskSecrets, + TaskState, + UpdateCheckpointOptions, +} from '@backstage/plugin-scaffolder-node'; import { JsonObject, Observable } from '@backstage/types'; import { Logger } from 'winston'; import ObservableImpl from 'zen-observable'; @@ -91,8 +95,12 @@ export class TaskManager implements TaskContext { }); } - async updateCheckpoint?(key: string, value: JsonObject): Promise { - this.task.state = { [key]: value }; + async updateCheckpoint?(options: UpdateCheckpointOptions): Promise { + if (this.task.state) { + this.task.state[options.key] = { ...options }; + } else { + this.task.state = { [options.key]: options }; + } await this.storage.saveCheckpoint?.({ taskId: this.task.taskId, state: this.task.state, diff --git a/plugins/scaffolder-backend/src/util/defineCheckpoint.ts b/plugins/scaffolder-backend/src/util/defineCheckpoint.ts index 6656005684..c349bdc7f3 100644 --- a/plugins/scaffolder-backend/src/util/defineCheckpoint.ts +++ b/plugins/scaffolder-backend/src/util/defineCheckpoint.ts @@ -16,15 +16,11 @@ import { JsonObject } from '@backstage/types'; -export type DefineCheckpointProps = { +export const defineCheckpoint = async (props: { checkpoint?: (key: string, fn: () => Promise) => Promise; key: string; fn: () => Promise; -}; - -export const defineCheckpoint = async ( - props: DefineCheckpointProps, -): Promise => { +}): Promise => { const { checkpoint, fn, key } = props; return checkpoint ? checkpoint?.(key, async () => { diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 99638e48af..4e66f1c0e3 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -26,4 +26,5 @@ export type { TaskEventType, TaskState, TaskStatus, + UpdateCheckpointOptions, } from './types'; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 7136211671..f57353c917 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -31,7 +31,14 @@ export type TaskSecrets = Record & { * * @public */ -export type TaskState = Record; +export type TaskState = { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonObject; + }; +}; /** * The status of each step of the Task @@ -108,6 +115,24 @@ export type TaskBrokerDispatchOptions = { createdBy?: string; }; +/** + * The options passed to {@link TaskBroker.updateCheckpoint} + * Parameters to store the result of the executed checkpoint + * + * @public + */ +export type UpdateCheckpointOptions = + | { + key: string; + status: 'success'; + value: JsonObject; + } + | { + key: string; + status: 'failed'; + reason: string; + }; + /** * Task * @@ -126,7 +151,7 @@ export interface TaskContext { emitLog(message: string, logMetadata?: JsonObject): Promise; - updateCheckpoint?(key: string, value: JsonObject): Promise; + updateCheckpoint?(options: UpdateCheckpointOptions): Promise; getWorkspaceName(): Promise; } From 911557bae827cecb5de6cadb76b9d52f6e0f1faf Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 4 Feb 2024 21:09:34 +0100 Subject: [PATCH 03/23] Checkpoint Signed-off-by: bnechyporenko --- packages/backend-next/src/index.ts | 1 + plugins/scaffolder-backend-module-github/package.json | 2 +- .../src/actions/helpers.ts | 11 ++++++----- plugins/scaffolder-backend/src/index.ts | 1 - .../src}/defineCheckpoint.ts | 0 plugins/scaffolder-common/src/index.ts | 2 ++ yarn.lock | 3 ++- 7 files changed, 12 insertions(+), 8 deletions(-) rename plugins/{scaffolder-backend/src/util => scaffolder-common/src}/defineCheckpoint.ts (100%) diff --git a/packages/backend-next/src/index.ts b/packages/backend-next/src/index.ts index 53a51fcfa7..dd99bab5d7 100644 --- a/packages/backend-next/src/index.ts +++ b/packages/backend-next/src/index.ts @@ -41,6 +41,7 @@ backend.add( backend.add(import('@backstage/plugin-permission-backend/alpha')); backend.add(import('@backstage/plugin-proxy-backend/alpha')); backend.add(import('@backstage/plugin-scaffolder-backend/alpha')); +backend.add(import('@backstage/plugin-scaffolder-backend-module-github')); backend.add(import('@backstage/plugin-search-backend-module-catalog/alpha')); backend.add(import('@backstage/plugin-search-backend-module-explore/alpha')); backend.add(import('@backstage/plugin-search-backend-module-techdocs/alpha')); diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index f48feaaae5..b3f5c856b3 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -37,7 +37,7 @@ "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", - "@backstage/plugin-scaffolder-backend": "workspace:^", + "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", "@backstage/types": "workspace:^", "@octokit/webhooks": "^10.0.0", diff --git a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts index 87c125eda2..f89440e4d5 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts @@ -37,7 +37,7 @@ import { entityRefToName, } from './gitHelpers'; import { JsonObject } from '@backstage/types'; -import { defineCheckpoint } from '@backstage/plugin-scaffolder-backend'; +import { defineCheckpoint } from '@backstage/plugin-scaffolder-common'; const DEFAULT_TIMEOUT_MS = 60_000; @@ -208,11 +208,12 @@ export async function createGithubRepoWithCollaboratorsAndTopics( `Failed to create the ${user.data.type} repository ${owner}/${repo}, ${e.message}`, ); } - return { newRepo }; + return { clone_url: newRepo.clone_url, html_url: newRepo.html_url }; }; - const { newRepo } = await defineCheckpoint<{ - newRepo: { clone_url: string; html_url: string }; + const { clone_url, html_url } = await defineCheckpoint<{ + clone_url: string; + html_url: string; }>({ key: 'v1.task.checkpoint.repo.creation', checkpoint, @@ -345,7 +346,7 @@ export async function createGithubRepoWithCollaboratorsAndTopics( } } - return newRepo; + return { clone_url, html_url }; } export async function initRepoPushAndProtect( diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index d59499739e..649a5df233 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -23,6 +23,5 @@ export * from './scaffolder'; export * from './service/router'; export * from './lib'; -export { defineCheckpoint } from './util/defineCheckpoint'; export * from './deprecated'; diff --git a/plugins/scaffolder-backend/src/util/defineCheckpoint.ts b/plugins/scaffolder-common/src/defineCheckpoint.ts similarity index 100% rename from plugins/scaffolder-backend/src/util/defineCheckpoint.ts rename to plugins/scaffolder-common/src/defineCheckpoint.ts diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index 4d9b5e39c6..c0434c3cdb 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -34,3 +34,5 @@ export type { TemplatePermissionsV1beta3, TemplateRecoveryV1beta3, } from './TemplateEntityV1beta3'; + +export { defineCheckpoint } from './defineCheckpoint'; diff --git a/yarn.lock b/yarn.lock index 6ffa8fc541..3020b8cb26 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8376,7 +8376,7 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/integration": "workspace:^" - "@backstage/plugin-scaffolder-backend": "workspace:^" + "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" "@backstage/types": "workspace:^" "@octokit/webhooks": ^10.0.0 @@ -27101,6 +27101,7 @@ __metadata: "@backstage/plugin-playlist-backend": "workspace:^" "@backstage/plugin-proxy-backend": "workspace:^" "@backstage/plugin-scaffolder-backend": "workspace:^" + "@backstage/plugin-scaffolder-backend-module-github": "workspace:^" "@backstage/plugin-search-backend": "workspace:^" "@backstage/plugin-search-backend-module-catalog": "workspace:^" "@backstage/plugin-search-backend-module-explore": "workspace:^" From 07c2c2e7a793576318aec0c25609c13c765540e4 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 4 Feb 2024 21:30:32 +0100 Subject: [PATCH 04/23] Checkpoint Signed-off-by: bnechyporenko --- packages/backend-next/package.json | 1 + .../src/scaffolder/tasks/DatabaseTaskStore.ts | 14 +++++++++++++- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 8 +++++++- .../src/scaffolder/tasks/StorageTaskBroker.ts | 8 ++++++-- .../src/scaffolder/tasks/types.ts | 6 ++++++ plugins/scaffolder-node/src/tasks/index.ts | 2 +- plugins/scaffolder-node/src/tasks/types.ts | 8 +++++--- 7 files changed, 39 insertions(+), 8 deletions(-) diff --git a/packages/backend-next/package.json b/packages/backend-next/package.json index eca36208a9..aca0acbdd3 100644 --- a/packages/backend-next/package.json +++ b/packages/backend-next/package.json @@ -52,6 +52,7 @@ "@backstage/plugin-playlist-backend": "workspace:^", "@backstage/plugin-proxy-backend": "workspace:^", "@backstage/plugin-scaffolder-backend": "workspace:^", + "@backstage/plugin-scaffolder-backend-module-github": "workspace:^", "@backstage/plugin-search-backend": "workspace:^", "@backstage/plugin-search-backend-module-catalog": "workspace:^", "@backstage/plugin-search-backend-module-explore": "workspace:^", diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index e6083a469f..14535d034c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -38,6 +38,7 @@ import { TaskStatus, TaskEventType, TaskSecrets, + TaskState, } from '@backstage/plugin-scaffolder-node'; import { DateTime, Duration } from 'luxon'; import { TaskRecovery, TaskSpec } from '@backstage/plugin-scaffolder-common'; @@ -396,7 +397,18 @@ export class DatabaseTaskStore implements TaskStore { }); } - async saveCheckpoint?(options: TaskStoreStateOptions): Promise { + async listCheckpoints({ + taskId, + }: { + taskId: string; + }): Promise<{ state: TaskState }> { + const state = await this.db('tasks') + .where({ id: taskId }) + .select('state'); + return { state: JSON.stringify(state) as unknown as TaskState }; + } + + async saveCheckpoint(options: TaskStoreStateOptions): Promise { if (options.state) { const serializedState = JSON.stringify(options.state); await this.db('tasks') diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index d746cf859f..407c623793 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -332,6 +332,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } const tmpDirs = new Array(); const stepOutput: { [outputName: string]: JsonValue } = {}; + const prevTaskState = await task.getCheckpoints?.(); for (const iteration of iterations) { if (iteration.each) { @@ -354,7 +355,12 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { fn: () => Promise, ) { try { - const value = await fn(); + let prevValue: U | undefined; + if (prevTaskState) { + prevValue = prevTaskState.state[key] as unknown as U; + } + + const value = prevValue ? prevValue : await fn(); task.updateCheckpoint?.({ key, status: 'success', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 01722edba9..b74c64d7a3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -19,7 +19,7 @@ import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSecrets, TaskState, - UpdateCheckpointOptions, + CheckpointRecord, } from '@backstage/plugin-scaffolder-node'; import { JsonObject, Observable } from '@backstage/types'; import { Logger } from 'winston'; @@ -95,7 +95,11 @@ export class TaskManager implements TaskContext { }); } - async updateCheckpoint?(options: UpdateCheckpointOptions): Promise { + async getCheckpoints?(): Promise<{ state: TaskState } | undefined> { + return this.storage.listCheckpoints?.({ taskId: this.task.taskId }); + } + + async updateCheckpoint?(options: CheckpointRecord): Promise { if (this.task.state) { this.task.state[options.key] = { ...options }; } else { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index b6225d46a9..54c0f19782 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -199,6 +199,12 @@ export interface TaskStore { emitLogEvent(options: TaskStoreEmitOptions): Promise; + listCheckpoints?({ + taskId, + }: { + taskId: string; + }): Promise<{ state: TaskState }>; + saveCheckpoint?(options: TaskStoreStateOptions): Promise; listEvents( diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 4e66f1c0e3..60023bf48e 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -26,5 +26,5 @@ export type { TaskEventType, TaskState, TaskStatus, - UpdateCheckpointOptions, + CheckpointRecord, } from './types'; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index f57353c917..c6654ea642 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -116,12 +116,12 @@ export type TaskBrokerDispatchOptions = { }; /** - * The options passed to {@link TaskBroker.updateCheckpoint} + * The record passed to {@link TaskBroker.updateCheckpoint?} * Parameters to store the result of the executed checkpoint * * @public */ -export type UpdateCheckpointOptions = +export type CheckpointRecord = | { key: string; status: 'success'; @@ -151,7 +151,9 @@ export interface TaskContext { emitLog(message: string, logMetadata?: JsonObject): Promise; - updateCheckpoint?(options: UpdateCheckpointOptions): Promise; + getCheckpoints?(): Promise<{ state: TaskState } | undefined>; + + updateCheckpoint?(options: CheckpointRecord): Promise; getWorkspaceName(): Promise; } From ca7ec6a0cb5f5ccffb0b5a902d403972956bf7cf Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Tue, 6 Feb 2024 20:30:27 +0100 Subject: [PATCH 05/23] Checkpoint Signed-off-by: bnechyporenko --- .../tasks/NunjucksWorkflowRunner.ts | 5 ++- plugins/scaffolder-node/src/tasks/types.ts | 36 +++++++++---------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 407c623793..e9fc334ea0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -357,7 +357,10 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { try { let prevValue: U | undefined; if (prevTaskState) { - prevValue = prevTaskState.state[key] as unknown as U; + const prevState = prevTaskState.state[key]; + if (prevState.status === 'success') { + prevValue = prevState.value as U; + } } const value = prevValue ? prevValue : await fn(); diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index c6654ea642..62933ad666 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -26,6 +26,24 @@ export type TaskSecrets = Record & { backstageToken?: string; }; +/** + * The record passed to {@link TaskBroker.updateCheckpoint?} + * Parameters to store the result of the executed checkpoint + * + * @public + */ +export type CheckpointRecord = + | { + key: string; + status: 'success'; + value: JsonObject; + } + | { + key: string; + status: 'failed'; + reason: string; + }; + /** * TaskState * @@ -115,24 +133,6 @@ export type TaskBrokerDispatchOptions = { createdBy?: string; }; -/** - * The record passed to {@link TaskBroker.updateCheckpoint?} - * Parameters to store the result of the executed checkpoint - * - * @public - */ -export type CheckpointRecord = - | { - key: string; - status: 'success'; - value: JsonObject; - } - | { - key: string; - status: 'failed'; - reason: string; - }; - /** * Task * From c6b132e7d933ed2c3bd25353ffb9d8b6817ec017 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Tue, 6 Feb 2024 20:41:26 +0100 Subject: [PATCH 06/23] wip Signed-off-by: bnechyporenko --- .changeset/sixty-queens-mix.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/sixty-queens-mix.md diff --git a/.changeset/sixty-queens-mix.md b/.changeset/sixty-queens-mix.md new file mode 100644 index 0000000000..52f2ca5c39 --- /dev/null +++ b/.changeset/sixty-queens-mix.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend-module-github': patch +'@backstage/plugin-scaffolder-common': patch +'@backstage/plugin-scaffolder-node': patch +--- + +Introducing checkpoints for scaffolder task action idempotency From 72d7c6867ab30512ba7609fc9f6609f52f0d4dca Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Tue, 6 Feb 2024 20:49:08 +0100 Subject: [PATCH 07/23] wip Signed-off-by: bnechyporenko --- .github/vale/config/vocabularies/Backstage/accept.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/vale/config/vocabularies/Backstage/accept.txt b/.github/vale/config/vocabularies/Backstage/accept.txt index 940e855828..46b6e9b7d6 100644 --- a/.github/vale/config/vocabularies/Backstage/accept.txt +++ b/.github/vale/config/vocabularies/Backstage/accept.txt @@ -158,6 +158,7 @@ hotspots http https Iain +idempotency Iglesias iLert img From 92582f17a011e39b647134d4dc127115fb54ea67 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Thu, 8 Feb 2024 20:43:27 +0100 Subject: [PATCH 08/23] wip Signed-off-by: bnechyporenko --- .../src/actions/github.ts | 1 - .../src/actions/helpers.ts | 172 +++++++----------- .../tasks/NunjucksWorkflowRunner.ts | 5 +- .../src/scaffolder/tasks/StorageTaskBroker.ts | 1 + .../scaffolder-common/src/defineCheckpoint.ts | 30 --- plugins/scaffolder-common/src/index.ts | 2 - plugins/scaffolder-node/src/actions/types.ts | 4 +- plugins/scaffolder-node/src/tasks/types.ts | 6 +- 8 files changed, 74 insertions(+), 147 deletions(-) delete mode 100644 plugins/scaffolder-common/src/defineCheckpoint.ts diff --git a/plugins/scaffolder-backend-module-github/src/actions/github.ts b/plugins/scaffolder-backend-module-github/src/actions/github.ts index ed55714852..91d7a34aea 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/github.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/github.ts @@ -244,7 +244,6 @@ export function createPublishGithubAction(options: { repoVariables, secrets, ctx.logger, - ctx.checkpoint, ); const remoteUrl = newRepo.clone_url; diff --git a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts index f89440e4d5..4d95a6494f 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/helpers.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/helpers.ts @@ -36,8 +36,6 @@ import { enableBranchProtectionOnDefaultRepoBranch, entityRefToName, } from './gitHelpers'; -import { JsonObject } from '@backstage/types'; -import { defineCheckpoint } from '@backstage/plugin-scaffolder-common'; const DEFAULT_TIMEOUT_MS = 60_000; @@ -140,10 +138,6 @@ export async function createGithubRepoWithCollaboratorsAndTopics( repoVariables: { [key: string]: string } | undefined, secrets: { [key: string]: string } | undefined, logger: Logger, - checkpoint?: ( - key: string, - fn: () => Promise, - ) => Promise, ) { // eslint-disable-next-line testing-library/no-await-sync-queries const user = await client.rest.users.getByUsername({ @@ -154,71 +148,59 @@ export async function createGithubRepoWithCollaboratorsAndTopics( await validateAccessTeam(client, access); } - const repoCreation = async () => { - const repoCreationPromise = - user.data.type === 'Organization' - ? client.rest.repos.createInOrg({ - name: repo, - org: owner, - private: repoVisibility === 'private', - // @ts-ignore https://github.com/octokit/types.ts/issues/522 - visibility: repoVisibility, - description: description, - delete_branch_on_merge: deleteBranchOnMerge, - allow_merge_commit: allowMergeCommit, - allow_squash_merge: allowSquashMerge, - squash_merge_commit_title: squashMergeCommitTitle, - squash_merge_commit_message: squashMergeCommitMessage, - allow_rebase_merge: allowRebaseMerge, - allow_auto_merge: allowAutoMerge, - homepage: homepage, - has_projects: hasProjects, - has_wiki: hasWiki, - has_issues: hasIssues, - }) - : client.rest.repos.createForAuthenticatedUser({ - name: repo, - private: repoVisibility === 'private', - description: description, - delete_branch_on_merge: deleteBranchOnMerge, - allow_merge_commit: allowMergeCommit, - allow_squash_merge: allowSquashMerge, - squash_merge_commit_title: squashMergeCommitTitle, - squash_merge_commit_message: squashMergeCommitMessage, - allow_rebase_merge: allowRebaseMerge, - allow_auto_merge: allowAutoMerge, - homepage: homepage, - has_projects: hasProjects, - has_wiki: hasWiki, - has_issues: hasIssues, - }); + const repoCreationPromise = + user.data.type === 'Organization' + ? client.rest.repos.createInOrg({ + name: repo, + org: owner, + private: repoVisibility === 'private', + // @ts-ignore https://github.com/octokit/types.ts/issues/522 + visibility: repoVisibility, + description: description, + delete_branch_on_merge: deleteBranchOnMerge, + allow_merge_commit: allowMergeCommit, + allow_squash_merge: allowSquashMerge, + squash_merge_commit_title: squashMergeCommitTitle, + squash_merge_commit_message: squashMergeCommitMessage, + allow_rebase_merge: allowRebaseMerge, + allow_auto_merge: allowAutoMerge, + homepage: homepage, + has_projects: hasProjects, + has_wiki: hasWiki, + has_issues: hasIssues, + }) + : client.rest.repos.createForAuthenticatedUser({ + name: repo, + private: repoVisibility === 'private', + description: description, + delete_branch_on_merge: deleteBranchOnMerge, + allow_merge_commit: allowMergeCommit, + allow_squash_merge: allowSquashMerge, + squash_merge_commit_title: squashMergeCommitTitle, + squash_merge_commit_message: squashMergeCommitMessage, + allow_rebase_merge: allowRebaseMerge, + allow_auto_merge: allowAutoMerge, + homepage: homepage, + has_projects: hasProjects, + has_wiki: hasWiki, + has_issues: hasIssues, + }); - let newRepo; + let newRepo; - try { - newRepo = (await repoCreationPromise).data; - } catch (e) { - assertError(e); - if (e.message === 'Resource not accessible by integration') { - logger.warn( - `The GitHub app or token provided may not have the required permissions to create the ${user.data.type} repository ${owner}/${repo}.`, - ); - } - throw new Error( - `Failed to create the ${user.data.type} repository ${owner}/${repo}, ${e.message}`, + try { + newRepo = (await repoCreationPromise).data; + } catch (e) { + assertError(e); + if (e.message === 'Resource not accessible by integration') { + logger.warn( + `The GitHub app or token provided may not have the required permissions to create the ${user.data.type} repository ${owner}/${repo}.`, ); } - return { clone_url: newRepo.clone_url, html_url: newRepo.html_url }; - }; - - const { clone_url, html_url } = await defineCheckpoint<{ - clone_url: string; - html_url: string; - }>({ - key: 'v1.task.checkpoint.repo.creation', - checkpoint, - fn: repoCreation, - }); + throw new Error( + `Failed to create the ${user.data.type} repository ${owner}/${repo}, ${e.message}`, + ); + } if (access?.startsWith(`${owner}/`)) { const [, team] = access.split('/'); @@ -231,19 +213,11 @@ export async function createGithubRepoWithCollaboratorsAndTopics( }); // No need to add access if it's the person who owns the personal account } else if (access && access !== owner) { - const addCollaborator = async () => { - await client.rest.repos.addCollaborator({ - owner, - repo, - username: access, - permission: 'admin', - }); - return {}; - }; - await defineCheckpoint({ - key: 'v1.task.checkpoint.add.collaborator', - checkpoint, - fn: addCollaborator, + await client.rest.repos.addCollaborator({ + owner, + repo, + username: access, + permission: 'admin', }); } @@ -251,19 +225,11 @@ export async function createGithubRepoWithCollaboratorsAndTopics( for (const collaborator of collaborators) { try { if ('user' in collaborator) { - const addCollaborator = async () => { - await client.rest.repos.addCollaborator({ - owner, - repo, - username: entityRefToName(collaborator.user), - permission: collaborator.access, - }); - return {}; - }; - await defineCheckpoint({ - key: `v1.task.checkpoint.add.collaborator.${collaborator.user}`, - checkpoint, - fn: addCollaborator, + await client.rest.repos.addCollaborator({ + owner, + repo, + username: entityRefToName(collaborator.user), + permission: collaborator.access, }); } else if ('team' in collaborator) { await client.rest.teams.addOrUpdateRepoPermissionsInOrg({ @@ -298,19 +264,11 @@ export async function createGithubRepoWithCollaboratorsAndTopics( } for (const [key, value] of Object.entries(repoVariables ?? {})) { - const createRepoVariable = async () => { - await client.rest.actions.createRepoVariable({ - owner, - repo, - name: key, - value: value, - }); - return {}; - }; - await defineCheckpoint({ - key: `v1.task.checkpoint.create.repo.variable.${key}`, - checkpoint, - fn: createRepoVariable, + await client.rest.actions.createRepoVariable({ + owner, + repo, + name: key, + value: value, }); } @@ -346,7 +304,7 @@ export async function createGithubRepoWithCollaboratorsAndTopics( } } - return { clone_url, html_url }; + return newRepo; } export async function initRepoPushAndProtect( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index e9fc334ea0..d9c1979eee 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -350,10 +350,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { logger: taskLogger, logStream: streamLogger, workspacePath, - async checkpoint( - key: string, + async checkpoint( + keySuffix: string, fn: () => Promise, ) { + const key = `v1.task.checkpoint.${keySuffix}`; try { let prevValue: U | undefined; if (prevTaskState) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index b74c64d7a3..d7fe1be6af 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -100,6 +100,7 @@ export class TaskManager implements TaskContext { } async updateCheckpoint?(options: CheckpointRecord): Promise { + // drop the key if (this.task.state) { this.task.state[options.key] = { ...options }; } else { diff --git a/plugins/scaffolder-common/src/defineCheckpoint.ts b/plugins/scaffolder-common/src/defineCheckpoint.ts deleted file mode 100644 index c349bdc7f3..0000000000 --- a/plugins/scaffolder-common/src/defineCheckpoint.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright 2021 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 { JsonObject } from '@backstage/types'; - -export const defineCheckpoint = async (props: { - checkpoint?: (key: string, fn: () => Promise) => Promise; - key: string; - fn: () => Promise; -}): Promise => { - const { checkpoint, fn, key } = props; - return checkpoint - ? checkpoint?.(key, async () => { - return await fn(); - }) - : fn(); -}; diff --git a/plugins/scaffolder-common/src/index.ts b/plugins/scaffolder-common/src/index.ts index c0434c3cdb..4d9b5e39c6 100644 --- a/plugins/scaffolder-common/src/index.ts +++ b/plugins/scaffolder-common/src/index.ts @@ -34,5 +34,3 @@ export type { TemplatePermissionsV1beta3, TemplateRecoveryV1beta3, } from './TemplateEntityV1beta3'; - -export { defineCheckpoint } from './defineCheckpoint'; diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index a7f44c4730..5abeb29a2b 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -16,7 +16,7 @@ import { Logger } from 'winston'; import { Writable } from 'stream'; -import { JsonObject } from '@backstage/types'; +import { JsonObject, JsonValue } from '@backstage/types'; import { TaskSecrets } from '../tasks'; import { TemplateInfo } from '@backstage/plugin-scaffolder-common'; import { UserEntity } from '@backstage/catalog-model'; @@ -35,7 +35,7 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint?( + checkpoint( key: string, fn: () => Promise, ): Promise; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 62933ad666..b2bff61e03 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -15,7 +15,7 @@ */ import { TaskSpec } from '@backstage/plugin-scaffolder-common'; -import { JsonObject, Observable } from '@backstage/types'; +import { JsonObject, JsonValue, Observable } from '@backstage/types'; /** * TaskSecrets @@ -36,7 +36,7 @@ export type CheckpointRecord = | { key: string; status: 'success'; - value: JsonObject; + value: JsonValue; } | { key: string; @@ -54,7 +54,7 @@ export type TaskState = { | { status: 'failed'; reason: string } | { status: 'success'; - value: JsonObject; + value: JsonValue; }; }; From a2ee37bc6d88f1e7bda14d4ea79f68e8e59c6fa4 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Thu, 8 Feb 2024 21:20:16 +0100 Subject: [PATCH 09/23] wip Signed-off-by: bnechyporenko --- plugins/scaffolder-backend/api-report.md | 30 ++++++++++++++ .../src/scaffolder/tasks/StorageTaskBroker.ts | 6 +-- .../src/scaffolder/tasks/index.ts | 1 + .../src/scaffolder/tasks/types.ts | 5 +++ plugins/scaffolder-node/api-report.md | 41 +++++++++++++++++++ plugins/scaffolder-node/src/actions/types.ts | 2 +- plugins/scaffolder-node/src/tasks/types.ts | 4 +- 7 files changed, 83 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 467887dfc5..742834a4df 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -9,6 +9,7 @@ import * as bitbucket from '@backstage/plugin-scaffolder-backend-module-bitbucke import * as bitbucketCloud from '@backstage/plugin-scaffolder-backend-module-bitbucket-cloud'; import * as bitbucketServer from '@backstage/plugin-scaffolder-backend-module-bitbucket-server'; import { CatalogApi } from '@backstage/catalog-client'; +import { CheckpointRecord } from '@backstage/plugin-scaffolder-node'; import { Config } from '@backstage/config'; import { Duration } from 'luxon'; import { executeShellCommand as executeShellCommand_2 } from '@backstage/plugin-scaffolder-node'; @@ -47,6 +48,7 @@ import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { TaskSecrets as TaskSecrets_2 } from '@backstage/plugin-scaffolder-node'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; +import { TaskState } from '@backstage/plugin-scaffolder-node'; import { TaskStatus as TaskStatus_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; @@ -359,6 +361,7 @@ export interface CurrentClaimedTask { createdBy?: string; secrets?: TaskSecrets_2; spec: TaskSpec; + state?: TaskState; taskId: string; } @@ -403,6 +406,10 @@ export class DatabaseTaskStore implements TaskStore { tasks: SerializedTask_2[]; }>; // (undocumented) + listCheckpoints({ taskId }: { taskId: string }): Promise<{ + state: TaskState; + }>; + // (undocumented) listEvents(options: TaskStoreListEventsOptions): Promise<{ events: SerializedTaskEvent_2[]; }>; @@ -418,6 +425,8 @@ export class DatabaseTaskStore implements TaskStore { ids: string[]; }>; // (undocumented) + saveCheckpoint(options: TaskStoreStateOptions): Promise; + // (undocumented) shutdownTask(options: TaskStoreShutDownTaskOptions): Promise; } @@ -519,11 +528,20 @@ export class TaskManager implements TaskContext { // (undocumented) emitLog(message: string, logMetadata?: JsonObject): Promise; // (undocumented) + getCheckpoints?(): Promise< + | { + state: TaskState; + } + | undefined + >; + // (undocumented) getWorkspaceName(): Promise; // (undocumented) get secrets(): TaskSecrets_2 | undefined; // (undocumented) get spec(): TaskSpecV1beta3; + // (undocumented) + updateCheckpoint?(options: CheckpointRecord): Promise; } // @public @deprecated (undocumented) @@ -559,6 +577,10 @@ export interface TaskStore { tasks: SerializedTask[]; }>; // (undocumented) + listCheckpoints?({ taskId }: { taskId: string }): Promise<{ + state: TaskState; + }>; + // (undocumented) listEvents(options: TaskStoreListEventsOptions): Promise<{ events: SerializedTaskEvent[]; }>; @@ -573,6 +595,8 @@ export interface TaskStore { ids: string[]; }>; // (undocumented) + saveCheckpoint?(options: TaskStoreStateOptions): Promise; + // (undocumented) shutdownTask?(options: TaskStoreShutDownTaskOptions): Promise; } @@ -610,6 +634,12 @@ export type TaskStoreShutDownTaskOptions = { taskId: string; }; +// @public +export type TaskStoreStateOptions = { + taskId: string; + state?: TaskState; +}; + // @public export class TaskWorker { // (undocumented) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index d7fe1be6af..6e01141dab 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -100,11 +100,11 @@ export class TaskManager implements TaskContext { } async updateCheckpoint?(options: CheckpointRecord): Promise { - // drop the key + const { key, ...value } = options; if (this.task.state) { - this.task.state[options.key] = { ...options }; + this.task.state[key] = value; } else { - this.task.state = { [options.key]: options }; + this.task.state = { [key]: value }; } await this.storage.saveCheckpoint?.({ taskId: this.task.taskId, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts index 9d231d7e7c..2da812e869 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -25,6 +25,7 @@ export type { TaskStoreEmitOptions, TaskStoreListEventsOptions, TaskStoreShutDownTaskOptions, + TaskStoreStateOptions, SerializedTask, SerializedTaskEvent, TaskStatus, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 54c0f19782..e30de5db94 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -113,6 +113,11 @@ export type TaskStoreEmitOptions = { body: TBody; }; +/** + * TaskStoreStateOptions + * + * @public + */ export type TaskStoreStateOptions = { taskId: string; state?: TaskState; diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index fec2205462..d3bc9d8500 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -30,6 +30,10 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; + checkpoint?( + key: string, + fn: () => Promise, + ): Promise; output( name: keyof TActionOutput, value: TActionOutput[keyof TActionOutput], @@ -60,6 +64,19 @@ export function addFiles(options: { logger?: Logger | undefined; }): Promise; +// @public +export type CheckpointRecord = + | { + key: string; + status: 'success'; + value: JsonValue; + } + | { + key: string; + status: 'failed'; + reason: string; + }; + // @public (undocumented) export function cloneRepo(options: { url: string; @@ -345,6 +362,13 @@ export interface TaskContext { // (undocumented) emitLog(message: string, logMetadata?: JsonObject): Promise; // (undocumented) + getCheckpoints?(): Promise< + | { + state: TaskState; + } + | undefined + >; + // (undocumented) getWorkspaceName(): Promise; // (undocumented) isDryRun?: boolean; @@ -352,6 +376,10 @@ export interface TaskContext { secrets?: TaskSecrets; // (undocumented) spec: TaskSpec; + // (undocumented) + state?: TaskState; + // (undocumented) + updateCheckpoint?(options: CheckpointRecord): Promise; } // @public @@ -362,6 +390,19 @@ export type TaskSecrets = Record & { backstageToken?: string; }; +// @public +export type TaskState = { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; +}; + // @public export type TaskStatus = | 'cancelled' diff --git a/plugins/scaffolder-node/src/actions/types.ts b/plugins/scaffolder-node/src/actions/types.ts index 5abeb29a2b..678d6f87d7 100644 --- a/plugins/scaffolder-node/src/actions/types.ts +++ b/plugins/scaffolder-node/src/actions/types.ts @@ -35,7 +35,7 @@ export type ActionContext< secrets?: TaskSecrets; workspacePath: string; input: TActionInput; - checkpoint( + checkpoint?( key: string, fn: () => Promise, ): Promise; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index b2bff61e03..a7fbf4ac76 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -27,7 +27,7 @@ export type TaskSecrets = Record & { }; /** - * The record passed to {@link TaskBroker.updateCheckpoint?} + * The record passed to TaskBroker for updating a checkpoint. * Parameters to store the result of the executed checkpoint * * @public @@ -45,7 +45,7 @@ export type CheckpointRecord = }; /** - * TaskState + * The state of all task's checkpoints * * @public */ From 2b900fee6dcd956bff1503bf89a602d45a7a195e Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sat, 10 Feb 2024 14:34:14 +0100 Subject: [PATCH 10/23] wip Signed-off-by: bnechyporenko --- plugins/scaffolder-backend/api-report.md | 123 ++++++++++++++---- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 51 ++++++-- .../tasks/NunjucksWorkflowRunner.ts | 2 +- .../src/scaffolder/tasks/StorageTaskBroker.ts | 47 +++++-- .../src/scaffolder/tasks/index.ts | 1 - .../src/scaffolder/tasks/types.ts | 42 +++--- plugins/scaffolder-node/api-report.md | 53 +++++--- plugins/scaffolder-node/src/tasks/index.ts | 1 - plugins/scaffolder-node/src/tasks/types.ts | 55 +++++--- 9 files changed, 274 insertions(+), 101 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 742834a4df..b226372d86 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -9,7 +9,6 @@ import * as bitbucket from '@backstage/plugin-scaffolder-backend-module-bitbucke import * as bitbucketCloud from '@backstage/plugin-scaffolder-backend-module-bitbucket-cloud'; import * as bitbucketServer from '@backstage/plugin-scaffolder-backend-module-bitbucket-server'; import { CatalogApi } from '@backstage/catalog-client'; -import { CheckpointRecord } from '@backstage/plugin-scaffolder-node'; import { Config } from '@backstage/config'; import { Duration } from 'luxon'; import { executeShellCommand as executeShellCommand_2 } from '@backstage/plugin-scaffolder-node'; @@ -22,6 +21,7 @@ import * as gitlab from '@backstage/plugin-scaffolder-backend-module-gitlab'; import { HumanDuration } from '@backstage/types'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { JsonObject } from '@backstage/types'; +import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { Logger } from 'winston'; @@ -48,7 +48,6 @@ import { TaskRecovery } from '@backstage/plugin-scaffolder-common'; import { TaskSecrets as TaskSecrets_2 } from '@backstage/plugin-scaffolder-node'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { TaskSpecV1beta3 } from '@backstage/plugin-scaffolder-common'; -import { TaskState } from '@backstage/plugin-scaffolder-node'; import { TaskStatus as TaskStatus_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateAction as TemplateAction_2 } from '@backstage/plugin-scaffolder-node'; import { TemplateActionOptions } from '@backstage/plugin-scaffolder-node'; @@ -361,7 +360,17 @@ export interface CurrentClaimedTask { createdBy?: string; secrets?: TaskSecrets_2; spec: TaskSpec; - state?: TaskState; + state?: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; taskId: string; } @@ -400,16 +409,29 @@ export class DatabaseTaskStore implements TaskStore { // (undocumented) getTask(taskId: string): Promise; // (undocumented) + getTaskState({ taskId }: { taskId: string }): Promise< + | { + state: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + >; + // (undocumented) heartbeatTask(taskId: string): Promise; // (undocumented) list(options: { createdBy?: string }): Promise<{ tasks: SerializedTask_2[]; }>; // (undocumented) - listCheckpoints({ taskId }: { taskId: string }): Promise<{ - state: TaskState; - }>; - // (undocumented) listEvents(options: TaskStoreListEventsOptions): Promise<{ events: SerializedTaskEvent_2[]; }>; @@ -425,7 +447,22 @@ export class DatabaseTaskStore implements TaskStore { ids: string[]; }>; // (undocumented) - saveCheckpoint(options: TaskStoreStateOptions): Promise; + saveCheckpoint(options: { + taskId: string; + state?: + | { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + } + | undefined; + }): Promise; // (undocumented) shutdownTask(options: TaskStoreShutDownTaskOptions): Promise; } @@ -528,9 +565,19 @@ export class TaskManager implements TaskContext { // (undocumented) emitLog(message: string, logMetadata?: JsonObject): Promise; // (undocumented) - getCheckpoints?(): Promise< + getTaskState?(): Promise< | { - state: TaskState; + state: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; } | undefined >; @@ -541,7 +588,19 @@ export class TaskManager implements TaskContext { // (undocumented) get spec(): TaskSpecV1beta3; // (undocumented) - updateCheckpoint?(options: CheckpointRecord): Promise; + updateCheckpoint?( + options: + | { + key: string; + status: 'success'; + value: JsonValue; + } + | { + key: string; + status: 'failed'; + reason: string; + }, + ): Promise; } // @public @deprecated (undocumented) @@ -571,16 +630,29 @@ export interface TaskStore { // (undocumented) getTask(taskId: string): Promise; // (undocumented) + getTaskState?({ taskId }: { taskId: string }): Promise< + | { + state: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + >; + // (undocumented) heartbeatTask(taskId: string): Promise; // (undocumented) list?(options: { createdBy?: string }): Promise<{ tasks: SerializedTask[]; }>; // (undocumented) - listCheckpoints?({ taskId }: { taskId: string }): Promise<{ - state: TaskState; - }>; - // (undocumented) listEvents(options: TaskStoreListEventsOptions): Promise<{ events: SerializedTaskEvent[]; }>; @@ -595,7 +667,20 @@ export interface TaskStore { ids: string[]; }>; // (undocumented) - saveCheckpoint?(options: TaskStoreStateOptions): Promise; + saveCheckpoint?(options: { + taskId: string; + state?: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; + }): Promise; // (undocumented) shutdownTask?(options: TaskStoreShutDownTaskOptions): Promise; } @@ -634,12 +719,6 @@ export type TaskStoreShutDownTaskOptions = { taskId: string; }; -// @public -export type TaskStoreStateOptions = { - taskId: string; - state?: TaskState; -}; - // @public export class TaskWorker { // (undocumented) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 14535d034c..0d6521dfd2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { JsonObject } from '@backstage/types'; +import { JsonObject, JsonValue } from '@backstage/types'; import { PluginDatabaseManager, resolvePackagePath, @@ -30,7 +30,6 @@ import { TaskStoreCreateTaskResult, TaskStoreShutDownTaskOptions, TaskStoreRecoverTaskOptions, - TaskStoreStateOptions, } from './types'; import { SerializedTaskEvent, @@ -38,7 +37,6 @@ import { TaskStatus, TaskEventType, TaskSecrets, - TaskState, } from '@backstage/plugin-scaffolder-node'; import { DateTime, Duration } from 'luxon'; import { TaskRecovery, TaskSpec } from '@backstage/plugin-scaffolder-common'; @@ -397,18 +395,49 @@ export class DatabaseTaskStore implements TaskStore { }); } - async listCheckpoints({ - taskId, - }: { - taskId: string; - }): Promise<{ state: TaskState }> { - const state = await this.db('tasks') + async getTaskState({ taskId }: { taskId: string }): Promise< + | { + state: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + > { + const [result] = await this.db('tasks') .where({ id: taskId }) .select('state'); - return { state: JSON.stringify(state) as unknown as TaskState }; + return result.state + ? { + state: JSON.parse(result.state) as unknown as { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }, + } + : undefined; } - async saveCheckpoint(options: TaskStoreStateOptions): Promise { + async saveCheckpoint(options: { + taskId: string; + state?: + | { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + } + | undefined; + }): Promise { if (options.state) { const serializedState = JSON.stringify(options.state); await this.db('tasks') diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index d9c1979eee..b5ffc73189 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -332,7 +332,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { } const tmpDirs = new Array(); const stepOutput: { [outputName: string]: JsonValue } = {}; - const prevTaskState = await task.getCheckpoints?.(); + const prevTaskState = await task.getTaskState?.(); for (const iteration of iterations) { if (iteration.each) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 6e01141dab..453df09c77 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -16,12 +16,8 @@ import { Config } from '@backstage/config'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; -import { - TaskSecrets, - TaskState, - CheckpointRecord, -} from '@backstage/plugin-scaffolder-node'; -import { JsonObject, Observable } from '@backstage/types'; +import { TaskSecrets } from '@backstage/plugin-scaffolder-node'; +import { JsonObject, JsonValue, Observable } from '@backstage/types'; import { Logger } from 'winston'; import ObservableImpl from 'zen-observable'; import { @@ -95,11 +91,35 @@ export class TaskManager implements TaskContext { }); } - async getCheckpoints?(): Promise<{ state: TaskState } | undefined> { - return this.storage.listCheckpoints?.({ taskId: this.task.taskId }); + async getTaskState?(): Promise< + | { + state: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + > { + return this.storage.getTaskState?.({ taskId: this.task.taskId }); } - async updateCheckpoint?(options: CheckpointRecord): Promise { + async updateCheckpoint?( + options: + | { + key: string; + status: 'success'; + value: JsonValue; + } + | { + key: string; + status: 'failed'; + reason: string; + }, + ): Promise { const { key, ...value } = options; if (this.task.state) { this.task.state[key] = value; @@ -168,7 +188,14 @@ export interface CurrentClaimedTask { /** * The state of checkpoints of the task. */ - state?: TaskState; + state?: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; /** * The creator of the task. */ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts index 2da812e869..9d231d7e7c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/index.ts @@ -25,7 +25,6 @@ export type { TaskStoreEmitOptions, TaskStoreListEventsOptions, TaskStoreShutDownTaskOptions, - TaskStoreStateOptions, SerializedTask, SerializedTaskEvent, TaskStatus, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index e30de5db94..215defd280 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -16,7 +16,7 @@ import { JsonValue, JsonObject, HumanDuration } from '@backstage/types'; import { TaskSpec, TaskStep } from '@backstage/plugin-scaffolder-common'; -import { TaskSecrets, TaskState } from '@backstage/plugin-scaffolder-node'; +import { TaskSecrets } from '@backstage/plugin-scaffolder-node'; import { TemplateAction, TaskStatus as _TaskStatus, @@ -113,16 +113,6 @@ export type TaskStoreEmitOptions = { body: TBody; }; -/** - * TaskStoreStateOptions - * - * @public - */ -export type TaskStoreStateOptions = { - taskId: string; - state?: TaskState; -}; - /** * TaskStoreListEventsOptions * @@ -204,13 +194,31 @@ export interface TaskStore { emitLogEvent(options: TaskStoreEmitOptions): Promise; - listCheckpoints?({ - taskId, - }: { - taskId: string; - }): Promise<{ state: TaskState }>; + getTaskState?({ taskId }: { taskId: string }): Promise< + | { + state: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + >; - saveCheckpoint?(options: TaskStoreStateOptions): Promise; + saveCheckpoint?(options: { + taskId: string; + state?: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; + }): Promise; listEvents( options: TaskStoreListEventsOptions, diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index d3bc9d8500..c87ec8183c 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -64,19 +64,6 @@ export function addFiles(options: { logger?: Logger | undefined; }): Promise; -// @public -export type CheckpointRecord = - | { - key: string; - status: 'success'; - value: JsonValue; - } - | { - key: string; - status: 'failed'; - reason: string; - }; - // @public (undocumented) export function cloneRepo(options: { url: string; @@ -362,9 +349,19 @@ export interface TaskContext { // (undocumented) emitLog(message: string, logMetadata?: JsonObject): Promise; // (undocumented) - getCheckpoints?(): Promise< + getTaskState?(): Promise< | { - state: TaskState; + state: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; } | undefined >; @@ -377,9 +374,31 @@ export interface TaskContext { // (undocumented) spec: TaskSpec; // (undocumented) - state?: TaskState; + state?: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; // (undocumented) - updateCheckpoint?(options: CheckpointRecord): Promise; + updateCheckpoint?( + options: + | { + key: string; + status: 'success'; + value: JsonValue; + } + | { + key: string; + status: 'failed'; + reason: string; + }, + ): Promise; } // @public diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 60023bf48e..99638e48af 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -26,5 +26,4 @@ export type { TaskEventType, TaskState, TaskStatus, - CheckpointRecord, } from './types'; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index a7fbf4ac76..46b82ebe53 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -26,24 +26,6 @@ export type TaskSecrets = Record & { backstageToken?: string; }; -/** - * The record passed to TaskBroker for updating a checkpoint. - * Parameters to store the result of the executed checkpoint - * - * @public - */ -export type CheckpointRecord = - | { - key: string; - status: 'success'; - value: JsonValue; - } - | { - key: string; - status: 'failed'; - reason: string; - }; - /** * The state of all task's checkpoints * @@ -142,7 +124,14 @@ export interface TaskContext { cancelSignal: AbortSignal; spec: TaskSpec; secrets?: TaskSecrets; - state?: TaskState; + state?: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; createdBy?: string; done: boolean; isDryRun?: boolean; @@ -151,9 +140,33 @@ export interface TaskContext { emitLog(message: string, logMetadata?: JsonObject): Promise; - getCheckpoints?(): Promise<{ state: TaskState } | undefined>; + getTaskState?(): Promise< + | { + state: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; + } + | undefined + >; - updateCheckpoint?(options: CheckpointRecord): Promise; + updateCheckpoint?( + options: + | { + key: string; + status: 'success'; + value: JsonValue; + } + | { + key: string; + status: 'failed'; + reason: string; + }, + ): Promise; getWorkspaceName(): Promise; } From 4a460034da4ac647818957506bfe1c2147afbac5 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sat, 10 Feb 2024 14:41:10 +0100 Subject: [PATCH 11/23] wip Signed-off-by: bnechyporenko --- plugins/scaffolder-node/api-report.md | 13 ------------- plugins/scaffolder-node/src/tasks/index.ts | 1 - plugins/scaffolder-node/src/tasks/types.ts | 14 -------------- 3 files changed, 28 deletions(-) diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index c87ec8183c..7e29e16aa0 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -409,19 +409,6 @@ export type TaskSecrets = Record & { backstageToken?: string; }; -// @public -export type TaskState = { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; -}; - // @public export type TaskStatus = | 'cancelled' diff --git a/plugins/scaffolder-node/src/tasks/index.ts b/plugins/scaffolder-node/src/tasks/index.ts index 99638e48af..930de95237 100644 --- a/plugins/scaffolder-node/src/tasks/index.ts +++ b/plugins/scaffolder-node/src/tasks/index.ts @@ -24,6 +24,5 @@ export type { TaskCompletionState, TaskContext, TaskEventType, - TaskState, TaskStatus, } from './types'; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 46b82ebe53..5a4838737c 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -26,20 +26,6 @@ export type TaskSecrets = Record & { backstageToken?: string; }; -/** - * The state of all task's checkpoints - * - * @public - */ -export type TaskState = { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; -}; - /** * The status of each step of the Task * From fb07d87c0dd0ef0446e382024aa939b952f963f4 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 11 Feb 2024 22:29:47 +0100 Subject: [PATCH 12/23] + unit test Signed-off-by: bnechyporenko --- .../tasks/NunjucksWorkflowRunner.test.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 3085bb8687..e6abfcb8de 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -133,6 +133,36 @@ describe('NunjucksWorkflowRunner', () => { }, }); + actionRegistry.register({ + id: 'checkpoints-action', + description: 'Mock action with checkpoints', + handler: async ctx => { + let key1 = 0; + let key2 = ''; + let i = 0; + + const incrementKey1 = async () => { + key1 += 1; + return key1; + }; + + const appendKey2 = async () => { + key2 += 'k'; + return key2; + }; + + while (i < 3) { + i += 1; + ctx.checkpoint?.('key1', incrementKey1); + } + + ctx.checkpoint?.('key2', appendKey2); + + ctx.output('key1', key1); + ctx.output('key2', key2); + }, + }); + mockedPermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.ALLOW }, ]); @@ -538,6 +568,29 @@ describe('NunjucksWorkflowRunner', () => { ); }); + it('should deal with checkpoints', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'checkpoints-action', + input: { foo: 1 }, + }, + ], + output: { + key1: '${{steps.test.output.key1}}', + key2: '${{steps.test.output.key2}}', + }, + }); + const result = await runner.execute(task); + + expect(result.output.key1).toEqual(3); + expect(result.output.key2).toEqual('k'); + }); + it('should template the output from simple actions', async () => { const task = createMockTaskWithSpec({ apiVersion: 'scaffolder.backstage.io/v1beta3', From f94cd8bf3e65e1c06db6656bee55b810f3762380 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 11 Feb 2024 23:14:05 +0100 Subject: [PATCH 13/23] + unit test Signed-off-by: bnechyporenko --- plugins/scaffolder-backend/api-report.md | 60 ++++++------- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 32 +++---- .../tasks/NunjucksWorkflowRunner.test.ts | 89 +++++++++++-------- .../tasks/NunjucksWorkflowRunner.ts | 17 ++-- .../src/scaffolder/tasks/StorageTaskBroker.ts | 14 ++- .../src/scaffolder/tasks/types.ts | 14 ++- plugins/scaffolder-node/api-report.md | 20 ++--- plugins/scaffolder-node/src/tasks/types.ts | 14 ++- 8 files changed, 131 insertions(+), 129 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index b226372d86..0bcd8ce33c 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -411,17 +411,15 @@ export class DatabaseTaskStore implements TaskStore { // (undocumented) getTaskState({ taskId }: { taskId: string }): Promise< | { - state: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; @@ -567,17 +565,15 @@ export class TaskManager implements TaskContext { // (undocumented) getTaskState?(): Promise< | { - state: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; @@ -632,17 +628,15 @@ export interface TaskStore { // (undocumented) getTaskState?({ taskId }: { taskId: string }): Promise< | { - state: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 0d6521dfd2..db890c330e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -397,14 +397,12 @@ export class DatabaseTaskStore implements TaskStore { async getTaskState({ taskId }: { taskId: string }): Promise< | { - state: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; } | undefined > { @@ -412,16 +410,14 @@ export class DatabaseTaskStore implements TaskStore { .where({ id: taskId }) .select('state'); return result.state - ? { - state: JSON.parse(result.state) as unknown as { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }, - } + ? (JSON.parse(result.state) as unknown as { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }) : undefined; } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index e6abfcb8de..2b8db0d0ba 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -18,6 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; +import { JsonValue } from '@backstage/types'; import { ConfigReader } from '@backstage/config'; import { TaskContext } from './types'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; @@ -137,29 +138,19 @@ describe('NunjucksWorkflowRunner', () => { id: 'checkpoints-action', description: 'Mock action with checkpoints', handler: async ctx => { - let key1 = 0; - let key2 = ''; - let i = 0; - - const incrementKey1 = async () => { - key1 += 1; - return key1; - }; - - const appendKey2 = async () => { - key2 += 'k'; - return key2; - }; - - while (i < 3) { - i += 1; - ctx.checkpoint?.('key1', incrementKey1); - } - - ctx.checkpoint?.('key2', appendKey2); + const key1 = await ctx.checkpoint?.('key1', async () => { + return 'updated'; + }); + const key2 = await ctx.checkpoint?.('key2', async () => { + return 'updated'; + }); + const key3 = await ctx.checkpoint?.('key3', async () => { + return 'updated'; + }); ctx.output('key1', key1); ctx.output('key2', key2); + ctx.output('key3', key3); }, }); @@ -569,26 +560,52 @@ describe('NunjucksWorkflowRunner', () => { }); it('should deal with checkpoints', async () => { - const task = createMockTaskWithSpec({ - apiVersion: 'scaffolder.backstage.io/v1beta3', - parameters: {}, - steps: [ - { - id: 'test', - name: 'name', - action: 'checkpoints-action', - input: { foo: 1 }, + const task = { + ...createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'checkpoints-action', + input: { foo: 1 }, + }, + ], + output: { + key1: '${{steps.test.output.key1}}', + key2: '${{steps.test.output.key2}}', + key3: '${{steps.test.output.key3}}', }, - ], - output: { - key1: '${{steps.test.output.key1}}', - key2: '${{steps.test.output.key2}}', + }), + getTaskState: (): Promise< + | { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + } + | undefined + > => { + return Promise.resolve({ + ['v1.task.checkpoint.key1']: { + status: 'success', + value: 'initial', + }, + ['v1.task.checkpoint.key2']: { + status: 'failed', + reason: 'fatal error', + }, + }); }, - }); + }; const result = await runner.execute(task); - expect(result.output.key1).toEqual(3); - expect(result.output.key2).toEqual('k'); + expect(result.output.key1).toEqual('initial'); + expect(result.output.key2).toEqual('updated'); + expect(result.output.key3).toEqual('updated'); }); it('should template the output from simple actions', async () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index b5ffc73189..1dd14bc905 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -358,18 +358,21 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { try { let prevValue: U | undefined; if (prevTaskState) { - const prevState = prevTaskState.state[key]; - if (prevState.status === 'success') { + const prevState = prevTaskState[key]; + if (prevState && prevState.status === 'success') { prevValue = prevState.value as U; } } const value = prevValue ? prevValue : await fn(); - task.updateCheckpoint?.({ - key, - status: 'success', - value, - }); + + if (!prevValue) { + task.updateCheckpoint?.({ + key, + status: 'success', + value, + }); + } return value; } catch (err) { task.updateCheckpoint?.({ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 453df09c77..0404f143de 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -93,14 +93,12 @@ export class TaskManager implements TaskContext { async getTaskState?(): Promise< | { - state: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; } | undefined > { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 215defd280..45c4774299 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -196,14 +196,12 @@ export interface TaskStore { getTaskState?({ taskId }: { taskId: string }): Promise< | { - state: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index 7e29e16aa0..7c024a0043 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -351,17 +351,15 @@ export interface TaskContext { // (undocumented) getTaskState?(): Promise< | { - state: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 5a4838737c..18383dbac3 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -128,14 +128,12 @@ export interface TaskContext { getTaskState?(): Promise< | { - state: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; } | undefined >; From 9c7e28a36e232456955e59123cd5d399d33e0457 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Tue, 13 Feb 2024 10:31:33 +0100 Subject: [PATCH 14/23] + unit test Signed-off-by: bnechyporenko --- package.json | 1 - plugins/scaffolder-backend/api-report.md | 4 +-- .../tasks/DatabaseTaskStore.test.ts | 27 +++++++++++++++++++ .../src/scaffolder/tasks/DatabaseTaskStore.ts | 2 +- .../src/scaffolder/tasks/StorageTaskBroker.ts | 2 +- .../src/scaffolder/tasks/types.ts | 2 +- yarn.lock | 1 - 7 files changed, 32 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index d3188d483a..646b9e43c8 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "@backstage/repo-tools": "workspace:*", "@changesets/cli": "^2.14.0", "@octokit/rest": "^19.0.3", - "@playwright/test": "^1.32.3", "@spotify/eslint-plugin": "^14.1.3", "@spotify/prettier-config": "^14.0.0", "@techdocs/cli": "workspace:*", diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 0bcd8ce33c..151c8568d5 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -445,7 +445,7 @@ export class DatabaseTaskStore implements TaskStore { ids: string[]; }>; // (undocumented) - saveCheckpoint(options: { + saveTaskState(options: { taskId: string; state?: | { @@ -661,7 +661,7 @@ export interface TaskStore { ids: string[]; }>; // (undocumented) - saveCheckpoint?(options: { + saveTaskState?(options: { taskId: string; state?: { [key: string]: diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts index 4c0eb94c82..deb72fb7d2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts @@ -188,4 +188,31 @@ describe('DatabaseTaskStore', () => { await store.shutdownTask({ taskId }); }).rejects.toThrow(ConflictError); }); + + it('should store checkpoints and retrieve task state', async () => { + const { store } = await createStore(); + const { taskId } = await store.createTask({ + spec: {} as TaskSpec, + createdBy: 'me', + }); + + await store.saveTaskState({ + taskId, + state: { + 'repo.create': { + status: 'success', + value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + }, + }, + }); + + const state = await store.getTaskState({ taskId }); + + expect(state).toStrictEqual({ + 'repo.create': { + status: 'success', + value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + }, + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index db890c330e..263fe3d435 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -421,7 +421,7 @@ export class DatabaseTaskStore implements TaskStore { : undefined; } - async saveCheckpoint(options: { + async saveTaskState(options: { taskId: string; state?: | { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 0404f143de..cc958b35b3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -124,7 +124,7 @@ export class TaskManager implements TaskContext { } else { this.task.state = { [key]: value }; } - await this.storage.saveCheckpoint?.({ + await this.storage.saveTaskState?.({ taskId: this.task.taskId, state: this.task.state, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 45c4774299..d0bb48f4f2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -206,7 +206,7 @@ export interface TaskStore { | undefined >; - saveCheckpoint?(options: { + saveTaskState?(options: { taskId: string; state?: { [key: string]: diff --git a/yarn.lock b/yarn.lock index 6379617840..0d58ba4587 100644 --- a/yarn.lock +++ b/yarn.lock @@ -41307,7 +41307,6 @@ __metadata: "@changesets/cli": ^2.14.0 "@manypkg/get-packages": ^1.1.3 "@octokit/rest": ^19.0.3 - "@playwright/test": ^1.32.3 "@spotify/eslint-plugin": ^14.1.3 "@spotify/prettier-config": ^14.0.0 "@techdocs/cli": "workspace:*" From 562270b6dd883f4d961bd0e07724bb5366a2a495 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sat, 17 Feb 2024 15:08:35 +0100 Subject: [PATCH 15/23] wip Signed-off-by: bnechyporenko --- package.json | 1 + plugins/scaffolder-backend/api-report.md | 71 ++----------------- .../tasks/DatabaseTaskStore.test.ts | 8 ++- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 31 ++------ .../tasks/NunjucksWorkflowRunner.test.ts | 30 ++++---- .../tasks/NunjucksWorkflowRunner.ts | 14 +++- .../src/scaffolder/tasks/StorageTaskBroker.ts | 16 +---- .../src/scaffolder/tasks/types.ts | 16 +---- plugins/scaffolder-node/api-report.md | 10 +-- plugins/scaffolder-node/src/tasks/types.ts | 7 +- yarn.lock | 1 + 11 files changed, 53 insertions(+), 152 deletions(-) diff --git a/package.json b/package.json index afc86f2460..e224d1e58a 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@backstage/repo-tools": "workspace:*", "@changesets/cli": "^2.14.0", "@octokit/rest": "^19.0.3", + "@playwright/test": "^1.32.3", "@spotify/eslint-plugin": "^14.1.3", "@spotify/prettier-config": "^14.0.0", "@techdocs/cli": "workspace:*", diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 4f005a0e28..f9a810ebea 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -360,17 +360,7 @@ export interface CurrentClaimedTask { createdBy?: string; secrets?: TaskSecrets_2; spec: TaskSpec; - state?: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + state?: JsonObject; taskId: string; } @@ -411,15 +401,7 @@ export class DatabaseTaskStore implements TaskStore { // (undocumented) getTaskState({ taskId }: { taskId: string }): Promise< | { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; + state: JsonObject; } | undefined >; @@ -445,22 +427,7 @@ export class DatabaseTaskStore implements TaskStore { ids: string[]; }>; // (undocumented) - saveTaskState(options: { - taskId: string; - state?: - | { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - } - | undefined; - }): Promise; + saveTaskState(options: { taskId: string; state?: JsonObject }): Promise; // (undocumented) shutdownTask(options: TaskStoreShutDownTaskOptions): Promise; } @@ -565,15 +532,7 @@ export class TaskManager implements TaskContext_2 { // (undocumented) getTaskState?(): Promise< | { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; + state?: JsonObject; } | undefined >; @@ -628,15 +587,7 @@ export interface TaskStore { // (undocumented) getTaskState?({ taskId }: { taskId: string }): Promise< | { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; + state: JsonObject; } | undefined >; @@ -663,17 +614,7 @@ export interface TaskStore { // (undocumented) saveTaskState?(options: { taskId: string; - state?: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; + state?: JsonObject; }): Promise; // (undocumented) shutdownTask?(options: TaskStoreShutDownTaskOptions): Promise; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts index deb72fb7d2..8fda4b5090 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts @@ -209,9 +209,11 @@ describe('DatabaseTaskStore', () => { const state = await store.getTaskState({ taskId }); expect(state).toStrictEqual({ - 'repo.create': { - status: 'success', - value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + state: { + 'repo.create': { + status: 'success', + value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + }, }, }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 263fe3d435..6422e53ee9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { JsonObject, JsonValue } from '@backstage/types'; +import { JsonObject } from '@backstage/types'; import { PluginDatabaseManager, resolvePackagePath, @@ -397,42 +397,19 @@ export class DatabaseTaskStore implements TaskStore { async getTaskState({ taskId }: { taskId: string }): Promise< | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; + state: JsonObject; } | undefined > { const [result] = await this.db('tasks') .where({ id: taskId }) .select('state'); - return result.state - ? (JSON.parse(result.state) as unknown as { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }) - : undefined; + return result.state ? { state: JSON.parse(result.state) } : undefined; } async saveTaskState(options: { taskId: string; - state?: - | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - } - | undefined; + state?: JsonObject; }): Promise { if (options.state) { const serializedState = JSON.stringify(options.state); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 8018812c07..eaf477f35f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -580,23 +580,27 @@ describe('NunjucksWorkflowRunner', () => { }), getTaskState: (): Promise< | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; + state: { + [key: string]: + | { status: 'failed'; reason: string } + | { + status: 'success'; + value: JsonValue; + }; + }; } | undefined > => { return Promise.resolve({ - ['v1.task.checkpoint.key1']: { - status: 'success', - value: 'initial', - }, - ['v1.task.checkpoint.key2']: { - status: 'failed', - reason: 'fatal error', + state: { + ['v1.task.checkpoint.key1']: { + status: 'success', + value: 'initial', + }, + ['v1.task.checkpoint.key2']: { + status: 'failed', + reason: 'fatal error', + }, }, }); }, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 1070972112..e3bf094009 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -79,6 +79,18 @@ type TemplateContext = { each?: JsonValue; }; +type TaskState = { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; +}; + const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { return taskSpec.apiVersion === 'scaffolder.backstage.io/v1beta3'; }; @@ -356,7 +368,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { try { let prevValue: U | undefined; if (prevTaskState) { - const prevState = prevTaskState[key]; + const prevState = (prevTaskState.state as TaskState)?.[key]; if (prevState && prevState.status === 'success') { prevValue = prevState.value as U; } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index 2287279ea9..c437795778 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -93,12 +93,7 @@ export class TaskManager implements TaskContext { async getTaskState?(): Promise< | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; + state?: JsonObject; } | undefined > { @@ -186,14 +181,7 @@ export interface CurrentClaimedTask { /** * The state of checkpoints of the task. */ - state?: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + state?: JsonObject; /** * The creator of the task. */ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index d0bb48f4f2..c0854af391 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -196,26 +196,14 @@ export interface TaskStore { getTaskState?({ taskId }: { taskId: string }): Promise< | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; + state: JsonObject; } | undefined >; saveTaskState?(options: { taskId: string; - state?: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + state?: JsonObject; }): Promise; listEvents( diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index 7c024a0043..3f2e4e369f 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -351,15 +351,7 @@ export interface TaskContext { // (undocumented) getTaskState?(): Promise< | { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; + state?: JsonObject; } | undefined >; diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 18383dbac3..0cca1a0ba2 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -128,12 +128,7 @@ export interface TaskContext { getTaskState?(): Promise< | { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; + state?: JsonObject; } | undefined >; diff --git a/yarn.lock b/yarn.lock index cd5f483ddc..65a4a0a508 100644 --- a/yarn.lock +++ b/yarn.lock @@ -41210,6 +41210,7 @@ __metadata: "@changesets/cli": ^2.14.0 "@manypkg/get-packages": ^1.1.3 "@octokit/rest": ^19.0.3 + "@playwright/test": ^1.32.3 "@spotify/eslint-plugin": ^14.1.3 "@spotify/prettier-config": ^14.0.0 "@techdocs/cli": "workspace:*" From 2d40f79c44bd7433478cfbc9043d66b9b82c39b6 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Sun, 18 Feb 2024 11:32:42 +0100 Subject: [PATCH 16/23] wip Signed-off-by: bnechyporenko --- plugins/scaffolder-node/api-report.md | 12 ------------ plugins/scaffolder-node/src/tasks/types.ts | 8 -------- 2 files changed, 20 deletions(-) diff --git a/plugins/scaffolder-node/api-report.md b/plugins/scaffolder-node/api-report.md index 3f2e4e369f..76e22c0c5a 100644 --- a/plugins/scaffolder-node/api-report.md +++ b/plugins/scaffolder-node/api-report.md @@ -364,18 +364,6 @@ export interface TaskContext { // (undocumented) spec: TaskSpec; // (undocumented) - state?: { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; - }; - // (undocumented) updateCheckpoint?( options: | { diff --git a/plugins/scaffolder-node/src/tasks/types.ts b/plugins/scaffolder-node/src/tasks/types.ts index 0cca1a0ba2..e57668e346 100644 --- a/plugins/scaffolder-node/src/tasks/types.ts +++ b/plugins/scaffolder-node/src/tasks/types.ts @@ -110,14 +110,6 @@ export interface TaskContext { cancelSignal: AbortSignal; spec: TaskSpec; secrets?: TaskSecrets; - state?: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; createdBy?: string; done: boolean; isDryRun?: boolean; From c729e282c505ae60e4215d35a209b866356209d0 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Mon, 19 Feb 2024 19:40:38 +0100 Subject: [PATCH 17/23] wip Signed-off-by: bnechyporenko --- package.json | 1 - .../tasks/DatabaseTaskStore.test.ts | 16 ++++++----- .../tasks/StorageTaskBroker.test.ts | 27 +++++++++++++++++++ .../src/scaffolder/tasks/StorageTaskBroker.ts | 17 ++++++++++-- yarn.lock | 1 - 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index e224d1e58a..afc86f2460 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "@backstage/repo-tools": "workspace:*", "@changesets/cli": "^2.14.0", "@octokit/rest": "^19.0.3", - "@playwright/test": "^1.32.3", "@spotify/eslint-plugin": "^14.1.3", "@spotify/prettier-config": "^14.0.0", "@techdocs/cli": "workspace:*", diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts index 8fda4b5090..fe9cf3c661 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.test.ts @@ -199,9 +199,11 @@ describe('DatabaseTaskStore', () => { await store.saveTaskState({ taskId, state: { - 'repo.create': { - status: 'success', - value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + checkpoints: { + 'repo.create': { + status: 'success', + value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + }, }, }, }); @@ -210,9 +212,11 @@ describe('DatabaseTaskStore', () => { expect(state).toStrictEqual({ state: { - 'repo.create': { - status: 'success', - value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + checkpoints: { + 'repo.create': { + status: 'success', + value: { repoUrl: 'https://github.com/backstage/backstage.git' }, + }, }, }, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index 3d2fb28538..f4bae43dbf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -242,4 +242,31 @@ describe('StorageTaskBroker', () => { const promise = broker.list({ createdBy: 'user:default/foo' }); await expect(promise).resolves.toEqual({ tasks: [task] }); }); + + it('should handle checkpoints in task state', async () => { + const broker = new StorageTaskBroker(storage, logger); + + await broker.dispatch({ + spec: { steps: [] } as unknown as TaskSpec, + createdBy: 'user:default/foo', + }); + + const taskA = await broker.claim(); + await taskA.updateCheckpoint?.({ + key: 'repo.create', + status: 'success', + value: 'https://github.com/backstage/backstage.git', + }); + + expect(await taskA.getTaskState?.()).toEqual({ + state: { + checkpoints: { + 'repo.create': { + status: 'success', + value: 'https://github.com/backstage/backstage.git', + }, + }, + }, + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts index c437795778..fec745a653 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.ts @@ -31,6 +31,19 @@ import { import { TaskStore } from './types'; import { readDuration } from './helper'; +type TaskState = { + checkpoints: { + [key: string]: + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; + }; +}; /** * TaskManager * @@ -115,9 +128,9 @@ export class TaskManager implements TaskContext { ): Promise { const { key, ...value } = options; if (this.task.state) { - this.task.state[key] = value; + (this.task.state as TaskState).checkpoints[key] = value; } else { - this.task.state = { [key]: value }; + this.task.state = { checkpoints: { [key]: value } }; } await this.storage.saveTaskState?.({ taskId: this.task.taskId, diff --git a/yarn.lock b/yarn.lock index 65a4a0a508..cd5f483ddc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -41210,7 +41210,6 @@ __metadata: "@changesets/cli": ^2.14.0 "@manypkg/get-packages": ^1.1.3 "@octokit/rest": ^19.0.3 - "@playwright/test": ^1.32.3 "@spotify/eslint-plugin": ^14.1.3 "@spotify/prettier-config": ^14.0.0 "@techdocs/cli": "workspace:*" From dbc8177a203b9779bf515b900500ba991dd4a86e Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Mon, 19 Feb 2024 19:44:46 +0100 Subject: [PATCH 18/23] wip Signed-off-by: bnechyporenko --- package.json | 1 + yarn.lock | 1 + 2 files changed, 2 insertions(+) diff --git a/package.json b/package.json index afc86f2460..e224d1e58a 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@backstage/repo-tools": "workspace:*", "@changesets/cli": "^2.14.0", "@octokit/rest": "^19.0.3", + "@playwright/test": "^1.32.3", "@spotify/eslint-plugin": "^14.1.3", "@spotify/prettier-config": "^14.0.0", "@techdocs/cli": "workspace:*", diff --git a/yarn.lock b/yarn.lock index cd5f483ddc..65a4a0a508 100644 --- a/yarn.lock +++ b/yarn.lock @@ -41210,6 +41210,7 @@ __metadata: "@changesets/cli": ^2.14.0 "@manypkg/get-packages": ^1.1.3 "@octokit/rest": ^19.0.3 + "@playwright/test": ^1.32.3 "@spotify/eslint-plugin": ^14.1.3 "@spotify/prettier-config": ^14.0.0 "@techdocs/cli": "workspace:*" From 81a3aa1ce61f139d4779b62bb3f7c052290323ca Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Wed, 21 Feb 2024 14:44:11 +0100 Subject: [PATCH 19/23] wip Signed-off-by: bnechyporenko --- .../tasks/NunjucksWorkflowRunner.test.ts | 27 ++++++++----------- .../tasks/NunjucksWorkflowRunner.ts | 4 ++- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index eaf477f35f..420ec69427 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -18,7 +18,7 @@ import { getVoidLogger } from '@backstage/backend-common'; import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; -import { JsonValue } from '@backstage/types'; +import { JsonObject } from '@backstage/types'; import { ConfigReader } from '@backstage/config'; import { TaskSpec } from '@backstage/plugin-scaffolder-common'; import { @@ -580,26 +580,21 @@ describe('NunjucksWorkflowRunner', () => { }), getTaskState: (): Promise< | { - state: { - [key: string]: - | { status: 'failed'; reason: string } - | { - status: 'success'; - value: JsonValue; - }; - }; + state: JsonObject; } | undefined > => { return Promise.resolve({ state: { - ['v1.task.checkpoint.key1']: { - status: 'success', - value: 'initial', - }, - ['v1.task.checkpoint.key2']: { - status: 'failed', - reason: 'fatal error', + checkpoints: { + ['v1.task.checkpoint.key1']: { + status: 'success', + value: 'initial', + }, + ['v1.task.checkpoint.key2']: { + status: 'failed', + reason: 'fatal error', + }, }, }, }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index e3bf094009..634ac420aa 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -368,7 +368,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { try { let prevValue: U | undefined; if (prevTaskState) { - const prevState = (prevTaskState.state as TaskState)?.[key]; + const prevState = ( + prevTaskState.state?.checkpoints as TaskState + )?.[key]; if (prevState && prevState.status === 'success') { prevValue = prevState.value as U; } From 9154a29cc7e5aa472ad5d1e9769134edaf4652c9 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Wed, 21 Feb 2024 14:47:07 +0100 Subject: [PATCH 20/23] wip Signed-off-by: bnechyporenko --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 634ac420aa..d8fb4cdc3c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -79,7 +79,7 @@ type TemplateContext = { each?: JsonValue; }; -type TaskState = { +type CheckpointsState = { [key: string]: | { status: 'failed'; @@ -369,7 +369,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { let prevValue: U | undefined; if (prevTaskState) { const prevState = ( - prevTaskState.state?.checkpoints as TaskState + prevTaskState.state?.checkpoints as CheckpointsState )?.[key]; if (prevState && prevState.status === 'success') { prevValue = prevState.value as U; From b0124b549b18ca7523c8196cb9f82a8447c150f8 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Wed, 21 Feb 2024 15:07:05 +0100 Subject: [PATCH 21/23] wip Signed-off-by: bnechyporenko --- .../tasks/NunjucksWorkflowRunner.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index d8fb4cdc3c..574f424fe4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -79,17 +79,15 @@ type TemplateContext = { each?: JsonValue; }; -type CheckpointsState = { - [key: string]: - | { - status: 'failed'; - reason: string; - } - | { - status: 'success'; - value: JsonValue; - }; -}; +type CheckpointState = + | { + status: 'failed'; + reason: string; + } + | { + status: 'success'; + value: JsonValue; + }; const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { return taskSpec.apiVersion === 'scaffolder.backstage.io/v1beta3'; @@ -369,7 +367,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { let prevValue: U | undefined; if (prevTaskState) { const prevState = ( - prevTaskState.state?.checkpoints as CheckpointsState + prevTaskState.state?.checkpoints as { + [key: string]: CheckpointState; + } )?.[key]; if (prevState && prevState.status === 'success') { prevValue = prevState.value as U; From a588d5ee0ad22b960d81b195f632d6d699acc2f2 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Fri, 23 Feb 2024 09:26:23 +0100 Subject: [PATCH 22/23] wip Signed-off-by: bnechyporenko --- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 6422e53ee9..2a39440775 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -404,7 +404,7 @@ export class DatabaseTaskStore implements TaskStore { const [result] = await this.db('tasks') .where({ id: taskId }) .select('state'); - return result.state ? { state: JSON.parse(result.state) } : undefined; + return result.state ? JSON.parse(result.state) : undefined; } async saveTaskState(options: { @@ -412,7 +412,7 @@ export class DatabaseTaskStore implements TaskStore { state?: JsonObject; }): Promise { if (options.state) { - const serializedState = JSON.stringify(options.state); + const serializedState = JSON.stringify({ state: options.state }); await this.db('tasks') .where({ id: options.taskId }) .update({ From 448650ccac5238bd3484980dfe11abeb8b22decf Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 27 Feb 2024 15:50:13 +0100 Subject: [PATCH 23/23] chore: fix Signed-off-by: blam --- .changeset/sixty-queens-mix.md | 2 -- plugins/scaffolder-backend-module-github/package.json | 2 -- yarn.lock | 2 -- 3 files changed, 6 deletions(-) diff --git a/.changeset/sixty-queens-mix.md b/.changeset/sixty-queens-mix.md index 52f2ca5c39..8fe3f74617 100644 --- a/.changeset/sixty-queens-mix.md +++ b/.changeset/sixty-queens-mix.md @@ -1,7 +1,5 @@ --- '@backstage/plugin-scaffolder-backend': minor -'@backstage/plugin-scaffolder-backend-module-github': patch -'@backstage/plugin-scaffolder-common': patch '@backstage/plugin-scaffolder-node': patch --- diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index 62b0415049..15e148b49c 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -42,9 +42,7 @@ "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", - "@backstage/plugin-scaffolder-common": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", - "@backstage/types": "workspace:^", "@octokit/webhooks": "^10.0.0", "libsodium-wrappers": "^0.7.11", "octokit": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 754d563751..1e712c97a9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8374,10 +8374,8 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/integration": "workspace:^" - "@backstage/plugin-scaffolder-common": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" "@backstage/plugin-scaffolder-node-test-utils": "workspace:^" - "@backstage/types": "workspace:^" "@octokit/webhooks": ^10.0.0 "@types/libsodium-wrappers": ^0.7.10 fs-extra: ^11.2.0