diff --git a/packages/common/src/pull-requests.test.ts b/packages/common/src/pull-requests.test.ts deleted file mode 100644 index 82c7ffb81..000000000 --- a/packages/common/src/pull-requests.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import type { Octokit } from 'octokit'; -import { generateBranchName, getExistingPullRequest } from './pull-requests'; - -/** - * Create a mocked version of the Octokit SDK that returns a given array of pull requests - */ -function mockOctokit(pulls: unknown[]) { - return { - // eslint-disable-next-line @typescript-eslint/no-unused-vars -- It's just a mock - paginate: (arg0: unknown, arg1: unknown) => Promise.resolve(pulls), - rest: { - pulls: { - list: () => {}, - }, - }, - } as Octokit; -} - -describe('getPullRequest', () => { - const featureBranch = { - head: { - ref: 'feature-branch', - }, - user: { - login: 'some-user', - type: 'User', - }, - }; - - const dependabotBranch = { - head: { - ref: 'integrate-dependabot-abcd', - }, - user: { - login: 'gu-dependency-graph-integrator[bot]', - type: 'Bot', - }, - }; - - const dependabotBranch2 = { - ...dependabotBranch, - head: { - ref: 'integrate-dependabot-efgh', - }, - }; - - it('should return undefined when no matching branch found', async () => { - const pulls = [featureBranch]; - const foundPull = await getExistingPullRequest( - mockOctokit(pulls), - 'repo', - 'owner', - 'gu-dependency-graph-integrator[bot]', - ); - expect(foundPull).toBeUndefined(); - }); - - it('should return pull request when author matches', async () => { - const pulls = [featureBranch, dependabotBranch]; - const foundPull = await getExistingPullRequest( - mockOctokit(pulls), - 'repo', - 'owner', - 'gu-dependency-graph-integrator[bot]', - ); - expect(foundPull).toEqual(dependabotBranch); - }); - - it('should return first pull request that matches and log warning', async () => { - const warn = jest.spyOn(console, 'warn'); - const pulls = [featureBranch, dependabotBranch, dependabotBranch2]; - const foundPull = await getExistingPullRequest( - mockOctokit(pulls), - 'repo', - 'owner', - 'gu-dependency-graph-integrator[bot]', - ); - expect(foundPull).toEqual(dependabotBranch); - expect(warn).toHaveBeenCalledWith( - 'More than one PR found on repo - choosing the first.', - ); - warn.mockRestore(); - }); -}); - -describe('generateBranchName', () => { - it('does not produce the same branch name twice', () => { - const prefix = 'hello'; - const branch1 = generateBranchName(prefix); - const branch2 = generateBranchName(prefix); - expect(branch1).not.toEqual(branch2); - }); -}); diff --git a/packages/dependency-graph-integrator/src/index.ts b/packages/dependency-graph-integrator/src/index.ts index 5ae57a57e..27c358be4 100644 --- a/packages/dependency-graph-integrator/src/index.ts +++ b/packages/dependency-graph-integrator/src/index.ts @@ -1,9 +1,5 @@ import type { SNSHandler } from 'aws-lambda'; import { parseEvent, stageAwareOctokit } from 'common/functions'; -import { - createPrAndAddToProject, - generateBranchName, -} from 'common/src/pull-requests'; import type { DependencyGraphIntegratorEvent } from 'common/src/types'; import type { Config } from './config'; import { getConfig } from './config'; @@ -12,6 +8,7 @@ import { depGraphPackageManager, generatePrBody, } from './file-generator'; +import { createPrAndAddToProject, generateBranchName } from './pull-requests'; import { enableDependabotAlerts } from './repo-functions'; import type { StatusCode } from './types'; diff --git a/packages/dependency-graph-integrator/src/pull-requests.test.ts b/packages/dependency-graph-integrator/src/pull-requests.test.ts new file mode 100644 index 000000000..d0aa386b2 --- /dev/null +++ b/packages/dependency-graph-integrator/src/pull-requests.test.ts @@ -0,0 +1,10 @@ +import { generateBranchName } from './pull-requests'; + +describe('generateBranchName', () => { + it('does not produce the same branch name twice', () => { + const prefix = 'hello'; + const branch1 = generateBranchName(prefix); + const branch2 = generateBranchName(prefix); + expect(branch1).not.toEqual(branch2); + }); +}); diff --git a/packages/common/src/pull-requests.ts b/packages/dependency-graph-integrator/src/pull-requests.ts similarity index 59% rename from packages/common/src/pull-requests.ts rename to packages/dependency-graph-integrator/src/pull-requests.ts index 1ab023a91..da6085fec 100644 --- a/packages/common/src/pull-requests.ts +++ b/packages/dependency-graph-integrator/src/pull-requests.ts @@ -1,9 +1,8 @@ import { randomBytes } from 'crypto'; -import type { Endpoints } from '@octokit/types'; +import { stageAwareOctokit } from 'common/src/functions'; +import { addPrToProject } from 'common/src/projects-graphql'; import type { Octokit } from 'octokit'; import { composeCreatePullRequest } from 'octokit-plugin-create-pull-request'; -import { stageAwareOctokit } from './functions'; -import { addPrToProject } from './projects-graphql'; interface Change { commitMessage: string; @@ -91,37 +90,6 @@ export async function requestTeamReview( } } -type PullRequestParameters = - Endpoints['GET /repos/{owner}/{repo}/pulls']['parameters']; - -type PullRequest = - Endpoints['GET /repos/{owner}/{repo}/pulls']['response']['data'][number]; - -function isGithubAuthor(pull: PullRequest, author: string) { - return pull.user?.login === author && pull.user.type === 'Bot'; -} - -export async function getExistingPullRequest( - octokit: Octokit, - repoName: string, - owner: string, - author: string, -) { - const pulls = await octokit.paginate(octokit.rest.pulls.list, { - owner, - repo: repoName, - state: 'open', - } satisfies PullRequestParameters); - - const found = pulls.filter((pull) => isGithubAuthor(pull, author)); - - if (found.length > 1) { - console.warn(`More than one PR found on ${repoName} - choosing the first.`); - } - - return found[0]; -} - export async function createPrAndAddToProject( stage: string, repoName: string, @@ -139,53 +107,40 @@ export async function createPrAndAddToProject( ) { if (stage === 'PROD') { const ghClient = octokit ?? (await stageAwareOctokit(stage)); - const existingPullRequest = await getExistingPullRequest( - ghClient, + + const pullRequestResponse = await createPullRequest(ghClient, { repoName, owner, - `${author}[bot]`, - ); + title: prTitle, + body: prBody, + branchName: branch, + changes: [ + { + commitMessage, + files: { + [fileName]: fileContents, + }, + }, + ], + admins, + }); + + if (pullRequestResponse?.html_url && pullRequestResponse.number) { + console.log( + 'Pull request successfully created:', + pullRequestResponse.html_url, + ); - if (!existingPullRequest) { - const pullRequestResponse = await createPullRequest(ghClient, { + await requestTeamReview( + ghClient, repoName, owner, - title: prTitle, - body: prBody, - branchName: branch, - changes: [ - { - commitMessage, - files: { - [fileName]: fileContents, - }, - }, - ], + pullRequestResponse.number, admins, - }); - - if (pullRequestResponse?.html_url && pullRequestResponse.number) { - console.log( - 'Pull request successfully created:', - pullRequestResponse.html_url, - ); - - await requestTeamReview( - ghClient, - repoName, - owner, - pullRequestResponse.number, - admins, - ); - - await addPrToProject(stage, repoName, boardNumber, author); - console.log('Updated project board'); - } - } else { - console.log( - `Existing pull request found. Skipping creating a new one.`, - existingPullRequest.html_url, ); + + await addPrToProject(stage, repoName, boardNumber, author); + console.log('Updated project board'); } } else { console.log(`Testing generation of ${fileName} for ${repoName}`); diff --git a/packages/repocop/src/index.ts b/packages/repocop/src/index.ts index 80c902283..4cda3c3ae 100644 --- a/packages/repocop/src/index.ts +++ b/packages/repocop/src/index.ts @@ -169,6 +169,7 @@ export async function main() { productionWorkflowUsages, repoOwners, dependencyGraphIntegratorRepoCount, + octokit, ); await writeEvaluationTable(repocopRules, prisma); diff --git a/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts b/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts index 62f6105a3..6a230e386 100644 --- a/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts +++ b/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.test.ts @@ -8,11 +8,13 @@ import type { Repository, RepositoryWithDepGraphLanguage, } from 'common/src/types'; +import type { Octokit } from 'octokit'; import { removeRepoOwner } from '../shared-utilities'; import { checkRepoForLanguage, createSnsEventsForDependencyGraphIntegration, doesRepoHaveDepSubmissionWorkflowForLanguage, + getExistingPullRequest, getReposWithoutWorkflows, } from './send-to-sns'; @@ -229,3 +231,85 @@ describe('When getting suitable events to send to SNS', () => { ]); }); }); + +/** + * Create a mocked version of the Octokit SDK that returns a given array of pull requests + */ +function mockOctokit(pulls: unknown[]) { + return { + // eslint-disable-next-line @typescript-eslint/no-unused-vars -- It's just a mock + paginate: (arg0: unknown, arg1: unknown) => Promise.resolve(pulls), + rest: { + pulls: { + list: () => {}, + }, + }, + } as Octokit; +} + +describe('getPullRequest', () => { + const featureBranch = { + head: { + ref: 'feature-branch', + }, + user: { + login: 'some-user', + type: 'User', + }, + }; + + const dependabotBranch = { + head: { + ref: 'integrate-dependabot-abcd', + }, + user: { + login: 'gu-dependency-graph-integrator[bot]', + type: 'Bot', + }, + }; + + const dependabotBranch2 = { + ...dependabotBranch, + head: { + ref: 'integrate-dependabot-efgh', + }, + }; + + it('should return undefined when no matching branch found', async () => { + const pulls = [featureBranch]; + const foundPull = await getExistingPullRequest( + mockOctokit(pulls), + 'repo', + 'owner', + 'gu-dependency-graph-integrator[bot]', + ); + expect(foundPull).toBeUndefined(); + }); + + it('should return pull request when author matches', async () => { + const pulls = [featureBranch, dependabotBranch]; + const foundPull = await getExistingPullRequest( + mockOctokit(pulls), + 'repo', + 'owner', + 'gu-dependency-graph-integrator[bot]', + ); + expect(foundPull).toEqual(dependabotBranch); + }); + + it('should return first pull request that matches and log warning', async () => { + const warn = jest.spyOn(console, 'warn'); + const pulls = [featureBranch, dependabotBranch, dependabotBranch2]; + const foundPull = await getExistingPullRequest( + mockOctokit(pulls), + 'repo', + 'owner', + 'gu-dependency-graph-integrator[bot]', + ); + expect(foundPull).toEqual(dependabotBranch); + expect(warn).toHaveBeenCalledWith( + 'More than one PR found on repo - choosing the first.', + ); + warn.mockRestore(); + }); +}); diff --git a/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.ts b/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.ts index 25d1c4295..dcfc25e02 100644 --- a/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.ts +++ b/packages/repocop/src/remediation/dependency_graph-integrator/send-to-sns.ts @@ -1,4 +1,5 @@ import { PublishCommand, SNSClient } from '@aws-sdk/client-sns'; +import type { Endpoints } from '@octokit/types'; import type { github_languages, guardian_github_actions_usage, @@ -12,6 +13,7 @@ import type { Repository, RepositoryWithDepGraphLanguage, } from 'common/src/types'; +import type { Octokit } from 'octokit'; import type { Config } from '../../config'; import { findContactableOwners, removeRepoOwner } from '../shared-utilities'; @@ -49,6 +51,36 @@ export function doesRepoHaveDepSubmissionWorkflowForLanguage( return false; } +type PullRequestParameters = + Endpoints['GET /repos/{owner}/{repo}/pulls']['parameters']; + +type PullRequest = + Endpoints['GET /repos/{owner}/{repo}/pulls']['response']['data'][number]; + +function isGithubAuthor(pull: PullRequest, author: string) { + return pull.user?.login === author && pull.user.type === 'Bot'; +} +export async function getExistingPullRequest( + octokit: Octokit, + repoName: string, + owner: string, + author: string, +) { + const pulls = await octokit.paginate(octokit.rest.pulls.list, { + owner, + repo: repoName, + state: 'open', + } satisfies PullRequestParameters); + + const found = pulls.filter((pull) => isGithubAuthor(pull, author)); + + if (found.length > 1) { + console.warn(`More than one PR found on ${repoName} - choosing the first.`); + } + + return found[0]; +} + export function createSnsEventsForDependencyGraphIntegration( reposWithoutWorkflows: RepositoryWithDepGraphLanguage[], repoOwnership: view_repo_ownership[], @@ -128,6 +160,7 @@ export async function sendReposToDependencyGraphIntegrator( productionWorkflowUsages: guardian_github_actions_usage[], repoOwners: view_repo_ownership[], repoCount: number, + octokit: Octokit, ): Promise { const reposRequiringDepGraphIntegration: RepositoryWithDepGraphLanguage[] = getReposWithoutWorkflows( @@ -141,10 +174,30 @@ export async function sendReposToDependencyGraphIntegrator( `Found ${reposRequiringDepGraphIntegration.length} repos requiring dependency graph integration`, ); - const selectedRepos = shuffle(reposRequiringDepGraphIntegration).slice( - 0, - repoCount, - ); + const shuffledRepos = shuffle(reposRequiringDepGraphIntegration); + + const selectedRepos: RepositoryWithDepGraphLanguage[] = []; + + while (selectedRepos.length < repoCount && shuffledRepos.length > 0) { + const repo = shuffledRepos.pop(); + if (repo) { + console.log('Checking for existing PR for', repo.name); + const existingPr = await getExistingPullRequest( + octokit, + repo.name, + 'guardian', + 'gu-dependency-graph-integrator[bot]', + ); + console.log( + existingPr + ? `Existing PR found for ${repo.name}` + : `PR not found for ${repo.name}`, + ); + if (!existingPr) { + selectedRepos.push(repo); + } + } + } const eventsToSend: DependencyGraphIntegratorEvent[] = createSnsEventsForDependencyGraphIntegration(selectedRepos, repoOwners); diff --git a/scripts/setup.sh b/scripts/setup.sh index 4e049c5ca..47eb971eb 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -95,7 +95,7 @@ setup_environment() { github_info_url="https://github.com/settings/tokens?type=beta" - token_text="# Required permissions are Metadata: Read, Administration: Read, Dependabot alerts: Read. See $github_info_url + token_text="# Required permissions are Metadata: Read, Administration: Read, Dependabot alerts: Read, Pull requests: Read. See $github_info_url GITHUB_ACCESS_TOKEN= "