feat: add new flag --silence-no-projects

[GenPage]

This is to be used when you:

• utilize multiple Atlantis servers running (due to networking restrictions/firewalls)
• dynamically generate your atlantis.yaml using a pre_webflow_hook
• Utilize a monorepo for your terraform/terragrunt

The idea is that if the dynamically generated atlantis.yaml does not return any matching projects when the commands are built, then there is nothing to respond to as another Atlantis instance will handle it.

This should fix comment spam when multiple Atlantis instances are setup on the same monorepo.

Here is an example of what this PR attempts to fix:

[jamengual]

LGTM

[jamengual]

@GenPage test are failing, when you have a chance take a look at the tests.

[msarvar]

Can't we put this logic around pullUpdater.updatePull work better? I'm not sure how useful this would be to general atlantis users, is it common to run several instances of atlantis pointing to the same repo? I'm also hesitant adding more configuration flags. Also any new feature needs tests.

[GenPage]

Can't we put this logic around pullUpdater.updatePull work better?

We could, we'd need to surface projectCmds into either the CommandContext or pass through as a parameter to avoid commenting. I thought the current way would be more efficient as these is no reason to step through the entire command runner if there is nothing to run against.

I'm not sure how useful this would be to general atlantis users, is it common to run several instances of atlantis pointing to the same repo? I'm also hesitant adding more configuration flags.

I understand the hesitation here. I can't speak to commonality but I have come across multiple users of Atlantis attempting to run multiple instances. Our main use case is for Terragrunt, we keep all our configuration for multiple AWS accounts in the one repo for easier maintainability and DRY thanks to Terragrunt. Plus we have strict security guidelines about cross-account traffic. (One instance can't talk to all AWS accounts.)

Also any new feature needs tests.

Sorry about that, I have been getting more familiar with the codebase and working to get up to speed on how the mocks and test are setup. Already working on adding those ASAP.

[jamengual]

@msarvar The use of one repo and multiple atlantis servers is as common as using a single one. In my experience and base on the companies that I worked for I found very strange the fact that a security team will allow the use of an orchestration server that can cross boundaries so for me is the opposite and that is why there is so many people hacking around the fact that atlantis does not support this.

[GenPage]

@msarvar I'm working on getting up to speed on understanding the E2E and test fixtures. Do you have any suggestions on how to test this properly with some examples?

I'm thinking I might have to create a new testfixure with a atlanits.yaml without any projects or matching files for a defined project.

https://github.com/runatlantis/atlantis/blob/master/server/testfixtures/test-repos/simple-yaml/atlantis.yaml

[msarvar]

@GenPage Sorry for a slow response. Since you are passing in the flag into the runner you should be able to test them as unit tests in the events/command_runner_test.go. There should be examples on how to write the in that file. In terms of e2e I don't know if you need to add a new test for that, just make sure existing ones are passing.

Some implementation thoughts, I think the code needs to reset PR status checks similar to how SilenceVCSStatusNoPlans works for autoplan. But instead of only doing that logic for autoplan it will be doing it for all commands. Consider a scenario, where you have several changes spanning across several atlantis servers but you decide to revert one of changes. This will leave your status checks representing non existent project. You code should be doing something along the lines of SilenceVCSStatusNoPlans flag but for all commands not just autoplan. And updateCommitStatus shouldn't change.
I'm also not sure how does PR status checks work themselves. Does each Atlantis server have its own PR status check, or is there 1 PR status per command? How many Atlantis specific status checks do you get in the PR, when running plan/apply on 2 server simultaneously?

[GenPage]

@msarvar No worries on the slow response, I appreciate the guidance! I do see some similar example in events/command_runner_test.go that I think can help me, similar to how you mention the SilenceVCSStatusNoPlans.

I just caught updateCommitStatus as well, I will revert those changes as I'm already noticing that issue in our dev environment. I will replicate the status check reset for all commands as well. As for the status checks, each server does have its own check if you configure the VCSStatusName flag in the user config

