voxpupuli / puppet-firewalld

Puppet module for managing firewalld
Apache License 2.0
40 stars 77 forks source link

port being converted to string in firewalld_custom_service #283

Closed traylenator closed 4 years ago

traylenator commented 4 years ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

firewalld::custom_service { 'hw_exporter':
  short       => 'hw_exporter',
  description => 'HW information exporter',
  port        => [
    {
      'port'     => 9169,
      'protocol' => 'tcp',
    },
  ],
}

What are you seeing

Failure/Error: it { is_expected.to compile.with_all_deps }
  error during compilation: Parameter ports failed on Firewalld_custom_service[hw_exporter]: Ports must be between 1-65535 instead of '9169' (file: /builds/ai/it-puppet-module-base/code/spec/fixtures/modules/firewalld/manifests/custom_service.pp, line: 67)
./spec/classes/init_spec.rb:290:in `block (4 levels) in <top (required)>'

What behaviour did you expect instead

Used to compile.

Any additional information you'd like to impart

Running a bisect on the firewalld fixture the changes was introduced with:

https://github.com/voxpupuli/puppet-firewalld/commit/a1993badda1663fc859c0ca4a2ec7c049aa76af3

alexjfisher commented 4 years ago

https://github.com/voxpupuli/puppet-firewalld/blob/5df4c2e912b9ddd96f17fbd90b64bef222efc3bd/lib/puppet/type/firewalld_custom_service.rb#L88 looks wrong. That string comparison isn't valid.

2.5.5 :001 > '999' > '1000'
 => true
trevor-vaughan commented 4 years ago

That's definitely a fix that needs to be made. Not sure if it fixes the stated issue though. Checking

trevor-vaughan commented 4 years ago

Ah, yep, this is the correct fix. Sorry for letting that slip through!

@alexjfisher Made a suggestion for a small code change and rubocop is whining.

traylenator commented 4 years ago

Thanks @alexjfisher @trevor-vaughan all good