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

Add enhancement for purging unmanaged zones #249

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yachub
Copy link

@yachub yachub commented Jan 13, 2020

Pull Request (PR) description

This pull request adds the ability to purge unmanaged firewalld zones.

This Pull Request (PR) fixes the following issues:

Fixes #134

@ghoneycutt
Copy link
Member

The tests are failing due to ruby style issues. You can likely test locally with rake rubocop. Would be nice to see tests added to the provider.

@yachub
Copy link
Author

yachub commented Jan 13, 2020

Hi @ghoneycutt Thanks for the quick response. Understood, I just fixed the styling issues and will push in a moment. I will look at adding tests to the provider as well.

@trevor-vaughan
Copy link
Collaborator

It is quite possible that this feature will cause flapping when used in conjunction with other things that manage firewall state. I would recommend adding the ability to match based on a regex as well as a blanket purge. For instance, I set my zones as <number>_simp_whatever so that I can easily identify which are mine and use the (undocumented) internal ordering capabilities of firewalld. Given this, I do NOT want to purge items laid down by Docker (or whatever), I just want to purge items that match ^\d+_simp.

@vox-pupuli-tasks
Copy link

Dear @thespain, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

1 similar comment
@vox-pupuli-tasks
Copy link

Dear @thespain, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@sigbjornaib
Copy link

Any update on this pull request? We are very much looking forward to this PR being merged.

@trevor-vaughan
Copy link
Collaborator

I still think that this needs some sort of safety valve.

The default is fine but I think there should be some sort of either match, or purge regex (probably both) that changes the behaviour to make it safe for vendor applications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

firewalld_zone should be purgeable (safely)
4 participants