Include firewalld only if manage_firewall is true

[uvNikita]

We are using puppetlabs/firewall module to setup firewall on our hosts. The problem is that it conflicts with crayfishx/firewalld module which slurm module includes independently of manage_firewall parameter value. So this changes should fix the case when users want to setup firewall using another module.

[Falkor]

Indeed, thanks for the additional check. Even if I'm surprised you got conflicts: we also use puppetlabs/firewall and crayfishx/firewalld within our control repo without any issue. Actually we use a wrapper definition tools::firewall::open_port to rely on both module depending on the context, here is the code:

define tools::firewall::open_port(
  Enum[
    'present',
    'absent'
  ]       $ensure   = 'present',
  String  $zone     = 'public',
  String  $protocol = 'tcp',
  String  $seltype  = '',
  Optional[String]                        $source     = undef,
  Optional[Integer[1,65535]]              $port       = undef,
  Optional[Tuple[Integer[1,65535], 2, 2]] $port_range = undef
)
{
  # $name is provided by define invocation
  $label = $name

  if ($port == undef and $port_range == undef) {
    fail("You must define either 'port' or 'port_range'")
  }
  if ($port != undef and $port_range != undef) {
    fail("You can't define both 'port' and 'port_range'")
  }
  if ($port_range != undef) {
    if ($port_range[0] > $port_range[1]) {
      fail("Malformed port range: ${port_range}")
    }
  }

  $range = $port_range ? {
    undef   => [$port, $port],
    default => $port_range,
  }

  if ($port != undef and $range[0] == $range[1]) {
    $firewall_port = $port
  } else {
    $firewall_port = "${range[0]}-${range[1]}"
  }

  if (defined(Class['firewalld'])) {
    ###### CentOS/RHEL 7 with firewalld
    firewalld_port { $label:
      ensure   => $ensure,
      zone     => $zone,
      port     => $firewall_port,
      protocol => $protocol,
    }
    if ($source != undef) {
      Firewalld_port[$label] {
        source  => $source
      }
    }

    if (defined(Class['selinux']) and $facts['os']['selinux']['enabled'] and !empty($seltype)) {
      selinux::port { "allow-${seltype}-port-${range[0]}-${range[1]}":
        ensure     => $ensure,
        seltype    => $seltype,
        protocol   => $protocol,
        port_range => [ $range[0], $range[1] ],
      }
    }
  }
  else
  {
    if ($source != undef) {
      $source_label = " from ${source}"
    } else {
      $source_label = ''
    }

    firewall { "${range[0]}-${range[1]} -- accept port ${protocol}/${range[0]}-${range[1]}${source_label}":
      dport  => $firewall_port,
      proto  => $protocol,
      action => 'accept',
    }

    if ($source != undef) {
      Firewall["${range[0]}-${range[1]} -- accept port ${protocol}/${range[0]}-${range[1]}${source_label}"] {
        source => $source,
      }
    }

  }
}

[uvNikita]

Thank you for the quick reply and merge! In particular, we are getting Duplicate declaration: Service[firewalld] is already declared on these lines: puppetlabs/firewall:manifests/linux/redhat.pp#L30 and crayfishx/puppet-firewalld:manifests/init.pp#L79.

[Falkor]

Hum, OK, I haven't pay attention about that within puppetlabs/firewall. Thanks for reporting it. At least with your fix you can use both ;) Thanks in all cases for checking our slurm module -- do not hesitate to report us any deviant behaviour.