Initial working implementation of ruby psql type/provider

This commit provides a working implementation of a ruby
type/provider (`postgresql_psql` for handling the PSQL
commands.  This is a little more flexible than doing it
via Exec resources, which is what the `postgresql::psql`
type was doing.

The old type is still present but now includes a
deprecation notification, and all of the other types
that were using the `::psql` type have been ported over
to use the `postgresql_psql` type instead.

README.md

@@ -20,7 +20,7 @@ This module provides the following defined resource types for managing postgres:
 
 And the fallback, analogous to exec resources, only for SQL statements:
 
- * `postgresql::psql`
+ * `postgresql_psql`
 
 Basic usage
 -----------

lib/puppet/provider/postgresql_psql/ruby.rb

@@ -0,0 +1,58 @@
+Puppet::Type.type(:postgresql_psql).provide(:ruby) do
+
+  def command()
+    if ((! resource[:unless]) or (resource[:unless].empty?))
+      if (resource[:refreshonly])
+        # So, if there's no 'unless', and we're in "refreshonly" mode,
+        # we need to return the target command here.  If we don't,
+        # then Puppet will generate an event indicating that this
+        # property has changed.
+        return resource[:command]
+      end
+
+      # if we're not in refreshonly mode, then we return nil,
+      # which will cause Puppet to sync this property.  This
+      # is what we want if there is no 'unless' value specified.
+      return nil
+    end
+
+    output, status = run_unless_sql_command(resource[:unless])
+
+    if status != 0
+      self.fail("Error evaluating 'unless' clause: '#{output}'")
+    end
+    result_count = output.strip.to_i
+    if result_count > 0
+      # If the 'unless' query returned rows, then we don't want to execute
+      # the 'command'.  Returning the target 'command' here will cause
+      # Puppet to treat this property as already being 'insync?', so it
+      # won't call the setter to run the 'command' later.
+      return resource[:command]
+    end
+
+    # Returning 'nil' here will cause Puppet to see this property
+    # as out-of-sync, so it will call the setter later.
+    nil
+  end
+
+  def command=(val)
+    output, status = run_sql_command(val)
+
+    if status != 0
+      self.fail("Error executing SQL; psql returned #{status}: '#{output}'")
+    end
+  end
+
+
+  def run_unless_sql_command(sql)
+    # for the 'unless' queries, we wrap the user's query in a 'SELECT COUNT',
+    # which makes it easier to parse and process the output.
+    run_sql_command('SELECT COUNT(*) FROM (' <<  sql << ') count')
+  end
+
+  def run_sql_command(sql)
+    Puppet::Util::SUIDManager.
+        run_and_capture('psql -t -c "' << sql.gsub('"', '\"') << '"', resource[:psql_user], resource[:psql_group])
+  end
+
+end

lib/puppet/type/postgresql_psql.rb

@@ -0,0 +1,74 @@
+Puppet::Type.newtype(:postgresql_psql) do
+
+  newparam(:name) do
+    desc "An arbitrary tag for your own reference; the name of the message."
+    isnamevar
+  end
+
+  newproperty(:command) do
+    desc 'The SQL command to execute via psql.'
+
+    defaultto { @resource[:name] }
+
+    def sync(refreshing = false)
+      # We're overriding 'sync' here in order to do some magic
+      # in support of providing a 'refreshonly' parameter.  This
+      # is kind of hacky because the logic for 'refreshonly' is
+      # spread between the type and the provider, but this is
+      # the least horrible way that I could determine to accomplish
+      # it.
+      #
+      # Note that our overridden version of 'sync' takes a parameter,
+      # 'refreshing', which the parent version doesn't take.  This
+      # allows us to call the sync method directly from the 'refresh'
+      # method, and then inside of the body of 'sync' we can tell
+      # whether or not we're refreshing.
+
+      if ((@resource[:refreshonly] == :false) || refreshing)
+        # If we're not in 'refreshonly' mode, or we're not currently
+        # refreshing, then we just call the parent method.
+        super()
+      else
+        # If we get here, it means we're in 'refreshonly' mode and
+        # we're not being called by the 'refresh' method, so we
+        # just no-op.  We'll be called again by the 'refresh'
+        # method momentarily.
+        nil
+      end
+    end
+  end
+
+  newparam(:unless) do
+    desc "An optional SQL command to execute prior to the main :command; " +
+        "this is generally intended to be used for idempotency, to check " +
+        "for the existence of an object in the database to determine whether " +
+        "or not the main SQL command needs to be executed at all."
+  end
+
+  newparam(:db) do
+    desc "The name of the database to execute the SQL command against."
+  end
+
+  newparam(:psql_user) do
+    desc "The system user account under which the psql command should be executed."
+    defaultto("postgres")
+  end
+
+  newparam(:psql_group) do
+    desc "The system user group account under which the psql command should be executed."
+    defaultto("postgres")
+  end
+
+  newparam(:refreshonly) do
+    desc "If 'true', then the SQL will only be executed via a notify/subscribe event."
+
+    defaultto(:false)
+  end
+
+  def refresh()
+    # All of the magic for this type is attached to the ':command' property, so
+    # we just need to sync it to accomplish a 'refresh'.
+    self.property(:command).sync(true)
+  end
+
+end

manifests/database.pp

@@ -31,19 +31,21 @@ define postgresql::database(
 
   $createdb_command = "${postgresql::params::createdb_path} --template=template0 --encoding '$charset' $locale_option '$dbname'"
 
+  postgresql_psql { "Check for existence of db '$dbname'":
+    command => "SELECT 1",
+    unless  => "SELECT datname FROM pg_database WHERE datname='$dbname'",
+  } ~>
+
   exec { $createdb_command :
-    unless  => "${postgresql::params::psql_path} --command=\"SELECT datname FROM pg_database WHERE datname=\'$dbname\' \" --pset=tuples_only | grep -q $dbname",
+    refreshonly => true,
     user    => 'postgres',
-  }
+  } ~>
 
   # This will prevent users from connecting to the database unless they've been
   #  granted privileges.
-  postgresql::psql {"REVOKE CONNECT ON DATABASE $dbname FROM public":
+  postgresql_psql {"REVOKE CONNECT ON DATABASE $dbname FROM public":
     db          => 'postgres',
-    user        => 'postgres',
-    unless      => 'SELECT 1 where 1 = 0',
     refreshonly => true,
-    subscribe   => Exec[$createdb_command],
   }
 
 }

manifests/database_grant.pp

@@ -49,9 +49,9 @@ define postgresql::database_grant(
     default => $privilege,
   }
 
-  postgresql::psql {"GRANT $privilege ON database $db TO $role":
+  postgresql_psql {"GRANT $privilege ON database $db TO $role":
     db           => $psql_db,
-    user    => $psql_user,
+    psql_user    => $psql_user,
     unless       => "SELECT 1 WHERE has_database_privilege('$role', '$db', '$unless_privilege')",
   }
 }

manifests/role.pp

@@ -32,9 +32,9 @@ define postgresql::role(
   $superuser_sql  = $superuser  ? { true => 'SUPERUSER' , default => 'NOSUPERUSER' }
 
   # TODO: FIXME: Will not correct the superuser / createdb / createrole / login status of a role that already exists
-  postgresql::psql {"CREATE ROLE ${username} ENCRYPTED PASSWORD '${password_hash}' $login_sql $createrole_sql $createdb_sql $superuser_sql":
+  postgresql_psql {"CREATE ROLE ${username} ENCRYPTED PASSWORD '${password_hash}' $login_sql $createrole_sql $createdb_sql $superuser_sql":
     db           => $db,
-    user    => 'postgres',
+    psql_user    => 'postgres',
     unless       => "SELECT rolname FROM pg_roles WHERE rolname='$username'",
   }
 }

spec/manifests/test_ruby_psql.pp

@@ -0,0 +1,30 @@
+# puppet-postgresql
+# For all details and documentation:
+# http://github.com/inkling/puppet-postgresql
+#
+# Copyright 2012- Inkling Systems, Inc.
+#
+# 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.
+
+class postgresql_tests::test_ruby_psql($command = $title, $unless) {
+
+  include postgresql::server
+
+  postgresql_psql { $title:
+    db          => 'postgres',
+    psql_user   => 'postgres',
+    command     => $command,
+    unless      => $unless,
+    require     => Class['postgresql::server'],
+  }
+}

spec/postgresql_spec.rb

@@ -50,7 +50,7 @@ describe "postgresql" do
     # Sahara ignores :cwd so we have to chdir for now, see https://github.com/jedi4ever/sahara/issues/9
     Dir.chdir(vagrant_dir)
 
-    # @env.cli("destroy") # Takes too long
+    @env.cli("destroy --force") # Takes too long
     @env.cli("up")
     
     # We are not testing the "package" resource type, so pull stuff in in advance
@@ -89,20 +89,35 @@ describe "postgresql" do
       # A bare-minimum class to add a DB to postgres, which will be running due to ubuntu
       test_class = 'class {"postgresql_tests::test_db": db => "postgresql_test_db" }'
 
+      begin
         # Run once to check for crashes
-      sudo_and_log("puppet apply -e '#{test_class}'")
+        sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'; [ $? == 2 ]")
 
         # Run again to check for idempotence
         sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'")
 
         # Check that the database name is present
         sudo_and_log('sudo -u postgres psql postgresql_test_db --command="select datname from pg_database limit 1"')
+      ensure
+        sudo_and_log('sudo -u postgres psql --command="drop database postgresql_test_db" postgres')
+      end
     end
   end
 
   describe 'postgresql::psql' do
+    it 'should emit a deprecation warning' do
+      test_class = 'class {"postgresql_tests::test_psql": command => "SELECT * FROM pg_datbase limit 1", unless => "SELECT 1 WHERE 1=1" }'
+
+      data = sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'; [ $? == 2 ]")
+
+      data.should match /postgresql::psql is deprecated/
+
+    end
+  end
+
+  describe 'postgresql_psql' do
     it 'should run some SQL when the unless query returns no rows' do
-      test_class = 'class {"postgresql_tests::test_psql": command => "SELECT \'foo\'", unless => "SELECT 1 WHERE 1=2" }'
+      test_class = 'class {"postgresql_tests::test_ruby_psql": command => "SELECT 1", unless => "SELECT 1 WHERE 1=2" }'
 
       # Run once to get all packages set up
       sudo_and_log("puppet apply -e '#{test_class}'")
@@ -112,7 +127,7 @@ describe "postgresql" do
     end
 
     it 'should not run SQL when the unless query returns rows' do
-      test_class = 'class {"postgresql_tests::test_psql": command => "SELECT * FROM pg_datbase limit 1", unless => "SELECT 1 WHERE 1=1" }'
+      test_class = 'class {"postgresql_tests::test_ruby_psql": command => "SELECT * FROM pg_datbase limit 1", unless => "SELECT 1 WHERE 1=1" }'
 
       # Run once to get all packages set up
       sudo_and_log("puppet apply -e '#{test_class}'")
@@ -121,14 +136,6 @@ describe "postgresql" do
       sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'")
     end
 
-    it 'should emit a deprecation warning' do
-      test_class = 'class {"postgresql_tests::test_psql": command => "SELECT * FROM pg_datbase limit 1", unless => "SELECT 1 WHERE 1=1" }'
-
-      data = sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'; [ $? == 2 ]")
-
-      data.should match /postgresql::psql is deprecated/
-
-    end
   end
 
   describe 'postgresql::user' do