Merge pull request #743 from bmjen/4.7.1_prep

4.7.1 Release Prep

.gitattributes

@@ -1,2 +1,5 @@
-*.sh  eol=lf
+#This file is generated by ModuleSync, do not edit.
 *.rb  eol=lf
+*.erb eol=lf
+*.pp  eol=lf
+*.sh  eol=lf

.gitignore

@@ -1,3 +1,4 @@
+#This file is generated by ModuleSync, do not edit.
 pkg/
 Gemfile.lock
 vendor/

.travis.yml

@@ -1,3 +1,4 @@
+#This file is generated by ModuleSync, do not edit.
 ---
 sudo: false
 language: ruby

CHANGELOG.md

@@ -1,3 +1,19 @@
+## Supported Release 4.7.1
+### Summary
+This release contains some bugfixes and documentation updates.
+
+#### Bugfixes
+- (MODULES-3024) Quote database objects when creating databases.
+- Properly escape case where password ends with '$'.
+- Fixes password change when postgres is configure to non-standard port.
+- Unpins concat dependency to be able to use concat 2.x.
+- Workaround to fix installing on Amazon Linux.
+- Fixes proper defaulting of `$service_provider` parameter.
+- Fixes postgres server init script naming on Amazon Linux.
+- Fixes service reload parameter on Arch Linux.
+- Adds missing onlyif_function to sequence grant code.
+- Fixes to the markdown of the README.
+
 ## Supported Release 4.7.0
 ### Summary
 A release with a considerable amount of new features, including remote db support and several platform support updates. Various bugfixes including several to address warnings and a sizable README update.

Gemfile

@@ -1,12 +1,14 @@
+#This file is generated by ModuleSync, do not edit.
+
 source ENV['GEM_SOURCE'] || "https://rubygems.org"
 
