diff --git a/README.md b/README.md index 98ec1e634..c94b2034b 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,18 @@ Refer to the [official documentation](https://docs.github.com/en/migrations/usin Refer to the [official documentation](https://docs.github.com/en/migrations/using-github-enterprise-importer/migrating-repositories-with-github-enterprise-importer/migrating-repositories-from-azure-devops-to-github-enterprise-cloud) for more details. +#### Cleaning Status Checks during Migration +When migrating from Azure DevOps, both branch policies and status checks are transferred to GitHub. However, many ADO status checks are not supported in GitHub. To clean status checks from GitHub branch protection rules after migration, use the `--clean-status-checks` option: + +>`gh ado2gh migrate-repo --ado-org ORGNAME --ado-team-project PROJECTNAME --ado-repo REPONAME --github-org ORGNAME --github-repo REPONAME --clean-status-checks` + +This option will: +- Complete the normal migration process (including transferring all branch policies) +- After migration completion, automatically remove status checks from the default branch's protection rule +- Preserve other branch protection settings like required reviewers, admin enforcement, etc. + +**Note:** This is a post-migration cleanup step, so it only affects the final GitHub repository state and does not impact the migration data transfer itself. + ### Bitbucket Server and Data Center to GitHub Usage 1. Create Personal Access Token for the target GitHub org (for more details on scopes needed refer to our [official documentation](https://docs.github.com/en/migrations/using-github-enterprise-importer/preparing-to-migrate-with-github-enterprise-importer/managing-access-for-github-enterprise-importer)). @@ -89,11 +101,11 @@ Refer to the [official documentation](https://docs.github.com/en/migrations/usin ### Skipping version checks -When the CLI is launched, it logs if a newer version of the CLI is available. You can skip this check by setting the `GEI_SKIP_VERSION_CHECK` environment variable to `true`. +When the CLI is launched, it logs if a newer version of the CLI is available. You can skip this check by setting the `GEI_SKIP_VERSION_CHECK` environment variable to `true`. ### Skipping GitHub status checks -When the CLI is launched, it logs a warning if there are any ongoing [GitHub incidents](https://www.githubstatus.com/) that might affect your use of the CLI. You can skip this check by setting the `GEI_SKIP_STATUS_CHECK` environment variable to `true`. +When the CLI is launched, it logs a warning if there are any ongoing [GitHub incidents](https://www.githubstatus.com/) that might affect your use of the CLI. You can skip this check by setting the `GEI_SKIP_STATUS_CHECK` environment variable to `true`. ## Contributions diff --git a/src/Octoshift/Services/GithubApi.cs b/src/Octoshift/Services/GithubApi.cs index bd10cf540..a746500ba 100644 --- a/src/Octoshift/Services/GithubApi.cs +++ b/src/Octoshift/Services/GithubApi.cs @@ -390,7 +390,7 @@ mutation startRepositoryMigration( $lockSource: Boolean)"; var gql = @" startRepositoryMigration( - input: { + input: { sourceId: $sourceId, ownerId: $ownerId, sourceRepositoryUrl: $sourceRepositoryUrl, @@ -456,7 +456,7 @@ mutation startOrganizationMigration ( $targetEnterpriseId: ID!, $sourceAccessToken: String!)"; var gql = @" - startOrganizationMigration( + startOrganizationMigration( input: { sourceOrgUrl: $sourceOrgUrl, targetOrgName: $targetOrgName, @@ -1077,7 +1077,7 @@ mutation abortRepositoryMigration( )"; var gql = @" abortRepositoryMigration( - input: { + input: { migrationId: $migrationId }) { success }"; @@ -1264,4 +1264,35 @@ private static CodeScanningAlertInstance BuildCodeScanningAlertInstance(JToken s StartColumn = (int)scanningAlertInstance["location"]["start_column"], EndColumn = (int)scanningAlertInstance["location"]["end_column"] }; + + public virtual async Task GetBranchProtection(string org, string repo, string branch) + { + var url = $"{_apiUrl}/repos/{org.EscapeDataString()}/{repo.EscapeDataString()}/branches/{branch.EscapeDataString()}/protection"; + + try + { + var response = await _client.GetAsync(url); + return JObject.Parse(response); + } + catch (HttpRequestException) + { + // Branch protection may not exist, return null + return null; + } + } + + public virtual async Task UpdateBranchProtection(string org, string repo, string branch, object protection) + { + var url = $"{_apiUrl}/repos/{org.EscapeDataString()}/{repo.EscapeDataString()}/branches/{branch.EscapeDataString()}/protection"; + await _client.PutAsync(url, protection); + } + + public virtual async Task> GetBranches(string org, string repo) + { + var url = $"{_apiUrl}/repos/{org.EscapeDataString()}/{repo.EscapeDataString()}/branches"; + var response = await _client.GetAsync(url); + var data = JArray.Parse(response); + + return data.Select(x => (string)x["name"]); + } } diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs index f1027f1ac..70c193a12 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/GithubApiTests.cs @@ -272,7 +272,7 @@ public async Task GetTeamMembers_Returns_Team_Members() ""id"": 1 }}, {{ - ""login"": ""{teamMember2}"", + ""login"": ""{teamMember2}"", ""id"": 2 }} ]"; @@ -286,7 +286,7 @@ public async Task GetTeamMembers_Returns_Team_Members() ""id"": 3 }}, {{ - ""login"": ""{teamMember4}"", + ""login"": ""{teamMember4}"", ""id"": 4 }} ]"; @@ -555,7 +555,7 @@ public async Task GetOrgMembershipForUser_Returns_User_Role() var role = "admin"; var response = $@" {{ - ""role"": ""{role}"" + ""role"": ""{role}"" }}"; _githubClientMock @@ -638,15 +638,15 @@ public async Task GetOrganizationId_Returns_The_Org_Id() $"{{\"query\":\"query($login: String!) {{organization(login: $login) {{ login, id, name }} }}\",\"variables\":{{\"login\":\"{GITHUB_ORG}\"}}}}"; var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""organization"": + ""organization"": {{ ""login"": ""{GITHUB_ORG}"", ""id"": ""{orgId}"", - ""name"": ""github"" - }} - }} + ""name"": ""github"" + }} + }} }}"); _githubClientMock @@ -672,15 +672,15 @@ public async Task GetOrganizationId_Retries_On_GQL_Error() var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""organization"": + ""organization"": {{ ""login"": ""{GITHUB_ORG}"", ""id"": ""{orgId}"", - ""name"": ""github"" - }} - }} + ""name"": ""github"" + }} + }} }}"); _githubClientMock @@ -739,15 +739,15 @@ public async Task GetOrganizationDatabaseId_Retries_On_GQL_Error() var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""organization"": + ""organization"": {{ ""login"": ""{GITHUB_ORG}"", ""databaseId"": ""{databaseId}"", - ""name"": ""github"" - }} - }} + ""name"": ""github"" + }} + }} }}"); _githubClientMock @@ -775,14 +775,14 @@ public async Task GetEnterpriseId_Returns_The_Enterprise_Id() $"{{\"query\":\"query($slug: String!) {{enterprise (slug: $slug) {{ slug, id }} }}\",\"variables\":{{\"slug\":\"{GITHUB_ENTERPRISE}\"}}}}"; var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""enterprise"": + ""enterprise"": {{ ""slug"": ""{GITHUB_ENTERPRISE}"", ""id"": ""{enterpriseId}"" - }} - }} + }} + }} }}"); _githubClientMock @@ -808,14 +808,14 @@ public async Task GetEnterpriseId_Retries_On_GQL_Error() var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""enterprise"": + ""enterprise"": {{ ""slug"": ""{GITHUB_ENTERPRISE}"", ""id"": ""{enterpriseId}"" - }} - }} + }} + }} }}"); _githubClientMock @@ -1005,7 +1005,7 @@ mutation startRepositoryMigration( $lockSource: Boolean)"; const string gql = @" startRepositoryMigration( - input: { + input: { sourceId: $sourceId, ownerId: $ownerId, sourceRepositoryUrl: $sourceRepositoryUrl, @@ -1115,7 +1115,7 @@ mutation startRepositoryMigration( $lockSource: Boolean)"; const string gql = @" startRepositoryMigration( - input: { + input: { sourceId: $sourceId, ownerId: $ownerId, sourceRepositoryUrl: $sourceRepositoryUrl, @@ -1201,7 +1201,7 @@ public async Task StartMigration_Does_Not_Throw_When_Errors_Is_Empty() // Arrange var response = JObject.Parse(@" { - ""data"": { + ""data"": { ""startRepositoryMigration"": { ""repositoryMigration"": { ""id"": ""RM_kgC4NjFhNmE2NGU2ZWE1YTQwMDA5ODliZjhi"" @@ -2018,14 +2018,14 @@ public async Task GetUserId_Returns_The_User_Id() var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""user"": + ""user"": {{ ""id"": ""{userId}"", - ""name"": ""{login}"" - }} - }} + ""name"": ""{login}"" + }} + }} }}"); _githubClientMock @@ -2125,7 +2125,7 @@ public async Task GetMannequins_Returns_NoMannequins() var url = $"https://api.github.com/graphql"; var payload = -@"{""query"":""query($id: ID!, $first: Int, $after: String) { +@"{""query"":""query($id: ID!, $first: Int, $after: String) { node(id: $id) { ... on Organization { mannequins(first: $first, after: $after) { @@ -2176,7 +2176,7 @@ public async Task GetMannequins_Returns_Mannequins() var url = $"https://api.github.com/graphql"; var payload = -@"{""query"":""query($id: ID!, $first: Int, $after: String) { +@"{""query"":""query($id: ID!, $first: Int, $after: String) { node(id: $id) { ... on Organization { mannequins(first: $first, after: $after) { @@ -2259,7 +2259,7 @@ public async Task GetMannequins_Retries_On_Error() var url = $"https://api.github.com/graphql"; var payload = -@"{""query"":""query($id: ID!, $first: Int, $after: String) { +@"{""query"":""query($id: ID!, $first: Int, $after: String) { node(id: $id) { ... on Organization { mannequins(first: $first, after: $after) { @@ -2343,7 +2343,7 @@ public async Task GetMannequinsByLogin_Returns_NoMannequins() var url = $"https://api.github.com/graphql"; var payload = -@"{""query"":""query($id: ID!, $first: Int, $after: String, $login: String) { +@"{""query"":""query($id: ID!, $first: Int, $after: String, $login: String) { node(id: $id) { ... on Organization { mannequins(first: $first, after: $after, login: $login) { @@ -2393,7 +2393,7 @@ public async Task GetMannequinsByLogin_Returns_Mannequins() var url = $"https://api.github.com/graphql"; var payload = -@"{""query"":""query($id: ID!, $first: Int, $after: String, $login: String) { +@"{""query"":""query($id: ID!, $first: Int, $after: String, $login: String) { node(id: $id) { ... on Organization { mannequins(first: $first, after: $after, login: $login) { @@ -2985,7 +2985,7 @@ public async Task GetDefaultBranch_Returns_Default_Branch_Field() var response = $@" {{ - ""default_branch"": ""main"" + ""default_branch"": ""main"" }}"; _githubClientMock @@ -3368,7 +3368,7 @@ public async Task UploadSarif_Returns_Id_From_Response() var response = $@" {{ ""id"": ""sarif-id"", - }} + }} "; _githubClientMock .Setup(m => m.PostAsync(url, It.Is(x => x.ToJson() == expectedPayload.ToJson()), null)) @@ -3401,7 +3401,7 @@ public async Task UploadSarif_Retries_On_502() var response = $@" {{ ""id"": ""sarif-id"", - }} + }} "; _githubClientMock .SetupSequence(m => m.PostAsync(url, It.Is(x => x.ToJson() == expectedPayload.ToJson()), null)) @@ -3426,7 +3426,7 @@ public async Task GetSarifProcessingStatus_Returns_Processing_Status_From_Respon {{ ""analyses_url"": ""https://api.,github.com/repos/{GITHUB_ORG}/{GITHUB_REPO}/code-scanning/sarifs/sarif-id"", ""processing_status"": ""pending"" - }} + }} "; _githubClientMock .Setup(m => m.GetAsync(url, null)) @@ -3453,7 +3453,7 @@ public async Task GetSarifProcessingStatus_Returns_Errors_From_Response() ""error1"", ""error2"" ] - }} + }} "; _githubClientMock .Setup(m => m.GetAsync(url, null)) @@ -3532,7 +3532,7 @@ mutation abortRepositoryMigration( )"; const string gql = @" abortRepositoryMigration( - input: { + input: { migrationId: $migrationId }) { success }"; @@ -3550,9 +3550,9 @@ mutation abortRepositoryMigration( const bool actualBooleanResponse = true; var response = JObject.Parse($@" {{ - ""data"": + ""data"": {{ - ""abortRepositoryMigration"": + ""abortRepositoryMigration"": {{ ""success"": ""{actualBooleanResponse}"" }} @@ -3591,7 +3591,7 @@ await _githubApi.Invoking(api => api.AbortMigration(migrationId)) [Fact] public async Task UploadArchiveToGithubStorage_Should_Upload_The_Content() { - //Arange + //Arange const string orgDatabaseId = "1234"; const string archiveName = "archiveName"; @@ -3963,6 +3963,110 @@ public async Task GetSecretScanningAlertsForRepository_Populates_ResolutionComme array[0].ResolverName.Should().Be("actor"); } + [Fact] + public async Task GetBranchProtection_Should_Return_Protection_Rules() + { + // Arrange + var org = "myorg"; + var repo = "myrepo"; + var branch = "main"; + var expectedUrl = $"https://api.github.com/repos/{org}/{repo}/branches/{branch}/protection"; + var responsePayload = @"{ + ""required_status_checks"": { + ""strict"": true, + ""contexts"": [""ci/build"", ""ci/test""] + }, + ""enforce_admins"": { + ""enabled"": true + }, + ""required_pull_request_reviews"": { + ""required_approving_review_count"": 2 + } + }"; + + _githubClientMock + .Setup(m => m.GetAsync(expectedUrl, null)) + .ReturnsAsync(responsePayload); + + // Act + var result = await _githubApi.GetBranchProtection(org, repo, branch); + + // Assert + result.Should().NotBeNull(); + result["required_status_checks"]["strict"].Value().Should().BeTrue(); + result["required_status_checks"]["contexts"].Should().HaveCount(2); + result["enforce_admins"]["enabled"].Value().Should().BeTrue(); + } + + [Fact] + public async Task GetBranchProtection_Should_Return_Null_When_No_Protection() + { + // Arrange + var org = "myorg"; + var repo = "myrepo"; + var branch = "main"; + var expectedUrl = $"https://api.github.com/repos/{org}/{repo}/branches/{branch}/protection"; + + _githubClientMock + .Setup(m => m.GetAsync(expectedUrl, null)) + .ThrowsAsync(new HttpRequestException("Not Found")); + + // Act + var result = await _githubApi.GetBranchProtection(org, repo, branch); + + // Assert + result.Should().BeNull(); + } + + [Fact] + public async Task UpdateBranchProtection_Should_Call_Put_With_Correct_Parameters() + { + // Arrange + var org = "myorg"; + var repo = "myrepo"; + var branch = "main"; + var expectedUrl = $"https://api.github.com/repos/{org}/{repo}/branches/{branch}/protection"; + var protection = new + { + required_status_checks = default(object), + enforce_admins = true, + required_pull_request_reviews = new { required_approving_review_count = 2 } + }; + + // Act + await _githubApi.UpdateBranchProtection(org, repo, branch, protection); + + // Assert + _githubClientMock.Verify(m => m.PutAsync(expectedUrl, protection, null), Times.Once); + } + + [Fact] + public async Task GetBranches_Should_Return_Branch_Names() + { + // Arrange + var org = "myorg"; + var repo = "myrepo"; + var expectedUrl = $"https://api.github.com/repos/{org}/{repo}/branches"; + var responsePayload = @"[ + {""name"": ""main"", ""commit"": {""sha"": ""abc123""}}, + {""name"": ""develop"", ""commit"": {""sha"": ""def456""}}, + {""name"": ""feature-branch"", ""commit"": {""sha"": ""ghi789""}} + ]"; + + _githubClientMock + .Setup(m => m.GetAsync(expectedUrl, null)) + .ReturnsAsync(responsePayload); + + // Act + var result = await _githubApi.GetBranches(org, repo); + + // Assert + result.Should().HaveCount(3); + result.Should().Contain("main"); + result.Should().Contain("develop"); + result.Should().Contain("feature-branch"); + } + private string Compact(string source) => source .Replace("\r", "") diff --git a/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs b/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs index f753a2c1f..2efe7acfa 100644 --- a/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs +++ b/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandlerTests.cs @@ -310,4 +310,218 @@ public async Task Sets_Target_Repo_Visibility_When_Specified() targetRepoVisibility, It.IsAny())); } + + [Fact] + public async Task Should_Disable_Status_Checks_When_Flag_Is_True() + { + // Arrange + _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); + _mockGithubApi.Setup(x => x.CreateAdoMigrationSource(GITHUB_ORG_ID, null).Result).Returns(MIGRATION_SOURCE_ID); + _mockGithubApi + .Setup(x => x.StartMigration( + MIGRATION_SOURCE_ID, + ADO_REPO_URL, + GITHUB_ORG_ID, + GITHUB_REPO, + ADO_TOKEN, + GITHUB_TOKEN, + null, + null, + false, + null, + false).Result) + .Returns(MIGRATION_ID); + _mockGithubApi.Setup(x => x.GetMigration(MIGRATION_ID).Result).Returns((State: RepositoryMigrationStatus.Succeeded, GITHUB_REPO, 0, null, null)); + + // Setup branch protection data + var branches = new List { "main", "develop" }; + _mockGithubApi.Setup(x => x.GetBranches(GITHUB_ORG, GITHUB_REPO).Result).Returns(branches); + + var protectionWithStatusChecks = new Newtonsoft.Json.Linq.JObject + { + ["required_status_checks"] = new Newtonsoft.Json.Linq.JObject + { + ["strict"] = true, + ["contexts"] = new Newtonsoft.Json.Linq.JArray { "ci/build", "ci/test" } + }, + ["enforce_admins"] = new Newtonsoft.Json.Linq.JObject { ["enabled"] = true }, + ["required_pull_request_reviews"] = new Newtonsoft.Json.Linq.JObject { ["required_approving_review_count"] = 2 } + }; + + _mockGithubApi.Setup(x => x.GetDefaultBranch(GITHUB_ORG, GITHUB_REPO).Result).Returns("main"); + _mockGithubApi.Setup(x => x.GetBranchProtection(GITHUB_ORG, GITHUB_REPO, "main").Result).Returns(protectionWithStatusChecks); + + _mockEnvironmentVariableProvider + .Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())) + .Returns(GITHUB_TOKEN); + _mockEnvironmentVariableProvider + .Setup(m => m.AdoPersonalAccessToken(It.IsAny())) + .Returns(ADO_TOKEN); + + // Act + var args = new MigrateRepoCommandArgs + { + AdoOrg = ADO_ORG, + AdoTeamProject = ADO_TEAM_PROJECT, + AdoRepo = ADO_REPO, + GithubOrg = GITHUB_ORG, + GithubRepo = GITHUB_REPO, + CleanStatusChecks = true + }; + await _handler.Handle(args); + + // Assert + _mockGithubApi.Verify(m => m.GetDefaultBranch(GITHUB_ORG, GITHUB_REPO), Times.Once); + _mockGithubApi.Verify(m => m.GetBranchProtection(GITHUB_ORG, GITHUB_REPO, "main"), Times.Once); + _mockGithubApi.Verify(m => m.UpdateBranchProtection(GITHUB_ORG, GITHUB_REPO, "main", It.IsAny()), Times.Once); + _mockOctoLogger.Verify(m => m.LogInformation("Cleaning status checks from default branch protection..."), Times.Once); + _mockOctoLogger.Verify(m => m.LogInformation("Cleaning status checks for branch 'main'"), Times.Once); + _mockOctoLogger.Verify(m => m.LogSuccess("Successfully cleaned status checks from default branch protection"), Times.Once); + } + + [Fact] + public async Task Should_Not_Disable_Status_Checks_When_Flag_Is_False() + { + // Arrange + _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); + _mockGithubApi.Setup(x => x.CreateAdoMigrationSource(GITHUB_ORG_ID, null).Result).Returns(MIGRATION_SOURCE_ID); + _mockGithubApi + .Setup(x => x.StartMigration( + MIGRATION_SOURCE_ID, + ADO_REPO_URL, + GITHUB_ORG_ID, + GITHUB_REPO, + ADO_TOKEN, + GITHUB_TOKEN, + null, + null, + false, + null, + false).Result) + .Returns(MIGRATION_ID); + _mockGithubApi.Setup(x => x.GetMigration(MIGRATION_ID).Result).Returns((State: RepositoryMigrationStatus.Succeeded, GITHUB_REPO, 0, null, null)); + + _mockEnvironmentVariableProvider + .Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())) + .Returns(GITHUB_TOKEN); + _mockEnvironmentVariableProvider + .Setup(m => m.AdoPersonalAccessToken(It.IsAny())) + .Returns(ADO_TOKEN); + + // Act + var args = new MigrateRepoCommandArgs + { + AdoOrg = ADO_ORG, + AdoTeamProject = ADO_TEAM_PROJECT, + AdoRepo = ADO_REPO, + GithubOrg = GITHUB_ORG, + GithubRepo = GITHUB_REPO, + CleanStatusChecks = false + }; + await _handler.Handle(args); + + // Assert - should not call any branch protection methods + _mockGithubApi.Verify(m => m.GetBranches(It.IsAny(), It.IsAny()), Times.Never); + _mockGithubApi.Verify(m => m.GetBranchProtection(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockGithubApi.Verify(m => m.UpdateBranchProtection(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockOctoLogger.Verify(m => m.LogInformation("Disabling status checks in branch protection rules..."), Times.Never); + } + + [Fact] + public async Task Should_Skip_Status_Check_Removal_When_Queue_Only_Is_True() + { + // Arrange + _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); + _mockGithubApi.Setup(x => x.CreateAdoMigrationSource(GITHUB_ORG_ID, null).Result).Returns(MIGRATION_SOURCE_ID); + _mockGithubApi + .Setup(x => x.StartMigration( + MIGRATION_SOURCE_ID, + ADO_REPO_URL, + GITHUB_ORG_ID, + GITHUB_REPO, + ADO_TOKEN, + GITHUB_TOKEN, + null, + null, + false, + null, + false).Result) + .Returns(MIGRATION_ID); + + _mockEnvironmentVariableProvider + .Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())) + .Returns(GITHUB_TOKEN); + _mockEnvironmentVariableProvider + .Setup(m => m.AdoPersonalAccessToken(It.IsAny())) + .Returns(ADO_TOKEN); + + // Act + var args = new MigrateRepoCommandArgs + { + AdoOrg = ADO_ORG, + AdoTeamProject = ADO_TEAM_PROJECT, + AdoRepo = ADO_REPO, + GithubOrg = GITHUB_ORG, + GithubRepo = GITHUB_REPO, + QueueOnly = true, + CleanStatusChecks = true + }; + await _handler.Handle(args); + + // Assert - should not call any branch protection methods when queue-only + _mockGithubApi.Verify(m => m.GetBranches(It.IsAny(), It.IsAny()), Times.Never); + _mockGithubApi.Verify(m => m.GetBranchProtection(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockGithubApi.Verify(m => m.UpdateBranchProtection(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task Should_Handle_Exception_During_Status_Check_Removal_Gracefully() + { + // Arrange + _mockGithubApi.Setup(x => x.GetOrganizationId(GITHUB_ORG).Result).Returns(GITHUB_ORG_ID); + _mockGithubApi.Setup(x => x.CreateAdoMigrationSource(GITHUB_ORG_ID, null).Result).Returns(MIGRATION_SOURCE_ID); + _mockGithubApi + .Setup(x => x.StartMigration( + MIGRATION_SOURCE_ID, + ADO_REPO_URL, + GITHUB_ORG_ID, + GITHUB_REPO, + ADO_TOKEN, + GITHUB_TOKEN, + null, + null, + false, + null, + false).Result) + .Returns(MIGRATION_ID); + _mockGithubApi.Setup(x => x.GetMigration(MIGRATION_ID).Result).Returns((State: RepositoryMigrationStatus.Succeeded, GITHUB_REPO, 0, null, null)); + + // Setup branch protection data that will throw an exception + _mockGithubApi.Setup(x => x.GetDefaultBranch(GITHUB_ORG, GITHUB_REPO)).ThrowsAsync(new System.Net.Http.HttpRequestException("API error")); + + _mockEnvironmentVariableProvider + .Setup(m => m.TargetGithubPersonalAccessToken(It.IsAny())) + .Returns(GITHUB_TOKEN); + _mockEnvironmentVariableProvider + .Setup(m => m.AdoPersonalAccessToken(It.IsAny())) + .Returns(ADO_TOKEN); + + // Act + var args = new MigrateRepoCommandArgs + { + AdoOrg = ADO_ORG, + AdoTeamProject = ADO_TEAM_PROJECT, + AdoRepo = ADO_REPO, + GithubOrg = GITHUB_ORG, + GithubRepo = GITHUB_REPO, + CleanStatusChecks = true + }; + + // Should not throw exception + await _handler.Handle(args); + + // Assert + _mockOctoLogger.Verify(m => m.LogInformation("Cleaning status checks from default branch protection..."), Times.Once); + _mockOctoLogger.Verify(m => m.LogWarning("Failed to clean status checks: API error"), Times.Once); + } } diff --git a/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs b/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs index dbea4b282..353e2b935 100644 --- a/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs +++ b/src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandTests.cs @@ -35,7 +35,7 @@ public void Should_Have_Options() { _command.Should().NotBeNull(); _command.Name.Should().Be("migrate-repo"); - _command.Options.Count.Should().Be(12); + _command.Options.Count.Should().Be(13); TestHelpers.VerifyCommandOption(_command.Options, "ado-org", true); TestHelpers.VerifyCommandOption(_command.Options, "ado-team-project", true); @@ -49,6 +49,7 @@ public void Should_Have_Options() TestHelpers.VerifyCommandOption(_command.Options, "github-pat", false); TestHelpers.VerifyCommandOption(_command.Options, "verbose", false); TestHelpers.VerifyCommandOption(_command.Options, "target-api-url", false); + TestHelpers.VerifyCommandOption(_command.Options, "clean-status-checks", false); } [Fact] @@ -95,5 +96,36 @@ public void It_Uses_Target_Api_Url_When_Provided() _mockGithubApiFactory.Verify(m => m.Create(targetApiUrl, It.IsAny(), githubPat)); } + + [Fact] + public void Should_Include_Disable_Status_Checks_In_Args() + { + var args = new MigrateRepoCommandArgs + { + AdoOrg = "foo-org", + AdoTeamProject = "blah-tp", + AdoRepo = "some-repo", + GithubOrg = "gh-org", + GithubRepo = "gh-repo", + CleanStatusChecks = true + }; + + args.CleanStatusChecks.Should().BeTrue(); + } + + [Fact] + public void Should_Default_Disable_Status_Checks_To_False() + { + var args = new MigrateRepoCommandArgs + { + AdoOrg = "foo-org", + AdoTeamProject = "blah-tp", + AdoRepo = "some-repo", + GithubOrg = "gh-org", + GithubRepo = "gh-repo" + }; + + args.CleanStatusChecks.Should().BeFalse(); + } } } diff --git a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs index 629d926a5..a77023701 100644 --- a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs +++ b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommand.cs @@ -27,6 +27,7 @@ public MigrateRepoCommand() : base( AddOption(GithubPat); AddOption(Verbose); AddOption(TargetApiUrl); + AddOption(CleanStatusChecks); } public Option AdoOrg { get; } = new("--ado-org") @@ -69,6 +70,10 @@ public MigrateRepoCommand() : base( public Option AdoPat { get; } = new("--ado-pat"); public Option GithubPat { get; } = new("--github-pat"); public Option Verbose { get; } = new("--verbose"); + public Option CleanStatusChecks { get; } = new("--clean-status-checks") + { + Description = "Cleans status checks from the default branch protection after migration. Keeps 'Require status checks to pass before merging' enabled but empties the status check lists, removing any migrated ADO build policies that may not be compatible." + }; public override MigrateRepoCommandHandler BuildHandler(MigrateRepoCommandArgs args, IServiceProvider sp) { diff --git a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs index a704883cc..7c253ece1 100644 --- a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs +++ b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs @@ -17,5 +17,6 @@ public class MigrateRepoCommandArgs : CommandArgs [Secret] public string GithubPat { get; set; } public string TargetApiUrl { get; set; } + public bool CleanStatusChecks { get; set; } } } diff --git a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs index a1a15d101..8585430b2 100644 --- a/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs +++ b/src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandHandler.cs @@ -1,5 +1,7 @@ using System; +using System.Net.Http; using System.Threading.Tasks; +using Newtonsoft.Json.Linq; using OctoshiftCLI.Commands; using OctoshiftCLI.Extensions; using OctoshiftCLI.Services; @@ -95,6 +97,12 @@ public async Task Handle(MigrateRepoCommandArgs args) _log.LogSuccess($"Migration completed (ID: {migrationId})! State: {migrationState}"); _warningsCountLogger.LogWarningsCount(warningsCount); _log.LogInformation(migrationLogAvailableMessage); + + // Clean status checks if requested + if (args.CleanStatusChecks) + { + await CleanStatusChecksFromBranchProtection(args.GithubOrg, args.GithubRepo); + } } private string GetAdoRepoUrl(string org, string project, string repo, string serverUrl) @@ -102,4 +110,110 @@ private string GetAdoRepoUrl(string org, string project, string repo, string ser serverUrl = serverUrl.HasValue() ? serverUrl.TrimEnd('/') : "https://dev.azure.com"; return $"{serverUrl}/{org.EscapeDataString()}/{project.EscapeDataString()}/_git/{repo.EscapeDataString()}"; } + + private async Task CleanStatusChecksFromBranchProtection(string org, string repo) + { + try + { + _log.LogInformation("Cleaning status checks from default branch protection..."); + + // Get only the default branch to optimize performance + var defaultBranch = await _githubApi.GetDefaultBranch(org, repo); + + if (string.IsNullOrEmpty(defaultBranch)) + { + _log.LogInformation("No default branch found, skipping status check cleanup"); + return; + } + + _log.LogInformation($"Processing default branch: {defaultBranch}"); + + try + { + var protection = await _githubApi.GetBranchProtection(org, repo, defaultBranch); + + if (protection != null) + { + await CleanStatusChecksFromProtection(org, repo, defaultBranch, protection); + } + else + { + _log.LogInformation($"No branch protection found for default branch '{defaultBranch}', skipping"); + } + } + catch (HttpRequestException ex) + { + _log.LogWarning($"Failed to process branch protection for default branch '{defaultBranch}': {ex.Message}"); + } + catch (OctoshiftCliException ex) + { + _log.LogWarning($"Failed to process branch protection for default branch '{defaultBranch}': {ex.Message}"); + } + + _log.LogSuccess("Successfully cleaned status checks from default branch protection"); + } + catch (HttpRequestException ex) + { + _log.LogWarning($"Failed to clean status checks: {ex.Message}"); + } + catch (OctoshiftCliException ex) + { + _log.LogWarning($"Failed to clean status checks: {ex.Message}"); + } + } + + private async Task CleanStatusChecksFromProtection(string org, string repo, string branch, JObject protection) + { + // Check if required_status_checks exists and clean the check list while keeping the requirement enabled + if (protection["required_status_checks"] != null && protection["required_status_checks"].Type != JTokenType.Null) + { + _log.LogInformation($"Cleaning status checks for branch '{branch}'"); + + // Keep "Require status checks to pass before merging" enabled but clean the status check list + // GitHub API requires contexts array to be present (can be empty) + var statusChecksSettings = new + { + strict = ExtractBooleanValue(protection["required_status_checks"]?["strict"]) || true, // Default to true if not set + contexts = Array.Empty() // Empty array as per GitHub API documentation examples + }; + + // Create a proper update payload that conforms to GitHub API schema + var updatePayload = new + { + required_status_checks = statusChecksSettings, // Keep enabled but with no specific checks + enforce_admins = ExtractBooleanValue(protection["enforce_admins"]), + required_pull_request_reviews = ExtractPullRequestReviewsSettings(protection["required_pull_request_reviews"]), + restrictions = ExtractRestrictionsSettings(protection["restrictions"]), + required_linear_history = ExtractBooleanValue(protection["required_linear_history"]), + allow_force_pushes = ExtractBooleanValue(protection["allow_force_pushes"]), + allow_deletions = ExtractBooleanValue(protection["allow_deletions"]), + block_creations = ExtractBooleanValue(protection["block_creations"]), + required_conversation_resolution = ExtractBooleanValue(protection["required_conversation_resolution"]), + lock_branch = ExtractBooleanValue(protection["lock_branch"]), + allow_fork_syncing = ExtractBooleanValue(protection["allow_fork_syncing"]) + }; + + await _githubApi.UpdateBranchProtection(org, repo, branch, updatePayload); + } + else + { + _log.LogInformation($"No status checks found for branch '{branch}', skipping"); + } + } + + private static bool ExtractBooleanValue(JToken token) => + token switch + { + null => false, + { Type: JTokenType.Null } => false, + { Type: JTokenType.Boolean } => (bool)token, + { Type: JTokenType.Object } => token["enabled"]?.Value() ?? false, + _ => false + }; + + private static object ExtractPullRequestReviewsSettings(JToken token) => + token?.Type == JTokenType.Null ? null : token?.DeepClone(); + + private static object ExtractRestrictionsSettings(JToken token) => + token?.Type == JTokenType.Null ? null : token?.DeepClone(); }