Skip to content

Commit

Permalink
fix(gitlab): Prevent considering non-head pipelines skipped by default
Browse files Browse the repository at this point in the history
By fixing nil pointer reference in 3428 we unveiled yet another bug now that the code is able to run further:

When an MR with no HeadPipeline exists its commits are set to skipped by default and thus create some mess when running "PullMergeable" (rendering the MR useless)

The bug was caught by the tests, however, in an attempt to make the build pass and unblock we create a patch to ignore that test.

In order to prevent further problems, this MR:

* Updates the test stack to run all plan/apply commands in the context
  of "HeadLess" Pipelines
* Fixes the default of skipped pipeline to false (as it is better to
  assume it is NOT skipped since that prevents the merge in most cases)
* Make all integratiion tests pass

References:

* Original MR: #3428
* Tests Patch MR : #3653
  • Loading branch information
marceloboeira committed Aug 22, 2023
1 parent 2777ad0 commit b78e2cd
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
2 changes: 1 addition & 1 deletion server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
// Prevent nil pointer error when mr.HeadPipeline is empty
// See: https://github.com/runatlantis/atlantis/issues/1852
commit := pull.HeadCommit
isPipelineSkipped := true
isPipelineSkipped := false
if mr.HeadPipeline != nil {
commit = mr.HeadPipeline.SHA
isPipelineSkipped = mr.HeadPipeline.Status == "skipped"
Expand Down
28 changes: 28 additions & 0 deletions server/events/vcs/gitlab_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,27 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
defaultMr,
true,
},
{
fmt.Sprintf("%s/apply: resource/default", vcsStatusName),
models.FailedCommitStatus,
gitlabServerVersions,
noHeadPipelineMR,
true,
},
{
fmt.Sprintf("%s/apply", vcsStatusName),
models.FailedCommitStatus,
gitlabServerVersions,
noHeadPipelineMR,
true,
},
{
fmt.Sprintf("%s/plan: resource/default", vcsStatusName),
models.FailedCommitStatus,
gitlabServerVersions,
noHeadPipelineMR,
false,
},
{
fmt.Sprintf("%s/plan", vcsStatusName),
models.PendingCommitStatus,
Expand All @@ -381,6 +402,13 @@ func TestGitlabClient_PullIsMergeable(t *testing.T) {
noHeadPipelineMR,
false,
},
{
fmt.Sprintf("%s/plan", vcsStatusName),
models.SuccessCommitStatus,
gitlabServerVersions,
noHeadPipelineMR,
true,
},
}
for _, serverVersion := range gitlabServerVersions {
for _, c := range cases {
Expand Down

0 comments on commit b78e2cd

Please sign in to comment.