(maint) address comments

Addresses a few of @hasegeli\'s comments. Adds a `command` parameter for more customizability.

Author: eputnam

README.md

@@ -1668,6 +1668,12 @@ Default value: ''
 
 When using a Unix socket and ident auth, this is the user you are running as.
 
+##### `command`
+
+This is the command run against the target database to verify connectivity.
+
+Default value: 'SELECT 1'
+
 ##### `host`
 
 Sets the hostname of the database you wish to test.

lib/puppet/type/postgresql_conn_validator.rb

@@ -35,15 +35,15 @@
     desc 'The port that the PostgreSQL server should be listening on.'
 
     validate do |value|
-      if value
-        value =~ /[0-9]+/
-      end
+      Integer(value)
+    end
+    munge do |value|
+      Integer(value)
     end
   end
 
   newparam(:connect_settings) do
     desc 'Hash of environment variables for connection to a db.'
-
   end
 
   newparam(:sleep) do
@@ -79,4 +79,10 @@
   newparam(:run_as) do
     desc "System user that will run the psql command."
   end
+
+  newparam(:command) do
+    desc "Command to run against target database."
+
+    defaultto "SELECT 1"
+  end
 end

lib/puppet/util/postgresql_validator.rb

@@ -10,18 +10,19 @@ def initialize(resource)
       def build_psql_cmd
         final_cmd = []
 
-        cmd_init = "#{@resource[:psql_path]} --tuples-only --quiet "
+        cmd_init = "#{@resource[:psql_path]} --tuples-only --quiet --no-psqlrc"
 
         final_cmd.push cmd_init
 
         cmd_parts = {
-          :host => "-h #{@resource[:host]}",
-          :port => "-p #{@resource[:port]}",
-          :db_username => "-U #{@resource[:db_username]}",
-          :db_name => "--dbname #{@resource[:db_name]}"
+          :host => "--host=#{@resource[:host]}",
+          :port => "--port=#{@resource[:port]}",
+          :db_username => "--username=#{@resource[:db_username]}",
+          :db_name => "--dbname=#{@resource[:db_name]}",
+          :command => "--command='#{@resource[:command]}'"
         }
 
-        cmd_parts[:db_password] = "-w " if @resource[:db_password]
+        cmd_parts[:db_password] = "--no-password " if @resource[:db_password]
 
         cmd_parts.each do |k,v|
           final_cmd.push v if @resource[k]
@@ -57,7 +58,7 @@ def execute_command
       end
 
       def build_validate_cmd
-        "/bin/echo 'SELECT 1' | #{parse_connect_settings.join(' ')} #{build_psql_cmd} "
+        "#{parse_connect_settings.join(' ')} #{build_psql_cmd} "
       end
     end
   end

manifests/validate_db_connection.pp

@@ -65,9 +65,6 @@
     $env = $pass_env
   }
 
-  warning($validate_cmd)
-  warning($env)
-
   $exec_name = "validate postgres connection for ${database_username}@${database_host}:${database_port}/${database_name}"
 
   exec { $exec_name:

ruby.rb

@@ -11,15 +11,15 @@
     {
       :psql_path   => '/usr/bin/psql',
       :host        => 'db.test.com',
-      :port        => '4444',
+      :port        => 4444,
       :db_username => 'testuser',
       :db_password => 'testpass'
     }
   end
 
   describe '#build_psql_cmd' do
     it 'contains expected commandline options' do
-      expect(provider.validator.build_psql_cmd).to match /\/usr\/bin\/psql.*-h.*-p.*-U.*/
+      expect(provider.validator.build_psql_cmd).to match /\/usr\/bin\/psql.*--host=.*--port=.*--username=.*/
     end
   end
 

spec/unit/puppet/provider/postgresql_conn_validator/ruby_spec.rb

@@ -9,7 +9,7 @@
   end
 
   describe "when validating attributes" do
-    [:name, :db_name, :db_username, :host, :port, :connect_settings, :sleep, :tries, :psql_path].each do |param|
+    [:name, :db_name, :db_username, :command, :host, :port, :connect_settings, :sleep, :tries, :psql_path].each do |param|
       it "should have a #{param} parameter" do
         expect(described_class.attrtype(param)).to eq(:param)
       end
@@ -18,7 +18,7 @@
 
   describe "when validating values" do
     describe "tries and sleep" do
-      [:tries, :sleep].each do |param|
+      [:tries, :sleep, :port].each do |param|
         it "#{param} should be able to cast value as integer" do
           expect { described_class.new(:name => 'test', param => '1') }.to_not raise_error
           expect { described_class.new(:name => 'test', param => 1) }.to_not raise_error