Merge pull request #25 from cprice-puppet/feature/master/port-psql-to-ruby

Initial working implementation of ruby psql type/provider

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

manifests/psql.pp

@@ -26,9 +26,9 @@ define postgresql::psql(
 
   require postgresql::params
 
-  # TODO: FIXME: shellquote does not work, and this regex works for trivial things but not nested escaping.
-  # Need a lexer, preferably a ruby SQL parser to catch errors at catalog time
-  # Possibly https://github.com/omghax/sql ?
+  # TODO: FIXME: shellquote does not work, and this regex works for trivial
+  # things but not nested escaping.  Need a lexer, preferably a ruby SQL parser
+  # to catch errors at catalog time.  Possibly https://github.com/omghax/sql ?
 
   if ($::postgres_default_version != '8.1') {
     $no_password_option = '--no-password'
@@ -38,7 +38,13 @@ define postgresql::psql(
   $quoted_command = regsubst($command, '"', '\\"', 'G')
   $quoted_unless  = regsubst($unless,  '"', '\\"', 'G')
 
-  exec {"/bin/echo \"${quoted_command}\" | ${psql} |egrep -v -q '^$'":
+  $final_cmd = "/bin/echo \"$quoted_command\" | $psql |egrep -v -q '^$'"
+
+  notify { "deprecation warning: $final_cmd":
+    message => "postgresql::psql is deprecated ; please use postgresql_psql instead.",
+  } ->
+
+  exec { $final_cmd:
     cwd         => '/tmp',
     user        => $user,
     returns     => 1,

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}":
-    db      => $db,
-    user    => 'postgres',
-    unless  => "SELECT rolname FROM pg_roles WHERE rolname='${username}'",
+  postgresql_psql {"CREATE ROLE ${username} ENCRYPTED PASSWORD '${password_hash}' $login_sql $createrole_sql $createdb_sql $superuser_sql":
+    db           => $db,
+    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

@@ -32,9 +32,12 @@ describe "postgresql" do
 
   def sudo_and_log(*args)
     @logger.debug("Running command: '#{args[0]}'")
-     @env.primary_vm.channel.sudo(args[0]) do |ch, data|
-       @logger.debug(data)
-     end
+    result = ""
+    @env.primary_vm.channel.sudo(args[0]) do |ch, data|
+      result << data
+      @logger.debug(data)
+    end
+    result
   end
 
   before(:all) do
@@ -47,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
@@ -85,31 +88,46 @@ 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" }'
-  
-      # Run once to check for crashes
-      sudo_and_log("puppet apply -e '#{test_class}'")
-  
-      # 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"')
+      begin
+        # Run once to check for crashes
+        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}'")
-      
+
       # Check for exit code 2
       sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}' ; [ $? == 2 ]")
     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}'")
@@ -117,6 +135,7 @@ describe "postgresql" do
       # Check for exit code 0
       sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'")
     end
+
   end
 
   describe 'postgresql::user' do
@@ -144,8 +163,10 @@ describe "postgresql" do
       # Run again to check for idempotence
       sudo_and_log("puppet apply --detailed-exitcodes -e '#{test_class}'")
   
-      # Check that the user can select from the table in
+      # Check that the user can create a table in the database
       sudo_and_log('sudo -u psql_grant_tester psql --command="create table foo (foo int)" postgres')
+
+      sudo_and_log('sudo -u psql_grant_tester psql --command="drop table foo" postgres')
     end
   end
 end