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
runatlantis#3695)

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: runatlantis#3428
* Tests Patch MR : runatlantis#3653
  • Loading branch information
marceloboeira authored and ijames-gc committed Feb 13, 2024
1 parent e28fdc0 commit 6bb5ba9
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 6bb5ba9

Please sign in to comment.