diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 35e95e0e0..2199f5f22 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,2 +1,5 @@ -- Added validation to detect and return clear error messages when a URL is provided instead of a name for organization, repository, or enterprise arguments (e.g., `--github-org`, `--github-target-org`, `--source-repo`, `--github-target-enterprise`) -- Added `--target-api-url` as an optional arg to the `add-team-to-repo` command +- **ado2gh**: Improved `rewire-pipeline` command to gracefully handle disabled repositories and pipelines with clear warnings instead of errors +- **ado2gh**: Fixed 404 errors when checking branch policies for classic pipelines with prefixed repository names +- **ado2gh**: Fixed 400 errors when running `rewire-pipeline --dry-run` on disabled pipelines +- **ado2gh**: Fixed misleading success messages when pipeline rewiring was skipped +- **ado2gh**: Fixed monitor timeout minutes to only display when --dry-run mode is enabled, reducing confusion during regular pipeline rewiring operations diff --git a/src/Octoshift/Commands/CommandArgs.cs b/src/Octoshift/Commands/CommandArgs.cs index a647ca2c8..34f8a4542 100644 --- a/src/Octoshift/Commands/CommandArgs.cs +++ b/src/Octoshift/Commands/CommandArgs.cs @@ -13,7 +13,7 @@ public abstract class CommandArgs public virtual void Validate(OctoLogger log) { } - public void Log(OctoLogger log) + public virtual void Log(OctoLogger log) { if (log is null) { diff --git a/src/Octoshift/Services/AdoApi.cs b/src/Octoshift/Services/AdoApi.cs index bd666daa9..a731e16a2 100644 --- a/src/Octoshift/Services/AdoApi.cs +++ b/src/Octoshift/Services/AdoApi.cs @@ -562,6 +562,18 @@ public virtual async Task ShareServiceConnection(string adoOrg, string adoTeamPr return (defaultBranch, clean, checkoutSubmodules, triggers); } + public virtual async Task IsPipelineEnabled(string org, string teamProject, int pipelineId) + { + var url = $"{_adoBaseUrl}/{org.EscapeDataString()}/{teamProject.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + + var response = await _client.GetAsync(url); + var data = JObject.Parse(response); + + // Check the queueStatus field - it can be "enabled", "disabled", or "paused" + var queueStatus = (string)data["queueStatus"]; + return string.IsNullOrEmpty(queueStatus) || queueStatus.Equals("enabled", StringComparison.OrdinalIgnoreCase); + } + public virtual async Task GetBoardsGithubRepoId(string org, string teamProject, string teamProjectId, string endpointId, string githubOrg, string githubRepo) { var url = $"{_adoBaseUrl}/{org.EscapeDataString()}/_apis/Contribution/HierarchyQuery?api-version=5.0-preview.1"; diff --git a/src/Octoshift/Services/AdoPipelineTriggerService.cs b/src/Octoshift/Services/AdoPipelineTriggerService.cs index 7f8c87b68..5a419a5c1 100644 --- a/src/Octoshift/Services/AdoPipelineTriggerService.cs +++ b/src/Octoshift/Services/AdoPipelineTriggerService.cs @@ -22,7 +22,7 @@ public class AdoPipelineTriggerService private readonly string _adoBaseUrl; // Cache for repository IDs and branch policies to avoid redundant API calls - private readonly Dictionary _repositoryIdCache = []; + private readonly Dictionary _repositoryCache = []; private readonly Dictionary _branchPolicyCache = []; public AdoPipelineTriggerService(AdoApi adoApi, OctoLogger log, string adoBaseUrl) @@ -36,7 +36,8 @@ public AdoPipelineTriggerService(AdoApi adoApi, OctoLogger log, string adoBaseUr /// Changes a pipeline's repository configuration from ADO to GitHub, applying /// trigger configuration based on branch policy requirements and existing settings. /// - public virtual async Task RewirePipelineToGitHub( + /// True if the pipeline was successfully rewired, false if it was skipped. + public virtual async Task RewirePipelineToGitHub( string adoOrg, string teamProject, int pipelineId, @@ -49,6 +50,9 @@ public virtual async Task RewirePipelineToGitHub( JToken originalTriggers = null, string targetApiUrl = null) { + ArgumentNullException.ThrowIfNull(adoOrg); + ArgumentNullException.ThrowIfNull(teamProject); + var url = $"{_adoBaseUrl}/{adoOrg.EscapeDataString()}/{teamProject.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; try @@ -56,113 +60,141 @@ public virtual async Task RewirePipelineToGitHub( var response = await _adoApi.GetAsync(url); var data = JObject.Parse(response); - var newRepo = CreateGitHubRepositoryConfiguration(githubOrg, githubRepo, defaultBranch, clean, checkoutSubmodules, connectedServiceId, targetApiUrl); var currentRepoName = data["repository"]?["name"]?.ToString(); - var isPipelineRequiredByBranchPolicy = await IsPipelineRequiredByBranchPolicy(adoOrg, teamProject, currentRepoName, pipelineId); + var currentRepoId = data["repository"]?["id"]?.ToString(); + + var newRepo = CreateGitHubRepositoryConfiguration(githubOrg, githubRepo, defaultBranch, clean, checkoutSubmodules, connectedServiceId, targetApiUrl); + var isPipelineRequiredByBranchPolicy = await IsPipelineRequiredByBranchPolicy(adoOrg, teamProject, currentRepoName, currentRepoId, pipelineId); LogBranchPolicyCheckResults(pipelineId, isPipelineRequiredByBranchPolicy); var payload = BuildPipelinePayload(data, newRepo, originalTriggers, isPipelineRequiredByBranchPolicy); await _adoApi.PutAsync(url, payload.ToObject(typeof(object))); + return true; } catch (HttpRequestException ex) when (ex.Message.Contains("404")) { // Pipeline not found - log warning and skip _log.LogWarning($"Pipeline {pipelineId} not found in {adoOrg}/{teamProject}. Skipping pipeline rewiring."); - return; + return false; } catch (HttpRequestException ex) { // Other HTTP errors during pipeline retrieval _log.LogWarning($"HTTP error retrieving pipeline {pipelineId} in {adoOrg}/{teamProject}: {ex.Message}. Skipping pipeline rewiring."); - return; + return false; } } /// /// Analyzes branch policies to determine if a pipeline is required for branch protection. /// - public async Task IsPipelineRequiredByBranchPolicy(string adoOrg, string teamProject, string repoName, int pipelineId) + public async Task IsPipelineRequiredByBranchPolicy(string adoOrg, string teamProject, string repoName, string repoId, int pipelineId) { ArgumentNullException.ThrowIfNull(adoOrg); ArgumentNullException.ThrowIfNull(teamProject); - if (string.IsNullOrEmpty(repoName)) + if (string.IsNullOrEmpty(repoName) && string.IsNullOrEmpty(repoId)) { - _log.LogWarning($"Branch policy check skipped for pipeline {pipelineId} - repository name not available. Pipeline trigger configuration may not preserve branch policy requirements."); + _log.LogWarning($"Branch policy check skipped for pipeline {pipelineId} - repository name and ID not available. Pipeline trigger configuration may not preserve branch policy requirements."); return false; } try { - // Get repository information first (with caching) - var repositoryId = await GetRepositoryIdWithCache(adoOrg, teamProject, repoName); + var (repositoryId, isRepositoryDisabled) = await GetRepositoryIdAndStatus(adoOrg, teamProject, repoName, repoId, pipelineId); if (string.IsNullOrEmpty(repositoryId)) { - _log.LogWarning($"Repository ID not found for {adoOrg}/{teamProject}/{repoName}. Branch policy check cannot be performed for pipeline {pipelineId}."); return false; } - // Get branch policies for the repository (with caching) - var policyData = await GetBranchPoliciesWithCache(adoOrg, teamProject, repositoryId); - - if (policyData?.Value == null || policyData.Value.Count == 0) + if (isRepositoryDisabled) { - _log.LogVerbose($"No branch policies found for repository {adoOrg}/{teamProject}/{repoName}. ADO Pipeline ID = {pipelineId} is not required by branch policy."); + var repoIdentifier = repoName ?? repoId; + _log.LogInformation($"Repository {adoOrg}/{teamProject}/{repoIdentifier} is disabled. Branch policy check skipped for pipeline {pipelineId} - will use default trigger configuration."); return false; } - // Look for enabled build validation policies that reference our pipeline - var isPipelineRequired = policyData.Value.Any(policy => - policy.Type?.DisplayName == "Build" && - policy.IsEnabled && - policy.Settings?.BuildDefinitionId == pipelineId.ToString()); - - if (isPipelineRequired) - { - _log.LogVerbose($"ADO Pipeline ID = {pipelineId} is required by branch policy in {adoOrg}/{teamProject}/{repoName}. Build status reporting will be enabled to support branch protection."); - } - else - { - _log.LogVerbose($"ADO Pipeline ID = {pipelineId} is not required by any branch policies in {adoOrg}/{teamProject}/{repoName}."); - } - - return isPipelineRequired; + return await CheckBranchPoliciesForPipeline(adoOrg, teamProject, repositoryId, repoName, repoId, pipelineId); } - catch (HttpRequestException ex) + catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException or JsonException or ArgumentException or InvalidOperationException) { - // If we can't determine branch policy status due to network issues, default to false - _log.LogWarning($"HTTP error during branch policy check for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoName}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); + LogBranchPolicyCheckError(ex, adoOrg, teamProject, repoName, repoId, pipelineId); return false; } - catch (TaskCanceledException ex) + } + + private async Task<(string repositoryId, bool isDisabled)> GetRepositoryIdAndStatus(string adoOrg, string teamProject, string repoName, string repoId, int pipelineId) + { + if (!string.IsNullOrEmpty(repoId)) { - // If branch policy checking times out, consider check failed - _log.LogWarning($"Branch policy check timed out for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoName}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); - return false; + _log.LogVerbose($"Using repository ID from pipeline definition for branch policy check: {repoId}"); + var (_, disabled) = await GetRepositoryInfoWithCache(adoOrg, teamProject, repoId, repoName); + return (repoId, disabled); } - catch (JsonException ex) + + var (id, disabledStatus) = await GetRepositoryInfoWithCache(adoOrg, teamProject, null, repoName); + if (string.IsNullOrEmpty(id)) { - // If we can't determine branch policy status due to JSON parsing issues, default to false - _log.LogWarning($"JSON parsing error during branch policy check for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoName}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); - return false; + _log.LogWarning($"Repository ID not found for {adoOrg}/{teamProject}/{repoName}. Branch policy check cannot be performed for pipeline {pipelineId}."); + return (null, false); } - catch (ArgumentException ex) + + return (id, disabledStatus); + } + + private async Task CheckBranchPoliciesForPipeline(string adoOrg, string teamProject, string repositoryId, string repoName, string repoId, int pipelineId) + { + var policyData = await GetBranchPoliciesWithCache(adoOrg, teamProject, repositoryId); + + if (policyData?.Value == null || policyData.Value.Count == 0) { - // If we can't determine branch policy status due to invalid arguments, default to false - _log.LogWarning($"Invalid argument error during branch policy check for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoName}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); + var repoIdentifier = repoName ?? repoId ?? "unknown"; + _log.LogVerbose($"No branch policies found for repository {adoOrg}/{teamProject}/{repoIdentifier}. ADO Pipeline ID = {pipelineId} is not required by branch policy."); return false; } - catch (InvalidOperationException ex) + + var isPipelineRequired = policyData.Value.Any(policy => + policy.Type?.DisplayName == "Build" && + policy.IsEnabled && + policy.Settings?.BuildDefinitionId == pipelineId.ToString()); + + LogBranchPolicyCheckResult(isPipelineRequired, adoOrg, teamProject, repoName, repoId, pipelineId); + return isPipelineRequired; + } + + private void LogBranchPolicyCheckResult(bool isPipelineRequired, string adoOrg, string teamProject, string repoName, string repoId, int pipelineId) + { + var repoIdentifier = repoName ?? repoId ?? "unknown"; + + if (isPipelineRequired) { - // If branch policy checking fails due to invalid state, consider check failed - _log.LogWarning($"Invalid operation error during branch policy check for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoName}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); - return false; + _log.LogVerbose($"ADO Pipeline ID = {pipelineId} is required by branch policy in {adoOrg}/{teamProject}/{repoIdentifier}. Build status reporting will be enabled to support branch protection."); + } + else + { + _log.LogVerbose($"ADO Pipeline ID = {pipelineId} is not required by any branch policies in {adoOrg}/{teamProject}/{repoIdentifier}."); } } + private void LogBranchPolicyCheckError(Exception ex, string adoOrg, string teamProject, string repoName, string repoId, int pipelineId) + { + var repoIdentifier = repoName ?? repoId ?? "unknown"; + var errorType = ex switch + { + HttpRequestException => "HTTP error", + TaskCanceledException => "Branch policy check timed out", + JsonException => "JSON parsing error", + ArgumentException => "Invalid argument error", + InvalidOperationException => "Invalid operation error", + _ => "Error" + }; + + _log.LogWarning($"{errorType} during branch policy check for pipeline {pipelineId} in {adoOrg}/{teamProject}/{repoIdentifier}: {ex.Message}. Pipeline trigger configuration may not preserve branch policy requirements."); + } + #region Private Helper Methods - Trigger Configuration Logic private void LogBranchPolicyCheckResults(int pipelineId, bool isPipelineRequiredByBranchPolicy) @@ -451,51 +483,66 @@ private bool HasTriggerType(JToken originalTriggers, string triggerType) #region Private Helper Methods - Caching /// - /// Gets the repository ID with caching to avoid redundant API calls for the same repository. + /// Gets the repository information (ID and disabled status) with caching to avoid redundant API calls for the same repository. /// - private async Task GetRepositoryIdWithCache(string adoOrg, string teamProject, string repoName) + private async Task<(string id, bool isDisabled)> GetRepositoryInfoWithCache(string adoOrg, string teamProject, string repoId, string repoName) { - var cacheKey = $"{adoOrg.ToUpper()}/{teamProject.ToUpper()}/{repoName.ToUpper()}"; + var identifier = !string.IsNullOrEmpty(repoId) ? repoId : repoName; + var cacheKey = $"{adoOrg.ToUpper()}/{teamProject.ToUpper()}/{identifier.ToUpper()}"; - if (_repositoryIdCache.TryGetValue(cacheKey, out var cachedId)) + if (_repositoryCache.TryGetValue(cacheKey, out var cachedInfo)) { - _log.LogVerbose($"Using cached repository ID for {adoOrg}/{teamProject}/{repoName}"); - return cachedId; + _log.LogVerbose($"Using cached repository information for {adoOrg}/{teamProject}/{identifier}"); + return cachedInfo; } - _log.LogVerbose($"Fetching repository ID for {adoOrg}/{teamProject}/{repoName}"); + _log.LogVerbose($"Fetching repository information for {adoOrg}/{teamProject}/{identifier}"); try { - var repoUrl = $"{_adoBaseUrl}/{adoOrg.EscapeDataString()}/{teamProject.EscapeDataString()}/_apis/git/repositories/{repoName.EscapeDataString()}?api-version=6.0"; + var repoUrl = $"{_adoBaseUrl}/{adoOrg.EscapeDataString()}/{teamProject.EscapeDataString()}/_apis/git/repositories/{identifier.EscapeDataString()}?api-version=6.0"; var repoResponse = await _adoApi.GetAsync(repoUrl); var repoData = JObject.Parse(repoResponse); var repositoryId = repoData["id"]?.ToString(); + var isDisabled = repoData["isDisabled"]?.ToString().Equals("true", StringComparison.OrdinalIgnoreCase) ?? false; if (!string.IsNullOrEmpty(repositoryId)) { - _repositoryIdCache[cacheKey] = repositoryId; - _log.LogVerbose($"Cached repository ID {repositoryId} for {adoOrg}/{teamProject}/{repoName}"); + var info = (repositoryId, isDisabled); + _repositoryCache[cacheKey] = info; + _log.LogVerbose($"Cached repository information (ID: {repositoryId}, Disabled: {isDisabled}) for {adoOrg}/{teamProject}/{identifier}"); + return info; } - return repositoryId; + return (null, false); + } + catch (HttpRequestException ex) when (ex.Message.Contains("404")) + { + // 404 typically means the repository is disabled or doesn't exist + // Treat it as disabled to avoid further API calls + // Log as verbose since the caller will log a more specific warning about the disabled repository + // Return (null, true) to indicate repository ID is unknown but repository is disabled + _log.LogVerbose($"Repository {adoOrg}/{teamProject}/{identifier} returned 404 - likely disabled or not found."); + (string id, bool isDisabled) info = (null, true); // Mark as disabled with null ID since identifier may be a name + _repositoryCache[cacheKey] = info; + return info; } catch (HttpRequestException ex) { // Don't cache failed requests - let the caller handle the error - _log.LogVerbose($"Failed to fetch repository ID for {adoOrg}/{teamProject}/{repoName}: {ex.Message}"); + _log.LogVerbose($"Failed to fetch repository information for {adoOrg}/{teamProject}/{identifier}: {ex.Message}"); throw; } catch (TaskCanceledException ex) { // Don't cache timeouts - let the caller handle the error - _log.LogVerbose($"Timeout fetching repository ID for {adoOrg}/{teamProject}/{repoName}: {ex.Message}"); + _log.LogVerbose($"Timeout fetching repository information for {adoOrg}/{teamProject}/{identifier}: {ex.Message}"); throw; } catch (JsonException ex) { // Don't cache JSON parsing errors - let the caller handle the error - _log.LogVerbose($"JSON parsing error for repository {adoOrg}/{teamProject}/{repoName}: {ex.Message}"); + _log.LogVerbose($"JSON parsing error for repository {adoOrg}/{teamProject}/{identifier}: {ex.Message}"); throw; } } diff --git a/src/Octoshift/Services/PipelineTestService.cs b/src/Octoshift/Services/PipelineTestService.cs index e67f24f93..1910ba57e 100644 --- a/src/Octoshift/Services/PipelineTestService.cs +++ b/src/Octoshift/Services/PipelineTestService.cs @@ -59,6 +59,16 @@ public async Task TestPipeline(PipelineTestArgs args) args.PipelineId = pipelineId; } + // Check if pipeline is disabled before attempting to queue a build + var isEnabled = await _adoApi.IsPipelineEnabled(args.AdoOrg, args.AdoTeamProject, args.PipelineId.Value); + if (!isEnabled) + { + _log.LogWarning($"Pipeline '{args.PipelineName}' (ID: {args.PipelineId.Value}) is disabled. Skipping pipeline test."); + testResult.ErrorMessage = "Pipeline is disabled"; + testResult.EndTime = DateTime.UtcNow; + return testResult; + } + // Get original repository information for restoration (originalRepoName, _, originalDefaultBranch, originalClean, originalCheckoutSubmodules) = await _adoApi.GetPipelineRepository(args.AdoOrg, args.AdoTeamProject, args.PipelineId.Value); diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs index 99308932f..f343ab795 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoApiTests.cs @@ -985,6 +985,81 @@ public async Task GetPipeline_Should_Return_Pipeline() Triggers.Should().NotBeNull(); } + [Fact] + public async Task IsPipelineEnabled_Should_Return_True_For_Enabled_Pipeline() + { + var pipelineId = 826263; + + var endpoint = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{ADO_TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + var response = new + { + id = pipelineId, + queueStatus = "enabled" + }; + + _mockAdoClient.Setup(x => x.GetAsync(endpoint).Result).Returns(response.ToJson()); + + var result = await sut.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, pipelineId); + + result.Should().BeTrue(); + } + + [Fact] + public async Task IsPipelineEnabled_Should_Return_False_For_Disabled_Pipeline() + { + var pipelineId = 826263; + + var endpoint = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{ADO_TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + var response = new + { + id = pipelineId, + queueStatus = "disabled" + }; + + _mockAdoClient.Setup(x => x.GetAsync(endpoint).Result).Returns(response.ToJson()); + + var result = await sut.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, pipelineId); + + result.Should().BeFalse(); + } + + [Fact] + public async Task IsPipelineEnabled_Should_Return_False_For_Paused_Pipeline() + { + var pipelineId = 826263; + + var endpoint = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{ADO_TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + var response = new + { + id = pipelineId, + queueStatus = "paused" + }; + + _mockAdoClient.Setup(x => x.GetAsync(endpoint).Result).Returns(response.ToJson()); + + var result = await sut.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, pipelineId); + + result.Should().BeFalse(); + } + + [Fact] + public async Task IsPipelineEnabled_Should_Return_True_For_Missing_QueueStatus() + { + var pipelineId = 826263; + + var endpoint = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{ADO_TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + var response = new + { + id = pipelineId + }; + + _mockAdoClient.Setup(x => x.GetAsync(endpoint).Result).Returns(response.ToJson()); + + var result = await sut.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, pipelineId); + + result.Should().BeTrue(); + } + [Fact] public async Task GetBoardsGithubRepoId_Should_Return_RepoId() { diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_BranchPolicyTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_BranchPolicyTests.cs index 7b789d188..0800b3150 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_BranchPolicyTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_BranchPolicyTests.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Net.Http; using System.Threading.Tasks; using FluentAssertions; using Moq; @@ -36,7 +37,8 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_True_When_Pipel var repoResponse = new { id = repositoryId, - name = REPO_NAME + name = REPO_NAME, + isDisabled = "false" }.ToJson(); var policyResponse = new @@ -70,12 +72,67 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_True_When_Pipel _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policyResponse); // Act - var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, PIPELINE_ID); + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); // Assert result.Should().BeTrue(); } + [Fact] + public async Task IsPipelineRequiredByBranchPolicy_Should_Use_Repository_Id_Directly_When_Provided() + { + // Arrange + var repositoryId = Guid.NewGuid().ToString(); + + var repoResponse = new + { + id = repositoryId, + name = REPO_NAME, + isDisabled = "false" + }.ToJson(); + + var policyResponse = new + { + count = 1, + value = new[] + { + new + { + id = 1, + type = new + { + id = "0609b952-1397-4640-95ec-e00a01b2c241", + displayName = "Build" + }, + isEnabled = true, + settings = new + { + buildDefinitionId = PIPELINE_ID, + displayName = PIPELINE_NAME, + validDuration = 0 + } + } + } + }.ToJson(); + + // When repository ID is provided, we still need to fetch repo details to check if it's disabled + var repoByIdUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{repositoryId.EscapeDataString()}?api-version=6.0"; + var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; + + _mockAdoApi.Setup(m => m.GetAsync(repoByIdUrl)).ReturnsAsync(repoResponse); + _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policyResponse); + + // Act - Pass repository ID directly instead of name + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, repositoryId, PIPELINE_ID); + + // Assert + result.Should().BeTrue(); + + // Verify that repository lookup by name was NOT called since we provided the ID + var repoByNameUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; + _mockAdoApi.Verify(m => m.GetAsync(repoByNameUrl), Times.Never); + } + [Fact] public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Pipeline_Not_In_Policy() { @@ -84,7 +141,8 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Pipe var repoResponse = new { id = repositoryId, - name = REPO_NAME + name = REPO_NAME, + isDisabled = "false" }.ToJson(); var policyResponse = new @@ -118,7 +176,7 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Pipe _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policyResponse); // Act - var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, PIPELINE_ID); + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); // Assert result.Should().BeFalse(); @@ -132,7 +190,8 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Poli var repoResponse = new { id = repositoryId, - name = REPO_NAME + name = REPO_NAME, + isDisabled = "false" }.ToJson(); var policyResponse = new @@ -166,7 +225,7 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Poli _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policyResponse); // Act - var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, PIPELINE_ID); + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); // Assert result.Should().BeFalse(); @@ -180,7 +239,8 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_No_B var repoResponse = new { id = repositoryId, - name = REPO_NAME + name = REPO_NAME, + isDisabled = "false" }.ToJson(); var policyResponse = new @@ -196,10 +256,56 @@ public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_No_B _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policyResponse); // Act - var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, PIPELINE_ID); + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); + + // Assert + result.Should().BeFalse(); + } + + [Fact] + public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Repository_Is_Disabled() + { + // Arrange + var repositoryId = Guid.NewGuid().ToString(); + var repoResponse = new + { + id = repositoryId, + name = REPO_NAME, + isDisabled = "true" + }.ToJson(); + + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; + + _mockAdoApi.Setup(m => m.GetAsync(repoUrl)).ReturnsAsync(repoResponse); + + // Act + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); + + // Assert + result.Should().BeFalse(); + + // Verify that branch policy check was NOT performed since repository is disabled + var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; + _mockAdoApi.Verify(m => m.GetAsync(policyUrl), Times.Never); + } + + [Fact] + public async Task IsPipelineRequiredByBranchPolicy_Should_Return_False_When_Repository_Returns_404() + { + // Arrange - Disabled repositories often return 404 when queried directly + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; + + _mockAdoApi.Setup(m => m.GetAsync(repoUrl)) + .ThrowsAsync(new HttpRequestException("Response status code does not indicate success: 404 (Not Found).")); + + // Act + var result = await _triggerService.IsPipelineRequiredByBranchPolicy(ADO_ORG, TEAM_PROJECT, REPO_NAME, null, PIPELINE_ID); // Assert result.Should().BeFalse(); + + // Verify that branch policy check was NOT performed since repository returned 404 + _mockAdoApi.Verify(m => m.GetAsync(It.Is(url => url.Contains("policy/configurations"))), Times.Never); } [Fact] @@ -235,8 +341,8 @@ public async Task RewirePipelineToGitHub_WhenPipelineNotRequiredByBranchPolicy_S // Mock repository lookup - return valid repository var repositoryId = "repo-123"; - var repoResponse = new { id = repositoryId, name = REPO_NAME }.ToJson(); - var repoUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; + var repoResponse = new { id = repositoryId, name = REPO_NAME, isDisabled = "false" }.ToJson(); + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; // Mock branch policies - return empty policies (not required by branch policy) var policies = new @@ -244,7 +350,7 @@ public async Task RewirePipelineToGitHub_WhenPipelineNotRequiredByBranchPolicy_S count = 0, value = Array.Empty() }.ToJson(); - var policyUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; + var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; @@ -253,9 +359,10 @@ public async Task RewirePipelineToGitHub_WhenPipelineNotRequiredByBranchPolicy_S _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policies); // Act - await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, originalTriggers, null); + var result = await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, originalTriggers, null); // Assert - Should preserve original triggers (both CI and PR, with build status reporting) + result.Should().BeTrue(); _mockAdoApi.Verify(m => m.PutAsync(pipelineUrl, It.Is(payload => VerifyTriggersPreserved(payload, true, true) )), Times.Once); @@ -282,8 +389,8 @@ public async Task RewirePipelineToGitHub_WhenPipelineRequiredByBranchPolicy_Shou // Mock repository lookup - return valid repository var repositoryId = "repo-123"; - var repoResponse = new { id = repositoryId, name = REPO_NAME }.ToJson(); - var repoUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; + var repoResponse = new { id = repositoryId, name = REPO_NAME, isDisabled = "false" }.ToJson(); + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; // Mock branch policies - return policy that requires this pipeline var policies = new @@ -300,7 +407,7 @@ public async Task RewirePipelineToGitHub_WhenPipelineRequiredByBranchPolicy_Shou } } }.ToJson(); - var policyUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; + var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; @@ -309,14 +416,108 @@ public async Task RewirePipelineToGitHub_WhenPipelineRequiredByBranchPolicy_Shou _mockAdoApi.Setup(m => m.GetAsync(policyUrl)).ReturnsAsync(policies); // Act - await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, null, null); + var result = await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, null, null); // Assert - Should enable both CI and PR triggers WITH build status reporting + result.Should().BeTrue(); _mockAdoApi.Verify(m => m.PutAsync(pipelineUrl, It.Is(payload => VerifyTriggersPreserved(payload, true, true) )), Times.Once); } + [Fact] + public async Task RewirePipelineToGitHub_Should_Skip_When_Repository_Is_Disabled() + { + // Arrange + var githubRepo = "test-repo"; + var serviceConnectionId = Guid.NewGuid().ToString(); + var defaultBranch = "main"; + var pipelineId = PIPELINE_ID; + var clean = "true"; + var checkoutSubmodules = "false"; + var repositoryId = Guid.NewGuid().ToString(); + + // Mock existing pipeline with disabled repository + var existingPipelineData = new + { + name = PIPELINE_NAME, + repository = new { name = REPO_NAME, id = repositoryId }, + someOtherProperty = "value" + }; + + // Mock repository lookup - return disabled repository + var repoResponse = new + { + id = repositoryId, + name = REPO_NAME, + isDisabled = "true" + }.ToJson(); + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{repositoryId.EscapeDataString()}?api-version=6.0"; + + var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + + _mockAdoApi.Setup(m => m.GetAsync(pipelineUrl)).ReturnsAsync(existingPipelineData.ToJson()); + _mockAdoApi.Setup(m => m.GetAsync(repoUrl)).ReturnsAsync(repoResponse); + + // Act + var result = await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, null, null); + + // Assert - Should succeed and call PutAsync even though repository is disabled + result.Should().BeTrue(); + _mockAdoApi.Verify(m => m.PutAsync(It.IsAny(), It.IsAny()), Times.Once); + + // Verify info message was logged about skipping branch policy check + _mockOctoLogger.Verify(m => m.LogInformation(It.Is(s => + s.Contains("disabled") && + s.Contains("Branch policy check skipped") && + s.Contains(pipelineId.ToString()) + )), Times.Once); + } + + [Fact] + public async Task RewirePipelineToGitHub_Should_Skip_When_Repository_Returns_404() + { + // Arrange + var githubRepo = "test-repo"; + var serviceConnectionId = Guid.NewGuid().ToString(); + var defaultBranch = "main"; + var pipelineId = PIPELINE_ID; + var clean = "true"; + var checkoutSubmodules = "false"; + var repositoryId = Guid.NewGuid().ToString(); + + // Mock existing pipeline + var existingPipelineData = new + { + name = PIPELINE_NAME, + repository = new { name = REPO_NAME, id = repositoryId }, + someOtherProperty = "value" + }; + + // Mock repository lookup - return 404 (likely disabled) + var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{repositoryId.EscapeDataString()}?api-version=6.0"; + + var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{pipelineId}?api-version=6.0"; + + _mockAdoApi.Setup(m => m.GetAsync(pipelineUrl)).ReturnsAsync(existingPipelineData.ToJson()); + _mockAdoApi.Setup(m => m.GetAsync(repoUrl)) + .ThrowsAsync(new HttpRequestException("Response status code does not indicate success: 404 (Not Found).")); + + // Act + var result = await _triggerService.RewirePipelineToGitHub(ADO_ORG, TEAM_PROJECT, pipelineId, defaultBranch, clean, checkoutSubmodules, "github-org", githubRepo, serviceConnectionId, null, null); + + // Assert - Should succeed and call PutAsync even though repository returned 404 + result.Should().BeTrue(); + _mockAdoApi.Verify(m => m.PutAsync(It.IsAny(), It.IsAny()), Times.Once); + + // Verify info message was logged about skipping branch policy check (repo treated as disabled) + _mockOctoLogger.Verify(m => m.LogInformation(It.Is(s => + s.Contains("disabled") && + s.Contains("Branch policy check skipped") && + s.Contains(pipelineId.ToString()) + )), Times.Once); + } + private static bool VerifyTriggersPreserved(object payload, bool enablePullRequestValidation, bool enableBuildStatusReporting) { var json = payload.ToJson(); diff --git a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ErrorHandlingTests.cs b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ErrorHandlingTests.cs index c4d2ed6a0..654c24650 100644 --- a/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ErrorHandlingTests.cs +++ b/src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ErrorHandlingTests.cs @@ -45,11 +45,13 @@ public async Task RewirePipelineToGitHub_Should_Skip_When_Pipeline_Not_Found_404 _mockAdoApi.Setup(x => x.GetAsync(pipelineUrl)) .ThrowsAsync(new HttpRequestException("Response status code does not indicate success: 404 (Not Found).")); - // Act & Assert - Should not throw exception, should handle gracefully - await _triggerService.Invoking(x => x.RewirePipelineToGitHub( + // Act + var result = await _triggerService.RewirePipelineToGitHub( ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules, - githubOrg, githubRepo, serviceConnectionId, null, null)) - .Should().NotThrowAsync(); + githubOrg, githubRepo, serviceConnectionId, null, null); + + // Assert - Should return false indicating skipped + result.Should().BeFalse(); // Verify that warning was logged _mockOctoLogger.Verify(x => x.LogWarning(It.Is(s => @@ -77,11 +79,13 @@ public async Task RewirePipelineToGitHub_Should_Skip_When_Pipeline_HTTP_Error() _mockAdoApi.Setup(x => x.GetAsync(pipelineUrl)) .ThrowsAsync(new HttpRequestException("Response status code does not indicate success: 500 (Internal Server Error).")); - // Act & Assert - Should not throw exception, should handle gracefully - await _triggerService.Invoking(x => x.RewirePipelineToGitHub( + // Act + var result = await _triggerService.RewirePipelineToGitHub( ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules, - githubOrg, githubRepo, serviceConnectionId, null, null)) - .Should().NotThrowAsync(); + githubOrg, githubRepo, serviceConnectionId, null, null); + + // Assert - Should return false indicating skipped + result.Should().BeFalse(); // Verify that warning was logged _mockOctoLogger.Verify(x => x.LogWarning(It.Is(s => @@ -119,7 +123,7 @@ public async Task RewirePipelineToGitHub_Should_Continue_When_Pipeline_Found() // Mock repository lookup for branch policy check var repositoryId = "repo-123"; - var repoResponse = new { id = repositoryId, name = REPO_NAME }.ToJson(); + var repoResponse = new { id = repositoryId, name = REPO_NAME, isDisabled = "false" }.ToJson(); _mockAdoApi.Setup(x => x.GetAsync(repoUrl)) .ReturnsAsync(repoResponse); @@ -130,11 +134,14 @@ public async Task RewirePipelineToGitHub_Should_Continue_When_Pipeline_Found() .ReturnsAsync(policies); // Act - await _triggerService.RewirePipelineToGitHub( + var result = await _triggerService.RewirePipelineToGitHub( ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules, githubOrg, githubRepo, serviceConnectionId, null, null); - // Assert - Verify that PutAsync was called (pipeline was successfully rewired) + // Assert - Should return true indicating successful rewiring + result.Should().BeTrue(); + + // Verify that PutAsync was called (pipeline was successfully rewired) _mockAdoApi.Verify(x => x.PutAsync(pipelineUrl, It.IsAny()), Times.Once); // Verify that no error warnings were logged diff --git a/src/OctoshiftCLI.Tests/Services/PipelineTestServiceTests.cs b/src/OctoshiftCLI.Tests/Services/PipelineTestServiceTests.cs index 714549dd3..d5a87b3e5 100644 --- a/src/OctoshiftCLI.Tests/Services/PipelineTestServiceTests.cs +++ b/src/OctoshiftCLI.Tests/Services/PipelineTestServiceTests.cs @@ -65,6 +65,9 @@ public async Task TestPipeline_Should_Perform_Complete_Test_Workflow() var buildId = 456; var buildUrl = "https://dev.azure.com/build/456"; + _mockAdoApi.Setup(x => x.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) + .ReturnsAsync(true); + _mockAdoApi.Setup(x => x.GetPipelineRepository(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) .ReturnsAsync((originalRepoName, "repo-id", originalDefaultBranch, originalClean, originalCheckoutSubmodules)); @@ -126,6 +129,9 @@ public async Task TestPipeline_Should_Lookup_Pipeline_ID_When_Not_Provided() _mockAdoApi.Setup(x => x.GetPipelineId(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_NAME)) .ReturnsAsync(PIPELINE_ID); + _mockAdoApi.Setup(x => x.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) + .ReturnsAsync(true); + _mockAdoApi.Setup(x => x.GetPipelineRepository(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) .ReturnsAsync(("repo", "id", "refs/heads/main", "true", "false")); @@ -159,6 +165,9 @@ public async Task TestPipeline_Should_Handle_Restoration_Failure() MonitorTimeoutMinutes = 1 }; + _mockAdoApi.Setup(x => x.IsPipelineEnabled(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(true); + _mockAdoApi.Setup(x => x.GetPipelineRepository(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(("repo", "id", "refs/heads/main", "true", "false")); @@ -183,5 +192,45 @@ public async Task TestPipeline_Should_Handle_Restoration_Failure() result.RestoredSuccessfully.Should().BeFalse(); result.ErrorMessage.Should().Contain("Failed to restore: Restore failed"); } + + [Fact] + public async Task TestPipeline_Should_Skip_Test_For_Disabled_Pipeline() + { + // Arrange + var args = new PipelineTestArgs + { + AdoOrg = ADO_ORG, + AdoTeamProject = ADO_TEAM_PROJECT, + PipelineName = PIPELINE_NAME, + PipelineId = PIPELINE_ID, + GithubOrg = GITHUB_ORG, + GithubRepo = GITHUB_REPO, + ServiceConnectionId = SERVICE_CONNECTION_ID, + MonitorTimeoutMinutes = 1 + }; + + _mockAdoApi.Setup(x => x.IsPipelineEnabled(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) + .ReturnsAsync(false); + + // Act + var result = await _service.TestPipeline(args); + + // Assert + result.Should().NotBeNull(); + result.ErrorMessage.Should().Be("Pipeline is disabled"); + result.EndTime.Should().NotBeNull(); + + // Verify that no pipeline operations were attempted + _mockAdoApi.Verify(x => x.GetPipelineRepository(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockAdoApi.Verify(x => x.GetPipeline(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockPipelineTriggerService.Verify(x => x.RewirePipelineToGitHub(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockAdoApi.Verify(x => x.QueueBuild(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + _mockAdoApi.Verify(x => x.RestorePipelineToAdoRepo(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + _mockOctoLogger.Verify(x => x.LogWarning(It.Is(s => s.Contains("is disabled"))), Times.Once); + } } } diff --git a/src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs b/src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs index d03731df2..faf2a937b 100644 --- a/src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs +++ b/src/OctoshiftCLI.Tests/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler_ErrorHandlingTests.cs @@ -103,6 +103,11 @@ public async Task HandleRegularRewire_Should_Succeed_When_Pipeline_Found() _mockAdoApi.Setup(x => x.GetPipeline(ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID)) .ReturnsAsync((defaultBranch, clean, checkoutSubmodules, triggers)); + _mockAdoPipelineTriggerService.Setup(x => x.RewirePipelineToGitHub( + ADO_ORG, ADO_TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules, + GITHUB_ORG, GITHUB_REPO, SERVICE_CONNECTION_ID, triggers, null)) + .ReturnsAsync(true); + var args = new RewirePipelineCommandArgs { AdoOrg = ADO_ORG, diff --git a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs index ec0735664..8e7335096 100644 --- a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs +++ b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommand.cs @@ -27,6 +27,9 @@ public RewirePipelineCommand() : base( AddOption(TargetApiUrl); AddOption(DryRun); AddOption(MonitorTimeoutMinutes); + + // Set default value for MonitorTimeoutMinutes since System.CommandLine doesn't use property defaults + MonitorTimeoutMinutes.SetDefaultValue(30); } public Option AdoOrg { get; } = new("--ado-org") @@ -71,7 +74,7 @@ public RewirePipelineCommand() : base( }; public Option MonitorTimeoutMinutes { get; } = new("--monitor-timeout-minutes") { - Description = "Timeout in minutes for monitoring build completion in dry-run mode. Defaults to 30 minutes." + Description = "(Dry-run mode only) Timeout in minutes for monitoring build completion. Defaults to 30 minutes." }; public override RewirePipelineCommandHandler BuildHandler(RewirePipelineCommandArgs args, IServiceProvider sp) diff --git a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs index 0564227b3..677f81831 100644 --- a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs +++ b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandArgs.cs @@ -1,4 +1,7 @@ -using OctoshiftCLI.Commands; +using System; +using OctoshiftCLI.Commands; +using OctoshiftCLI.Extensions; +using OctoshiftCLI.Services; namespace OctoshiftCLI.AdoToGithub.Commands.RewirePipeline { @@ -16,5 +19,43 @@ public class RewirePipelineCommandArgs : CommandArgs public string TargetApiUrl { get; set; } public bool DryRun { get; set; } public int MonitorTimeoutMinutes { get; set; } = 30; + + public override void Log(OctoLogger log) + { + if (log is null) + { + throw new ArgumentNullException(nameof(log)); + } + + log.Verbose = Verbose; + + // Log all properties except MonitorTimeoutMinutes + log.LogInformation($"ADO ORG: {AdoOrg}"); + log.LogInformation($"ADO TEAM PROJECT: {AdoTeamProject}"); + + if (AdoPipeline.HasValue()) + { + log.LogInformation($"ADO PIPELINE: {AdoPipeline}"); + } + + if (AdoPipelineId.HasValue) + { + log.LogInformation($"ADO PIPELINE ID: {AdoPipelineId}"); + } + + log.LogInformation($"GITHUB ORG: {GithubOrg}"); + log.LogInformation($"GITHUB REPO: {GithubRepo}"); + log.LogInformation($"SERVICE CONNECTION ID: {ServiceConnectionId}"); + + if (TargetApiUrl.HasValue()) + { + log.LogInformation($"TARGET API URL: {TargetApiUrl}"); + } + + if (DryRun) + { + log.LogInformation($"DRY RUN: true"); + } + } } } diff --git a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs index 47ea3b121..92a76f754 100644 --- a/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs +++ b/src/ado2gh/Commands/RewirePipeline/RewirePipelineCommandHandler.cs @@ -52,6 +52,7 @@ public async Task Handle(RewirePipelineCommandArgs args) private async Task HandleDryRun(RewirePipelineCommandArgs args) { _log.LogInformation("Starting dry-run mode: Testing pipeline rewiring to GitHub..."); + _log.LogInformation($"Monitor timeout: {args.MonitorTimeoutMinutes} minutes"); var pipelineTestArgs = new PipelineTestArgs { @@ -103,7 +104,7 @@ private async Task HandleRegularRewire(RewirePipelineCommandArgs args) var (defaultBranch, clean, checkoutSubmodules, triggers) = await _adoApi.GetPipeline(args.AdoOrg, args.AdoTeamProject, adoPipelineId); // Use the specialized service for complex trigger logic - await _pipelineTriggerService.RewirePipelineToGitHub( + var rewired = await _pipelineTriggerService.RewirePipelineToGitHub( args.AdoOrg, args.AdoTeamProject, adoPipelineId, @@ -116,7 +117,10 @@ await _pipelineTriggerService.RewirePipelineToGitHub( triggers, args.TargetApiUrl); - _log.LogSuccess("Successfully rewired pipeline"); + if (rewired) + { + _log.LogSuccess("Successfully rewired pipeline"); + } } catch (HttpRequestException ex) when (ex.Message.Contains("404")) { @@ -126,7 +130,7 @@ await _pipelineTriggerService.RewirePipelineToGitHub( } catch (ArgumentException ex) when (ex.ParamName == "pipeline") { - // Pipeline lookup failed - log error and fail gracefully + // Pipeline lookup failed - log error and fail gracefully _log.LogError($"Pipeline lookup failed: {ex.Message}"); throw new OctoshiftCliException($"Unable to find the specified pipeline. Please verify the pipeline name and try again."); }