Add --tokenKMSEncryptionKey option for PubSubToSplunkTemplate. #123
Conversation
jaketf
left a comment
jaketf
left a comment
There was a problem hiding this comment.
can we be more specific of the purpose of this KMS key in the param name to avoid confusion about pubsub and KMS
src/main/java/com/google/cloud/teleport/templates/common/SplunkConverters.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/teleport/templates/common/SplunkConverters.java
Outdated
Show resolved
Hide resolved
melbrodrigues
changed the title
|
This is great! Thanks for the quick work here @melbrodrigues 🙌 |
sabhyankar
left a comment
sabhyankar
left a comment
There was a problem hiding this comment.
Minor nit but looks good otherwise!
Thanks @melbrodrigues
There was a problem hiding this comment.
Can you add a JavaDoc here to clarify what we are doing with this static method?
|
@rarsan FYI |
sabhyankar
added
the
Google LGTM
| */ | ||
| private static ValueProvider<String> maybeDecrypt( | ||
| ValueProvider<String> unencryptedToken, ValueProvider<String> kmsKey) { | ||
| return new KMSEncryptedNestedValueProvider(unencryptedToken, kmsKey); |
There was a problem hiding this comment.
thx @melbrodrigues for this update.
Shouldn't this parameter be named encryptedToken?
There was a problem hiding this comment.
Good catch @rarsan - I think the value may or may-not-be encrypted tbh since this is backwards compatible. The user could choose to send the hec token without any encryption.
Since this is a private method, we can correct it post merge.
7 participants
Adds an optional parameter to specify a Cloud KMS Encryption Key if an encrypted
tokenparameter is passed to the template.Fixes #122