-def location_for(place, fake_version = nil)
+def location_for(place, version = nil)
   if place =~ /^(git[:@][^#]*)#(.*)/
-    [fake_version, { :git => $1, :branch => $2, :require => false }].compact
+    [version, { :git => $1, :branch => $2, :require => false}].compact
   elsif place =~ /^file:\/\/(.*)/
-    ['>= 0', { :path => File.expand_path($1), :require => false }]
+    ['>= 0', { :path => File.expand_path($1), :require => false}]
   else
-    [place, { :require => false }]
+    [place, version, { :require => false}].compact
   end
 end
 
@@ -20,29 +22,18 @@ group :development, :unit_tests do
   gem 'simplecov',                 :require => false
 end
 group :system_tests do
-  gem 'beaker-puppet_install_helper',  :require => false
-  if beaker_version = ENV['BEAKER_VERSION']
-    gem 'beaker', *location_for(beaker_version)
-  end
-  if beaker_rspec_version = ENV['BEAKER_RSPEC_VERSION']
-    gem 'beaker-rspec', *location_for(beaker_rspec_version)
-  else
-    gem 'beaker-rspec',  :require => false
-  end
-  gem 'master_manipulator',            :require => false
+  gem 'beaker-rspec',                  *location_for(ENV['BEAKER_RSPEC_VERSION'] || '>= 3.4')
+  gem 'beaker',                        *location_for(ENV['BEAKER_VERSION'])
   gem 'serverspec',                    :require => false
+  gem 'beaker-puppet_install_helper',  :require => false
+  gem 'master_manipulator',            :require => false
+  gem 'beaker-hostgenerator',          *location_for(ENV['BEAKER_HOSTGENERATOR_VERSION'])
 end
 
-if facterversion = ENV['FACTER_GEM_VERSION']
-  gem 'facter', facterversion, :require => false
-else
-  gem 'facter', :require => false
-end
+gem 'facter', *location_for(ENV['FACTER_GEM_VERSION'])
+gem 'puppet', *location_for(ENV['PUPPET_GEM_VERSION'])
 
-if puppetversion = ENV['PUPPET_GEM_VERSION']
-  gem 'puppet', puppetversion, :require => false
-else
-  gem 'puppet', :require => false
-end
 
-# vim:ft=ruby
+if File.exists? "#{__FILE__}.local"
+  eval(File.read("#{__FILE__}.local"), binding)
+end

LICENSE

@@ -187,7 +187,7 @@
       same "printed page" as the copyright notice for easier
       identification within third-party archives.
 
-   Copyright 2013 Puppet Labs
+   Copyright [yyyy] [name of copyright owner]
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.

NOTICE

@@ -1,8 +1,8 @@
-Puppetlabs postgresql module
+postgresql puppet module
 
-Copyright 2012 Inkling Systems Inc
-Copyright 2012-2013 Puppet Labs Inc
-Copyright 2012-2013 Camptocamp SA.
+Copyright (C) 2012-2016 Puppet Labs, Inc.
+Copyright (C) 2012 Inkling Systems Inc
+Copyright (C) 2012-2013 Camptocamp SA.
 
 This product includes software developed by:
   The Puppet Labs Inc (http://www.puppetlabs.com/).
@@ -12,3 +12,18 @@ This product includes also software developed by:
 
 This product includes also software developed by:
   Inkling Systems Inc (https://www.inkling.com/)
+
+Puppet Labs can be contacted at: info@puppetlabs.com
+
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.

README.md

@@ -229,13 +229,14 @@ postgresql::server::pg_ident_rule{ 'Map the SSL certificate of the backup server
   system_username   => 'repli1.example.com',
   database_username => 'replication',
 }
+```
 
 This would create a user name map in `pg_ident.conf` similar to:
 
 ```
-# Rule Name: Map the SSL certificate of the backup server as a replication user
-# Description: none
-# Order: 150
+#Rule Name: Map the SSL certificate of the backup server as a replication user
+#Description: none
+#Order: 150
 sslrepli  repli1.example.com  replication
 ```
 
@@ -280,10 +281,9 @@ recovery_min_apply_delay = 0
 
 Only the specified parameters are recognized in the template. The `recovery.conf` is only be created if at least one parameter is set **and** [manage_recovery_conf](#manage_recovery_conf) is set to true.
 
-
 ### Validate connectivity
 
-To validate client connections to a remote PostgreSQL database before starting dependent tasks, use the `postgresql::validate_db_connection` resource.  You can use this on any node where the PostgreSQL client software is installed. It is often chained to other tasks such as starting an application server or performing a database migration.
+To validate client connections to a remote PostgreSQL database before starting dependent tasks, use the `postgresql::validate_db_connection` resource. You can use this on any node where the PostgreSQL client software is installed. It is often chained to other tasks such as starting an application server or performing a database migration.
 
 Example usage:
 
@@ -299,11 +299,11 @@ exec { 'rake db:migrate':
 }
 ```
 
- ## Reference
+## Reference
 
 The posgresql module comes with many options for configuring the server. While you are unlikely to use all of the settings below, they provide a decent amount of control over your security settings.
 
-Classes:
+**Classes:**
 
 * [postgresql::client](#postgresqlclient)
 * [postgresql::globals](#postgresqlglobals)
@@ -316,7 +316,7 @@ Classes:
 * [postgresql::server::contrib](#postgresqlservercontrib)
 * [postgresql::server::postgis](#postgresqlserverpostgis)
 
-Defined Types:
+**Defined Types:**
 
 * [postgresql::server::config_entry](#postgresqlserverconfig_entry)
 * [postgresql::server::database](#postgresqlserverdatabase)
@@ -332,13 +332,13 @@ Defined Types:
 * [postgresql::server::tablespace](#postgresqlservertablespace)
 * [postgresql::validate_db_connection](#postgresqlvalidate_db_connection)
 
-Types:
+**Types:**
 
 * [postgresql_psql](#custom-resource-postgresql_psql)
 * [postgresql_replication_slot](#custom-resource-postgresql_replication_slot)
 * [postgresql_conf](#custom-resource-postgresql_conf)
 
-Functions:
+**Functions:**
 
 * [postgresql_password](#function-postgresql_password)
 * [postgresql_acls_to_resources_hash](#function-postgresql_acls_to_resources_hashacl_array-id-order_offset)
@@ -367,7 +367,7 @@ Specifies the path to validate the connection script. Default: '/usr/local/bin/v
 
 Installs PostgreSQL bindings for Postgres-Docs. Set the following parameters if you have a custom version you would like to install.
 
->**Note:** Make sure to add any necessary yum or apt repositories if specifying a custom version.
+**Note:** Make sure to add any necessary yum or apt repositories if specifying a custom version.
 
 ##### `package_name`
 
@@ -379,7 +379,7 @@ Whether the PostgreSQL docs package resource should be present. Valid values: 'p
 
 #### postgresql::globals
 
->**Note:** Most server-specific defaults should be overriden in the `postgresql::server` class. This class should be used only if you are using a non-standard OS, or if you are changing elements that can only be changed here, such as `version` or `manage_package_repo`.
+**Note:** Most server-specific defaults should be overriden in the `postgresql::server` class. This class should be used only if you are using a non-standard OS, or if you are changing elements that can only be changed here, such as `version` or `manage_package_repo`.
 
 ##### `bindir`
 
@@ -405,7 +405,7 @@ Overrides the default PostgreSQL contrib package name. Default: OS dependent.
 
 Overrides the default PostgreSQL data directory for the target platform. Default: OS dependent.
 
->**Note:** Changing the datadir after installation causes the server to come to a full stop before making the change. For RedHat systems, the data directory must be labeled appropriately for SELinux. On Ubuntu, you must explicitly set `needs_initdb = true` to allow Puppet to initialize the database in the new datadir (`needs_initdb` defaults to true on other systems).
+**Note:** Changing the datadir after installation causes the server to come to a full stop before making the change. For RedHat systems, the data directory must be labeled appropriately for SELinux. On Ubuntu, you must explicitly set `needs_initdb = true` to allow Puppet to initialize the database in the new datadir (`needs_initdb` defaults to true on other systems).
 
 **Warning:** If datadir is changed from the default, Puppet does not manage purging of the original data directory, which causes it to fail if the data directory is changed back to the original.
 
@@ -545,11 +545,11 @@ Overrides the `ensure` parameter during package installation. Defaults to `prese
 
 Overrides the default package name for the distribution you are installing to. Defaults to `postgresql-devel` or `postgresql<version>-devel` depending on your distro.
 
-####postgresql::lib::java
+#### postgresql::lib::java
 
 Installs PostgreSQL bindings for Java (JDBC). Set the following parameters if you have a custom version you would like to install.
 
->**Note:** Make sure to add any necessary yum or apt repositories if specifying a custom version.
+**Note:** Make sure to add any necessary yum or apt repositories if specifying a custom version.
 
 ##### `package_ensure`
 
@@ -583,7 +583,7 @@ Specifies the name of the postgresql PL/Python package.
 
 Specifies whether the package is present. Valid values: 'present', 'absent'. Default: 'present'.
 
-####postgresql::lib::python
+#### postgresql::lib::python
 
 Installs PostgreSQL Python libraries. 
 
@@ -595,7 +595,7 @@ Specifies whether the package is present. Valid values: 'present', 'absent'. Def
 
 The name of the PostgreSQL Python package.
 
-####postgresql::server
+#### postgresql::server
 
 ##### `createdb_path`
 
@@ -1094,6 +1094,7 @@ Sets the hash to use during password creation. If the password is not already pr
 postgresql::server::role { "myusername":
 password_hash => postgresql_password('myusername', 'mypassword'),
 }
+```
 
 ##### `replication`
 
@@ -1107,7 +1108,6 @@ Specifies whether to grant super user capability for the new role. Default: fals
 
 Defines the username of the role to create. Defaults to the namevar.
 
-
 #### postgresql::server::schema
 
 Creates a schema.

Rakefile

@@ -9,3 +9,34 @@ PuppetLint.configuration.send('disable_class_inherits_from_params_class')
 PuppetLint.configuration.send('disable_documentation')
 PuppetLint.configuration.send('disable_single_quote_string_with_variables')
 PuppetLint.configuration.ignore_paths = ["spec/**/*.pp", "pkg/**/*.pp"]
+
+desc 'Generate pooler nodesets'
+task :gen_nodeset do
+  require 'beaker-hostgenerator'
+  require 'securerandom'
+  require 'fileutils'
+
+  agent_target = ENV['TEST_TARGET']
+  if ! agent_target
+    STDERR.puts 'TEST_TARGET environment variable is not set'
+    STDERR.puts 'setting to default value of "redhat-64default."'
+    agent_target = 'redhat-64default.'
+  end
+
+  master_target = ENV['MASTER_TEST_TARGET']
+  if ! master_target
+    STDERR.puts 'MASTER_TEST_TARGET environment variable is not set'
+    STDERR.puts 'setting to default value of "redhat7-64mdcl"'
+    master_target = 'redhat7-64mdcl'
+  end
+
+  targets = "#{master_target}-#{agent_target}"
+  cli = BeakerHostGenerator::CLI.new([targets])
+  nodeset_dir = "tmp/nodesets"
+  nodeset = "#{nodeset_dir}/#{targets}-#{SecureRandom.uuid}.yaml"
+  FileUtils.mkdir_p(nodeset_dir)
+  File.open(nodeset, 'w') do |fh|
+    fh.print(cli.execute)
+  end
+  puts nodeset
+end

lib/puppet/parser/functions/postgresql_escape.rb

@@ -11,7 +11,7 @@ module Puppet::Parser::Functions
 
     password = args[0]
 
-    if password !~ /\$\$/ 
+    if password !~ /\$\$/ and password[-1] != '$'
       retval = "$$#{password}$$"
     else
       escape = Digest::MD5.hexdigest(password)[0..5].gsub(/\d/,'')
@@ -20,6 +20,6 @@ module Puppet::Parser::Functions
       end
       retval = "$#{escape}$#{password}$#{escape}$"
     end
-    retval 
+    retval
   end
 end

manifests/params.pp

@@ -14,7 +14,7 @@ class postgresql::params inherits postgresql::globals {
   $service_enable             = true
   $service_manage             = true
   $service_restart_on_change  = true
-  $service_provider           = $service_provider
+  $service_provider           = $postgresql::globals::service_provider
   $manage_pg_hba_conf         = pick($manage_pg_hba_conf, true)
   $manage_pg_ident_conf       = pick($manage_pg_ident_conf, true)
   $manage_recovery_conf       = pick($manage_recovery_conf, false)
@@ -30,7 +30,7 @@ class postgresql::params inherits postgresql::globals {
       $version_parts      = split($version, '[.]')
       $package_version    = "${version_parts[0]}${version_parts[1]}"
 
-      if $version == $postgresql::globals::default_version {
+      if $version == $postgresql::globals::default_version and $::operatingsystem != 'Amazon' {
         $client_package_name    = pick($client_package_name, 'postgresql')
         $server_package_name    = pick($server_package_name, 'postgresql-server')
         $contrib_package_name   = pick($contrib_package_name,'postgresql-contrib')
@@ -56,7 +56,7 @@ class postgresql::params inherits postgresql::globals {
         $plperl_package_name    = pick($plperl_package_name, "postgresql${package_version}-plperl")
         $plpython_package_name  = pick($plpython_package_name, "postgresql${package_version}-plpython")
         $service_name           = $::operatingsystem ? {
-          'Amazon' => pick($service_name, "postgresql${version}"),
+          'Amazon' => pick($service_name, "postgresql${version_parts[0]}${version_parts[1]}"),
           default  => pick($service_name, "postgresql-${version}"),
         }
         $bindir                 = $::operatingsystem ? {
@@ -112,7 +112,7 @@ class postgresql::params inherits postgresql::globals {
       $psql_path              = pick($psql_path, "${bindir}/psql")
 
       $service_status         = $service_status
-      $service_reload         = "service ${service_name} reload"
+      $service_reload         = "systemctl reload ${service_name}"
       $python_package_name    = pick($python_package_name, 'python-psycopg2')
       # Archlinux does not have a perl::DBD::Pg package
       $perl_package_name      = pick($perl_package_name, 'undef')

manifests/server/database.pp

@@ -55,7 +55,7 @@ define postgresql::server::database(
 
   $template_option = $template ? {
     undef   => '',
-    default => "TEMPLATE=${template}",
+    default => "TEMPLATE=\"${template}\"",
   }
 
   $encoding_option = $encoding ? {
@@ -65,7 +65,7 @@ define postgresql::server::database(
 
   $tablespace_option = $tablespace ? {
     undef   => '',
-    default => "TABLESPACE=${tablespace}",
+    default => "TABLESPACE=\"${tablespace}\"",
   }
 
   if $createdb_path != undef{
@@ -73,7 +73,7 @@ define postgresql::server::database(
   }
 
   postgresql_psql { "Create db '${dbname}'":
-    command => "CREATE DATABASE \"${dbname}\" WITH OWNER=${owner} ${template_option} ${encoding_option} ${locale_option} ${tablespace_option}",
+    command => "CREATE DATABASE \"${dbname}\" WITH OWNER=\"${owner}\" ${template_option} ${encoding_option} ${locale_option} ${tablespace_option}",
     unless  => "SELECT datname FROM pg_database WHERE datname='${dbname}'",
     db      => $default_db,
     require => Class['postgresql::server::service']
@@ -99,7 +99,7 @@ define postgresql::server::database(
       default => 'shobj_description',
     }
     Postgresql_psql[ "Create db '${dbname}'" ]->
-    postgresql_psql {"COMMENT ON DATABASE ${dbname} IS '${comment}'":
+    postgresql_psql {"COMMENT ON DATABASE \"${dbname}\" IS '${comment}'":
       unless => "SELECT pg_catalog.${comment_information_function}(d.oid, 'pg_database') as \"Description\" FROM pg_catalog.pg_database d WHERE datname = '${dbname}' AND pg_catalog.${comment_information_function}(d.oid, 'pg_database') = '${comment}'",
       db     => $dbname,
     }

manifests/server/grant.pp

@@ -96,11 +96,13 @@ define postgresql::server::grant (
       validate_string($unless_privilege,'USAGE','ALL','ALL PRIVILEGES')
       $unless_function = 'has_sequence_privilege'
       $on_db = $db
+      $onlyif_function = undef
     }
     'ALL SEQUENCES IN SCHEMA': {
       validate_string($_privilege,'USAGE','ALL','ALL PRIVILEGES')
       $unless_function = 'custom'
       $on_db = $db
+      $onlyif_function = undef
 
       $schema = $object_name
 
@@ -247,4 +249,4 @@ define postgresql::server::grant (
   if($db != undef and defined(Postgresql::Server::Database[$db])) {
     Postgresql::Server::Database[$db]->Postgresql_psql["grant:${name}"]
   }
-}
+}

manifests/server/passwd.pp

@@ -22,6 +22,7 @@ class postgresql::server::passwd {
       cwd         => '/tmp',
       environment => [
         "PGPASSWORD=${postgres_password}",
+        "PGPORT=${port}",
         "NEWPASSWD_ESCAPED=${escaped}",
       ],
       # With this command we're passing -h to force TCP authentication, which

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "puppetlabs-postgresql",
-  "version": "4.7.0",
+  "version": "4.7.1",
   "author": "Inkling/Puppet Labs",
   "summary": "Offers support for basic management of PostgreSQL databases.",
   "license": "Apache-2.0",
@@ -10,8 +10,9 @@
   "dependencies": [
     {"name":"puppetlabs/stdlib","version_requirement":"4.x"},
     {"name":"puppetlabs/apt","version_requirement":">=1.8.0 <3.0.0"},
-    {"name":"puppetlabs/concat","version_requirement":">= 1.1.0 <2.0.0"}
+    {"name":"puppetlabs/concat","version_requirement":">= 1.1.0 <3.0.0"}
   ],
+  "data_provider": null,
   "operatingsystem_support": [
     {
       "operatingsystem": "RedHat",

spec/acceptance/db_spec.rb

@@ -8,14 +8,14 @@ describe 'postgresql::server::db', :unless => UNSUPPORTED_PLATFORMS.include?(fac
         class { 'postgresql::server':
           postgres_password => 'space password',
         }
-        postgresql::server::tablespace { 'postgresql_test_db':
+        postgresql::server::tablespace { 'postgresql-test-db':
           location => '#{tmpdir}',
         } ->
-        postgresql::server::db { 'postgresql_test_db':
+        postgresql::server::db { 'postgresql-test-db':
           comment    => 'testcomment',
-          user       => 'test',
+          user       => 'test-user',
           password   => 'test1',
-          tablespace => 'postgresql_test_db',
+          tablespace => 'postgresql-test-db',
         }
       EOS
 
@@ -27,12 +27,12 @@ describe 'postgresql::server::db', :unless => UNSUPPORTED_PLATFORMS.include?(fac
       shell("chmod 600 /root/.pgpass")
       shell("psql -U postgres -h localhost --command='\\l'")
 
-      psql('--command="select datname from pg_database" postgresql_test_db') do |r|
-        expect(r.stdout).to match(/postgresql_test_db/)
+      psql('--command="select datname from pg_database" "postgresql-test-db"') do |r|
+        expect(r.stdout).to match(/postgresql-test-db/)
         expect(r.stderr).to eq('')
       end
 
-      psql('--command="SELECT 1 FROM pg_roles WHERE rolname=\'test\'"') do |r|
+      psql('--command="SELECT 1 FROM pg_roles WHERE rolname=\'test-user\'"') do |r|
         expect(r.stdout).to match(/\(1 row\)/)
       end
 
@@ -43,11 +43,11 @@ describe 'postgresql::server::db', :unless => UNSUPPORTED_PLATFORMS.include?(fac
       else
         comment_information_function = "obj_description"
       end
-      psql("--dbname postgresql_test_db --command=\"SELECT pg_catalog.#{comment_information_function}(d.oid, 'pg_database') FROM pg_catalog.pg_database d WHERE datname = 'postgresql_test_db' AND pg_catalog.#{comment_information_function}(d.oid, 'pg_database') = 'testcomment'\"") do |r|
+      psql("--dbname postgresql-test-db --command=\"SELECT pg_catalog.#{comment_information_function}(d.oid, 'pg_database') FROM pg_catalog.pg_database d WHERE datname = 'postgresql-test-db' AND pg_catalog.#{comment_information_function}(d.oid, 'pg_database') = 'testcomment'\"") do |r|
         expect(r.stdout).to match(/\(1 row\)/)
       end
     ensure
-      psql('--command="drop database postgresql_test_db" postgres')
+      psql('--command=\'drop database "postgresql-test-db" postgres\'')
       psql('--command="DROP USER test"')
     end
   end

spec/spec_helper.rb

@@ -1,3 +1,4 @@
+#This file is generated by ModuleSync, do not edit.
 require 'puppetlabs_spec_helper/module_spec_helper'
 
 # put local configuration and setup into spec_helper_local

spec/unit/classes/server_spec.rb

@@ -45,6 +45,7 @@ describe 'postgresql::server', :type => :class do
         'user'        => 'postgres',
         'environment' => [
           "PGPASSWORD=new-p@s$word-to-set",
+          "PGPORT=5432",
           "NEWPASSWD_ESCAPED=$$new-p@s$word-to-set$$"
         ],
         'unless'      => "/usr/bin/psql -h localhost -p 5432 -c 'select 1' > /dev/null",

spec/unit/defines/server/database_spec.rb

@@ -27,7 +27,7 @@ describe 'postgresql::server::database', :type => :define do
     let (:params) {{ :comment => 'test comment',
                      :connect_settings => {} }}
 
-    it { is_expected.to contain_postgresql_psql("COMMENT ON DATABASE test IS 'test comment'").with_connect_settings( {} ) }
+    it { is_expected.to contain_postgresql_psql("COMMENT ON DATABASE \"test\" IS 'test comment'").with_connect_settings( {} ) }
   end
 
   context "with specific db connection settings - default port" do

spec/unit/functions/postgresql_escape_spec.rb

@@ -8,3 +8,7 @@ describe 'postgresql_escape', :type => :puppet_function do
   it { is_expected.to run.with_params('fo$$o').
     and_return('$ed$fo$$o$ed$') }
 end
+describe 'postgresql_escape', :type => :puppet_function do
+  it { is_expected.to run.with_params('foo$').
+    and_return('$a$foo$$a$') }
+end