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

Enhancement: --set-default-zone #98

Closed
hpcprofessional opened this issue Nov 8, 2016 · 9 comments
Closed

Enhancement: --set-default-zone #98

hpcprofessional opened this issue Nov 8, 2016 · 9 comments
Labels
enhancement New feature or request
Milestone

Comments

@hpcprofessional
Copy link

It would be extra convenient if one could configure the default zone easily via this module.

Right now I've added this to wrapper code:

exec { "firewalld: set default zone to "${zone}"":
command => "firewall-cmd --set-default-zone ${zone}",
path => '/bin',
logoutput => false,
unless => "firewall-cmd --get-default-zone | grep ${zone}",
#Enforces ordering, but also checks/sets after every reload...
subscribe => Exec['firewalld::reload'],
}

@crayfishx
Copy link
Contributor

Hi,

What benefit does this bring? We agreed in #42 that we wouldn't use default zones for the module configuration and the user should be clear about what zone they are configuring to avoid confusion. Are you proposing this to make module configuration easier (which was already discussed in #42) or are there other benefits outside of Puppet to setting it?

@hpcprofessional
Copy link
Author

I guess there are a couple of things going on:

  1. The site I'm working with upgraded to this module from https://forge.puppet.com/jpopelka/firewalld. Consequently, they expected the feature to exist since it existed there.
  2. Perhaps within the other module's paradigm, the site structured their firewall rules to put everything in a custom zone and then set the default zone to custom. Wearing a belt with suspenders? Perhaps, but part of external validation of the fw configuration is verification that the default zone is set.
  3. Execs are sub-optimal and if at all possible should be migrated to types and providers. :-)

@crayfishx
Copy link
Contributor

Hmm, I can't see quite what they are using it for in the jpopelka module... I guess the question is what benefit do you achieve by running that exec? What do you miss if you don't run it? I'm happy to add it if there is a use case, I just don't understand what it is yet :-)

Just trying to identify why you need to do it at all when you're using puppet to manage it....

@jbnance
Copy link

jbnance commented Dec 2, 2016

I would like to be able to set the default zone for two reasons:

  • So that I don't have to enumerate interfaces and assign them explicitly to a zone
  • For those times that interfaces are added to the system prior to an explicit firewall policy being associated with the interface

In general we prefer not to modify the zones/services provided by upstream.

Specific to the second point, imagine this scenario. A new interface is added that isn't already accounted for in a known list (because separate teams manage the [virtual] hardware and the OS configuration). We want the default to be drop everything except ICMP. The current firewalld default is "public". So for us this would mean either setting the default zone to a modified "drop" or editing the "public" zone.

@divad
Copy link

divad commented Dec 20, 2016

I agree, being able to set the default zone seems to me (and forgive my ignorance if I am wrong!) a standard feature you'd expect since its something that firewalld itself supports. I've been hunting for ages until I found this bug on how to set it!

@crayfishx crayfishx added accepted enhancement New feature or request and removed needs decision labels Dec 20, 2016
@crayfishx
Copy link
Contributor

Ok, there seems to be enough interest in this feature that I've marked it as accepted.

Design wise, I'm guessing since this is a fairly global option, and only one zone can be the default zone, it would make sense as an attribute to the firewalld base class?.... something like

class { 'firewalld':
  ...
  default_zone => 'public',
}

?

@jbnance
Copy link

jbnance commented Dec 20, 2016

I agree that a class parameter is well suited for this.

@ghenzler
Copy link

ghenzler commented Jan 6, 2017

+1 for this feature, the proposed syntax looks good to me.

@crayfishx
Copy link
Contributor

FYI all I've been on vacation but should have time to implement this feature this week

@crayfishx crayfishx added this to the 3.2.0 milestone Feb 27, 2017
crayfishx added a commit that referenced this issue Feb 28, 2017
crayfishx added a commit that referenced this issue Feb 28, 2017
Port ranges (#107) and default_zone option (#98)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants