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

feat(api): change the defaults for BaseBranch and HeadCommit #3984

Closed
wants to merge 1 commit into from

Conversation

rgs1
Copy link

@rgs1 rgs1 commented Nov 16, 2023

As of now, the /api/plan endpoint constructs a PullRequest that uses the provided Ref parameter as the base branch and head commit. This causes issues when running additional steps along with plan (e.g.: policy check, infracost, etc).

Here's an example error:

repo was already cloned but is not at correct commit, wanted \"pr-plan-test\" got \"7a19f2011...\"

This will then trigger a new clone which will override needed artifacts from the previous steps which will then cause /api/plan to fail.

By properly setting HeadCommit to HEAD this issue is avoided.

@rgs1 rgs1 requested a review from a team as a code owner November 16, 2023 15:51
@github-actions github-actions bot added the go Pull requests that update Go code label Nov 16, 2023
As of now, the /api/plan endpoint constructs a PullRequest
that uses the provided Ref parameter as the base branch
and head commit. This causes issues when running additional
steps along with plan (e.g.: policy check, infracost, etc).

Here's an example error:

```
repo was already cloned but is not at correct commit, wanted \"pr-plan-test\" got \"7a19f2011...\"
```

This will then trigger a new clone which will override needed artifacts from the
previous steps which will then cause /api/plan to fail.

By properly setting HeadCommit to HEAD this issue is avoided.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
@rgs1 rgs1 force-pushed the fix-defaults-plan-api branch from fe7f367 to 355bfaa Compare November 16, 2023 16:55
@rgs1 rgs1 changed the title API: change the defaults for BaseBranch and HeadCommit feat(api): change the defaults for BaseBranch and HeadCommit Nov 16, 2023
@jamengual jamengual added waiting-on-review Waiting for a review from a maintainer api-endpoints Adding API endpoints to Atlantis labels Nov 16, 2023
@jamengual
Copy link
Contributor

What would happen to people where the base branch is still master or something else?

@rgs1
Copy link
Author

rgs1 commented Nov 16, 2023

This is actually not quite right, closing for now and will follow-up with the right fix.

@rgs1 rgs1 closed this Nov 16, 2023
@rgs1
Copy link
Author

rgs1 commented Nov 16, 2023

What would happen to people where the base branch is still master or something else?

Sorry closed before seeing this comment -- yeah I think the right approach is to extend APIRequest to include BaseBranch and HeadCommit as optional parameters. Testing this locally with atlantis-drift-detection as the client to see how it all works together.

@jamengual
Copy link
Contributor

Thanks @rgs1, we have been trying to implement Drift detection in atlantis natively since https://github.com/cresta/atlantis-drift-detection came out, if you have time and know golang well, contact me in Slack.

@rgs1
Copy link
Author

rgs1 commented Nov 16, 2023

Follow-up in #3985.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-endpoints Adding API endpoints to Atlantis go Pull requests that update Go code waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants