-
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
fix: ensure cloning workingdir before calling plan api #3584
base: main
Are you sure you want to change the base?
Conversation
result, err := a.apiPlan(request, ctx) | ||
if err != nil { | ||
a.apiReportError(w, http.StatusInternalServerError, err) | ||
return | ||
} | ||
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, 0) // nolint: errcheck | ||
defer a.Locker.UnlockByPull(ctx.HeadRepo.FullName, ctx.Pull.Num) // nolint: errcheck |
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.
#2465 introduced a lock for each PR, but the release of the lock appears to have been omitted.
Ready to be reviewed. |
Although the failing check seems irrelevant to my change, should I take some actions? |
@terakoya76 any updates here? |
I will be reviewing this week |
I did review this, but I have some reservations about introducing another "Clone" while we are still trying to figure out clones and project locks in #3345. We will internally discuss further within the maintainer team. |
FWIW, testing this internally along with #3985 plus atlantis-drift-detection seems to working for us. We'll put some load on it and see if it's reliable. |
We've been using for more than a few months now, still looking good. |
@runatlantis/core-contributors please review |
@terakoya76, can you resolve the conflicts on this? |
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
@terakoya76, could you sign the commits to give this another review? |
Thanks for all the coordination. |
fd2dd47
to
774cac6
Compare
Signed-off-by: Hajime Terasawa <[email protected]>
Signed-off-by: Hajime Terasawa <[email protected]>
Signed-off-by: Hajime Terasawa <[email protected]>
774cac6
to
417e1ac
Compare
I failed to leave recent merge commits in place and only sign off messages on past commits. |
@terakoya76 could you check the status checks? some of them are failing |
Signed-off-by: Hajime Terasawa <[email protected]>
what
Fix #2949 by cloning the working dir before processing plan/apply api call.
why
As reported in #2949, when calling plan/apply from api, if the target workingdir has not been cloned by atlantis, an error occurs and plan/apply cannot be performed.
Specifically, if an error is returned when executing the buildProjectPlanCommand method GetWorkingDir, the event reported in Issue.
atlantis/server/events/project_command_builder.go
Line 459 in b6466bd
This makes it particularly difficult to execute plan/api from a lifecycle outside of a pull request. This patch allows developers to use the api to build drift detection mechanisms, etc.
tests
Added unit tests.
And locally tested with the payload like this.
references