Add Infisical KMS provider - #88
Conversation
…nfisical_kms_integration_test.go`, and `infisical_kms_test.go` files. - Add support for Infisical in the secrets management system, including methods for putting, getting, and deleting secrets. - Implement integration tests for full lifecycle operations with Infisical KMS.
…dependency changes, implement `infisical/client.go` for KMS client functionality, and modify tests in `infisical_kms_test.go` to align with new client structure.
|
Hi @Adityadan, can we please get a review on this PR? It should be good to go with no new dependencies! |
adityadani
left a comment
adityadani
left a comment
There was a problem hiding this comment.
In general this library provides three modes:
- Integration with KMS providers that generate a "DEK" which then can be used for encrypting data/credentials etc. -> Vaulttransit, AWS KMS etc
- Integration with KMS providers that actually store the secrets used for encryption with them -> Vault, Azure KV etc
- Google Cloud is another mode and a special case which is similar to what you have implemented which is to encrypt the provided data and store it in the persistence store. But it then supports two KeyContexts: PublicData and CustomData.
Which model suits Infisical better? And if it is 3. can we honor PublicData and CustomData behavior similar to what has been done in Google Cloud so that the clients of this library do not see any changes?
…ty in `client_test.go`, covering authentication retries, error handling, and secure URL validation.
Hey @adityadani, thank you for the review! The model that suits Infisical is indeed the model 3. I updated the PR to handle PublicData and CustomData behavior as it's done in the Google Cloud provider. I also addressed all the other comments on the PR. |
adityadani
left a comment
adityadani
left a comment
There was a problem hiding this comment.
The new changes look good.
The clientSecret is still being sent as plainText over HTTP. If that is expected in the Infisical KMS product, please add that as a comment. Looks good to merge once that is addressed.
Should be resolved here! |
Thank you @adityadani! As @varonix0 pointed out, this should already be solved! |
|
Hey @adityadani, I see your approval, but I'm unable to merge it myself. Could you do it, please? |
3 participants
Summary
infisical-kmssecrets backend that delegates encryption/decryption to Infisical KMS