From d5192aadee3aa05a11ae758a836b0d27c1f0dfa6 Mon Sep 17 00:00:00 2001 From: Simon Hoenscheid Date: Fri, 25 Aug 2023 15:29:58 +0200 Subject: [PATCH 1/2] uniform port parameter and $port_override value assignment --- manifests/server/database.pp | 9 +++++---- manifests/server/default_privileges.pp | 11 +++-------- manifests/server/extension.pp | 15 ++++----------- manifests/server/grant.pp | 11 +++-------- manifests/server/reassign_owned_by.pp | 13 ++++--------- manifests/server/role.pp | 12 ++++-------- 6 files changed, 23 insertions(+), 48 deletions(-) diff --git a/manifests/server/database.pp b/manifests/server/database.pp index bfc7d53247..1282659596 100644 --- a/manifests/server/database.pp +++ b/manifests/server/database.pp @@ -9,6 +9,7 @@ # @param locale Overrides the locale during creation of the database. # @param istemplate Defines the database as a template if set to true. # @param connect_settings Specifies a hash of environment variables used when connecting to a remote server. +# @param port Specifies the port for the PostgreSQL server port to connect to, default is 5432. define postgresql::server::database ( Optional[String[1]] $comment = undef, String[1] $dbname = $title, @@ -19,6 +20,7 @@ Optional[String[1]] $locale = $postgresql::server::locale, Boolean $istemplate = false, Hash $connect_settings = $postgresql::server::default_connect_settings, + Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::params::port, ) { $createdb_path = $postgresql::server::createdb_path $user = $postgresql::server::user @@ -34,11 +36,10 @@ $version = $postgresql::server::_version } - # If the connection settings do not contain a port, then use the local server port if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port = undef + $port_override = $port } else { - $port = $postgresql::server::port + $port_override = $port } # Set the defaults for the postgresql_psql resource @@ -47,7 +48,7 @@ psql_user => $user, psql_group => $group, psql_path => $psql_path, - port => $port, + port => $port_override, connect_settings => $connect_settings, } diff --git a/manifests/server/default_privileges.pp b/manifests/server/default_privileges.pp index 34c4475f66..4add415d1d 100644 --- a/manifests/server/default_privileges.pp +++ b/manifests/server/default_privileges.pp @@ -10,7 +10,7 @@ # @param psql_db Defines the database to execute the grant against. This should not ordinarily be changed from the default. # @param psql_user Specifies the OS user for running psql. Default value: The default user for the module, usually 'postgres'. # @param psql_path Specifies the OS user for running psql. Default value: The default user for the module, usually 'postgres'. -# @param port Specifies the port to access the server. Default value: The default user for the module, usually '5432'. +# @param port Specifies the port for the PostgreSQL server port to connect to, default is 5432. # @param connect_settings Specifies a hash of environment variables used when connecting to a remote server. # @param group Specifies the user group to which the privileges will be granted. define postgresql::server::default_privileges ( @@ -59,15 +59,10 @@ } } - # - # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port - # - if $port != undef { + if $connect_settings != undef and 'PGPORT' in $connect_settings { $port_override = $port - } elsif $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = undef } else { - $port_override = $postgresql::server::port + $port_override = $port } if $target_role != undef { diff --git a/manifests/server/extension.pp b/manifests/server/extension.pp index 9f28e3e85c..1b21e4c92d 100644 --- a/manifests/server/extension.pp +++ b/manifests/server/extension.pp @@ -18,7 +18,7 @@ # @param package_ensure # Overrides default package deletion behavior. By default, the package specified with package_name is installed when the extension is # activated and removed when the extension is deactivated. To override this behavior, set the ensure value for the package. -# @param port Port to use when connecting. +# @param port Specifies the port for the PostgreSQL server port to connect to, default is 5432. # @param connect_settings Specifies a hash of environment variables used when connecting to a remote server. # @param database_resource_name Specifies the resource name of the DB being managed. Defaults to the parameter $database, if left blank. define postgresql::server::extension ( @@ -29,7 +29,7 @@ Optional[String[1]] $version = undef, Enum['present', 'absent'] $ensure = 'present', Optional[String[1]] $package_name = undef, - Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, + Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, Hash $connect_settings = postgresql::default('default_connect_settings'), String[1] $database_resource_name = $database, ) { @@ -74,24 +74,17 @@ } } - # - # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port - # - if $port != undef { + if $connect_settings != undef and 'PGPORT' in $connect_settings { $port_override = $port - } elsif $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = undef } else { - $port_override = $postgresql::server::port + $port_override = $port } postgresql_psql { "${database}: ${command}": - psql_user => $user, psql_group => $group, psql_path => $psql_path, connect_settings => $connect_settings, - db => $database, port => $port_override, command => $command, diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index c6bd7fe24b..0903c30f14 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -12,7 +12,7 @@ # @param object_arguments Specifies any arguments to be passed alongisde the access grant. # @param psql_db Specifies the database to execute the grant against. This should not ordinarily be changed from the default # @param psql_user Sets the OS user to run psql. -# @param port Port to use when connecting. +# @param port Specifies the port for the PostgreSQL server port to connect to, default is 5432. # @param onlyif_exists Create grant only if doesn't exist # @param connect_settings Specifies a hash of environment variables used when connecting to a remote server. # @param ensure Specifies whether to grant or revoke the privilege. Default is to grant the privilege. Valid values: 'present', 'absent'. @@ -74,15 +74,10 @@ $_object_name = $object_name } - # - # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port - # - if $port != undef { + if $connect_settings != undef and 'PGPORT' in $connect_settings { $port_override = $port - } elsif $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = undef } else { - $port_override = $postgresql::server::port + $port_override = $port } ## Munge the input values diff --git a/manifests/server/reassign_owned_by.pp b/manifests/server/reassign_owned_by.pp index 4d5eac43d4..b1c0a74afe 100644 --- a/manifests/server/reassign_owned_by.pp +++ b/manifests/server/reassign_owned_by.pp @@ -1,12 +1,12 @@ # @summary Define for reassigning the ownership of objects within a database. # @note # This enables us to force the a particular ownership for objects within a database -# +# # @param old_role Specifies the role or user who is the current owner of the objects in the specified db # @param new_role Specifies the role or user who will be the new owner of these objects # @param db Specifies the database to which the 'REASSIGN OWNED' will be applied # @param psql_user Specifies the OS user for running psql. -# @param port Port to use when connecting. +# @param port Specifies the port for the PostgreSQL server port to connect to, default is 5432. # @param connect_settings Specifies a hash of environment variables used when connecting to a remote server. define postgresql::server::reassign_owned_by ( String $old_role, @@ -21,15 +21,10 @@ $group = $postgresql::server::group $psql_path = $postgresql::server::psql_path - # - # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port - # - if $port != undef { + if $connect_settings != undef and 'PGPORT' in $connect_settings { $port_override = $port - } elsif $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = undef } else { - $port_override = $postgresql::server::port + $port_override = $port } $onlyif = "SELECT tablename FROM pg_catalog.pg_tables WHERE diff --git a/manifests/server/role.pp b/manifests/server/role.pp index dad17be125..23ad09942b 100644 --- a/manifests/server/role.pp +++ b/manifests/server/role.pp @@ -29,7 +29,7 @@ Boolean $createdb = false, Boolean $createrole = false, String[1] $db = $postgresql::server::default_database, - Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, + Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, Boolean $login = true, Boolean $inherit = true, Boolean $superuser = false, @@ -50,15 +50,11 @@ } else { $password_hash } - # - # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port - # - if $port != undef { + + if $connect_settings != undef and 'PGPORT' in $connect_settings { $port_override = $port - } elsif $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = undef } else { - $port_override = $postgresql::server::port + $port_override = $port } # If possible use the version of the remote database, otherwise From 86efa5ca909b2166639aea5fb148e87d98a92bfe Mon Sep 17 00:00:00 2001 From: Simon Hoenscheid Date: Fri, 25 Aug 2023 20:48:35 +0200 Subject: [PATCH 2/2] implement feedback --- manifests/server/database.pp | 8 ++------ manifests/server/default_privileges.pp | 8 ++------ manifests/server/extension.pp | 9 ++------- manifests/server/grant.pp | 8 ++------ manifests/server/reassign_owned_by.pp | 8 ++------ manifests/server/role.pp | 8 ++------ 6 files changed, 12 insertions(+), 37 deletions(-) diff --git a/manifests/server/database.pp b/manifests/server/database.pp index 1282659596..6da9250b9f 100644 --- a/manifests/server/database.pp +++ b/manifests/server/database.pp @@ -20,7 +20,7 @@ Optional[String[1]] $locale = $postgresql::server::locale, Boolean $istemplate = false, Hash $connect_settings = $postgresql::server::default_connect_settings, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::params::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, ) { $createdb_path = $postgresql::server::createdb_path $user = $postgresql::server::user @@ -36,11 +36,7 @@ $version = $postgresql::server::_version } - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) # Set the defaults for the postgresql_psql resource Postgresql_psql { diff --git a/manifests/server/default_privileges.pp b/manifests/server/default_privileges.pp index 4add415d1d..a2b039928d 100644 --- a/manifests/server/default_privileges.pp +++ b/manifests/server/default_privileges.pp @@ -28,7 +28,7 @@ String $schema = 'public', String $psql_db = $postgresql::server::default_database, String $psql_user = $postgresql::server::user, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, Hash $connect_settings = $postgresql::server::default_connect_settings, Enum['present', 'absent'] $ensure = 'present', String $group = $postgresql::server::group, @@ -59,11 +59,7 @@ } } - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) if $target_role != undef { $_target_role = " FOR ROLE ${target_role}" diff --git a/manifests/server/extension.pp b/manifests/server/extension.pp index 1b21e4c92d..542442f064 100644 --- a/manifests/server/extension.pp +++ b/manifests/server/extension.pp @@ -29,7 +29,7 @@ Optional[String[1]] $version = undef, Enum['present', 'absent'] $ensure = 'present', Optional[String[1]] $package_name = undef, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, Hash $connect_settings = postgresql::default('default_connect_settings'), String[1] $database_resource_name = $database, ) { @@ -73,12 +73,7 @@ fail("Unknown value for ensure '${ensure}'.") } } - - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) postgresql_psql { "${database}: ${command}": psql_user => $user, diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index 0903c30f14..1a0ebdc238 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -41,7 +41,7 @@ Array[String[1],0] $object_arguments = [], String $psql_db = $postgresql::server::default_database, String $psql_user = $postgresql::server::user, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, Boolean $onlyif_exists = false, Hash $connect_settings = $postgresql::server::default_connect_settings, Enum['present', 'absent'] $ensure = 'present', @@ -74,11 +74,7 @@ $_object_name = $object_name } - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) ## Munge the input values $_object_type = upcase($object_type) diff --git a/manifests/server/reassign_owned_by.pp b/manifests/server/reassign_owned_by.pp index b1c0a74afe..b47a890244 100644 --- a/manifests/server/reassign_owned_by.pp +++ b/manifests/server/reassign_owned_by.pp @@ -13,7 +13,7 @@ String $new_role, String $db, String $psql_user = $postgresql::server::user, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, Hash $connect_settings = $postgresql::server::default_connect_settings, ) { $sql_command = "REASSIGN OWNED BY \"${old_role}\" TO \"${new_role}\"" @@ -21,11 +21,7 @@ $group = $postgresql::server::group $psql_path = $postgresql::server::psql_path - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) $onlyif = "SELECT tablename FROM pg_catalog.pg_tables WHERE schemaname NOT IN ('pg_catalog', 'information_schema') AND diff --git a/manifests/server/role.pp b/manifests/server/role.pp index 23ad09942b..82079038e4 100644 --- a/manifests/server/role.pp +++ b/manifests/server/role.pp @@ -29,7 +29,7 @@ Boolean $createdb = false, Boolean $createrole = false, String[1] $db = $postgresql::server::default_database, - Variant[String[1], Stdlib::Port, Integer] $port = $postgresql::server::port, + Optional[Variant[String[1], Stdlib::Port, Integer]] $port = undef, Boolean $login = true, Boolean $inherit = true, Boolean $superuser = false, @@ -51,11 +51,7 @@ $password_hash } - if $connect_settings != undef and 'PGPORT' in $connect_settings { - $port_override = $port - } else { - $port_override = $port - } + $port_override = pick($port, $postgresql::server::port) # If possible use the version of the remote database, otherwise # fallback to our local DB version