atlantis
atlantis copied to clipboard
runatlantis
Add --strict-plan-file-list config
Right now it's possible to plan directories outside the scope of the PR changed list using -d. This blocks that behavior if the server is configured with the --strict-plan-file-list.
@jamengual I've reproduced the behavior in this pull request. The terragrunt code itself it's not working but it's possible to see that another directory unrelated to the PR can be planned.
The expected is to:
- Open a PR
- Plan
- Apply the planned resources
But currently is possible to:
- Open a PR
- Run
atlantis plan -d another-dir-not-related-to-pr - Then run
atlantis apply -d another-dir-not-related-to-pr
If the plan with -d fails the pipeline status will stay failed until a new commit is added to solve it.
is this a monorepo case?
On Thu., Aug. 11, 2022, 6:31 p.m. Fabiano Soares Honorato, < @.***> wrote:
@jamengual https://github.com/jamengual I've reproduced the behavior in this pull request https://github.com/Fabianoshz/atlantis-test/pull/3. The terragrunt code itself it's not working, but it's possible to see that another directory unrelated to the PR can be planned.
The expected is to:
- Open a PR
- Plan
- Apply the planned resources
But currently is possible to:
- Open a PR
- Run atlantis plan -d another-dir-not-related-to-pr
- Then run atlantis apply -d another-dir-not-related-to-pr
If the plan with -d fails the pipeline status will stay failed until a new commit is added to solve it. [image: image] https://user-images.githubusercontent.com/14815404/184268549-0a51a7a3-0fe2-4654-84c2-dd139a8aa810.png
— Reply to this email directly, view it on GitHub https://github.com/runatlantis/atlantis/pull/2440#issuecomment-1212646291, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERGQLYB7ZGQPI7GAWXDVYWSQRANCNFSM56J2WIEQ . You are receiving this because you were mentioned.Message ID: @.***>
In this case yes, to explain further, different teams working on the same repository might try to plan and apply resources that don't belong to them.
The problem we see here is that nothing stops a user to add an empty line to a dir outside the scope of the PR and I do not think this change will make a difference.
maybe https://www.runatlantis.io/docs/policy-checking.html#how-it-works could help here but not sure.
Atlantis was not designed for monorepo in mind but many people use it but it need ingenuity in some cases
Yes, it's possible to open a PR with an empty line but if that's the case we can prevent that user from applying by enforcing a code owner of that directory to approve the PR.
In the case described it's possible to bypass the code owners by simply opening a PR which the user should be able to change and then planing and applying a directory outside of the user normal boundaries after the code owners approval.
By allowing this breach the user may also create an unrecoverable failed step if the plan/apply fails which will require a new commit adding the directory that the user should have never changed in the first place.
@Fabianoshz thank you for adding this PR. I added a few comments. Please review and resolve conflicts.
@Fabianoshz fixed tests by running gofmt -w. Please address the missing project check comment.
Sorry for my absence, I've had some personal issues. Came back today and managed to add the logic to projects too but I'm still not satisfied with the code yet.
@nitrocode @jamengual how should I approach writing a test for this? Looking at the code here the values are defined for all the cases but since this is a server side flag I believe it should have 1 or 2 cases instead of changing for everyone. Maybe because this is false by default we don't test? I don't know the policy for this.
@nitrocode @jamengual how should I approach writing a test for this? Looking at the code here the values are defined for all the cases but since this is a server side flag I believe it should have 1 or 2 cases instead of changing for everyone. Maybe because this is false by default we don't test? I don't know the policy for this.
yes, do no need to change for everyone, just add 1 or 2 more functions.
Tests added, I believe this should be good to go, sorry this took more time than I expected.
Thank you @Fabianoshz for the tests. It's looking very good. Just had another comment and want to get some more eyes on this.
Thank you again @Fabianoshz . I added 2 last comments regarding the documentation. Please address and we can get this merged.
Thanks @Fabianoshz for all your efforts here! We really appreciate your work. Let us know if you want to contribute more. By the way, we're also looking for more maintainers if you're interested, please message us on slack.
Hey @nitrocode, I sure want to contribute more, I'll reach out on slack!