From a55a34798922fdbd6fd9dab3ac4c4fe08782f2ac Mon Sep 17 00:00:00 2001 From: Rhys Koedijk <rhys@koedijk.co.nz> Date: Tue, 22 Oct 2024 20:39:00 +1300 Subject: [PATCH] Throw error if branch name of new pull request conflicts with existing branches (#1424) --- extension/tasks/dependabotV2/index.ts | 33 +++++++++++++------ .../azure-devops/AzureDevOpsWebApiClient.ts | 24 ++++++++++++++ .../DependabotOutputProcessor.ts | 22 ++++++++++++- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 581862ed..6df91cf1 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -65,7 +65,8 @@ async function run() { : null; // Fetch the active pull requests created by the author user - const prAuthorActivePullRequests = await prAuthorClient.getActivePullRequestProperties( + const existingBranchNames = await prAuthorClient.getBranchNames(taskInputs.project, taskInputs.repository); + const existingPullRequests = await prAuthorClient.getActivePullRequestProperties( taskInputs.project, taskInputs.repository, await prAuthorClient.getUserId(), @@ -74,7 +75,13 @@ async function run() { // Initialise the Dependabot updater dependabot = new DependabotCli( DependabotCli.CLI_IMAGE_LATEST, // TODO: Add config for this? - new DependabotOutputProcessor(taskInputs, prAuthorClient, prApproverClient, prAuthorActivePullRequests), + new DependabotOutputProcessor( + taskInputs, + prAuthorClient, + prApproverClient, + existingPullRequests, + existingBranchNames, + ), taskInputs.debug, ); @@ -105,6 +112,7 @@ async function run() { // Loop through the [targeted] update blocks in dependabot.yaml and perform updates for (const update of updates) { const updateId = updates.indexOf(update).toString(); + const packageEcosystem = update['package-ecosystem']; // Parse the last dependency list snapshot (if any) from the project properties. // This is required when doing a security-only update as dependabot requires the list of vulnerable dependencies to be updated. @@ -112,13 +120,18 @@ async function run() { const dependencyList = parseProjectDependencyListProperty( await prAuthorClient.getProjectProperties(taskInputs.projectId), taskInputs.repository, - update['package-ecosystem'], + packageEcosystem, ); // Parse the Dependabot metadata for the existing pull requests that are related to this update // Dependabot will use this to determine if we need to create new pull requests or update/close existing ones - const existingPullRequests = parsePullRequestProperties(prAuthorActivePullRequests, update['package-ecosystem']); - const existingPullRequestDependencies = Object.entries(existingPullRequests).map(([id, deps]) => deps); + const existingPullRequestsForPackageEcosystem = parsePullRequestProperties( + existingPullRequests, + packageEcosystem, + ); + const existingPullRequestDependenciesForPackageEcosystem = Object.entries( + existingPullRequestsForPackageEcosystem, + ).map(([id, deps]) => deps); // Run an update job for "all dependencies"; this will create new pull requests for dependencies that need updating failedTasks += handleUpdateOperationResults( @@ -129,17 +142,17 @@ async function run() { update, dependabotConfig.registries, dependencyList?.['dependencies'], - existingPullRequestDependencies, + existingPullRequestDependenciesForPackageEcosystem, ), dependabotUpdaterOptions, ), ); // If there are existing pull requests, run an update job for each one; this will resolve merge conflicts and close pull requests that are no longer needed - const numberOfPullRequestsToUpdate = Object.keys(existingPullRequests).length; + const numberOfPullRequestsToUpdate = Object.keys(existingPullRequestsForPackageEcosystem).length; if (numberOfPullRequestsToUpdate > 0) { if (!taskInputs.skipPullRequests) { - for (const pullRequestId in existingPullRequests) { + for (const pullRequestId in existingPullRequestsForPackageEcosystem) { failedTasks += handleUpdateOperationResults( await dependabot.update( DependabotJobBuilder.newUpdatePullRequestJob( @@ -147,8 +160,8 @@ async function run() { pullRequestId, update, dependabotConfig.registries, - existingPullRequestDependencies, - existingPullRequests[pullRequestId], + existingPullRequestDependenciesForPackageEcosystem, + existingPullRequestsForPackageEcosystem[pullRequestId], ), dependabotUpdaterOptions, ), diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 77313c9c..9f8880ec 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -97,6 +97,30 @@ export class AzureDevOpsWebApiClient { } } + /** + * Get the list of branch names for a repository. + * Requires scope "Code (Read)" (vso.code). + * @param project + * @param repository + * @returns + */ + public async getBranchNames(project: string, repository: string): Promise<string[]> { + try { + const refs = await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/refs`, + ); + if (!refs) { + throw new Error(`Repository '${project}/${repository}' not found`); + } + + return refs.value?.map((r) => normalizeBranchName(r.name)) || []; + } catch (e) { + error(`Failed to list branch names for '${project}/${repository}': ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + return undefined; + } + } + /** * Get the properties for all active pull request created by the supplied user. * Requires scope "Code (Read)" (vso.code). diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 0eba0eb6..5b97a563 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -17,6 +17,7 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess private readonly prAuthorClient: AzureDevOpsWebApiClient; private readonly prApproverClient: AzureDevOpsWebApiClient; private readonly existingPullRequests: IPullRequestProperties[]; + private readonly existingBranchNames: string[]; private readonly taskInputs: ISharedVariables; // Custom properties used to store dependabot metadata in projects. @@ -36,11 +37,13 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess prAuthorClient: AzureDevOpsWebApiClient, prApproverClient: AzureDevOpsWebApiClient, existingPullRequests: IPullRequestProperties[], + existingBranchNames: string[], ) { this.taskInputs = taskInputs; this.prAuthorClient = prAuthorClient; this.prApproverClient = prApproverClient; this.existingPullRequests = existingPullRequests; + this.existingBranchNames = existingBranchNames; } /** @@ -97,7 +100,6 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess return true; } - // Create a new pull request const changedFiles = getPullRequestChangedFilesForOutputData(data); const dependencies = getPullRequestDependenciesPropertyValueForOutputData(data); const targetBranch = @@ -110,6 +112,24 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess dependencies['dependencies'] || dependencies, update.config['pull-request-branch-name']?.separator, ); + + // Check if the source branch already exists or conflicts with an existing branch + const existingBranch = this.existingBranchNames?.find((branch) => sourceBranch == branch) || []; + if (existingBranch.length) { + error( + `Unable to create pull request as source branch '${sourceBranch}' already exists; Delete the existing branch and try again.`, + ); + return false; + } + const conflictingBranches = this.existingBranchNames?.filter((branch) => sourceBranch.startsWith(branch)) || []; + if (conflictingBranches.length) { + error( + `Unable to create pull request as source branch '${sourceBranch}' would conflict with existing branch(es) '${conflictingBranches.join(', ')}'; Delete the conflicting branch(es) and try again.`, + ); + return false; + } + + // Create a new pull request const newPullRequestId = await this.prAuthorClient.createPullRequest({ project: project, repository: repository,