Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: send pipelineID in gitlabs SetCommitStatus if the mr gets found, fallback to branch ref #4785

Merged
merged 1 commit into from
Aug 8, 2024
Merged

fix: send pipelineID in gitlabs SetCommitStatus if the mr gets found, fallback to branch ref #4785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions server/events/vcs/gitlab_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -406,17 +406,17 @@ 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
// refTarget is only set to the head branch of the MR if HeadPipeline is not found
// when HeadPipeline is found we set the pipelineID for the request instead
var refTarget *string
var pipelineID *int

retries := 1
delay := 2 * time.Second
var mr *gitlab.MergeRequest
var err error

// Try to get the MR details a couple of times in case the pipeline is not yet assigned to the MR
for i := 0; i <= retries; i++ {
mr, err = g.GetMergeRequest(logger, pull.BaseRepo.FullName, pull.Num)
if err != nil {
Expand All @@ -425,14 +425,17 @@ 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)
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)
}
Expand Down Expand Up @@ -461,7 +464,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 {
Expand Down
10 changes: 6 additions & 4 deletions server/events/vcs/gitlab_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ 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"
)

var projectID = 4580910

const gitlabPipelineSuccessMrID = 488598

// Test that the base url gets set properly.
func TestNewGitlabClient_BaseURL(t *testing.T) {
gitlabClientUnderTest = true
Expand Down Expand Up @@ -302,7 +304,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":%d}`, c.expState, gitlabPipelineSuccessMrID)
Equals(t, exp, string(body))
defer r.Body.Close() // nolint: errcheck
w.Write([]byte("{}")) // nolint: errcheck
Expand Down Expand Up @@ -393,7 +395,7 @@ func TestGitlabClient_UpdateStatusRetryable(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":%d}`, c.expState, gitlabPipelineSuccessMrID)
Equals(t, exp, string(body))
defer r.Body.Close() // nolint: errcheck

Expand Down