Skip to content

Commit 89ad955

Browse files
author
George Hansper
committed
postgresql::server::grant with ensure => absent uses REVOKE instead of GRANT - update SQL as recommended
1 parent c603966 commit 89ad955

File tree

3 files changed

+45
-43
lines changed

3 files changed

+45
-43
lines changed

‎manifests/server/grant.pp

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,15 @@
1414
) {
1515

1616
case $ensure {
17-
'present': {
17+
default: {
18+
# default is 'present'
1819
$sql_command = 'GRANT %s ON %s "%s" TO "%s"'
1920
$unless_is = true
2021
}
2122
'absent': {
2223
$sql_command = 'REVOKE %s ON %s "%s" FROM "%s"'
2324
$unless_is = false
2425
}
25-
default: {
26-
fail("Unknown value for ensure '${ensure}'.")
27-
}
2826
}
2927

3028
$group = $postgresql::server::group
@@ -254,40 +252,44 @@
254252
if $_privilege == 'ALL' or $_privilege == 'ALL PRIVILEGES' {
255253
# GRANT ALL
256254
$custom_unless = "SELECT 1 FROM
257-
( SELECT tablename FROM pg_catalog.pg_tables
258-
WHERE schemaname = '${schema}' EXCEPT
259-
SELECT table_name FROM
260-
(SELECT table_name,count(privilege_type) FROM information_schema.role_table_grants
261-
WHERE grantee = '${role}' AND table_schema = '${schema}'
262-
AND privilege_type IN ('SELECT','UPDATE','INSERT','DELETE','TRIGGER','REFERENCES','TRUNCATE')
263-
GROUP BY table_name
264-
) AS P WHERE P.count >= 7
265-
) AS TBLS HAVING TBLS.count=0"
255+
( SELECT 1 FROM pg_catalog.pg_tables AS t,
256+
(VALUES ('SELECT'), ('UPDATE'), ('INSERT'), ('DELETE'), ('TRIGGER'), ('REFERENCES'), ('TRUNCATE')) AS p(privilege_type)
257+
WHERE t.schemaname = '${schema}'
258+
AND NOT EXISTS (
259+
SELECT 1 FROM information_schema.role_table_grants AS g
260+
WHERE g.grantee = '${role}'
261+
AND g.table_schema = '${schema}'
262+
AND g.privilege_type = p.privilege_type
263+
)
264+
) AS privs_missing HAVING privs_missing.count=0"
266265

267266
} else {
268267
# GRANT $_privilege
269268
$custom_unless = "SELECT 1 FROM
270-
( SELECT tablename FROM pg_catalog.pg_tables
271-
WHERE schemaname = '${schema}' EXCEPT
272-
SELECT table_name FROM
273-
information_schema.role_table_grants
274-
WHERE grantee = '${role}' AND table_schema = '${schema}' AND privilege_type = '${_privilege}'
275-
) AS TBLS HAVING TBLS.count=0"
269+
( SELECT 1 FROM pg_catalog.pg_tables AS t
270+
WHERE t.schemaname = '${schema}'
271+
AND NOT EXISTS (
272+
SELECT 1 FROM information_schema.role_table_grants AS g
273+
WHERE g.grantee = '${role}'
274+
AND g.table_schema = '${schema}'
275+
AND g.privilege_type = '${_privilege}'
276+
)
277+
) AS tbls HAVING tbls.count=0"
276278
}
277279
} else {
278280
if $_privilege == 'ALL' or $_privilege == 'ALL PRIVILEGES' {
279281
# REVOKE ALL
280282
$custom_unless = "SELECT 1 FROM
281283
( SELECT table_name FROM information_schema.role_table_grants
282284
WHERE grantee = '${role}' AND table_schema ='${schema}'
283-
) AS TBLS HAVING TBLS.count=0"
285+
) AS tbls HAVING tbls.count=0"
284286
} else {
285287
# REVOKE $_privilege
286288
$custom_unless = "SELECT 1 FROM
287289
( SELECT table_name FROM information_schema.role_table_grants
288290
WHERE grantee = '${role}' AND table_schema ='${schema}'
289291
AND privilege_type = '${_privilege}'
290-
) AS TBLS HAVING TBLS.count=0"
292+
) AS tbls HAVING tbls.count=0"
291293
}
292294
}
293295

@@ -332,8 +334,8 @@
332334
$_unless = $unless_function ? {
333335
false => undef,
334336
'custom' => $custom_unless,
335-
default => "SELECT * FROM ${unless_function}('${role}',
336-
'${_granted_object}', '${unless_privilege}') WHERE ${unless_function} = ${unless_is}",
337+
default => "SELECT 1 WHERE ${unless_function}('${role}',
338+
'${_granted_object}', '${unless_privilege}') = ${unless_is}",
337339
}
338340

