postgresql::server::grant with ensure => absent uses REVOKE instead of GRANT - update SQL as recommended

Author: George Hansper

manifests/server/grant.pp

@@ -14,17 +14,15 @@
 ) {
 
   case $ensure {
-    'present': {
+    default: {
+      # default is 'present'
       $sql_command = 'GRANT %s ON %s "%s" TO "%s"'
       $unless_is = true
     }
     'absent': {
       $sql_command = 'REVOKE %s ON %s "%s" FROM "%s"'
       $unless_is = false
     }
-    default: {
-      fail("Unknown value for ensure '${ensure}'.")
-    }
   }
 
   $group     = $postgresql::server::group
@@ -254,40 +252,44 @@
         if $_privilege == 'ALL' or $_privilege == 'ALL PRIVILEGES' {
           # GRANT ALL
           $custom_unless = "SELECT 1 FROM
-             ( SELECT tablename FROM pg_catalog.pg_tables
-               WHERE schemaname = '${schema}' EXCEPT
-                SELECT table_name FROM
-                (SELECT table_name,count(privilege_type) FROM information_schema.role_table_grants
-                  WHERE grantee = '${role}' AND table_schema = '${schema}'
-                  AND privilege_type IN ('SELECT','UPDATE','INSERT','DELETE','TRIGGER','REFERENCES','TRUNCATE')
-                  GROUP BY table_name
-                ) AS P WHERE P.count >= 7
-             ) AS TBLS HAVING TBLS.count=0"
+             ( SELECT 1 FROM pg_catalog.pg_tables AS t,
+               (VALUES ('SELECT'), ('UPDATE'), ('INSERT'), ('DELETE'), ('TRIGGER'), ('REFERENCES'), ('TRUNCATE')) AS p(privilege_type)
+               WHERE t.schemaname = '${schema}'
+                 AND NOT EXISTS (
+                   SELECT 1 FROM information_schema.role_table_grants AS g
+                   WHERE g.grantee = '${role}'
+                     AND g.table_schema = '${schema}'
+                     AND g.privilege_type = p.privilege_type
+                   )
+             ) AS privs_missing HAVING privs_missing.count=0"
 
         } else {
           # GRANT $_privilege
           $custom_unless = "SELECT 1 FROM
-             ( SELECT tablename FROM pg_catalog.pg_tables
-               WHERE schemaname = '${schema}' EXCEPT
-                 SELECT table_name FROM
-                 information_schema.role_table_grants
-                 WHERE grantee = '${role}' AND table_schema = '${schema}' AND privilege_type = '${_privilege}'
-             ) AS TBLS HAVING TBLS.count=0"
+             ( SELECT 1 FROM pg_catalog.pg_tables AS t
+               WHERE t.schemaname = '${schema}'
+                 AND NOT EXISTS (
+                   SELECT 1 FROM information_schema.role_table_grants AS g
+                   WHERE g.grantee = '${role}'
+                     AND g.table_schema = '${schema}'
+                     AND g.privilege_type = '${_privilege}'
+                   )
+             ) AS tbls HAVING tbls.count=0"
         }
       } else {
         if $_privilege == 'ALL' or $_privilege == 'ALL PRIVILEGES' {
           # REVOKE ALL
           $custom_unless = "SELECT 1 FROM
              ( SELECT table_name FROM information_schema.role_table_grants
                WHERE grantee = '${role}' AND table_schema ='${schema}'
-             ) AS TBLS HAVING TBLS.count=0"
+             ) AS tbls HAVING tbls.count=0"
         } else {
           # REVOKE $_privilege
           $custom_unless = "SELECT 1 FROM
              ( SELECT table_name FROM information_schema.role_table_grants
                WHERE grantee = '${role}' AND table_schema ='${schema}'
                AND privilege_type = '${_privilege}'
-             ) AS TBLS HAVING TBLS.count=0"
+             ) AS tbls HAVING tbls.count=0"
         }
       }
 
