puppetlabs/puppetlabs-postgresql

commas separated privileges for default_privileges doesn't work as expected.

momo57420 opened this issue · 2 comments

Describe the Bug

I tried to set several privileges as default privileges for a role in a schema but it failed in error during execution (Illegal value for $privilege parameter.)

Expected Behavior

ALTER DEFAULT PRIVILEGES FOR ROLE xyz_dml IN SCHEMA xyz_sch GRANT INSERT,SELECT,UPDATE,DELETE ON TABLES TO "xyz_aoo"

Steps to Reproduce

Hi there,

I have created a type called t_default_privileges

type Db_profile::Postgresql::T_default_privileges =
  Struct[
    user => String,
    ensure => Enum['present','absent'],
    db => String,
    owner => Optional[String],
    privilege => String,
    schema => String,
    object_type => Enum['FUNCTIONS','ROUTINES','SEQUENCES','TABLES','TYPES'],
  ]

Then i have created a class default_privileges

class db_profile::postgresql::server::default_privileges (
  Boolean $is_primary = $db_profile::postgresql::server::is_primary,
  Optional[Array[Db_profile::Postgresql::T_default_privileges]]
    $default_privileges      = $db_profile::postgresql::server::default_privileges,
) {
  Anchor['postgresql::server::service::end']
  -> Class['db_profile::postgresql::server::default_privileges']

  if $is_primary {
    each($default_privileges) |$default_privilege| {

      notify {"Running DDP ----> Custom ------> $default_privilege":}

      if $default_privilege['ensure'] == 'present' {
        postgresql::server::default_privileges{$default_privilege['user']:
          target_role => $default_privilege['owner'],
          ensure => 'present',
          db  => $default_privilege['db'],
          role   => $default_privilege['user'],
          privilege => $default_privilege['privilege'],
          schema => $default_privilege['schema'],
          object_type => $default_privilege['object_type'],
        }
       }else{
        postgresql::server::default_privileges{$default_privilege['user']:
          target_role => $default_privilege['owner'],
          ensure => 'absent',
          db  => $default_privilege['db'],
          role   => $default_privilege['user'],
          privilege => $default_privilege['privilege'],
          schema => $default_privilege['schema'],
          object_type => $default_privilege['object_type'],
        }
      }
    }
  }
}

Then in hiera, i have defined

db_profile::postgresql::server::default_privileges:
    - user: xyz_aoo
      ensure: present
      db: xyz
      privilege: INSERT,SELECT,UPDATE,DELETE
      owner: xyz_dml 
      schema: xyz_sch
      object_type: TABLES

The result is that it goes directly in default: { fail('Illegal value for $privilege parameter') } while testing $_privilege in /manifest/servers/default_privileges.pp

    'TABLES': {
      case $_privilege {
        /^ALL$/: { $_check_privilege = 'arwdDxt' }
        /^DELETE$/: { $_check_privilege = 'd' }
        /^INSERT$/: { $_check_privilege = 'a' }
        /^REFERENCES$/: { $_check_privilege = 'x' }
        /^SELECT$/: { $_check_privilege = 'r' }
        /^TRIGGER$/: { $_check_privilege = 'd' }
        /^TRUNCATE$/: { $_check_privilege = 'D' }
        /^UPDATE$/: { $_check_privilege = 'w' }
        default: { fail('Illegal value for $privilege parameter') }
      }

It seems that the regexp used does not match expression with comma separated values if $_privilege is build like priv1,priv2,priv3 etc.

If i change fail by notify in default then there is a problem with the unless command as it has not retrieved the correct $_check_privilege variable.
But the grant_command is correct with ALTER DEFAULT PRIVILEGES FOR ROLE xyz_dml IN SCHEMA xyz_sch GRANT INSERT,SELECT,UPDATE,DELETE ON TABLES TO "xyz_aoo"'

However, it is stated in the header
# @param privilege Specifies comma-separated list of privileges to grant. Valid options: depends on object type.

With only one privilege it is working, with several ones separated by commas, it doesnt 't work.

Am i doing something wrong ?

Thanks

Environment

Red Hat 8.7
Postgres 13

Hi @momo57420 - Thanks for raising this, this is interesting and looks like a genuine bug with the "$privilege" parameter.

Just from a quick glance at a high level, i would say the '$privilege' parameter and respective documenation needs updating to also accept an array of strings and perform some logic to join those privileges as a single comma-separated list. I'll label this issue so it can get properly prioritised by the team (or if you're feeling up to it, we are accepting PRs).

Thanks again.

Hi there,

In the meantime, as a workaround, I replaced in manifest/servers/default_privileges.pp the regexp with this one :

    'TABLES': {
      case $_privilege {
        /^ALL$/: { $_check_privilege = 'arwdDxt' }
        /(SELECT|INSERT|UPDATE|DELETE|REFERENCES|TRIGGER|TRUNCATE)(,)*/: { $_check_privilege = 'rwdDxt' }
        #/^DELETE$/: { $_check_privilege = 'd' }
        #/^INSERT$/: { $_check_privilege = 'a' }
        #/^REFERENCES$/: { $_check_privilege = 'x' }
        #/^SELECT$/: { $_check_privilege = 'r' }
        #/^TRIGGER$/: { $_check_privilege = 'd' }
        #/^TRUNCATE$/: { $_check_privilege = 'D' }
        #/^UPDATE$/: { $_check_privilege = 'w' }
        default: { fail('Illegal value for $privilege parameter') }
      }
      $_check_type = 'r'

The only problem is with $check_privilege, it should be set with a combination of privileges declared.
Maybe with a split function to construct an array and test each position of the array.

But even if the $check_privilege is not correct, it is an unless cmd.
So it will execute the sql command anyway as the unless cmd is false.

Thanks