Skip to content

Commit 09289b8

Browse files
bastelfreakbastelfreak
committed
Prefer $connect_settings over explicit parameters
This is the outcome of a large refactoring in #1450 and discussions on IRC and slack. We noticed that the behaviour of `$connect_settings` is currently inconsistent. Sometimes it's preferred over explicity parameters, sometimes not. Reminder: The idea of `$connect_settings` is to provide a hash with environment variable to tell puppet to manage a remote database instead of a local instance. One example is: https://github.com/puppetlabs/puppetlabs-postgresql/blob/93386b48861ff41d5f47033afee592e0506526c5/manifests/server/grant.pp#L80-L86 ``` if $port { $port_override = $port } elsif 'PGPORT' in $connect_settings { $port_override = undef } else { $port_override = $postgresql::server::port } ``` Here the local $port parameter is preferred over `$connect_settings`. The problem is that we now cannot set a default in the resource for `$port`. The default is hardcoded to `$postgresql::server::port`. This becomes. Other classes handle it in a different way: https://github.com/puppetlabs/puppetlabs-postgresql/blob/93386b48861ff41d5f47033afee592e0506526c5/manifests/server/database.pp#L41-L46 ``` if 'PGPORT' in $connect_settings { $port_override = undef } else { $port_override = $port } ``` Here `$connct_settings` is checked first. It defaults to an empty hash. If `PGPORT` isn't in it, `$port` is used. The logic is shorter and enables us to expose $port as a parameter with a default value. With this approach users can decide if they pass `$port` or `$connect_settings`. At the moment the remote database support is broken because the logic to parse `$connect_settings` isn't consistent. The goal of this PR is to unify the logic and prefer `$connect_settings` if it's provided by the user.
1 parent e4b334d commit 09289b8

17 files changed

+84
-132
lines changed

