Merge pull request #134 from bastelfreak/old

delete legacy `policy` param in ferm::rule

4 files changed, 16 insertions, 78 deletions

diff --git a/README.md b/README.md
index 262fe9d..20179fe 100644
--- a/README.md
+++ b/README.md
@@ -60,7 +60,7 @@ You can easily define rules in Puppet (they don't need to be exported resources)
 ```puppet
   @@ferm::rule{"allow_kafka_server2server-${trusted['certname']}":
     chain  => 'INPUT',
-    policy => 'ACCEPT',
+    action => 'ACCEPT',
     proto  => 'tcp',
     dport  => [9092, 9093],
     saddr  => "(${facts['networking']['ip6']}/128 ${facts['networking']['ip']}/32)",
@@ -95,7 +95,7 @@ subnets:
 ferm::rules:
   'allow_http_https':
     chain: 'INPUT'
-    policy: 'ACCEPT'
+    action: 'ACCEPT'
     proto: 'tcp'
     dport:
       - 80
diff --git a/REFERENCE.md b/REFERENCE.md
index bc2fe1d..6db3b49 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -487,7 +487,7 @@ ferm::rule{'allow-ssh-localhost':
 ```puppet
 ferm::rule{'drop-icmp-time-exceeded':
   chain         => 'OUTPUT',
-  policy        => 'DROP',
+  action        => 'DROP',
   proto         => 'icmp',
   proto_options => 'icmp-type time-exceeded',
 }
@@ -498,7 +498,7 @@ ferm::rule{'drop-icmp-time-exceeded':
 ```puppet
 ferm::rule{'allow_consul':
   chain  => 'INPUT',
-  policy => 'ACCEPT',
+  action => 'ACCEPT',
   proto  => ['udp', 'tcp'],
   dport  => 8301,
 }
@@ -512,7 +512,6 @@ The following parameters are available in the `ferm::rule` defined type:
 * [`proto`](#proto)
 * [`comment`](#comment)
 * [`action`](#action)
-* [`policy`](#policy)
 * [`dport`](#dport)
 * [`sport`](#sport)
 * [`saddr`](#saddr)
@@ -544,24 +543,11 @@ Default value: `$name`
 
 ##### <a name="action"></a>`action`
 
-Data type: `Optional[Ferm::Actions]`
-
-Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name)
-Default value: undef
-Allowed values: (RETURN|ACCEPT|DROP|REJECT|NOTRACK|LOG|MARK|DNAT|SNAT|MASQUERADE|REDIRECT|String[1])
-
-Default value: ``undef``
-
-##### <a name="policy"></a>`policy`
-
-Data type: `Optional[Ferm::Policies]`
+Data type: `Ferm::Actions`
 
-Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name) [DEPRECATED]
-Default value: undef
+Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name). The parameter is mandatory.
 Allowed values: (RETURN|ACCEPT|DROP|REJECT|NOTRACK|LOG|MARK|DNAT|SNAT|MASQUERADE|REDIRECT|String[1])
 
-Default value: ``undef``
-
 ##### <a name="dport"></a>`dport`
 
 Data type: `Optional[Ferm::Port]`
diff --git a/manifests/rule.pp b/manifests/rule.pp
index 49d5292..c069568 100644
--- a/manifests/rule.pp
+++ b/manifests/rule.pp
@@ -21,7 +21,7 @@
 # @example Confuse people that do a traceroute/mtr/ping to your system
 #   ferm::rule{'drop-icmp-time-exceeded':
 #     chain         => 'OUTPUT',
-#     policy        => 'DROP',
+#     action        => 'DROP',
 #     proto         => 'icmp',
 #     proto_options => 'icmp-type time-exceeded',
 #   }
@@ -29,7 +29,7 @@
 # @example allow multiple protocols
 #   ferm::rule{'allow_consul':
 #     chain  => 'INPUT',
-#     policy => 'ACCEPT',
+#     action => 'ACCEPT',
 #     proto  => ['udp', 'tcp'],
 #     dport  => 8301,
 #   }
@@ -37,11 +37,7 @@
 # @param chain Configure the chain where we want to add the rule
 # @param proto Which protocol do we want to match, typically UDP or TCP
 # @param comment A comment that will be added to the ferm config and to ip{,6}tables
-# @param action Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name)
-#   Default value: undef
-#   Allowed values: (RETURN|ACCEPT|DROP|REJECT|NOTRACK|LOG|MARK|DNAT|SNAT|MASQUERADE|REDIRECT|String[1])
-# @param policy Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name) [DEPRECATED]
-#   Default value: undef
+# @param action Configure what we want to do with the packet (drop/accept/reject, can also be a target chain name). The parameter is mandatory.
 #   Allowed values: (RETURN|ACCEPT|DROP|REJECT|NOTRACK|LOG|MARK|DNAT|SNAT|MASQUERADE|REDIRECT|String[1])
 # @param dport The destination port, can be a single port number as integer or an Array of integers (which will then use the multiport matcher)
 # @param sport The source port, can be a single port number as integer or an Array of integers (which will then use the multiport matcher)
@@ -56,9 +52,8 @@
 define ferm::rule (
   String[1] $chain,
   Ferm::Protocols $proto,
+  Ferm::Actions $action,
   String $comment = $name,
-  Optional[Ferm::Actions] $action = undef,
-  Optional[Ferm::Policies] $policy = undef,
   Optional[Ferm::Port] $dport = undef,
   Optional[Ferm::Port] $sport = undef,
   Optional[Variant[Array, String[1]]] $saddr = undef,
@@ -68,24 +63,13 @@ define ferm::rule (
   Enum['absent','present'] $ensure = 'present',
   Ferm::Tables $table = 'filter',
 ) {
-  if $policy and $action {
-    fail('Cannot specify both policy and action. Do not provide policy when using the new action param.')
-  } elsif $policy and ! $action {
-    warning('The param "policy" is deprecated (superseded by "action") and will be dropped in a future release.')
-    $action_temp = $policy
-  } elsif $action and ! $policy {
-    $action_temp = $action
-  } else {
-    fail('Exactly one of "action" or the deprecated "policy" param is required.')
-  }
-
-  if $action_temp in ['RETURN', 'ACCEPT', 'DROP', 'REJECT', 'NOTRACK', 'LOG', 'MARK', 'DNAT', 'SNAT', 'MASQUERADE', 'REDIRECT'] {
-    $action_real = $action_temp
+  if $action in ['RETURN', 'ACCEPT', 'DROP', 'REJECT', 'NOTRACK', 'LOG', 'MARK', 'DNAT', 'SNAT', 'MASQUERADE', 'REDIRECT'] {
+    $action_real = $action
   } else {
     # assume the action contains a target chain, so prefix it with the "jump" statement
-    $action_real = "jump ${action_temp}"
+    $action_real = "jump ${action}"
     # make sure the target chain is created before we try to add rules to it
-    Ferm::Chain <| chain == $action_temp and table == $table |> -> Ferm::Rule[$name]
+    Ferm::Chain <| chain == $action and table == $table |> -> Ferm::Rule[$name]
   }
 
   $proto_real = $proto ? {
diff --git a/spec/defines/rule_spec.rb b/spec/defines/rule_spec.rb
index f2601c6..7529fce 100644
--- a/spec/defines/rule_spec.rb
+++ b/spec/defines/rule_spec.rb
@@ -11,7 +11,7 @@ describe 'ferm::rule', type: :define do
         'include ferm'
       end
 
-      context 'without policy or action' do
+      context 'without action' do
         let(:title) { 'filter-ssh' }
         let :params do
           {
@@ -22,39 +22,7 @@ describe 'ferm::rule', type: :define do
           }
         end
 
-        it { is_expected.to compile.and_raise_error(%r{Exactly one of "action" or the deprecated "policy" param is required}) }
-      end
-
-      context 'with both policy and action' do
-        let(:title) { 'filter-ssh' }
-        let :params do
-          {
-            chain: 'INPUT',
-            policy: 'ACCEPT',
-            action: 'ACCEPT',
-            proto: 'tcp',
-            dport: 22,
-            saddr: '127.0.0.1'
-          }
-        end
-
-        it { is_expected.to compile.and_raise_error(%r{Cannot specify both policy and action}) }
-      end
-
-      context 'without a specific interface using legacy policy param' do
-        let(:title) { 'filter-ssh' }
-        let :params do
-          {
-            chain: 'INPUT',
-            policy: 'ACCEPT',
-            proto: 'tcp',
-            dport: 22,
-            saddr: '127.0.0.1'
-          }
-        end
-
-        it { is_expected.to compile.with_all_deps }
-        it { is_expected.to contain_concat__fragment('INPUT-filter-ssh').with_content("mod comment comment 'filter-ssh' proto tcp dport 22 saddr @ipfilter((127.0.0.1)) ACCEPT;\n") }
+        it { is_expected.not_to compile }
       end
 
       context 'without a specific interface' do