sql: admins now have full access to all external connections #155657
Merged
Conversation
This file contains hidden or 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 |
|
In a follow-up will address the issue of |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where admin users could not see external connections created by non-admin users when running SHOW EXTERNAL CONNECTIONS. The fix ensures that admins now have full access to all external connections by automatically granting ALL privileges to the admin role for external connection objects.
- Modified privilege cache logic to grant admin users ALL privileges on external connections
- Added comprehensive test coverage to verify admin access and non-admin isolation
- Restructured existing privilege handling code to use a switch statement for better maintainability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/sql/syntheticprivilegecache/cache.go | Updated privilege handling to grant admin users ALL privileges on external connections and refactored conditional logic into a switch statement |
| pkg/sql/show_external_connection_test.go | Added new test file to verify that admin users can see all external connections while non-admin users cannot see others' connections |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
kev-cao
force-pushed
the
sql/external-conn-admin
branch
from
48f9257 to
5706ec1
Compare
Previously, if a non-admin user were to create an external connection, that connection would not show up in the output of `SHOW EXTERNAL CONNECTIONS` that was run by an admin. This patch fixes this bug so that admins can now see/use all external connections. Fixes: cockroachdb#146773 Release note: Admin users now have full access to all external connections.
kev-cao
force-pushed
the
sql/external-conn-admin
branch
from
5706ec1 to
ae3c866
Compare
|
TFTR! bors r=rafiss |
craig bot
pushed a commit
that referenced
this pull request
Oct 21, 2025
155411: restore: prevent scatter of non-empty ranges in online restore r=jeffswenson a=kev-cao In conventional restore, we prevent the scattering of non-empty ranges by passing in a max size for the `AdminScatter` request. In online restore, while we expect to almost always be scattering empty ranges, due to the fact that the link phase can retry, it is possible to scatter ranges that contain external SSTs. This breaks our golden rule of only scattering empty ranges. This commit teaches online restore to only ever scatter empty ranges. Fixes: #144599 Release note: None 155653: roachtest: run INSPECT as background job and poll for completion r=spilchen a=spilchen Long-running INSPECT statements (e.g., >3h) have previously failed due to connection resets, despite no server crash. This likely stems from a missed keep-alive or a client timeout, though the exact cause is unclear. This change updates the roachtest to: - Run the INSPECT statement as a background job using a short statement_timeout (5s). - Poll the job table for job completion instead of waiting synchronously. - Report progress at 10% intervals to improve visibility without overwhelming the logs. Fixes #155610 Release note: none Epic: none 155657: sql: admins now have full access to all external connections r=rafiss a=kev-cao Previously, if a non-admin user were to create an external connection, that connection would not show up in the output of `SHOW EXTERNAL CONNECTIONS` that was run by an admin. This patch fixes this bug so that admins can now see/use all external connections. Fixes: #146773 Release note: Admin users now have full access to all external connections. Co-authored-by: Kevin Cao <[email protected]> Co-authored-by: Matt Spilchen <[email protected]>
|
Build failed (retrying...): |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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, if a non-admin user were to create an external connection, that connection would not show up in the output of
SHOW EXTERNAL CONNECTIONSthat was run by an admin.This patch fixes this bug so that admins can now see/use all external connections.
Fixes: #146773
Release note: Admin users now have full access to all external connections.