From bcd917ffbacf01fb2b75b6504698fec85db4bf7a Mon Sep 17 00:00:00 2001
From: MB <marcelo@boeira.me>
Date: Wed, 23 Aug 2023 23:07:38 +0200
Subject: [PATCH] fix(gitlab): Prevent considering non-head pipelines skipped
 by default (#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: https://github.com/runatlantis/atlantis/pull/3428
* Tests Patch MR : https://github.com/runatlantis/atlantis/pull/3653
---
 server/events/vcs/gitlab_client.go      |  2 +-
 server/events/vcs/gitlab_client_test.go | 28 +++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go
index f96d964fda..030629c8b8 100644
--- a/server/events/vcs/gitlab_client.go
+++ b/server/events/vcs/gitlab_client.go
@@ -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"
diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go
index 5d4e6a701b..a7f1a77be6 100644
--- a/server/events/vcs/gitlab_client_test.go
+++ b/server/events/vcs/gitlab_client_test.go
@@ -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,
@@ -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 {