From 92582f17a011e39b647134d4dc127115fb54ea67 Mon Sep 17 00:00:00 2001 From: bnechyporenko Date: Thu, 8 Feb 2024 20:43:27 +0100 Subject: [PATCH] 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; }; };