Write encryption data to the Kafka protobuf messages #3976
Conversation
…he encryption in the protobuf record. Contributes toward opensearch-project#3655. Signed-off-by: David Venable <[email protected]>
dlvenable
requested review from
chenqi0805,
engechas,
graytaylor0,
dinujoh,
kkondaka,
asifsmohammed and
oeyh
as code owners
| @@ -16,4 +16,13 @@ message BufferData { | |||
| * is unencrypted data. | |||
| */ | |||
| bytes data = 2; | |||
|
|
|||
There was a problem hiding this comment.
I think this structure have version field to support changes like this for backward compatibility.
There was a problem hiding this comment.
Are you proposing an optional string version = 3; that we'd use to track versions of this? What would the values of these be?
I gather that protobuf messages do not need a version string. Typically developers handle this by keeping backward compatibility.
If we add a version, would it be something as simple as 1? And then we change to 2 if we ever do choose to make a breaking change?
There was a problem hiding this comment.
it should be mandatory and should change whenever the definition of the protobuf message structure changes. This PR is changing the existing structure to a new structure. What happens if writer (producer) to kafka writes one version and the reader of kafka is a different dataprepper version with modified structure?
There was a problem hiding this comment.
Protobuf has built-in support for this. A Data Prepper running with this structure will be ok with the old data because it is marked as optional.
There was a problem hiding this comment.
I see. How does this code handle if the protobuf message doesn't have the new fields? I understand the protobuf libraries handle it gracefully, don't the code need to handle it as well?
There was a problem hiding this comment.
Right now this code is only adding the new messages as noted in the description. I will have a follow-on PR that will use the fields (if provided) to decrypt the data. If not provided, then we will attempt to decrypt with the current key.
Description
When writing Kafka buffer events, save additional information about the encryption in the protobuf record.
This adds two new fields to the Protobuf message:
encryptedencrypted_data_keyThis work does not include using that data when reading. I plan to do that in a follow-on PR, but want to keep the PRs from getting too large.
Issues Resolved
Contributes toward #3655.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.