[BUG] postgres_privileges.present not idempotent for functions

[OrangeDog]

Description
Using postgres_privileges.present to grant permissions on functions isn't idempotent when following the conventions in the postgres documentation (e.g. https://www.postgresql.org/docs/12/app-pgrewind.html#id-1.9.5.9.8)

It seems it's constructing an invalid query to try to get the current status:

[ERROR] Command '['/usr/bin/psql', '--no-align', '--no-readline', '--no-psqlrc', '--no-password', '--dbname', 'postgres', '-v', 'datestyle=ISO,MDY', '-c', "COPY (SELECT rolname AS name FROM pg_catalog.pg_proc p JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace WHERE nspname = 'public' AND p.oid::regprocedure::text = 'pg_catalog.pg_ls_dir(text, boolean, boolean)' ORDER BY proname, proargtypes) TO STDOUT WITH CSV HEADER"]' failed with return code: 1
[ERROR] stderr: ERROR: column "rolname" does not exist
LINE 1: COPY (SELECT rolname AS name FROM pg_catalog.pg_proc p JOIN ...
HINT: Perhaps you meant to reference the column "p.proname".
[ERROR] retcode: 1
[ERROR] Error connecting to Postgresql server

Setup

{% for fn in (
  'pg_catalog.pg_ls_dir(text, boolean, boolean)',
  'pg_catalog.pg_stat_file(text, boolean)',
  'pg_catalog.pg_read_binary_file(text)',
  'pg_catalog.pg_read_binary_file(text, bigint, bigint, boolean)'
) %}
GRANT EXECUTE ON function {{ fn }} TO rewind_user:
  postgres_privileges.present:
    - name: rewind_user
    - object_name: '{{ fn }}'
    - object_type: function
    - privileges: [ EXECUTE ]
{% endfor %}

Steps to Reproduce the behavior
Apply the state multiple times.

Expected behavior
Should only execute the first time.

Versions Report
PostgreSQL: 12.6-0ubuntu0.20.04.1

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
          Salt: 3002.2

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: 2.0.6
     gitpython: 3.0.7
        Jinja2: 2.10.1
       libgit2: 0.28.3
      M2Crypto: 0.31.0
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: 1.0.3
        Python: 3.8.5 (default, Jul 28 2020, 12:59:40)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2

System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-65-generic
        system: Linux
       version: Ubuntu 20.04 focal

Additional context
Also the documentation of the state doesn't list function as a valid type, but the module documentation does.

[OrangeDog]

A query like this would get you the ACL info from postgres 12

SELECT p.proacl
FROM pg_catalog.pg_proc p JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname = 'pg_catalog' AND p.oid::regprocedure::text = 'pg_ls_dir(text,boolean,boolean)'

Note you have to parse out the nspname and remove the spaces from the input function signature.

Getting a list of roles with execute permissions out of that in an SQL query is very complex. Doing it in python is probably prefered. Or directly query for the intended role:

SELECT p.proacl ~ 'rewind_user=X/postgres'::aclitem AS granted
FROM pg_catalog.pg_proc p JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname = 'pg_catalog' AND p.oid::regprocedure::text = 'pg_ls_dir(text,boolean,boolean)'

However, that will give an error if the role does not exist. It may also not work if the grantor (postgres in this example) is different.

[anitakrueger]

Is there any update on this by any chance?

[OrangeDog]

In general, pg_catalog will have breaking changes between major releases, so this sort of stuff needs a lot of proactive maintenance or alternative methods.

[anitakrueger]

I understand postgres entities aren't easy to manage. I was really just wondering if there was any progress on this or if there is a workaround maybe?
We are trying to manage our postgres privileges with this and have quite a few functions. I honestly can't remember if this worked in 9.6 but we have upgraded to 12.5 now anyways.

[OrangeDog]

The workaround I use is to add state requisites so it only runs when a user is created.
Assuming nobody's going to REVOKE them, that does the job.

[anitakrueger]

Oh you're right. My state only failed the second time I ran it. Well, that works for me for now.
Yea, revoking privs is a whole other ball game.

[OrangeDog]

@anilsil why did you change this to low severity. It's certainly not a cosmetic issue.