Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Add postgresql_grant_role resource #189

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

dvdliao
Copy link
Contributor

@dvdliao dvdliao commented Sep 26, 2020

credit to: @Vince-Chenal

closes: #154

postgresql/resource_postgresql_grant_role.go Outdated Show resolved Hide resolved
}

func resourcePostgreSQLGrantRoleRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*Client)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there not a worry of a panic if the type assertion fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've typically seen providers implemented without a worry here

postgresql/resource_postgresql_grant_role.go Outdated Show resolved Hide resolved
postgresql/resource_postgresql_grant_role.go Outdated Show resolved Hide resolved
postgresql/resource_postgresql_grant_role.go Outdated Show resolved Hide resolved
website/docs/r/postgresql_grant_role.html.markdown Outdated Show resolved Hide resolved
dvdliao and others added 5 commits September 28, 2020 19:54
Co-authored-by: Danny Hermes <daniel.j.hermes@gmail.com>
Co-authored-by: Danny Hermes <daniel.j.hermes@gmail.com>
Co-authored-by: Danny Hermes <daniel.j.hermes@gmail.com>
Co-authored-by: Danny Hermes <daniel.j.hermes@gmail.com>
@wimi
Copy link

wimi commented Oct 1, 2020

Thank you for your PR and work, @Vince-Chenal and @dvdliao!

I just wanted to ask - there is already https://www.terraform.io/docs/providers/postgresql/r/postgresql_role.html#roles, which does something similar, but on a very basic level.

How will this resource coexist with existing property on postgresql_role? I have a feeling that there will be a conflict after terraform refresh or maybe even on plan.

Generally I like the direction - gives more flexibility and might actually solve some issues out of the box (#173).

@dvdliao
Copy link
Contributor Author

dvdliao commented Oct 1, 2020

It should coexist, but not be used together, postgresql_role as it stands now is authoritative, this new resource postgresql_grant_role is not; so it can be used to manage roles that are managed by something else.
For the github issues, the common theme is the postgres_role was created by the cloud provider/console, Therefore managing grants with postgresql_role doesn't make much sense because it also manages the role password and other things which is out of terraform's control and would also have drift.

see something like: https://www.terraform.io/docs/providers/google/r/sql_user.html
The resource in this PR would allow to manage role grants even though the role itself was created by the cloud console api.

heres another example in terraform where we have the same resources that are authoritative and nonauthoritative: https://www.terraform.io/docs/providers/google/r/google_project_iam.html

@wimi
Copy link

wimi commented Oct 2, 2020

Thank you, this is what I meant - these resources should not be used on the same role. Thanks for linking google example, wasn't aware of these kind of cases.
I think slightly more descriptive docs are needed (similarly to what you linked in google - there is a lot of warnings about not using some resources on same object).

@cyrilgdn
Copy link
Contributor

@dvdliao Thanks for your work on this, I'll take a look in the next days.

It's hashicorp stuff that we don't have access to anymore.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to grant role membership to existing user
7 participants