From e13e6c1ae0e1848051892d0306030a528b01524a Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Fri, 16 Jul 2021 19:31:42 +0200 Subject: delete legacy `policy` param in ferm::rule --- README.md | 4 ++-- REFERENCE.md | 22 ++++------------------ manifests/rule.pp | 32 ++++++++------------------------ spec/defines/rule_spec.rb | 36 ++---------------------------------- 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` ##### `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`` - -##### `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`` - ##### `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 -- cgit v1.2.3