-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add the /plan and /apply endpoints #997
Conversation
This is awesome. I really hope it can get picked up into master! |
Hey sorry I haven't been able to get to these. They're the top of my review list now for the next release. |
Are these programmatic endpoints coming soon? |
Hi All, |
@remilapeyre can you fix the conclicts? |
Hi, I will try to come back to this some time this week. Also, I did not thought too long about the payload for the API you are welcome to share your thoughts about it. |
Is there a plan to get this in? |
Any updates? |
We have been busy with other stuff so it is hard to say if this will get in before is reviewed. I do not have an ETA for you guys. |
Hi, due to lack of time to work on this, I won't have time to rebase the work or make all the corrections too get it merged, but feel free to reuse any part that may be useful if you want to open another PR |
Is this still something planned? |
@nishkrishnan any plans to take this forward. I can help to rebase and address any code review comments if needed. |
If this gets merged, Atlantis will be on its way to implement "Drift Detection" which is a notable feature in Spacelift and Terraform Enterprise. |
@nitrocode Someone will have to pick this up and finish it, it was created long ago and I will have to be updated it to the current master. |
server/events/vcs/github_client.go
Outdated
@@ -501,3 +501,7 @@ func (g *GithubClient) DownloadRepoConfigFile(pull models.PullRequest) (bool, [] | |||
func (g *GithubClient) SupportsSingleFileDownload(repo models.Repo) bool { | |||
return true | |||
} | |||
|
|||
func (g *GithubClient) GetCloneURL(VCSHostType models.VCSHostType, repo string) (string, error) { | |||
return "", fmt.Errorf("not yet implemented") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed for plan/apply endpoints to work for github? If so can we also prioritize this before merging? Most users of Atlantis are probably on GitHub
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! Implemented.
A full drift detection workflow also needs to list all the valid callers of /plan. Otherwise we could manually parts the atlantis.yaml file. Is a /list also in scope? |
} | ||
if result.HasErrors() { | ||
code = http.StatusInternalServerError | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the apiApply function call and the next 2 if statements are the only differences when compared to the Plan function. Can we combine these 2 functions together and use an if statement to check if it should do an apply or not? That way it would DRY up the code a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand there is plenty of room for improvement, but I'm less sure about the specific change you are proposing. Could you suggest the change you described to avoid confusion? Much appreciated!
The output of
No I think the main focus for this PR is to have a POC first, so that people can test with it and provide more feedback. However, it's highly encouraged to open an issue and elaborate more on features like the |
@runatlantis/maintainers May I have another look at this PR? I've done most of the changes I felt necessary, but please let me know if there's anything missing or can be improved! |
|
Good job and thanks to everyone involved. This was much needed 😍 |
please test this a lot on the prerelease https://github.com/runatlantis/atlantis/releases/tag/v0.19.8-pre.20220722 We need a lot of feedback. |
Thank you @remilapeyre and @lilincmu and everyone involved! This is fantastic. |
YES!@!!!!! I am excited. :) |
Thank you so much for this! |
I think documentation would be required for this, especially about the request/response bodies. BTW this is awesome, thanks! |
Please note a caveat regarding WebAuthentication middleware: #2455 (for ppl getting |
If you have policy checks enabled, the /plan endpoint probably won't work, as described in #2456. |
Relevant repo |
* Add the /plan and /apply endpoints * Resolve conflicts * Fix wrong merge * Add missing methods for mocks * Fix linting error * Fix linting error * Move api plan/apply into APIController * Extract commond code into helper functions * Implement GetCloneURL for GitHub Co-authored-by: Li Lin <li.lin@hashicorp.com>
See #937