https://www.runatlantis.io/docs/server-configuration.html#vcs-status-name

Once I get tests passing I'll work on site documentation.

[GenPage]

@msarvar On second thought, I think we can fix the implementation by not submitting the first pending status check until after prjCmdBuilder has run and we determine affected projects. This was my original intention with modifying updateCommitStatus but I didn't have as much understanding of the codebase then.

The reason for this is because the assumption is at least one of the Atlantis servers responds with a passing check, instead of N (number of servers) * 3 commands (plan, policy_check, apply). (In our specific case at Autodesk that would be 18 checks). Unless you can think of an issue where no projects triggering any servers would be an issue.

[msarvar]

@GenPage I think that will still produce stale status checks if your server doesn't have autoplan enabled.
For instance, a PR has 4 projects with changes, and you run plan command on the server A. Then a new commit reverts those changes, and and when you re-run the plan command server A will not update previously created status checks, unless we reset the status checks every time. This can be worse if your plans fail, now you have failed status check that is stale. This is similar to this PR that was created for autoplan #1323. You can probably limit status check updates to 1 update per command. If you're running atlantis plan it will only update its own status check and doesn't change apply status check.

And for consistency your atlantis servers should always create status checks even if they're not running any plans in your PR.

[codecov]

Codecov Report

Merging #1469 (50eacc0) into master (3bb6f21) will increase coverage by 0.12%.
The diff coverage is 82.02%.

@@            Coverage Diff             @@
##           master    #1469      +/-   ##
==========================================
+ Coverage   70.08%   70.21%   +0.12%     
==========================================
  Files          94       94              
  Lines        6485     6530      +45     
==========================================
+ Hits         4545     4585      +40     
- Misses       1554     1555       +1     
- Partials      386      390       +4     

Impacted Files	Coverage Δ
cmd/server.go	79.09% <ø> (ø)
server/user_config.go	100.00% <ø> (ø)
server/events/policy_check_command_runner.go	21.05% <54.54%> (-0.83%)	⬇️
server/events/approve_policies_command_runner.go	70.37% <75.00%> (+1.48%)	⬆️
server/events/apply_command_runner.go	72.22% <81.81%> (+0.61%)	⬆️
server/events/plan_command_runner.go	47.79% <86.95%> (+6.84%)	⬆️
server/events/delete_lock_command.go	75.86% <100.00%> (+0.86%)	⬆️
server/events/unlock_command_runner.go	88.23% <100.00%> (+2.52%)	⬆️
server/server.go	65.69% <100.00%> (+0.54%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bb6f21...50eacc0. Read the comment docs.

[msarvar]

Looks good overall, I have several change requests.

[GenPage]

Rebased against master, I addressed all of your comments @msarvar. Please let me know if I missed anything

[msarvar]

Looks good, before merging please update the comments in the a.silenceVCSStatusNoProjects checks to reflect their command, right now it is mentions all 3 possible commands.

[GenPage]

@msarvar I think its ready now, thanks!

[jasonrberk]

this is awesome and solves my exact issues. Will prevent me to unnecessary pre-workflow hooks that set an intentionally invalid repo name to avoid the "cross talk"

[GenPage]

@msarvar @nishkrishnan I fixed the merge conflicts, can we get this merged for the next stable release?

[RoryKiefer]

Currently planning to fork this branch and merge in main/current release just for this feature. Big +1 here.

[ipeacocks]

It's quite interesting to see how automatically genereted atlantis.yaml looks. Should it be located not in repo (as should be) but locally on server?

[GenPage]

@ipeacocks Yes, that is what we currently do. We use Terragrunt so we use https://github.com/transcend-io/terragrunt-atlantis-config with a pre-workflow-hook to generate the atlantis.yaml. This way we only generate the projects for the specific AWS accounts the server is located in.