ui: hide reset options for non-admins #95303
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
maryliag
force-pushed
the
admin-checks
branch
from
9ae563c to
20eaf6d
Compare
THardy98
approved these changes
Jan 16, 2023
There was a problem hiding this comment.
Reviewed 15 of 15 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @maryliag)
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx line 220 at r1 (raw file):
isTenant: false, hasViewActivityRedactedRole: false, hasAdminRole: false,
Do we need these default props when they're already stored as part of the UIConfigState ?
maryliag
force-pushed
the
admin-checks
branch
from
20eaf6d to
036d56d
Compare
maryliag
commented
Jan 16, 2023
There was a problem hiding this comment.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx line 220 at r1 (raw file):
Previously, THardy98 (Thomas Hardy) wrote…
Do we need these default props when they're already stored as part of the
UIConfigState?
good point, we're not using this, so I remove here and in a few other places.
maryliag
force-pushed
the
admin-checks
branch
from
036d56d to
8e6d3f8
Compare
maryliag
force-pushed
the
admin-checks
branch
from
8e6d3f8 to
0c3e0e2
Compare
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
force-pushed
the
admin-checks
branch
from
0c3e0e2 to
2884bb0
Compare
|
TFTR! |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 2884bb0 to blathers/backport-release-22.2-95303: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
maryliag
added a commit
to maryliag/cockroach
that referenced
this pull request
Jan 18, 2023
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 pull request
Jan 19, 2023
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
maryliag
added a commit
to maryliag/cockroach
that referenced
this pull request
Jan 19, 2023
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 pull request
Jan 19, 2023
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 statsandreset index statsfrom the Console when the user is a non-admin.