Incorrect apply error message for unsigned commits

[dannyrife]

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

chart version: 4.0.5 and image version v0.19.6
Trying an atlantis apply via github comment failed with error "Apply Failed: Pull request must be approved by at least one person other than the author before running apply." This message was incorrect because the PR was correctly reviewed, but the repo required signed commit messages, there error stopping merge was

Merging is blocked: The base branch requires all commits to be signed. Learn more about signing commits.

seems like it hits

atlantis/server/events/apply_requirement_handler.go

Line 24 in 90e92e3

return "Pull request must be approved by at least one person other than the author before running apply.", nil

The message about the PR not being mergeable would have been better since it would prompt the user to look at why it wasn't mergeable.

Reproduction Steps

chart version: 4.0.5 and image version v0.19.6
Create a repo with a .CODEOWNERS setup and requiring verified signed commits for merging. Create a PR that requires atlantis changes but without signing the commit. Attempt to do an atlantis apply.

Environment details

chart version: 4.0.5 and image version v0.19.6

[magneland]

👍

[jamengual]

if you want to give it a go and create a PR, it will be welcome.

[nitrocode]

I went through the code a little bit because I was curious too. This may help anyone who wants to contribute to this ticket.

The block you mentioned shows that IsApproved = false. We may need another boolean like IsApprovedWithSignedCommits ?

atlantis/server/events/apply_requirement_handler.go

Lines 19 to 25 in 90e92e3

	func (a *AggregateApplyRequirements) ValidateProject(repoDir string, ctx command.ProjectContext) (failure string, err error) {
	for _, req := range ctx.ApplyRequirements {
	switch req {
	case raw.ApprovedApplyRequirement:
	if !ctx.PullReqStatus.ApprovalStatus.IsApproved {
	return "Pull request must be approved by at least one person other than the author before running apply.", nil
	}

The approvalStatus holds the information of this logic

atlantis/server/events/vcs/pull_status_fetcher.go

Lines 22 to 23 in 4cacaeb

	func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (pullStatus models.PullReqStatus, err error) {
	approvalStatus, err := f.client.PullIsApproved(repo, pull)

Which is received from the corresponding VCS (github)

atlantis/server/events/vcs/pull_status_fetcher.go

Lines 22 to 37 in 4cacaeb

	func (f *pullReqStatusFetcher) FetchPullStatus(repo models.Repo, pull models.PullRequest, vcsstatusname string) (pullStatus models.PullReqStatus, err error) {
	approvalStatus, err := f.client.PullIsApproved(repo, pull)
	if err != nil {
	return pullStatus, errors.Wrapf(err, "fetching pull approval status for repo: %s, and pull number: %d", repo.FullName, pull.Num)
	}
	mergeable, err := f.client.PullIsMergeable(repo, pull, vcsstatusname)
	if err != nil {
	return pullStatus, errors.Wrapf(err, "fetching mergeability status for repo: %s, and pull number: %d", repo.FullName, pull.Num)
	}
	return models.PullReqStatus{
	ApprovalStatus: approvalStatus,
	Mergeable: mergeable,
	}, err
	}

[dgteixeira]

@dannyrife , can you share the repo side configurations you have for this?

We currently are trying to avoid Atlantis to apply whenever commits are not verified, but it applies anyway. It just fails when trying to merge the PR, which is why I opened this issue #2017