Service filename bugfix

This started out as a fix for the custom service filename but spiraled a
bit further during debugging.

* Auto-correct the filename for firewalld::custom_service
* Add a function `firewalld::safe_filename` for munging filenames to
  safely work with firewalld. The allowed characters were determined by
  experimentation and are not documented.
* Ensure that firewalld_zone resources automatically reload the firewall
* Ensure that firewalld_zone names are no longer than 17 characters per
  the manual
* Create firewalld::reload and firewalld::reload::complete classes to
  allow easier resource chaining
* Fix spacing in init.pp
* Ensure that all of the Execs that call firewall-cmd happen after the
  service is running
* Ensure that all permanent configuration changes happen before the
  firewalld service is started/restarted. This is critical if switching
  from nft to iptables due to the segfault bug in nft
* Ensure that all custom service declarations happen before all
  firewalld_zone resources are triggered. This is automatic in all
  native types
* Updated the service.xml.erb to an EPP file

Closes #265

functions/safe_filename.pp

@@ -0,0 +1,70 @@
+# @summary Returns a string that is safe for firewalld filenames
+#
+## @example Regular Filename
+#   $filename = 'B@d Characters!'
+#   firewalld::safe_filename($orig_string)
+#
+#   Result => 'B_d_Characters_'
+#
+# @example Filename with Options
+#   $filename = 'B@d Characters!.txt'
+#   firewalld::safe_filename(
+#     $filename,
+#     {
+#       'replacement_string' => '@@',
+#       'file_extension'     => '.txt'
+#     }
+#   )
+#
+#   Result => 'B@@d@@Characters@@.txt'
+#
+# @param filename
+#   The String to process
+#
+# @param options
+#   Various processing options
+#
+# @param options [String[1]] replacement_string
+#   The String to use when replacing invalid characters
+#
+# @option options [String[1]] file_extension
+#   This will be stripped from the end of the string prior to processing and
+#   re-added afterwards
+#
+# @return [String]
+#   Processed string
+#
+function firewalld::safe_filename(
+  String[1] $filename,
+  Struct[
+    {
+      'replacement_string' => String[1],
+      'file_extension'     => Optional[String[1]]
+    }
+  ]         $options  = { 'replacement_string' => '_'}
+) {
+
+  # If we have an extension defined
+  if $options['file_extension'] {
+
+    # See if the string ends with the extension
+    $_extension_length = length($options['file_extension'])
+    if $filename[-($_extension_length), -1] == $options['file_extension'] {
+
+      # And extract the base filename
+      $_basename = $filename[0, -($_extension_length) - 1]
+    }
+  }
+
+  # If we extraced a base filename substitute on that and re-add the file extension
+  if defined('$_basename') {
+    sprintf('%s%s',
+      regsubst($_basename, '[^\w-]', $options['replacement_string'], 'G'),
+      $options['file_extension']
+    )
+  }
+  # Otherwise, just substitute on the original filename
+  else {
+    regsubst($filename, '[^\w-]', $options['replacement_string'], 'G')
+  }
+}

lib/puppet/provider/firewalld_zone/firewall_cmd.rb

@@ -197,4 +197,8 @@ def short
   def short=(new_short)
     execute_firewall_cmd(['--set-short', new_short], @resource[:name], true, false)
   end
+
+  def flush
+    reload_firewall
+  end
 end

lib/puppet/type/firewalld_service.rb

@@ -1,19 +1,20 @@
 require 'puppet'
 
 Puppet::Type.newtype(:firewalld_service) do
-  @doc = "Assigns a service to a specific firewalld zone.
-    firewalld_service will autorequire the firewalld_zone specified in the zone parameter and the firewalld::custom_service
-		specified in the service parameter, so there is no need to add dependencies for this
+  @doc = <<-EOM
+      Assigns a service to a specific firewalld zone.
+      firewalld_service will autorequire the firewalld_zone specified in the
+      zone parameter and the firewalld::custom_service specified in the service
+      parameter, so there is no need to add dependencies for this
 
-    Example:
+      Example:
 
         firewalld_service {'Allow SSH in the public Zone':
-            ensure  => 'present',
-            zone    => 'public',
-            service => 'ssh',
+          ensure  => 'present',
+          zone    => 'public',
+          service => 'ssh',
         }
-
-  "
+  EOM
 
   ensurable do
     newvalue(:present) do

lib/puppet/type/firewalld_zone.rb

@@ -49,6 +49,7 @@ def generate
 
   newparam(:name) do
     desc 'Name of the rule resource in Puppet'
+    isnamevar
   end
 
   newparam(:zone) do
@@ -162,6 +163,14 @@ def retrieve
     end
   end
 
+  validate do
+    [:zone, :name].each do |attr|
+      if self[attr] && (self[attr]).to_s.length > 17
+        raise(Puppet::Error, "Zone identifier '#{attr}' must be less than 18 characters long")
+      end
+    end
+  end
+
   autorequire(:service) do
     ['firewalld']
   end

manifests/custom_service.pp

@@ -45,18 +45,28 @@
   Enum['present','absent'] $ensure      = 'present',
 ) {
 
-  file{"${config_dir}/${filename}.xml":
+  include firewalld::reload
+
+  # Service files may only contain alphanumeric characters and underscores.
+  # This is not documented, but has been experimentally confirmed.
+  $_safe_filename = firewalld::safe_filename($filename)
+
+  $_content = epp(
+    "${module_name}/service.xml.epp",
+    'short'       => $short,
+    'description' => $description,
+    'port'        => $port,
+    'module'      => $module,
+    'destination' => $destination,
+    'filename'    => $filename,
+    'config_dir'  => $config_dir,
+    'ensure'      => $ensure
+  )
+
+  file{ "${config_dir}/${_safe_filename}.xml":
     ensure  => $ensure,
-    content => template('firewalld/service.xml.erb'),
+    content => $_content,
     mode    => '0644',
-    notify  => Exec["firewalld::custom_service::reload-${name}"],
+    notify  => Class['firewalld::reload'],
   }
-
-  exec{ "firewalld::custom_service::reload-${name}":
-    path        =>'/usr/bin:/bin',
-    command     => 'firewall-cmd --reload',
-    onlyif      => 'firewall-cmd --state',
-    refreshonly => true,
-  }
-
 }