sql: filter out zones from crdb_internal.zones if user has no privilege#42046
Closed
otan wants to merge 1 commit intocockroachdb:masterfrom
Closed
sql: filter out zones from crdb_internal.zones if user has no privilege#42046otan wants to merge 1 commit intocockroachdb:masterfrom
otan wants to merge 1 commit intocockroachdb:masterfrom
Conversation
Member
fa4d536 to
8b8f309
Compare
Contributor
Author
|
(i think the test is flaky) |
Release note (bug fix): filter out zones from crdb_internal.zones if user has no privileges
8b8f309 to
6de55a1
Compare
otan
commented
Oct 31, 2019
| 22 RANGE liveness | ||
|
|
||
| query TT | ||
| SELECT * FROM [SHOW ALL ZONE CONFIGURATIONS] ORDER BY 1 |
Contributor
Author
There was a problem hiding this comment.
just realised this doesn't work, woops.
Contributor
Author
|
realised that this doesn't actually fix |
craig bot
pushed a commit
that referenced
this pull request
Oct 31, 2019
41914: sqlmigrations: don't run baked-in migrations r=andreimatei a=andreimatei Before this patch, the migration runner had a mechanism by which it attempted to not run migrations that were included in the cluster bootstrap schema. That mechanism was weak. A migration could be marked as includedInBootstrap, and then the migration runner would skip it if the current node had just performed a bootstrap. However, the respective migration would still be run (aimlessly) by a 2nd node joining the cluster, or even by the same node that just skipped them, after a restart. This patch improves the mechanism such that migrations included in the bootstrap image are never run if the cluster's bootstrap version is high enough. It does so by allowing one to mark migrations with the version at which they've been introduced, and the runner will skip them if the cluster's bootstrap version is >=. This means that migrations that want to be marked in this way also need to introduce a cluster version. The benefit of this change, besides faster node startup time, removal of confusing entries from the system.event_log table and more determinism (because now some migrations are never run instead of not run on single node clusters but run on multi-node clusters), is that logic tests that hardcode expected cluster events are more easily written. Before this patch, those tests had to explicitly ignore some events that are generated by migrations (because, again, depending on the configuration, the migration would sometimes run and sometimes not). It's now the second time this happens to a change that I personally want to make, and it's very surprising and scary when it happens to you. You can see the edit in the event_log logic test for a case that I'm not correcting (there's probably others in the same file but I haven't looked). Release note: None. 41991: storage/engine: move MVCCIncrementalIterator from storageccl to engine r=petermattis a=hueypark This is prerequisites for [storage/engine: add Pebble support to ExportToSst](#41739). Recovered some tests from [engine: Add ExportToSst method with all logic moved to C++](75f1314). See #41945 Release note: None 42066: sql: SHOW ZONE CONFIGURATION only if user has permissions r=otan a=otan Previously, SHOW ZONE CONFIGURATION did not respect permissions on tables. Patch that up by checking permissions when using this command. Resolves the command in #40917. Has merge conflicts with #42046; I will fix them up with whatever lands first. Release note (bug fix): fix bug where zones would display in `SHOW ZONE CONFIGURATION` even though the user has no permission to view that table Co-authored-by: Andrei Matei <andrei@cockroachlabs.com> Co-authored-by: Jaewan Park <jaewan.huey.park@gmail.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
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
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.
Addresses the fix suggested by #40917 -- but doesn't actually fix the
SHOW ZONE CONFIGURATIONissue - onlySHOW ALL ZONE CONFIGURATIONS-- i can fix theSHOW ZONEseparately. I still think this fix is worthwhileIf we do not have permissions in a table for a specific zone, do not show it for the crdb_internal.zone virtual table
Release note (bug fix): filter out zones from crdb_internal.zones if user has no privileges