- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2.9k
 
Add Vault Transit KMS client #14451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add Vault Transit KMS client #14451
Conversation
Signed-off-by: Endi Caushi <[email protected]>
Signed-off-by: Endi Caushi <[email protected]>
Signed-off-by: Endi Caushi <[email protected]>
| 
           cc @ggershinsky  | 
    
| 
           Thanks @mrendi29 . I don't have much experience with the Vault. But from the Iceberg encryption interface pov, this looks good.  | 
    
| awssdk-s3accessgrants = { module = "software.amazon.s3.accessgrants:aws-s3-accessgrants-java-plugin", version.ref = "awssdk-s3accessgrants" } | ||
| azuresdk-bom = { module = "com.azure:azure-sdk-bom", version.ref = "azuresdk-bom" } | ||
| bson = { module = "org.mongodb:bson", version.ref = "bson-ver"} | ||
| bettercloud-vault = { module = "com.bettercloud:vault-java-driver", version.ref = "bettercloud-vault" } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I learned before that adding a new dependency also requires adding license information to LICENSE file.
| 
           @nastra @amogh-jahagirdar what do you think, how can Iceberg support key management solutions other than the cloud provider's solution? Instead of adopting the key management client implementation itself, should Iceberg instead provide an option to dynamically load (maybe there's already an option like this?) the key management implementation with the parameters?  | 
    
| 
           Okay, never mind, I found the answer, looks like dynamic loading of KeyManagementClient is supported by   | 
    
| 
           Although the ultimate decision is up to the project PMCs, I personally think that it is probably not a good idea to put this in the core module. Putting it there means that everyone, who uses Iceberg will also take the risk (I refer here mostly to security risks) of the additional dependency required for interaction with HashiCorp vault (not to mention the minimal risk of the client implementation itself), even when they don't need it at all. Other key vault clients are implemented within a cloud provider specific module.  | 
    
Addresses #14437
We are planning to use HashiCorp Vault Transit engine as our KMS of choice. This is the preliminary client that we are using so far so I thought to contribute this back to the community.