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

Support granular users #19

Merged
merged 10 commits into from
Nov 13, 2019

Conversation

arnarg
Copy link

@arnarg arnarg commented Nov 6, 2019

Closes #10 and #18

I decided to pull the DropOnDelete changes into this one as I needed to change it a little bit with this one.

This pull request changes so that Postgres can now create schemas in a database. It will maintain reader and writer roles for the schemas it is responsible for that we can grant to PostgresUser chosen by the privileges property (OWNER|READ|WRITE).

  • The reader role only has SELECT on all tables in all schemas specified in Postgres.
  • The writer role has SELECT,INSERT,UPDATE,DELETE on all tables in all schemas specified in Postgres.

You can still skip specifying any schemas in Postgres but then you just have to manage readers and writers on your own (or just be owner) as the ones created by the operator will not be aware of the schemas.

I also made sure we don't mutate the spec. I use ArgoCD to maintain our Helm charts and it will be unhappy with any differences in spec.

@arnarg
Copy link
Author

arnarg commented Nov 6, 2019

@hitman99 The postgres package and the reconcile functions are becoming kind of a mess.

I'd like to refactor them a little bit after this pull request.

I want to split pkg/postgres/postgres.go into many smaller files with functions groupped together depending on relation (i.e. database.go, role.go, utils.go, ...).

The reconcile functions are already split into sections (deletion logic, creation logic, ...) I'd like to have those sections become separate isolated functions (maybe in separate files depending on relation).

Would that be ok?

@hitman99
Copy link
Member

hitman99 commented Nov 6, 2019

@arnarg yeah, refactoring is a welcome change! Good call with not mutating the spec, I think that at the time I was writing the Status update logic there was no way of updating just the status.

@hitman99
Copy link
Member

hitman99 commented Nov 6, 2019

@arnarg I'd like to thank you for your contributions and I'm going to make you a maintainer of postgres-operator 😉 But we'll still need to review each other's pull requests before merging 😄

@hitman99
Copy link
Member

hitman99 commented Nov 6, 2019

I'm kind of swamped at the moment, but I'll try to review this PR as soon as I can

@arnarg
Copy link
Author

arnarg commented Nov 6, 2019

I'd like to thank you for your contributions and I'm going to make you a maintainer of postgres-operator

Oh wow! Thanks!

But we'll still need to review each other's pull requests before merging

Of course! 😉 I also really like discussions in issues and pull requests, it makes solving the problems much easier.

I'm kind of swamped at the moment, but I'll try to review this PR as soon as I can

No problem

Arnar Gauti Ingason added 2 commits November 7, 2019 15:27
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.

There should be an option in Postgres CRD to drop database and role on deletion
2 participants