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.

Sign up for GitHub

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

Jump to bottom

fix: Enforce explicitly setting SHA in API requests #5146

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Enforce explicitly setting SHA in API requests #5146

wants to merge 1 commit into from
+79 −61

Conversation

lukaspj
Copy link

@lukaspj lukaspj commented Dec 7, 2024

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

@lukaspj lukaspj requested review from a team as code owners December 7, 2024 22:47
@lukaspj lukaspj requested review from chenrui333, lukemassa and X-Guardian and removed request for a team December 7, 2024 22:47
@github-actions github-actions bot added the go Pull requests that update Go code label Dec 7, 2024
@lukaspj lukaspj force-pushed the feat/explicitly-set-sha-in-api branch from 6f80b59 to 940222c Compare December 7, 2024 22:49
@lukaspj lukaspj changed the title Enforce explicitly setting SHA in API requests fix: Enforce explicitly setting SHA in API requests Dec 7, 2024
@jamengual jamengual added the waiting-on-review Waiting for a review from a maintainer label Dec 31, 2024
@jamengual
Copy link
Contributor

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

@jamengual jamengual added waiting-on-response Waiting for a response from the user api-endpoints Adding API endpoints to Atlantis labels Dec 31, 2024
@lukaspj
Copy link
Author

lukaspj commented Jan 2, 2025

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
Copy link
Contributor

jamengual commented Jan 2, 2025 •
edited
Loading

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
Copy link
Author

lukaspj commented Jan 6, 2025

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

@github-actions github-actions bot added the docs Documentation label Jan 6, 2025
@lukaspj lukaspj force-pushed the feat/explicitly-set-sha-in-api branch from 7f6e09b to e677273 Compare January 8, 2025 08:24
@github-actions github-actions bot added build Relating to how we build Atlantis dependencies PRs that update a dependency file provider/github provider/gitlab github-actions website blog labels Jan 8, 2025
@lukaspj lukaspj force-pushed the feat/explicitly-set-sha-in-api branch from e677273 to 3342140 Compare January 8, 2025 08:26
@lukaspj lukaspj force-pushed the feat/explicitly-set-sha-in-api branch from 3342140 to b6a422a Compare January 8, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenrui333 chenrui333 Awaiting requested review from chenrui333 chenrui333 is a code owner automatically assigned from runatlantis/maintainers

@lukemassa lukemassa Awaiting requested review from lukemassa lukemassa is a code owner automatically assigned from runatlantis/core-contributors

@X-Guardian X-Guardian Awaiting requested review from X-Guardian X-Guardian is a code owner automatically assigned from runatlantis/core-contributors

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
api-endpoints Adding API endpoints to Atlantis blog build Relating to how we build Atlantis dependencies PRs that update a dependency file docs Documentation github-actions go Pull requests that update Go code provider/github provider/gitlab security waiting-on-response Waiting for a response from the user waiting-on-review Waiting for a review from a maintainer website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Atlantis API fails to execute for GitHub projects
3 participants
@lukaspj @jamengual @nitrocode