From e42be0e20d978c14bc6069f69e98f81bcbb957f0 Mon Sep 17 00:00:00 2001 From: Marcelo Boeira Date: Fri, 4 Aug 2023 11:36:30 +0200 Subject: [PATCH] fix(gitlab): Prevent nil pointer dereference when HeadPipeline is empty In this particular example `mr.HeadPipeline.SHA` panics on a nil pointer dereference because HeadPipeline is empty. This seems to be caused by the lack of permission to update the commit status. ```go runtime.gopanic runtime/panic.go:1038 runtime.panicmem runtime/panic.go:221 runtime.sigpanic runtime/signal_unix.go:735 github.com/runatlantis/atlantis/server/events/vcs.(*GitlabClient).PullIsMergeable github.com/runatlantis/atlantis/server/events/vcs/gitlab_client.go:208 github.com/runatlantis/atlantis/server/events/vcs.(*ClientProxy).PullIsMergeable github.com/runatlantis/atlantis/server/events/vcs/proxy.go:72 github.com/runatlantis/atlantis/server/events/vcs.(*pullReqStatusFetcher).FetchPullStatus github.com/runatlantis/atlantis/server/events/vcs/pull_status_fetcher.go:28 github.com/runatlantis/atlantis/server/events.(*ApplyCommandRunner).Run github.com/runatlantis/atlantis/server/events/apply_command_runner.go:105 github.com/runatlantis/atlantis/server/events.(*DefaultCommandRunner).RunCommentCommand github.com/runatlantis/atlantis/server/events/command_runner.go:252 ``` The least invasive solution is to simply use the commit-hash from pull and guess that the pipeline was "skipped" unless the HeadPipeline is there. The outcome is: When mr.HeadPipeline is present: - use the commit hash and status from the HeadPipeline When mr.HeadPipeline is NOT present: - use the commit hash from pull request struct - assume the pipeline was "skipped" In cases where GitLab is configured to require a pipeline to pass, this results on a message saying the MR is not mergeable. More info: - https://github.com/runatlantis/atlantis/issues/1852 --- scripts/e2e-deps.sh | 2 +- server/events/vcs/gitlab_client.go | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/scripts/e2e-deps.sh b/scripts/e2e-deps.sh index b01559bad9..4ac9b9272b 100755 --- a/scripts/e2e-deps.sh +++ b/scripts/e2e-deps.sh @@ -18,7 +18,7 @@ unzip terraform_${TERRAFORM_VERSION}_linux_amd64.zip chmod +x terraform cp terraform /home/circleci/go/bin # Download ngrok to create a tunnel to expose atlantis server -wget https://bin.equinox.io/c/bNyj1mQVY4c/ngrok-v2-stable-linux-amd64.zip -O ngrok-stable-linux-amd64.zip +wget https://bin.equinox.io/c/bNyj1mQVY4c/ngrok-v2-stable-linux-amd64.zip -O ngrok-stable-linux-amd64.zip unzip ngrok-stable-linux-amd64.zip chmod +x ngrok # Download jq diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index cd1a05645c..f96d964fda 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -280,6 +280,15 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest return false, err } + // Prevent nil pointer error when mr.HeadPipeline is empty + // See: https://github.com/runatlantis/atlantis/issues/1852 + commit := pull.HeadCommit + isPipelineSkipped := true + if mr.HeadPipeline != nil { + commit = mr.HeadPipeline.SHA + isPipelineSkipped = mr.HeadPipeline.Status == "skipped" + } + // Get project configuration project, _, err := g.Client.Projects.GetProject(mr.ProjectID, nil) if err != nil { @@ -287,7 +296,7 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest } // Get Commit Statuses - statuses, _, err := g.Client.Commits.GetCommitStatuses(mr.ProjectID, mr.HeadPipeline.SHA, nil) + statuses, _, err := g.Client.Commits.GetCommitStatuses(mr.ProjectID, commit, nil) if err != nil { return false, err } @@ -302,7 +311,6 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest } } - isPipelineSkipped := mr.HeadPipeline.Status == "skipped" allowSkippedPipeline := project.AllowMergeOnSkippedPipeline && isPipelineSkipped ok, err := g.SupportsDetailedMergeStatus()