‎README.md‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,15 @@ class { 'postgresql::server':
175175

176176
### Manage remote users, roles, and permissions
177177

178-
Remote SQL objects are managed using the same Puppet resources as local SQL objects, along with a [`connect_settings`](#connect_settings) hash. This provides control over how Puppet connects to the remote Postgres instances and which version is used for generating SQL commands.
178+
Remote SQL objects are managed using the same Puppet resources as local SQL objects, along with a `$connect_settings` hash. This provides control over how Puppet connects to the remote Postgres instances and which version is used for generating SQL commands.
179179

180-
The `connect_settings` hash can contain environment variables to control Postgres client connections, such as 'PGHOST', 'PGPORT', 'PGPASSWORD', and 'PGSSLKEY'. See the [PostgreSQL Environment Variables](http://www.postgresql.org/docs/9.4/static/libpq-envars.html) documentation for a complete list of variables.
180+
The `connect_settings` hash can contain environment variables to control Postgres client connections, such as 'PGHOST', 'PGPORT', 'PGPASSWORD', 'PGUSER' and 'PGSSLKEY'. See the [PostgreSQL Environment Variables](https://www.postgresql.org/docs/current/libpq-envars.html) documentation for a complete list of variables.
181181

182-
Additionally, you can specify the target database version with the special value of 'DBVERSION'. If the `connect_settings` hash is omitted or empty, then Puppet connects to the local PostgreSQL instance.
182+
Additionally, you can specify the target database version with the special value of 'DBVERSION'. If the `$connect_settings` hash is omitted or empty, then Puppet connects to the local PostgreSQL instance.
183+
184+
**The $connect_settings hash has priority over the explicit variables like $port and $user**
185+
186+
When a user provides only the `$port` parameter to a resource and no `$connect_settings`, `$port` will be used. When `$connect_settings` contains `PGPORT` and `$port` is set, `$connect_settings['PGPORT']` will be used.
183187

184188
You can provide a `connect_settings` hash for each of the Puppet resources, or you can set a default `connect_settings` hash in `postgresql::globals`. Configuring `connect_settings` per resource allows SQL objects to be created on multiple databases by multiple users.
185189

‎REFERENCE.md‎

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ Default value: `undef`
420420

421421
##### <a name="-postgresql--globals--datadir"></a>`datadir`
422422

423-
Data type: `Optional[String[1]]`
423+
Data type: `Optional[Stdlib::Absolutepath]`
424424

425425
Overrides the default PostgreSQL data directory for the target platform.
426426
Changing the datadir after installation causes the server to come to a full stop before making the change.
@@ -434,31 +434,31 @@ Default value: `undef`
434434

435435
##### <a name="-postgresql--globals--confdir"></a>`confdir`
436436

437-
Data type: `Optional[String[1]]`
437+
Data type: `Optional[Stdlib::Absolutepath]`
438438

439439
Overrides the default PostgreSQL configuration directory for the target platform.
440440

441441
Default value: `undef`
442442

443443
##### <a name="-postgresql--globals--bindir"></a>`bindir`
444444

445-
Data type: `Optional[String[1]]`
445+
Data type: `Optional[Stdlib::Absolutepath]`
446446

447447
Overrides the default PostgreSQL binaries directory for the target platform.
448448

449449
Default value: `undef`
450450

451451
##### <a name="-postgresql--globals--xlogdir"></a>`xlogdir`
452452

453-
Data type: `Optional[String[1]]`
453+
Data type: `Optional[Stdlib::Absolutepath]`
454454

455455
Overrides the default PostgreSQL xlog directory.
456456

457457
Default value: `undef`
458458

459459
##### <a name="-postgresql--globals--logdir"></a>`logdir`
460460

461-
Data type: `Optional[String[1]]`
461+
Data type: `Optional[Stdlib::Absolutepath]`
462462

463463
Overrides the default PostgreSQL log directory.
464464

@@ -660,7 +660,7 @@ Default value: `false`
660660

661661
##### <a name="-postgresql--globals--module_workdir"></a>`module_workdir`
662662

663-
Data type: `Optional[String[1]]`
663+
Data type: `Optional[Stdlib::Absolutepath]`
664664

665665
Specifies working directory under which the psql command should be executed.
666666
May need to specify if '/tmp' is on volume mounted with noexec option.
@@ -1124,23 +1124,23 @@ Default value: `$postgresql::params::recovery_conf_path`
11241124

11251125
##### <a name="-postgresql--server--datadir"></a>`datadir`
11261126

1127-
Data type: `String[1]`
1127+
Data type: `Stdlib::Absolutepath`
11281128

11291129
PostgreSQL data directory
11301130

11311131
Default value: `$postgresql::params::datadir`
11321132

11331133
##### <a name="-postgresql--server--xlogdir"></a>`xlogdir`
11341134

1135-
Data type: `Optional[String[1]]`
1135+
Data type: `Optional[Stdlib::Absolutepath]`
11361136

11371137
PostgreSQL xlog directory
11381138

11391139
Default value: `$postgresql::params::xlogdir`
11401140

11411141
##### <a name="-postgresql--server--logdir"></a>`logdir`
11421142

1143-
Data type: `Optional[String[1]]`
1143+
Data type: `Optional[Stdlib::Absolutepath]`
11441144

11451145
PostgreSQL log directory
11461146

@@ -1268,7 +1268,7 @@ Default value: `$postgresql::params::manage_selinux`
12681268

12691269
##### <a name="-postgresql--server--module_workdir"></a>`module_workdir`
12701270

1271-
Data type: `String[1]`
1271+
Data type: `Stdlib::Absolutepath`
12721272

12731273
Working directory for the PostgreSQL module
12741274

@@ -1754,19 +1754,19 @@ Default value: `$postgresql::server::group`
17541754

17551755
##### <a name="-postgresql--server--database_grant--connect_settings"></a>`connect_settings`
17561756

1757-
Data type: `Optional[Hash]`
1757+
Data type: `Hash`
17581758

17591759
Specifies a hash of environment variables used when connecting to a remote server.
17601760

1761-
Default value: `undef`
1761+
Default value: `$postgresql::server::default_connect_settings`
17621762

17631763
##### <a name="-postgresql--server--database_grant--port"></a>`port`
17641764

1765-
Data type: `Optional[Stdlib::Port]`
1765+
Data type: `Stdlib::Port`
17661766

17671767
Port to use when connecting.
17681768

1769-
Default value: `undef`
1769+
Default value: `$postgresql::server::port`
17701770

17711771
### <a name="postgresql--server--db"></a>`postgresql::server::db`
17721772

@@ -2116,11 +2116,11 @@ Default value: `undef`
21162116

21172117
##### <a name="-postgresql--server--extension--port"></a>`port`
21182118

2119-
Data type: `Optional[Stdlib::Port]`
2119+
Data type: `Stdlib::Port`
21202120

21212121
Port to use when connecting.
21222122

2123-
Default value: `undef`
2123+
Default value: `postgresql::default('port')`
21242124

21252125
##### <a name="-postgresql--server--extension--connect_settings"></a>`connect_settings`
21262126

@@ -2267,11 +2267,11 @@ Default value: `$postgresql::server::user`
22672267

22682268
##### <a name="-postgresql--server--grant--port"></a>`port`
22692269

2270-
Data type: `Optional[Stdlib::Port]`
2270+
Data type: `Stdlib::Port`
22712271

22722272
Port to use when connecting.
22732273

2274-
Default value: `undef`
2274+
Default value: `$postgresql::server::port`
22752275

22762276
##### <a name="-postgresql--server--grant--onlyif_exists"></a>`onlyif_exists`
22772277

@@ -2585,15 +2585,15 @@ Default value: `$postgresql::server::manage_postgresql_conf_perms`
25852585

25862586
##### <a name="-postgresql--server--instance--config--datadir"></a>`datadir`
25872587

2588-
Data type: `String[1]`
2588+
Data type: `Stdlib::Absolutepath`
25892589

25902590
PostgreSQL data directory
25912591

25922592
Default value: `$postgresql::server::datadir`
25932593

25942594
##### <a name="-postgresql--server--instance--config--logdir"></a>`logdir`
25952595

2596-
Data type: `Optional[String[1]]`
2596+
Data type: `Optional[Stdlib::Absolutepath]`
25972597

25982598
PostgreSQL log directory
25992599

@@ -2700,7 +2700,7 @@ Default value: `$postgresql::server::data_checksums`
27002700

27012701
##### <a name="-postgresql--server--instance--initdb--datadir"></a>`datadir`
27022702

2703-
Data type: `String[1]`
2703+
Data type: `Stdlib::Absolutepath`
27042704

27052705
PostgreSQL data directory
27062706

@@ -2753,7 +2753,7 @@ Default value: `$postgresql::server::locale`
27532753

27542754
##### <a name="-postgresql--server--instance--initdb--logdir"></a>`logdir`
27552755

2756-
Data type: `Optional[String[1]]`
2756+
Data type: `Optional[Stdlib::Absolutepath]`
27572757

27582758
PostgreSQL log directory
27592759

@@ -2785,7 +2785,7 @@ Default value: `$postgresql::server::manage_xlogdir`
27852785

27862786
##### <a name="-postgresql--server--instance--initdb--module_workdir"></a>`module_workdir`
27872787

2788-
Data type: `String[1]`
2788+
Data type: `Stdlib::Absolutepath`
27892789

27902790
Working directory for the PostgreSQL module
27912791

@@ -2818,7 +2818,7 @@ Default value: `$postgresql::server::username`
28182818

28192819
##### <a name="-postgresql--server--instance--initdb--xlogdir"></a>`xlogdir`
28202820

2821-
Data type: `Optional[String[1]]`
2821+
Data type: `Optional[Stdlib::Absolutepath]`
28222822

28232823
PostgreSQL xlog/WAL directory
28242824

@@ -2884,7 +2884,7 @@ Default value: `$postgresql::server::port`
28842884

28852885
##### <a name="-postgresql--server--instance--late_initdb--module_workdir"></a>`module_workdir`
28862886

2887-
Data type: `String[1]`
2887+
Data type: `Stdlib::Absolutepath`
28882888

28892889
Working directory for the PostgreSQL module
28902890

@@ -2951,7 +2951,7 @@ Default value: `$postgresql::server::default_database`
29512951

29522952
##### <a name="-postgresql--server--instance--passwd--module_workdir"></a>`module_workdir`
29532953

2954-
Data type: `String[1]`
2954+
Data type: `Stdlib::Absolutepath`
29552955

29562956
Working directory for the PostgreSQL module
29572957

@@ -3554,11 +3554,11 @@ Default value: `$postgresql::server::default_database`
35543554

35553555
##### <a name="-postgresql--server--role--port"></a>`port`
35563556

3557-
Data type: `Optional[Stdlib::Port]`
3557+
Data type: `Stdlib::Port`
35583558

35593559
Port to use when connecting.
35603560

3561-
Default value: `undef`
3561+
Default value: `postgresql::default('port')`
35623562

35633563
##### <a name="-postgresql--server--role--login"></a>`login`
35643564

@@ -3697,6 +3697,7 @@ The following parameters are available in the `postgresql::server::schema` defin
36973697
* [`owner`](#-postgresql--server--schema--owner)
36983698
* [`schema`](#-postgresql--server--schema--schema)
36993699
* [`connect_settings`](#-postgresql--server--schema--connect_settings)
3700+
* [`port`](#-postgresql--server--schema--port)
37003701

37013702
##### <a name="-postgresql--server--schema--db"></a>`db`
37023703

@@ -3730,6 +3731,14 @@ Specifies a hash of environment variables used when connecting to a remote serve
37303731

37313732
Default value: `$postgresql::server::default_connect_settings`
37323733

3734+
##### <a name="-postgresql--server--schema--port"></a>`port`
3735+
3736+
Data type: `Stdlib::Port`
3737+
3738+
the post the postgresql instance is listening on.
3739+
3740+
Default value: `$postgresql::server::port`
3741+
37333742
### <a name="postgresql--server--table_grant"></a>`postgresql::server::table_grant`
37343743

37353744
This resource wraps the grant resource to manage table grants specifically.
@@ -3840,6 +3849,7 @@ The following parameters are available in the `postgresql::server::tablespace` d
38403849
* [`owner`](#-postgresql--server--tablespace--owner)
38413850
* [`spcname`](#-postgresql--server--tablespace--spcname)
38423851
* [`connect_settings`](#-postgresql--server--tablespace--connect_settings)
3852+
* [`port`](#-postgresql--server--tablespace--port)
38433853

38443854
##### <a name="-postgresql--server--tablespace--location"></a>`location`
38453855

@@ -3879,6 +3889,14 @@ Specifies a hash of environment variables used when connecting to a remote serve
38793889

38803890
Default value: `$postgresql::server::default_connect_settings`
38813891

3892+
##### <a name="-postgresql--server--tablespace--port"></a>`port`
3893+
3894+
Data type: `Stdlib::Port`
3895+
3896+
the port of the postgresql instance that sould be used.
3897+
3898+
Default value: `$postgresql::server::port`
3899+
38823900
## Resource types
38833901

38843902
### <a name="postgresql_conf"></a>`postgresql_conf`

‎manifests/server/database.pp‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,8 @@
3030
String[1] $default_db = $postgresql::server::default_database,
3131
Stdlib::Port $port = $postgresql::server::port
3232
) {
33-
# If possible use the version of the remote database, otherwise
34-
# fallback to our local DB version
35-
if 'DBVERSION' in $connect_settings {
36-
$version = $connect_settings['DBVERSION']
37-
} else {
38-
$version = $postgresql::server::_version
39-
}
40-
41-
# If the connection settings do not contain a port, then use the local server port
42-
if 'PGPORT' in $connect_settings {
43-
$port_override = undef
44-
} else {
45-
$port_override = $port
46-
}
33+
$version = pick($connect_settings['DBVERSION'], $postgresql::server::_version)
34+
$port_override = pick($connect_settings['PGPORT'], $port)
4735

4836
# Set the defaults for the postgresql_psql resource
4937
Postgresql_psql {

‎manifests/server/database_grant.pp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
Optional[Enum['present', 'absent']] $ensure = undef,
1717
Optional[String[1]] $psql_db = undef,
1818
String[1] $psql_user = $postgresql::server::user,
19-
Optional[Hash] $connect_settings = undef,
19+
Hash $connect_settings = $postgresql::server::default_connect_settings,
2020
String[1] $psql_group = $postgresql::server::group,
21-
Optional[Stdlib::Port] $port = undef,
21+
Stdlib::Port $port = $postgresql::server::port,
2222
) {
2323
postgresql::server::grant { "database:${name}":
2424
ensure => $ensure,

‎manifests/server/default_privileges.pp‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,8 @@
3535
Stdlib::Absolutepath $psql_path = $postgresql::server::psql_path,
3636
Optional[String] $target_role = undef,
3737
) {
38-
# If possible use the version of the remote database, otherwise
39-
# fallback to our local DB version
40-
if 'DBVERSION' in $connect_settings {
41-
$version = $connect_settings['DBVERSION']
42-
} else {
43-
$version = $postgresql::server::_version
44-
}
38+
$version = pick($connect_settings['DBVERSION'],postgresql::default('version'))
39+
$port_override = pick($connect_settings['PGPORT'], $port)
4540

4641
if (versioncmp($version, '9.6') == -1) {
4742
fail 'Default_privileges is only useable with PostgreSQL >= 9.6'
@@ -59,17 +54,6 @@
5954
}
6055
}
6156

62-
#
63-
# Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port
64-
#
65-
if $port {
66-
$port_override = $port
67-
} elsif 'PGPORT' in $connect_settings {
68-
$port_override = undef
69-
} else {
70-
$port_override = $postgresql::server::port
71-
}
72-
7357
if $target_role {
7458
$_target_role = " FOR ROLE ${target_role}"
7559
$_check_target_role = "/${target_role}"

‎manifests/server/extension.pp‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
Optional[String[1]] $version = undef,
3333
Enum['present', 'absent'] $ensure = 'present',
3434
Optional[String[1]] $package_name = undef,
35-
Optional[Stdlib::Port] $port = undef,
35+
Stdlib::Port $port = postgresql::default('port'),
3636
Hash $connect_settings = postgresql::default('default_connect_settings'),
3737
String[1] $database_resource_name = $database,
3838
String[1] $user = postgresql::default('user'),
@@ -76,16 +76,7 @@
7676
}
7777
}
7878

79-
#
80-
# Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port
81-
#
82-
if $port {
83-
$port_override = $port
84-
} elsif 'PGPORT' in $connect_settings {
85-
$port_override = undef
86-
} else {
87-
$port_override = $postgresql::server::port
88-
}
79+
$port_override = pick($connect_settings['PGPORT'], $port)
8980

9081
postgresql_psql { "${database}: ${command}":
9182
psql_user => $user,

0 commit comments

Comments
 (0)