From af35bd94441b16a8fbfe47061059b45f956ca8d0 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 30 Sep 2024 23:51:50 +1300 Subject: [PATCH 01/17] Add more logging --- .../azure-devops/AzureDevOpsWebApiClient.ts | 115 +++++++++++------- 1 file changed, 71 insertions(+), 44 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 8a4de107..9105db20 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -133,7 +133,38 @@ export class AzureDevOpsWebApiClient { const userId = await this.getUserId(); const git = await this.connection.getGitApi(); - // Create the source branch and commit the file changes + // Map the list of the pull request reviewer ids + // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. + // We treat assignees as required reviewers and all other reviewers as optional. + const allReviewers: IdentityRefWithVote[] = []; + if (pr.assignees?.length > 0) { + for (const assignee of pr.assignees) { + const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); + if (identityId) { + allReviewers.push({ + id: identityId, + isRequired: true, + isFlagged: true, + }); + } else { + warning(` ! Unable to resolve assignee identity '${assignee}'`); + } + } + } + if (pr.reviewers?.length > 0) { + for (const reviewer of pr.reviewers) { + const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); + if (identityId) { + allReviewers.push({ + id: identityId, + }); + } else { + warning(` ! Unable to resolve reviewer identity '${reviewer}'`); + } + } + } + + // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} change(s) to branch '${pr.source.branch}'...`); const push = await git.createPush( { @@ -165,37 +196,10 @@ export class AzureDevOpsWebApiClient { pr.repository, pr.project, ); - - // Build the list of the pull request reviewers - // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. - // We treat assignees as required reviewers and all other reviewers as optional. - const allReviewers: IdentityRefWithVote[] = []; - if (pr.assignees?.length > 0) { - for (const assignee of pr.assignees) { - const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); - if (identityId) { - allReviewers.push({ - id: identityId, - isRequired: true, - isFlagged: true, - }); - } else { - warning(` - Unable to resolve assignee identity '${assignee}'`); - } - } - } - if (pr.reviewers?.length > 0) { - for (const reviewer of pr.reviewers) { - const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); - if (identityId) { - allReviewers.push({ - id: identityId, - }); - } else { - warning(` - Unable to resolve reviewer identity '${reviewer}'`); - } - } + if (!push?.commits?.length) { + throw new Error('Failed to push changes to source branch, no commits were created'); } + console.log(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); @@ -218,11 +222,15 @@ export class AzureDevOpsWebApiClient { pr.project, true, ); + if (!pullRequest?.pullRequestId) { + throw new Error('Failed to create pull request, no pull request id was returned'); + } + console.log(` - Created pull request: #${pullRequest.pullRequestId}.`); // Add the pull request properties if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); - await git.updatePullRequestProperties( + const newProperties = await git.updatePullRequestProperties( null, pr.properties.map((property) => { return { @@ -235,6 +243,9 @@ export class AzureDevOpsWebApiClient { pullRequest.pullRequestId, pr.project, ); + if (!newProperties?.count) { + throw new Error('Failed to add dependency metadata properties to pull request'); + } } // TODO: Upload the pull request description as a 'changes.md' file attachment? @@ -243,8 +254,8 @@ export class AzureDevOpsWebApiClient { // Set the pull request auto-complete status if (pr.autoComplete) { - console.info(` - Setting auto-complete...`); - await git.updatePullRequest( + console.info(` - Updating auto-complete options...`); + const updatedPullRequest = await git.updatePullRequest( { autoCompleteSetBy: { id: userId, @@ -261,9 +272,12 @@ export class AzureDevOpsWebApiClient { pullRequest.pullRequestId, pr.project, ); + if (!updatedPullRequest || updatedPullRequest.autoCompleteSetBy?.id !== userId) { + throw new Error('Failed to set auto-complete on pull request'); + } } - console.info(` - Pull request #${pullRequest.pullRequestId} was created successfully.`); + console.log(` - Pull request was created successfully.`); return pullRequest.pullRequestId; } catch (e) { error(`Failed to create pull request: ${e}`); @@ -346,8 +360,12 @@ export class AzureDevOpsWebApiClient { options.repository, options.project, ); + if (!push?.commits?.length) { + throw new Error('Failed to push changes to source branch, no commits were created'); + } + console.info(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); - console.info(` - Pull request #${options.pullRequestId} was updated successfully.`); + console.info(` - Pull request was updated successfully.`); return true; } catch (e) { error(`Failed to update pull request: ${e}`); @@ -373,7 +391,7 @@ export class AzureDevOpsWebApiClient { // Approve the pull request console.info(` - Creating reviewer vote on pull request...`); - await git.createPullRequestReviewer( + const userVote = await git.createPullRequestReviewer( { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected isReapprove: true, @@ -383,8 +401,11 @@ export class AzureDevOpsWebApiClient { userId, options.project, ); + if (userVote?.vote != 10) { + throw new Error('Failed to approve pull request, vote was not recorded'); + } - console.info(` - Pull request #${options.pullRequestId} was approved.`); + console.info(` - Pull request was approved successfully.`); } catch (e) { error(`Failed to approve pull request: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -411,8 +432,8 @@ export class AzureDevOpsWebApiClient { // Add a comment to the pull request, if supplied if (options.comment) { - console.info(` - Adding comment to pull request...`); - await git.createThread( + console.info(` - Adding abandonment reason comment to pull request...`); + const thread = await git.createThread( { status: CommentThreadStatus.Closed, comments: [ @@ -429,11 +450,14 @@ export class AzureDevOpsWebApiClient { options.pullRequestId, options.project, ); + if (!thread?.id) { + throw new Error('Failed to add comment to pull request, thread was not created'); + } } // Close the pull request console.info(` - Abandoning pull request...`); - const pullRequest = await git.updatePullRequest( + const abandonedPullRequest = await git.updatePullRequest( { status: PullRequestStatus.Abandoned, closedBy: { @@ -444,14 +468,17 @@ export class AzureDevOpsWebApiClient { options.pullRequestId, options.project, ); + if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { + throw new Error('Failed to close pull request, status was not updated'); + } // Delete the source branch if required if (options.deleteSourceBranch) { console.info(` - Deleting source branch...`); await git.updateRef( { - name: `refs/heads/${pullRequest.sourceRefName}`, - oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + name: `refs/heads/${abandonedPullRequest.sourceRefName}`, + oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, newObjectId: '0000000000000000000000000000000000000000', isLocked: false, }, @@ -461,7 +488,7 @@ export class AzureDevOpsWebApiClient { ); } - console.info(` - Pull request #${options.pullRequestId} was closed successfully.`); + console.info(` - Pull request was closed successfully.`); return true; } catch (e) { error(`Failed to close pull request: ${e}`); From 868b20ed77b421212097adcf6de606f535cf7dc3 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Tue, 1 Oct 2024 18:15:56 +1300 Subject: [PATCH 02/17] Clean up logging --- .../utils/azure-devops/AzureDevOpsWebApiClient.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 9105db20..aef49a2c 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -165,7 +165,7 @@ export class AzureDevOpsWebApiClient { } // Create the source branch and push a commit with the dependency file changes - console.info(` - Pushing ${pr.changes.length} change(s) to branch '${pr.source.branch}'...`); + console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); const push = await git.createPush( { refUpdates: [ @@ -199,7 +199,7 @@ export class AzureDevOpsWebApiClient { if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); } - console.log(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); + console.info(` - Pushed commit: ${push.commits.map((c) => c.commitId).join(', ')}.`); // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); @@ -225,7 +225,7 @@ export class AzureDevOpsWebApiClient { if (!pullRequest?.pullRequestId) { throw new Error('Failed to create pull request, no pull request id was returned'); } - console.log(` - Created pull request: #${pullRequest.pullRequestId}.`); + console.info(` - Created pull request: #${pullRequest.pullRequestId}.`); // Add the pull request properties if (pr.properties?.length > 0) { @@ -277,7 +277,7 @@ export class AzureDevOpsWebApiClient { } } - console.log(` - Pull request was created successfully.`); + console.info(` - Pull request was created successfully.`); return pullRequest.pullRequestId; } catch (e) { error(`Failed to create pull request: ${e}`); @@ -327,7 +327,7 @@ export class AzureDevOpsWebApiClient { } // Push changes to the source branch - console.info(` - Pushing ${options.changes.length} change(s) branch '${pullRequest.sourceRefName}'...`); + console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); const push = await git.createPush( { refUpdates: [ From 840d5cd1c4a6660c60a7dd501b5243956a451d13 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Wed, 2 Oct 2024 12:02:00 +1300 Subject: [PATCH 03/17] Use Rest API instead of TypedClient, fixes undefined responses in some environments --- .../azure-devops/AzureDevOpsWebApiClient.ts | 118 ++++++++++++------ 1 file changed, 80 insertions(+), 38 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index aef49a2c..bd567262 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -8,6 +8,7 @@ import { PullRequestStatus, } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { error, warning } from 'azure-pipelines-task-lib/task'; +import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; import { IFileChange } from './interfaces/IFileChange'; import { IPullRequest } from './interfaces/IPullRequest'; import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; @@ -88,21 +89,21 @@ export class AzureDevOpsWebApiClient { ): Promise { try { const git = await this.connection.getGitApi(); - const pullRequests = await git.getPullRequests( - repository, - { - creatorId: isGuid(creator) ? creator : await this.getUserId(creator), - status: PullRequestStatus.Active, - }, - project, - ); + const pullRequests = (await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests?searchCriteria.creatorId=${isGuid(creator) ? creator : await this.getUserId(creator)}&searchCriteria.status${PullRequestStatus.Active}&api-version=7.1`, + ))?.value || []; if (!pullRequests || pullRequests.length === 0) { return []; } return await Promise.all( pullRequests.map(async (pr) => { - const properties = (await git.getPullRequestProperties(repository, pr.pullRequestId, project))?.value || {}; + const properties = + ( + await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties?api-version=7.1`, + ) + )?.value || {}; return { id: pr.pullRequestId, properties: @@ -166,7 +167,8 @@ export class AzureDevOpsWebApiClient { // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); - const push = await git.createPush( + const push = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes?api-version=7.1`, { refUpdates: [ { @@ -193,8 +195,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - pr.repository, - pr.project, ); if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); @@ -203,7 +203,8 @@ export class AzureDevOpsWebApiClient { // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); - const pullRequest = await git.createPullRequest( + const pullRequest = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests?api-version=7.1`, { sourceRefName: `refs/heads/${pr.source.branch}`, targetRefName: `refs/heads/${pr.target.branch}`, @@ -218,9 +219,6 @@ export class AzureDevOpsWebApiClient { }), isDraft: false, // TODO: Add config for this? }, - pr.repository, - pr.project, - true, ); if (!pullRequest?.pullRequestId) { throw new Error('Failed to create pull request, no pull request id was returned'); @@ -230,8 +228,8 @@ export class AzureDevOpsWebApiClient { // Add the pull request properties if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); - const newProperties = await git.updatePullRequestProperties( - null, + const newProperties = await this.restApiPatch( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties?api-version=7.1`, pr.properties.map((property) => { return { op: 'add', @@ -239,9 +237,7 @@ export class AzureDevOpsWebApiClient { value: property.value, }; }), - pr.repository, - pullRequest.pullRequestId, - pr.project, + 'application/json-patch+json', ); if (!newProperties?.count) { throw new Error('Failed to add dependency metadata properties to pull request'); @@ -255,7 +251,8 @@ export class AzureDevOpsWebApiClient { // Set the pull request auto-complete status if (pr.autoComplete) { console.info(` - Updating auto-complete options...`); - const updatedPullRequest = await git.updatePullRequest( + const updatedPullRequest = await this.restApiPatch( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}?api-version=7.1`, { autoCompleteSetBy: { id: userId, @@ -268,9 +265,6 @@ export class AzureDevOpsWebApiClient { transitionWorkItems: false, }, }, - pr.repository, - pullRequest.pullRequestId, - pr.project, ); if (!updatedPullRequest || updatedPullRequest.autoCompleteSetBy?.id !== userId) { throw new Error('Failed to set auto-complete on pull request'); @@ -297,6 +291,7 @@ export class AzureDevOpsWebApiClient { pullRequestId: number; changes: IFileChange[]; skipIfCommitsFromUsersOtherThan?: string; + skipIfNotBehindTargetBranch?: boolean; skipIfNoConflicts?: boolean; }): Promise { console.info(`Updating pull request #${options.pullRequestId}...`); @@ -310,25 +305,35 @@ export class AzureDevOpsWebApiClient { throw new Error(`Pull request #${options.pullRequestId} not found`); } + // Skip if the pull request has been modified by another user + if (options.skipIfCommitsFromUsersOtherThan) { + const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); + if (commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan)) { + console.info(` - Skipping update as pull request has been modified by another user.`); + return true; + } + } + + // Skip if the source branch is not behind the target branch + if (options.skipIfNotBehindTargetBranch) { + // TODO: https://learn.microsoft.com/en-us/rest/api/azure/devops/git/stats/get?view=azure-devops-rest-7.1&tabs=HTTP#examples + //const stats = await this.restApiGet(...) + //if (stats.behindCount > 0) { + // console.info(` - Skipping update as source branch is not behind target branch.`); + // return true; + //} + } + // Skip if no merge conflicts if (options.skipIfNoConflicts && pullRequest.mergeStatus !== PullRequestAsyncStatus.Conflicts) { console.info(` - Skipping update as pull request has no merge conflicts.`); return true; } - // Skip if the pull request has been modified by another user - const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); - if ( - options.skipIfCommitsFromUsersOtherThan && - commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan) - ) { - console.info(` - Skipping update as pull request has been modified by another user.`); - return true; - } - // Push changes to the source branch console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); - const push = await git.createPush( + const push = await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes?api-version=7.1`, { refUpdates: [ { @@ -357,8 +362,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - options.repository, - options.project, ); if (!push?.commits?.length) { throw new Error('Failed to push changes to source branch, no commits were created'); @@ -546,6 +549,45 @@ export class AzureDevOpsWebApiClient { console.debug(e); // Dump the error stack trace to help with debugging } } + + private async restApiGet(url: string): Promise { + return await this.restApiRequest(await this.connection.rest.client.get(url, { Accept: 'application/json' })); + } + + private async restApiPost(url: string, data?: any): Promise { + return await this.restApiRequest( + await this.connection.rest.client.post(url, JSON.stringify(data), { 'Content-Type': 'application/json' }), + ); + } + + private async restApiPatch(url: string, data?: any, contentType?: string): Promise { + return await this.restApiRequest( + await this.connection.rest.client.patch(url, JSON.stringify(data), { + 'Content-Type': contentType || 'application/json', + }), + ); + } + + private async restApiRequest(response: IHttpClientResponse): Promise { + try { + const body = JSON.parse(await response.readBody()); + if (body?.errorCode !== undefined && body?.message) { + throw new Error(body.message); + } + return body; + } catch (error) { + var responseStatusCode = error?.response?.statusCode; + if (responseStatusCode === 404) { + return undefined; + } else if (responseStatusCode === 401) { + throw new Error(`No access token has been provided to access '${response.message.url}'`); + } else if (responseStatusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${response.message.url}'`); + } else { + throw error; + } + } + } } function normalizeDevOpsPath(path: string): string { From 587aea9c3dba7e8a07258f695e8f882b2c2087cd Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 01:44:13 +1300 Subject: [PATCH 04/17] Refactor DevOps client to use rest client instead of typed client --- .../azure-devops/AzureDevOpsWebApiClient.ts | 358 ++++++++++-------- .../resolveAzureDevOpsIdentities.ts | 161 -------- .../DependabotOutputProcessor.ts | 6 +- 3 files changed, 202 insertions(+), 323 deletions(-) delete mode 100644 extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 12f6df75..d9143134 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -12,62 +12,81 @@ import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; import { IFileChange } from './interfaces/IFileChange'; import { IPullRequest } from './interfaces/IPullRequest'; import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; -import { resolveAzureDevOpsIdentities } from './resolveAzureDevOpsIdentities'; /** * Wrapper for DevOps WebApi client with helper methods for easier management of dependabot pull requests */ export class AzureDevOpsWebApiClient { private readonly organisationApiUrl: string; + private readonly identityApiUrl: string; private readonly accessToken: string; private readonly connection: WebApi; - private cachedUserIds: Record; + private authenticatedUserId: string; + private resolvedUserIds: Record; + + public static API_VERSION = '7.1'; constructor(organisationApiUrl: string, accessToken: string) { this.organisationApiUrl = organisationApiUrl; + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl); this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); - this.cachedUserIds = {}; + this.resolvedUserIds = {}; } /** - * Get the identity of a user by email address. If no email is provided, the identity of the authenticated user is returned. - * @param email + * Get the identity of the authenticated user. * @returns */ - public async getUserId(email?: string): Promise { - // If no email is provided, resolve to the authenticated user - if (!email) { - this.cachedUserIds[this.accessToken] ||= (await this.connection.connect())?.authenticatedUser?.id || ''; - return this.cachedUserIds[this.accessToken]; - } + public async getUserId(): Promise { + this.authenticatedUserId ||= (await this.connection.connect()).authenticatedUser.id; + return this.authenticatedUserId; + } - // Otherwise, do a cached identity lookup of the supplied email address - // TODO: When azure-devops-node-api supports Graph API, use that instead of the REST API - else if (!this.cachedUserIds[email]) { - const identities = await resolveAzureDevOpsIdentities(new URL(this.organisationApiUrl), [email]); - identities.forEach((i) => (this.cachedUserIds[i.input] ||= i.id)); + /** + * Get the identity id from a user name, email, or group name. + * Requires scope "Identity (Read)" (vso.identity). + * @param nameEmailOrGroup + * @returns + */ + public async resolveIdentityId(nameEmailOrGroup?: string): Promise { + if (this.resolvedUserIds[nameEmailOrGroup]) { + return this.resolvedUserIds[nameEmailOrGroup]; + } + try { + const identities = await this.restApiGet(`${this.identityApiUrl}/_apis/identities`, { + searchFilter: 'General', + filterValue: nameEmailOrGroup, + queryMembership: 'None', + }); + if (!identities?.value || identities.value.length === 0) { + return undefined; + } + this.resolvedUserIds[nameEmailOrGroup] = identities.value.map((i) => i.id) || []; + return this.resolvedUserIds[nameEmailOrGroup]; + } catch (e) { + error(`Failed to resolve user id: ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + return undefined; } - - return this.cachedUserIds[email]; } /** - * Get the default branch for a repository + * Get the default branch for a repository. + * Requires scope "Code (Read)" (vso.code). * @param project * @param repository * @returns */ public async getDefaultBranch(project: string, repository: string): Promise { try { - const git = await this.connection.getGitApi(); - const repo = await git.getRepository(repository, project); + const repo = await this.restApiGet(`${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}`); + console.log(repo); if (!repo) { throw new Error(`Repository '${project}/${repository}' not found`); } - // Strip reference prefix from the branch name, the caller doesn't need to know this - return repo.defaultBranch?.replace(/^refs\/heads\//i, ''); + return normalizeBranchName(repo.defaultBranch); } catch (e) { error(`Failed to get default branch for '${project}/${repository}': ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -76,7 +95,8 @@ export class AzureDevOpsWebApiClient { } /** - * Get the properties for all active pull request created by the supplied user + * Get the properties for all active pull request created by the supplied user. + * Requires scope "Code (Read)" (vso.code). * @param project * @param repository * @param creator @@ -88,10 +108,16 @@ export class AzureDevOpsWebApiClient { creator: string, ): Promise { try { - const git = await this.connection.getGitApi(); - const pullRequests = (await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests?searchCriteria.creatorId=${isGuid(creator) ? creator : await this.getUserId(creator)}&searchCriteria.status${PullRequestStatus.Active}&api-version=7.1`, - ))?.value || []; + const pullRequests = + ( + await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, + { + 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), + 'searchCriteria.status': 'Active', + }, + ) + )?.value || []; if (!pullRequests || pullRequests.length === 0) { return []; } @@ -101,7 +127,7 @@ export class AzureDevOpsWebApiClient { const properties = ( await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties?api-version=7.1`, + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, ) )?.value || {}; return { @@ -124,7 +150,9 @@ export class AzureDevOpsWebApiClient { } /** - * Create a new pull request + * Create a new pull request. + * Requires scope "Code (Write)" (vso.code_write). + * Requires scope "Identity (Read)" (vso.identity), if assignees or reviewers are specified. * @param pr * @returns */ @@ -132,7 +160,6 @@ export class AzureDevOpsWebApiClient { console.info(`Creating pull request '${pr.title}'...`); try { const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); // Map the list of the pull request reviewer ids // NOTE: Azure DevOps does not have a concept of assignees, only reviewers. @@ -140,7 +167,7 @@ export class AzureDevOpsWebApiClient { const allReviewers: IdentityRefWithVote[] = []; if (pr.assignees?.length > 0) { for (const assignee of pr.assignees) { - const identityId = isGuid(assignee) ? assignee : await this.getUserId(assignee); + const identityId = isGuid(assignee) ? assignee : await this.resolveIdentityId(assignee); if (identityId) { allReviewers.push({ id: identityId, @@ -154,7 +181,7 @@ export class AzureDevOpsWebApiClient { } if (pr.reviewers?.length > 0) { for (const reviewer of pr.reviewers) { - const identityId = isGuid(reviewer) ? reviewer : await this.getUserId(reviewer); + const identityId = isGuid(reviewer) ? reviewer : await this.resolveIdentityId(reviewer); if (identityId) { allReviewers.push({ id: identityId, @@ -168,7 +195,7 @@ export class AzureDevOpsWebApiClient { // Create the source branch and push a commit with the dependency file changes console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pr.source.branch}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes`, { refUpdates: [ { @@ -184,7 +211,7 @@ export class AzureDevOpsWebApiClient { return { changeType: change.changeType, item: { - path: normalizeDevOpsPath(change.path), + path: normalizeFilePath(change.path), }, newContent: { content: Buffer.from(change.content, change.encoding).toString('base64'), @@ -204,7 +231,7 @@ export class AzureDevOpsWebApiClient { // Create the pull request console.info(` - Creating pull request to merge '${pr.source.branch}' into '${pr.target.branch}'...`); const pullRequest = await this.restApiPost( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests`, { sourceRefName: `refs/heads/${pr.source.branch}`, targetRefName: `refs/heads/${pr.target.branch}`, @@ -229,7 +256,7 @@ export class AzureDevOpsWebApiClient { if (pr.properties?.length > 0) { console.info(` - Adding dependency metadata to pull request properties...`); const newProperties = await this.restApiPatch( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}/properties`, pr.properties.map((property) => { return { op: 'add', @@ -252,7 +279,7 @@ export class AzureDevOpsWebApiClient { if (pr.autoComplete) { console.info(` - Updating auto-complete options...`); const updatedPullRequest = await this.restApiPatch( - `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}?api-version=7.1`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pullRequest.pullRequestId}`, { autoCompleteSetBy: { id: userId, @@ -281,7 +308,8 @@ export class AzureDevOpsWebApiClient { } /** - * Update a pull request + * Update a pull request. + * Requires scope "Code (Read & Write)" (vso.code, vso.code_write). * @param options * @returns */ @@ -290,25 +318,32 @@ export class AzureDevOpsWebApiClient { repository: string; pullRequestId: number; changes: IFileChange[]; - skipIfCommitsFromUsersOtherThan?: string; + skipIfDraft?: boolean; + skipIfCommitsFromAuthorsOtherThan?: string; skipIfNotBehindTargetBranch?: boolean; - skipIfNoConflicts?: boolean; }): Promise { console.info(`Updating pull request #${options.pullRequestId}...`); try { - const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); - // Get the pull request details - const pullRequest = await git.getPullRequest(options.repository, options.pullRequestId, options.project); + const pullRequest = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + ); if (!pullRequest) { throw new Error(`Pull request #${options.pullRequestId} not found`); } - // Skip if the pull request has been modified by another user - if (options.skipIfCommitsFromUsersOtherThan) { - const commits = await git.getPullRequestCommits(options.repository, options.pullRequestId, options.project); - if (commits.some((c) => c.author?.email !== options.skipIfCommitsFromUsersOtherThan)) { + // Skip if the pull request is a draft + if (options.skipIfDraft && pullRequest.isDraft) { + console.info(` - Skipping update as pull request is currently marked as a draft.`); + return true; + } + + // Skip if the pull request has been modified by another author + if (options.skipIfCommitsFromAuthorsOtherThan) { + const commits = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/commits`, + ); + if (commits?.value?.some((c) => c.author?.email !== options.skipIfCommitsFromAuthorsOtherThan)) { console.info(` - Skipping update as pull request has been modified by another user.`); return true; } @@ -316,24 +351,22 @@ export class AzureDevOpsWebApiClient { // Skip if the source branch is not behind the target branch if (options.skipIfNotBehindTargetBranch) { - // TODO: https://learn.microsoft.com/en-us/rest/api/azure/devops/git/stats/get?view=azure-devops-rest-7.1&tabs=HTTP#examples - //const stats = await this.restApiGet(...) - //if (stats.behindCount > 0) { - // console.info(` - Skipping update as source branch is not behind target branch.`); - // return true; - //} - } - - // Skip if no merge conflicts - if (options.skipIfNoConflicts && pullRequest.mergeStatus !== PullRequestAsyncStatus.Conflicts) { - console.info(` - Skipping update as pull request has no merge conflicts.`); - return true; + const stats = await this.restApiGet( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/stats/branches`, + { + name: normalizeBranchName(pullRequest.sourceRefName), + }, + ); + if (stats?.behindCount === 0) { + console.info(` - Skipping update as source branch is not behind target branch.`); + return true; + } } // Push changes to the source branch console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes?api-version=7.1`, + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes`, { refUpdates: [ { @@ -351,7 +384,7 @@ export class AzureDevOpsWebApiClient { return { changeType: change.changeType, item: { - path: normalizeDevOpsPath(change.path), + path: normalizeFilePath(change.path), }, newContent: { content: Buffer.from(change.content, change.encoding).toString('base64'), @@ -378,7 +411,8 @@ export class AzureDevOpsWebApiClient { } /** - * Approve a pull request + * Approve a pull request. + * Requires scope "Code (Write)" (vso.code_write). * @param options * @returns */ @@ -389,20 +423,15 @@ export class AzureDevOpsWebApiClient { }): Promise { console.info(`Approving pull request #${options.pullRequestId}...`); try { - const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); - // Approve the pull request console.info(` - Creating reviewer vote on pull request...`); - const userVote = await git.createPullRequestReviewer( + const userId = await this.getUserId(); + const userVote = await this.restApiPut( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/reviewers/${userId}`, { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected isReapprove: true, }, - options.repository, - options.pullRequestId, - userId, - options.project, ); if (userVote?.vote != 10) { throw new Error('Failed to approve pull request, vote was not recorded'); @@ -417,7 +446,8 @@ export class AzureDevOpsWebApiClient { } /** - * Close a pull request + * Close a pull request. + * Requires scope "Code (Write)" (vso.code_write). * @param options * @returns */ @@ -431,12 +461,12 @@ export class AzureDevOpsWebApiClient { console.info(`Closing pull request #${options.pullRequestId}...`); try { const userId = await this.getUserId(); - const git = await this.connection.getGitApi(); // Add a comment to the pull request, if supplied if (options.comment) { console.info(` - Adding abandonment reason comment to pull request...`); - const thread = await git.createThread( + const thread = await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/threads`, { status: CommentThreadStatus.Closed, comments: [ @@ -449,9 +479,6 @@ export class AzureDevOpsWebApiClient { }, ], }, - options.repository, - options.pullRequestId, - options.project, ); if (!thread?.id) { throw new Error('Failed to add comment to pull request, thread was not created'); @@ -460,16 +487,14 @@ export class AzureDevOpsWebApiClient { // Close the pull request console.info(` - Abandoning pull request...`); - const abandonedPullRequest = await git.updatePullRequest( + const abandonedPullRequest = await this.restApiPatch( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, { status: PullRequestStatus.Abandoned, closedBy: { id: userId, }, }, - options.repository, - options.pullRequestId, - options.project, ); if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { throw new Error('Failed to close pull request, status was not updated'); @@ -478,16 +503,14 @@ export class AzureDevOpsWebApiClient { // Delete the source branch if required if (options.deleteSourceBranch) { console.info(` - Deleting source branch...`); - await git.updateRef( + await this.restApiPost( + `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/refs`, { - name: `refs/heads/${abandonedPullRequest.sourceRefName}`, + name: abandonedPullRequest.sourceRefName, oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, newObjectId: '0000000000000000000000000000000000000000', isLocked: false, }, - options.repository, - '', - options.project, ); } @@ -500,97 +523,96 @@ export class AzureDevOpsWebApiClient { } } - /** - * Get project properties - * @param projectId - * @param valueBuilder - * @returns - */ - public async getProjectProperties(projectId: string): Promise | undefined> { - try { - const core = await this.connection.getCoreApi(); - const properties = await core.getProjectProperties(projectId); - return properties?.map((p) => ({ [p.name]: p.value }))?.reduce((a, b) => ({ ...a, ...b }), {}); - } catch (e) { - error(`Failed to get project properties: ${e}`); - console.debug(e); // Dump the error stack trace to help with debugging - return undefined; - } - } - - /** - * Update a project property - * @param project - * @param name - * @param valueBuilder - * @returns - */ - public async updateProjectProperty( - projectId: string, - name: string, - valueBuilder: (existingValue: string) => string, - ): Promise { - try { - // Get the existing project property value - const core = await this.connection.getCoreApi(); - const properties = await core.getProjectProperties(projectId); - const propertyValue = properties?.find((p) => p.name === name)?.value; - - // Update the project property - await core.setProjectProperties(undefined, projectId, [ - { - op: 'add', - path: '/' + name, - value: valueBuilder(propertyValue || ''), - }, - ]); - } catch (e) { - error(`Failed to update project property '${name}': ${e}`); - console.debug(e); // Dump the error stack trace to help with debugging - } + private async restApiGet( + url: string, + params?: Record, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const queryString = Object.keys(params || {}) + .map((key) => `${key}=${params[key]}`) + .join('&'); + const fullUrl = `${url}?api-version=${apiVersion}${queryString ? `&${queryString}` : ''}`; + return await this.restApiRequest('GET', url, () => + this.connection.rest.client.get(fullUrl, { + Accept: 'application/json', + }), + ); } - private async restApiGet(url: string): Promise { - return await this.restApiRequest(await this.connection.rest.client.get(url, { Accept: 'application/json' })); + private async restApiPost( + url: string, + data?: any, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('POST', url, () => + this.connection.rest.client.post(fullUrl, JSON.stringify(data), { + 'Content-Type': 'application/json', + }), + ); } - private async restApiPost(url: string, data?: any): Promise { - return await this.restApiRequest( - await this.connection.rest.client.post(url, JSON.stringify(data), { 'Content-Type': 'application/json' }), + private async restApiPut( + url: string, + data?: any, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('PUT', url, () => + this.connection.rest.client.put(fullUrl, JSON.stringify(data), { + 'Content-Type': 'application/json', + }), ); } - private async restApiPatch(url: string, data?: any, contentType?: string): Promise { - return await this.restApiRequest( - await this.connection.rest.client.patch(url, JSON.stringify(data), { + private async restApiPatch( + url: string, + data?: any, + contentType?: string, + apiVersion: string = AzureDevOpsWebApiClient.API_VERSION, + ): Promise { + const fullUrl = `${url}?api-version=${apiVersion}`; + return await this.restApiRequest('PATCH', url, () => + this.connection.rest.client.patch(fullUrl, JSON.stringify(data), { 'Content-Type': contentType || 'application/json', }), ); } - private async restApiRequest(response: IHttpClientResponse): Promise { + private async restApiRequest( + method: string, + url: string, + request: () => Promise, + ): Promise { + console.debug(`🌎 🠊 [${method}] ${url}`); + const response = await request(); + console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); + if (response.message.statusCode === 401) { + throw new Error(`No access token has been provided to access '${url}'`); + } + if (response.message.statusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${url}'`); + } + if (response.message.statusCode < 200 || response.message.statusCode > 299) { + throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); + } try { - const body = JSON.parse(await response.readBody()); - if (body?.errorCode !== undefined && body?.message) { - throw new Error(body.message); - } - return body; - } catch (error) { - var responseStatusCode = error?.response?.statusCode; - if (responseStatusCode === 404) { - return undefined; - } else if (responseStatusCode === 401) { - throw new Error(`No access token has been provided to access '${response.message.url}'`); - } else if (responseStatusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${response.message.url}'`); - } else { - throw error; + const responseBodyJson = JSON.parse(await response.readBody()); + if (responseBodyJson?.errorCode !== undefined && responseBodyJson?.message) { + // .NET API error response + throw new Error(responseBodyJson.message); } + + return responseBodyJson; + } catch (e) { + // JSON parsing failed, log the error and return undefined + console.debug(response.message); + throw new Error(`Failed to parse response body as JSON: ${e}`); } } } -function normalizeDevOpsPath(path: string): string { +function normalizeFilePath(path: string): string { // Convert backslashes to forward slashes, convert './' => '/' and ensure the path starts with a forward slash if it doesn't already, this is how DevOps paths are formatted return path .replace(/\\/g, '/') @@ -598,6 +620,11 @@ function normalizeDevOpsPath(path: string): string { .replace(/^([^/])/, '/$1'); } +function normalizeBranchName(branch: string): string { + // Strip the 'refs/heads/' prefix from the branch name, if present + return branch.replace(/^refs\/heads\//i, ''); +} + function mergeCommitMessage(id: number, title: string, description: string): string { // // The merge commit message should contain the PR number and title for tracking. @@ -625,3 +652,14 @@ function isGuid(guid: string): boolean { const regex = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/; return regex.test(guid); } + +function getIdentityApiUrl(organisationApiUrl: string): string { + const uri = new URL(organisationApiUrl); + const hostname = uri.hostname.toLowerCase(); + + // If the organisation is hosted on Azure DevOps, use the 'vssps.dev.azure.com' domain + if (hostname === 'dev.azure.com' || hostname.endsWith('.visualstudio.com')) { + uri.host = 'vssps.dev.azure.com'; + } + return uri.toString(); +} diff --git a/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts b/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts deleted file mode 100644 index 77e0a617..00000000 --- a/extension/tasks/dependabotV2/utils/azure-devops/resolveAzureDevOpsIdentities.ts +++ /dev/null @@ -1,161 +0,0 @@ -import axios from 'axios'; -import * as tl from 'azure-pipelines-task-lib/task'; -import extractOrganization from '../extractOrganization'; - -export interface IIdentity { - /** - * The identity id to use for PR reviewer or assignee Id. - */ - id: string; - /** - * Human readable Username. - */ - displayName?: string; - /** - * The provided input to use for searching an identity. - */ - input: string; -} - -/** - * Resolves the given input email addresses to an array of IIdentity information. - * It also handles non email input, which is assumed to be already an identity id - * to pass as reviewer id to an PR. - * - * @param organizationUrl - * @param inputs - * @returns - */ -export async function resolveAzureDevOpsIdentities(organizationUrl: URL, inputs: string[]): Promise { - const result: IIdentity[] = []; - - tl.debug(`Attempting to fetch configuration file via REST API ...`); - for (const input of inputs) { - if (input.indexOf('@') > 0) { - // input is email to look-up - const identityInfo = await querySubject(organizationUrl, input); - if (identityInfo) { - result.push(identityInfo); - } - } else { - // input is already identity id - result.push({ id: input, input: input }); - } - } - return result; -} - -/** - * Returns whether the extension is run in a hosted environment (as opposed to an on-premise environment). - * In Azure DevOps terms, hosted environment is also known as "Azure DevOps Services" and on-premise environment is known as - * "Team Foundation Server" or "Azure DevOps Server". - */ -export function isHostedAzureDevOps(uri: URL): boolean { - const hostname = uri.hostname.toLowerCase(); - return hostname === 'dev.azure.com' || hostname.endsWith('.visualstudio.com'); -} - -function decodeBase64(input: string): string { - return Buffer.from(input, 'base64').toString('utf8'); -} - -function encodeBase64(input: string): string { - return Buffer.from(input, 'utf8').toString('base64'); -} - -function isSuccessStatusCode(statusCode?: number): boolean { - return statusCode >= 200 && statusCode <= 299; -} - -async function querySubject(organizationUrl: URL, email: string): Promise { - if (isHostedAzureDevOps(organizationUrl)) { - const organization: string = extractOrganization(organizationUrl.toString()); - return await querySubjectHosted(organization, email); - } else { - return await querySubjectOnPrem(organizationUrl, email); - } -} - -/** - * Make the HTTP Request for an OnPrem Azure DevOps Server to resolve an email to an IIdentity - * @param organizationUrl - * @param email - * @returns - */ -async function querySubjectOnPrem(organizationUrl: URL, email: string): Promise { - const url = `${organizationUrl}_apis/identities?searchFilter=MailAddress&queryMembership=None&filterValue=${email}`; - tl.debug(`GET ${url}`); - try { - const response = await axios.get(url, { - headers: { - Authorization: `Basic ${encodeBase64('PAT:' + tl.getVariable('System.AccessToken'))}`, - Accept: 'application/json;api-version=5.0', - }, - }); - - if (isSuccessStatusCode(response.status)) { - return { - id: response.data.value[0]?.id, - displayName: response.data.value[0]?.providerDisplayName, - input: email, - }; - } - } catch (error) { - const responseStatusCode = error?.response?.status; - tl.debug(`HTTP Response Status: ${responseStatusCode}`); - if (responseStatusCode > 400 && responseStatusCode < 500) { - tl.debug(`Access token is ${tl.getVariable('System.AccessToken')?.length > 0 ? 'not' : ''} null or empty.`); - throw new Error(`The access token provided is empty or does not have permissions to access '${url}'`); - } else { - throw error; - } - } -} - -/** - * * Make the HTTP Request for a hosted Azure DevOps Service, to resolve an email to an IIdentity - * @param organization - * @param email - * @returns - */ -async function querySubjectHosted(organization: string, email: string): Promise { - // make HTTP request - const url = `https://vssps.dev.azure.com/${organization}/_apis/graph/subjectquery`; - tl.debug(`GET ${url}`); - try { - const response = await axios.post(url, { - headers: { - 'Authorization': `Basic ${encodeBase64('PAT:' + tl.getVariable('System.AccessToken'))}`, - 'Accept': 'application/json;api-version=6.0-preview.1', - 'Content-Type': 'application/json', - }, - data: { - query: email, - subjectKind: ['User'], - }, - }); - - tl.debug(`Got Http Response: ${response.status}`); - - if (!isSuccessStatusCode(response.status) || response.data.value.length === 0) { - throw new Error('Failed to resolve given email in organization'); - } - - const descriptor: string = response.data.value[0]?.descriptor || ''; - const id = decodeBase64(descriptor.substring(descriptor.indexOf('.') + 1)); - return { - id: id, - displayName: response.data.value[0]?.displayName, - input: email, - }; - } catch (error) { - const responseStatusCode = error?.response?.status; - tl.debug(`HTTP Response Status: ${responseStatusCode}`); - if (responseStatusCode > 400 && responseStatusCode < 500) { - tl.debug(`Access token is ${tl.getVariable('System.AccessToken')?.length > 0 ? 'not' : ''} null or empty.`); - throw new Error(`The access token provided is empty or does not have permissions to access '${url}'`); - } else { - throw error; - } - } -} diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 97ab9d00..dc8428b5 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -180,9 +180,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfCommitsFromUsersOtherThan: + skipIfDraft: true, // TODO: Add config for this? + // TODO: Add config for this? + skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNoConflicts: true, + skipIfNotBehindTargetBranch: true, // TODO: Add config for this? }); // Re-approve the pull request, if required From 9561273b7fe1cd155398a0a41e65331512e3909e Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:01:57 +1300 Subject: [PATCH 05/17] Clean up --- extension/tasks/dependabotV2/index.ts | 12 +----- .../DependabotOutputProcessor.ts | 39 ++----------------- 2 files changed, 5 insertions(+), 46 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index 94c4c87f..c52ad2d5 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -4,7 +4,6 @@ import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { DependabotOutputProcessor, - parseProjectDependencyListProperty, parsePullRequestProperties, } from './utils/dependabot-cli/DependabotOutputProcessor'; import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; @@ -84,15 +83,6 @@ async function run() { for (const update of updates) { const updateId = updates.indexOf(update).toString(); - // 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'], - ); - // 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']); @@ -104,7 +94,7 @@ async function run() { updateId, update, dependabotConfig.registries, - dependencyList?.['dependencies'], + undefined, // TODO: Implement this, required for security-only updates existingPullRequestDependencies, ); const allDependenciesUpdateOutputs = await dependabot.update(allDependenciesJob, dependabotUpdaterOptions); diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index dc8428b5..8fdd2fc8 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -18,10 +18,6 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess private readonly existingPullRequests: IPullRequestProperties[]; private readonly taskInputs: ISharedVariables; - // Custom properties used to store dependabot metadata in projects. - // https://learn.microsoft.com/en-us/rest/api/azure/devops/core/projects/set-project-properties - public static PROJECT_PROPERTY_NAME_DEPENDENCY_LIST = 'Dependabot.DependencyList'; - // Custom properties used to store dependabot metadata in pull requests. // https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-properties public static PR_PROPERTY_NAME_PACKAGE_MANAGER = 'Dependabot.PackageManager'; @@ -58,25 +54,9 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // See: https://github.com/dependabot/cli/blob/main/internal/model/update.go case 'update_dependency_list': - // Store the dependency list snapshot in project properties, if configured + // Store the dependency list snapshot, if configured if (this.taskInputs.storeDependencyList) { - console.info(`Storing the dependency list snapshot for project '${project}'...`); - await this.prAuthorClient.updateProjectProperty( - this.taskInputs.projectId, - DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST, - function (existingValue: string) { - const repoDependencyLists = JSON.parse(existingValue || '{}'); - repoDependencyLists[repository] = repoDependencyLists[repository] || {}; - repoDependencyLists[repository][update.job['package-manager']] = { - 'dependencies': data['dependencies'], - 'dependency-files': data['dependency_files'], - 'last-updated': new Date().toISOString(), - }; - - return JSON.stringify(repoDependencyLists); - }, - ); - console.info(`Dependency list snapshot was updated for project '${project}'`); + // TODO: Store the dependency list snapshot } return true; @@ -180,11 +160,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, // TODO: Add config for this? - // TODO: Add config for this? + skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, // TODO: Add config for this? + skipIfNotBehindTargetBranch: true, }); // Re-approve the pull request, if required @@ -286,16 +265,6 @@ export function buildPullRequestProperties(packageManager: string, dependencies: ]; } -export function parseProjectDependencyListProperty( - properties: Record, - repository: string, - packageManager: string, -): any { - const dependencyList = properties?.[DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST] || '{}'; - const repoDependencyLists = JSON.parse(dependencyList); - return repoDependencyLists[repository]?.[packageManager]; -} - export function parsePullRequestProperties( pullRequests: IPullRequestProperties[], packageManager: string | null, From 8012d22ccdc41f3ae81461e57f515b5b81d87a44 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:21:50 +1300 Subject: [PATCH 06/17] Revert "Clean up" This reverts commit 9561273b7fe1cd155398a0a41e65331512e3909e. --- extension/tasks/dependabotV2/index.ts | 12 +++++- .../DependabotOutputProcessor.ts | 39 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/extension/tasks/dependabotV2/index.ts b/extension/tasks/dependabotV2/index.ts index c52ad2d5..94c4c87f 100644 --- a/extension/tasks/dependabotV2/index.ts +++ b/extension/tasks/dependabotV2/index.ts @@ -4,6 +4,7 @@ import { DependabotCli } from './utils/dependabot-cli/DependabotCli'; import { DependabotJobBuilder } from './utils/dependabot-cli/DependabotJobBuilder'; import { DependabotOutputProcessor, + parseProjectDependencyListProperty, parsePullRequestProperties, } from './utils/dependabot-cli/DependabotOutputProcessor'; import { IDependabotUpdate } from './utils/dependabot/interfaces/IDependabotConfig'; @@ -83,6 +84,15 @@ async function run() { for (const update of updates) { const updateId = updates.indexOf(update).toString(); + // 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'], + ); + // 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']); @@ -94,7 +104,7 @@ async function run() { updateId, update, dependabotConfig.registries, - undefined, // TODO: Implement this, required for security-only updates + dependencyList?.['dependencies'], existingPullRequestDependencies, ); const allDependenciesUpdateOutputs = await dependabot.update(allDependenciesJob, dependabotUpdaterOptions); diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 8fdd2fc8..dc8428b5 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -18,6 +18,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess private readonly existingPullRequests: IPullRequestProperties[]; private readonly taskInputs: ISharedVariables; + // Custom properties used to store dependabot metadata in projects. + // https://learn.microsoft.com/en-us/rest/api/azure/devops/core/projects/set-project-properties + public static PROJECT_PROPERTY_NAME_DEPENDENCY_LIST = 'Dependabot.DependencyList'; + // Custom properties used to store dependabot metadata in pull requests. // https://learn.microsoft.com/en-us/rest/api/azure/devops/git/pull-request-properties public static PR_PROPERTY_NAME_PACKAGE_MANAGER = 'Dependabot.PackageManager'; @@ -54,9 +58,25 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // See: https://github.com/dependabot/cli/blob/main/internal/model/update.go case 'update_dependency_list': - // Store the dependency list snapshot, if configured + // Store the dependency list snapshot in project properties, if configured if (this.taskInputs.storeDependencyList) { - // TODO: Store the dependency list snapshot + console.info(`Storing the dependency list snapshot for project '${project}'...`); + await this.prAuthorClient.updateProjectProperty( + this.taskInputs.projectId, + DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST, + function (existingValue: string) { + const repoDependencyLists = JSON.parse(existingValue || '{}'); + repoDependencyLists[repository] = repoDependencyLists[repository] || {}; + repoDependencyLists[repository][update.job['package-manager']] = { + 'dependencies': data['dependencies'], + 'dependency-files': data['dependency_files'], + 'last-updated': new Date().toISOString(), + }; + + return JSON.stringify(repoDependencyLists); + }, + ); + console.info(`Dependency list snapshot was updated for project '${project}'`); } return true; @@ -160,10 +180,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, + skipIfDraft: true, // TODO: Add config for this? + // TODO: Add config for this? skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, + skipIfNotBehindTargetBranch: true, // TODO: Add config for this? }); // Re-approve the pull request, if required @@ -265,6 +286,16 @@ export function buildPullRequestProperties(packageManager: string, dependencies: ]; } +export function parseProjectDependencyListProperty( + properties: Record, + repository: string, + packageManager: string, +): any { + const dependencyList = properties?.[DependabotOutputProcessor.PROJECT_PROPERTY_NAME_DEPENDENCY_LIST] || '{}'; + const repoDependencyLists = JSON.parse(dependencyList); + return repoDependencyLists[repository]?.[packageManager]; +} + export function parsePullRequestProperties( pullRequests: IPullRequestProperties[], packageManager: string | null, From 37917b5931f222bdce0727d5382913e99f603aac Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 02:24:10 +1300 Subject: [PATCH 07/17] Revert --- .../azure-devops/AzureDevOpsWebApiClient.ts | 50 +++++++++++++++++++ .../DependabotOutputProcessor.ts | 5 +- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index d9143134..744e9bfd 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -523,6 +523,56 @@ export class AzureDevOpsWebApiClient { } } + /** + * Get project properties + * @param projectId + * @param valueBuilder + * @returns + */ + public async getProjectProperties(projectId: string): Promise | undefined> { + try { + const core = await this.connection.getCoreApi(); + const properties = await core.getProjectProperties(projectId); + return properties?.map((p) => ({ [p.name]: p.value }))?.reduce((a, b) => ({ ...a, ...b }), {}); + } catch (e) { + error(`Failed to get project properties: ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + return undefined; + } + } + + /** + * Update a project property + * @param project + * @param name + * @param valueBuilder + * @returns + */ + public async updateProjectProperty( + projectId: string, + name: string, + valueBuilder: (existingValue: string) => string, + ): Promise { + try { + // Get the existing project property value + const core = await this.connection.getCoreApi(); + const properties = await core.getProjectProperties(projectId); + const propertyValue = properties?.find((p) => p.name === name)?.value; + + // Update the project property + await core.setProjectProperties(undefined, projectId, [ + { + op: 'add', + path: '/' + name, + value: valueBuilder(propertyValue || ''), + }, + ]); + } catch (e) { + error(`Failed to update project property '${name}': ${e}`); + console.debug(e); // Dump the error stack trace to help with debugging + } + } + private async restApiGet( url: string, params?: Record, diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index dc8428b5..c1e81aff 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -180,11 +180,10 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess repository: repository, pullRequestId: pullRequestToUpdate.id, changes: getPullRequestChangedFilesForOutputData(data), - skipIfDraft: true, // TODO: Add config for this? - // TODO: Add config for this? + skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, - skipIfNotBehindTargetBranch: true, // TODO: Add config for this? + skipIfNotBehindTargetBranch: true, }); // Re-approve the pull request, if required From e9c950bd62f2e84a7a5d57d9ef2cdb65b514e703 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 10:33:55 +1300 Subject: [PATCH 08/17] Remove double forward slash in DevOps API urls --- .../utils/azure-devops/AzureDevOpsWebApiClient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 744e9bfd..0cf13d6f 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -27,8 +27,8 @@ export class AzureDevOpsWebApiClient { public static API_VERSION = '7.1'; constructor(organisationApiUrl: string, accessToken: string) { - this.organisationApiUrl = organisationApiUrl; - this.identityApiUrl = getIdentityApiUrl(organisationApiUrl); + this.organisationApiUrl = organisationApiUrl.replace(/\/$/, ''); // trim trailing slash + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, '');; // trim trailing slash this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); this.resolvedUserIds = {}; From d037409b54d5e3a8ef5f2242c6acf21bff0d01bb Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 10:35:36 +1300 Subject: [PATCH 09/17] Add more detailed error info when REST APIs fail --- .../azure-devops/AzureDevOpsWebApiClient.ts | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 0cf13d6f..bf94d6f5 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -28,7 +28,7 @@ export class AzureDevOpsWebApiClient { constructor(organisationApiUrl: string, accessToken: string) { this.organisationApiUrl = organisationApiUrl.replace(/\/$/, ''); // trim trailing slash - this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, '');; // trim trailing slash + this.identityApiUrl = getIdentityApiUrl(organisationApiUrl).replace(/\/$/, ''); // trim trailing slash this.accessToken = accessToken; this.connection = new WebApi(organisationApiUrl, getPersonalAccessTokenHandler(accessToken)); this.resolvedUserIds = {}; @@ -636,28 +636,24 @@ export class AzureDevOpsWebApiClient { ): Promise { console.debug(`🌎 🠊 [${method}] ${url}`); const response = await request(); + const body = await response.readBody(); console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); - if (response.message.statusCode === 401) { - throw new Error(`No access token has been provided to access '${url}'`); - } - if (response.message.statusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${url}'`); - } - if (response.message.statusCode < 200 || response.message.statusCode > 299) { - throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); - } try { - const responseBodyJson = JSON.parse(await response.readBody()); - if (responseBodyJson?.errorCode !== undefined && responseBodyJson?.message) { - // .NET API error response - throw new Error(responseBodyJson.message); + if (response.message.statusCode === 401) { + throw new Error(`No access token has been provided to access '${url}'`); } - - return responseBodyJson; + if (response.message.statusCode === 403) { + throw new Error(`The access token provided does not have permissions to access '${url}'`); + } + if (response.message.statusCode < 200 || response.message.statusCode > 299) { + throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); + } + return JSON.parse(body); } catch (e) { - // JSON parsing failed, log the error and return undefined - console.debug(response.message); - throw new Error(`Failed to parse response body as JSON: ${e}`); + if (body) { + console.debug(body); + } + throw e; } } } From 302e36a437726509fe0ea2ad834e005dfb8f30b3 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 11:29:45 +1300 Subject: [PATCH 10/17] Fix for identity id being mapped as an array instead of a string --- .../dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index bf94d6f5..07573639 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -62,7 +62,7 @@ export class AzureDevOpsWebApiClient { if (!identities?.value || identities.value.length === 0) { return undefined; } - this.resolvedUserIds[nameEmailOrGroup] = identities.value.map((i) => i.id) || []; + this.resolvedUserIds[nameEmailOrGroup] = identities.value[0]?.id; return this.resolvedUserIds[nameEmailOrGroup]; } catch (e) { error(`Failed to resolve user id: ${e}`); From 27586186b849694c75ffb5feb2fe9036c1328585 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:13:15 +1300 Subject: [PATCH 11/17] Add documentation for configuring assigness and reviewers --- README.md | 20 ++++++++++++++++++-- extension/README.md | 1 + 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index fc6962b8..b7ed31e3 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ In this repository you'll find: - [Configuring private feeds and registries](#configuring-private-feeds-and-registries) - [Configuring security advisories and known vulnerabilities](#configuring-security-advisories-and-known-vulnerabilities) - [Configuring experiments](#configuring-experiments) +- [Configuring assignees and reviewers](#configuring-assignees-and-reviewers) - [Unsupported features and configurations](#unsupported-features-and-configurations) * [Extension Task](#extension-task) + [dependabot@V2](#dependabotv2) @@ -174,11 +175,23 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u | NuGet | nuget_native_updater | true/false | https://github.com/dependabot/dependabot-core/pull/10521 | | NuGet | nuget_dependency_solver | true/false | https://github.com/dependabot/dependabot-core/pull/10343 | - - > [!TIP] > To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code). + + +## Configuring assignees and reviewers +Dependabot allows for the configuration of both [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers). However, Azure DevOps does not have the concept of pull request assignees. Because of this, `assignees` will be treated as **required** reviewers and `reviewers` will be treated as **optional** reviewers. + +Reviewers can be any of the following values: + +- User GUID +- User username +- User email address +- User full [display] name +- Group name +- Team name + ## Unsupported features and configurations We aim to support all [official configuration options](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file), but there are some limitations for: @@ -193,15 +206,18 @@ We aim to support all [official configuration options](https://docs.github.com/e - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are only supported if task input `useUpdateScriptVNext: true` is set. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are only supported if task input `useUpdateScriptVNext: true` is set. - [`ignore`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#ignore) may not behave to official specifications unless task input `useUpdateScriptVNext: true` is set. If you are having issues, search for related issues such as before creating a new issue. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addressess; group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. Support is _slightly_ improved when task input `useUpdateScriptVNext: true` is set, but not still not fully supported. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Updater Docker image +- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addressess and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Server - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are not supported. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addressess and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ## Migration Guide diff --git a/extension/README.md b/extension/README.md index abf31027..06493650 100644 --- a/extension/README.md +++ b/extension/README.md @@ -105,5 +105,6 @@ Dependabot uses Docker containers, which may take time to install if not already - [Configuring private feeds and registries](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-private-feeds-and-registries) - [Configuring security advisories and known vulnerabilities](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-security-advisories-and-known-vulnerabilities) - [Configuring experiments](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-experiments) +- [Configuring assignees and reviewers](https://github.com/tinglesoftware/dependabot-azure-devops/#configuring-assignees-and-reviewers) - [Unsupported features and configurations](https://github.com/tinglesoftware/dependabot-azure-devops/#unsupported-features-and-configurations) - [Task migration guide for V1 → V2](https://github.com/tinglesoftware/dependabot-azure-devops/blob/main/docs/migrations/v1-to-v2.md) From a3e87ddae77b7c52dc93285e69393ea3f108f546 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:15:56 +1300 Subject: [PATCH 12/17] Fix typos --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b7ed31e3..5bc37eac 100644 --- a/README.md +++ b/README.md @@ -206,18 +206,18 @@ We aim to support all [official configuration options](https://docs.github.com/e - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are only supported if task input `useUpdateScriptVNext: true` is set. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are only supported if task input `useUpdateScriptVNext: true` is set. - [`ignore`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#ignore) may not behave to official specifications unless task input `useUpdateScriptVNext: true` is set. If you are having issues, search for related issues such as before creating a new issue. -- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addressess; group/team names are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids or email addresses; group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. Support is _slightly_ improved when task input `useUpdateScriptVNext: true` is set, but not still not fully supported. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Updater Docker image -- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addressess and group/team names are not supported. +- `DEPENDABOT_ASSIGNEES` and `DEPENDABOT_REVIEWERS` must be a list of user guids; email addresses and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ### Server - [`directories`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#directories) are not supported. - [`groups`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups) are not supported. -- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addressess and group/team names are not supported. +- [`assignees`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#assignees) and [`reviewers`](https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#reviewers) must be a list of user guids; email addresses and group/team names are not supported. - Private feed/registry authentication may not work with all package ecyosystems. See [problems with authentication](https://github.com/tinglesoftware/dependabot-azure-devops/discussions/1317) for more. ## Migration Guide From 00248ce59482091a45810b33cb460f3029f0a2e7 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:23:09 +1300 Subject: [PATCH 13/17] Alerts cannot be embeeded inside
--- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 5bc37eac..cf2c746e 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,6 @@ Experiments vary depending on the package ecyosystem used; They can be enabled u | NuGet | nuget_native_updater | true/false | https://github.com/dependabot/dependabot-core/pull/10521 | | NuGet | nuget_dependency_solver | true/false | https://github.com/dependabot/dependabot-core/pull/10343 | -> [!TIP] > To find the latest list of Dependabot experiments, search the `dependabot-core` GitHub repository using queries like ["enabled?(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Fenabled%5CW%5C%28.*%5C%29%2F&type=code) and ["options.fetch(x)"](https://github.com/search?q=repo%3Adependabot%2Fdependabot-core+%2Foptions%5C.fetch%5C%28.*%2C%2F&type=code).
From d86e16b9e1c3ffab5a1486a91a20743172f80109 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 12:32:15 +1300 Subject: [PATCH 14/17] Clean up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 07573639..f7229f62 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -46,24 +46,24 @@ export class AzureDevOpsWebApiClient { /** * Get the identity id from a user name, email, or group name. * Requires scope "Identity (Read)" (vso.identity). - * @param nameEmailOrGroup + * @param userNameEmailOrGroupName * @returns */ - public async resolveIdentityId(nameEmailOrGroup?: string): Promise { - if (this.resolvedUserIds[nameEmailOrGroup]) { - return this.resolvedUserIds[nameEmailOrGroup]; + public async resolveIdentityId(userNameEmailOrGroupName?: string): Promise { + if (this.resolvedUserIds[userNameEmailOrGroupName]) { + return this.resolvedUserIds[userNameEmailOrGroupName]; } try { const identities = await this.restApiGet(`${this.identityApiUrl}/_apis/identities`, { searchFilter: 'General', - filterValue: nameEmailOrGroup, + filterValue: userNameEmailOrGroupName, queryMembership: 'None', }); if (!identities?.value || identities.value.length === 0) { return undefined; } - this.resolvedUserIds[nameEmailOrGroup] = identities.value[0]?.id; - return this.resolvedUserIds[nameEmailOrGroup]; + this.resolvedUserIds[userNameEmailOrGroupName] = identities.value[0]?.id; + return this.resolvedUserIds[userNameEmailOrGroupName]; } catch (e) { error(`Failed to resolve user id: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging @@ -81,7 +81,6 @@ export class AzureDevOpsWebApiClient { public async getDefaultBranch(project: string, repository: string): Promise { try { const repo = await this.restApiGet(`${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}`); - console.log(repo); if (!repo) { throw new Error(`Repository '${project}/${repository}' not found`); } @@ -108,32 +107,26 @@ export class AzureDevOpsWebApiClient { creator: string, ): Promise { try { - const pullRequests = - ( - await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, - { - 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), - 'searchCriteria.status': 'Active', - }, - ) - )?.value || []; - if (!pullRequests || pullRequests.length === 0) { + const pullRequests = await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests`, + { + 'searchCriteria.creatorId': isGuid(creator) ? creator : await this.getUserId(), + 'searchCriteria.status': 'Active', + }, + ); + if (!pullRequests?.value || pullRequests.value.length === 0) { return []; } return await Promise.all( - pullRequests.map(async (pr) => { - const properties = - ( - await this.restApiGet( - `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, - ) - )?.value || {}; + pullRequests.value.map(async (pr) => { + const properties = await this.restApiGet( + `${this.organisationApiUrl}/${project}/_apis/git/repositories/${repository}/pullrequests/${pr.pullRequestId}/properties`, + ); return { id: pr.pullRequestId, properties: - Object.keys(properties).map((key) => { + Object.keys(properties?.value || {}).map((key) => { return { name: key, value: properties[key]?.$value, From 0d51fb7cec8ccec4dd43c6c46ab8c90e27bcef4f Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 14:41:34 +1300 Subject: [PATCH 15/17] Tidy up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 152 +++++++++--------- .../azure-devops/interfaces/IPullRequest.ts | 53 +++++- .../interfaces/IPullRequestProperties.ts | 10 -- .../DependabotOutputProcessor.ts | 9 +- 4 files changed, 137 insertions(+), 87 deletions(-) delete mode 100644 extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index f7229f62..6dd47487 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -9,9 +9,13 @@ import { } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { error, warning } from 'azure-pipelines-task-lib/task'; import { IHttpClientResponse } from 'typed-rest-client/Interfaces'; -import { IFileChange } from './interfaces/IFileChange'; -import { IPullRequest } from './interfaces/IPullRequest'; -import { IPullRequestProperties } from './interfaces/IPullRequestProperties'; +import { + IAbandonPullRequest, + IApprovePullRequest, + ICreatePullRequest, + IPullRequestProperties, + IUpdatePullRequest, +} from './interfaces/IPullRequest'; /** * Wrapper for DevOps WebApi client with helper methods for easier management of dependabot pull requests @@ -129,7 +133,7 @@ export class AzureDevOpsWebApiClient { Object.keys(properties?.value || {}).map((key) => { return { name: key, - value: properties[key]?.$value, + value: properties.value[key]?.$value, }; }) || [], }; @@ -149,7 +153,7 @@ export class AzureDevOpsWebApiClient { * @param pr * @returns */ - public async createPullRequest(pr: IPullRequest): Promise { + public async createPullRequest(pr: ICreatePullRequest): Promise { console.info(`Creating pull request '${pr.title}'...`); try { const userId = await this.getUserId(); @@ -303,68 +307,85 @@ export class AzureDevOpsWebApiClient { /** * Update a pull request. * Requires scope "Code (Read & Write)" (vso.code, vso.code_write). - * @param options + * @param pr * @returns */ - public async updatePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - changes: IFileChange[]; - skipIfDraft?: boolean; - skipIfCommitsFromAuthorsOtherThan?: string; - skipIfNotBehindTargetBranch?: boolean; - }): Promise { - console.info(`Updating pull request #${options.pullRequestId}...`); + public async updatePullRequest(pr: IUpdatePullRequest): Promise { + console.info(`Updating pull request #${pr.pullRequestId}...`); try { // Get the pull request details const pullRequest = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}`, ); if (!pullRequest) { - throw new Error(`Pull request #${options.pullRequestId} not found`); + throw new Error(`Pull request #${pr.pullRequestId} not found`); } // Skip if the pull request is a draft - if (options.skipIfDraft && pullRequest.isDraft) { + if (pr.skipIfDraft && pullRequest.isDraft) { console.info(` - Skipping update as pull request is currently marked as a draft.`); return true; } // Skip if the pull request has been modified by another author - if (options.skipIfCommitsFromAuthorsOtherThan) { + if (pr.skipIfCommitsFromAuthorsOtherThan) { const commits = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/commits`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/commits`, ); - if (commits?.value?.some((c) => c.author?.email !== options.skipIfCommitsFromAuthorsOtherThan)) { + if (commits?.value?.some((c) => c.author?.email !== pr.skipIfCommitsFromAuthorsOtherThan)) { console.info(` - Skipping update as pull request has been modified by another user.`); return true; } } + // Get the branch stats to check if the source branch is behind the target branch + const stats = await this.restApiGet( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/stats/branches`, + { + name: normalizeBranchName(pullRequest.sourceRefName), + }, + ); + if (stats?.behindCount === undefined) { + throw new Error(`Failed to get branch stats for '${pullRequest.sourceRefName}'`); + } + // Skip if the source branch is not behind the target branch - if (options.skipIfNotBehindTargetBranch) { - const stats = await this.restApiGet( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/stats/branches`, - { - name: normalizeBranchName(pullRequest.sourceRefName), - }, + if (pr.skipIfNotBehindTargetBranch && stats.behindCount === 0) { + console.info(` - Skipping update as source branch is not behind target branch.`); + return true; + } + + // Rebase the target branch into the source branch to reset the "behind" count + const sourceBranchName = normalizeBranchName(pullRequest.sourceRefName); + const targetBranchName = normalizeBranchName(pullRequest.targetRefName); + if (stats.behindCount > 0) { + console.info( + ` - Rebasing '${targetBranchName}' into '${sourceBranchName}' (${stats.behindCount} commit(s) behind)...`, ); - if (stats?.behindCount === 0) { - console.info(` - Skipping update as source branch is not behind target branch.`); - return true; + const rebase = await this.restApiPost( + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, + [ + { + name: pullRequest.sourceRefName, + oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + newObjectId: pr.commit, + }, + ], + ); + if (rebase?.value?.[0]?.success !== true) { + throw new Error('Failed to rebase the target branch into the source branch'); } } - // Push changes to the source branch - console.info(` - Pushing ${options.changes.length} file change(s) branch '${pullRequest.sourceRefName}'...`); + // Push all file changes to the source branch + console.info(` - Pushing ${pr.changes.length} file change(s) to branch '${pullRequest.sourceRefName}'...`); const push = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pushes`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pushes`, { refUpdates: [ { name: pullRequest.sourceRefName, - oldObjectId: pullRequest.lastMergeSourceCommit.commitId, + oldObjectId: pr.commit, }, ], commits: [ @@ -372,8 +393,9 @@ export class AzureDevOpsWebApiClient { comment: pullRequest.mergeStatus === PullRequestAsyncStatus.Conflicts ? 'Resolve merge conflicts' - : 'Update dependency files', - changes: options.changes.map((change) => { + : `Rebase with '${targetBranchName}'`, + author: pr.author, + changes: pr.changes.map((change) => { return { changeType: change.changeType, item: { @@ -406,24 +428,20 @@ export class AzureDevOpsWebApiClient { /** * Approve a pull request. * Requires scope "Code (Write)" (vso.code_write). - * @param options + * @param pr * @returns */ - public async approvePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - }): Promise { - console.info(`Approving pull request #${options.pullRequestId}...`); + public async approvePullRequest(pr: IApprovePullRequest): Promise { + console.info(`Approving pull request #${pr.pullRequestId}...`); try { // Approve the pull request - console.info(` - Creating reviewer vote on pull request...`); + console.info(` - Updating reviewer vote on pull request...`); const userId = await this.getUserId(); const userVote = await this.restApiPut( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/reviewers/${userId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/reviewers/${userId}`, { vote: 10, // 10 - approved 5 - approved with suggestions 0 - no vote -5 - waiting for author -10 - rejected - isReapprove: true, + isReapprove: false, // don't re-approve if already approved }, ); if (userVote?.vote != 10) { @@ -439,27 +457,21 @@ export class AzureDevOpsWebApiClient { } /** - * Close a pull request. + * Abandon a pull request. * Requires scope "Code (Write)" (vso.code_write). - * @param options + * @param pr * @returns */ - public async closePullRequest(options: { - project: string; - repository: string; - pullRequestId: number; - comment: string; - deleteSourceBranch: boolean; - }): Promise { - console.info(`Closing pull request #${options.pullRequestId}...`); + public async abandonPullRequest(pr: IAbandonPullRequest): Promise { + console.info(`Abandoning pull request #${pr.pullRequestId}...`); try { const userId = await this.getUserId(); // Add a comment to the pull request, if supplied - if (options.comment) { + if (pr.comment) { console.info(` - Adding abandonment reason comment to pull request...`); const thread = await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}/threads`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}/threads`, { status: CommentThreadStatus.Closed, comments: [ @@ -467,7 +479,7 @@ export class AzureDevOpsWebApiClient { author: { id: userId, }, - content: options.comment, + content: pr.comment, commentType: CommentType.System, }, ], @@ -478,10 +490,10 @@ export class AzureDevOpsWebApiClient { } } - // Close the pull request + // Abandon the pull request console.info(` - Abandoning pull request...`); const abandonedPullRequest = await this.restApiPatch( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/pullrequests/${options.pullRequestId}`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/pullrequests/${pr.pullRequestId}`, { status: PullRequestStatus.Abandoned, closedBy: { @@ -490,14 +502,14 @@ export class AzureDevOpsWebApiClient { }, ); if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { - throw new Error('Failed to close pull request, status was not updated'); + throw new Error('Failed to abandon pull request, status was not updated'); } // Delete the source branch if required - if (options.deleteSourceBranch) { + if (pr.deleteSourceBranch) { console.info(` - Deleting source branch...`); await this.restApiPost( - `${this.organisationApiUrl}/${options.project}/_apis/git/repositories/${options.repository}/refs`, + `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, { name: abandonedPullRequest.sourceRefName, oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, @@ -507,10 +519,10 @@ export class AzureDevOpsWebApiClient { ); } - console.info(` - Pull request was closed successfully.`); + console.info(` - Pull request was abandoned successfully.`); return true; } catch (e) { - error(`Failed to close pull request: ${e}`); + error(`Failed to abandon pull request: ${e}`); console.debug(e); // Dump the error stack trace to help with debugging return false; } @@ -632,12 +644,6 @@ export class AzureDevOpsWebApiClient { const body = await response.readBody(); console.debug(`🌎 🠈 [${response.message.statusCode}] ${response.message.statusMessage}`); try { - if (response.message.statusCode === 401) { - throw new Error(`No access token has been provided to access '${url}'`); - } - if (response.message.statusCode === 403) { - throw new Error(`The access token provided does not have permissions to access '${url}'`); - } if (response.message.statusCode < 200 || response.message.statusCode > 299) { throw new Error(`Request to '${url}' failed: ${response.message.statusCode} ${response.message.statusMessage}`); } diff --git a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts index aaa21a02..f27464bf 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequest.ts @@ -2,9 +2,20 @@ import { GitPullRequestMergeStrategy } from 'azure-devops-node-api/interfaces/Gi import { IFileChange } from './IFileChange'; /** - * Pull request creation + * Pull request properties */ -export interface IPullRequest { +export interface IPullRequestProperties { + id: number; + properties?: { + name: string; + value: string; + }[]; +} + +/** + * Pull request creation request + */ +export interface ICreatePullRequest { project: string; repository: string; source: { @@ -35,3 +46,41 @@ export interface IPullRequest { value: string; }[]; } + +/** + * Pull request update request + */ +export interface IUpdatePullRequest { + project: string; + repository: string; + pullRequestId: number; + commit?: string; + author?: { + email: string; + name: string; + }; + changes: IFileChange[]; + skipIfDraft?: boolean; + skipIfCommitsFromAuthorsOtherThan?: string; + skipIfNotBehindTargetBranch?: boolean; +} + +/** + * Pull request approval request + */ +export interface IApprovePullRequest { + project: string; + repository: string; + pullRequestId: number; +} + +/** + * Pull request abandon request + */ +export interface IAbandonPullRequest { + project: string; + repository: string; + pullRequestId: number; + comment: string; + deleteSourceBranch: boolean; +} diff --git a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts b/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts deleted file mode 100644 index 2fd2e6fb..00000000 --- a/extension/tasks/dependabotV2/utils/azure-devops/interfaces/IPullRequestProperties.ts +++ /dev/null @@ -1,10 +0,0 @@ -/** - * Pull request properties - */ -export interface IPullRequestProperties { - id: number; - properties?: { - name: string; - value: string; - }[]; -} diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index c1e81aff..7607f7d3 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -3,7 +3,7 @@ import { error, warning } from 'azure-pipelines-task-lib/task'; import * as crypto from 'crypto'; import * as path from 'path'; import { AzureDevOpsWebApiClient } from '../azure-devops/AzureDevOpsWebApiClient'; -import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequestProperties'; +import { IPullRequestProperties } from '../azure-devops/interfaces/IPullRequest'; import { IDependabotUpdate } from '../dependabot/interfaces/IDependabotConfig'; import { ISharedVariables } from '../getSharedVariables'; import { IDependabotUpdateOperation } from './interfaces/IDependabotUpdateOperation'; @@ -179,6 +179,11 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess project: project, repository: repository, pullRequestId: pullRequestToUpdate.id, + commit: data['base-commit-sha'] || update.job.source.commit, + author: { + email: this.taskInputs.authorEmail || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_EMAIL, + name: this.taskInputs.authorName || DependabotOutputProcessor.PR_DEFAULT_AUTHOR_NAME, + }, changes: getPullRequestChangedFilesForOutputData(data), skipIfDraft: true, skipIfCommitsFromAuthorsOtherThan: @@ -219,7 +224,7 @@ export class DependabotOutputProcessor implements IDependabotUpdateOutputProcess // How do we detect this? Do we need to? // Close the pull request - return await this.prAuthorClient.closePullRequest({ + return await this.prAuthorClient.abandonPullRequest({ project: project, repository: repository, pullRequestId: pullRequestToClose.id, From 29dab82b106e30ab91f8be26771ff611077448d1 Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 14:52:43 +1300 Subject: [PATCH 16/17] Tidy up --- .../azure-devops/AzureDevOpsWebApiClient.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts index 6dd47487..fe00ad27 100644 --- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts +++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts @@ -501,22 +501,27 @@ export class AzureDevOpsWebApiClient { }, }, ); - if (abandonedPullRequest?.status !== PullRequestStatus.Abandoned) { + if (abandonedPullRequest?.status?.toLowerCase() !== 'abandoned') { throw new Error('Failed to abandon pull request, status was not updated'); } // Delete the source branch if required if (pr.deleteSourceBranch) { console.info(` - Deleting source branch...`); - await this.restApiPost( + const deletedBranch = await this.restApiPost( `${this.organisationApiUrl}/${pr.project}/_apis/git/repositories/${pr.repository}/refs`, - { - name: abandonedPullRequest.sourceRefName, - oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, - newObjectId: '0000000000000000000000000000000000000000', - isLocked: false, - }, + [ + { + name: abandonedPullRequest.sourceRefName, + oldObjectId: abandonedPullRequest.lastMergeSourceCommit.commitId, + newObjectId: '0000000000000000000000000000000000000000', + isLocked: false, + }, + ], ); + if (deletedBranch?.value?.[0]?.success !== true) { + throw new Error('Failed to delete the source branch'); + } } console.info(` - Pull request was abandoned successfully.`); From 0ea23dbd9bef779c178f103753d14a63e0aef1ae Mon Sep 17 00:00:00 2001 From: Rhys Koedijk Date: Mon, 7 Oct 2024 15:51:47 +1300 Subject: [PATCH 17/17] Fix undefined reference when reading branch name seprarator --- .../utils/dependabot-cli/DependabotOutputProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts index 7607f7d3..1b08e0da 100644 --- a/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts +++ b/extension/tasks/dependabotV2/utils/dependabot-cli/DependabotOutputProcessor.ts @@ -326,7 +326,7 @@ export function parsePullRequestProperties( function getSourceBranchNameForUpdate(update: IDependabotUpdate, targetBranch: string, dependencies: any): string { const prefix = 'dependabot'; // TODO: Add config for this? Task V1 supported this via DEPENDABOT_BRANCH_NAME_PREFIX - const separator = update['pull-request-branch-name'].separator || '/'; + const separator = update['pull-request-branch-name']?.separator || '/'; const packageEcosystem = update['package-ecosystem']; const targetBranchName = targetBranch?.replace(/^\/+|\/+$/g, ''); // strip leading/trailing slashes if (dependencies['dependency-group-name']) {