cliccl: add encryption-registry-list command
#89873
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 |
nicktrav
added a commit
to nicktrav/cockroach
that referenced
this pull request
Oct 12, 2022
Remove an obsolete, broken command. The functionality has been superseded by cockroachdb#89095 and cockroachdb#89873. Touches cockroachdb#89095. Epic: None. Release note: None.
This was referenced Oct 12, 2022
jbowens
approved these changes
Oct 13, 2022
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nicktrav)
pkg/storage/pebble_file_registry.go line 539 at r1 (raw file):
r.mu.Lock() defer r.mu.Unlock() return r.mu.entries
r.mu.entries may be mutated after List returns. I realize this likely isn't a problem for the debug command since there shouldn't be any concurrent filesystem operations, but I think to be defensive we should return a copy. Either a copy of the map, or maybe a sorted slice of []*enginepb.FileEntrys?
craig bot
pushed a commit
that referenced
this pull request
Oct 13, 2022
89874: cmdccl: remove `enc_utils` r=jbowens a=nicktrav Remove an obsolete, broken command. The functionality has been superseded by #89095 and #89873. Touches #89095. Epic: None. Release note: None. 89905: multitenant: fix failing distsql test r=cucaroach a=cucaroach Test was erroneously expecting a bundle in the bundle=off config. Fixes: #89720 Release note: None 89910: sql: `tree.ParseJSON` should not ignore trailing data r=miretskiy a=miretskiy `tree.ParseJSON` uses `Decoder.More()` method to determine if the input contains trailing data. The implementation made incorrect assumptions as to the reason why `Decoder.More()` allows input to contain `]` or `}` characters, even when JSON object has been consumed. This PR fixes and comments those faulty assumptions, and fixes multiple existing tests that seemed to rely on those faulty assumptions. In particular, prior to this PR, the following input would be allowed (i.e. extra end of object character `}`): ``` demo@127.0.0.1:26257/movr> select '{"longKey1":"longValue1"}}'::jsonb; jsonb ------------------------------ {"longKey1": "longValue1"} (1 row) ``` But so would the following be allowed: ``` demo@127.0.0.1:26257/movr> select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb; jsonb ------------------------------ {"longKey1": "longValue1"} (1 row) ``` So, the issue is: if above conversion was executed to insert JSONB into the database, we would silently truncate (corrupt) JSON input, and instead of returning an error, we would return success. This behavior is wrong, and this PR fixes it so that an error is returned: ``` select '{"longKey1":"longValue1"}} should this data be ignored?'::jsonb; ERROR: could not parse JSON: trailing characters after JSON document SQLSTATE: 22P02 ``` Release note (bug fix): Do not silently truncate trailing characters when attempting to convert corrupt JSON string input into JSONb. Release note (backward-incompatible change): This change may be backward incompatible to the applications that previously might have been able to insert corrupt JSON data, but now will receive an error. Co-authored-by: Nick Travers <travers@cockroachlabs.com> Co-authored-by: Tommy Reilly <treilly@cockroachlabs.com> Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
The existing `enc_util` package contains a tool that could be used to dump the files in an encryption registry. This command has been broken since the file registry format was updated. Add the `(*PebbleFileRegistry).List` function, that returns a map of files in the registry. Adapt existing test cases. Add a `debug encryption-registry-list` command that will print all files contained in the registry of an encrypted store. This is useful for debugging which store / data key was used to encrypt each file, replacing the equivalent functionality in `enc_util`. Touches: cockroachdb#89095. Epic: None. Release note (ops change): Adds a new command that can be used by an operator to list the files present in the Encryption-At-Rest file registry.
nicktrav
force-pushed
the
nickt.enc-in-situ-file-registry-list
branch
from
5931757
to
0d467be
Compare
nicktrav
commented
Oct 14, 2022
There was a problem hiding this comment.
Reviewable status:
pkg/storage/pebble_file_registry.go line 539 at r1 (raw file):
Previously, jbowens (Jackson Owens) wrote…
r.mu.entriesmay be mutated afterListreturns. I realize this likely isn't a problem for the debug command since there shouldn't be any concurrent filesystem operations, but I think to be defensive we should return a copy. Either a copy of the map, or maybe a sorted slice of[]*enginepb.FileEntrys?
Good call. Done. Kept the map, as the file names are useful.
|
Ignore my reflexive |
|
TFTR! bors r=jbowenns |
|
Build succeeded: |
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.
The existing
enc_utilpackage contains a tool that could be used to dump the files in an encryption registry. This command has been broken since the file registry format was updated.Add the
(*PebbleFileRegistry).Listfunction, that returns a map of files in the registry. Adapt existing test cases.Add a
debug encryption-registry-listcommand that will print all files contained in the registry of an encrypted store. This is useful for debugging which store / data key was used to encrypt each file, replacing the equivalent functionality inenc_util.Touches: #89095.
Epic: None.
Release note (ops change): Adds a new command that can be used by an operator to list the files present in the Encryption-At-Rest file registry.