syntheticprivilege: admin always has ALL global privileges #110976
Conversation
As we move away from requiring the admin role to perform cluster debug/repair operations, we want to use a privilege instead. To facilitate that, the admin role now implicitly has ALL global privileges. The privilege for admins is not revokeable. Release note: None
Since we document privileges on the GlobalPrivilegeObject using the phrase "system privilege", we should make the error message say that too. Release note: None
|
This change is |
There was a problem hiding this comment.
One minor test comment, not sure if its feasible
Reviewed 1 of 1 files at r1, 12 of 12 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @adityamaru and @rafiss)
pkg/sql/syntheticprivilegecache/cache.go line 176 at r2 (raw file):
// Admin always has ALL global privileges. if spo.GetObjectType() == privilege.Global { privDesc.Grant(username.AdminRoleName(), privilege.List{privilege.ALL}, true)
As a sanity, is there some show statement that will display this that we can pop into a logic test?
There was a problem hiding this comment.
tftr!
bors r+
Reviewable status:
pkg/sql/syntheticprivilegecache/cache.go line 176 at r2 (raw file):
Previously, fqazi (Faizan Qazi) wrote…
As a sanity, is there some show statement that will display this that we can pop into a logic test?
the logic tests already are updated in this PR
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
syntheticprivilege: admin always has ALL global privileges
As we move away from requiring the admin role to perform cluster
debug/repair operations, we want to use a privilege instead. To
facilitate that, the admin role now implicitly has ALL global
privileges. The privilege for admins is not revokeable.
sql: use better error message for missing system privilege
Since we document privileges on the GlobalPrivilegeObject using the
phrase "system privilege", we should make the error message say that
too.
informs #109814
Release note: None