Trying to add custom service with protocols and not ports, results in an error on first run

[nmaludy]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 7
• Ruby: 2.7
• Distribution: CentOS 8
• Module version: 4.4.0

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

  firewalld_custom_service { 'ospf':
    ensure    => present,
    protocols => ['ospf'],
  }
  firewalld_service { 'ospf':
    ensure  => present,
    zone    => public,
  }

What are you seeing

err - Puppet - Could not set 'present' on ensure: undefined method `delete' for nil:NilClass (file: /home/nmaludy/storage/home/src/keyspoke/puppet-keyspoke/site-modules/role/manifests/frr.pp, line: 14)
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld.rb:59:in `block in execute_firewall_cmd'
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld.rb:59:in `map'
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld.rb:59:in `execute_firewall_cmd'
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld.rb:77:in `execute_firewall_cmd'
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:130:in `block in protocols='
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:128:in `each'
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:128:in `protocols='
/tmp/d20210407-5925-18t7q0k/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:33:in `create'

I added some debug output to show what is going wrong

debug - Puppet::Type::Firewalld_custom_service::ProviderFirewall_cmd - Executing --state command - current value 
debug - Puppet - args = ["--state"]
debug - Puppet - args.flatten = ["--state"]
debug - Puppet - Executing: '/usr/bin/firewall-cmd --state'
debug - Puppet - args = ["--get-services"]
debug - Puppet - args.flatten = ["--get-services"]
debug - Puppet - Executing: '/usr/bin/firewall-cmd --permanent --get-services'
debug - Firewalld_custom_service[ospf](provider=firewall_cmd) - Adding new custom service to firewalld: ospf
debug - Puppet - args = ["--new-service", "ospf"]
debug - Puppet - args.flatten = ["--new-service", "ospf"]
debug - Puppet - Executing: '/usr/bin/firewall-cmd --permanent --new-service ospf'
debug - Puppet - args = ["--service", "ospf", "--get-protocols"]
debug - Puppet - args.flatten = ["--service", "ospf", "--get-protocols"]
debug - Puppet - Executing: '/usr/bin/firewall-cmd --permanent --service ospf --get-protocols'
debug - Puppet - args = ["--service", "ospf", "--add-protocol", "ospf"]
debug - Puppet - args.flatten = ["--service", "ospf", "--add-protocol", "ospf"]
debug - Puppet - Executing: '/usr/bin/firewall-cmd --permanent --service ospf --add-protocol ospf'
debug - Puppet - args = ["--service", "ospf", "--add-protocol", nil]
debug - Puppet - args.flatten = ["--service", "ospf", "--add-protocol", nil]
err - Puppet - Could not set 'present' on ensure: undefined method `delete' for nil:NilClass (file: /home/nmaludy/storage/home/src/keyspoke/puppet-keyspoke/site-modules/role/manifests/frr.pp, line: 14)
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld.rb:61:in `block in execute_firewall_cmd'
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld.rb:61:in `map'
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld.rb:61:in `execute_firewall_cmd'
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld.rb:79:in `execute_firewall_cmd'
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:130:in `block in protocols='
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:128:in `each'
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:128:in `protocols='
/tmp/d20210407-6964-16lpsr0/modules/firewalld/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb:33:in `create'

What behaviour did you expect instead

I expected the custom service to be created successfully

Any additional information you'd like to impart

It looks like the extra nil value in the array is coming from this line here: https://github.com/voxpupuli/puppet-firewalld/blob/master/lib/puppet/provider/firewalld_custom_service/firewall_cmd.rb#L123

Specifically because i'm not passing in ports so that ends up creating [nil] and gets concatenated to the should value.

Instead we should check if ports is :unset and ignore it.