Grant all tables in schema fixup #1096
Conversation
postgresql::server::grant is a typeThe enclosing module is declared in 51 of 576 indexed public Puppetfiles. that may have no external impact to Forge modules. These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report. Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only. |
|
Thanks for submitting the PR @georgehansper. |
spec/acceptance/server/grant_spec.rb
Outdated
| @@ -263,7 +277,7 @@ class { 'postgresql::server': } | |||
| EOS | |||
|
|
|||
| if Gem::Version.new(postgresql_version) >= Gem::Version.new('9.0') | |||
| idempotent_apply(pp) | |||
| apply_manifest(pp, expect_changes: true) | |||
There was a problem hiding this comment.
Could you please add the idempotent_apply function back since we need to test the idempotency also which applies the manifest twice.
There was a problem hiding this comment.
I have update the tests to use the idempotent_apply() function.
idempotent_apply() is done in 2 stages, because the 1st one sets up the table permissions, and the 2nd one (which uses postgresql::server::grant) 'fixes' the permissions.
When this was a single idempotent_apply() the testbench complained because postgresql::server::grant was making changes every time (as it was supposed to).
Let me know if you want further changes made to the tests.
|
Thanks @sheenaajay for merging this PR |
…in_schema_fixup Grant all tables in schema fixup
This pull request addresses the issues in:
In summary, the problem was that the code was working for adding a single privilege or ALL PRIVILEGES a single table but failed to invoke the necessary changes if there was one or more tables already present that had the required single privilege or all privileges.
I have included the code from #1056 verbatim, and adapted it for the case of ALL PRIVILEGES
The code in pull request 1056 is a bit more readable than the original code, I feel.
I have also updated the associated tests in grant_spec.rb to detect the original problem.