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.

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

Remove requirement for Admin role. #79571

Closed
data-matt opened this issue Apr 7, 2022 · 2 comments · Fixed by #110609
Closed

Remove requirement for Admin role. #79571

data-matt opened this issue Apr 7, 2022 · 2 comments · Fixed by #110609
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@data-matt
Copy link

data-matt commented Apr 7, 2022

Is your feature request related to a problem? Please describe.
Our users can't use the admin account.

No feature should be gated behind the admin.

Every feature should be grantable, via a role.

In theory, a user with access to all roles, should be equivalent to admin.

Describe the solution you'd like
Every line of code that is gated by things like isAdmin must have an equivalent when we call method hasRoleOption(ctx, userName, roleoption)

Describe alternatives you've considered
Giving out admin, which is untenable.

Jira issue: CRDB-14911
Epic CRDB-17640

@data-matt data-matt added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Apr 7, 2022
@mwang1026 mwang1026 removed their assignment Apr 7, 2022
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 19, 2022
@mwang1026
Copy link

Done with sync

@exalate-issue-sync exalate-issue-sync bot changed the title Remove requirement for Admin role. Remove requirement for Admin role. May 25, 2022
@cockroachdb cockroachdb deleted a comment from mari-crl May 25, 2022
@rafiss
Copy link
Collaborator

rafiss commented Jun 27, 2022

The internal “isAdmin” functions/names in our code could be changed to include the word "unsafe" or "deprecated" or something like that.

@rafiss rafiss assigned RichardJCai and unassigned abhinavg6 Jun 27, 2022
@exalate-issue-sync exalate-issue-sync bot assigned rafiss and unassigned RichardJCai Oct 4, 2022
@craig craig bot closed this as completed in 235babd Sep 22, 2023
craig bot pushed a commit that referenced this issue Nov 27, 2023
114861: server: remove admin checks from server-v2 r=rafiss a=rafiss

These are now replaced by checks for the VIEWCLUSTERMETADATA privilege. The equivalent change was already made on the v1 server.

The following endpoints are affected (these endpoints are not used in the frontend, so this change does not have any user-facing impact):
- /sessions/
- /nodes/
- /nodes/{node_id}/ranges/
- /ranges/{range_id:[0-9]+}/
- /events/

informs #114384
informs #79571
informs #109814
Release note: None

115000: streamingccl: only return lag replanning error if lagging node has not advanced r=stevendanna a=msbutler

Previously, the frontier processor would return a lag replanning error if it detected a lagging node and after the hwm had advanced during the flow. This implies the frontier processor could replan as soon as a lagging node finished its catchup scan and bumped the hwm, but was still far behind the other nodes, as we observed in #114706. Ideally, the frontier processor should not throw this replanning error because the lagging node is making progress and because replanning can cause repeated work.

This patch prevents this scenario by teaching the frontier processor to only throw a replanning error if:
- the hwm has advanced in the flow
- two consecutive lagging node checks detected a lagging node and the hwm has not advanced during those two checks.

Informs #114706

Release note: none

115046: catalog/lease: fix flake for TestTableCreationPushesTxnsInRecentPast r=fqazi a=fqazi

Previously, TestTableCreationPushesTxnsInRecentPast could flake because we attempted to increase odds of hitting the uncertainty interval error by adding a delay on KV RPC calls. This wasn't effective and could still intermittent failures, and instead we are going to directly modify the uncertainty interval by setting a large MaxOffset on the clock. This will cause the desired behaviour in a more deterministic way.

Fixes: #114366

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants