-
Notifications
You must be signed in to change notification settings - Fork 60
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
K8SPG-571 grant user access to public schema #1097
base: main
Are you sure you want to change the base?
Conversation
internal/postgres/users.go
Outdated
var err error | ||
var stdout string | ||
var stderr string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did these variables need to be defined here? We can simply do stdout, stderr, err := exec...
, right?
internal/postgres/users.go
Outdated
for i := range users { | ||
user := users[i] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use the way to iterate on elements of an array without using indexes:
for _, user := range users { ...
// Grant a specific user access to the public schema in every database where they already have permissions. | ||
// +optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend this comment with a pointer to the databases field to increase clarity:
// Grant the user access to the public schema in each database listed under `databases`.
// +optional
|
||
// Grant a specific user access to the public schema in every database where they already have permissions. | ||
// +optional | ||
GrantPublicSchemaAccess *bool `json:"grantPublicSchemaAccess,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name seems fine to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the changes with Natali, I'm happy with them.
@@ -19655,6 +19655,10 @@ spec: | |||
type: string | |||
type: array | |||
x-kubernetes-list-type: set | |||
grantPublicSchemaAccess: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nmarukovich please confirm this name with our PM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, variable name make sense
commit: 0819c1b |
CHANGE DESCRIPTION
Problem:
By default, when user is created for a specific database, it only has access to a schema matching its username. However, in some cases, granting access to the public schema may also be necessary. To accommodate this need, the option to enable public schema access for users was added.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc
) and QA (Needs QA
)?Tests
Config/Logging/Testability