br: add encryption config to streaming backup #1255
Conversation
|
|
proto/brpb.proto
Outdated
| @@ -665,7 +673,7 @@ message DataFileGroup { | |||
| } | |||
|
|
|||
| message DataFileInfo { | |||
| // SHA256 of the file. | |||
| // Logical checksum of all the kv pairs in the file. | |||
There was a problem hiding this comment.
This isn't a logical checksum over raw kvs, the checksum was executed after compression if i recall right.
There was a problem hiding this comment.
discussed offline, this is pre-compression checksum of the file but not exactly logical too. It's the checksum of the plaintext file.
| if [ "$major" -eq 3 ] && [ "$minor" -ge 8 ]; then | ||
| return 0 | ||
| fi | ||
| # protobuf bumps the major version to 21 after 3. | ||
| # https://github.com/protocolbuffers/protobuf/releases/tag/v21.7 | ||
| if [ "$major" -ge 21 ]; then |
There was a problem hiding this comment.
Thank you! Now we no more need to download an old protoc for passing the check...
Tristan1900
force-pushed
the
add-encryption-stream-backup
branch
from
96ae3e8
to
c24f599
Compare
Tristan1900
force-pushed
the
add-encryption-stream-backup
branch
from
e964b0f
to
48f62dd
Compare
|
hey folks, @BornChanger @overvenus, could you take a look at this change when you get a chance? Thanks! |
proto/import_sstpb.proto
Outdated
| @@ -394,6 +395,9 @@ message KVMeta { | |||
|
|
|||
| // the compression type for the file. | |||
| backup.CompressionType compression_type = 13; | |||
|
|
|||
| // encryption information of the kv file, not set if encryption is not enabled. | |||
| encryptionpb.FileEncryptionInfo file_encryption_info = 19; | |||
There was a problem hiding this comment.
The largest tag number in this message is 13. Why 19?
There was a problem hiding this comment.
good catch! fixed
|
|
||
| // not recommended in production. | ||
| // user needs to pass back the same data key for restore. | ||
| message PlainTextDataKey {} |
There was a problem hiding this comment.
Empty message, how to pass data keys?
There was a problem hiding this comment.
right, it's a placeholder as this proto is going to be serialized and stored as part of the metadata in external storage. The actual plaintext key is going to be passed back by user during restore so not get exposed if external storage is breached.
|
@overvenus thanks for the review! Sorry for the late reply, was doing e2e testing and added one more proto message. Please take an another look if you have time! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BornChanger, overvenus The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
FileEncryptionInfoto store encrypted data key information, add it toDataFileInfothat's uploaded to S3 as the backup file metadata.MasterKeyKmswith optional GCP and Azure KMS information so can used to construct master key backendMasterKeytoApplyRequestso during log restore master key information can be passed down to TiKV.check.shto accommodate version formatmajor.minorbesidesmajor.minor.patch, add checks for protobuf 27 major release version after major version 3