Atlantis v0.27.0 hits nil pointer dereference panic on github client

[adkafka]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Overview of the Issue

Since upgrading our Atlantis to v0.27.0 yesterday, we have seen at least two panics logged with the same stack trace:

runtime error: invalid memory address or nil pointer dereference
runtime/panic.go:261 (0x450737)
runtime/signal_unix.go:861 (0x450705)
github.com/runatlantis/atlantis/server/events/vcs/github_client.go:583 (0xd95a0c)
github.com/runatlantis/atlantis/server/events/vcs/instrumented_client.go:237 (0xd9f6cf)
github.com/runatlantis/atlantis/server/events/vcs/proxy.go:84 (0xda0f2c)
github.com/runatlantis/atlantis/server/events/commit_status_updater.go:103 (0xfb6a77)
github.com/runatlantis/atlantis/server/jobs/job_url_setter.go:41 (0xc40f42)
github.com/runatlantis/atlantis/server/events/project_command_runner.go:194 (0xfd99ea)
github.com/runatlantis/atlantis/server/events/project_command_runner.go:164 (0xfd9404)
github.com/runatlantis/atlantis/server/events/instrumented_project_command_runner.go:74 (0xfc01d9)
github.com/runatlantis/atlantis/server/events/instrumented_project_command_runner.go:38 (0xfbf724)
github.com/runatlantis/atlantis/server/events/project_command_pool_executor.go:48 (0xfd8858)
github.com/runatlantis/atlantis/server/events/plan_command_runner.go:135 (0xfc6192)
github.com/runatlantis/atlantis/server/events/plan_command_runner.go:304 (0xfc899c)
github.com/runatlantis/atlantis/server/events/command_runner.go:221 (0xfac14c)
runtime/asm_amd64.s:1650 (0x46cb00)

This pairs with a comment on the github PR, showing the panic:

As indicated by the stack trace, the panic is occurring when atlantis is updating the status of the PR on github.

The last line of the stack trace points to this line of code:
https://github.com/runatlantis/atlantis/blob/v0.27.0/server/events/vcs/github_client.go#L583

Which was added via this PR: #3876

I believe the issue is that when we go to log this, we are not checking if resp is non-null. In this case, I believe there was an error in the previous line, resulting in resp being null, and therefore resp.StatusCode is throwing a panic.

Reproduction Steps

I believe this issue can be reproduced by running Atlantis v0.27.0, and having the prior line return an error

https://github.com/runatlantis/atlantis/blob/v0.27.0/server/events/vcs/github_client.go#L582

Logs

Logs

models/shell_command_runner.go:161    successfully ran "/atlantis-data/bin/terraform1.6.6 plan REDACTED"
vcs/instrumented_client.go:236    updating vcs status
runtime error: invalid memory address or nil pointer dereference
runtime/panic.go:261 (0x450737)
runtime/signal_unix.go:861 (0x450705)
github.com/runatlantis/atlantis/server/events/vcs/github_client.go:583 (0xd95a0c)
github.com/runatlantis/atlantis/server/events/vcs/instrumented_client.go:237 (0xd9f6cf)
github.com/runatlantis/atlantis/server/events/vcs/proxy.go:84 (0xda0f2c)
github.com/runatlantis/atlantis/server/events/commit_status_updater.go:103 (0xfb6a77)
github.com/runatlantis/atlantis/server/jobs/job_url_setter.go:41 (0xc40f42)
github.com/runatlantis/atlantis/server/events/project_command_runner.go:194 (0xfd99ea)
github.com/runatlantis/atlantis/server/events/project_command_runner.go:164 (0xfd9404)
github.com/runatlantis/atlantis/server/events/instrumented_project_command_runner.go:74 (0xfc01d9)
github.com/runatlantis/atlantis/server/events/instrumented_project_command_runner.go:38 (0xfbf724)
github.com/runatlantis/atlantis/server/events/project_command_pool_executor.go:48 (0xfd8858)
github.com/runatlantis/atlantis/server/events/plan_command_runner.go:135 (0xfc6192)
github.com/runatlantis/atlantis/server/events/plan_command_runner.go:304 (0xfc899c)
github.com/runatlantis/atlantis/server/events/command_runner.go:221 (0xfac14c)
runtime/asm_amd64.s:1650 (0x46cb00)

Environment details

• Atlantis version: v0.27.0
• Deployment method: eks
• If not running the latest Atlantis version have you tried to reproduce this issue on the latest version: N/A
• Atlantis flags: Not relevant

Additional Context

Related issues / PRs:
#3876
Sounds similar to #3756, but likely a different root cause

[lukemassa]

Ah yeah that's a good catch. It looks like a number of the debug lines added by #3876 are going to have similar problems. I wonder what people think about adding a helper function:

func (g *GithubClient) logAPICall(prefix, path string, resp *github.Response, err error) {
	format := fmt.Sprintf("%s %s returned: %%v", prefix, path)
	if err != nil {
		g.logger.Debug(format, err)
		return
	}
	g.logger.Debug(format, resp.StatusCode)
}

Then translating calls like:

g.logger.Debug("POST /repos/%v/%v/issues/%d/comments returned: %v", repo.Owner, repo.Name, pullNum, resp.StatusCode)

To

g.logAPICall("POST", fmt.Sprintf("/repos/%v/%v/issues/%d/comments", repo.Owner, repo.Name, pullNum), resp, err)

Otherwise we'll have to check the err before logging every time. Unless there's a cleaner way to do it?

[adkafka]

I opened a PR with a proposed fix for this issue: #4082

It is a simple fix, and there is a chance I missed one. But we can at least start from there.