mirror of
https://git.vectorsigma.ru/public/atlantis.git
synced 2026-07-28 22:28:25 +00:00
fix: Change GitLab UpdateStatus Function to get Pipeline from Commit rather than the Merge Request (#5033)
Signed-off-by: X-Guardian <simonheather99@gmail.com>
This commit is contained in:
Simon Heather
GitHub
@@ -395,7 +395,6 @@ func (g *GitlabClient) SupportsDetailedMergeStatus(logger logging.SimpleLogging)
|
||||
|
||||
// UpdateStatus updates the build status of a commit.
|
||||
func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {
|
||||
logger.Debug("Updating GitLab commit status for '%s' to '%s'", src, state)
|
||||
gitlabState := gitlab.Pending
|
||||
switch state {
|
||||
case models.PendingCommitStatus:
|
||||
@@ -406,43 +405,47 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
|
||||
gitlabState = gitlab.Success
|
||||
}
|
||||
|
||||
// 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
|
||||
logger.Info("Updating GitLab commit status for '%s' to '%s'", src, gitlabState)
|
||||
|
||||
setCommitStatusOptions := &gitlab.SetCommitStatusOptions{
|
||||
State: gitlabState,
|
||||
Context: gitlab.Ptr(src),
|
||||
Description: gitlab.Ptr(description),
|
||||
TargetURL: &url,
|
||||
}
|
||||
|
||||
retries := 1
|
||||
delay := 2 * time.Second
|
||||
var mr *gitlab.MergeRequest
|
||||
var commit *gitlab.Commit
|
||||
var resp *gitlab.Response
|
||||
var err error
|
||||
|
||||
// Try to get the MR details a couple of times in case the pipeline is not yet assigned to the MR
|
||||
// Try a couple of times to get the pipeline ID for the commit
|
||||
for i := 0; i <= retries; i++ {
|
||||
mr, err = g.GetMergeRequest(logger, pull.BaseRepo.FullName, pull.Num)
|
||||
commit, resp, err = g.Client.Commits.GetCommit(repo.FullName, pull.HeadCommit, nil)
|
||||
if resp != nil {
|
||||
logger.Debug("GET /projects/%s/repository/commits/%d: %d", pull.BaseRepo.ID(), pull.HeadCommit, resp.StatusCode)
|
||||
}
|
||||
if err != nil {
|
||||
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)
|
||||
// set pipeline ID for the req once found
|
||||
pipelineID = gitlab.Ptr(mr.HeadPipeline.ID)
|
||||
if commit.LastPipeline != nil {
|
||||
logger.Info("Pipeline found for commit %s, setting pipeline ID to %d", pull.HeadCommit, commit.LastPipeline.ID)
|
||||
// Set the pipeline ID to the last pipeline that ran for the commit
|
||||
setCommitStatusOptions.PipelineID = gitlab.Ptr(commit.LastPipeline.ID)
|
||||
break
|
||||
}
|
||||
if i != retries {
|
||||
logger.Debug("Head pipeline not found for merge request %d. Retrying in %s",
|
||||
pull.Num, delay)
|
||||
logger.Info("No pipeline found for commit %s, retrying in %s", pull.HeadCommit, 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)
|
||||
// If we've exhausted all retries, set the Ref to the branch name
|
||||
logger.Info("No pipeline found for commit %s, setting Ref to %s", pull.HeadCommit, pull.HeadBranch)
|
||||
setCommitStatusOptions.Ref = gitlab.Ptr(pull.HeadBranch)
|
||||
}
|
||||
}
|
||||
|
||||
var (
|
||||
resp *gitlab.Response
|
||||
maxAttempts = 10
|
||||
retryer = &backoff.Backoff{
|
||||
Jitter: true,
|
||||
@@ -455,19 +458,11 @@ func (g *GitlabClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re
|
||||
"attempt", i+1,
|
||||
"max_attempts", maxAttempts,
|
||||
"repo", repo.FullName,
|
||||
"commit", pull.HeadCommit,
|
||||
"commit", commit.ShortID,
|
||||
"state", state.String(),
|
||||
)
|
||||
|
||||
_, resp, err = g.Client.Commits.SetCommitStatus(repo.FullName, pull.HeadCommit, &gitlab.SetCommitStatusOptions{
|
||||
State: gitlabState,
|
||||
Context: gitlab.Ptr(src),
|
||||
Description: gitlab.Ptr(description),
|
||||
TargetURL: &url,
|
||||
// only one of these should get sent in the request
|
||||
PipelineID: pipelineID,
|
||||
Ref: refTarget,
|
||||
})
|
||||
_, resp, err = g.Client.Commits.SetCommitStatus(repo.FullName, pull.HeadCommit, setCommitStatusOptions)
|
||||
|
||||
if resp != nil {
|
||||
logger.Debug("POST /projects/%s/statuses/%s returned: %d", repo.FullName, pull.HeadCommit, resp.StatusCode)
|
||||
|
||||
@@ -3,7 +3,6 @@ package vcs
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
@@ -25,10 +24,39 @@ var projectID = 4580910
|
||||
|
||||
const gitlabPipelineSuccessMrID = 488598
|
||||
|
||||
const updateStatusDescription = "description"
|
||||
const updateStatusTargetUrl = "https://google.com"
|
||||
const updateStatusSrc = "src"
|
||||
const updateStatusHeadBranch = "test"
|
||||
|
||||
/* UpdateStatus request JSON body object */
|
||||
type UpdateStatusJsonBody struct {
|
||||
State string `json:"state"`
|
||||
Context string `json:"context"`
|
||||
TargetUrl string `json:"target_url"`
|
||||
Description string `json:"description"`
|
||||
PipelineId int `json:"pipeline_id"`
|
||||
Ref string `json:"ref"`
|
||||
}
|
||||
|
||||
/* GetCommit response last_pipeline JSON object */
|
||||
type GetCommitResponseLastPipeline struct {
|
||||
ID int `json:"id"`
|
||||
}
|
||||
|
||||
/* GetCommit response JSON object */
|
||||
type GetCommitResponse struct {
|
||||
LastPipeline GetCommitResponseLastPipeline `json:"last_pipeline"`
|
||||
}
|
||||
|
||||
/* Empty struct for JSON marshalling */
|
||||
type EmptyStruct struct{}
|
||||
|
||||
// Test that the base url gets set properly.
|
||||
func TestNewGitlabClient_BaseURL(t *testing.T) {
|
||||
gitlabClientUnderTest = true
|
||||
defer func() { gitlabClientUnderTest = false }()
|
||||
|
||||
cases := []struct {
|
||||
Hostname string
|
||||
ExpBaseURL string
|
||||
@@ -273,8 +301,6 @@ func TestGitlabClient_MergePull(t *testing.T) {
|
||||
|
||||
func TestGitlabClient_UpdateStatus(t *testing.T) {
|
||||
logger := logging.NewNoopLogger(t)
|
||||
pipelineSuccess, err := os.ReadFile("testdata/gitlab-pipeline-success.json")
|
||||
Ok(t, err)
|
||||
|
||||
cases := []struct {
|
||||
status models.CommitStatus
|
||||
@@ -302,18 +328,42 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/statuses/sha":
|
||||
gotRequest = true
|
||||
|
||||
body, err := io.ReadAll(r.Body)
|
||||
var updateStatusJsonBody UpdateStatusJsonBody
|
||||
err := json.NewDecoder(r.Body).Decode(&updateStatusJsonBody)
|
||||
Ok(t, err)
|
||||
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
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/1":
|
||||
|
||||
Equals(t, c.expState, updateStatusJsonBody.State)
|
||||
Equals(t, updateStatusSrc, updateStatusJsonBody.Context)
|
||||
Equals(t, updateStatusTargetUrl, updateStatusJsonBody.TargetUrl)
|
||||
Equals(t, updateStatusDescription, updateStatusJsonBody.Description)
|
||||
Equals(t, gitlabPipelineSuccessMrID, updateStatusJsonBody.PipelineId)
|
||||
|
||||
defer r.Body.Close() // nolint: errcheck
|
||||
|
||||
setStatusJsonResponse, err := json.Marshal(EmptyStruct{})
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(setStatusJsonResponse)
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/repository/commits/sha":
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write(pipelineSuccess) // nolint: errcheck
|
||||
|
||||
getCommitResponse := GetCommitResponse{
|
||||
LastPipeline: GetCommitResponseLastPipeline{
|
||||
ID: gitlabPipelineSuccessMrID,
|
||||
},
|
||||
}
|
||||
getCommitJsonResponse, err := json.Marshal(getCommitResponse)
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/":
|
||||
// Rate limiter requests.
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
default:
|
||||
t.Errorf("got unexpected request at %q", r.RequestURI)
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
@@ -339,18 +389,158 @@ func TestGitlabClient_UpdateStatus(t *testing.T) {
|
||||
Num: 1,
|
||||
BaseRepo: repo,
|
||||
HeadCommit: "sha",
|
||||
HeadBranch: "test",
|
||||
}, c.status, "src", "description", "https://google.com")
|
||||
HeadBranch: updateStatusHeadBranch,
|
||||
},
|
||||
c.status,
|
||||
updateStatusSrc,
|
||||
updateStatusDescription,
|
||||
updateStatusTargetUrl,
|
||||
)
|
||||
Ok(t, err)
|
||||
Assert(t, gotRequest, "expected to get the request")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
|
||||
func TestGitlabClient_UpdateStatusGetCommitRetryable(t *testing.T) {
|
||||
logger := logging.NewNoopLogger(t)
|
||||
|
||||
cases := []struct {
|
||||
title string
|
||||
status models.CommitStatus
|
||||
commitsWithNoLastPipeline int
|
||||
expNumberOfRequests int
|
||||
expRefOrPipelineId string
|
||||
}{
|
||||
// Ensure that GetCommit with last pipeline id sets the pipeline id.
|
||||
{
|
||||
title: "GetCommit with a pipeline id",
|
||||
status: models.PendingCommitStatus,
|
||||
commitsWithNoLastPipeline: 0,
|
||||
expNumberOfRequests: 1,
|
||||
expRefOrPipelineId: "PipelineId",
|
||||
},
|
||||
// Ensure that 1 x GetCommit with no pipelines sets the pipeline id.
|
||||
{
|
||||
title: "1 x GetCommit with no last pipeline id",
|
||||
status: models.PendingCommitStatus,
|
||||
commitsWithNoLastPipeline: 1,
|
||||
expNumberOfRequests: 2,
|
||||
expRefOrPipelineId: "PipelineId",
|
||||
},
|
||||
// Ensure that 2 x GetCommit with no last pipeline id sets the ref.
|
||||
{
|
||||
title: "2 x GetCommit with no last pipeline id",
|
||||
status: models.PendingCommitStatus,
|
||||
commitsWithNoLastPipeline: 2,
|
||||
expNumberOfRequests: 2,
|
||||
expRefOrPipelineId: "Ref",
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
t.Run(c.title, func(t *testing.T) {
|
||||
handledNumberOfRequests := 0
|
||||
|
||||
testServer := httptest.NewServer(
|
||||
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch r.RequestURI {
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/statuses/sha":
|
||||
var updateStatusJsonBody UpdateStatusJsonBody
|
||||
err := json.NewDecoder(r.Body).Decode(&updateStatusJsonBody)
|
||||
Ok(t, err)
|
||||
|
||||
Equals(t, "running", updateStatusJsonBody.State)
|
||||
Equals(t, updateStatusSrc, updateStatusJsonBody.Context)
|
||||
Equals(t, updateStatusTargetUrl, updateStatusJsonBody.TargetUrl)
|
||||
Equals(t, updateStatusDescription, updateStatusJsonBody.Description)
|
||||
if c.expRefOrPipelineId == "Ref" {
|
||||
Equals(t, updateStatusHeadBranch, updateStatusJsonBody.Ref)
|
||||
} else {
|
||||
Equals(t, gitlabPipelineSuccessMrID, updateStatusJsonBody.PipelineId)
|
||||
}
|
||||
|
||||
defer r.Body.Close()
|
||||
|
||||
getCommitJsonResponse, err := json.Marshal(EmptyStruct{})
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/repository/commits/sha":
|
||||
handledNumberOfRequests++
|
||||
noCommitLastPipeline := handledNumberOfRequests <= c.commitsWithNoLastPipeline
|
||||
|
||||
w.WriteHeader(http.StatusOK)
|
||||
if noCommitLastPipeline {
|
||||
getCommitJsonResponse, err := json.Marshal(EmptyStruct{})
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
} else {
|
||||
getCommitResponse := GetCommitResponse{
|
||||
LastPipeline: GetCommitResponseLastPipeline{
|
||||
ID: gitlabPipelineSuccessMrID,
|
||||
},
|
||||
}
|
||||
getCommitJsonResponse, err := json.Marshal(getCommitResponse)
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
}
|
||||
|
||||
case "/api/v4/":
|
||||
// Rate limiter requests.
|
||||
w.WriteHeader(http.StatusOK)
|
||||
|
||||
default:
|
||||
t.Errorf("got unexpected request at %q", r.RequestURI)
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
}
|
||||
}))
|
||||
|
||||
internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL))
|
||||
Ok(t, err)
|
||||
|
||||
client := &GitlabClient{
|
||||
Client: internalClient,
|
||||
Version: nil,
|
||||
PollingInterval: 10 * time.Millisecond,
|
||||
}
|
||||
|
||||
repo := models.Repo{
|
||||
FullName: "runatlantis/atlantis",
|
||||
Owner: "runatlantis",
|
||||
Name: "atlantis",
|
||||
}
|
||||
|
||||
err = client.UpdateStatus(
|
||||
logger,
|
||||
repo,
|
||||
models.PullRequest{
|
||||
Num: 1,
|
||||
BaseRepo: repo,
|
||||
HeadCommit: "sha",
|
||||
HeadBranch: updateStatusHeadBranch,
|
||||
},
|
||||
c.status,
|
||||
updateStatusSrc,
|
||||
updateStatusDescription,
|
||||
updateStatusTargetUrl,
|
||||
)
|
||||
Ok(t, err)
|
||||
|
||||
Assert(t, c.expNumberOfRequests == handledNumberOfRequests,
|
||||
fmt.Sprintf("expected %d number of requests, but processed %d", c.expNumberOfRequests, handledNumberOfRequests))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGitlabClient_UpdateStatusSetCommitStatusConflictRetryable(t *testing.T) {
|
||||
logger := logging.NewNoopLogger(t)
|
||||
pipelineSuccess, err := os.ReadFile("testdata/gitlab-pipeline-success.json")
|
||||
Ok(t, err)
|
||||
|
||||
cases := []struct {
|
||||
status models.CommitStatus
|
||||
@@ -393,21 +583,40 @@ func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
|
||||
handledNumberOfRequests++
|
||||
shouldSendConflict := handledNumberOfRequests <= c.numberOfConflicts
|
||||
|
||||
body, err := io.ReadAll(r.Body)
|
||||
var updateStatusJsonBody UpdateStatusJsonBody
|
||||
err := json.NewDecoder(r.Body).Decode(&updateStatusJsonBody)
|
||||
Ok(t, err)
|
||||
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))
|
||||
|
||||
Equals(t, c.expState, updateStatusJsonBody.State)
|
||||
Equals(t, updateStatusSrc, updateStatusJsonBody.Context)
|
||||
Equals(t, updateStatusTargetUrl, updateStatusJsonBody.TargetUrl)
|
||||
Equals(t, updateStatusDescription, updateStatusJsonBody.Description)
|
||||
|
||||
defer r.Body.Close() // nolint: errcheck
|
||||
|
||||
if shouldSendConflict {
|
||||
w.WriteHeader(http.StatusConflict)
|
||||
}
|
||||
|
||||
w.Write([]byte("{}")) // nolint: errcheck
|
||||
getCommitJsonResponse, err := json.Marshal(EmptyStruct{})
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/merge_requests/1":
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/projects/runatlantis%2Fatlantis/repository/commits/sha":
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write(pipelineSuccess) // nolint: errcheck
|
||||
|
||||
getCommitResponse := GetCommitResponse{
|
||||
LastPipeline: GetCommitResponseLastPipeline{
|
||||
ID: gitlabPipelineSuccessMrID,
|
||||
},
|
||||
}
|
||||
getCommitJsonResponse, err := json.Marshal(getCommitResponse)
|
||||
Ok(t, err)
|
||||
|
||||
_, err = w.Write(getCommitJsonResponse)
|
||||
Ok(t, err)
|
||||
|
||||
case "/api/v4/":
|
||||
// Rate limiter requests.
|
||||
@@ -440,7 +649,12 @@ func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
|
||||
BaseRepo: repo,
|
||||
HeadCommit: "sha",
|
||||
HeadBranch: "test",
|
||||
}, c.status, "src", "description", "https://google.com")
|
||||
},
|
||||
c.status,
|
||||
updateStatusSrc,
|
||||
updateStatusDescription,
|
||||
updateStatusTargetUrl,
|
||||
)
|
||||
|
||||
if c.expError {
|
||||
ErrContains(t, "failed to update commit status for 'runatlantis/atlantis' @ 'sha' to 'src' after 10 attempts", err)
|
||||
@@ -449,7 +663,8 @@ func TestGitlabClient_UpdateStatusRetryable(t *testing.T) {
|
||||
Ok(t, err)
|
||||
}
|
||||
|
||||
Assert(t, c.expNumberOfRequests == handledNumberOfRequests, fmt.Sprintf("expected %d number of requests, but processed %d", c.expNumberOfRequests, handledNumberOfRequests))
|
||||
Assert(t, c.expNumberOfRequests == handledNumberOfRequests,
|
||||
fmt.Sprintf("expected %d number of requests, but processed %d", c.expNumberOfRequests, handledNumberOfRequests))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user