From ce0d10cb4804f8441f420581a6df3f270d875e6c Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Thu, 12 Sep 2024 11:52:59 +0200 Subject: [PATCH 01/18] support reviewers, prevent_self_review and wait_timer Signed-off-by: Raghunandan Balachandran --- .../src/actions/gitHubEnvironment.examples.ts | 57 +++++++ .../githubEnvironment.examples.test.ts | 150 ++++++++++++++++++ .../src/actions/githubEnvironment.test.ts | 124 +++++++++++++++ .../src/actions/githubEnvironment.ts | 43 +++++ 4 files changed, 374 insertions(+) diff --git a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts index 7c5f7efa3d..44ddcc2175 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts @@ -302,4 +302,61 @@ export const examples: TemplateExample[] = [ ], }), }, + { + description: 'Create a GitHub Environment with Wait Timer', + example: yaml.stringify({ + steps: [ + { + action: 'github:environment:create', + name: 'Create Environment', + input: { + repoUrl: 'github.com?repo=repository&owner=owner', + name: 'envname', + wait_timer: 1000, + }, + }, + ], + }), + }, + { + description: 'Create a GitHub Environment with Prevent Self Review', + example: yaml.stringify({ + steps: [ + { + action: 'github:environment:create', + name: 'Create Environment', + input: { + repoUrl: 'github.com?repo=repository&owner=owner', + name: 'envname', + prevent_self_review: true, + }, + }, + ], + }), + }, + { + description: 'Create a GitHub Environment with Reviewers', + example: yaml.stringify({ + steps: [ + { + action: 'github:environment:create', + name: 'Create Environment', + input: { + repoUrl: 'github.com?repo=repository&owner=owner', + name: 'envname', + reviewers: [ + { + Type: 'User', + ID: 1, + }, + { + Type: 'Team', + ID: 2, + }, + ], + }, + }, + ], + }), + }, ]; diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts index c8b133859b..7d0b72c8ba 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts @@ -95,6 +95,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( mockOctokit.rest.repos.createDeploymentBranchPolicy, @@ -128,6 +131,9 @@ describe('github:environment:create examples', () => { protected_branches: true, custom_branch_policies: false, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -162,6 +168,9 @@ describe('github:environment:create examples', () => { protected_branches: false, custom_branch_policies: true, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -208,6 +217,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -280,6 +292,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -331,6 +346,9 @@ describe('github:environment:create examples', () => { custom_branch_policies: true, protected_branches: false, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -382,6 +400,9 @@ describe('github:environment:create examples', () => { custom_branch_policies: true, protected_branches: false, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -473,6 +494,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -503,6 +527,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -536,6 +563,9 @@ describe('github:environment:create examples', () => { custom_branch_policies: true, protected_branches: false, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -593,6 +623,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -643,6 +676,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -697,6 +733,9 @@ describe('github:environment:create examples', () => { custom_branch_policies: false, protected_branches: true, }, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -726,6 +765,9 @@ describe('github:environment:create examples', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: false, }); expect( @@ -782,4 +824,112 @@ describe('github:environment:create examples', () => { secret_name: 'SECRET2', }); }); + + it(`should ${examples[14].description}`, async () => { + const input = yaml.parse(examples[14].example).steps[0].input; + + await action.handler({ + ...mockContext, + input, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + wait_timer: 1000, + reviewers: null, + prevent_self_review: false, + }); + expect( + mockOctokit.rest.repos.createDeploymentBranchPolicy, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createEnvironmentVariable, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.getEnvironmentPublicKey, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createOrUpdateEnvironmentSecret, + ).not.toHaveBeenCalled(); + }); + + it(`should ${examples[15].description}`, async () => { + const input = yaml.parse(examples[15].example).steps[0].input; + + await action.handler({ + ...mockContext, + input, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + wait_timer: 0, + reviewers: null, + prevent_self_review: true, + }); + expect( + mockOctokit.rest.repos.createDeploymentBranchPolicy, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createEnvironmentVariable, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.getEnvironmentPublicKey, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createOrUpdateEnvironmentSecret, + ).not.toHaveBeenCalled(); + }); + + it(`should ${examples[16].description}`, async () => { + const input = yaml.parse(examples[16].example).steps[0].input; + + await action.handler({ + ...mockContext, + input, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + wait_timer: 0, + reviewers: [ + { + Type: 'User', + ID: 1, + }, + { + Type: 'Team', + ID: 2, + }, + ], + prevent_self_review: false, + }); + expect( + mockOctokit.rest.repos.createDeploymentBranchPolicy, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createEnvironmentVariable, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.getEnvironmentPublicKey, + ).not.toHaveBeenCalled(); + expect( + mockOctokit.rest.actions.createOrUpdateEnvironmentSecret, + ).not.toHaveBeenCalled(); + }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts index 139d8028eb..2c729e2e5a 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts @@ -94,6 +94,9 @@ describe('github:environment:create', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); }); @@ -119,6 +122,9 @@ describe('github:environment:create', () => { protected_branches: true, custom_branch_policies: false, }, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); }); @@ -145,6 +151,9 @@ describe('github:environment:create', () => { protected_branches: false, custom_branch_policies: true, }, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); expect( @@ -190,6 +199,9 @@ describe('github:environment:create', () => { protected_branches: false, custom_branch_policies: true, }, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); expect( @@ -231,6 +243,9 @@ describe('github:environment:create', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); expect( @@ -277,6 +292,9 @@ describe('github:environment:create', () => { repo: 'repository', environment_name: 'envname', deployment_branch_policy: null, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, }); expect( @@ -316,4 +334,110 @@ describe('github:environment:create', () => { environment_name: 'envname', }); }); + + it('should work with wait_timer', async () => { + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + wait_timer: 1000, + }, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + reviewers: null, + wait_timer: 1000, + prevent_self_review: false, + }); + }); + + it('should work with prevent_self_review set to true', async () => { + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + prevent_self_review: true, + }, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + reviewers: null, + wait_timer: 0, + prevent_self_review: true, + }); + }); + + it('should work with prevent_self_review set to false', async () => { + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + prevent_self_review: false, + }, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + reviewers: null, + wait_timer: 0, + prevent_self_review: false, + }); + }); + + it('should work with reviewers', async () => { + await action.handler({ + ...mockContext, + input: { + ...mockContext.input, + reviewers: [ + { + Type: 'User', + ID: 1, + }, + { + Type: 'Team', + ID: 2, + }, + ], + }, + }); + + expect( + mockOctokit.rest.repos.createOrUpdateEnvironment, + ).toHaveBeenCalledWith({ + owner: 'owner', + repo: 'repository', + environment_name: 'envname', + deployment_branch_policy: null, + wait_timer: 0, + prevent_self_review: false, + reviewers: [ + { + Type: 'User', + ID: 1, + }, + { + Type: 'Team', + ID: 2, + }, + ], + }); + }); }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts index d3e8d0e5d2..8861aa5bd3 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts @@ -48,6 +48,9 @@ export function createGithubEnvironmentAction(options: { environmentVariables?: { [key: string]: string }; secrets?: { [key: string]: string }; token?: string; + wait_timer?: number; + prevent_self_review?: boolean; + reviewers?: Array<{ type?: 'User' | 'Team'; id?: number }> | null; }>({ id: 'github:environment:create', description: 'Creates Deployment Environments', @@ -120,6 +123,35 @@ export function createGithubEnvironmentAction(options: { type: 'string', description: 'The token to use for authorization to GitHub', }, + wait_timer: { + title: 'Wait Timer', + type: 'integer', + description: + 'The time to wait before creating or updating the environment (in milliseconds)', + }, + prevent_self_review: { + title: 'Prevent Self Review', + type: 'boolean', + description: 'Whether to prevent self-review for this environment', + }, + reviewers: { + title: 'Reviewers', + type: 'array', + description: 'Reviewers for this environment', + items: { + type: 'object', + properties: { + type: { + title: 'Type', + type: 'string', + }, + id: { + title: 'ID', + type: 'integer', + }, + }, + }, + }, }, }, }, @@ -133,8 +165,16 @@ export function createGithubEnvironmentAction(options: { environmentVariables, secrets, token: providedToken, + wait_timer, + prevent_self_review, + reviewers, } = ctx.input; + // Wait for the specified time before creating or updating the environment + if (wait_timer) { + await new Promise(resolve => setTimeout(resolve, wait_timer)); + } + const octokitOptions = await getOctokitOptions({ integrations, token: providedToken, @@ -158,6 +198,9 @@ export function createGithubEnvironmentAction(options: { repo: repo, environment_name: name, deployment_branch_policy: deploymentBranchPolicy ?? null, + wait_timer: wait_timer ?? 0, + prevent_self_review: prevent_self_review ?? false, + reviewers: reviewers ?? null, }); if (customBranchPolicyNames) { From d0ea1748aa4c7e1ccb7a0db19d4fa73a377d0efe Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Sun, 15 Sep 2024 23:23:43 +0200 Subject: [PATCH 02/18] Change imput param keys to camelCase. Fetch entityrefs from catalog and get users and teams from github for env reviewers. Signed-off-by: Raghunandan Balachandran --- .../package.json | 2 + .../src/actions/gitHubEnvironment.examples.ts | 13 +-- .../githubEnvironment.examples.test.ts | 4 +- .../src/actions/githubEnvironment.test.ts | 72 ++++++++++++----- .../src/actions/githubEnvironment.ts | 80 +++++++++++++------ .../src/module.ts | 5 +- 6 files changed, 117 insertions(+), 59 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index 9ebd61f809..2ad78b0000 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -44,9 +44,11 @@ "dependencies": { "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", + "@backstage/catalog-client": "workspace:^", "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", + "@backstage/plugin-catalog-node": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", "@octokit/webhooks": "^10.0.0", "libsodium-wrappers": "^0.7.11", diff --git a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts index 44ddcc2175..877a5bdf35 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts @@ -328,7 +328,7 @@ export const examples: TemplateExample[] = [ input: { repoUrl: 'github.com?repo=repository&owner=owner', name: 'envname', - prevent_self_review: true, + preventSelfReview: true, }, }, ], @@ -344,16 +344,7 @@ export const examples: TemplateExample[] = [ input: { repoUrl: 'github.com?repo=repository&owner=owner', name: 'envname', - reviewers: [ - { - Type: 'User', - ID: 1, - }, - { - Type: 'Team', - ID: 2, - }, - ], + reviewers: ['group:defautl/team-a', 'user:defautl/johndoe'], }, }, ], diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts index 7d0b72c8ba..3c710928ca 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts @@ -909,11 +909,11 @@ describe('github:environment:create examples', () => { wait_timer: 0, reviewers: [ { - Type: 'User', + Type: 'Team', ID: 1, }, { - Type: 'Team', + Type: 'User', ID: 2, }, ], diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts index 2c729e2e5a..9a4ffac162 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts @@ -19,6 +19,7 @@ import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test- import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; +import { CatalogClient } from '@backstage/catalog-client'; const mockOctokit = { rest: { @@ -32,8 +33,19 @@ const mockOctokit = { createOrUpdateEnvironment: jest.fn(), get: jest.fn(), }, + teams: { + getByName: jest.fn(), + }, + users: { + getByUsername: jest.fn(), + }, }, }; + +const mockCatalogClient: Partial = { + getEntitiesByRefs: jest.fn(), +}; + jest.mock('octokit', () => ({ Octokit: class { constructor() { @@ -42,6 +54,10 @@ jest.mock('octokit', () => ({ }, })); +jest.mock('@backstage/catalog-client', () => ({ + CatalogClient: mockCatalogClient, +})); + const publicKey = '2Sg8iYjAxxmI2LvUXpJjkYrMxURPc8r+dB7TJyvvcCU='; describe('github:environment:create', () => { @@ -76,9 +92,36 @@ describe('github:environment:create', () => { id: 'repoid', }, }); + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { + id: 1, + }, + }); + mockOctokit.rest.teams.getByName.mockResolvedValue({ + data: { + id: 2, + }, + }); + (mockCatalogClient.getEntitiesByRefs as jest.Mock).mockResolvedValue({ + items: [ + { + kind: 'User', + metadata: { + name: 'johndoe', + }, + }, + { + kind: 'Group', + metadata: { + name: 'team-a', + }, + }, + ], + }); action = createGithubEnvironmentAction({ integrations, + catalog: mockCatalogClient as unknown as CatalogClient, }); }); @@ -340,7 +383,7 @@ describe('github:environment:create', () => { ...mockContext, input: { ...mockContext.input, - wait_timer: 1000, + waitTimer: 1000, }, }); @@ -357,12 +400,12 @@ describe('github:environment:create', () => { }); }); - it('should work with prevent_self_review set to true', async () => { + it('should work with preventSelfReview set to true', async () => { await action.handler({ ...mockContext, input: { ...mockContext.input, - prevent_self_review: true, + preventSelfReview: true, }, }); @@ -379,12 +422,12 @@ describe('github:environment:create', () => { }); }); - it('should work with prevent_self_review set to false', async () => { + it('should work with preventSelfReview set to false', async () => { await action.handler({ ...mockContext, input: { ...mockContext.input, - prevent_self_review: false, + preventSelfReview: false, }, }); @@ -406,16 +449,7 @@ describe('github:environment:create', () => { ...mockContext, input: { ...mockContext.input, - reviewers: [ - { - Type: 'User', - ID: 1, - }, - { - Type: 'Team', - ID: 2, - }, - ], + reviewers: ['group:defautl/team-a', 'user:defautl/johndoe'], }, }); @@ -430,12 +464,12 @@ describe('github:environment:create', () => { prevent_self_review: false, reviewers: [ { - Type: 'User', - ID: 1, + id: 1, + type: 'User', }, { - Type: 'Team', - ID: 2, + id: 2, + type: 'Team', }, ], }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts index 8861aa5bd3..b482503717 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts @@ -24,6 +24,7 @@ import { getOctokitOptions } from './helpers'; import { Octokit } from 'octokit'; import Sodium from 'libsodium-wrappers'; import { examples } from './gitHubEnvironment.examples'; +import { CatalogClient } from '@backstage/catalog-client'; /** * Creates an `github:environment:create` Scaffolder action that creates a Github Environment. @@ -32,8 +33,9 @@ import { examples } from './gitHubEnvironment.examples'; */ export function createGithubEnvironmentAction(options: { integrations: ScmIntegrationRegistry; + catalog: CatalogClient; }) { - const { integrations } = options; + const { integrations, catalog } = options; // For more information on how to define custom actions, see // https://backstage.io/docs/features/software-templates/writing-custom-actions return createTemplateAction<{ @@ -48,9 +50,9 @@ export function createGithubEnvironmentAction(options: { environmentVariables?: { [key: string]: string }; secrets?: { [key: string]: string }; token?: string; - wait_timer?: number; - prevent_self_review?: boolean; - reviewers?: Array<{ type?: 'User' | 'Team'; id?: number }> | null; + waitTimer?: number; + preventSelfReview?: boolean; + reviewers?: string[]; }>({ id: 'github:environment:create', description: 'Creates Deployment Environments', @@ -123,13 +125,13 @@ export function createGithubEnvironmentAction(options: { type: 'string', description: 'The token to use for authorization to GitHub', }, - wait_timer: { + waitTimer: { title: 'Wait Timer', type: 'integer', description: 'The time to wait before creating or updating the environment (in milliseconds)', }, - prevent_self_review: { + preventSelfReview: { title: 'Prevent Self Review', type: 'boolean', description: 'Whether to prevent self-review for this environment', @@ -139,17 +141,7 @@ export function createGithubEnvironmentAction(options: { type: 'array', description: 'Reviewers for this environment', items: { - type: 'object', - properties: { - type: { - title: 'Type', - type: 'string', - }, - id: { - title: 'ID', - type: 'integer', - }, - }, + type: 'string', }, }, }, @@ -165,15 +157,14 @@ export function createGithubEnvironmentAction(options: { environmentVariables, secrets, token: providedToken, - wait_timer, - prevent_self_review, + waitTimer, + preventSelfReview, reviewers, } = ctx.input; - // Wait for the specified time before creating or updating the environment - if (wait_timer) { - await new Promise(resolve => setTimeout(resolve, wait_timer)); - } + // When environment creation step is executed right after a repo publish step, the repository might not be available immediately. + // Add a 2-second delay before initiating the steps in this action. + await new Promise(resolve => setTimeout(resolve, 2000)); const octokitOptions = await getOctokitOptions({ integrations, @@ -193,14 +184,51 @@ export function createGithubEnvironmentAction(options: { repo: repo, }); + // convert reviewers from catalog entity to Github user or team + const githubReviewers: { type: 'User' | 'Team'; id: number }[] = []; + if (reviewers) { + // Fetch reviewers from Catalog + const { items: reviewersEntityRefs } = await catalog.getEntitiesByRefs({ + entityRefs: reviewers, + }); + for (const reviewerEntityRef of reviewersEntityRefs) { + if (reviewerEntityRef?.kind === 'User') { + try { + const user = await client.rest.users.getByUsername({ + username: reviewerEntityRef.metadata.name, + }); + githubReviewers.push({ + type: 'User', + id: user.data.id, + }); + } catch (error) { + console.log('User not found:', error); + } + } else if (reviewerEntityRef?.kind === 'Group') { + try { + const team = await client.rest.teams.getByName({ + org: owner, + team_slug: reviewerEntityRef.metadata.name, + }); + githubReviewers.push({ + type: 'Team', + id: team.data.id, + }); + } catch (error) { + console.log('Team not found:', error); + } + } + } + } + await client.rest.repos.createOrUpdateEnvironment({ owner: owner, repo: repo, environment_name: name, deployment_branch_policy: deploymentBranchPolicy ?? null, - wait_timer: wait_timer ?? 0, - prevent_self_review: prevent_self_review ?? false, - reviewers: reviewers ?? null, + wait_timer: waitTimer ?? 0, + prevent_self_review: preventSelfReview ?? false, + reviewers: githubReviewers.length ? githubReviewers : null, }); if (customBranchPolicyNames) { diff --git a/plugins/scaffolder-backend-module-github/src/module.ts b/plugins/scaffolder-backend-module-github/src/module.ts index a5676ab8c9..4873cf445f 100644 --- a/plugins/scaffolder-backend-module-github/src/module.ts +++ b/plugins/scaffolder-backend-module-github/src/module.ts @@ -35,6 +35,7 @@ import { DefaultGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; +import { catalogServiceRef } from '@backstage/plugin-catalog-node'; /** * @public @@ -48,8 +49,9 @@ export const githubModule = createBackendModule({ deps: { scaffolder: scaffolderActionsExtensionPoint, config: coreServices.rootConfig, + catalog: catalogServiceRef, }, - async init({ scaffolder, config }) { + async init({ scaffolder, config, catalog }) { const integrations = ScmIntegrations.fromConfig(config); const githubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); @@ -68,6 +70,7 @@ export const githubModule = createBackendModule({ }), createGithubEnvironmentAction({ integrations, + catalog, }), createGithubIssuesLabelAction({ integrations, From 42c13f1530e1ec80d2a698a30c99f5103c2980ea Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Sun, 15 Sep 2024 23:24:37 +0200 Subject: [PATCH 03/18] turn off testing library's aggressive reporting that flagged getByUsername and getByName github apis as sync apis in non-test code Signed-off-by: Raghunandan Balachandran --- .eslintrc.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.eslintrc.js b/.eslintrc.js index 01cad187be..caddf390f4 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -263,4 +263,7 @@ module.exports = { }, }, ], + settings: { + 'testing-library/custom-queries': 'off', + }, }; From be990859518dd16d8c04b52736fb36bb298134e6 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Sun, 15 Sep 2024 23:28:21 +0200 Subject: [PATCH 04/18] Add max no of entities option to MultiEntityPicker Signed-off-by: Raghunandan Balachandran --- .../MultiEntityPicker.test.tsx | 75 +++++++++++++++++++ .../MultiEntityPicker/MultiEntityPicker.tsx | 10 ++- .../fields/MultiEntityPicker/schema.ts | 4 + 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx index 4be07b139d..1ec7b56fa5 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx @@ -704,4 +704,79 @@ describe('', () => { expect(onChange).toHaveBeenCalledWith([]); }); }); + + describe('Multiselect maxNoOfEntities option', () => { + beforeEach(() => { + const testEntities = [ + makeEntity('Group', 'default', 'team-a'), + makeEntity('Group', 'default', 'squad-b'), + makeEntity('User', 'default', 'user-a'), + makeEntity('User', 'default', 'user-b'), + ]; + + uiSchema = { + 'ui:options': { + maxNoOfEntities: 2, + catalogFilter: [ + { + kind: ['Group'], + 'metadata.name': 'test-entity', + }, + { + kind: ['User'], + 'metadata.name': 'test-entity', + }, + ], + }, + allowArbitraryValues: true, + }; + props = { + onChange, + schema, + required: false, + uiSchema, + rawErrors, + formData, + } as unknown as FieldProps; + + catalogApi.getEntities.mockResolvedValue({ items: testEntities }); + }); + + it('User selects item', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.mouseDown(input); + const optionsBefore = screen.getAllByRole('option'); + + // Check that all options are enabled + optionsBefore.forEach(option => { + expect(option).toHaveAttribute('aria-disabled', 'false'); + }); + + fireEvent.mouseDown(input); + + // Select two options from the dropdown + fireEvent.change(input, { target: { value: 'team-a' } }); + fireEvent.blur(input); + + fireEvent.change(input, { target: { value: 'user-a' } }); + fireEvent.blur(input); + + expect(onChange).toHaveBeenCalledWith(['team-a']); + + fireEvent.mouseDown(input); + const optionsAfter = screen.getAllByRole('option'); + + // Check that all options are disabled when macNoOfEntities is reached + optionsAfter.forEach(option => { + expect(option).toHaveAttribute('aria-disabled', 'true'); + }); + }); + }); }); diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index d800c9b372..f620c75d07 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -34,7 +34,7 @@ import FormControl from '@material-ui/core/FormControl'; import Autocomplete, { AutocompleteChangeReason, } from '@material-ui/lab/Autocomplete'; -import React, { useCallback, useEffect } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import useAsync from 'react-use/esm/useAsync'; import { FieldValidation } from '@rjsf/utils'; import { @@ -65,6 +65,7 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { const defaultKind = uiSchema['ui:options']?.defaultKind; const defaultNamespace = uiSchema['ui:options']?.defaultNamespace || undefined; + const [noOfItemsSelected, setNoOfItemsSelected] = useState(0); const catalogApi = useApi(catalogApiRef); const entityPresentationApi = useApi(entityPresentationApiRef); @@ -92,6 +93,9 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { const allowArbitraryValues = uiSchema['ui:options']?.allowArbitraryValues ?? true; + // if not specified, default to undefined + const maxNoOfEntities = uiSchema['ui:options']?.maxNoOfEntities ?? undefined; + const onSelect = useCallback( (_: any, refs: (string | Entity)[], reason: AutocompleteChangeReason) => { const values = refs @@ -125,6 +129,7 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { }) .filter(ref => ref !== undefined) as string[]; + setNoOfItemsSelected(values.length); onChange(values); }, [onChange, formData, defaultKind, defaultNamespace, allowArbitraryValues], @@ -158,6 +163,9 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { : entities?.entityRefToPresentation.get(stringifyEntityRef(option)) ?.entityRef! } + getOptionDisabled={_options => + maxNoOfEntities ? noOfItemsSelected >= maxNoOfEntities : false + } autoSelect freeSolo={allowArbitraryValues} renderInput={params => ( diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts b/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts index 2621157d02..0c7989e8bf 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts @@ -47,6 +47,10 @@ export const MultiEntityPickerFieldSchema = makeFieldSchemaFromZod( .or(entityQueryFilterExpressionSchema) .optional() .describe('List of key-value filter expression for entities'), + maxNoOfEntities: z + .number() + .optional() + .describe('The maximum number of entities that can be selected'), }), ); From 51b4b97bdced780c8d7cdf2ad5b7dd8316ebf6e0 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Sun, 15 Sep 2024 23:28:42 +0200 Subject: [PATCH 05/18] yarn lock changes Signed-off-by: Raghunandan Balachandran --- yarn.lock | 2 ++ 1 file changed, 2 insertions(+) diff --git a/yarn.lock b/yarn.lock index c235bba130..59a5d59adb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7107,10 +7107,12 @@ __metadata: "@backstage/backend-common": "workspace:^" "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-test-utils": "workspace:^" + "@backstage/catalog-client": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/integration": "workspace:^" + "@backstage/plugin-catalog-node": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" "@backstage/plugin-scaffolder-node-test-utils": "workspace:^" "@octokit/webhooks": ^10.0.0 From 878c8080b7ff675ac425ba839b78009a13d1f84f Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 01:36:41 +0200 Subject: [PATCH 06/18] Pass a catalogClient to the Github Environment Action Signed-off-by: Raghunandan Balachandran --- .../src/actions/gitHubEnvironment.examples.ts | 4 +- .../githubEnvironment.examples.test.ts | 48 +++++++++++++++++-- .../src/actions/githubEnvironment.test.ts | 8 ++-- .../src/actions/githubEnvironment.ts | 13 ++--- .../src/module.ts | 11 +++-- .../actions/builtin/createBuiltinActions.ts | 1 + 6 files changed, 65 insertions(+), 20 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts index 877a5bdf35..850fd24847 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/gitHubEnvironment.examples.ts @@ -312,7 +312,7 @@ export const examples: TemplateExample[] = [ input: { repoUrl: 'github.com?repo=repository&owner=owner', name: 'envname', - wait_timer: 1000, + waitTimer: 1000, }, }, ], @@ -344,7 +344,7 @@ export const examples: TemplateExample[] = [ input: { repoUrl: 'github.com?repo=repository&owner=owner', name: 'envname', - reviewers: ['group:defautl/team-a', 'user:defautl/johndoe'], + reviewers: ['group:default/team-a', 'user:default/johndoe'], }, }, ], diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts index 3c710928ca..e89aa466bc 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.examples.test.ts @@ -20,6 +20,7 @@ import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; import yaml from 'yaml'; import { examples } from './gitHubEnvironment.examples'; +import { CatalogApi } from '@backstage/catalog-client'; const mockOctokit = { rest: { @@ -33,8 +34,17 @@ const mockOctokit = { createOrUpdateEnvironment: jest.fn(), get: jest.fn(), }, + teams: { + getByName: jest.fn(), + }, + users: { + getByUsername: jest.fn(), + }, }, }; +const mockCatalogClient: Partial = { + getEntitiesByRefs: jest.fn(), +}; jest.mock('octokit', () => ({ Octokit: class { constructor() { @@ -42,6 +52,9 @@ jest.mock('octokit', () => ({ } }, })); +jest.mock('@backstage/catalog-client', () => ({ + CatalogClient: mockCatalogClient, +})); const publicKey = '2Sg8iYjAxxmI2LvUXpJjkYrMxURPc8r+dB7TJyvvcCU='; @@ -72,9 +85,36 @@ describe('github:environment:create examples', () => { id: 'repoid', }, }); + mockOctokit.rest.users.getByUsername.mockResolvedValue({ + data: { + id: 1, + }, + }); + mockOctokit.rest.teams.getByName.mockResolvedValue({ + data: { + id: 2, + }, + }); + (mockCatalogClient.getEntitiesByRefs as jest.Mock).mockResolvedValue({ + items: [ + { + kind: 'User', + metadata: { + name: 'johndoe', + }, + }, + { + kind: 'Group', + metadata: { + name: 'team-a', + }, + }, + ], + }); action = createGithubEnvironmentAction({ integrations, + catalogClient: mockCatalogClient as CatalogApi, }); }); @@ -909,12 +949,12 @@ describe('github:environment:create examples', () => { wait_timer: 0, reviewers: [ { - Type: 'Team', - ID: 1, + type: 'User', + id: 1, }, { - Type: 'User', - ID: 2, + type: 'Team', + id: 2, }, ], prevent_self_review: false, diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts index 9a4ffac162..1397942a7c 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.test.ts @@ -19,7 +19,7 @@ import { createMockActionContext } from '@backstage/plugin-scaffolder-node-test- import { TemplateAction } from '@backstage/plugin-scaffolder-node'; import { ConfigReader } from '@backstage/config'; import { ScmIntegrations } from '@backstage/integration'; -import { CatalogClient } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; const mockOctokit = { rest: { @@ -42,7 +42,7 @@ const mockOctokit = { }, }; -const mockCatalogClient: Partial = { +const mockCatalogClient: Partial = { getEntitiesByRefs: jest.fn(), }; @@ -121,7 +121,7 @@ describe('github:environment:create', () => { action = createGithubEnvironmentAction({ integrations, - catalog: mockCatalogClient as unknown as CatalogClient, + catalogClient: mockCatalogClient as CatalogApi, }); }); @@ -449,7 +449,7 @@ describe('github:environment:create', () => { ...mockContext, input: { ...mockContext.input, - reviewers: ['group:defautl/team-a', 'user:defautl/johndoe'], + reviewers: ['group:default/team-a', 'user:default/johndoe'], }, }); diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts index b482503717..d121c32872 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts @@ -24,7 +24,7 @@ import { getOctokitOptions } from './helpers'; import { Octokit } from 'octokit'; import Sodium from 'libsodium-wrappers'; import { examples } from './gitHubEnvironment.examples'; -import { CatalogClient } from '@backstage/catalog-client'; +import { CatalogApi } from '@backstage/catalog-client'; /** * Creates an `github:environment:create` Scaffolder action that creates a Github Environment. @@ -33,9 +33,9 @@ import { CatalogClient } from '@backstage/catalog-client'; */ export function createGithubEnvironmentAction(options: { integrations: ScmIntegrationRegistry; - catalog: CatalogClient; + catalogClient: CatalogApi; }) { - const { integrations, catalog } = options; + const { integrations, catalogClient } = options; // For more information on how to define custom actions, see // https://backstage.io/docs/features/software-templates/writing-custom-actions return createTemplateAction<{ @@ -188,9 +188,10 @@ export function createGithubEnvironmentAction(options: { const githubReviewers: { type: 'User' | 'Team'; id: number }[] = []; if (reviewers) { // Fetch reviewers from Catalog - const { items: reviewersEntityRefs } = await catalog.getEntitiesByRefs({ - entityRefs: reviewers, - }); + const { items: reviewersEntityRefs } = + await catalogClient.getEntitiesByRefs({ + entityRefs: reviewers, + }); for (const reviewerEntityRef of reviewersEntityRefs) { if (reviewerEntityRef?.kind === 'User') { try { diff --git a/plugins/scaffolder-backend-module-github/src/module.ts b/plugins/scaffolder-backend-module-github/src/module.ts index 4873cf445f..03835cf411 100644 --- a/plugins/scaffolder-backend-module-github/src/module.ts +++ b/plugins/scaffolder-backend-module-github/src/module.ts @@ -35,7 +35,7 @@ import { DefaultGithubCredentialsProvider, ScmIntegrations, } from '@backstage/integration'; -import { catalogServiceRef } from '@backstage/plugin-catalog-node'; +import { CatalogClient } from '@backstage/catalog-client'; /** * @public @@ -49,12 +49,15 @@ export const githubModule = createBackendModule({ deps: { scaffolder: scaffolderActionsExtensionPoint, config: coreServices.rootConfig, - catalog: catalogServiceRef, + discovery: coreServices.discovery, }, - async init({ scaffolder, config, catalog }) { + async init({ scaffolder, config, discovery }) { const integrations = ScmIntegrations.fromConfig(config); const githubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); + const catalogClient = new CatalogClient({ + discoveryApi: discovery, + }); scaffolder.addActions( createGithubActionsDispatchAction({ @@ -70,7 +73,7 @@ export const githubModule = createBackendModule({ }), createGithubEnvironmentAction({ integrations, - catalog, + catalogClient, }), createGithubIssuesLabelAction({ integrations, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index e2392150cc..3c09d91c2d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -241,6 +241,7 @@ export const createBuiltinActions = ( }), createGithubEnvironmentAction({ integrations, + catalogClient, }), createGithubDeployKeyAction({ integrations, From ef2b0da3cc728a958493026bb34876eda35441ef Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 01:39:19 +0200 Subject: [PATCH 07/18] Fix spacing in eslintrc Signed-off-by: Raghunandan Balachandran --- .eslintrc.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index caddf390f4..eb2a07b3b0 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -264,6 +264,6 @@ module.exports = { }, ], settings: { - 'testing-library/custom-queries': 'off', - }, + 'testing-library/custom-queries': 'off', + }, }; From c8b1b78a3f5699f2a7ada1c39f36b9f5120549c7 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 01:41:51 +0200 Subject: [PATCH 08/18] remove unused dep in scaffolde module github Signed-off-by: Raghunandan Balachandran --- plugins/scaffolder-backend-module-github/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index 2ad78b0000..e4fadf57de 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -48,7 +48,6 @@ "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", - "@backstage/plugin-catalog-node": "workspace:^", "@backstage/plugin-scaffolder-node": "workspace:^", "@octokit/webhooks": "^10.0.0", "libsodium-wrappers": "^0.7.11", From 79760810ba26855ebd075caadcdbafec7be489d8 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 01:46:26 +0200 Subject: [PATCH 09/18] add changeset Signed-off-by: Raghunandan Balachandran --- .changeset/tall-lamps-design.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/tall-lamps-design.md diff --git a/.changeset/tall-lamps-design.md b/.changeset/tall-lamps-design.md new file mode 100644 index 0000000000..e074ee284e --- /dev/null +++ b/.changeset/tall-lamps-design.md @@ -0,0 +1,11 @@ +--- +'@backstage/plugin-scaffolder-backend-module-github': patch +'@backstage/plugin-scaffolder-backend': patch +'@backstage/plugin-scaffolder': patch +--- + +'@backstage/plugin-scaffolder-backend-module-github' +Added support for all request parameters in the Github create/update environment API in the Github environment create scaffolder action. + +'@backstage/plugin-scaffolder' +Adds maxNoOfEntities optional parameter to the MultiEntityPicker. From 68a7072d45093c4f37cb30cdbe412fa62508afc9 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 01:50:42 +0200 Subject: [PATCH 10/18] yarn lock changes Signed-off-by: Raghunandan Balachandran --- yarn.lock | 1 - 1 file changed, 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 59a5d59adb..2919b58307 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7112,7 +7112,6 @@ __metadata: "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/integration": "workspace:^" - "@backstage/plugin-catalog-node": "workspace:^" "@backstage/plugin-scaffolder-node": "workspace:^" "@backstage/plugin-scaffolder-node-test-utils": "workspace:^" "@octokit/webhooks": ^10.0.0 From e3c0b4472e2dc76da7a777369431a6c06d9ad59e Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 02:15:13 +0200 Subject: [PATCH 11/18] Fix API reports Signed-off-by: Raghunandan Balachandran --- plugins/scaffolder-backend-module-github/api-report.md | 5 +++++ plugins/scaffolder/api-report.md | 1 + 2 files changed, 6 insertions(+) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index ec9cb2e31a..f145eba829 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -4,6 +4,7 @@ ```ts import { BackendFeature } from '@backstage/backend-plugin-api'; +import { CatalogApi } from '@backstage/catalog-client'; import { Config } from '@backstage/config'; import { createPullRequest } from 'octokit-plugin-create-pull-request'; import { GithubCredentialsProvider } from '@backstage/integration'; @@ -66,6 +67,7 @@ export function createGithubDeployKeyAction(options: { // @public export function createGithubEnvironmentAction(options: { integrations: ScmIntegrationRegistry; + catalogClient: CatalogApi; }): TemplateAction< { repoUrl: string; @@ -89,6 +91,9 @@ export function createGithubEnvironmentAction(options: { } | undefined; token?: string | undefined; + waitTimer?: number | undefined; + preventSelfReview?: boolean | undefined; + reviewers?: string[] | undefined; }, JsonObject >; diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index fb53761145..9dd32b9e68 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -235,6 +235,7 @@ export const MultiEntityPickerFieldExtension: FieldExtensionComponent_2< >[] | undefined; allowArbitraryValues?: boolean | undefined; + maxNoOfEntities?: number | undefined; } >; From 286647dd9dc9ea6f0151b655436ed7e778e00c65 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 15:31:44 +0200 Subject: [PATCH 12/18] Make catalog client optional in github env create action Signed-off-by: Raghunandan Balachandran --- .../api-report.md | 2 +- .../package.json | 1 + .../src/actions/githubEnvironment.ts | 19 ++++++++++++------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend-module-github/api-report.md b/plugins/scaffolder-backend-module-github/api-report.md index f145eba829..86964b7623 100644 --- a/plugins/scaffolder-backend-module-github/api-report.md +++ b/plugins/scaffolder-backend-module-github/api-report.md @@ -67,7 +67,7 @@ export function createGithubDeployKeyAction(options: { // @public export function createGithubEnvironmentAction(options: { integrations: ScmIntegrationRegistry; - catalogClient: CatalogApi; + catalogClient?: CatalogApi; }): TemplateAction< { repoUrl: string; diff --git a/plugins/scaffolder-backend-module-github/package.json b/plugins/scaffolder-backend-module-github/package.json index e4fadf57de..fad69f590c 100644 --- a/plugins/scaffolder-backend-module-github/package.json +++ b/plugins/scaffolder-backend-module-github/package.json @@ -45,6 +45,7 @@ "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", "@backstage/catalog-client": "workspace:^", + "@backstage/catalog-model": "workspace:^", "@backstage/config": "workspace:^", "@backstage/errors": "workspace:^", "@backstage/integration": "workspace:^", diff --git a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts index d121c32872..29c0a3ce38 100644 --- a/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts +++ b/plugins/scaffolder-backend-module-github/src/actions/githubEnvironment.ts @@ -25,6 +25,7 @@ import { Octokit } from 'octokit'; import Sodium from 'libsodium-wrappers'; import { examples } from './gitHubEnvironment.examples'; import { CatalogApi } from '@backstage/catalog-client'; +import { Entity } from '@backstage/catalog-model'; /** * Creates an `github:environment:create` Scaffolder action that creates a Github Environment. @@ -33,7 +34,7 @@ import { CatalogApi } from '@backstage/catalog-client'; */ export function createGithubEnvironmentAction(options: { integrations: ScmIntegrationRegistry; - catalogClient: CatalogApi; + catalogClient?: CatalogApi; }) { const { integrations, catalogClient } = options; // For more information on how to define custom actions, see @@ -187,11 +188,15 @@ export function createGithubEnvironmentAction(options: { // convert reviewers from catalog entity to Github user or team const githubReviewers: { type: 'User' | 'Team'; id: number }[] = []; if (reviewers) { + let reviewersEntityRefs: Array = []; // Fetch reviewers from Catalog - const { items: reviewersEntityRefs } = - await catalogClient.getEntitiesByRefs({ - entityRefs: reviewers, - }); + const catalogResponse = await catalogClient?.getEntitiesByRefs({ + entityRefs: reviewers, + }); + if (catalogResponse?.items?.length) { + reviewersEntityRefs = catalogResponse.items; + } + for (const reviewerEntityRef of reviewersEntityRefs) { if (reviewerEntityRef?.kind === 'User') { try { @@ -203,7 +208,7 @@ export function createGithubEnvironmentAction(options: { id: user.data.id, }); } catch (error) { - console.log('User not found:', error); + ctx.logger.error('User not found:', error); } } else if (reviewerEntityRef?.kind === 'Group') { try { @@ -216,7 +221,7 @@ export function createGithubEnvironmentAction(options: { id: team.data.id, }); } catch (error) { - console.log('Team not found:', error); + ctx.logger.error('Team not found:', error); } } } From cb951005be624b8d4dc8e065da84ec62be1ac394 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 15:38:59 +0200 Subject: [PATCH 13/18] changed tests to use user events to make sure the UI options are disabled when the maxNoOfEntities have been selected in the MultiEntityPicker Signed-off-by: Raghunandan Balachandran --- .../MultiEntityPicker.test.tsx | 91 ++++++++++++++----- 1 file changed, 69 insertions(+), 22 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx index 1ec7b56fa5..973afc2ad1 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx @@ -24,6 +24,7 @@ import { import { renderInTestApp, TestApiProvider } from '@backstage/test-utils'; import { fireEvent, screen } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; import React from 'react'; import { MultiEntityPicker } from './MultiEntityPicker'; import { MultiEntityPickerProps } from './schema'; @@ -716,7 +717,6 @@ describe('', () => { uiSchema = { 'ui:options': { - maxNoOfEntities: 2, catalogFilter: [ { kind: ['Group'], @@ -742,7 +742,11 @@ describe('', () => { catalogApi.getEntities.mockResolvedValue({ items: testEntities }); }); - it('User selects item', async () => { + it('limit the number of selected entities when maxNoOfEntities is specified', async () => { + const uiOptions = props.uiSchema['ui:options']; + if (uiOptions) { + uiOptions.maxNoOfEntities = 2; + } await renderInTestApp( @@ -752,31 +756,74 @@ describe('', () => { const input = screen.getByRole('textbox'); fireEvent.mouseDown(input); - const optionsBefore = screen.getAllByRole('option'); - - // Check that all options are enabled - optionsBefore.forEach(option => { - expect(option).toHaveAttribute('aria-disabled', 'false'); - }); + const optionA = screen.getByText('team-a'); + await userEvent.click(optionA as HTMLElement); fireEvent.mouseDown(input); - - // Select two options from the dropdown - fireEvent.change(input, { target: { value: 'team-a' } }); - fireEvent.blur(input); - - fireEvent.change(input, { target: { value: 'user-a' } }); - fireEvent.blur(input); - - expect(onChange).toHaveBeenCalledWith(['team-a']); + const optionB = screen.getByText('user-b'); + await userEvent.click(optionB as HTMLElement); fireEvent.mouseDown(input); - const optionsAfter = screen.getAllByRole('option'); + const optionC = screen.getByText('user-a'); + await expect(() => + userEvent.click(optionC as HTMLElement), + ).rejects.toThrow(/pointer-events: none/); - // Check that all options are disabled when macNoOfEntities is reached - optionsAfter.forEach(option => { - expect(option).toHaveAttribute('aria-disabled', 'true'); - }); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenNthCalledWith(1, ['group:default/team-a']); + expect(onChange).toHaveBeenNthCalledWith(2, [ + 'group:default/team-a', + 'user:default/user-b', + ]); + expect(onChange).not.toHaveBeenNthCalledWith(3, [ + 'group:default/team-a', + 'user:default/user-b', + 'user:default/user-a', + ]); + }); + + it('does not limit the number of selected entities when maxNoOfEntities is not specified', async () => { + await renderInTestApp( + + + , + ); + + const input = screen.getByRole('textbox'); + + fireEvent.mouseDown(input); + const optionA = screen.getByText('team-a'); + await userEvent.click(optionA as HTMLElement); + + fireEvent.mouseDown(input); + const optionB = screen.getByText('user-b'); + await userEvent.click(optionB as HTMLElement); + + fireEvent.mouseDown(input); + const optionC = screen.getByText('user-a'); + await userEvent.click(optionC as HTMLElement); + + fireEvent.mouseDown(input); + const optionD = screen.getByText('squad-b'); + await userEvent.click(optionD as HTMLElement); + + expect(onChange).toHaveBeenCalledTimes(4); + expect(onChange).toHaveBeenNthCalledWith(1, ['group:default/team-a']); + expect(onChange).toHaveBeenNthCalledWith(2, [ + 'group:default/team-a', + 'user:default/user-b', + ]); + expect(onChange).toHaveBeenNthCalledWith(3, [ + 'group:default/team-a', + 'user:default/user-b', + 'user:default/user-a', + ]); + expect(onChange).toHaveBeenNthCalledWith(4, [ + 'group:default/team-a', + 'user:default/user-b', + 'user:default/user-a', + 'group:default/squad-b', + ]); }); }); }); From aa3ed8486e079e8c2d6988f0efc40abee8c12565 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 17:58:45 +0200 Subject: [PATCH 14/18] Use JsonSchema maxItems instead of defining a maxNoOfEntities prop Signed-off-by: Raghunandan Balachandran --- .../fields/MultiEntityPicker/MultiEntityPicker.test.tsx | 8 +++----- .../fields/MultiEntityPicker/MultiEntityPicker.tsx | 6 +++--- .../src/components/fields/MultiEntityPicker/schema.ts | 4 ---- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx index 973afc2ad1..edd777652b 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.test.tsx @@ -743,10 +743,7 @@ describe('', () => { }); it('limit the number of selected entities when maxNoOfEntities is specified', async () => { - const uiOptions = props.uiSchema['ui:options']; - if (uiOptions) { - uiOptions.maxNoOfEntities = 2; - } + props.schema.maxItems = 2; await renderInTestApp( @@ -782,7 +779,8 @@ describe('', () => { ]); }); - it('does not limit the number of selected entities when maxNoOfEntities is not specified', async () => { + it('does not limit the number of selected entities when maxItems is not specified', async () => { + props.schema.maxItems = undefined; await renderInTestApp( diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx index f620c75d07..f514bfc2bb 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/MultiEntityPicker.tsx @@ -93,8 +93,8 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { const allowArbitraryValues = uiSchema['ui:options']?.allowArbitraryValues ?? true; - // if not specified, default to undefined - const maxNoOfEntities = uiSchema['ui:options']?.maxNoOfEntities ?? undefined; + // if not specified, maxItems defaults to undefined + const maxItems = props.schema.maxItems; const onSelect = useCallback( (_: any, refs: (string | Entity)[], reason: AutocompleteChangeReason) => { @@ -164,7 +164,7 @@ export const MultiEntityPicker = (props: MultiEntityPickerProps) => { ?.entityRef! } getOptionDisabled={_options => - maxNoOfEntities ? noOfItemsSelected >= maxNoOfEntities : false + maxItems ? noOfItemsSelected >= maxItems : false } autoSelect freeSolo={allowArbitraryValues} diff --git a/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts b/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts index 0c7989e8bf..2621157d02 100644 --- a/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts +++ b/plugins/scaffolder/src/components/fields/MultiEntityPicker/schema.ts @@ -47,10 +47,6 @@ export const MultiEntityPickerFieldSchema = makeFieldSchemaFromZod( .or(entityQueryFilterExpressionSchema) .optional() .describe('List of key-value filter expression for entities'), - maxNoOfEntities: z - .number() - .optional() - .describe('The maximum number of entities that can be selected'), }), ); From f7bcbf943004a1d077e3d86fa244787d7a2e741e Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 18:03:31 +0200 Subject: [PATCH 15/18] yarn lock changes Signed-off-by: Raghunandan Balachandran --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index 2919b58307..7c1328bbed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7108,6 +7108,7 @@ __metadata: "@backstage/backend-plugin-api": "workspace:^" "@backstage/backend-test-utils": "workspace:^" "@backstage/catalog-client": "workspace:^" + "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" "@backstage/errors": "workspace:^" From 9181797bb2c1f10a7e95041bc761335b7843b226 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 18:06:52 +0200 Subject: [PATCH 16/18] edit changeset Signed-off-by: Raghunandan Balachandran --- .changeset/tall-lamps-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/tall-lamps-design.md b/.changeset/tall-lamps-design.md index e074ee284e..bb38e7b392 100644 --- a/.changeset/tall-lamps-design.md +++ b/.changeset/tall-lamps-design.md @@ -8,4 +8,4 @@ Added support for all request parameters in the Github create/update environment API in the Github environment create scaffolder action. '@backstage/plugin-scaffolder' -Adds maxNoOfEntities optional parameter to the MultiEntityPicker. +Uses schema maxItems parameter when specified to disable the MultiEntityPicker when the maxItesm limit is reached. From e68ceed1fe0cea62ced256a37a429977942db3f4 Mon Sep 17 00:00:00 2001 From: Raghunandan Balachandran Date: Mon, 16 Sep 2024 19:06:54 +0200 Subject: [PATCH 17/18] regen api reports Signed-off-by: Raghunandan Balachandran --- plugins/scaffolder/api-report.md | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index 9dd32b9e68..fb53761145 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -235,7 +235,6 @@ export const MultiEntityPickerFieldExtension: FieldExtensionComponent_2< >[] | undefined; allowArbitraryValues?: boolean | undefined; - maxNoOfEntities?: number | undefined; } >; From a1feaf99a1a635497d72dfd6c1ea06ea7e35efc9 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Tue, 17 Sep 2024 08:47:42 +0200 Subject: [PATCH 18/18] Update tall-lamps-design.md Signed-off-by: Ben Lambert --- .changeset/tall-lamps-design.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.changeset/tall-lamps-design.md b/.changeset/tall-lamps-design.md index bb38e7b392..5b1a07e5de 100644 --- a/.changeset/tall-lamps-design.md +++ b/.changeset/tall-lamps-design.md @@ -4,8 +4,6 @@ '@backstage/plugin-scaffolder': patch --- -'@backstage/plugin-scaffolder-backend-module-github' Added support for all request parameters in the Github create/update environment API in the Github environment create scaffolder action. -'@backstage/plugin-scaffolder' -Uses schema maxItems parameter when specified to disable the MultiEntityPicker when the maxItesm limit is reached. +Disable MultiEntityPicker when `maxItems` limit is reached defined in `JSONSchema`