Skip to content

Commit

Permalink
Revert icmp_block_inversion as tests don't pass
Browse files Browse the repository at this point in the history
  • Loading branch information
jcpunk committed Sep 18, 2023
1 parent fa1f9c0 commit a27c8c7
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 48 deletions.
11 changes: 2 additions & 9 deletions REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,7 @@ not work, and will generate an error. This is a limitation of firewalld itself,

#### Examples

##### Create a zone called `restricted` allowing only `echo-request` icmp types
##### Create a zone called `restricted`

```puppet
firewalld_zone { 'restricted':
Expand All @@ -1209,8 +1209,7 @@ firewalld_zone { 'restricted':
purge_rich_rules => true,
purge_services => true,
purge_ports => true,
icmp_blocks => 'echo-request'
icmp_block_inversion => true,
icmp_blocks => 'router-advertisement'
}
```

Expand All @@ -1226,12 +1225,6 @@ The basic property that the resource should be in.

Default value: `present`

##### `icmp_block_inversion`

Valid values: `true`, `false`

Can be set to true or false, specifies whether to set icmp_block_inversion from the zone

##### `icmp_blocks`

Specify the icmp-blocks for the zone. Can be a single string specifying one icmp type,
Expand Down
20 changes: 0 additions & 20 deletions lib/puppet/provider/firewalld_zone/firewall_cmd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def create
self.sources = (@resource[:sources]) if @resource[:sources]
self.interfaces = @resource[:interfaces]
self.icmp_blocks = (@resource[:icmp_blocks]) if @resource[:icmp_blocks]
self.icmp_block_inversion = (@resource[:icmp_block_inversion]) if @resource[:icmp_block_inversion]
self.description = (@resource[:description]) if @resource[:description]
self.short = (@resource[:short]) if @resource[:short]
end
Expand Down Expand Up @@ -156,25 +155,6 @@ def icmp_blocks=(new_icmp_blocks)
end
end

def icmp_block_inversion
if execute_firewall_cmd(['--query-icmp-block-inversion'], @resource[:name]).chomp == 'no'
:false
else
:true
end
end

def icmp_block_inversion=(bool)
case bool
when :true
debug("adding icmp block inversion for zone #{@resource[:name]}")
execute_firewall_cmd(['--add-icmp-block-inversion'], @resource[:name])
when :false
debug("removing icmp block inversion for zone #{@resource[:name]}")
execute_firewall_cmd(['--remove-icmp-block-inversion'], @resource[:name])
end
end

# rubocop:disable Style/AccessorMethodName
def get_rules
perm = execute_firewall_cmd(['--list-rich-rules']).split(%r{\n})
Expand Down
11 changes: 2 additions & 9 deletions lib/puppet/type/firewalld_zone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
Note that setting `ensure => 'absent'` to the built in firewalld zones will
not work, and will generate an error. This is a limitation of firewalld itself, not the module.
@example Create a zone called `restricted` allowing only `echo-request` icmp types
@example Create a zone called `restricted`
firewalld_zone { 'restricted':
ensure => present,
target => '%%REJECT%%',
Expand All @@ -28,8 +28,7 @@
purge_rich_rules => true,
purge_services => true,
purge_ports => true,
icmp_blocks => 'echo-request'
icmp_block_inversion => true,
icmp_blocks => 'router-advertisement'
}
DOC

Expand Down Expand Up @@ -126,12 +125,6 @@ def insync?(is)
end
end

newproperty(:icmp_block_inversion) do
desc 'Can be set to true or false, specifies whether to set icmp_block_inversion from the zone'
newvalue(:true)
newvalue(:false)
end

newproperty(:purge_rich_rules) do
desc "When set to true any rich_rules associated with this zone
that are not managed by Puppet will be removed.
Expand Down
2 changes: 0 additions & 2 deletions spec/unit/puppet/provider/firewalld_zone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
resource.expects(:[]).with(:sources).returns(nil).at_least_once
resource.expects(:[]).with(:interfaces).returns(['eth0']).at_least_once
resource.expects(:[]).with(:icmp_blocks).returns(nil).at_least_once
resource.expects(:[]).with(:icmp_block_inversion).returns(false).at_least_once
resource.expects(:[]).with(:description).returns(nil).at_least_once
resource.expects(:[]).with(:short).returns('little description').at_least_once
provider.expects(:execute_firewall_cmd).with(['--list-interfaces'])
Expand All @@ -51,7 +50,6 @@
resource.expects(:[]).with(:sources).returns(nil).at_least_once
resource.expects(:[]).with(:interfaces).returns(['eth0']).at_least_once
resource.expects(:[]).with(:icmp_blocks).returns(nil).at_least_once
resource.expects(:[]).with(:icmp_block_inversion).returns(false).at_least_once
resource.expects(:[]).with(:description).returns(nil).at_least_once
resource.expects(:[]).with(:short).returns('little description').at_least_once
provider.expects(:execute_firewall_cmd).with(['--list-interfaces'])
Expand Down
9 changes: 1 addition & 8 deletions spec/unit/puppet/type/firewalld_zone_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
end
end

%i[target icmp_blocks icmp_block_inversion sources purge_rich_rules purge_services purge_ports].each do |param|
%i[target icmp_blocks sources purge_rich_rules purge_services purge_ports].each do |param|
it "has a #{param} parameter" do
expect(described_class.attrtype(param)).to eq(:property)
end
Expand All @@ -37,7 +37,6 @@
target: '%%REJECT%%',
interfaces: ['eth0'],
icmp_blocks: %w[redirect router-advertisment],
icmp_block_inversion: true,
sources: ['192.168.2.2', '10.72.1.100']
)
end
Expand Down Expand Up @@ -70,7 +69,6 @@
provider.expects(:execute_firewall_cmd).with(['--set-target', '%%REJECT%%'])

provider.expects(:icmp_blocks=).with(%w[redirect router-advertisment])
provider.expects(:icmp_block_inversion=).with(:true)

provider.expects(:sources).returns([])
provider.expects(:execute_firewall_cmd).with(['--add-source', '192.168.2.2'])
Expand Down Expand Up @@ -120,11 +118,6 @@
provider.sources = ['valy']
end

it 'sets icmp_block_inversion' do
provider.expects(:execute_firewall_cmd).with(['--query-icmp-block-inversion'], 'restricted').returns('no')
provider.expects(:execute_firewall_cmd).with(['--add-icmp-block-inversion'], 'restricted')
end

it 'gets icmp_blocks' do
# shouldn't this list the zone?
provider.expects(:execute_firewall_cmd).with(['--list-icmp-blocks']).returns('val')
Expand Down

0 comments on commit a27c8c7

Please sign in to comment.