fix: Allow and encourage explicitly setting SHA in API requests

[lukaspj]

what

I propose adding the Commit SHA as a required field on API requests.

why

When running API Requests, we are currently only requiring the following fields:

type APIRequest struct {
	Repository string `validate:"required"`
	Ref          string `validate:"required"`
	Type       string `validate:"required"`
	PR           int
	Projects  []string
	Paths      []struct {
		Directory string
		Workspace string
	}
}

However, this is not sufficient information as many operations rely on knowing the exact commit we are working on and not just the Ref, which is a moving target.

Furthermore, it's more reliable to explicitly state the SHA you want to perform actions on, otherwise you could get unexpected results as the pipeline you are currently working with locally might be pointing to a different version of the Ref than the one that Atlantis fetches.

tests

I would like guidance on how to appropriately testing this change as I'm fairly unfamiliar with the Atlantis codebase.

references

closes #5143

[jamengual]

@lukaspj could you update the api docs too? Thanks.

[lukaspj]

I certainly can, but I was unsure exactly how to handle the docs side of it, considering this is a breaking change as-is.

Would you prefer me to make it default to the previous behaviour when no SHA is provided or make it required like it is now?

[jamengual]

I certainly can, but I was unsure exactly how to handle the docs side of it, considering this is a breaking change as-is.

Would you prefer me to make it default to the previous behaviour when no SHA is provided or make it required like it is now?

That will be even better.

the docs you will need to change https://www.runatlantis.io/docs/api-endpoints.html which is here in the repo https://github.com/runatlantis/atlantis/blob/main/runatlantis.io/docs/api-endpoints.md

[lukaspj]

I updated it to be a heavy suggestion instead of a hard requirement, please let me know what you think.

[X-Guardian]

Hi @lukaspj, I think this PR exposes a can-of-worms regarding how this API is supposed to work.

Currently, a git ref needs to be specified with an optional PR number. In my testing, If I didn't specify a PR number, I get a failed to build command: 404 Not Found error, as the VCS client is trying to get the file changes on a non-existent PR 0.
If I specify a valid PR number and branch ref, then the API will correctly respond with a plan for the branch, but fail to update the job statuses of the PR, because it is using the branch ref for the sha in the context which is not correct.
There is also no validation that the branch specified is actually associated with the PR.
With your change we now have a PR, branch ref and commit SHA with no validation that they are related.

My feeling is that this API should just specify a PR number and an optional SHA, but no ref. The ref needs to be discovered from the PR and the commit sha needs to be verified that it is valid for the PR. If no SHA is specified, the latest commit SHA for the PR should be used.

This will be a breaking change but the API is very broken already!

[lukaspj]

Hi @lukaspj, I think this PR exposes a can-of-worms regarding how this API is supposed to work.

Currently, a git ref needs to be specified with an optional PR number. In my testing, If I didn't specify a PR number, I get a failed to build command: 404 Not Found error, as the VCS client is trying to get the file changes on a non-existent PR 0.

If I specify a valid PR number and branch ref, then the API will correctly respond with a plan for the branch, but fail to update the job statuses of the PR, because it is using the branch ref for the sha in the context which is not correct.

There is also no validation that the branch specified is actually associated with the PR.

With your change we now have a PR, branch ref and commit SHA with no validation that they are related.

My feeling is that this API should just specify a PR number and an optional SHA, but no ref. The ref needs to be discovered from the PR and the commit sha needs to be verified that it is valid for the PR. If no SHA is specified, the latest commit SHA for the PR should be used.

This will be a breaking change but the API is very broken already!

The API seems very broken indeed. We have tried to make heads and tails of it for a while.

Our dream was to be able to use Atlantis to run a reconciliation loop, so we can plan/apply at regular intervals to keep actual state in line with desired state.

However with the way it's written currently, that seems pretty hard to achieve. And what you are proposing here is essentially a programmatic way to run "atlantis apply". Which makes sense but we still dream of being able to run plan/apply on an arbitrary ref.

Nevertheless, with where the API is right now it makes sense to make it PR-centric as you suggested and make it clear that its purpose is to provide a programmatic interface to perform an equivalent of writing "atlantis" commands on a pr

[X-Guardian]

OK, so you want to run a plan through the API for a project based on a branch that is not linked to a PR?

[lukaspj]

Exactly, that's why we started looking at the API, so we could run apply on the main branch once a day.

We have hundreds of Terraform projects that rely on a few modules.
Re-applying each of these projects when one of our modules change is a hassle and often it means that our velocity drops and/or we have state drift.

So we want the reconciliation loop to ensure that new versions get applied to our environments.

[snehalsn]

@X-Guardian Hi, We are currently on atlantis version 0.32.0 and looking to use plan api to check drifts in terraform workspaces as well and stuck with the below error :
422 Validation Failed [{Resource:Status Field:sha Code:custom Message:sha must be a 40 character SHA1}].
We are passing a PR number that gets created programmatically and is compared against the master (ref).
Our plan is running atlantis plan api for a workspace and check if there is a drift. Passing the correct PR number also does not work here. Is there any work around to solve this issue?

[X-Guardian]

Hi @snehalsn, as you can read from the discussion here, the Atlantis plan and apply APIs are pretty broken at the moment. Looking back at the initial PR that introduced them back in 2022, #997, the implementation was really only a proof of concept. There are currently many scenarios where they don't work.