Ability to name custom service

[BrandonIngalls]

While testing the creation of custom services via the ::firewalld module I came across some behavior that I did not expect -- it is documented and it works exactly as the documentation says it does, but it does not map to what I expected of a custom firewalld service definition.

Normal firewalld zone .xml file

[~]$ rpm -qil firewalld-0.4.3.2-1.fc23.noarch 
Name        : firewalld
Version     : 0.4.3.2
Release     : 1.fc23
Architecture: noarch
Install Date: Sat 13 Aug 2016 02:22:35 PM EDT
Group       : Unspecified
Size        : 1869790
License     : GPLv2+
Signature   : RSA/SHA256, Tue 05 Jul 2016 05:08:48 PM EDT, Key ID 32474cf834ec9cba
Source RPM  : firewalld-0.4.3.2-1.fc23.src.rpm
Build Date  : Tue 05 Jul 2016 01:57:36 PM EDT
Build Host  : buildvm-20.phx2.fedoraproject.org
Relocations : (not relocatable)
Packager    : Fedora Project
Vendor      : Fedora Project
URL         : http://www.firewalld.org
Summary     : A firewall daemon with D-Bus interface providing a dynamic firewall
Description :
firewalld is a firewall service daemon that provides a dynamic customizable
firewall with a D-Bus interface.
...
/usr/lib/firewalld/services/mosh.xml
...

[~]$ cat /usr/lib/firewalld/services/mosh.xml
<?xml version="1.0" encoding="utf-8"?>
<service>
  <short>Mobile shell that supports roaming and intelligent local echo.</short>
  <description>Mosh is a remote terminal application that supports intermittent network connectivity, roaming to different IP address without dropping the connection, intelligent local echo and line editing to reduct the effects of "network lag" on high-latency connections.</description>
  <port protocol="udp" port="60000-61000"/>
</service>

Firewalld looks in places like /usr/lib/firewalld/services/ and /etc/firewalld/services/ for service definitions, when one issues firewall-cmd [--permanent] --add-service=mosh firewalld looks for mosh.xml in related folders.

CentOS / RHEL do not have a built-in definition for mosh, so I attempted to mimic the Fedora provided xml file via firewalld::custom_service.

include ::firewalld

firewalld::custom_service { 'Create mosh firewalld service definition':
  short       => 'Mobile shell that supports roaming and intelligent local echo.',
  description => 'Mosh is a remote terminal application that supports intermittent network connectivity, roaming to different IP address without dropping the connection, intelligent local echo and line editing to reduct the effects of "network lag" on high-latency connections.',
  port => [
    {port => '60000-61000', protocol => 'udp'}
  ],
}

As this module creates a ${short}.xml service file I ended up with a rather large name.

[root@puppet-firewalld-test firewalld-test]# puppet apply test2.pp                                                                                                                                                 
Notice: Compiled catalog for puppet-firewalld-test.local in environment production in 1.09 seconds
Notice: /Stage[main]/Main/Firewalld::Custom_service[Create mosh firewalld service defination]/File[/etc/firewalld/services/Mobile shell that supports roaming and intelligent local echo..xml]/ensure: created
Notice: /Stage[main]/Main/Firewalld::Custom_service[Create mosh firewalld service defination]/Exec[firewalld::custom_service::reload-Create mosh firewalld service definition]: Triggered 'refresh' from 1 events
Notice: Finished catalog run in 2.21 seconds

[root@puppet-firewalld-test firewalld-test]# ls /etc/firewalld/services/
Mobile shell that supports roaming and intelligent local echo..xml

# Copy the file to example1.xml to show that I am able to --add-service example1
[root@puppet-firewalld-test firewalld-test]# cp /etc/firewalld/services/Mobile\ shell\ that\ supports\ roaming\ and\ intelligent\ local\ echo..xml /etc/firewalld/services/example1.xml

[root@puppet-firewalld-test firewalld-test]# firewall-cmd --reload
success

[root@puppet-firewalld-test firewalld-test]# firewall-cmd --add-service example1
success

[root@puppet-firewalld-test firewalld-test]# firewall-cmd --list-services
example1

If possible I would like to see the ability to specify the name of a custom service, the short tag is for a small summary of what a service is/does, it should not be the basis for how we reference the custom services we create.

[crayfishx]

@BrandonIngalls yes, I see your issue... I would expect the following to work:

firewalld::custom_service { 'mosh':
  short       => 'Mobile shell that supports roaming and intelligent local echo.',
  description => 'Mosh is a remote terminal application that supports intermittent network connectivity, roaming to different IP address without dropping the connection, intelligent local echo and line editing to reduct the effects of "network lag" on high-latency connections.',
  port => [
    {port => '60000-61000', protocol => 'udp'}
  ],
}

or

firewalld::custom_service { 'Create mosh firewalld service definition':
  name      => 'mosh',
  short       => 'Mobile shell that supports roaming and intelligent local echo.',
  description => 'Mosh is a remote terminal application that supports intermittent network connectivity, roaming to different IP address without dropping the connection, intelligent local echo and line editing to reduct the effects of "network lag" on high-latency connections.',
  port => [
    {port => '60000-61000', protocol => 'udp'}
  ],
}

This works if we create the filename from $name rather than from $short

https://github.com/crayfishx/puppet-firewalld/blob/master/manifests/custom_service.pp#L58

I've put a PR in for it - I'm not sure how we should treat this one, it's technically a breaking change but I kind of feel this is a bug. PR #76 would break any implementation taken straight from the docs, so Im not sure if we should release that in 3

[crayfishx]

We could merge #77 in 3.x, that is non breaking and allows you to set a filename - this won't break existing implementations, we can then deprecate this and replace it for the $name pattern (as in #76 ) for 4.0.0 - does that sound reasonable?

[BrandonIngalls]

Long term I think the best way to handle a custom service definition would be to name the file ${name/title}.xml.

firewalld::custom_service { 'mosh':
  short       => 'Mobile shell that supports roaming and intelligent local echo.',
  description => 'Mosh is a remote terminal application that supports intermittent network connectivity, roaming to different IP address without dropping the connection, intelligent local echo and line editing to reduct the effects of "network lag" on high-latency connections.',
  port => [
    {port => '60000-61000', protocol => 'udp'}
  ],
}

I think that would help cut down on accidental duplicate services in the future.

But that change would have to be in 4.0.0, so I would think about adding some sort of deprecation warning if filename is not specified for 3.x.x.

[crayfishx]

Released in 3.1.0 - I'll raise a new ticket for the 4.0.0 spec for this