Skip to content

Commit 3042b1b

Browse files
timisoetantimisoetan
authored
Merge pull request #1526 from github/fix/classic-pipeline-rewire-support
Fix rewire-pipeline for Classic (Designer) pipelines
2 parents ecd3402 + 911b025 commit 3042b1b

3 files changed

Lines changed: 279 additions & 8 deletions

File tree

‎RELEASENOTES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- **ado2gh**: Fixed `rewire-pipeline` for Classic (Designer) pipelines by detecting the pipeline process type and using the correct settingsSourceType and trigger configuration.

‎src/Octoshift/Services/AdoPipelineTriggerService.cs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,15 @@ public virtual async Task<bool> RewirePipelineToGitHub(
6363
var currentRepoName = data["repository"]?["name"]?.ToString();
6464
var currentRepoId = data["repository"]?["id"]?.ToString();
6565

66+
// Detect pipeline process type: 1 = Classic/Designer, 2 = YAML
67+
var processType = (int)(data["process"]?["type"] ?? 2);
68+
6669
var newRepo = CreateGitHubRepositoryConfiguration(githubOrg, githubRepo, defaultBranch, clean, checkoutSubmodules, connectedServiceId, targetApiUrl);
6770
var isPipelineRequiredByBranchPolicy = await IsPipelineRequiredByBranchPolicy(adoOrg, teamProject, currentRepoName, currentRepoId, pipelineId);
6871

6972
LogBranchPolicyCheckResults(pipelineId, isPipelineRequiredByBranchPolicy);
7073

71-
var payload = BuildPipelinePayload(data, newRepo, originalTriggers, isPipelineRequiredByBranchPolicy);
74+
var payload = BuildPipelinePayload(data, newRepo, originalTriggers, isPipelineRequiredByBranchPolicy, processType);
7275

7376
await _adoApi.PutAsync(url, payload.ToObject(typeof(object)));
7477
return true;
@@ -206,8 +209,9 @@ private void LogBranchPolicyCheckResults(int pipelineId, bool isPipelineRequired
206209
_log.LogInformation(branchPolicyMessage);
207210
}
208211

209-
private JObject BuildPipelinePayload(JObject data, object newRepo, JToken originalTriggers, bool isPipelineRequiredByBranchPolicy)
212+
private JObject BuildPipelinePayload(JObject data, object newRepo, JToken originalTriggers, bool isPipelineRequiredByBranchPolicy, int processType)
210213
{
214+
var isClassicPipeline = processType == 1;
211215
var payload = new JObject();
212216

213217
foreach (var prop in data.Properties())
@@ -218,18 +222,23 @@ private JObject BuildPipelinePayload(JObject data, object newRepo, JToken origin
218222
}
219223
else if (prop.Name == "triggers")
220224
{
221-
prop.Value = DetermineTriggerConfiguration(originalTriggers, isPipelineRequiredByBranchPolicy);
225+
// Classic pipelines keep their original triggers; YAML pipelines get reconfigured
226+
prop.Value = isClassicPipeline
227+
? (originalTriggers ?? prop.Value)
228+
: DetermineTriggerConfiguration(originalTriggers, isPipelineRequiredByBranchPolicy);
222229
}
223230

224231
payload.Add(prop.Name, prop.Value);
225232
}
226233

227-
// Add triggers if no triggers property exists
228-
payload["triggers"] ??= DetermineTriggerConfiguration(originalTriggers, isPipelineRequiredByBranchPolicy);
234+
if (!isClassicPipeline)
235+
{
236+
// Add triggers if no triggers property exists (YAML pipelines only)
237+
payload["triggers"] ??= DetermineTriggerConfiguration(originalTriggers, isPipelineRequiredByBranchPolicy);
238+
}
229239

230-
// Use YAML definitions instead of UI override settings
231-
// settingsSourceType: 2 = Use YAML definitions, 1 = Override from UI
232-
payload["settingsSourceType"] = 2;
240+
// settingsSourceType: 1 = UI/Designer override (Classic), 2 = YAML definitions
241+
payload["settingsSourceType"] = isClassicPipeline ? 1 : 2;
233242

234243
return payload;
235244
}

