Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypass role grant logic for pg_database_owner role #316

Closed
wants to merge 1 commit into from
Closed

Bypass role grant logic for pg_database_owner role #316

wants to merge 1 commit into from

Conversation

joshsouza
Copy link

See issue #301. This appears to solve the issue, but may have unanticipated side-effects.

This is my incredibly naive attempt at fixing the issue here. I don't understand enough of what is being done here and why to understand if this is a terrible idea, or what limitations this will introduce. I imagine that without this, if Terraform is using a role that doesn't have proper permissions to grant permissions, it will fail (but for a different reason), which may be why this code was originally in place.

I have tried this out by debugging locally with this change in place on a state impacted by the failure in #301 and it was able to apply for my state without a problem.

I would love to get any feedback on what this PR would need to be up to necessary standards to be pulled in.

See issue #301. This appears to solve the issue, but may have unanticipated side-effects.
@romanstingler
Copy link

@cyrilgdn can we prioritize that, as it is a blocker for POSTGRES15

@mattthaber
Copy link

mattthaber commented Jul 19, 2023

@cyrilgdn (and @kylejohnson, not sure if you are a new maintainer?) any idea when this would get in? Or when supporting pg_database_owner will be implemented?

This is a pretty major blocker if people cant upgrade their PG versions

@cyrilgdn
Copy link
Owner

Hi,

I had a look last Sunday week-end but I'm don't think thix fix is enough, I'll try to confirm that next week-end and to enhance the fix if needed.

@cyrilgdn cyrilgdn self-assigned this Jul 20, 2023
@fjlopezs
Copy link

Hi @cyrilgdn. Just for curiosity, have you had a chance to take a look at this?

@ToniRib
Copy link

ToniRib commented Aug 2, 2023

Also following along on this thread. This is currently a blocker for us to stand up postgres 15 databases on AWS Aurora. Basically it means teams who need a new database must use 14.x, which just means we're going to have to deal with more upgrades in the future.

@tarvip
Copy link

tarvip commented Aug 8, 2023

I tested this fix with Google Cloud SQL PostgreSQL 15 and it seems to work fine.

@ThomasKasene
Copy link

I'm no expert on this, but should the change introduced in this PR only apply when PostgreSQL versions >= 14? (Or 15? Although I think the role was introduced in PostgreSQL 14.)

That way it won't introduce any (unlikely) side-effects on earlier versions.

@joshsouza
Copy link
Author

joshsouza commented Aug 14, 2023

I'm no expert on this, but should the change introduced in this PR only apply when PostgreSQL versions >= 14? (Or 15? Although I think the role was introduced in PostgreSQL 14.)

That way it won't introduce any (unlikely) side-effects on earlier versions.

This change would only apply to databases with the pg_database_owner role. Which is any database PSQL 14 and up (since it's created by default), or any older database where someone chose to create a role by that name (possible). So, side-effects are possible, though I would hope that creating a role with the same name on a lower version would mean that bypassing the logic that this does would remain valid. Obviously that's an assumption though.

I am also not sure that this is actually the ideal "fix" to the problem, it's my best attempt to work around an issue with no knowledge on what's really happening or intended here.

So short answer: Yeah, that is probably a reasonable check that could be added here, but rather than add more complex logic to the fix I'm proposing, I'd like to give @cyrilgdn time/space to assess the root cause and determine if this is the right approach, or if there's a more complete solution.

@jindrichskupa
Copy link

@cyrilgdn Please, we are blocked on this. We can't manage new databases or migrate to 15.

@andyhuynh3
Copy link

Running into this issue as well when trying to create Aurora Postgres 15.x instances on AWS.

@cyrilgdn
Copy link
Owner

cyrilgdn commented Sep 9, 2023

Replaced by #348

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

Successfully merging this pull request may close these issues.

10 participants