terraform-provider-postgresql
terraform-provider-postgresql copied to clipboard
Making possible to specify default 'role' parameter for the user
This PR adds possibility to set the default role parameter for the user.
This is effectively the following statement on database level:
ALTER ROLE role_name SET role TO role_parameter_value
This is my first PR on github, so apologies if something is not filled in - just let me know and I'll fix this.
As for the code, it is my first golang code as well (yay!) - so I tried to stick to style of other parts of the code as closely as possible.
Looking at the code, it might be a good idea to somehow unify handling of default parameters - if there will be more. I'm willing to grab the subject in the future.
@cyrilgdn Not sure if I have to create Issue ticket for that PR as well... So, ping! Hope you get this notification.
We'd really love to have this feature in the provider :)
Hi @wimi ,
Thanks for opening this PR!
To be honest I don't know what's the role parameter is for a user :) ? Could you explain to me what it does ? (I don't find the documentation)
I need to think a bit about this because it's not the first time we want to configure a role parameter (there's also #132 ), maybe we could define some kind of map of parameters.
Hi @cyrilgdn!
Role parameter allows you to change the identifier of the currently logged in user (https://www.postgresql.org/docs/current/sql-set-role.html). This is useful when you want to implement seamless user/password rotation scenario. If you need more details about it, here is a blog where it is described: https://www.jannikarndt.de/blog/2018/08/rotating_postgresql_passwords_with_no_downtime/
I've been looking at the code and currently there are already two parameters set for the user: search_path and statement_timeout. Each of those have its own property in TF Role resource. That's why I went the same route with role attribute - as an additional property in Role.
I was also thinking about implementing a map of parameters for role, but in this case we'd have legacy properties search_path/statement_timeout and new "role_properties" map. We could implement some sanity check to allow using only legacy approach or new one, but not both at the same time. This approach will certainly take longer to implement than what I did so far though :) I am also not 100% sure about the data types of each of parameter, that might be a little bit problematic as well.
So maybe instead of implementing generic parameters map we could still have individual properties - added on a "as needed basis", but with some generic handling for these in the code. Right now each of those parameters seem to be handled by very similar code. If you agree with than - I can take a look at how we can generalize this and make code more reusable for future extensibility.
Hi @cyrilgdn,
Actually, I think I implemented a rough idea of setting a map of default parameters for role. It is still in early WIP phase, but I think it already has a structure we can discuss. It also coexists with parameters already set by standalone role properties.
Can we discuss my idea? Here is a draft PR for that: #155
I am really interested in pushing this subject - as we'd like to have this feature this way or another.