‎src/OctoshiftCLI.Tests/Octoshift/Services/AdoPipelineTriggerService_ClassicPipelineTests.cs‎

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using FluentAssertions;
4+
using Moq;
5+
using Newtonsoft.Json.Linq;
6+
using OctoshiftCLI.Extensions;
7+
using OctoshiftCLI.Services;
8+
using Xunit;
9+
10+
namespace OctoshiftCLI.Tests.Octoshift.Services
11+
{
12+
public class AdoPipelineTriggerService_ClassicPipelineTests
13+
{
14+
private const string ADO_ORG = "foo-org";
15+
private const string TEAM_PROJECT = "foo-project";
16+
private const string REPO_NAME = "foo-repo";
17+
private const int PIPELINE_ID = 123;
18+
private const string ADO_SERVICE_URL = "https://dev.azure.com";
19+
20+
private readonly Mock<OctoLogger> _mockOctoLogger = TestHelpers.CreateMock<OctoLogger>();
21+
private readonly Mock<AdoApi> _mockAdoApi = TestHelpers.CreateMock<AdoApi>();
22+
private readonly AdoPipelineTriggerService _triggerService;
23+
24+
public AdoPipelineTriggerService_ClassicPipelineTests()
25+
{
26+
_triggerService = new AdoPipelineTriggerService(_mockAdoApi.Object, _mockOctoLogger.Object, ADO_SERVICE_URL);
27+
}
28+
29+
[Fact]
30+
public async Task RewirePipelineToGitHub_Should_Use_SettingsSourceType_1_For_Classic_Pipeline()
31+
{
32+
// Arrange
33+
var githubOrg = "github-org";
34+
var githubRepo = "github-repo";
35+
var serviceConnectionId = Guid.NewGuid().ToString();
36+
var defaultBranch = "main";
37+
var clean = "true";
38+
var checkoutSubmodules = "false";
39+
40+
var originalTriggers = new JArray
41+
{
42+
new JObject
43+
{
44+
["triggerType"] = "continuousIntegration",
45+
["branchFilters"] = new JArray { "+refs/heads/main" }
46+
}
47+
};
48+
49+
// Classic pipeline definition with process.type = 1
50+
var existingPipelineData = new JObject
51+
{
52+
["name"] = "classic-build-pipeline",
53+
["process"] = new JObject { ["type"] = 1 },
54+
["repository"] = new JObject { ["name"] = REPO_NAME, ["id"] = "repo-id-123" },
55+
["triggers"] = originalTriggers
56+
};
57+
58+
var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{PIPELINE_ID}?api-version=6.0";
59+
60+
_mockAdoApi.Setup(x => x.GetAsync(pipelineUrl))
61+
.ReturnsAsync(existingPipelineData.ToString());
62+
63+
// Mock repository lookup for branch policy check
64+
var repoResponse = new { id = "repo-id-123", name = REPO_NAME, isDisabled = "false" }.ToJson();
65+
var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0";
66+
_mockAdoApi.Setup(x => x.GetAsync(repoUrl))
67+
.ReturnsAsync(repoResponse);
68+
69+
// Mock branch policies (empty)
70+
var policies = new { count = 0, value = Array.Empty<object>() }.ToJson();
71+
var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId=repo-id-123&api-version=6.0";
72+
_mockAdoApi.Setup(x => x.GetAsync(policyUrl))
73+
.ReturnsAsync(policies);
74+
75+
JObject capturedPayload = null;
76+
_mockAdoApi.Setup(x => x.PutAsync(pipelineUrl, It.IsAny<object>()))
77+
.Callback<string, object>((_, payload) => capturedPayload = JObject.FromObject(payload))
78+
.Returns(Task.CompletedTask);
79+
80+
// Act
81+
var result = await _triggerService.RewirePipelineToGitHub(
82+
ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
83+
githubOrg, githubRepo, serviceConnectionId, originalTriggers, null);
84+
85+
// Assert
86+
result.Should().BeTrue();
87+
capturedPayload.Should().NotBeNull();
88+
((int)capturedPayload["settingsSourceType"]).Should().Be(1, "Classic pipelines must use settingsSourceType=1 (UI/Designer)");
89+
}
90+
91+
[Fact]
92+
public async Task RewirePipelineToGitHub_Should_Use_SettingsSourceType_2_For_Yaml_Pipeline()
93+
{
94+
// Arrange
95+
var githubOrg = "github-org";
96+
var githubRepo = "github-repo";
97+
var serviceConnectionId = Guid.NewGuid().ToString();
98+
var defaultBranch = "main";
99+
var clean = "true";
100+
var checkoutSubmodules = "false";
101+
102+
// YAML pipeline definition with process.type = 2
103+
var existingPipelineData = new JObject
104+
{
105+
["name"] = "yaml-pipeline",
106+
["process"] = new JObject { ["type"] = 2 },
107+
["repository"] = new JObject { ["name"] = REPO_NAME, ["id"] = "repo-id-456" },
108+
["triggers"] = new JArray()
109+
};
110+
111+
var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{PIPELINE_ID}?api-version=6.0";
112+
113+
_mockAdoApi.Setup(x => x.GetAsync(pipelineUrl))
114+
.ReturnsAsync(existingPipelineData.ToString());
115+
116+
// Mock repository lookup
117+
var repoResponse = new { id = "repo-id-456", name = REPO_NAME, isDisabled = "false" }.ToJson();
118+
var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0";
119+
_mockAdoApi.Setup(x => x.GetAsync(repoUrl))
120+
.ReturnsAsync(repoResponse);
121+
122+
// Mock branch policies (empty)
123+
var policies = new { count = 0, value = Array.Empty<object>() }.ToJson();
124+
var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId=repo-id-456&api-version=6.0";
125+
_mockAdoApi.Setup(x => x.GetAsync(policyUrl))
126+
.ReturnsAsync(policies);
127+
128+
JObject capturedPayload = null;
129+
_mockAdoApi.Setup(x => x.PutAsync(pipelineUrl, It.IsAny<object>()))
130+
.Callback<string, object>((_, payload) => capturedPayload = JObject.FromObject(payload))
131+
.Returns(Task.CompletedTask);
132+
133+
// Act
134+
var result = await _triggerService.RewirePipelineToGitHub(
135+
ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
136+
githubOrg, githubRepo, serviceConnectionId, null, null);
137+
138+
// Assert
139+
result.Should().BeTrue();
140+
capturedPayload.Should().NotBeNull();
141+
((int)capturedPayload["settingsSourceType"]).Should().Be(2, "YAML pipelines must use settingsSourceType=2 (YAML definitions)");
142+
}
143+
144+
[Fact]
145+
public async Task RewirePipelineToGitHub_Should_Preserve_Original_Triggers_For_Classic_Pipeline()
146+
{
147+
// Arrange
148+
var githubOrg = "github-org";
149+
var githubRepo = "github-repo";
150+
var serviceConnectionId = Guid.NewGuid().ToString();
151+
var defaultBranch = "main";
152+
var clean = "true";
153+
var checkoutSubmodules = "false";
154+
155+
var originalTriggers = new JArray
156+
{
157+
new JObject
158+
{
159+
["triggerType"] = "continuousIntegration",
160+
["branchFilters"] = new JArray { "+refs/heads/main", "+refs/heads/develop" },
161+
["batchChanges"] = true
162+
}
163+
};
164+
165+
// Classic pipeline
166+
var existingPipelineData = new JObject
167+
{
168+
["name"] = "classic-build-pipeline",
169+
["process"] = new JObject { ["type"] = 1 },
170+
["repository"] = new JObject { ["name"] = REPO_NAME, ["id"] = "repo-id-123" },
171+
["triggers"] = new JArray
172+
{
173+
new JObject { ["triggerType"] = "continuousIntegration", ["branchFilters"] = new JArray { "+refs/heads/old" } }
174+
}
175+
};
176+
177+
var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{PIPELINE_ID}?api-version=6.0";
178+
179+
_mockAdoApi.Setup(x => x.GetAsync(pipelineUrl))
180+
.ReturnsAsync(existingPipelineData.ToString());
181+
182+
var repoResponse = new { id = "repo-id-123", name = REPO_NAME, isDisabled = "false" }.ToJson();
183+
var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0";
184+
_mockAdoApi.Setup(x => x.GetAsync(repoUrl)).ReturnsAsync(repoResponse);
185+
186+
var policies = new { count = 0, value = Array.Empty<object>() }.ToJson();
187+
var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId=repo-id-123&api-version=6.0";
188+
_mockAdoApi.Setup(x => x.GetAsync(policyUrl)).ReturnsAsync(policies);
189+
190+
JObject capturedPayload = null;
191+
_mockAdoApi.Setup(x => x.PutAsync(pipelineUrl, It.IsAny<object>()))
192+
.Callback<string, object>((_, payload) => capturedPayload = JObject.FromObject(payload))
193+
.Returns(Task.CompletedTask);
194+
195+
// Act
196+
var result = await _triggerService.RewirePipelineToGitHub(
197+
ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
198+
githubOrg, githubRepo, serviceConnectionId, originalTriggers, null);
199+
200+
// Assert
201+
result.Should().BeTrue();
202+
capturedPayload.Should().NotBeNull();
203+
204+
// Classic pipelines should preserve the original triggers, not reconfigure them for YAML
205+
var triggers = (JArray)capturedPayload["triggers"];
206+
triggers.Should().NotBeNull();
207+
triggers.Count.Should().Be(1);
208+
triggers[0]["triggerType"].ToString().Should().Be("continuousIntegration");
209+
// Should use the passed-in originalTriggers, which has main+develop
210+
((JArray)triggers[0]["branchFilters"]).Count.Should().Be(2);
211+
}
212+
213+
[Fact]
214+
public async Task RewirePipelineToGitHub_Should_Default_To_Yaml_When_Process_Type_Missing()
215+
{
216+
// Arrange
217+
var githubOrg = "github-org";
218+
var githubRepo = "github-repo";
219+
var serviceConnectionId = Guid.NewGuid().ToString();
220+
var defaultBranch = "main";
221+
var clean = "true";
222+
var checkoutSubmodules = "false";
223+
224+
// Pipeline definition without process.type (legacy or unexpected response)
225+
var existingPipelineData = new JObject
226+
{
227+
["name"] = "some-pipeline",
228+
["repository"] = new JObject { ["name"] = REPO_NAME, ["id"] = "repo-id-789" },
229+
["triggers"] = new JArray()
230+
};
231+
232+
var pipelineUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/build/definitions/{PIPELINE_ID}?api-version=6.0";
233+
234+
_mockAdoApi.Setup(x => x.GetAsync(pipelineUrl))
235+
.ReturnsAsync(existingPipelineData.ToString());
236+
237+
var repoResponse = new { id = "repo-id-789", name = REPO_NAME, isDisabled = "false" }.ToJson();
238+
var repoUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0";
239+
_mockAdoApi.Setup(x => x.GetAsync(repoUrl)).ReturnsAsync(repoResponse);
240+
241+
var policies = new { count = 0, value = Array.Empty<object>() }.ToJson();
242+
var policyUrl = $"{ADO_SERVICE_URL}/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId=repo-id-789&api-version=6.0";
243+
_mockAdoApi.Setup(x => x.GetAsync(policyUrl)).ReturnsAsync(policies);
244+
245+
JObject capturedPayload = null;
246+
_mockAdoApi.Setup(x => x.PutAsync(pipelineUrl, It.IsAny<object>()))
247+
.Callback<string, object>((_, payload) => capturedPayload = JObject.FromObject(payload))
248+
.Returns(Task.CompletedTask);
249+
250+
// Act
251+
var result = await _triggerService.RewirePipelineToGitHub(
252+
ADO_ORG, TEAM_PROJECT, PIPELINE_ID, defaultBranch, clean, checkoutSubmodules,
253+
githubOrg, githubRepo, serviceConnectionId, null, null);
254+
255+
// Assert - should default to YAML behavior (settingsSourceType=2)
256+
result.Should().BeTrue();
257+
capturedPayload.Should().NotBeNull();
258+
((int)capturedPayload["settingsSourceType"]).Should().Be(2, "When process type is missing, should default to YAML (settingsSourceType=2)");
259+
}
260+
}
261+
}

0 commit comments

Comments
 (0)