@@ -332,8 +334,8 @@
   $_unless = $unless_function ? {
       false    => undef,
       'custom' => $custom_unless,
-      default  => "SELECT * FROM ${unless_function}('${role}',
-                  '${_granted_object}', '${unless_privilege}') WHERE ${unless_function} = ${unless_is}",
+      default  => "SELECT 1 WHERE ${unless_function}('${role}',
+                  '${_granted_object}', '${unless_privilege}') = ${unless_is}",
   }
 
   $_onlyif = $onlyif_function ? {

spec/acceptance/server/grant_spec.rb

@@ -321,7 +321,7 @@ class { 'postgresql::server': }
       it 'should grant select on a table to a user' do
         begin
           pp = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'grant select on test_tbl':
               privilege   => 'SELECT',
               object_type => 'TABLE',
@@ -334,7 +334,7 @@ class { 'postgresql::server': }
           EOS
 
           pp_revoke = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'revoke select on test_tbl':
               ensure      => absent,
               privilege   => 'SELECT',
@@ -346,17 +346,17 @@ class { 'postgresql::server': }
                                Postgresql::Server::Role[$user], ]
             }
           EOS
-  
+
           apply_manifest(pp_install, :catch_failures => true)
-  
+
           #postgres version
           result = shell('psql --version')
           version = result.stdout.match(%r{\s(\d\.\d)})[1]
-  
+
           if version >= '9.0'
             apply_manifest(pp, :catch_failures => true)
             apply_manifest(pp, :catch_changes => true)
-  
+
             ## Check that the privilege was granted to the user
             psql("-d #{db} --tuples-only --command=\"SELECT * FROM has_table_privilege('#{user}', 'test_tbl', 'SELECT')\"", user) do |r|
               expect(r.stdout).to match(/t/)
@@ -374,11 +374,11 @@ class { 'postgresql::server': }
           end
         end
       end
-  
+
       it 'should grant update on all tables to a user' do
         begin
           pp = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'grant update on all tables':
               privilege   => 'UPDATE',
               object_type => 'ALL TABLES IN SCHEMA',
@@ -391,7 +391,7 @@ class { 'postgresql::server': }
           EOS
 
           pp_revoke = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'revoke update on all tables':
               ensure      => absent,
               privilege   => 'UPDATE',
@@ -403,17 +403,17 @@ class { 'postgresql::server': }
                                Postgresql::Server::Role[$user], ]
             }
           EOS
-  
+
           apply_manifest(pp_install, :catch_failures => true)
-  
+
           #postgres version
           result = shell('psql --version')
           version = result.stdout.match(%r{\s(\d\.\d)})[1]
-  
+
           if version >= '9.0'
             apply_manifest(pp, :catch_failures => true)
             apply_manifest(pp, :catch_changes => true)
-  
+
             ## Check that all privileges were granted to the user
             psql("-d #{db} --command=\"SELECT table_name,privilege_type FROM information_schema.role_table_grants
                   WHERE grantee = '#{user}' AND table_schema = 'public'\"", user) do |r|
@@ -437,7 +437,7 @@ class { 'postgresql::server': }
       it 'should grant all on all tables to a user' do
         begin
           pp = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'grant all on all tables':
               privilege   => 'ALL',
               object_type => 'ALL TABLES IN SCHEMA',
@@ -450,7 +450,7 @@ class { 'postgresql::server': }
           EOS
 
           pp_revoke = pp_create_table + <<-EOS.unindent
-  
+
             postgresql::server::grant { 'revoke all on all tables':
               ensure      => absent,
               privilege   => 'ALL',
@@ -462,17 +462,17 @@ class { 'postgresql::server': }
                                Postgresql::Server::Role[$user], ]
             }
           EOS
-  
+
           apply_manifest(pp_install, :catch_failures => true)
-  
+
           #postgres version
           result = shell('psql --version')
           version = result.stdout.match(%r{\s(\d\.\d)})[1]
-  
+
           if version >= '9.0'
             apply_manifest(pp, :catch_failures => true)
             apply_manifest(pp, :catch_changes => true)
-  
+
             ## Check that all privileges were granted to the user
             psql("-d #{db} --tuples-only --command=\"SELECT table_name,count(privilege_type) FROM information_schema.role_table_grants
                   WHERE grantee = '#{user}' AND table_schema = 'public'

spec/unit/defines/server/grant_spec.rb

@@ -50,7 +50,7 @@
     it { is_expected.to contain_postgresql_psql('grant:test').with(
       {
         'command' => /GRANT USAGE ON SEQUENCE "test" TO\s* "test"/m,
-        'unless'  => /SELECT [*] FROM has_sequence_privilege\('test',\s* 'test', 'USAGE'\) WHERE has_sequence_privilege = true/m,
+        'unless'  => /SELECT 1 WHERE has_sequence_privilege\('test',\s* 'test', 'USAGE'\) = true/m,
       }
     ) }
   end