Skip to content

Commit

Permalink
fix(gitlab): Prevent nil pointer dereference when HeadPipeline is empty
Browse files Browse the repository at this point in the history
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.

```
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:
- runatlantis#1852
  • Loading branch information
marceloboeira committed Jan 6, 2022
1 parent 767a5e7 commit 9183e65
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
6 changes: 3 additions & 3 deletions scripts/e2e-deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ chmod +x terraform
cp terraform /go/bin/
# Download ngrok to create a tunnel to expose atlantis server
wget https://bin.equinox.io/c/4VmDzA7iaHb/ngrok-stable-linux-amd64.zip
unzip ngrok-stable-linux-amd64.zip
chmod +x ngrok
unzip ngrok-stable-linux-amd64.zip
chmod +x ngrok
wget https://stedolan.github.io/jq/download/linux64/jq
chmod +x jq
# Copy github config file - replace with circleci user later
cp .gitconfig ~/.gitconfig
cp .gitconfig ~/.gitconfig
12 changes: 10 additions & 2 deletions server/events/vcs/gitlab_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,23 @@ 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 {
return false, err
}

// 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
}
Expand All @@ -218,7 +227,6 @@ func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest
}
}

isPipelineSkipped := mr.HeadPipeline.Status == "skipped"
allowSkippedPipeline := project.AllowMergeOnSkippedPipeline && isPipelineSkipped
if mr.MergeStatus == "can_be_merged" &&
mr.ApprovalsBeforeMerge <= 0 &&
Expand Down

0 comments on commit 9183e65

Please sign in to comment.