Fix incorrectly quoted GRANT cmd on functions

[olifre]

The existing code failed miserably to grant permissions on functions.

Using the following code:

postgresql::server::grant { "grant_func_privs":
  role                    => 'rewind',
  db                      => 'postgres',
  privilege               => 'EXECUTE',
  object_type             => 'FUNCTION',
  object_name             => 'pg_catalog.pg_ls_dir',
  object_arguments        => ['text', 'boolean', 'boolean'],
  require                 => Postgresql::Server::Role[$rewind_user],
}

The created queries were:

GRANT EXECUTE ON FUNCTION "pg_catalog.pg_ls_dir"("text","boolean","boolean") TO "rewind";

and for the "unless" case:

SELECT COUNT(*) FROM (SELECT 1 WHERE has_function_privilege('rewind', 'pg_catalog.pg_ls_dir("text","boolean","boolean")', 'EXECUTE') = true);"

The patches in this series fix that to be:

GRANT EXECUTE ON FUNCTION pg_catalog.pg_ls_dir(text,boolean,boolean) TO "rewind";
SELECT COUNT(*) FROM (SELECT 1 WHERE has_function_privilege('rewind', 'pg_catalog.pg_ls_dir(text,boolean,boolean)', 'EXECUTE') = true);"

since both the types and the complete function expression must not be quoted.

[puppet-community-rangefinder]

postgresql::server::grant is a type

The enclosing module is declared in 50 of 578 indexed public Puppetfiles.

Breaking changes to this file WILL impact these modules (exact match):

• landcareresearch-ckan

---

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.

[sanfrancrisko]

Thanks for the fix @olifre ! I noticed there's a typo causing syntax check failures. Also, after checking out and running tests, there are going to be some test failures due to the change in syntax of the sql command:

bundle exec rake spec

Failed examples:

rspec ./spec/unit/defines/server/grant_spec.rb:263 # postgresql::server::grant function is expected to contain Postgresql_psql[grant:test] with command =~ /GRANT EXECUTE ON FUNCTION "test"\("text","text"\) TO\s* "test"/m and unless =~ /SELECT 1 WHERE has_function_privilege\('test',\s* 'test\("text","text"\)', 'EXECUTE'\)/m
rspec ./spec/unit/defines/server/grant_spec.rb:288 # postgresql::server::grant function with schema is expected to contain Postgresql_psql[grant:test] with command =~ /GRANT EXECUTE ON FUNCTION "myschema"."test"\("text","text"\) TO\s* "test"/m and unless =~ /SELECT 1 WHERE has_function_privilege\('test',\s* 'myschema.test\("text","text"\)', 'EXECUTE'\)/m

Could you have a look at the tests within grant_spec.rb? It would be good if we could add additional coverage to handle scenarios where we want to quote the command vs scenarios where we do not.

Please feel free to reach out if you need any help in that regard.

Thanks again for the PR!

[olifre]

Thanks for the friendly feedback!
I was sadly unable to run PDK on my platform with this module. It seems something is very broken my platform (Gentoo), since even after running:

pdk bundle install --path vendor/bundle

(which succeeds fine and installs the dependencies), trying to validate or unit test will make PDK attempt to reinstall all dependencies with sudo, completely ignoring they are already there in vendor/bundle.

Unless you have an idea what is wrong (or can RTFM me), I could only develop the tests "blindly" (using travis-feedback). I can certainly do that, but it may take a bit.

[olifre]

@sanfrancrisko That should have done the trick to get the Static and Spec parts green.

I can also look at the acceptance tests, they should run through fine now, but they only ever worked since they never tested granting permissions to a function taking parameters, so it would be better to add something like this to accepteance tests (I can try tomorrow, hopefully won't take too many iterations).

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@508571a). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1150   +/-   ##
=========================================
  Coverage          ?   50.43%           
=========================================
  Files             ?       15           
  Lines             ?      458           
  Branches          ?        0           
=========================================
  Hits              ?      231           
  Misses            ?      227           
  Partials          ?        0

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 508571a...681fba1. Read the comment docs.

[olifre]

I did now also add an acceptance test for granting EXECUTE to functions with arguments.
Must have been beginner's luck, it seems to have played out well on my second attempt (and that was my first acceptance test ever) ;-).

[sanfrancrisko]

Thanks for updating the current tests and adding the new coverage too @olifre. Appreciate you doing this as your first acceptance test! 😄

Change LGTM!

Regarding the issues you were seeing with the PDK - I asked some of my more PDK-savvy colleagues what they thought might be the issue. A few suggestions:

• Drop the --path parameter from the bundle install command and let the PDK handle this
• If you're using Bundler >= 2 you can set the path (seeing as using --path is now deprecated) globally: pdk bundle config set path <PATH> (FWIW, I have it set it to .bundle)

Before running pdk bundle install and pdk test unit, I would recommend removing the Gemfile.lock and vendor directory from your repo. Let me know how it works out.

[olifre]

@sanfrancrisko Thanks for the friendly feedback and the PDK help!
Sadly, it fails for me — I have PDK 1.8.0.0 (latest packaged version for Gentoo) and Bunder 1.17.2 (newer versions are available up to 2.1.4, but I should probably not mix them with an old PDK).

After cleaning up (removing {{Gemfile.lock}} and {{vendor}}) and then running:

pdk bundle install

it tries to run sudo and wants to overwrite my system's ruby installation :-(.
For the config (even with bundler 2) I would expect it won't work since PDK sets BUNDLE_IGNORE_CONFIG: 1 (as I see in the debug output) so any config would be ignored — no? Potentially, that's also why the path setting in the older bundler version fails.