terraform-provider-postgresql icon indicating copy to clipboard operation
terraform-provider-postgresql copied to clipboard

Support for ALL PROCEDURES and ALL ROUTINES

Open awendland opened this issue 4 years ago • 4 comments

First off, thank you @cyrilgdn for maintaining this terraform provider! It's been such a wonderful way to declaratively manage Postgres roles and privileges.

Feature:

In version 11, Postgres separated procedures out from functions in grant commands. Because of this change it's no longer possible to manage procedures using this provider. The docs state: "ALL FUNCTIONS also affects aggregate functions, but not procedures, again just like the specific-object GRANT command. Use ALL ROUTINES to include procedures." (source).

I'd like to help extend this provider to support ALL PROCEDURES and ALL ROUTINES. I'm not particularly familiar with golang or terraform providers, so any guidance/assistance would be appreciated.

Plan:

As far as I can tell the main areas that would need to be updated are:

  • https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant.go#L349
  • https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant.go#L382
  • https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant.go#L16
  • https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/helpers.go#L242

awendland avatar Jun 11 '21 14:06 awendland

This only applies to the GRANT command. ALTER DEFAULT PRIVILEGES actually treats procedures and functions as the same thing:

The words FUNCTIONS and ROUTINES are equivalent in this command. (ROUTINES is preferred going forward as the standard term for functions and procedures taken together. In earlier PostgreSQL releases, only the word FUNCTIONS was allowed. It is not possible to set default privileges for functions and procedures separately.)

(postgres 12 docs)

awendland avatar Jun 11 '21 15:06 awendland

Hi @awendland,

Thanks for opening this issue and for your reasearch on this subject.

I'd like to help extend this provider to support ALL PROCEDURES and ALL ROUTINES.

Your help is welcome, if I read correctly we could replace FUNCTION by ROUTINE and it will be manage both functions and routines no? Of course if you need to separate FUNCTION and PROCEDURE you can do it. But then you'll need to specify the value of prokind where the provider reads the pg_proc table (which contains the definition and ACLs of all function and routines): https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant.go#L266 (value of prokind should be something like f for function and p for procedure if I remember correctly)

Your update plan is good, but there's at least 2 more points that you need to manage:

  • Compatibility with older versions of Postgres. We have a features version map, you can check how it's used for existing defined features.
  • Fix and add the necessary tests for new features: https://github.com/cyrilgdn/terraform-provider-postgresql/blob/master/postgresql/resource_postgresql_grant_test.go#L373

Of course I'd be happy to help your for all of that. So you can create draft PR with the modifications you mentioned, test if it works, fix the tests if broken and ping me so I can already have a look.

cyrilgdn avatar Jun 11 '21 16:06 cyrilgdn

I'm also interested in this and started a PR in https://github.com/cyrilgdn/terraform-provider-postgresql/pull/128. Please let me know if it's missing anything!

alec-rabold avatar Aug 25 '21 19:08 alec-rabold

Did #128 resolve this issue? If so can we close this out.

AndrewJackson2020 avatar May 10 '24 02:05 AndrewJackson2020