-
Notifications
You must be signed in to change notification settings - Fork 2.8k
add section for managed databases and related perms (close #1677, close #3783) #5228
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
Conversation
|
Deploy preview for hasura-docs ready! Built with commit 5d59563 |
allpwrfulroot
left a comment
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.
Extra thanks for the GCP note at the end!
5cf6fce to
f84e01d
Compare
| GRANT ALL ON ALL SEQUENCES IN SCHEMA <schema-name> TO hasurauser; | ||
| GRANT ALL ON ALL FUNCTIONS IN SCHEMA <schema-name> TO hasurauser; | ||
| Note for managed databases (AWS RDS, GCP Cloud SQL, etc.) |
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.
This needs to be a sub-section of the the permissions section.
Also having a heading with "Note for" seems odd. Maybe something like "Caveats for managed databases (..."?
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 wouldn't call it a caveat. Just something extra to do or ignore.
| -- GRANT USAGE ON SCHEMA <schema-name> TO hasurauser; | ||
| -- GRANT ALL ON ALL TABLES IN SCHEMA <schema-name> TO hasurauser; | ||
| -- GRANT ALL ON ALL SEQUENCES IN SCHEMA <schema-name> TO hasurauser; | ||
| GRANT USAGE ON SCHEMA <schema-name> TO hasurauser; |
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.
Copy-pasting the entire block as it is would cause errors, so shouldn't these lines be left commented?
|
|
||
| .. admonition:: Google Cloud SQL | ||
|
|
||
| On Google Cloud SQL, running ``ALTER SCHEMA hdb_catalog OWNER TO hasurauser;`` may give you an error ``ERROR: must be member of role "hasurauser"``. You can fix this by running ``GRANT hasurauser to postgres;`` first, assuming "postgres" is the superuser that you are running the commands with. |
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.
Can this be kept consistent with the above section. I dont see a need for this to be a note without the code-blocks just because this is specific for Google Cloud SQL. We can just say, "In Google Cloud SQL you might see the following error:" followed but the code-block style we have used above
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.
Added above
a5dd977 to
9d4227c
Compare
|
Review app https://hge-ci-pull-5228.herokuapp.com is deleted |
|
Sharing an edge case: Granting SELECT privilege on one table in public is sufficient to have the SELECT privilege on pg_catalog and information_schema, as explained by @tirumaraiselvan |
Description
Postgres by default has a PUBLIC keyword used for grants: https://www.postgresql.org/docs/current/sql-grant.html. PUBLIC has SELECT permission on many tables in pg_catalog and information_schema by default and hence all users have select access to these (unless specifically revoked?)
We can check table access for all users via:
This means on AWS RDS (and others), we expect to not give any special privileges to pg_catalog or information_schema. I have tested on few major versions on AWS RDS and GCP as well and indeed this is the case.
close #1677, close #3783
Changelog
CHANGELOG.mdis updated with user-facing content relevant to this PR. If no changelog is required, then add theno-changelog-requiredlabel.Affected components