There is no service_provider default when used on Puppet 7

jcbollinger commented 1 year ago

Describe the Bug

When the module is used with Puppet 7 clients, catalog compilation fails unless parameter postgresql::globals::service_provideris explicitly set in Hiera data or a resource-like class declaration.

This is a regression: the module used to choose a suitable provider without help.

Expected Behavior

The module should choose the target system's default service provider automatically. The manifest set and site data should not need to specify one explicitly unless something different from the default is required.

Steps to Reproduce

Steps to reproduce the behavior:

Install puppetlabs-postgresql 9.2.0
Attempt to compile a catalog containing this declaration:
```
include postgresql::server
```
with facts from a Puppet 7 client.

Catalog compilation then fails with an error such as this spec test output shows:

  1) demo on redhat-8-x86_64 is expected to compile into a catalogue without dependency cycles
     Failure/Error: it { is_expected.to compile.with_all_deps }
       error during compilation: Evaluation Error: Error while evaluating a Function Call, pick(): must receive at least one non empty value (file: /.../environments/production/modules/demo/spec/fixtures/modules/postgresql/manifests/params.pp, line: 88, column: 10) on node x.y.z
     # ./spec/classes/demo_spec.rb:10:in `block (4 levels) in <top (required)>'

Environment

Puppet 7.26.0
puppetlabs-posgresql 9.2.0
RHEL 7 server, RHEL 7-8, CentOS 7-8, Rocky 8-9 clients

Additional Context

The module seems to be trying to use $facts['service_provider']to identify the default provider, but this fact is not served by the version of facter bundled with Puppet 7.

The issue can be worked around by providing a suitable value for class parameter postgresql::globals::service_provider.

smortex commented 1 year ago

The service_provider fact is provided by stdlib since version 4.10.0: https://github.com/puppetlabs/puppetlabs-stdlib/pull/506 (released in 2015).

The module currently require version 9.0.0 or newer of stdlib: https://github.com/puppetlabs/puppetlabs-postgresql/blob/476734001c9aade2c9731d32ce4446d7e21142bf/metadata.json#L13

Please check your module dependencies:

root@puppet ~ # puppet module list

jcbollinger commented 1 year ago

The service_provider fact is provided by stdlib since version 4.10.0: puppetlabs/puppetlabs-stdlib#506 (released in 2015).

The module currently require version 9.0.0 or newer of stdlib:

https://github.com/puppetlabs/puppetlabs-postgresql/blob/476734001c9aade2c9731d32ce4446d7e21142bf/metadata.json#L13

Please check your module dependencies:
root@puppet ~ # puppet module list

Ok, I'm seeing that fact if I run facter --puppet on a client node, so I seem to have mischaracterized the nature of the issue. Nevertheless, I do see the error quoted in the above bug report in some of my unit tests, and I can confirm that the affected modules are using new enough stdlib (9.3.0 in one example) in their test fixtures. Thus, alternative steps to reproduce:

Create a new module with PDK:
```
pdk new module demo
```
In the new module, create a class
```
pdk new class demo
```
and in it put the declaration presented above:
```
class demo {
  include postgresql::server
}
```

Configure the test fixture with puppetlabs-postgresql and its dependencies: .fixtures.yml

---
fixtures:
  forge_modules:
     apt:        "puppetlabs/apt"
     concat:     "puppetlabs/concat"
     postgresql: "puppetlabs/postgresql"
     stdlib:     "puppetlabs/stdlib"

Run the test suite:
```
pdk test unit
```

Again, this is a regression. My unit tests were working before upgrading puppetlabs-postgresql. The workaround described above was effective at getting them working again, but that's not a satisfactory resolution.

ekohl commented 1 year ago

You need to use a custom fact in your test environment. In Vox Pupuli we have a helper that automatically adds it: https://github.com/voxpupuli/voxpupuli-test/blob/8014068e5fb0c84ba61dd9abc1bf4bcc79db1470/lib/voxpupuli/test/facts.rb#L71-L96

The PDK maintainers could implement a similar workaround, but it's a problem that all modules relying on custom facts will face.

https://github.com/puppetlabs/puppetlabs-postgresql/pull/1518 will lessen the impact, but the root cause is still there.

Closing this as it can't be solved without dropping the use of the custom fact.

puppetlabs / puppetlabs-postgresql