Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

marcus-rev
Copy link

@marcus-rev marcus-rev commented Aug 16, 2024

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 /api/plan throws 500 error when using GitHub App  #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

…logic around prNumber=0.

- Updating mergeToBaseBranch to only look for PR refs if GitHub app is enabled _and_ the plan/apply includes a PR number, which can optionally be 0.
@marcus-rev marcus-rev requested review from a team as code owners August 16, 2024 19:50
@marcus-rev marcus-rev requested review from chenrui333, lukemassa and nitrocode and removed request for a team August 16, 2024 19:50
@github-actions github-actions bot added the go Pull requests that update Go code label Aug 16, 2024
jamengual and others added 6 commits August 16, 2024 20:35
…included in the API call. When checking out a branch directly without a PR number, HEAD should be fine vs HEAD^2.
… plan. This eliminates the following error in Atlantis server: 422 Validation Failed [{Resource:Status Field:sha Code:custom Message:sha must be a 40 character SHA1}]
@X-Guardian
Copy link
Contributor

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.

@X-Guardian X-Guardian added waiting-on-response Waiting for a response from the user needs tests Change requires tests labels Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code needs tests Change requires tests waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/api/plan throws 500 error when using GitHub App
3 participants