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.

Sign up for GitHub

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

Jump to bottom

DBConsole SQL Activity shows 'reset SQL stats' for unprivileged users #95213

Closed
fabiog1901 opened this issue Jan 13, 2023 — with Slack · 1 comment · Fixed by #95303 or #95461
Closed

DBConsole SQL Activity shows 'reset SQL stats' for unprivileged users #95213

fabiog1901 opened this issue Jan 13, 2023 — with Slack · 1 comment · Fixed by #95303 or #95461
Assignees
@maryliag
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@fabiog1901 Slack
Copy link
Contributor

fabiog1901 commented Jan 13, 2023 •
edited by cockroach-jira-scripts
Loading

In the DB Console, a user with VIEWACTIVITY can click on the "reset sql stats" link in the SQL Activity page, but it has no effect. Yet, the UI doesn't display that the cause is a lack of permission.

Jira issue: CRDB-23406
@blathers-crl
Copy link

blathers-crl bot commented Jan 13, 2023

Hi @fabiog1901, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@fabiog1901 fabiog1901 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jan 13, 2023
@maryliag maryliag self-assigned this Jan 16, 2023
@maryliag maryliag mentioned this issue Jan 16, 2023
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 16, 2023
@maryliag
ui: hide reset options for non-admins
20eaf6d 
Previously, we were showing reset sql stats and reset
index stats options for all users. If a non-admin user tried
to reset, it wasn't doing the reset as expected, but no
message was being displayed.
This commit now hides these options for non-admin so it's
no longer confusing to see the options but not being
able to use it.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` and `reset index stats`
from the Console when the user is a non-admin.
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 16, 2023
@maryliag
ui: hide reset options for non-admins
036d56d 
Previously, we were showing reset sql stats and reset
index stats options for all users. If a non-admin user tried
to reset, it wasn't doing the reset as expected, but no
message was being displayed.
This commit now hides these options for non-admin so it's
no longer confusing to see the options but not being
able to use it.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` and `reset index stats`
from the Console when the user is a non-admin.
@maryliag maryliag mentioned this issue Jan 16, 2023
Merged
craig bot pushed a commit that referenced this issue Jan 16, 2023
@maryliag
Merge #95303
43cd78e 
95303: ui: hide reset options for non-admins r=maryliag a=maryliag

Previously, we were showing reset sql stats and reset index stats options for all users. If a non-admin user tried to reset, it wasn't doing the reset as expected, but no message was being displayed.
This commit now hides these options for non-admin so it's no longer confusing to see the options but not being able to use it.

Fixes #95213

https://www.loom.com/share/d82672e9ec994a6e9200fd9094ee9b55

Release note (ui change): Remove `reset sql stats` and `reset index stats` from the Console when the user is a non-admin.

Co-authored-by: maryliag <[email protected]>
@craig craig bot closed this as completed in 2884bb0 Jan 17, 2023
@maryliag maryliag mentioned this issue Jan 17, 2023
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 17, 2023
@maryliag
ui: hide reset options for non-admins
422176b 
Previously, we were showing reset sql stats and reset
index stats options for all users. If a non-admin user tried
to reset, it wasn't doing the reset as expected, but no
message was being displayed.
This commit now hides these options for non-admin so it's
no longer confusing to see the options but not being
able to use it.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` and `reset index stats`
from the Console when the user is a non-admin.
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 17, 2023
@maryliag
ui: hide reset options for non-admins
ab384e5 
Previously, we were showing reset sql stats and reset
index stats options for all users. If a non-admin user tried
to reset, it wasn't doing the reset as expected, but no
message was being displayed.
This commit now hides these options for non-admin so it's
no longer confusing to see the options but not being
able to use it.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` and `reset index stats`
from the Console when the user is a non-admin.
@maryliag maryliag mentioned this issue Jan 18, 2023
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 18, 2023
@maryliag
ui: remove reset sql stats for non-admin
4b6d9d8 
Continuation from cockroachdb#95303

The previous PR missed the reset on the Transactions tab.
This PR removes the reset sql stats for non-admin users.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` from Transactions
page for non-admins.
craig bot pushed a commit that referenced this issue Jan 19, 2023
@maryliag @adityamaru @aliher1911
Merge #95461 #95466 #95467 #95504 #95512
f27fa9d 
95461: ui: remove reset sql stats for non-admin r=maryliag a=maryliag

Continuation from #95303

The previous PR missed the reset on the Transactions tab. This PR removes the reset sql stats for non-admin users.

Fixes #95213

Release note (ui change): Remove `reset sql stats` from Transactions page for non-admins.

95466: ingesting: fixup privileges granted during database restore r=rafiss a=adityamaru

Previously, all schemas and tables that were ingested as part of a database restore would "inherit" the privileges of the database. The database would be granted `CONNECT` for the `public` role and `ALL` to `admin` and `root`, and so all ingested schemas would have `ALL` for `admin` and `root`. Since 21.2 we have moved away from tables/schemas inheriting privileges from the parent database and so this logic is stale and partly incorrect. It is incorrect because the restored `public` schema does not have `CREATE` and `USAGE` granted to the `public` role. These privileges are always granted to `public` schemas of a database and so there is a discrepancy in restore's behaviour.

This change simplifies the logic to grant schemas and tables their default set of privileges if ingested via a database restore. It leaves the logic for cluster and table restores unchanged.

Release note (bug fix): fixes a bug where a database restore would not grant `CREATE` and `USAGE` on the public schema to the public role

Fixes: #95456

95467: schemachanger: a bunch of small fixes r=postamar a=postamar

Informs #88294.

Release note: None

95504: schemachangerccl: rename generated tests r=postamar a=postamar

This commit adds an underscore in the generated tests' name where there previously was one missing.

Informs #88294.

Release note: None

95512: kvserver: fix flaky TestLearnerReplicateQueueRace test r=tbg a=aliher1911

Test was not expecting raft snapshots and injected failure at wrong moments.

Fixes #94993

Release note: None

Co-authored-by: maryliag <[email protected]>
Co-authored-by: adityamaru <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>
This was referenced Jan 19, 2023
Merged
Merged
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 19, 2023
@maryliag
ui: remove reset sql stats for non-admin
2cc3e77 
Continuation from cockroachdb#95303

The previous PR missed the reset on the Transactions tab.
This PR removes the reset sql stats for non-admin users.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` from Transactions
page for non-admins.
maryliag added a commit to maryliag/cockroach that referenced this issue Jan 19, 2023
@maryliag
ui: remove reset sql stats for non-admin
3c2b503 
Continuation from cockroachdb#95303

The previous PR missed the reset on the Transactions tab.
This PR removes the reset sql stats for non-admin users.

Fixes cockroachdb#95213

Release note (ui change): Remove `reset sql stats` from Transactions
page for non-admins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maryliag maryliag

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Milestone
No milestone
2 participants
@maryliag @fabiog1901