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

all resources: Grant object owners to connected user when needed. #146

Merged
merged 5 commits into from
Jul 17, 2020

Conversation

cyrilgdn
Copy link
Contributor

@cyrilgdn cyrilgdn commented Jun 7, 2020

This should fix most of permissions problems the provider has when running against RDS databases (and maybe other Cloud provider, but didn't test yet)

This also adds a new run of integration tests against a RDS-like Postgres.

Some tests are disabled for this environment, either because it really need to be superuser (e.g. to create a new superuser) or because the environment is not totally similar as RDS (e.g.: for the extensions, I need to check how I can reproduce RDS behavior).

This replaces #139 and #135
Thanks to @slocke716 and @rimmington for your work on these problems that help me to create a more generic fix. If you have time to test this PR on your environment to assert it fixes your use cases, and even more to make a review of this PR, it would be very helpful 😊 .

This (hopefully) fixes #36

@slocke716
Copy link

This should fix most of permissions problems the provider has when running against RDS databases (and maybe other Cloud provider, but didn't test yet)

This also adds a new run of integration tests against a RDS-like Postgres.

Some tests are disabled for this environment, either because it really need to be superuser (e.g. to create a new superuser) or because the environment is not totally similar as RDS (e.g.: for the extensions, I need to check how I can reproduce RDS behavior).

This replaces #139 and #135
Thanks to @slocke716 and @rimmington for your work on these problems that help me to create a more generic fix. If you have time to test this PR on your environment to assert it fixes your use cases, and even more to make a review of this PR, it would be very helpful .

This (hopefully) fixes #36

Sorry for the dreadfully late reply. Do you still need testing against an rds instance? If so, is it sufficient to just spin up a "free-tier" and point the tests to this or are you looking for something more detailed?

@cyrilgdn
Copy link
Contributor Author

Sorry for the dreadfully late reply. Do you still need testing against an rds instance? If so, is it sufficient to just spin up a "free-tier" and point the tests to this or are you looking for something more detailed?

@slocke716 It was more if you can easily test and assert it fixes the specific use case you created your PR for.

We're using this patched version against RDS so I know it works but maybe you have a specific use case that we are not covering neither does the acceptance tests.

cyrilgdn and others added 5 commits July 17, 2020 10:04
If the configured admin user in the provider is not superuser,
we need to temporarily grant to it the roles owning objects we are modifying.
We fixed a lot of problems for the non-superuser RDS `postgres` role
so it's the best way to test it.
It tries to connect psql instead of just parsing logs
so we are sure database is ready.
Co-authored-by: Joe Marty <joe.marty@smartersorting.com>
@cyrilgdn cyrilgdn merged commit 67b50a9 into master Jul 17, 2020
@cyrilgdn cyrilgdn deleted the grant-rds branch July 17, 2020 08:20
@cyrilgdn
Copy link
Contributor Author

FYI, this has just been released in v1.7.0.

@mltsy
Copy link
Contributor

mltsy commented Sep 1, 2020

It works! Thank you so much for this amazing birthday gift! 🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not destroy postgresql_role after create
4 participants