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

firewalld_ipset needs an adjunct native type to selectively purge unmanaged resources #236

Open
trevor-vaughan opened this issue Oct 22, 2019 · 2 comments

Comments

@trevor-vaughan
Copy link
Collaborator

Currently, the module has two options:

  1. Purge all unmanaged ipsets
  2. Never purge any ipsets

The issue with 1. is that services, such a kubernetes management systems, may lay down ipsets to persist at some point. These need to be persisted over time.

The issue with 2. is that, whenever you change your ipsets, you end up with entries that may no longer be valid that remain on your system and that may actually be invalid for firewalld (a mistaken rule).

I would suggest two changes. First, I would ensure that all items added to the system by the module be prefixed with something that makes it obvious that it is managed by puppet (something like pup_mgmt would work fine.

Second, I would allow users to purge but with two modes.

  • Mode 1: Purge any old puppet managed items that happen to remain (default)
  • Mode 2: Purge any unrecognized resources (good for consistency, bad for interoperability)
@igalic
Copy link
Contributor

igalic commented Oct 23, 2019

what do you propose as transition from one version that doesn't support such prefixes, to one that does?

@trevor-vaughan
Copy link
Collaborator Author

@igalic The current module has a "purge everything" mode that would work via firewalld::purge_unknown_ipsets. There might be another way to track what puppet is managing, but I haven't found it yet. The firewall-cmd options don't seem to have any way to add comments that I can see.

This may need to be a breaking change bump but I'm honestly not sure how people were managing anything complex with this without it since literally renaming an ipset, or removing it from management, would leave it on the system to be persisted by firewalld forever.

Basically, the current behavior is unpredictable from a traditional Puppet point of view and this should be fixed but, unfortunately, we have to allow for everything else in the world also mucking about with it by design.

This may be a larger issue since this actually persists into all of the different spaces, icmptypes, services, zones, etc. I'm not sure if it would be better to have a single purge resource that covers everything, pop it into individual resources, or extend the existing resources with a purge parameter.

Either way, we need some pattern matching capabilities so that we can keep other daemons in place. This may even need to be an Array of globs or regexes for safety.

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

No branches or pull requests

2 participants