diff --git a/.changeset/bright-mugs-remember.md b/.changeset/bright-mugs-remember.md new file mode 100644 index 0000000000..b1746aee4a --- /dev/null +++ b/.changeset/bright-mugs-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +The `versions:bump` command will now reject `*` as a pattern. diff --git a/.changeset/few-ducks-cross.md b/.changeset/few-ducks-cross.md new file mode 100644 index 0000000000..8f1bc15e15 --- /dev/null +++ b/.changeset/few-ducks-cross.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend-module-gitlab': patch +--- + +Fix automated assignment of reviewers for instances without premium/ultimate license (404). Introduce opt-in flag for automatic reviewer assignment based on approval rules diff --git a/.changeset/slow-bananas-beam.md b/.changeset/slow-bananas-beam.md new file mode 100644 index 0000000000..40bc5f21e9 --- /dev/null +++ b/.changeset/slow-bananas-beam.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Only allow pass through of `.mjs` in Jest transform if static ESM is supported. diff --git a/microsite/data/plugins/scaleops.yaml b/microsite/data/plugins/scaleops.yaml new file mode 100644 index 0000000000..f11733b35f --- /dev/null +++ b/microsite/data/plugins/scaleops.yaml @@ -0,0 +1,9 @@ +title: ScaleOps +author: terasky.com +authorUrl: https://github.com/terasky-oss +category: Kubernetes +description: View Rightsizing and resource usage data for your Kubernetes workloads right from within Backstage! +documentation: https://github.com/TeraSky-OSS/backstage-plugins/tree/main/plugins/scaleops-frontend +iconUrl: https://avatars.githubusercontent.com/u/100293501?s=200&v=4 +npmPackageName: '@terasky/backstage-plugin-scaleops-frontend' +addedDate: '2024-12-30' diff --git a/packages/cli/config/jestSwcTransform.js b/packages/cli/config/jestSwcTransform.js index a77683b939..4183349554 100644 --- a/packages/cli/config/jestSwcTransform.js +++ b/packages/cli/config/jestSwcTransform.js @@ -29,7 +29,7 @@ function createTransformer(config) { } // Skip transformation of .mjs files, they should only be used if ESM support is available - if (filePath.endsWith('.mjs')) { + if (jestOptions.supportsStaticESM && filePath.endsWith('.mjs')) { return { code: source }; } diff --git a/packages/cli/src/commands/versions/bump.ts b/packages/cli/src/commands/versions/bump.ts index 1e1e5deff2..6f8ce0a8a0 100644 --- a/packages/cli/src/commands/versions/bump.ts +++ b/packages/cli/src/commands/versions/bump.ts @@ -88,6 +88,8 @@ export default async (opts: OptionValues) => { if (!pattern) { console.log(`Using default pattern glob ${DEFAULT_PATTERN_GLOB}`); pattern = DEFAULT_PATTERN_GLOB; + } else if (pattern === '*') { + throw new Error(`Rejected pattern '*', please use a more specific pattern`); } else { console.log(`Using custom pattern glob ${pattern}`); } diff --git a/plugins/scaffolder-backend-module-gitlab/report.api.md b/plugins/scaffolder-backend-module-gitlab/report.api.md index 6d4c4bad30..aae92a0278 100644 --- a/plugins/scaffolder-backend-module-gitlab/report.api.md +++ b/plugins/scaffolder-backend-module-gitlab/report.api.md @@ -210,6 +210,7 @@ export const createPublishGitlabMergeRequestAction: (options: { removeSourceBranch?: boolean | undefined; assignee?: string | undefined; reviewers?: string[] | undefined; + assignReviewersFromApprovalRules?: boolean | undefined; }, JsonObject >; diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts index 874764b5ba..08194b52cc 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.test.ts @@ -52,6 +52,11 @@ const mockGitlabClient = { }, MergeRequests: { create: jest.fn(async (repoId: string) => { + if (repoId === 'owner/repo-without-approval-rule-license') { + return { + iid: 6, + }; + } if (repoId === 'owner/repo-without-approvals') { return { iid: 5, @@ -63,6 +68,11 @@ const mockGitlabClient = { }; }), show: jest.fn(async (repoId: string, iid: number) => { + if (repoId === 'owner/repo-without-approval-rule-license' && iid === 6) { + return { + iid: 6, + }; + } if (repoId === 'owner/repo' && iid === 4) { return { iid: 4, @@ -91,6 +101,16 @@ const mockGitlabClient = { ) { return []; } + if ( + repoId === 'owner/repo-without-approval-rule-license' && + options.mergerequestIId === 6 + ) { + throw new Error('Not Found', { + cause: { + description: '404 Not found', + }, + }); + } return [ { id: 123, @@ -608,7 +628,7 @@ describe('createGitLabMergeRequest', () => { }); describe('createGitlabMergeRequestWithReviewers', () => { - it('no reviewers are set when a no reviewer are passed in options', async () => { + it('no dedicated reviewers are set when a no reviewer are passed in options but mr approval reviewers are set when flag activated', async () => { const input = { repoUrl: 'gitlab.com?repo=repo&owner=owner', title: 'Create my new MR', @@ -617,6 +637,7 @@ describe('createGitLabMergeRequest', () => { removeSourceBranch: false, targetPath: 'Subdirectory', assignee: 'John Smith', + assignReviewersFromApprovalRules: true, }; mockDir.setContent({ [workspacePath]: { @@ -667,6 +688,7 @@ describe('createGitLabMergeRequest', () => { targetPath: 'Subdirectory', assignee: 'John Smith', reviewers: ['Jane Doe', 'Bob Vance'], + assignReviewersFromApprovalRules: true, }; mockDir.setContent({ [workspacePath]: { @@ -708,7 +730,53 @@ describe('createGitLabMergeRequest', () => { ); }); - it('reviewer is set correcly when a valid reviewer username is passed in options and no MR rules exist', async () => { + it('reviewer is set correcly when a valid reviewer username is passed in options in combination with deactivated approval rules', async () => { + const input = { + repoUrl: 'gitlab.com?repo=repo&owner=owner', + title: 'Create my new MR', + branchName: 'new-mr', + description: 'This is an important change', + removeSourceBranch: false, + targetPath: 'Subdirectory', + assignee: 'John Smith', + reviewers: ['Jane Doe', 'Bob Vance'], + assignReviewersFromApprovalRules: false, + }; + mockDir.setContent({ + [workspacePath]: { + source: { 'foo.txt': 'Hello there!' }, + irrelevant: { 'bar.txt': 'Nothing to see here' }, + }, + }); + + const ctx = createMockActionContext({ input, workspacePath }); + await instance.handler(ctx); + + expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + ); + expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); + expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( + 'owner/repo', + 'new-mr', + 'main', + 'Create my new MR', + { + description: 'This is an important change', + removeSourceBranch: false, + assigneeId: 123, + reviewerIds: [456, 234], // Jane Doe and Bob Vance + }, + ); + expect( + mockGitlabClient.MergeRequestApprovals.allApprovalRules, + ).not.toHaveBeenCalled(); + expect(mockGitlabClient.MergeRequests.edit).not.toHaveBeenCalled(); + }); + + it('reviewer is set correctly when a valid reviewer username is passed in options and no MR rules exist and approval rules are activated', async () => { const input = { repoUrl: 'gitlab.com?repo=repo-without-approvals&owner=owner', title: 'Create my new MR', @@ -718,6 +786,7 @@ describe('createGitLabMergeRequest', () => { targetPath: 'Subdirectory', assignee: 'John Smith', reviewers: ['Jane Doe', 'Bob Vance'], + assignReviewersFromApprovalRules: true, }; mockDir.setContent({ [workspacePath]: { @@ -755,16 +824,18 @@ describe('createGitLabMergeRequest', () => { expect(mockGitlabClient.MergeRequests.edit).not.toHaveBeenCalled(); }); - it('reviewers are set correcly when valid reviewers username are passed in options', async () => { + it('reviewer is set correcly when a valid reviewer username is passed in options and MR rules are not included in the Gitlab license (404)', async () => { const input = { - repoUrl: 'gitlab.com?repo=repo&owner=owner', + repoUrl: + 'gitlab.com?repo=repo-without-approval-rule-license&owner=owner', title: 'Create my new MR', branchName: 'new-mr', description: 'This is an important change', removeSourceBranch: false, targetPath: 'Subdirectory', assignee: 'John Smith', - reviewers: ['Jane Doe', 'John Smith'], + reviewers: ['Jane Doe', 'Bob Vance'], + assignReviewersFromApprovalRules: true, }; mockDir.setContent({ [workspacePath]: { @@ -774,16 +845,17 @@ describe('createGitLabMergeRequest', () => { }); const ctx = createMockActionContext({ input, workspacePath }); + ctx.logger.warn = jest.fn(); await instance.handler(ctx); expect(mockGitlabClient.Branches.create).toHaveBeenCalledWith( - 'owner/repo', + 'owner/repo-without-approval-rule-license', 'new-mr', 'main', ); expect(mockGitlabClient.Commits.create).not.toHaveBeenCalled(); expect(mockGitlabClient.MergeRequests.create).toHaveBeenCalledWith( - 'owner/repo', + 'owner/repo-without-approval-rule-license', 'new-mr', 'main', 'Create my new MR', @@ -791,9 +863,19 @@ describe('createGitLabMergeRequest', () => { description: 'This is an important change', removeSourceBranch: false, assigneeId: 123, - reviewerIds: [456, 123], + reviewerIds: [456, 234], // Jane Doe and Bob Vance }, ); + expect( + mockGitlabClient.MergeRequestApprovals.allApprovalRules, + ).toHaveBeenCalledWith('owner/repo-without-approval-rule-license', { + mergerequestIId: 6, + }); + expect(mockGitlabClient.MergeRequests.edit).not.toHaveBeenCalled(); + expect(ctx.logger.warn).toHaveBeenCalledWith( + 'Failed to retrieve approval rules for MR 6: Error: Not Found. Proceeding with MR creation without reviewers from approval rules.', + ); + expect(ctx.output).toHaveBeenCalledWith('targetBranchName', 'main'); // This ensures that the MR scaffolder step finishes successfully and all errors are catched. }); it('assignee is not set when a valid assignee username is not passed in options', async () => { diff --git a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts index 607a904d0a..26f7eff4ce 100644 --- a/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts +++ b/plugins/scaffolder-backend-module-gitlab/src/actions/gitlabMergeRequest.ts @@ -25,6 +25,8 @@ import { RepositoryTreeSchema, CommitAction, SimpleUserSchema, + ExpandedMergeRequestSchema, + Camelize, } from '@gitbeaker/rest'; import path from 'path'; import { ScmIntegrationRegistry } from '@backstage/integration'; @@ -81,6 +83,57 @@ async function getFileAction( return defaultCommitAction; } +async function getReviewersFromApprovalRules( + api: InstanceType, + mergerequestIId: number, + repoID: string, + ctx: any, +): Promise { + try { + // Because we don't know the code owners before the MR is created, we can't check the approval rules beforehand. + // Getting the approval rules beforehand is very difficult, especially, because of the inheritance rules for groups. + // Code owners take a moment to be processed and added to the approval rules after the MR is created. + + let mergeRequest: + | ExpandedMergeRequestSchema + | Camelize = await api.MergeRequests.show( + repoID, + mergerequestIId, + ); + + while ( + mergeRequest.detailed_merge_status === 'preparing' || + mergeRequest.detailed_merge_status === 'approvals_syncing' || + mergeRequest.detailed_merge_status === 'checking' + ) { + mergeRequest = await api.MergeRequests.show(repoID, mergeRequest.iid); + ctx.logger.info(`${mergeRequest.detailed_merge_status}`); + } + + const approvalRules = await api.MergeRequestApprovals.allApprovalRules( + repoID, + { + mergerequestIId: mergeRequest.iid, + }, + ); + + return approvalRules + .filter(rule => rule.eligible_approvers !== undefined) + .map(rule => { + return rule.eligible_approvers as SimpleUserSchema[]; + }) + .flat() + .map(user => user.id); + } catch (e) { + ctx.logger.warn( + `Failed to retrieve approval rules for MR ${mergerequestIId}: ${getErrorMessage( + e, + )}. Proceeding with MR creation without reviewers from approval rules.`, + ); + return []; + } +} + /** * Create a new action that creates a gitlab merge request. * @@ -106,6 +159,7 @@ export const createPublishGitlabMergeRequestAction = (options: { removeSourceBranch?: boolean; assignee?: string; reviewers?: string[]; + assignReviewersFromApprovalRules?: boolean; }>({ id: 'publish:gitlab:merge-request', examples, @@ -193,6 +247,12 @@ which uses additional API calls in order to detect whether to 'create', 'update' }, description: 'Users that will be assigned as reviewers', }, + assignReviewersFromApprovalRules: { + title: 'Assign reviewers from approval rules', + type: 'boolean', + description: + 'Automatically assign reviewers from the approval rules of the MR. Includes Codeowners', + }, }, }, output: { @@ -404,48 +464,37 @@ which uses additional API calls in order to detect whether to 'create', 'update' }, ); - // Because we don't know the code owners before the MR is created, we can't check the approval rules beforehand. - // Getting the approval rules beforehand is very difficult, especially, because of the inheritance rules for groups. - // Code owners take a moment to be processed and added to the approval rules after the MR is created. + if (ctx.input.assignReviewersFromApprovalRules) { + try { + const reviewersFromApprovalRules = + await getReviewersFromApprovalRules( + api, + mergeRequest.iid, + repoID, + ctx, + ); + if (reviewersFromApprovalRules.length > 0) { + const eligibleUserIds = new Set([ + ...reviewersFromApprovalRules, + ...(reviewerIds ?? []), + ]); - while ( - mergeRequest.detailed_merge_status === 'preparing' || - mergeRequest.detailed_merge_status === 'approvals_syncing' || - mergeRequest.detailed_merge_status === 'checking' - ) { - mergeRequest = await api.MergeRequests.show(repoID, mergeRequest.iid); - ctx.logger.info(`${mergeRequest.detailed_merge_status}`); + mergeRequest = await api.MergeRequests.edit( + repoID, + mergeRequest.iid, + { + reviewerIds: Array.from(eligibleUserIds), + }, + ); + } + } catch (e) { + ctx.logger.warn( + `Failed to assign reviewers from approval rules: ${getErrorMessage( + e, + )}.`, + ); + } } - - const approvalRules = await api.MergeRequestApprovals.allApprovalRules( - repoID, - { - mergerequestIId: mergeRequest.iid, - }, - ); - - if (approvalRules.length !== 0) { - const eligibleApprovers = approvalRules - .filter(rule => rule.eligible_approvers !== undefined) - .map(rule => { - return rule.eligible_approvers as SimpleUserSchema[]; - }) - .flat(); - - const eligibleUserIds = new Set([ - ...eligibleApprovers.map(user => user.id), - ...(reviewerIds ?? []), - ]); - - mergeRequest = await api.MergeRequests.edit( - repoID, - mergeRequest.iid, - { - reviewerIds: Array.from(eligibleUserIds), - }, - ); - } - ctx.output('projectid', repoID); ctx.output('targetBranchName', targetBranch); ctx.output('projectPath', repoID); diff --git a/plugins/scaffolder-backend/report.api.md b/plugins/scaffolder-backend/report.api.md index b542cb1c94..f21bcd76f7 100644 --- a/plugins/scaffolder-backend/report.api.md +++ b/plugins/scaffolder-backend/report.api.md @@ -360,6 +360,7 @@ export const createPublishGitlabMergeRequestAction: (options: { removeSourceBranch?: boolean | undefined; assignee?: string | undefined; reviewers?: string[] | undefined; + assignReviewersFromApprovalRules?: boolean | undefined; }, JsonObject >;