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.

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

feat: Allow for negations in repo allowlist #3414

Merged
merged 4 commits into from
Jul 31, 2023

Conversation

lukemassa
Copy link
Contributor

@lukemassa lukemassa commented May 18, 2023

what

Update the flag an optional flag --repo-allowlist to allow for rules that begin with ! to omit repos.

Existing allow lists (as long as they don't have rules that start with !) would be unaffected

why

In general, useful to be able to exclude repos included by a glob. As a specific example, in our setup, we have repos foo/* that we want to be tracked by atlantis, but sometimes we want to omit a repo to do some testing, so this will allow us to do --repos-allowlist='foo/*,!foo/bar'.

tests

I searched the whole repo for "allow.?list" and updated each place it occurred.

I added several unit tests to the previous code that matched the allow list.

references

Closes #1180

@lukemassa lukemassa requested a review from a team as a code owner May 18, 2023 19:17
@github-actions github-actions bot added docs Documentation go Pull requests that update Go code labels May 18, 2023
@lukemassa lukemassa marked this pull request as draft May 18, 2023 21:19
@lukemassa lukemassa marked this pull request as ready for review May 19, 2023 14:04
@lukemassa lukemassa mentioned this pull request May 19, 2023
@lukemassa lukemassa changed the title Implement repos denylist feat: Repos denylist May 19, 2023
jamengual
jamengual previously approved these changes May 27, 2023
@jamengual
Copy link
Contributor

Thanks for the contribution @lukemassa LGTM @GenPage @nitrocode any thoughts?

@lukemassa
Copy link
Contributor Author

@GenPage @nitrocode any thoughts here?

@nitrocode
Copy link
Member

nitrocode commented Jun 14, 2023

The naming of this flag is unclear to me without reading the description.

It's not so much a --deny-list but an --exclude-from-allow-list.

Allow us to discuss this internally.

@jamengual
Copy link
Contributor

The naming of this flag is unclear to me without reading the description.

It's not so much a --deny-list but an --exclude-from-allow-list.

Allow us to discuss this internally.

I agree with @nitrocode the name could be confusing with some people.

@lukemassa
Copy link
Contributor Author

The naming of this flag is unclear to me without reading the description.

It's not so much a --deny-list but an --exclude-from-allow-list.

Allow us to discuss this internally.

Yeah that makes sense. Either works for me, happy to change if that's the preference.

@jamengual
Copy link
Contributor

The naming of this flag is unclear to me without reading the description.
It's not so much a --deny-list but an --exclude-from-allow-list.
Allow us to discuss this internally.

Yeah that makes sense. Either works for me, happy to change if that's the preference.

let's change it to --exclude-from-allow-list then and thanks.

@lukemassa
Copy link
Contributor Author

For consistency it should be probably be --exclude-from-repo-allowlist (since there are other allow lists like --var-file-allowlist), right?

Although another option might be, instead of having --exclude-from-repo-allowlist and --repo-allowlist, given that as you say it's not really a "denylist", to use !s like

--repo-allowlist='github.com/foo/*,!github.com/foo/bar`

@GenPage
Copy link
Member

GenPage commented Jun 20, 2023

@lukemassa if we can avoid adding another flag and contain the new feature with the existing allowlist, that is more ideal.

@lukemassa
Copy link
Contributor Author

lukemassa commented Jun 21, 2023

@lukemassa if we can avoid adding another flag and contain the new feature with the existing allowlist, that is more ideal.

Cool I'll give this a shot!

@lukemassa lukemassa changed the title feat: Repos denylist feat: Allow for negations in repo allowlist Jun 23, 2023
@lukemassa
Copy link
Contributor Author

@GenPage @nitrocode @jamengual I've updated the code, comment, and PR summary to contain the changes in the existing flag

Copy link
Contributor

@jamengual jamengual left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lukemassa LGTM

@GenPage GenPage enabled auto-merge (squash) July 31, 2023 13:21
@GenPage GenPage merged commit cbb86c2 into runatlantis:main Jul 31, 2023
14 checks passed
@kevin-fitlg
Copy link
Contributor

kevin-fitlg commented Dec 10, 2023

Hey @lukemassa
I cannot make this work in my environment. It would be good if you can advise.
My current auto-plan-file-list is like this: deployments/**/*.tf,deployments/**/*.yaml,deployments/**/*.yml,deployments/**/*.json,deployments/**/*.config,**/*.tf

I have atlantis deployment files somewhere under deployments folder. Lets assume its like deployments/azure/prd/useast2/atlantis and I want to exclude this path from being handled by atlantis engine.

So, I have tried different combinations like !*atlantis* , !**/atlantis , !**/*atlnatis* and even the full path, i.e. !deployments/azure/prd/useast2/atlantis but it does not work. Atlantis still plans changes in this path.

I basically add the negation part at the beginning of my existing allow-list, like this: !**/atlantis,deployments/**/*.tf,deployments/**/*.yaml,deployments/**/*.yml,deployments/**/*.json,deployments/**/*.config,**/*.tf

However, it does not work.

I read your code changes and it should work but your tests does not seems to be including any wildcard negations. Although, when I add the full path, it does not work either!

I am using atlantis 0.26.0 which includes your features.

It would be great if you could help with this.

Thanks :)

@lukemassa
Copy link
Contributor Author

@kevin-fitlg note that my change didn't affect the --auto-plan-file-list flag/setting, only the --repo-allowlist flag/setting. --repo-allowlist always starts with a hostname (https://www.runatlantis.io/docs/server-configuration.html#repo-allowlist), so that doesn't look like the format you're discussing here.

According to the documentation for https://www.runatlantis.io/docs/server-configuration.html#autoplan-file-list it uses the docker ignorefile syntax (https://docs.docker.com/build/building/context/#matching). I hope this helps!

ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* feat: Omit repos from allowlist

* Add quote in comment

* Better comment

* Remove test
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
* feat: Omit repos from allowlist

* Add quote in comment

* Better comment

* Remove test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add repo deny list
5 participants