Workaround for duplicate resource http_port #120

voxpupuli / puppet-squid

Puppet module for configuration of squid caching proxy.

https://forge.puppet.com/puppet/squid

Other

12 stars 54 forks source link

Workaround for duplicate resource http_port #120 #152

Closed granquet closed 4 years ago

granquet commented 4 years ago

This is a rebase ontop of master for: https://github.com/voxpupuli/puppet-squid/pull/135 The review comment from @ekohl has been adressed in the process.

The module is now able to handle multiple server declarations for the same port on different IPs.

granquet commented 4 years ago

I've added a minor correction (unrelated to the topic of the PR though) in the way the check is made for selinux. on the two systems I've tried:

CentOS Linux release 8.1.1911 (Core) with facter 3.14.4 $facts['os']['selinux'] is a hash and thus testing for true doesn't seem right (and doesn't work)
Scibian 9 ( Debian 9.13 ) with facter 2.4.6 the selinux key in $facts['os'] doesn't exists.and testing for $facts['selinux'] seems frowned upon

my best bet seems to be: has_key($facts['os'],'selinux') and $facts['os']['selinux']['enabled'] == true It makes rake/travis happy, works on my old debian (without selinux) and on CentOS (with selinux).

I'm really new to ruby and puppet in general... so, not sure of what I'm doing :) please advise?

bastelfreak commented 4 years ago

@ekohl do we really want to support facter 2? This isn't really maintained by Puppet since a long time and we dropped that in many many modules. Also I think this makes the code harder to read. @granquet is there a reason for not using the Puppetlabs repositories for modern puppet/facter? We don't officially support Puppet 4 / facter 2 anymore.

ekohl commented 4 years ago

I think if fact('os.selinux.enabled') {} is not harder to read than if $fact['os']['selinux']['enabled'] {}. There may be cases where it's less clear, but in this case there is no fallback to support facter 2. IMHO it's fine to keep it that way if it makes a user happy.

bastelfreak commented 4 years ago

okay, fine by me.

granquet commented 4 years ago

I would say that we have installed the machines with puppet 4/facter 2 and we will stick with it until the end of life of the machines. the next machine we are installing are running puppet 6/facter 3.

let me drop that PR, as @ralfbosz updated the original one I was intending to get merged with this one. https://github.com/voxpupuli/puppet-squid/pull/135