pg_hba_rule does not properly verify address parameter

[tuxmea]

According to PostgreSQL documentation there are only specific data possible for address:
https://www.postgresql.org/docs/current/auth-pg-hba-conf.html

• IPV4 CIDR
• IPV6 CIDR
• FQDN
• the strings 'samenet' or 'samehost'
• a domain
• a domain with a starting dot

fixes #1373

[puppet-community-rangefinder]

postgresql::server::pg_hba_rule is a type

Breaking changes to this file WILL impact these 14 modules (exact match):

• puppetlabs-puppetdb
• wdijkerman-zabbix
• puppet-zabbix
• it2ndq-barman
• deric-barman
• nnutter-testdb
• chedi-django
• landcareresearch-postgis
• SchnWalter-happydev
• lwo-dataverse
• conzar-pidservice
• katello-gutterball
• landcareresearch-pidservice
• deric-pgprobackup

Breaking changes to this file MAY impact these 4 modules (near match):

• icinga-icinga
• tedivm-hieratic
• Aethylred-puppetdashboard
• fatdragon-pg_streaming_replication

This module is declared in 70 of 579 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[smortex]

According to the link in the docstring:

You can also write all to match any IP address, samehost to match any of the server's own IP addresses, or samenet to match any address in any subnet that the server is directly connected to.

[GSPatton]

hey @tuxmea. The spec tests with puppet 6 and 7 have both failed. Address these failures and we can have a look! Cheers

[tuxmea]

@GSPatton I have issues identifying the error:
Both tests show the following:

Failures:

  1) postgresql::server::default_privileges valid object_type schemas on postgres < 10.0 
     Got 0 failures and 2 other errors:

     1.1) Failure/Error: result_facts.merge!(munge_facts(facts)) if self.respond_to?(:facts)

          TypeError:
            no implicit conversion of nil into Hash
          # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `load'
          # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `<top (required)>'
          # /opt/hostedtoolcache/Ruby/2.7.6/x64/bin/bundle:23:in `load'
          # /opt/hostedtoolcache/Ruby/2.7.6/x64/bin/bundle:23:in `<main>'

     1.2) Failure/Error: @logs.clear

          NoMethodError:
            undefined method `clear' for nil:NilClass
          # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `load'
          # ./vendor/bundle/ruby/2.7.0/bin/rspec:23:in `<top (required)>'
          # /opt/hostedtoolcache/Ruby/2.7.6/x64/bin/bundle:23:in `load'
          # /opt/hostedtoolcache/Ruby/2.7.6/x64/bin/bundle:23:in `<main>'

Finished in 7 minutes 24 seconds (files took 1.62 seconds to load)
228 examples, 1 failure

Failed examples:

rspec ./spec/defines/server/default_privileges_spec.rb:132 # postgresql::server::default_privileges valid object_type schemas on postgres < 10.0 

[smortex]

The last PR merged in the main branch had similar CI failures, so these failures seems unrelated to this change.

The change itself looks good to me 👍

[tuxmea]

@david22swan Can you please explain why this is backwards-incompatible?
When specifying an invalid entry, the postgresql service will not start.

[david22swan]

Anything that alters what a parameter accepts in such a way as to limit it is technically backwards incompatible.
Since you are limiting it to the correct input only and anything that doesn't match it would have caused errors anyway it is debatable you can probably get it change to bugfix.
Just of the opinion that it needs discussed over first.

[ekohl]

Anything that alters what a parameter accepts in such a way as to limit it is technically backwards incompatible.

I think I agree with @tuxmea here. Where previously it would be invalid and fail at runtime (starting the service would fail), now it will fail at catalog compilation. I think it could be an enhancement instead of backwards incompatible change.

[bastelfreak]

I agree with @ekohl and @tuxmea here. I think this is a good enhancement but not a breaking change.

[david22swan]

Not that I'm caving to peer pressure, but yeh. Your explanation sounds good so gonna go with feature.
Like the type change to, just need the rubocop errors fixed and I'd be happy to merge.

[tuxmea]

@david22swan lint fixes have been added.

[david22swan]

LGTM

Gonna do a squash merge since there's a dozen commits, but otherwise this is all good. Thanks for getting back on it so fast.