diff --git a/.rspec b/.rspec
index 8c18f1ab..4cd92bfe 100644
--- a/.rspec
+++ b/.rspec
@@ -1,2 +1,3 @@
 --format documentation
 --color
+--fail-fast
diff --git a/README.md b/README.md
index 3cc63d67..2ca19926 100644
--- a/README.md
+++ b/README.md
@@ -216,6 +216,17 @@ The following paramters are the element of the rich rule, only _one_ may be used
 
 The `firewalld::custom_service` defined type creates and manages custom services. It makes the service usable by firewalld, but does not add it to any zones. To do that, use the firewalld::service type.
 
+---
+
+> The `firewalld::custom_service` is **DEPRECATED** and will be removed in a
+> future release. Please use the `firewalld_custom_service` native type.
+>
+> Please note that there are slight differences in the parameters that will
+> require modifications to the `firewalld::custom_services` Hash if utilized from
+> Hiera.
+
+---
+
 _Example in Class_:
 
 ```puppet
diff --git a/REFERENCE.md b/REFERENCE.md
index 85028f86..dbad77eb 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -11,10 +11,11 @@
 
 **Defined types**
 
-* [`firewalld::custom_service`](#firewalldcustom_service): == Type: firewalld::custom_service  Creates a new service definition for use in firewalld  See the README.md for usage instructions for this 
+* [`firewalld::custom_service`](#firewalldcustom_service): Creates a new service definition for use in firewalld
 
 **Resource types**
 
+* [`firewalld_custom_service`](#firewalld_custom_service): Creates a custom firewalld service.
 * [`firewalld_direct_chain`](#firewalld_direct_chain): Allow to create a custom chain in iptables/ip6tables/ebtables using firewalld direct interface.  Example:      firewalld_direct_chain {'Add c
 * [`firewalld_direct_passthrough`](#firewalld_direct_passthrough): Allow to create a custom passthroughhrough traffic in iptables/ip6tables/ebtables using firewalld direct interface.  Example:      firewalld_
 * [`firewalld_direct_purge`](#firewalld_direct_purge): Allow to purge direct rules in iptables/ip6tables/ebtables using firewalld direct interface.  Example:      firewalld_direct_purge {'chain': 
@@ -306,37 +307,39 @@ A common point for triggering an intermediary firewalld full reload using firewa
 
 ### firewalld::custom_service
 
-== Type: firewalld::custom_service
+**DEPRECATED**: Please use the `firewalld_custom_service` native type moving forward
 
-Creates a new service definition for use in firewalld
+This defined type will be removed in a future release
 
-See the README.md for usage instructions for this defined type
+Andrew Patik <andrewpatik@gmail.com>
+Trevor Vaughan <tvaughan@onyxpoint.com>
 
-=== Examples
+#### Examples
 
-   firewalld::custom_service{'My Custom Service':
-     short       => 'MyService',
-     description => 'My Custom Service is a daemon that does whatever',
-     port        => [
-       {
-           'port'     => '1234'
-           'protocol' => 'tcp'
-       },
-       {
-           'port'     => '1234'
-           'protocol' => 'udp'
-       },
-     ],
-     module      => ['nf_conntrack_netbios_ns'],
-     destination => {
-       'ipv4' => '127.0.0.1',
-       'ipv6' => '::1'
-     }
-   }
+##### 
 
-=== Authors
+```puppet
 
