Skip to content

Commit 2f7aa72

Browse files
committed
branchprotector: remove protection from excluded branches
Fix issue where excluded branches retain existing protection instead of being removed. When a branch is added to the exclude list in branchprotector configuration, the tool correctly stops applying new protection rules but does not remove existing protection from branches that were previously protected. This change adds logic to detect excluded branches that are currently protected and queue them for removal by sending requirements with Request: nil, which triggers RemoveBranchProtection() in the configureBranches() function. The fix prevents push failures like: remote: error: GH006: Protected branch update failed for refs/heads/konflux-branch remote: - Changes must be made through a pull request. Changes: - Add detection logic for excluded protected branches in UpdateRepo() - Send removal requests (Request: nil) for such branches - Update tests to expect removal requests for excluded protected branches - Add code comment explaining the call flow to RemoveBranchProtection Fixes #477
1 parent e2a3dbc commit 2f7aa72

File tree

2 files changed

+41
-0
lines changed

2 files changed

+41
-0
lines changed

cmd/branchprotector/protect.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,13 @@ func (p *protector) UpdateRepo(orgName string, repoName string, repo config.Repo
326326
}
327327

328328
branches := map[string]github.Branch{}
329+
var allBranches []github.Branch
329330
for _, onlyProtected := range []bool{false, true} { // put true second so b.Protected is set correctly
330331
bs, err := p.client.GetBranches(orgName, repoName, onlyProtected)
331332
if err != nil {
332333
return fmt.Errorf("list branches: %w", err)
333334
}
335+
allBranches = append(allBranches, bs...)
334336
for _, b := range bs {
335337
_, ok := repo.Branches[b.Name]
336338
if !ok && branchInclusions != nil && branchInclusions.MatchString(b.Name) {
@@ -346,6 +348,27 @@ func (p *protector) UpdateRepo(orgName string, repoName string, repo config.Repo
346348
}
347349
}
348350

351+
// Handle excluded branches that are currently protected and need removal
352+
if branchExclusions != nil {
353+
seen := make(map[string]bool)
354+
for _, b := range allBranches {
355+
if b.Protected && branchExclusions.MatchString(b.Name) && !seen[b.Name] {
356+
seen[b.Name] = true
357+
// Check if this branch is not in the branches map (i.e., it was excluded)
358+
if _, inBranches := branches[b.Name]; !inBranches {
359+
logrus.Infof("%s/%s=%s: removing protection from excluded branch", orgName, repoName, b.Name)
360+
p.updates <- requirements{
361+
Org: orgName,
362+
Repo: repoName,
363+
Branch: b.Name,
364+
Request: nil, // nil Request triggers RemoveBranchProtection
365+
// Flow: updates channel -> configureBranches() -> client.RemoveBranchProtection()
366+
}
367+
}
368+
}
369+
}
370+
}
371+
349372
var apps, collaborators, teams []string
350373
if p.verifyRestrictions {
351374
apps, err = p.authorizedApps(orgName)

cmd/branchprotector/protect_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,12 @@ branch-protection:
11091109
`,
11101110

11111111
expected: []requirements{
1112+
{
1113+
Org: "kubernetes",
1114+
Repo: "test-infra",
1115+
Branch: "skip",
1116+
Request: nil, // nil Request triggers RemoveBranchProtection
1117+
},
11121118
{
11131119
Org: "kubernetes",
11141120
Repo: "test-infra",
@@ -1133,6 +1139,18 @@ branch-protection:
11331139
- sk.*
11341140
`,
11351141
expected: []requirements{
1142+
{
1143+
Org: "kubernetes",
1144+
Repo: "test-infra",
1145+
Branch: "skip",
1146+
Request: nil, // nil Request triggers RemoveBranchProtection
1147+
},
1148+
{
1149+
Org: "kubernetes",
1150+
Repo: "test-infra",
1151+
Branch: "foobar1",
1152+
Request: nil, // nil Request triggers RemoveBranchProtection
1153+
},
11361154
{
11371155
Org: "kubernetes",
11381156
Repo: "test-infra",

0 commit comments

Comments
 (0)