From 6cb50cb61ffb1cd1b74319640db4eb6fe9876621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 11 Oct 2023 09:01:38 -1000 Subject: [PATCH 1/2] Fix `postgresql::default()` return value for unknown parameters When calling `postgresql::default()` with a name that has no corresponding parameter, the function returns the string "postgresql::globals::" which is probably not intended given the comment above the code. The usage of pick seems to indicate that an exception should be raised when a parameter is not found in `params` nor `globals.pp`, so adjust the code accordingly. --- functions/default.pp | 3 +- spec/functions/postgresql_default_spec.rb | 34 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 spec/functions/postgresql_default_spec.rb diff --git a/functions/default.pp b/functions/default.pp index 7852530791..e7b11f3b34 100644 --- a/functions/default.pp +++ b/functions/default.pp @@ -10,6 +10,5 @@ function postgresql::default( #search for the variable name in params first #then fall back to globals if not found - pick( getvar("postgresql::params::${parameter_name}"), - "postgresql::globals::${parameter_name}") + pick(getvar("postgresql::params::${parameter_name}"), getvar("postgresql::globals::${parameter_name}")) } diff --git a/spec/functions/postgresql_default_spec.rb b/spec/functions/postgresql_default_spec.rb new file mode 100644 index 0000000000..12ecde207e --- /dev/null +++ b/spec/functions/postgresql_default_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'postgresql::default' do + let(:facts) do + { + 'os' => { + 'family' => 'Debian', + 'name' => 'Debian', + 'release' => { + 'full' => '11.7', + 'major' => '11', + 'minor' => '7', + } + } + } + end + + let(:pre_condition) do + <<~PP + class { 'postgresql::server': + } + PP + end + + # parameter in params.pp only + it { is_expected.to run.with_params('port').and_return(5432) } + + # parameter in globals.pp only + it { is_expected.to run.with_params('default_connect_settings').and_return({}) } + + it { is_expected.to run.with_params('a_parameter_that_does_not_exist').and_raise_error(Puppet::ParseError, %r{pick\(\): must receive at least one non empty value}) } +end From 4e15857d64bea9027d6f3b789771fb90057a2ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 11 Oct 2023 09:13:53 -1000 Subject: [PATCH 2/2] Simplify `postgresql::default()` `postgresql::params` inherits from `postgresql::globals` so any variable defined in `postgresql::globals` can be accessed by reading it from `postgresql::params`, making it redundant to check both. diff --git a/functions/default.pp b/functions/default.pp index e36610b..41b5006 100644 --- a/functions/default.pp +++ b/functions/default.pp @@ -8,8 +8,7 @@ function postgresql::default( ) { include postgresql::params - #search for the variable name in params first - #then fall back to globals if not found - pick(getvar("postgresql::params::${parameter_name}"), - getvar("postgresql::globals::${parameter_name}")) + # Search for the variable name in params. + # params inherits from globals, so it will also catch these variables. + pick(getvar("postgresql::params::${parameter_name}")) } --- functions/default.pp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/functions/default.pp b/functions/default.pp index e7b11f3b34..41b500642b 100644 --- a/functions/default.pp +++ b/functions/default.pp @@ -8,7 +8,7 @@ function postgresql::default( ) { include postgresql::params - #search for the variable name in params first - #then fall back to globals if not found - pick(getvar("postgresql::params::${parameter_name}"), getvar("postgresql::globals::${parameter_name}")) + # Search for the variable name in params. + # params inherits from globals, so it will also catch these variables. + pick(getvar("postgresql::params::${parameter_name}")) }