public/atlantis
1
0
Fork 0
mirror of https://git.vectorsigma.ru/public/atlantis.git synced 2026-08-05 04:38:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Prevent panics when logging HTTP response status in github and gitlab client (#4082)

Browse Source 
Fix potential nil pointers. See https://github.com/runatlantis/atlantis/issues/4081 for context.
This commit is contained in:
Adam Kafka
2023-12-22 08:36:04 -08:00
committed by GitHub
parent 1035d9263a
commit f5d92c60b5
2 changed files with 103 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
server/events/vcs/github_client.go
View File

@@ -141,7 +141,9 @@ listloop:
attemptDelay = 2*attemptDelay + 1*time.Second
pageFiles, resp, err := g.client.PullRequests.ListFiles(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
g.logger.Debug("[attempt %d] GET /repos/%v/%v/pulls/%d/files returned: %v", i+1, repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("[attempt %d] GET /repos/%v/%v/pulls/%d/files returned: %v", i+1, repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
ghErr, ok := err.(*github.ErrorResponse)
if ok && ghErr.Response.StatusCode == 404 {
@@ -190,7 +192,9 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri
comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
for i := range comments {
_, resp, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueComment{Body: &comments[i]})
g.logger.Debug("POST /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
}
if err != nil {
return err
}
@@ -201,7 +205,9 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri
// ReactToComment adds a reaction to a comment.
func (g *GithubClient) ReactToComment(repo models.Repo, _ int, commentID int64, reaction string) error {
_, resp, err := g.client.Reactions.CreateIssueCommentReaction(g.ctx, repo.Owner, repo.Name, commentID, reaction)
g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions returned: %v", repo.Owner, repo.Name, commentID, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/issues/comments/%d/reactions returned: %v", repo.Owner, repo.Name, commentID, resp.StatusCode)
}
return err
}
@@ -214,7 +220,9 @@ func (g *GithubClient) HidePrevCommandComments(repo models.Repo, pullNum int, co
Direction: github.String("asc"),
ListOptions: github.ListOptions{Page: nextPage},
})
g.logger.Debug("GET /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "listing comments")
}
@@ -328,7 +336,9 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
opts.Page = nextPage
}
pageReviews, resp, err := g.client.PullRequests.ListReviews(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
g.logger.Debug("GET /repos/%v/%v/pulls/%d/reviews returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d/reviews returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return approvalStatus, errors.Wrap(err, "getting reviews")
}
@@ -398,7 +408,9 @@ func isRequiredCheck(check string, required []string) bool {
func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *github.PullRequest, vcstatusname string) (bool, error) {
//check combined status api
status, resp, err := g.client.Repositories.GetCombinedStatus(g.ctx, *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, nil)
g.logger.Debug("GET /repos/%v/%v/commits/%s/status returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/commits/%s/status returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting combined status")
}
@@ -415,7 +427,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu
//get required status checks
required, resp, err := g.client.Repositories.GetBranchProtection(context.Background(), repo.Owner, repo.Name, *pull.Base.Ref)
g.logger.Debug("GET /repos/%v/%v/branches/%s/protection returned: %v", repo.Owner, repo.Name, *pull.Base.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/branches/%s/protection returned: %v", repo.Owner, repo.Name, *pull.Base.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting required status checks")
}
@@ -426,7 +440,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu
//check check suite/check run api
checksuites, resp, err := g.client.Checks.ListCheckSuitesForRef(context.Background(), *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, nil)
g.logger.Debug("GET /repos/%v/%v/commits/%s/check-suites returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/commits/%s/check-suites returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *pull.Head.Ref, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting check suites for ref")
}
@@ -436,7 +452,9 @@ func (g *GithubClient) GetCombinedStatusMinusApply(repo models.Repo, pull *githu
if *c.Status == "completed" {
//iterate over the runs inside the suite
suite, resp, err := g.client.Checks.ListCheckRunsCheckSuite(context.Background(), *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, nil)
g.logger.Debug("GET /repos/%v/%v/check-suites/%d/check-runs returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/check-suites/%d/check-runs returned: %v", *pull.Head.Repo.Owner.Login, repo.Name, *c.ID, resp.StatusCode)
}
if err != nil {
return false, errors.Wrap(err, "getting check runs for check suite")
}
@@ -548,7 +566,9 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe
attemptDelay = 2*attemptDelay + 1*time.Second
pull, resp, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num)
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, num, resp.StatusCode)
}
if err == nil {
return pull, nil
}
@@ -580,7 +600,9 @@ func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, s
TargetURL: &url,
}
_, resp, err := g.client.Repositories.CreateStatus(g.ctx, repo.Owner, repo.Name, pull.HeadCommit, status)
g.logger.Debug("POST /repos/%v/%v/statuses/%s returned: %v", repo.Owner, repo.Name, pull.HeadCommit, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/statuses/%s returned: %v", repo.Owner, repo.Name, pull.HeadCommit, resp.StatusCode)
}
return err
}
@@ -589,7 +611,9 @@ func (g *GithubClient) MergePull(pull models.PullRequest, _ models.PullRequestOp
// Users can set their repo to disallow certain types of merging.
// We detect which types aren't allowed and use the type that is.
repo, resp, err := g.client.Repositories.Get(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name)
g.logger.Debug("GET /repos/%v/%v returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "fetching repo info")
}
@@ -621,7 +645,9 @@ func (g *GithubClient) MergePull(pull models.PullRequest, _ models.PullRequestOp
// the commit message as it normally would.
"",
options)
g.logger.Debug("POST /repos/%v/%v/pulls/%d/merge returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /repos/%v/%v/pulls/%d/merge returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "merging pull request")
}
@@ -683,7 +709,9 @@ func (g *GithubClient) GetTeamNamesForUser(repo models.Repo, user models.User) (
func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, error) {
ctx := context.Background()
cfg, resp, err := g.client.Apps.CompleteAppManifest(ctx, code)
g.logger.Debug("POST /app-manifests/%s/conversions returned: %v", code, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /app-manifests/%s/conversions returned: %v", code, resp.StatusCode)
}
data := &GithubAppTemporarySecrets{
ID: cfg.GetID(),
Key: cfg.GetPEM(),
@@ -701,7 +729,9 @@ func (g *GithubClient) ExchangeCode(code string) (*GithubAppTemporarySecrets, er
func (g *GithubClient) GetFileContent(pull models.PullRequest, fileName string) (bool, []byte, error) {
opt := github.RepositoryContentGetOptions{Ref: pull.HeadBranch}
fileContent, _, resp, err := g.client.Repositories.GetContents(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, &opt)
g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/contents/%s returned: %v", pull.BaseRepo.Owner, pull.BaseRepo.Name, fileName, resp.StatusCode)
}
if resp.StatusCode == http.StatusNotFound {
return false, []byte{}, nil
@@ -725,7 +755,9 @@ func (g *GithubClient) SupportsSingleFileDownload(_ models.Repo) bool {
func (g *GithubClient) GetCloneURL(_ models.VCSHostType, repo string) (string, error) {
parts := strings.Split(repo, "/")
repository, resp, err := g.client.Repositories.Get(g.ctx, parts[0], parts[1])
g.logger.Debug("GET /repos/%v/%v returned: %v", parts[0], parts[1], resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v returned: %v", parts[0], parts[1], resp.StatusCode)
}
if err != nil {
return "", err
}
@@ -734,7 +766,9 @@ func (g *GithubClient) GetCloneURL(_ models.VCSHostType, repo string) (string, e
func (g *GithubClient) GetPullLabels(repo models.Repo, pull models.PullRequest) ([]string, error) {
pullDetails, resp, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, pull.Num)
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /repos/%v/%v/pulls/%d returned: %v", repo.Owner, repo.Name, pull.Num, resp.StatusCode)
}
if err != nil {
return nil, err
}

70
server/events/vcs/gitlab_client.go
View File

@@ -139,7 +139,9 @@ func (g *GitlabClient) GetModifiedFiles(repo models.Repo, pull models.PullReques
pollingStart := time.Now()
for {
resp, err = g.Client.Do(req, mr)
g.logger.Debug("GET %s returned: %d", apiURL, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET %s returned: %d", apiURL, resp.StatusCode)
}
if err != nil {
return nil, err
}
@@ -179,7 +181,9 @@ func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment stri
comments := common.SplitComment(comment, gitlabMaxCommentLength, sepEnd, sepStart)
for _, c := range comments {
_, resp, err := g.Client.Notes.CreateMergeRequestNote(repo.FullName, pullNum, &gitlab.CreateMergeRequestNoteOptions{Body: gitlab.Ptr(c)})
g.logger.Debug("POST /projects/%s/merge_requests/%d/notes returned: %d", repo.FullName, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /projects/%s/merge_requests/%d/notes returned: %d", repo.FullName, pullNum, resp.StatusCode)
}
if err != nil {
return err
}
@@ -190,7 +194,9 @@ func (g *GitlabClient) CreateComment(repo models.Repo, pullNum int, comment stri
// ReactToComment adds a reaction to a comment.
func (g *GitlabClient) ReactToComment(repo models.Repo, pullNum int, commentID int64, reaction string) error {
_, resp, err := g.Client.AwardEmoji.CreateMergeRequestAwardEmojiOnNote(repo.FullName, pullNum, int(commentID), &gitlab.CreateAwardEmojiOptions{Name: reaction})
g.logger.Debug("POST /projects/%s/merge_requests/%d/notes/%d/award_emoji returned: %d", repo.FullName, pullNum, commentID, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /projects/%s/merge_requests/%d/notes/%d/award_emoji returned: %d", repo.FullName, pullNum, commentID, resp.StatusCode)
}
return err
}
@@ -206,7 +212,9 @@ func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, co
OrderBy: gitlab.Ptr("created_at"),
ListOptions: gitlab.ListOptions{Page: nextPage},
})
g.logger.Debug("GET /projects/%s/merge_requests/%d/notes returned: %d", repo.FullName, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s/merge_requests/%d/notes returned: %d", repo.FullName, pullNum, resp.StatusCode)
}
if err != nil {
return errors.Wrap(err, "listing comments")
}
@@ -246,7 +254,9 @@ func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, co
supersededComment := summaryHeader + lineFeed + comment.Body + lineFeed + summaryFooter + lineFeed
_, resp, err := g.Client.Notes.UpdateMergeRequestNote(repo.FullName, pullNum, comment.ID, &gitlab.UpdateMergeRequestNoteOptions{Body: &supersededComment})
g.logger.Debug("PUT /projects/%s/merge_requests/%d/notes/%d returned: %d", repo.FullName, pullNum, comment.ID, resp.StatusCode)
if resp != nil {
g.logger.Debug("PUT /projects/%s/merge_requests/%d/notes/%d returned: %d", repo.FullName, pullNum, comment.ID, resp.StatusCode)
}
if err != nil {
return errors.Wrapf(err, "updating comment %d", comment.ID)
}
@@ -258,7 +268,9 @@ func (g *GitlabClient) HidePrevCommandComments(repo models.Repo, pullNum int, co
// PullIsApproved returns true if the merge request was approved.
func (g *GitlabClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (approvalStatus models.ApprovalStatus, err error) {
approvals, resp, err := g.Client.MergeRequests.GetMergeRequestApprovals(repo.FullName, pull.Num)
g.logger.Debug("GET /projects/%s/merge_requests/%d/approvals returned: %d", repo.FullName, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s/merge_requests/%d/approvals returned: %d", repo.FullName, pull.Num, resp.StatusCode)
}
if err != nil {
return approvalStatus, err
}
@@ -283,7 +295,9 @@ func (g *GitlabClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
// - https://gitlab.com/gitlab-org/gitlab-ce/issues/42344
func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest, vcsstatusname string) (bool, error) {
mr, resp, err := g.Client.MergeRequests.GetMergeRequest(repo.FullName, pull.Num, nil)
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repo.FullName, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repo.FullName, pull.Num, resp.StatusCode)
}
if err != nil {
return false, err
}
@@ -299,14 +313,18 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
// Get project configuration
project, resp, err := g.Client.Projects.GetProject(mr.ProjectID, nil)
g.logger.Debug("GET /projects/%d returned: %d", mr.ProjectID, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%d returned: %d", mr.ProjectID, resp.StatusCode)
}
if err != nil {
return false, err
}
// Get Commit Statuses
statuses, _, err := g.Client.Commits.GetCommitStatuses(mr.ProjectID, commit, nil)
g.logger.Debug("GET /projects/%d/commits/%s/statuses returned: %d", mr.ProjectID, commit, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%d/commits/%s/statuses returned: %d", mr.ProjectID, commit, resp.StatusCode)
}
if err != nil {
return false, err
}
@@ -407,13 +425,17 @@ func (g *GitlabClient) UpdateStatus(repo models.Repo, pull models.PullRequest, s
TargetURL: &url,
Ref: gitlab.Ptr(refTarget),
})
g.logger.Debug("POST /projects/%s/statuses/%s returned: %d", repo.FullName, pull.HeadCommit, resp.StatusCode)
if resp != nil {
g.logger.Debug("POST /projects/%s/statuses/%s returned: %d", repo.FullName, pull.HeadCommit, resp.StatusCode)
}
return err
}
func (g *GitlabClient) GetMergeRequest(repoFullName string, pullNum int) (*gitlab.MergeRequest, error) {
mr, resp, err := g.Client.MergeRequests.GetMergeRequest(repoFullName, pullNum, nil)
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repoFullName, pullNum, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repoFullName, pullNum, resp.StatusCode)
}
return mr, err
}
@@ -449,7 +471,9 @@ func (g *GitlabClient) MergePull(pull models.PullRequest, pullOptions models.Pul
err, "unable to merge merge request, it was not possible to retrieve the merge request")
}
project, resp, err := g.Client.Projects.GetProject(mr.ProjectID, nil)
g.logger.Debug("GET /projects/%d returned: %d", mr.ProjectID, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%d returned: %d", mr.ProjectID, resp.StatusCode)
}
if err != nil {
return errors.Wrap(
err, "unable to merge merge request, it was not possible to check the project requirements")
@@ -466,7 +490,9 @@ func (g *GitlabClient) MergePull(pull models.PullRequest, pullOptions models.Pul
MergeCommitMessage: &commitMsg,
ShouldRemoveSourceBranch: &pullOptions.DeleteSourceBranchOnMerge,
})
g.logger.Debug("PUT /projects/%s/merge_requests/%d/merge returned: %d", pull.BaseRepo.FullName, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("PUT /projects/%s/merge_requests/%d/merge returned: %d", pull.BaseRepo.FullName, pull.Num, resp.StatusCode)
}
return errors.Wrap(err, "unable to merge merge request, it may not be in a mergeable state")
}
@@ -483,7 +509,9 @@ func (g *GitlabClient) DiscardReviews(_ models.Repo, _ models.PullRequest) error
// GetVersion returns the version of the Gitlab server this client is using.
func (g *GitlabClient) GetVersion() (*version.Version, error) {
versionResp, resp, err := g.Client.Version.GetVersion()
g.logger.Debug("GET /version returned: %d", resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /version returned: %d", resp.StatusCode)
}
if err != nil {
return nil, err
}
@@ -531,8 +559,10 @@ func (g *GitlabClient) GetFileContent(pull models.PullRequest, fileName string)
opt := gitlab.GetRawFileOptions{Ref: gitlab.Ptr(pull.HeadBranch)}
bytes, resp, err := g.Client.RepositoryFiles.GetRawFile(pull.BaseRepo.FullName, fileName, &opt)
g.logger.Debug("GET /projects/%s/repository/files/%s/raw returned: %d", pull.BaseRepo.FullName, fileName, resp.StatusCode)
if resp.StatusCode == http.StatusNotFound {
if resp != nil {
g.logger.Debug("GET /projects/%s/repository/files/%s/raw returned: %d", pull.BaseRepo.FullName, fileName, resp.StatusCode)
}
if resp != nil && resp.StatusCode == http.StatusNotFound {
return false, []byte{}, nil
}
@@ -549,7 +579,9 @@ func (g *GitlabClient) SupportsSingleFileDownload(_ models.Repo) bool {
func (g *GitlabClient) GetCloneURL(_ models.VCSHostType, repo string) (string, error) {
project, resp, err := g.Client.Projects.GetProject(repo, nil)
g.logger.Debug("GET /projects/%s returned: %d", repo, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s returned: %d", repo, resp.StatusCode)
}
if err != nil {
return "", err
}
@@ -558,7 +590,9 @@ func (g *GitlabClient) GetCloneURL(_ models.VCSHostType, repo string) (string, e
func (g *GitlabClient) GetPullLabels(repo models.Repo, pull models.PullRequest) ([]string, error) {
mr, resp, err := g.Client.MergeRequests.GetMergeRequest(repo.FullName, pull.Num, nil)
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repo.FullName, pull.Num, resp.StatusCode)
if resp != nil {
g.logger.Debug("GET /projects/%s/merge_requests/%d returned: %d", repo.FullName, pull.Num, resp.StatusCode)
}
if err != nil {
return nil, err