From 08fda6d7d443ae3bf65b1482004c776f066818ac Mon Sep 17 00:00:00 2001
From: Fitz <a.fitzpatrick@efg.gg>
Date: Fri, 8 Mar 2024 08:17:34 +0000
Subject: [PATCH] fix: change some log levels, increase retries and use
 pipeline id for commit status

---
 server/events/vcs/gitlab_client.go      | 39 +++++++++++++++----------
 server/events/vcs/gitlab_client_test.go |  6 ++--
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go
index c4cb837a4e..27c8c3e291 100644
--- a/server/events/vcs/gitlab_client.go
+++ b/server/events/vcs/gitlab_client.go
@@ -25,12 +25,12 @@ import (
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/vcs/common"
 
-	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/go-version"
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/logging"
 
 	"github.com/runatlantis/atlantis/server/events/models"
-	gitlab "github.com/xanzy/go-gitlab"
+	"github.com/xanzy/go-gitlab"
 )
 
 // gitlabMaxCommentLength is the maximum number of chars allowed by Gitlab in a
@@ -397,14 +397,13 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 		gitlabState = gitlab.Success
 	}
 
-	// refTarget is set to the head pipeline of the MR if it exists, or else it is set to the head branch
-	// of the MR. This is needed because the commit status is only shown in the MR if the pipeline is
-	// assigned to an MR reference.
-	// Try to get the MR details a couple of times in case the pipeline is not yet assigned to the MR
-	refTarget := pull.HeadBranch
+	// create these pointers here so that they can be nil when they get to the request and are omitted
+	var refTarget *string
+	var pipelineID *int
+
+	retries := 2
+	delay := 5 * time.Second
 
-	retries := 1
-	delay := 2 * time.Second
 	var mr *gitlab.MergeRequest
 	var err error
 
@@ -414,17 +413,24 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 			return err
 		}
 		if mr.HeadPipeline != nil {
-			logger.Debug("Head pipeline found for merge request %d, source '%s'. refTarget '%s'",
-				pull.Num, mr.HeadPipeline.Source, mr.HeadPipeline.Ref)
-			refTarget = mr.HeadPipeline.Ref
+			logger.Info("Head pipeline found for merge request %d, source '%s'. pipelineID '%s'",
+				pull.Num, mr.HeadPipeline.Source, mr.HeadPipeline.ID)
+			pipelineID = gitlab.Ptr(mr.HeadPipeline.ID)
+
+			// let's check to see if the pipeline sha matches the head commit.
+			if mr.HeadPipeline.SHA != pull.HeadCommit {
+				logger.Err("Head pipeline SHA does not match pull head commit")
+			}
 			break
 		}
 		if i != retries {
-			logger.Debug("Head pipeline not found for merge request %d. Retrying in %s",
+			logger.Info("Head pipeline not found for merge request %d. Retrying in %s",
 				pull.Num, delay)
 			time.Sleep(delay)
 		} else {
-			logger.Debug("Head pipeline not found for merge request %d.",
+			headBranch := pull.HeadBranch
+			refTarget = gitlab.Ptr(headBranch)
+			logger.Warn("Head pipeline not found for merge request %d.",
 				pull.Num)
 		}
 	}
@@ -434,7 +440,8 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 		Context:     gitlab.Ptr(src),
 		Description: gitlab.Ptr(description),
 		TargetURL:   &url,
-		Ref:         gitlab.Ptr(refTarget),
+		PipelineID:  pipelineID,
+		Ref:         refTarget,
 	})
 	if resp != nil {
 		logger.Debug("POST /projects/%s/statuses/%s returned: %d", repo.FullName, pull.HeadCommit, resp.StatusCode)
@@ -461,7 +468,7 @@ func (g *GitlabClient) WaitForSuccessPipeline(logger logging.SimpleLogging, ctx
 		case <-ctx.Done():
 			// validation check time out
 			cancel()
-			return //ctx.Err()
+			return // ctx.Err()
 
 		default:
 			mr, _ := g.GetMergeRequest(logger, pull.BaseRepo.FullName, pull.Num)
diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go
index 5c463e85cf..0ce23a9c64 100644
--- a/server/events/vcs/gitlab_client_test.go
+++ b/server/events/vcs/gitlab_client_test.go
@@ -12,11 +12,11 @@ import (
 	"testing"
 	"time"
 
-	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/go-version"
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/logging"
-	gitlab "github.com/xanzy/go-gitlab"
+	"github.com/xanzy/go-gitlab"
 
 	. "github.com/runatlantis/atlantis/testing"
 )
@@ -303,7 +303,7 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
 
 						body, err := io.ReadAll(r.Body)
 						Ok(t, err)
-						exp := fmt.Sprintf(`{"state":"%s","ref":"patch-1-merger","context":"src","target_url":"https://google.com","description":"description"}`, c.expState)
+						exp := fmt.Sprintf(`{"state":"%s","context":"src","target_url":"https://google.com","description":"description","pipeline_id":488598}`, c.expState)
 						Equals(t, exp, string(body))
 						defer r.Body.Close()  // nolint: errcheck
 						w.Write([]byte("{}")) // nolint: errcheck