fix: update FileWorkspace logic to use pull request refs only when PR number is specified - Githubissues

runatlantis / atlantis

Terraform Pull Request Automation

https://www.runatlantis.io

Other

7.83k stars 1.06k forks source link

fix: update FileWorkspace logic to use pull request refs only when PR number is specified #4851

Open marcus-rev opened 3 months ago

marcus-rev commented 3 months ago

what

Updating mergeToBaseBranch logic to only checkout by pull/{pr.number}/head if there is indeed a pull request number specified as part of the plan/apply.
This change should mitigate this error:

failed to build command: running git fetch origin pull/0/head:: fatal: couldn't find remote ref pull/0/head

Updating clone logic to skip using HEAD^2 when using the checkout merge strategy whilst no PR number is specified. If a plan is triggered remotely on main for instance, this leads to 128 errors from git:

will re-clone repo, could not determine if was at correct commit: git rev-parse HEAD^2: exit status 128: fatal: ambiguous argument 'HEAD^2': unknown revision or path not in the working tree.

Updating logic to skip making a VCS call when there isn't a pull request. This mitigates this error:

updating project PR status%!(EXTRA *github.ErrorResponse=POST https://api.github.com/repos/<your-org>/<your-terraform-repo>/statuses/master: 422 Validation Failed [{Resource:Status Field:sha Code:custom Message:sha must be a 40 character SHA1}])...

why

This should fix the issue detailed in #4850
Per the docs, POSTs to /api/plan and /api/apply can optionally omit the PR parameter. Therefore, we should check to see if it's 0 before pivoting to a pull request based reference checkout.
We shouldn't make calls to VCS if there is no pull request to update statuses on. This leads to 422 errors that can be avoided.

tests

references

https://github.com/runatlantis/atlantis/blob/6fe0303279839faadb76e94de94d7c250876b336/runatlantis.io/docs/api-endpoints.md#post-apiplan

Should close #4850

X-Guardian commented 2 weeks ago

Thanks for this @marcus-rev. Can you add a unit test for this change, and confirm in the PR description that you have tested this in your own local environment.