339341
$_onlyif = $onlyif_function ? {

‎spec/acceptance/server/grant_spec.rb

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ class { 'postgresql::server': }
321321
it 'should grant select on a table to a user' do
322322
begin
323323
pp = pp_create_table + <<-EOS.unindent
324-
324+
325325
postgresql::server::grant { 'grant select on test_tbl':
326326
privilege => 'SELECT',
327327
object_type => 'TABLE',
@@ -334,7 +334,7 @@ class { 'postgresql::server': }
334334
EOS
335335

336336
pp_revoke = pp_create_table + <<-EOS.unindent
337-
337+
338338
postgresql::server::grant { 'revoke select on test_tbl':
339339
ensure => absent,
340340
privilege => 'SELECT',
@@ -346,17 +346,17 @@ class { 'postgresql::server': }
346346
Postgresql::Server::Role[$user], ]
347347
}
348348
EOS
349-
349+
350350
apply_manifest(pp_install, :catch_failures => true)
351-
351+
352352
#postgres version
353353
result = shell('psql --version')
354354
version = result.stdout.match(%r{\s(\d\.\d)})[1]
355-
355+
356356
if version >= '9.0'
357357
apply_manifest(pp, :catch_failures => true)
358358
apply_manifest(pp, :catch_changes => true)
359-
359+
360360
## Check that the privilege was granted to the user
361361
psql("-d #{db} --tuples-only --command=\"SELECT * FROM has_table_privilege('#{user}', 'test_tbl', 'SELECT')\"", user) do |r|
362362
expect(r.stdout).to match(/t/)
@@ -374,11 +374,11 @@ class { 'postgresql::server': }
374374
end
375375
end
376376
end
377-
377+
378378
it 'should grant update on all tables to a user' do
379379
begin
380380
pp = pp_create_table + <<-EOS.unindent
381-
381+
382382
postgresql::server::grant { 'grant update on all tables':
383383
privilege => 'UPDATE',
384384
object_type => 'ALL TABLES IN SCHEMA',
@@ -391,7 +391,7 @@ class { 'postgresql::server': }
391391
EOS
392392

393393
pp_revoke = pp_create_table + <<-EOS.unindent
394-
394+
395395
postgresql::server::grant { 'revoke update on all tables':
396396
ensure => absent,
397397
privilege => 'UPDATE',
@@ -403,17 +403,17 @@ class { 'postgresql::server': }
403403
Postgresql::Server::Role[$user], ]
404404
}
405405
EOS
406-
406+
407407
apply_manifest(pp_install, :catch_failures => true)
408-
408+
409409
#postgres version
410410
result = shell('psql --version')
411411
version = result.stdout.match(%r{\s(\d\.\d)})[1]
412-
412+
413413
if version >= '9.0'
414414
apply_manifest(pp, :catch_failures => true)
415415
apply_manifest(pp, :catch_changes => true)
416-
416+
417417
## Check that all privileges were granted to the user
418418
psql("-d #{db} --command=\"SELECT table_name,privilege_type FROM information_schema.role_table_grants
419419
WHERE grantee = '#{user}' AND table_schema = 'public'\"", user) do |r|
@@ -437,7 +437,7 @@ class { 'postgresql::server': }
437437
it 'should grant all on all tables to a user' do
438438
begin
439439
pp = pp_create_table + <<-EOS.unindent
440-
440+
441441
postgresql::server::grant { 'grant all on all tables':
442442
privilege => 'ALL',
443443
object_type => 'ALL TABLES IN SCHEMA',
@@ -450,7 +450,7 @@ class { 'postgresql::server': }
450450
EOS
451451

452452
pp_revoke = pp_create_table + <<-EOS.unindent
453-
453+
454454
postgresql::server::grant { 'revoke all on all tables':
455455
ensure => absent,
456456
privilege => 'ALL',
@@ -462,17 +462,17 @@ class { 'postgresql::server': }
462462
Postgresql::Server::Role[$user], ]
463463
}
464464
EOS
465-
465+
466466
apply_manifest(pp_install, :catch_failures => true)
467-
467+
468468
#postgres version
469469
result = shell('psql --version')
470470
version = result.stdout.match(%r{\s(\d\.\d)})[1]
471-
471+
472472
if version >= '9.0'
473473
apply_manifest(pp, :catch_failures => true)
474474
apply_manifest(pp, :catch_changes => true)
475-
475+
476476
## Check that all privileges were granted to the user
477477
psql("-d #{db} --tuples-only --command=\"SELECT table_name,count(privilege_type) FROM information_schema.role_table_grants
478478
WHERE grantee = '#{user}' AND table_schema = 'public'

‎spec/unit/defines/server/grant_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
it { is_expected.to contain_postgresql_psql('grant:test').with(
5151
{
5252
'command' => /GRANT USAGE ON SEQUENCE "test" TO\s* "test"/m,
53-
'unless' => /SELECT [*] FROM has_sequence_privilege\('test',\s* 'test', 'USAGE'\) WHERE has_sequence_privilege = true/m,
53+
'unless' => /SELECT 1 WHERE has_sequence_privilege\('test',\s* 'test', 'USAGE'\) = true/m,
5454
}
5555
) }
5656
end

0 commit comments

Comments
 (0)