-
Notifications
You must be signed in to change notification settings - Fork 124
External PR 1458:Refine GitHub endpoint filtering logic #1466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug in the --rewire-pipelines functionality for Azure DevOps to GitHub migrations when using GitHub Enterprise Cloud with data residency. The issue was that GitHubProximaPipelines service connections were incorrectly being matched by GitHub organization name instead of team project name, unlike the standard GitHub service connections.
Key Changes:
- Updated the service endpoint matching logic to properly differentiate between
GitHubandGitHubProximaPipelinesconnection types - Modified
GitHubconnections to match by GitHub org name (unchanged behavior) - Modified
GitHubProximaPipelinesconnections to match by team project name (corrected behavior)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Octoshift/Services/AdoApi.cs | Refactored the endpoint matching logic to use separate AND conditions for each connection type, ensuring GitHubProximaPipelines matches by team project name |
| src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs | Updated test data to use ADO_TEAM_PROJECT instead of GITHUB_ORG for the GitHubProximaPipelines connection type |
| RELEASENOTES.md | Added user-friendly description of the bug fix for the upcoming release |
Comments suppressed due to low confidence (1)
src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs:276
- Consider adding a negative test case for
GitHubProximaPipelinestype that verifies null is returned when the team project name doesn't match. This would be analogous to the existing testGetGithubAppId_Should_Return_Null_When_No_Team_Projects_Have_Endpoint()which tests the negative case for the "GitHub" type with a wrong org name.
Example test: A GitHubProximaPipelines endpoint with name "wrongTeamProject" should return null when looking for ADO_TEAM_PROJECT.
public async Task GetGithubAppId_Should_Recognize_GitHubProximaPipelines_Service_Connection()
{
var teamProjects = new List<string>() { ADO_TEAM_PROJECT };
var appId = Guid.NewGuid().ToString();
var json = new object[]
{
new
{
type = "GitHubProximaPipelines",
name = ADO_TEAM_PROJECT,
id = appId
}
};
var response = JArray.Parse(json.ToJson());
_mockAdoClient.Setup(x => x.GetWithPagingAsync($"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{ADO_TEAM_PROJECT.EscapeDataString()}/_apis/serviceendpoint/endpoints?api-version=6.0-preview.4").Result).Returns(response);
var result = await sut.GetGithubAppId(ADO_ORG, GITHUB_ORG, teamProjects);
result.Should().Be(appId);
}
Unit Test Results 1 files 1 suites 10m 25s ⏱️ Results for commit ccadb4b. ♻️ This comment has been updated with latest results. |
begonaguereca
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve updaing github enpoint logic
ThirdPartyNotices.txt(if applicable)Shipping #1458