Skip to content

Commit

Permalink
cli: fix debug pebble commands on encrypted stores
Browse files Browse the repository at this point in the history
Currently the debug pebble commands only work correctly on an
encrypted store if the encrypted store's path is `cockroach-data` or
the store directory is passed using `--store` (in addition to being
passed to the pebble subcommand itself). What's worse, knowledge of
this subtle fact was lost among team members.

The root cause is that we are trying to resolve encryption options
using the server config.  The difficulty is that there are a bunch of
different commands and there is no unified way to obtain the store
directory of interest

To fix this, we create `autoDecryptFS`. This is a `vfs.FS`
implementation which is able to automatically detect encrypted paths
and use the correct unencrypted FS. It does this by having a list of
known encrypted stores (the ones in the `--enterprise-encryption`
flag), and looking for any of these paths as ancestors of any path in
an operation. This new implementation replaces `swappableFS` and
`absoluteFS`.

We also improve the error message when we try to open an encrypted
store without setting up the key correctly.

Fixes: #110121

Release note (bug fix): `cockroach debug pebble` commands now work
correctly with encrypted stores which don't use the default
`cockroach-data` path without having to also pass `--store`.
  • Loading branch information
RaduBerinde committed Sep 11, 2023
1 parent 0485b7e commit c049ba0
Show file tree
Hide file tree
Showing 8 changed files with 403 additions and 196 deletions.
7 changes: 7 additions & 0 deletions pkg/ccl/cliccl/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,13 @@ with their env type and encryption settings (if applicable).
&storeEncryptionSpecs, cliflagsccl.EnterpriseEncryption)

cli.PopulateStorageConfigHook = fillEncryptionOptionsForStore
cli.EncryptedStorePathsHook = func() []string {
var res []string
for _, spec := range storeEncryptionSpecs.Specs {
res = append(res, spec.Path)
}
return res
}
}

// fillEncryptionOptionsForStore fills the StorageConfig fields
Expand Down
5 changes: 3 additions & 2 deletions pkg/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ load("//pkg/testutils:buildutil/buildutil.bzl", "disallowed_imports_test")
go_library(
name = "cli",
srcs = [
"absolute_fs.go",
"auth.go",
"auto_decrypt_fs.go",
"cert.go",
"cli.go",
"client_url.go",
Expand Down Expand Up @@ -56,7 +56,6 @@ go_library(
"start_windows.go",
"statement_bundle.go",
"statement_diag.go",
"swappable_fs.go",
"testutils.go",
"tsdump.go",
"userfile.go",
Expand Down Expand Up @@ -238,6 +237,7 @@ go_library(
"@com_github_cockroachdb_errors//hintdetail",
"@com_github_cockroachdb_errors//oserror",
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_cockroachdb_pebble//:pebble",
"@com_github_cockroachdb_pebble//objstorage/remote",
"@com_github_cockroachdb_pebble//tool",
"@com_github_cockroachdb_pebble//vfs",
Expand Down Expand Up @@ -308,6 +308,7 @@ go_test(
name = "cli_test",
size = "large",
srcs = [
"auto_decrypt_fs_test.go",
"cert_test.go",
"cli_debug_test.go",
"cli_test.go",
Expand Down
143 changes: 0 additions & 143 deletions pkg/cli/absolute_fs.go

This file was deleted.

Loading

0 comments on commit c049ba0

Please sign in to comment.