-Andrew Patik <andrewpatik@gmail.com>
+firewalld::custom_service{'My Custom Service':
+  short       => 'MyService',
+  description => 'My Custom Service is a daemon that does whatever',
+  port        => [
+    {
+        'port'     => '1234'
+        'protocol' => 'tcp'
+    },
+    {
+        'port'     => '1234'
+        'protocol' => 'udp'
+    },
+  ],
+  module      => ['nf_conntrack_netbios_ns'],
+  destination => {
+    'ipv4' => '127.0.0.1',
+    'ipv6' => '::1'
+  }
+}
+```
 
 #### Parameters
 
@@ -411,6 +414,96 @@ Default value: 'present'
 
 ## Resource types
 
+### firewalld_custom_service
+
+You will still need to create a `firewalld_service` resource to bind your new
+service to a zone.
+
+#### Examples
+
+##### Creating a custom 'test' service
+
+```puppet
+firewalld_custom_service {'test':
+    ensure  => present,
+    ports   => [{'port' => '1234', 'protocol' => 'tcp'}]
+}
+```
+
+#### Properties
+
+The following properties are available in the `firewalld_custom_service` type.
+
+##### `ensure`
+
+Valid values: present, absent
+
+The basic property that the resource should be in.
+
+Default value: present
+
+##### `short`
+
+Valid values: %r{.+}
+
+The short description of the service
+
+##### `description`
+
+Valid values: %r{.+}
+
+The long description of the service
+
+##### `ports`
+
+An Array of allowed port/protocol Hashes or Strings of the form `port/protocol`
+
+Default value: unset
+
+##### `protocols`
+
+Valid values: %r{^[^\s#]+$}
+
+Protocols allowed by the service as defined in /etc/protocols
+
+Default value: unset
+
+##### `modules`
+
+Valid values: %r{^[\w-]+$}
+
+The list of netfilter modules to add to the service
+
+Default value: unset
+
+##### `ipv4_destination`
+
+Valid values: %r{^[^/]+(/\d+)?$}
+
+The IPv4 destination network of the service
+
+Default value: unset
+
+##### `ipv6_destination`
+
+Valid values: %r{^[^/]+(/\d+)?$}
+
+The IPv6 destination network of the service
+
+Default value: unset
+
+#### Parameters
+
+The following parameters are available in the `firewalld_custom_service` type.
+
+##### `name`
+
+Valid values: %r{.+}
+
+namevar
+
+The target filename of the resource (without the .xml suffix)
+
 ### firewalld_direct_chain
 
 Allow to create a custom chain in iptables/ip6tables/ebtables using firewalld direct interface.
@@ -1026,7 +1119,7 @@ Result => 'B--d--Characters--.txt'
 
 #### `firewalld::safe_filename(String[1] $filename, Struct[
     {
-      'replacement_string' => Pattern[/[\w-]/],
+      'replacement_string' => Pattern[/^[\w-]+$/],
       'file_extension'     => Optional[String[1]]
     }
   ] $options = { 'replacement_string' => '_'})`
@@ -1071,7 +1164,7 @@ The String to process
 
 Data type: `Struct[
     {
-      'replacement_string' => Pattern[/[\w-]/],
+      'replacement_string' => Pattern[/^[\w-]+$/],
       'file_extension'     => Optional[String[1]]
     }
   ]`
diff --git a/functions/safe_filename.pp b/functions/safe_filename.pp
index a4e693d8..ffd8c76e 100644
--- a/functions/safe_filename.pp
+++ b/functions/safe_filename.pp
@@ -38,7 +38,7 @@ function firewalld::safe_filename(
   String[1] $filename,
   Struct[
     {
-      'replacement_string' => Pattern[/[\w-]/],
+      'replacement_string' => Pattern[/^[\w-]+$/],
       'file_extension'     => Optional[String[1]]
     }
   ]         $options  = { 'replacement_string' => '_'}
diff --git a/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb b/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb
new file mode 100644
index 00000000..666e5790
--- /dev/null
+++ b/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb
@@ -0,0 +1,235 @@
+require 'puppet'
+require File.join(File.dirname(__FILE__), '..', 'firewalld.rb')
+
+Puppet::Type.type(:firewalld_custom_service).provide(
+  :firewall_cmd,
+  parent: Puppet::Provider::Firewalld
+) do
+  desc 'Interact with firewall-cmd'
+
+  mk_resource_methods
+
+  def exists?
+    builtin = true
+
+    found_resource = execute_firewall_cmd(['--get-services'], nil).strip.split(' ').include?(@resource[:name])
+
+    if found_resource && execute_firewall_cmd(['--path-service', @resource[:name]], nil).start_with?('/etc')
+      builtin = false
+    end
+
+    return false if builtin && (@resource[:ensure] == :absent)
+
+    found_resource
+  end
+
+  def create
+    debug("Adding new custom service to firewalld: #{@resource[:name]}")
+    execute_firewall_cmd(['--new-service', @resource[:name]], nil)
+
+    send(:short=, @resource[:short]) if @resource[:short]
+    send(:description=, @resource[:description]) if @resource[:description]
+    ports && send(:ports=, @resource[:ports]) unless @resource[:ports].include?(:unset)
+    protocols && send(:protocols=, @resource[:protocols]) unless @resource[:protocols].include?(:unset)
+    modules && send(:modules=, @resource[:modules]) unless @resource[:modules].include?(:unset)
+    send(:ipv4_destination=, @resource[:ipv4_destination]) unless @resource[:ipv4_destination] == :unset
+    send(:ipv6_destination=, @resource[:ipv6_destination]) unless @resource[:ipv6_destination] == :unset
+  end
+
+  def destroy
+    execute_firewall_cmd(['--delete-service', @resource[:name]], nil)
+  rescue Puppet::ExecutionFailure
+    execute_firewall_cmd(['--load-service-default', @resource[:name]], nil)
+  end
+
+  def short
+    execute_firewall_cmd(['--service', @resource[:name], '--get-short'], nil).strip
+  end
+
+  def short=(should)
+    execute_firewall_cmd(['--service', @resource[:name], '--set-short', should], nil)
+  end
+
+  def description
+    execute_firewall_cmd(['--service', @resource[:name], '--get-description'], nil).strip
+  end
+
+  def description=(should)
+    execute_firewall_cmd(['--service', @resource[:name], '--set-description', should], nil)
+  end
+
+  def ports
+    @property_hash[:ports] = execute_firewall_cmd(['--service', @resource[:name], '--get-ports'], nil).strip.split(%r{\s+}).map do |entry|
+      port, proto = entry.strip.split('/')
+      { 'port' => port, 'protocol' => proto }
+    end
+
+    @property_hash[:ports]
+  end
+
+  def ports=(should)
+    to_add = []
+    to_remove = []
+
+    if Array(should).include?(:unset)
+      to_remove = @property_hash[:ports]
+    else
+      to_remove = @property_hash[:ports] - should
+      to_add = should - @property_hash[:ports]
+    end
+
+    errors = []
+    to_add.each do |entry|
+      # Protocols could have been specified in there
+      next unless entry['port']
+
+      begin
+        port_str = "#{entry['port']}/#{entry['protocol']}"
+
+        execute_firewall_cmd(['--service', @resource[:name], '--add-port', port_str], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not add port '#{port_str} to #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    to_remove .each do |entry|
+      begin
+        port_str = "#{entry['port']}/#{entry['protocol']}"
+
+        execute_firewall_cmd(['--service', @resource[:name], '--remove-port', port_str], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not remove port '#{port_str} from #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    raise Puppet::ResourceError, errors.join("\n") unless errors.empty?
+  end
+
+  def protocols
+    @property_hash[:protocols] = execute_firewall_cmd(['--service', @resource[:name], '--get-protocols'], nil).strip.split(%r{\s+})
+
+    @property_hash[:protocols]
+  end
+
+  def protocols=(should)
+    to_add = []
+    to_remove = []
+
+    if Array(should).include?(:unset)
+      to_remove = @property_hash[:protocols]
+    else
+      to_remove = @property_hash[:protocols] - should
+      to_add = (
+        should + Array(@resource[:ports]).select { |x| x['port'].nil? }.map { |x| x['protocol'] }
+      ) - @property_hash[:protocols]
+    end
+
+    errors = []
+    to_add.each do |entry|
+      begin
+        execute_firewall_cmd(['--service', @resource[:name], '--add-protocol', entry], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not add protocol '#{entry} to #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    to_remove.each do |entry|
+      begin
+        execute_firewall_cmd(['--service', @resource[:name], '--remove-protocol', entry], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not remove protocol'#{entry} from #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    raise Puppet::ResourceError, errors.join("\n") unless errors.empty?
+  end
+
+  def modules
+    @property_hash[:modules] = execute_firewall_cmd(['--service', @resource[:name], '--get-modules'], nil).strip.split(%r{\s+})
+
+    @property_hash[:modules]
+  end
+
+  def modules=(should)
+    to_add = []
+    to_remove = []
+
+    if Array(should).include?(:unset)
+      to_remove = @property_hash[:modules]
+    else
+      to_remove = @property_hash[:modules] - should
+      to_add = should - @property_hash[:modules]
+    end
+
+    errors = []
+    to_add.each do |entry|
+      begin
+        execute_firewall_cmd(['--service', @resource[:name], '--add-module', entry], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not add module '#{entry} to #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    to_remove.each do |entry|
+      begin
+        execute_firewall_cmd(['--service', @resource[:name], '--remove-module', entry], nil)
+      rescue Puppet::ExecutionFailure => e
+        errors << "Could not remove module '#{entry} from #{@resource[:name]}' => #{e}"
+      end
+    end
+
+    raise Puppet::ResourceError, errors.join("\n") unless errors.empty?
+  end
+
+  def ipv4_destination
+    @property_hash[:ipv4_destination] = destinations['ipv4']
+    @property_hash[:ipv4_destination] ||= ''
+
+    @property_hash[:ipv4_destination]
+  end
+
+  def ipv4_destination=(should)
+    if should == :unset
+      execute_firewall_cmd(['--service', @resource[:name], '--remove-destination', 'ipv4'], nil) unless @property_hash[:ipv4_destination].empty?
+    else
+      execute_firewall_cmd(['--service', @resource[:name], '--set-destination', "ipv4:#{should}"], nil)
+    end
+  end
+
+  def ipv6_destination
+    @property_hash[:ipv6_destination] = destinations['ipv6']
+    @property_hash[:ipv6_destination] ||= ''
+
+    @property_hash[:ipv6_destination]
+  end
+
+  def ipv6_destination=(should)
+    if should == :unset
+      execute_firewall_cmd(['--service', @resource[:name], '--remove-destination', 'ipv6'], nil) unless @property_hash[:ipv6_destination].empty?
+    else
+      execute_firewall_cmd(['--service', @resource[:name], '--set-destination', "ipv6:#{should}"], nil)
+    end
+  end
+
+  def flush
+    reload_firewall
+  end
+
+  private
+
+  # Return a Hash of destinations
+  #
+  # @example IPv4 and IPv6 destinations
+  #
+  #   { 'ipv4' => '1.2.3.0/24', 'ipv6' => '::1' }
+  #
+  # @return [Hash[String,String]]
+  def destinations
+    return @destinations if @destinations
+
+    @destinations = execute_firewall_cmd(['--service', @resource[:name], '--get-destinations'], nil).strip.split(%r{\s+})
+    @destinations = Hash[@destinations.map { |x| x.split(':', 2) }]
+
+    @destinations
+  end
+end
diff --git a/lib/puppet/type/firewalld_custom_service.rb b/lib/puppet/type/firewalld_custom_service.rb
new file mode 100644
index 00000000..3f26583f
--- /dev/null
+++ b/lib/puppet/type/firewalld_custom_service.rb
@@ -0,0 +1,210 @@
+require 'puppet'
+
+Puppet::Type.newtype(:firewalld_custom_service) do
+  desc <<-DOC
+  @summary Creates a custom firewalld service.
+
+
+  You will still need to create a `firewalld_service` resource to bind your new
+  service to a zone.
+
+  @example Creating a custom 'test' service
+    firewalld_custom_service {'test':
+        ensure  => present,
+        ports   => [{'port' => '1234', 'protocol' => 'tcp'}]
+    }
+  DOC
+
+  ensurable do
+    defaultvalues
+    defaultto(:present)
+  end
+
+  newparam(:name, namevar: :true) do
+    desc 'The target filename of the resource (without the .xml suffix)'
+
+    newvalues(%r{.+})
+
+    munge do |value|
+      value.gsub(%r{[^\w-]}, '_')
+    end
+  end
+
+  newproperty(:short) do
+    desc 'The short description of the service'
+
+    newvalues(%r{.+})
+  end
+
+  newproperty(:description) do
+    desc 'The long description of the service'
+
+    newvalues(%r{.+})
+  end
+
+  newproperty(:ports, array_matching: :all) do
+    desc 'An Array of allowed port/protocol Hashes or Strings of the form `port/protocol`'
+
+    defaultto(:unset)
+
+    munge do |value|
+      return value if value == :unset
+
+      if value.is_a?(Hash)
+        value = Hash[value.map { |k, v| [k, v.to_s] }]
+      else
+        port, protocol = value.split('/')
+
+        # Just a protocol is valid
+        if port && !protocol
+          value = { 'protocol' => port }
+        else
+          port = port.to_s
+
+          # Handle the legacy format from the module
+          port.tr!(':', '-')
+
+          value = { 'port' => port, 'protocol' => protocol }
+        end
+      end
+
+      value
+    end
+
+    validate do |value|
+      return if value == :unset
+
+      value = munge(value)
+
+      if value.is_a?(Hash)
+        raise Puppet::ParseError, 'You must specify a protocol' unless value['protocol']
+
+        if value['port']
+          test_value = value['port'].to_s
+
+          port_regexp = Regexp.new('^\d+(-\d+)?$')
+          raise Puppet::ParseError, "Ports must match #{port_regexp}" unless port_regexp.match?(test_value)
+
+          invalid_ports = test_value.split('-').select { |x| (x == '0') || (x > '65535') }
+          raise Puppet::ParseError, %(Ports must be between 1-65535 instead of '#{invalid_ports.join("' ,'")}') unless invalid_ports.empty?
+        end
+
+        allowed_protocols = %w[tcp udp sctp dccp]
+        unless allowed_protocols.include?(value['protocol'])
+          raise Puppet::ParseError, "The protocol must be one of '#{allowed_protocols.join(', ')}'. Got '#{value['protocol']}'"
+        end
+      end
+    end
+
+    def insync?(is)
+      return true if Array(should).include?(:unset) && Array(is).empty?
+      return false if Array(should).include?(:unset) && !Array(is).empty?
+
+      is.reject { |x| x['port'].nil? }.sort_by { |x| x['port'] } ==
+        should.reject { |x| x['port'].nil? }.sort_by { |x| x['port'] }
+    end
+  end
+
+  newproperty(:protocols, array_matching: :all) do
+    desc 'Protocols allowed by the service as defined in /etc/protocols'
+
+    newvalues(%r{^[^\s#]+$})
+
+    defaultto(:unset)
+
+    def insync?(is)
+      return true if Array(should).include?(:unset) && Array(is).empty?
+      return false if Array(should).include?(:unset) && !Array(is).empty?
+
+      protocols = Array(should)
+
+      unless Array(@resource[:ports]).include?(:unset)
+        protocols = (
+          Array(should) +
+          @resource[:ports].select { |x| x['port'].nil? }.map { |x| x['protocol'] }
+        ).uniq
+      end
+
+      protocols.sort == Array(is).sort
+    end
+  end
+
+  newproperty(:modules, array_matching: :all) do
+    desc 'The list of netfilter modules to add to the service'
+
+    newvalues(%r{^[\w-]+$})
+
+    defaultto(:unset)
+
+    munge do |value|
+      return value if value == :unset
+
+      value.split('nf_conntrack_').last
+    end
+
+    def insync?(is)
+      return true if Array(should).include?(:unset) && Array(is).empty?
+      return false if Array(should).include?(:unset) && !Array(is).empty?
+
+      Array(is).sort == Array(should).sort
+    end
+  end
+
+  newproperty(:ipv4_destination) do
+    desc 'The IPv4 destination network of the service'
+
+    newvalues(%r{^[^/]+(/\d+)?$})
+
+    defaultto(:unset)
+
+    validate do |value|
+      return if value == :unset
+
+      require 'ipaddr'
+
+      begin
+        addr = IPAddr.new(value)
+
+        raise Puppet::ParseError, "#{value} is not an IPv4 address" unless addr.ipv4?
+      rescue => e
+        raise Puppet::ParseError, e.to_s
+      end
+    end
+
+    def insync?(is)
+      return true if should == :unset && is.empty?
+      is == should
+    end
+  end
+
+  newproperty(:ipv6_destination) do
+    desc 'The IPv6 destination network of the service'
+
+    newvalues(%r{^[^/]+(/\d+)?$})
+
+    defaultto(:unset)
+
+    validate do |value|
+      return if value == :unset
+
+      require 'ipaddr'
+
+      begin
+        addr = IPAddr.new(value)
+
+        raise Puppet::ParseError, "#{value} is not an IPv6 address" unless addr.ipv6?
+      rescue => e
+        raise Puppet::ParseError, e.to_s
+      end
+    end
+
+    def insync?(is)
+      return true if should == :unset && is.empty?
+      is == should
+    end
+  end
+
+  autorequire(:service) do
+    ['firewalld']
+  end
+end
diff --git a/lib/puppet/type/firewalld_service.rb b/lib/puppet/type/firewalld_service.rb
index ce5c5bea..3cc6bffb 100644
--- a/lib/puppet/type/firewalld_service.rb
+++ b/lib/puppet/type/firewalld_service.rb
@@ -49,9 +49,10 @@
   end
 
   autorequire(:service) do
-    ['firewalld'] +
-      catalog.resources.select do |res|
-        res.title == "Firewalld::Custom_service[#{self[:service]}]"
-      end
+    ['firewalld']
+  end
+
+  autorequire(:firewalld_custom_service) do
+    self[:service].gsub(%r{[^\w-]}, '_') if self[:service]
   end
 end
diff --git a/manifests/custom_service.pp b/manifests/custom_service.pp
index ba0f4ec6..06c138d4 100644
--- a/manifests/custom_service.pp
+++ b/manifests/custom_service.pp
@@ -1,10 +1,10 @@
-# == Type: firewalld::custom_service
+# @summary Creates a new service definition for use in firewalld
 #
-# Creates a new service definition for use in firewalld
+# **DEPRECATED**: Please use the `firewalld_custom_service` native type moving forward
 #
-# See the README.md for usage instructions for this defined type
+# This defined type will be removed in a future release
 #
-# === Examples
+# @example
 #
 #    firewalld::custom_service{'My Custom Service':
 #      short       => 'MyService',
@@ -26,10 +26,8 @@
 #      }
 #    }
 #
-# === Authors
-#
 # Andrew Patik <andrewpatik@gmail.com>
-#
+# Trevor Vaughan <tvaughan@onyxpoint.com>
 #
 define firewalld::custom_service (
   String                   $short       = $name,
@@ -45,30 +43,28 @@
   Enum['present','absent'] $ensure      = 'present',
 ) {
 
-  include firewalld::reload
+  $_args = delete_undef_values({
+    'ensure'           => $ensure,
+    'short'            => $short,
+    'description'      => $description,
+    'ports'            => $port,
+    'modules'          => $module,
+    'ipv4_destination' => $destination.dig('ipv4'),
+    'ipv6_destination' => $destination.dig('ipv6'),
+  })
 
-  # 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
+  # Remove legacy files so that we don't end up with conflicts
+  #
+  # This functionality will be removed in a future release
+  unless $_safe_filename == $short {
+    file { "${config_dir}/${short}.xml":
+      ensure => absent,
     }
-  )
+  }
 
-  file{ "${config_dir}/${_safe_filename}.xml":
-    ensure  => $ensure,
-    content => $_content,
-    mode    => '0644',
-    notify  => Class['firewalld::reload'],
+  firewalld_custom_service { $_safe_filename:
+    * => $_args,
   }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
index fe1d0552..90ce591a 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -239,11 +239,6 @@
     }
   }
 
-  # Custom service files have to be in place before the zone is triggered
-  # otherwise you may end up with the zone being unable to find declared
-  # services.
-  Firewalld::Custom_service <||> -> Firewalld_zone <||>
-
   # TODO: Replace these with a native firewalld_reload type and automatically
   # hook the types together properly.
   Firewalld_direct_chain <||> ~> Class['firewalld::reload']
diff --git a/spec/acceptance/suites/default/00_default_spec.rb b/spec/acceptance/suites/default/00_default_spec.rb
index f27fb19b..ecb33581 100644
--- a/spec/acceptance/suites/default/00_default_spec.rb
+++ b/spec/acceptance/suites/default/00_default_spec.rb
@@ -6,60 +6,203 @@
   # Additional tests should be added to cover all module capabilities
   hosts.each do |host|
     context "on #{host}" do
-      let(:manifest) do
-        <<-EOM
-          class { 'firewalld':
-            lockdown     => 'yes',
-            default_zone => 'test',
-            log_denied   => 'unicast'
-          }
-
-          firewalld_zone { 'test':
-            ensure           => 'present',
-            purge_rich_rules => true,
-            purge_services   => true,
-            purge_ports      => true,
-            target           => 'DROP',
-            require          => Service['firewalld']
-          }
-
-          firewalld::custom_service { 'test_sshd':
-            short       => 'test_sshd',
-            description => 'Test SSH Access',
-            port        => [{ 'port' => '22', 'protocol' => 'tcp' }],
-            require     => Service['firewalld']
-          }
-
-          firewalld_service { 'test_sshd':
-            zone    => 'test',
-            require => Service['firewalld']
-          }
-        EOM
-      end
+      context 'custom services' do
+        let(:manifest) do
+          <<-EOM
+            class { 'firewalld':
+              lockdown     => 'yes',
+              default_zone => 'test',
+              log_denied   => 'unicast'
+            }
 
-      it 'runs successfully' do
-        apply_manifest_on(host, manifest, catch_failures: true)
-      end
+            class ssh_test {
+              firewalld_service{ 'test_sshd': zone => 'test' }
 
-      it 'is idempotent' do
-        apply_manifest_on(host, manifest, catch_changes: true)
-      end
+  # TODO: Switch this when the defined type gets deprecated
+              firewalld::custom_service{ 'test_sshd':
+                description => 'Test SSH Access',
+                port        => [{ 'port' => '22', 'protocol' => 'tcp' }]
+              }
+            }
 
-      it 'is running firewalld' do
-        svc = YAML.load(on(host, 'puppet resource service firewalld --to_yaml').output)
-        expect(svc['service']['firewalld']['ensure']).to match('running')
-      end
+            firewalld_zone{ 'test':
+              ensure           => 'present',
+              purge_rich_rules => true,
+              purge_services   => true,
+              purge_ports      => true,
+              target           => 'DROP'
+            }
+
+            class other_service {
+              firewalld_service{ 'test_thing': zone => 'test' }
+
+              #{test_thing_resource}
+            }
+
+            # Check for looping
+            include ssh_test
+            include other_service
+
+            Class['ssh_test'] -> Class['other_service']
+          EOM
+        end
+
+        context 'with a default test resource' do
+          let(:test_thing_resource) do
+            <<-EOM
+              firewalld_custom_service{ 'test_thing':
+                description      => 'Random service test',
+                ports            => ['1234/tcp', { 'port' => '1234', 'protocol' => 'udp' }],
+                protocols        => ['ip', 'smp'],
+                modules          => ['nf_conntrack_tftp', 'nf_conntrack_snmp'],
+                ipv4_destination => '1.2.3.4/23',
+                ipv6_destination => '::1'
+              }
+            EOM
+          end
+
+          it 'runs successfully' do
+            apply_manifest_on(host, manifest, catch_failures: true)
+          end
+
+          it 'is idempotent' do
+            apply_manifest_on(host, manifest, catch_changes: true)
+          end
+
+          it 'is running firewalld' do
+            svc = YAML.load(on(host, 'puppet resource service firewalld --to_yaml').output)
+            expect(svc['service']['firewalld']['ensure']).to match('running')
+          end
+
+          it 'has "test" as the default zone' do
+            default_zone = on(host, 'firewall-cmd --get-default-zone').output.strip
+            expect(default_zone).to eq('test')
+          end
+
+          it 'has the "test_sshd" service in the "test" zone' do
+            test_services = on(host, 'firewall-cmd --list-services --zone=test').output.strip.split(%r{\s+})
+            expect(test_services).to include('test_sshd')
+          end
+
+          context 'custom service' do
+            it 'exists' do
+              expect(on(host, 'firewall-cmd --permanent --info-service=test_thing').output).not_to be_empty
+            end
+
+            it 'has the proper description' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-description').output.strip).to eq('Random service test')
+            end
+
+            it 'has no short description' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-short').output.strip).to be_empty
+            end
+
+            it 'has the proper ports' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-ports').output.strip).to eq('1234/tcp 1234/udp')
+            end
 
-      it 'has "test" as the default zone' do
-        default_zone = on(host, 'firewall-cmd --get-default-zone').output.strip
-        expect(default_zone).to eq('test')
+            it 'has the proper protocols' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-protocols').output.strip).to eq('ip smp')
+            end
+
+            it 'has the proper modules' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-modules').output.strip).to eq('tftp snmp')
+            end
+
+            it 'has the proper destinations' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-destinations').output.strip).to eq('ipv4:1.2.3.4/23 ipv6:::1')
+            end
+          end
+        end
+
+        context 'with a modified test resource' do
+          let(:test_thing_resource) do
+            <<-EOM
+              firewalld_custom_service{ 'test_thing':
+                short => 'Short test',
+                ports => ['1235/tcp', { 'port' => '1236', 'protocol' => 'tcp' }],
+              }
+            EOM
+          end
+
+          it 'runs successfully' do
+            apply_manifest_on(host, manifest, catch_failures: true)
+          end
+
+          it 'is idempotent' do
+            apply_manifest_on(host, manifest, catch_changes: true)
+          end
+
+          context 'custom service' do
+            it 'exists' do
+              expect(on(host, 'firewall-cmd --permanent --info-service=test_thing').output).not_to be_empty
+            end
+
+            it 'retains the description' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-description').output.strip).to eq('Random service test')
+            end
+
+            it 'has the proper short description' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-short').output.strip).to eq('Short test')
+            end
+
+            it 'has the proper ports' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-ports').output.strip).to eq('1235/tcp 1236/tcp')
+            end
+
+            it 'has no protocols' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-protocols').output.strip).to be_empty
+            end
+
+            it 'has no modules' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-modules').output.strip).to be_empty
+            end
+
+            it 'has no destinations' do
+              expect(on(host, 'firewall-cmd --permanent --service=test_thing --get-destinations').output.strip).to be_empty
+            end
+          end
+        end
       end
 
-      it 'has the "test_sshd" service in the "test" zone' do
-        test_services = on(host, 'firewall-cmd --list-services --zone=test').output.strip.split(%r{\s+})
-        expect(test_services).to include('test_sshd')
+      context 'built-in overrides' do
+        let(:manifest) do
+          <<-EOM
+            firewalld_custom_service{ 'dhcp':
+              short            => 'DHCP Override',
+              description      => 'The DHCP Defaults are Silly',
+              ports            => ['1234/tcp', { 'port' => '1234', 'protocol' => 'udp' }],
+              protocols        => ['ip', 'smp'],
+              modules          => ['nf_conntrack_tftp', 'nf_conntrack_snmp'],
+              ipv4_destination => '1.2.3.4/23',
+              ipv6_destination => '::1'
+            }
+          EOM
+        end
+
+        let(:cleanup_manifest) { "firewalld_custom_service{ 'dhcp': ensure => 'absent' }" }
+
+        it 'overrides built-in services' do
+          apply_manifest_on(host, manifest, catch_failures: true)
+          expect(file_exists_on(host, '/etc/firewalld/services/dhcp.xml')).to be true
+        end
+
+        it 'is idempotent' do
+          apply_manifest_on(host, manifest, catch_changes: true)
+        end
+
+        it 'removes override changes' do
+          apply_manifest_on(host, cleanup_manifest, catch_failures: true)
+          expect(file_exists_on(host, '/etc/firewalld/services/dhcp.xml')).to be false
+        end
+
+        it 'is idempotent' do
+          apply_manifest_on(host, cleanup_manifest, catch_changes: true)
+        end
       end
+    end
 
+    context 'disable firewalld' do
       it 'returns a fact when firewalld is not running' do
         on(host, 'puppet resource service firewalld ensure=stopped')
         expect(pfact_on(host, 'firewalld_version')).to match(%r{^\d})
diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
index 4fb4ba7a..d93b03ea 100644
--- a/spec/classes/init_spec.rb
+++ b/spec/classes/init_spec.rb
@@ -192,11 +192,42 @@
     end
 
     it do
+      is_expected.not_to contain_file('/etc/firewalld/services/MyService.xml')
+
       is_expected.to contain_firewalld__custom_service('MyService').
         with_ensure('present').
         with_short('MyService').
         with_port([{ 'port' => '1234', 'protocol' => 'tcp' }, { 'port' => '1234', 'protocol' => 'udp' }])
     end
+
+    context 'with an invalid filename' do
+      let(:params) do
+        {
+          'custom_services' =>
+          {
+            'MyService' =>
+              {
+                'ensure' => 'present',
+                'short' => 'My Service',
+                'description' => 'My Custom service',
+                'port' => [
+                  { 'port' => '1234', 'protocol' => 'tcp' },
+                  { 'port' => '1234', 'protocol' => 'udp' }
+                ]
+              }
+          }
+        }
+      end
+
+      it do
+        is_expected.to contain_file('/etc/firewalld/services/My Service.xml').with_ensure('absent')
+
+        is_expected.to contain_firewalld__custom_service('MyService').
+          with_short('My Service').
+          with_ensure('present').
+          with_port([{ 'port' => '1234', 'protocol' => 'tcp' }, { 'port' => '1234', 'protocol' => 'udp' }])
+      end
+    end
   end
 
   context 'with default_zone' do
diff --git a/spec/defines/custom_service_spec.rb b/spec/defines/custom_service_spec.rb
index ae9387ba..a629ab6a 100644
--- a/spec/defines/custom_service_spec.rb
+++ b/spec/defines/custom_service_spec.rb
@@ -2,13 +2,6 @@
 
 describe 'firewalld::custom_service' do
   let(:title) { 'My Service' }
-  let(:xml) do
-    File.read(File.join(File.dirname(__FILE__), '..', 'fixtures', 'services', 'custom_service.xml'))
-  end
-
-  let(:xml_port_range) do
-    File.read(File.join(File.dirname(__FILE__), '..', 'fixtures', 'services', 'custom_service_port_range.xml'))
-  end
 
   context 'when defining with specific ports' do
     let(:params) do
@@ -54,64 +47,21 @@
     end
 
     it do
-      is_expected.to contain_file('/etc/firewalld/services/myservice.xml').with(
-        content: xml
+      is_expected.to contain_firewalld_custom_service('myservice').with(
+        short: params[:short],
+        description: params[:description],
+        ports: params[:port],
+        modules: params[:module],
+        ipv4_destination: params[:destination]['ipv4'],
+        ipv6_destination: params[:destination]['ipv6']
       )
     end
   end
-  context 'when defining with specific filename' do
-    let(:params) do
-      {
-        short: 'myservice',
-        filename: 'myservice_file',
-        description: 'My multi port service',
-        port: [
-          {
-            'port'     => '8000',
-            'protocol' => 'tcp'
-          },
-          {
-            'port'     => '8000',
-            'protocol' => 'udp'
-          },
-          {
-            'port'     => '8001',
-            'protocol' => 'tcp'
-          },
-          {
-            'port'     => '8001',
-            'protocol' => 'udp'
-          },
-          {
-            'port'     => '8002',
-            'protocol' => 'tcp'
-          },
-          {
-            'port'     => '8002',
-            'protocol' => 'udp'
-          },
-          {
-            'protocol' => 'vrrp'
-          }
-        ],
-        module: ['nf_conntrack_netbios_ns'],
-        destination: {
-          'ipv4' => '127.0.0.1',
-          'ipv6' => '::1'
-        }
-      }
-    end
 
-    it do
-      is_expected.to contain_file('/etc/firewalld/services/myservice_file.xml').with(
-        content: xml
-      )
-    end
-  end
   context 'when defining with integer ports' do
     let(:params) do
       {
-        short: 'myservice',
+        short: 'my service',
         filename: 'myservice_file',
         description: 'My multi port service',
         port: [
@@ -154,11 +104,17 @@
     end
 
     it do
-      is_expected.to contain_file('/etc/firewalld/services/myservice_file.xml').with(
-        content: xml
+      is_expected.to contain_firewalld_custom_service('myservice_file').with(
+        short: params[:short],
+        description: params[:description],
+        ports: params[:port],
+        modules: params[:module],
+        ipv4_destination: params[:destination]['ipv4'],
+        ipv6_destination: params[:destination]['ipv6']
       )
     end
   end
+
   context 'when defining with a port range' do
     let(:params) do
       {
@@ -173,18 +129,15 @@
             'port'     => '8000:8002',
             'protocol' => 'udp'
           }
-        ],
-        module: ['nf_conntrack_netbios_ns'],
-        destination: {
-          'ipv4' => '127.0.0.1',
-          'ipv6' => '::1'
-        }
+        ]
       }
     end
 
     it do
-      is_expected.to contain_file('/etc/firewalld/services/myservice.xml').with(
-        content: xml_port_range
+      is_expected.to contain_firewalld_custom_service('myservice').with(
+        short: params[:short],
+        description: params[:description],
+        ports: params[:port]
       )
     end
   end
diff --git a/spec/fixtures/services/custom_service.xml b/spec/fixtures/services/custom_service.xml
deleted file mode 100644
index 12d568a6..00000000
--- a/spec/fixtures/services/custom_service.xml
+++ /dev/null
@@ -1,17 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<service>
-    <short>myservice</short>
-    <description>My multi port service</description>
-
-    <port protocol="tcp" port="8000" />
-    <port protocol="udp" port="8000" />
-    <port protocol="tcp" port="8001" />
-    <port protocol="udp" port="8001" />
-    <port protocol="tcp" port="8002" />
-    <port protocol="udp" port="8002" />
-    <port protocol="vrrp" port="" />
-
-    <module name="nf_conntrack_netbios_ns" />
-
-    <destination ipv4="127.0.0.1" ipv6="::1" />
-</service>
diff --git a/spec/fixtures/services/custom_service_port_range.xml b/spec/fixtures/services/custom_service_port_range.xml
deleted file mode 100644
index a5df5bc8..00000000
--- a/spec/fixtures/services/custom_service_port_range.xml
+++ /dev/null
@@ -1,12 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<service>
-    <short>myservice</short>
-    <description>My multi port service</description>
-
-    <port protocol="tcp" port="8000-8002" />
-    <port protocol="udp" port="8000-8002" />
-
-    <module name="nf_conntrack_netbios_ns" />
-
-    <destination ipv4="127.0.0.1" ipv6="::1" />
-</service>
diff --git a/spec/unit/puppet/provider/firewalld_custom_service_spec.rb b/spec/unit/puppet/provider/firewalld_custom_service_spec.rb
new file mode 100644
index 00000000..3bbf0710
--- /dev/null
+++ b/spec/unit/puppet/provider/firewalld_custom_service_spec.rb
@@ -0,0 +1,331 @@
+require 'spec_helper'
+
+provider_class = Puppet::Type.type(:firewalld_custom_service).provider(:firewall_cmd)
+
+describe provider_class do
+  require 'rexml/document'
+  include REXML
+
+  # rubocop:disable RSpec/MultipleExpectations
+
+  before do
+    # rubocop:disable RSpec/AnyInstance
+    provider.class.stubs(:execute_firewall_cmd).returns(Object.any_instance.stubs(exitstatus: 0))
+    # rubocop:enable RSpec/AnyInstance
+  end
+
+  let(:provider) { resource.provider }
+
+  context 'simplest resource creation' do
+    let(:resource) do
+      Puppet::Type.type(:firewalld_custom_service).new(
+        ensure: :present,
+        name: 'test_service'
+      )
+    end
+
+    it 'creates the service' do
+      provider.expects(:execute_firewall_cmd).with(['--new-service', resource[:name]], nil)
+      provider.create
+    end
+
+    it 'retrieves and formats the system ports' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--get-ports'], nil).returns('123/tcp 456/udp')
+
+      expect(provider.ports).to eq([
+                                     {
+                                       'port' => '123',
+                                       'protocol' => 'tcp'
+                                     },
+                                     {
+                                       'port' => '456',
+                                       'protocol' => 'udp'
+                                     }
+                                   ])
+    end
+
+    it 'retrieves and formats the system destinations' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--get-destinations'], nil).returns('ipv4:1.2.3.4/23 ipv6:::1')
+
+      expect(provider.ipv4_destination).to eq('1.2.3.4/23')
+      expect(provider.ipv6_destination).to eq('::1')
+    end
+
+    it 'exists when returned by the system' do
+      provider.expects(:execute_firewall_cmd).with(['--get-services'], nil).returns("#{resource[:name]} foo bar baz")
+      provider.expects(:execute_firewall_cmd).with(['--path-service', resource[:name]], nil).returns('/etc/foo_bar_baz.xml')
+
+      expect(provider.exists?).to eq true
+    end
+
+    it 'does not exist when not returned by the system' do
+      provider.expects(:execute_firewall_cmd).with(['--get-services'], nil).returns('foo bar baz')
+
+      expect(provider.exists?).to eq false
+    end
+  end
+
+  context 'resource deletion' do
+    let(:resource) do
+      Puppet::Type.type(:firewalld_custom_service).new(
+        ensure: :absent,
+        name: 'test_service'
+      )
+    end
+
+    it 'runs delete-service when it is not a builtin' do
+      provider.expects(:execute_firewall_cmd).with(['--delete-service', resource[:name]], nil)
+      provider.destroy
+    end
+
+    it 'runs load-service-defaults when it is a builtin' do
+      provider.expects(:execute_firewall_cmd).with(['--delete-service', resource[:name]], nil).raises(Puppet::ExecutionFailure, 'nooooooooooooo')
+      provider.expects(:execute_firewall_cmd).with(['--load-service-default', resource[:name]], nil)
+      provider.destroy
+    end
+  end
+
+  context 'all parameters populated' do
+    let(:resource) do
+      Puppet::Type.type(:firewalld_custom_service).new(
+        ensure: :present,
+        name: 'test_service',
+        short: 'Short Name',
+        description: 'This is a description',
+        ports: [
+          '123/tcp',
+          '234/udp',
+          { 'port' => 345, 'protocol' => 'udp' },
+          { 'port' => '456', 'protocol' => 'tcp' },
+          { 'protocol' => 'dccp' }
+        ],
+        protocols: %w[foo bar baz],
+        modules: %w[nf_thingy nf_other_thingy],
+        ipv4_destination: '1.2.3.0/24',
+        ipv6_destination: '::1'
+      )
+    end
+
+    it 'creates the service' do
+      provider.expects(:execute_firewall_cmd).with(['--new-service', resource[:name]], nil)
+
+      provider.expects(:short=).with(resource[:short])
+      provider.expects(:description=).with(resource[:description])
+      provider.expects(:ports).returns(true)
+      provider.expects(:ports=).with(resource[:ports])
+      provider.expects(:protocols).returns(true)
+      provider.expects(:protocols=).with(resource[:protocols])
+      provider.expects(:modules).returns(true)
+      provider.expects(:modules=).with(resource[:modules])
+      provider.expects(:ipv4_destination=).with(resource[:ipv4_destination])
+      provider.expects(:ipv6_destination=).with(resource[:ipv6_destination])
+
+      provider.create
+    end
+
+    it 'sets the short description' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--set-short', resource[:short]], nil)
+      provider.short = resource[:short]
+    end
+
+    it 'sets the full description' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--set-description', resource[:description]], nil)
+      provider.description = resource[:description]
+    end
+
+    context 'setting ports' do
+      it 'works without existing ports' do
+        [
+          '123/tcp',
+          '234/udp',
+          '345/udp',
+          '456/tcp'
+        ].each do |port|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-port', port], nil)
+        end
+
+        ports = []
+        provider.instance_variable_set('@property_hash', ports: ports)
+        provider.ports = resource[:ports]
+      end
+
+      it 'works with disjoint existing ports' do
+        [
+          '123/tcp',
+          '234/udp',
+          '345/udp',
+          '456/tcp'
+        ].each do |port|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-port', port], nil)
+        end
+
+        ports = [
+          { 'port' => '789', 'protocol' => 'udp' },
+          { 'port' => '8910', 'protocol' => 'tcp' }
+        ]
+        provider.instance_variable_set('@property_hash', ports: ports)
+
+        [
+          '789/udp',
+          '8910/tcp'
+        ].each do |port|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-port', port], nil)
+        end
+
+        provider.ports = resource[:ports]
+      end
+
+      it 'works with overlapping existing ports' do
+        [
+          '123/tcp',
+          '234/udp',
+          '456/tcp'
+        ].each do |port|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-port', port], nil)
+        end
+
+        ports = [
+          { 'port' => '345', 'protocol' => 'udp' },
+          { 'port' => '8910', 'protocol' => 'tcp' }
+        ]
+        provider.instance_variable_set('@property_hash', ports: ports)
+
+        [
+          '8910/tcp'
+        ].each do |port|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-port', port], nil)
+        end
+
+        provider.ports = resource[:ports]
+      end
+    end
+
+    context 'setting protocols' do
+      it 'works without existing protocols' do
+        %w[
+          foo
+          bar
+          baz
+          dccp
+        ].each do |protocol|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-protocol', protocol], nil)
+        end
+
+        protocols = []
+        provider.instance_variable_set('@property_hash', protocols: protocols)
+        provider.protocols = resource[:protocols]
+      end
+
+      it 'works with disjoint existing protocols' do
+        %w[
+          foo
+          bar
+          baz
+          dccp
+        ].each do |protocol|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-protocol', protocol], nil)
+        end
+
+        protocols = %w[
+          icmp
+          test
+        ]
+        provider.instance_variable_set('@property_hash', protocols: protocols)
+
+        protocols.each do |protocol|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-protocol', protocol], nil)
+        end
+
+        provider.protocols = resource[:protocols]
+      end
+
+      it 'works with overlapping existing protocols' do
+        %w[
+          foo
+          baz
+          dccp
+        ].each do |protocol|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-protocol', protocol], nil)
+        end
+
+        protocols = %w[
+          bar
+          icmp
+        ]
+        provider.instance_variable_set('@property_hash', protocols: protocols)
+
+        ['icmp'].each do |protocol|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-protocol', protocol], nil)
+        end
+
+        provider.protocols = resource[:protocols]
+      end
+    end
+
+    context 'setting modules' do
+      it 'works without existing modules' do
+        %w[
+          nf_thingy
+          nf_other_thingy
+        ].each do |mod|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-module', mod], nil)
+        end
+
+        modules = []
+        provider.instance_variable_set('@property_hash', modules: modules)
+        provider.modules = resource[:modules]
+      end
+
+      it 'works with disjoint existing modules' do
+        %w[
+          nf_thingy
+          nf_other_thingy
+        ].each do |mod|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-module', mod], nil)
+        end
+
+        modules = %w[
+          nf_stuff
+          nf_bob
+        ]
+        provider.instance_variable_set('@property_hash', modules: modules)
+
+        modules.each do |mod|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-module', mod], nil)
+        end
+
+        provider.modules = resource[:modules]
+      end
+
+      it 'works with overlapping existing modules' do
+        [
+          'nf_other_thingy'
+        ].each do |mod|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--add-module', mod], nil)
+        end
+
+        modules = %w[
+          nf_thingy
+          nf_bob
+        ]
+        provider.instance_variable_set('@property_hash', modules: modules)
+
+        ['nf_bob'].each do |mod|
+          provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--remove-module', mod], nil)
+        end
+
+        provider.modules = resource[:modules]
+      end
+    end
+
+    it 'sets the ipv4_destination' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--set-destination', "ipv4:#{resource[:ipv4_destination]}"], nil)
+      provider.ipv4_destination = resource[:ipv4_destination]
+    end
+
+    it 'sets the ipv6_destination' do
+      provider.expects(:execute_firewall_cmd).with(['--service', resource[:name], '--set-destination', "ipv6:#{resource[:ipv6_destination]}"], nil)
+      provider.ipv6_destination = resource[:ipv6_destination]
+    end
+  end
+end
diff --git a/spec/unit/puppet/type/firewalld_custom_service_spec.rb b/spec/unit/puppet/type/firewalld_custom_service_spec.rb
new file mode 100644
index 00000000..95c9b4fe
--- /dev/null
+++ b/spec/unit/puppet/type/firewalld_custom_service_spec.rb
@@ -0,0 +1,324 @@
+require 'spec_helper'
+
+describe Puppet::Type.type(:firewalld_custom_service) do
+  before do
+    # rubocop:disable RSpec/InstanceVariable
+    @catalog = Puppet::Resource::Catalog.new
+    described_class.any_instance.stubs(:catalog).returns(@catalog) # rubocop:disable RSpec/AnyInstance
+    Puppet::Provider::Firewalld.any_instance.stubs(:state).returns(:true) # rubocop:disable RSpec/AnyInstance
+    # rubocop:enable RSpec/InstanceVariable
+  end
+
+  context ':name validation' do
+    it 'has :name as its namevar' do
+      expect(described_class.key_attributes).to eq([:name])
+    end
+
+    it 'accepts valid names' do
+      resource = described_class.new(name: 'test_test')
+      expect(resource[:name]).to eq('test_test')
+    end
+  end
+
+  context ':short validation' do
+    it 'accepts a valid short name' do
+      short = 'Short Name'
+
+      resource = described_class.new(
+        name: 'test',
+        short: short
+      )
+
+      expect(resource[:short]).to eq(short)
+    end
+
+    it 'rejects an invalid short name' do
+      short = ''
+
+      expect do
+        described_class.new(
+          name: 'test',
+          short: short
+        )
+      end. to raise_error(%r{Valid values match})
+    end
+  end
+
+  context ':description validation' do
+    it 'accepts a valid description' do
+      description = 'This is a description'
+
+      resource = described_class.new(
+        name: 'test',
+        description: description
+      )
+
+      expect(resource[:description]).to eq(description)
+    end
+
+    it 'rejects an invalid description' do
+      description = ''
+
+      expect do
+        described_class.new(
+          name: 'test',
+          description: description
+        )
+      end. to raise_error(%r{Valid values match})
+    end
+  end
+
+  context ':ports validation' do
+    valid_ports = [
+      '1234-4567/tcp',
+      '1234:4567/tcp',
+      '1234/udp',
+      '1234/sctp',
+      '1234/dccp',
+      'tcp',
+      { 'protocol' => 'tcp' },
+      { 'port' => 1234, 'protocol' => 'tcp' },
+      { 'port' => 1234, 'protocol' => 'udp' },
+      { 'port' => 1234, 'protocol' => 'sctp' },
+      { 'port' => 1234, 'protocol' => 'dccp' },
+      [
+        '1234/tcp',
+        { 'port' => 1234, 'protocol' => 'udp' },
+        { 'port' => '1234', 'protocol' => 'sctp' },
+        { 'port' => 1234, 'protocol' => 'dccp' }
+      ]
+    ]
+
+    invalid_ports = [
+      'bob/tcp',
+      '/tcp',
+      { 'port' => 1234 },
+      [
+        '1234/tcp',
+        { 'port' => 1234, 'protocol' => 'udp' },
+        { 'port' => '1234', 'protocol' => 'sctp' },
+        { 'port' => 'bob', 'protocol' => 'dccp' }
+      ]
+    ]
+
+    out_of_range_ports = [
+      '0-100/tcp',
+      '1-65536/tcp',
+      '0/tcp',
+      '96758/tcp'
+    ]
+
+    invalid_protocols = [
+      '1234/bob',
+      { 'port' => 1234, 'protocol' => 'bob' }
+    ]
+
+    valid_ports.each do |port|
+      it "accepts port '#{port}'" do
+        expect do
+          described_class.new(
+            name: 'test',
+            ports: port
+          )
+        end.not_to raise_error
+      end
+    end
+
+    invalid_ports.each do |port|
+      it "rejects port '#{port}'" do
+        expect do
+          described_class.new(
+            name: 'test',
+            ports: port
+          )
+        end.to raise_error(%r{(Ports must match|must specify a protocol)})
+      end
+    end
+
+    out_of_range_ports.each do |port|
+      it "rejects port '#{port}'" do
+        expect do
+          described_class.new(
+            name: 'test',
+            ports: port
+          )
+        end.to raise_error(%r{Ports must be between})
+      end
+    end
+
+    invalid_protocols.each do |protocol|
+      it "rejects protocol'#{protocol}'" do
+        expect do
+          described_class.new(
+            name: 'test',
+            ports: protocol
+          )
+        end.to raise_error(%r{protocol must be one of})
+      end
+    end
+  end
+
+  context ':protocols validation' do
+    it 'accepts valid protocols' do
+      protocols = ['thing', 'other-thing', 'stuff', '@@bob']
+
+      resource = described_class.new(
+        name: 'test',
+        protocols: protocols
+      )
+
+      expect(resource[:protocols]).to eq(protocols)
+    end
+
+    context 'invalid protocols' do
+      protocols = [
+        '',
+        '#foo',
+        'foo bar'
+      ]
+
+      protocols.each do |protocol|
+        it "rejects #{protocol}" do
+          expect do
+            described_class.new(
+              name: 'test',
+              protocols: protocol
+            )
+          end. to raise_error(%r{Valid values match})
+        end
+      end
+    end
+  end
+
+  context ':modules validation' do
+    it 'accepts valid modules' do
+      modules = ['nf_conntrack_ftp', 'thing', 'other_thing', 'new-thing']
+      expected_modules = ['ftp', 'thing', 'other_thing', 'new-thing']
+
+      resource = described_class.new(
+        name: 'test',
+        modules: modules
+      )
+
+      expect(resource[:modules]).to eq(expected_modules)
+    end
+
+    context 'invalid modules' do
+      modules = [
+        '',
+        '#foo',
+        'foo bar'
+      ]
+
+      modules.each do |mod|
+        it "rejects #{mod}" do
+          expect do
+            described_class.new(
+              name: 'test',
+              modules: mod
+            )
+          end. to raise_error(%r{Valid values match})
+        end
+      end
+    end
+  end
+
+  context ':ipv4_destination validation' do
+    context 'valid destinations' do
+      valid_destinations = [
+        '1.2.3.4/5',
+        '2.3.4.5'
+      ]
+
+      valid_destinations.each do |destination|
+        it "accepts #{destination}" do
+          resource = described_class.new(
+            name: 'test',
+            ipv4_destination: destination
+          )
+
+          expect(resource[:ipv4_destination]).to eq(destination)
+        end
+      end
+    end
+
+    context 'invalid destinations' do
+      invalid_destinations = [
+        '',
+        '1.2.3.4/bob',
+        '::1/alice',
+        'stuff/24',
+        '::1/128'
+      ]
+
+      invalid_destinations.each do |destination|
+        it "rejects #{destination}" do
+          expect do
+            described_class.new(
+              name: 'test',
+              ipv4_destination: destination
+            )
+          end. to raise_error(%r{(Valid values match|invalid address|not an IPv4)})
+        end
+      end
+    end
+  end
+
+  context ':ipv6_destination validation' do
+    context 'valid destinations' do
+      valid_destinations = [
+        '::1/128',
+        '::1'
+      ]
+
+      valid_destinations.each do |destination|
+        it "accepts #{destination}" do
+          resource = described_class.new(
+            name: 'test',
+            ipv6_destination: destination
+          )
+
+          expect(resource[:ipv6_destination]).to eq(destination)
+        end
+      end
+    end
+
+    context 'invalid destinations' do
+      invalid_destinations = [
+        '',
+        '1.2.3.4/bob',
+        '::1/alice',
+        'stuff/24',
+        '1.2.3.4/5',
+        '2.3.4.5'
+      ]
+
+      invalid_destinations.each do |destination|
+        it "rejects #{destination}" do
+          expect do
+            described_class.new(
+              name: 'test',
+              ipv6_destination: destination
+            )
+          end. to raise_error(%r{(Valid values match|invalid address|not an IPv6)})
+        end
+      end
+    end
+  end
+
+  context 'autorequires' do
+    # rubocop:disable RSpec/InstanceVariable
+    before do
+      firewalld_service = Puppet::Type.type(:service).new(name: 'firewalld')
+      @catalog.add_resource(firewalld_service)
+    end
+
+    it 'autorequires the firewalld service' do
+      resource = described_class.new(name: 'test')
+      @catalog.add_resource(resource)
+
+      expect(resource.autorequire.map { |rp| rp.source.to_s }).to include('Service[firewalld]')
+    end
+    # rubocop:enable RSpec/InstanceVariable
+  end
+end
diff --git a/templates/service.xml.epp b/templates/service.xml.epp
deleted file mode 100644
index 15ba249c..00000000
--- a/templates/service.xml.epp
+++ /dev/null
@@ -1,40 +0,0 @@
-<%- |
-  String                  $short,
-  Optional[String]        $description,
-  Optional[Array[Hash]]   $port,
-  Optional[Array[String]] $module,
-  Optional[Hash]          $destination,
-  String                  $filename     = $short,
-  Stdlib::Unixpath        $config_dir,
-  String                  $ensure
-| -%>
-<?xml version="1.0" encoding="utf-8"?>
-<service>
-    <short><%= $short %></short>
-    <%- if $description { -%>
-    <description><%= $description %></description>
-    <%- } -%>
-
-<%
-  if $port {
-    $port.each |$i| {
-      if $i['port'] and $i['port'] != '' {
--%>
-    <port<% if $i['protocol'] { %> protocol="<%= $i['protocol'] %>"<% } %> port="<%= regsubst("${i['port']}", ':', '-', 'G') %>" />
-<%    } else { -%>
-    <port protocol="<%= $i['protocol'] %>" port="" />
-<%    } -%>
-<%  } -%>
-
-<% } -%>
-<% if $module {
-     sort($module).each |$j| {
--%>
-    <module name="<%= $j %>" />
-<%   } -%>
-
-<% } -%>
-<% if $destination { -%>
-    <destination<% if $destination['ipv4'] { %> ipv4="<%= $destination['ipv4'] %>"<% } %><% if $destination['ipv6'] { %> ipv6="<%= $destination['ipv6'] %>"<% } %> />
-<% } -%>
-</service>