Skip to content

Commit b9d5da5

Browse files
tpolekhinnitrocode
authored and
ijames-gc
committed
fix: allow Require Linear History when selecting merge method (runatlantis#3211)
* fix: account for Require Linear History when selecting merge method * fix logic and add tests * add nolint hints for w.Write --------- Co-authored-by: nitrocode <[email protected]>
1 parent 7468add commit b9d5da5

File tree

3 files changed

+156
-27
lines changed

3 files changed

+156
-27
lines changed

‎server/events/vcs/github_client.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -579,13 +579,23 @@ func (g *GithubClient) MergePull(pull models.PullRequest, pullOptions models.Pul
579579
if err != nil {
580580
return errors.Wrap(err, "fetching repo info")
581581
}
582+
protection, _, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner.GetLogin(), *repo.Name, pull.BaseBranch)
583+
if err != nil {
584+
if !errors.Is(err, github.ErrBranchNotProtected) {
585+
return errors.Wrap(err, "getting branch protection rules")
586+
}
587+
}
588+
requireLinearHistory := false
589+
if protection != nil {
590+
requireLinearHistory = protection.GetRequireLinearHistory().Enabled
591+
}
582592
const (
583593
defaultMergeMethod = "merge"
584594
rebaseMergeMethod = "rebase"
585595
squashMergeMethod = "squash"
586596
)
587597
method := defaultMergeMethod
588-
if !repo.GetAllowMergeCommit() {
598+
if !repo.GetAllowMergeCommit() || requireLinearHistory {
589599
if repo.GetAllowRebaseMerge() {
590600
method = rebaseMergeMethod
591601
} else if repo.GetAllowSquashMerge() {

‎server/events/vcs/github_client_test.go

+139-26
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,10 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) {
787787
defer r.Body.Close() // nolint: errcheck
788788
w.WriteHeader(c.code)
789789
w.Write([]byte(resp)) // nolint: errcheck
790+
case "/api/v3/repos/runatlantis/atlantis/branches/master/protection":
791+
w.WriteHeader(404)
792+
w.Write([]byte("{\"message\":\"Branch not protected\"}")) // nolint: errcheck
793+
return
790794
default:
791795
t.Errorf("got unexpected request at %q", r.RequestURI)
792796
http.Error(w, "not found", http.StatusNotFound)
@@ -813,7 +817,8 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) {
813817
Hostname: "github.com",
814818
},
815819
},
816-
Num: 1,
820+
Num: 1,
821+
BaseBranch: "master",
817822
}, models.PullRequestOptions{
818823
DeleteSourceBranchOnMerge: false,
819824
})
@@ -831,40 +836,132 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) {
831836
// use that method
832837
func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
833838
cases := map[string]struct {
834-
allowMerge bool
835-
allowRebase bool
836-
allowSquash bool
837-
expMethod string
839+
allowMerge bool
840+
allowRebase bool
841+
allowSquash bool
842+
requireLinearHistory bool
843+
protectedBranch bool
844+
expMethod string
838845
}{
839846
"all true": {
840-
allowMerge: true,
841-
allowRebase: true,
842-
allowSquash: true,
843-
expMethod: "merge",
847+
allowMerge: true,
848+
allowRebase: true,
849+
allowSquash: true,
850+
requireLinearHistory: false,
851+
protectedBranch: false,
852+
expMethod: "merge",
844853
},
845854
"all false (edge case)": {
846-
allowMerge: false,
847-
allowRebase: false,
848-
allowSquash: false,
849-
expMethod: "merge",
855+
allowMerge: false,
856+
allowRebase: false,
857+
allowSquash: false,
858+
requireLinearHistory: false,
859+
protectedBranch: false,
860+
expMethod: "merge",
850861
},
851862
"merge: false rebase: true squash: true": {
852-
allowMerge: false,
853-
allowRebase: true,
854-
allowSquash: true,
855-
expMethod: "rebase",
863+
allowMerge: false,
864+
allowRebase: true,
865+
allowSquash: true,
866+
requireLinearHistory: false,
867+
protectedBranch: false,
868+
expMethod: "rebase",
856869
},
857870
"merge: false rebase: false squash: true": {
858-
allowMerge: false,
859-
allowRebase: false,
860-
allowSquash: true,
861-
expMethod: "squash",
871+
allowMerge: false,
872+
allowRebase: false,
873+
allowSquash: true,
874+
requireLinearHistory: false,
875+
protectedBranch: false,
876+
expMethod: "squash",
862877
},
863878
"merge: false rebase: true squash: false": {
864-
allowMerge: false,
865-
allowRebase: true,
866-
allowSquash: false,
867-
expMethod: "rebase",
879+
allowMerge: false,
880+
allowRebase: true,
881+
allowSquash: false,
882+
requireLinearHistory: false,
883+
protectedBranch: false,
884+
expMethod: "rebase",
885+
},
886+
"protected, all true, rlh: false": {
887+
allowMerge: true,
888+
allowRebase: true,
889+
allowSquash: true,
890+
requireLinearHistory: false,
891+
protectedBranch: true,
892+
expMethod: "merge",
893+
},
894+
"protected, all false (edge case), rlh: false": {
895+
allowMerge: false,
896+
allowRebase: false,
897+
allowSquash: false,
898+
requireLinearHistory: false,
899+
protectedBranch: true,
900+
expMethod: "merge",
901+
},
902+
"protected, merge: false rebase: true squash: true, rlh: false": {
903+
allowMerge: false,
904+
allowRebase: true,
905+
allowSquash: true,
906+
requireLinearHistory: false,
907+
protectedBranch: true,
908+
expMethod: "rebase",
909+
},
910+
"protected, merge: false rebase: false squash: true, rlh: false": {
911+
allowMerge: false,
912+
allowRebase: false,
913+
allowSquash: true,
914+
requireLinearHistory: false,
915+
protectedBranch: true,
916+
expMethod: "squash",
917+
},
918+
"protected, merge: false rebase: true squash: false, rlh: false": {
919+
allowMerge: false,
920+
allowRebase: true,
921+
allowSquash: false,
922+
requireLinearHistory: false,
923+
protectedBranch: true,
924+
expMethod: "rebase",
925+
},
926+
"protected, all true, rlh: true": {
927+
allowMerge: true,
928+
allowRebase: true,
929+
allowSquash: true,
930+
requireLinearHistory: true,
931+
protectedBranch: true,
932+
expMethod: "rebase",
933+
},
934+
"protected, all false (edge case), rlh: true": {
935+
allowMerge: false,
936+
allowRebase: false,
937+
allowSquash: false,
938+
requireLinearHistory: true,
939+
protectedBranch: true,
940+
expMethod: "merge",
941+
},
942+
"protected, merge: false rebase: true squash: true, rlh: true": {
943+
allowMerge: false,
944+
allowRebase: true,
945+
allowSquash: true,
946+
requireLinearHistory: true,
947+
protectedBranch: true,
948+
expMethod: "rebase",
949+
},
950+
"protected, merge: false rebase: false squash: true, rlh: true": {
951+
allowMerge: false,
952+
allowRebase: false,
953+
allowSquash: true,
954+
requireLinearHistory: true,
955+
protectedBranch: true,
956+
expMethod: "squash",
957+
},
958+
"protected, merge: false rebase: true squash: false, rlh: true": {
959+
allowMerge: false,
960+
allowRebase: true,
961+
allowSquash: false,
962+
requireLinearHistory: true,
963+
protectedBranch: true,
964+
expMethod: "rebase",
868965
},
869966
}
870967

@@ -874,7 +971,10 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
874971
// Modify response.
875972
jsBytes, err := os.ReadFile("testdata/github-repo.json")
876973
Ok(t, err)
974+
rlhBytes, err := os.ReadFile("testdata/github-branch-protection-require-linear-history.json")
975+
Ok(t, err)
877976
resp := string(jsBytes)
977+
protected := string(rlhBytes)
878978
resp = strings.Replace(resp,
879979
`"allow_squash_merge": true`,
880980
fmt.Sprintf(`"allow_squash_merge": %t`, c.allowSquash),
@@ -887,13 +987,25 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
887987
`"allow_rebase_merge": true`,
888988
fmt.Sprintf(`"allow_rebase_merge": %t`, c.allowRebase),
889989
-1)
990+
protected = strings.Replace(protected,
991+
`"enabled": true`,
992+
fmt.Sprintf(`"enabled": %t`, c.requireLinearHistory),
993+
-1)
890994

891995
testServer := httptest.NewTLSServer(
892996
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
893997
switch r.RequestURI {
894998
case "/api/v3/repos/runatlantis/atlantis":
895999
w.Write([]byte(resp)) // nolint: errcheck
8961000
return
1001+
case "/api/v3/repos/runatlantis/atlantis/branches/master/protection":
1002+
if c.protectedBranch {
1003+
w.Write([]byte(protected)) // nolint: errcheck
1004+
} else {
1005+
w.WriteHeader(404)
1006+
w.Write([]byte("{\"message\":\"Branch not protected\"}")) // nolint: errcheck
1007+
}
1008+
return
8971009
case "/api/v3/repos/runatlantis/atlantis/pulls/1/merge":
8981010
body, err := io.ReadAll(r.Body)
8991011
Ok(t, err)
@@ -936,7 +1048,8 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
9361048
Hostname: "github.com",
9371049
},
9381050
},
939-
Num: 1,
1051+
Num: 1,
1052+
BaseBranch: "master",
9401053
}, models.PullRequestOptions{
9411054
DeleteSourceBranchOnMerge: false,
9421055
})

‎server/events/vcs/testdata/github-branch-protection-require-linear-history.json

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"url": "https://api.github.com/repos/octocat/Hello-World/branches/master/protection",
3+
"required_linear_history": {
4+
"enabled": true
5+
}
6+
}

0 commit comments

Comments
 (0)