Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error if branch name of new pull request conflicts with an existing branch #1424

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions extension/tasks/dependabotV2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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(),
Expand All @@ -57,7 +58,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,
);

Expand Down Expand Up @@ -88,20 +95,26 @@ 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.
// Automatic discovery of vulnerable dependencies during a security-only update is not currently supported by dependabot-updater.
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(
Expand All @@ -112,26 +125,26 @@ 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(
taskInputs,
pullRequestId,
update,
dependabotConfig.registries,
existingPullRequestDependencies,
existingPullRequests[pullRequestId],
existingPullRequestDependenciesForPackageEcosystem,
existingPullRequestsForPackageEcosystem[pullRequestId],
),
dependabotUpdaterOptions,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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.
Expand All @@ -35,11 +36,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;
}

/**
Expand Down Expand Up @@ -95,7 +98,6 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess
return true;
}

// Create a new pull request
const changedFiles = getPullRequestChangedFilesForOutputData(data);
const dependencies = getPullRequestDependenciesPropertyValueForOutputData(data);
const targetBranch =
Expand All @@ -108,6 +110,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,
Expand Down