fix: dont set gitlab status if new commit has been pushed, also send …

…the pipeline id in the request rather than ref (#6)

server/events/vcs/gitlab_client.go

@@ -22,14 +22,14 @@ import (
 	"strings"
 	"time"
 
-	version "github.com/hashicorp/go-version"
+	"github.com/hashicorp/go-version"
 	"github.com/jpillora/backoff"
 	"github.com/pkg/errors"
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs/common"
 	"github.com/runatlantis/atlantis/server/logging"
-	gitlab "github.com/xanzy/go-gitlab"
+	"github.com/xanzy/go-gitlab"
 )
 
 // gitlabMaxCommentLength is the maximum number of chars allowed by Gitlab in a
@@ -410,7 +410,8 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 	// 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
+	var refTarget *string
+	var pipelineID *int
 
 	retries := 1
 	delay := 2 * time.Second
@@ -425,14 +426,21 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 		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
+			// set pipeline ID for the req once found
+			pipelineID = gitlab.Ptr(mr.HeadPipeline.ID)
+			// if these are different then there is already a new commit so let's stop setting any statuses and return
+			if mr.HeadPipeline.SHA != pull.HeadCommit {
+				return errors.Errorf("mr.HeadPipeline.SHA: '%s' does not match pull.HeadCommit '%s'", mr.HeadPipeline.SHA, pull.HeadCommit)
+			}
 			break
 		}
 		if i != retries {
 			logger.Debug("Head pipeline not found for merge request %d. Retrying in %s",
 				pull.Num, delay)
 			time.Sleep(delay)
 		} else {
+			// set the ref target here if the pipeline wasn't found
+			refTarget = gitlab.Ptr(pull.HeadBranch)
 			logger.Debug("Head pipeline not found for merge request %d.",
 				pull.Num)
 		}
@@ -461,7 +469,9 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
 			Context:     gitlab.Ptr(src),
 			Description: gitlab.Ptr(description),
 			TargetURL:   &url,
-			Ref:         gitlab.Ptr(refTarget),
+			// only one of these should get sent in the request
+			PipelineID: pipelineID,
+			Ref:        refTarget,
 		})
 
 		if resp != nil {

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"
 )
@@ -297,12 +297,12 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
 			testServer := httptest.NewServer(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					switch r.RequestURI {
-					case "/api/v4/projects/runatlantis%2Fatlantis/statuses/sha":
+					case "/api/v4/projects/runatlantis%2Fatlantis/statuses/67cb91d3f6198189f433c045154a885784ba6977":
 						gotRequest = true
 
 						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
@@ -336,7 +336,7 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
 				models.PullRequest{
 					Num:        1,
 					BaseRepo:   repo,
-					HeadCommit: "sha",
+					HeadCommit: "67cb91d3f6198189f433c045154a885784ba6977",
 					HeadBranch: "test",
 				}, c.status, "src", "description", "https://google.com")
 			Ok(t, err)
@@ -387,13 +387,13 @@ func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
 			testServer := httptest.NewServer(
 				http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 					switch r.RequestURI {
-					case "/api/v4/projects/runatlantis%2Fatlantis/statuses/sha":
+					case "/api/v4/projects/runatlantis%2Fatlantis/statuses/67cb91d3f6198189f433c045154a885784ba6977":
 						handledNumberOfRequests++
 						shouldSendConflict := handledNumberOfRequests <= c.numberOfConflicts
 
 						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
 
@@ -436,12 +436,12 @@ func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
 				models.PullRequest{
 					Num:        1,
 					BaseRepo:   repo,
-					HeadCommit: "sha",
+					HeadCommit: "67cb91d3f6198189f433c045154a885784ba6977",
 					HeadBranch: "test",
 				}, c.status, "src", "description", "https://google.com")
 
 			if c.expError {
-				ErrContains(t, "failed to update commit status for 'runatlantis/atlantis' @ 'sha' to 'src' after 10 attempts", err)
+				ErrContains(t, "failed to update commit status for 'runatlantis/atlantis' @ '67cb91d3f6198189f433c045154a885784ba6977' to 'src' after 10 attempts", err)
 				ErrContains(t, "409", err)
 			} else {
 				Ok(t, err)