sops
sops copied to clipboard
getsops
Add YC KMS provider support
SDK documentation was found here. Service implemented in the same way as GCP KMS.
THis PR adding support for YC KMS as an encryption provider.
Resolves issue https://github.com/mozilla/sops/issues/1052
Have put this on my review list but it might take some time before I can take a proper look at it, as I am not too familiar with the YC KMS client.
Have put this on my review list but it might take some time before I can take a proper look at it, as I am not too familiar with the YC KMS client.
Thank you!
One of the things I can see by quickly scanning this is that it seems to lack a way to overwrite the credentials being used, as was made possible by all my rewritten implementations.
This has proven to be of importance when you want to integrate SOPS in another Go application while not relying on runtime environment variables, you can probably get an idea of what I am aiming at by looking at e.g. https://github.com/mozilla/sops/blob/develop/azkv/keysource.go#L97-L111 (or any of the other key source implementations).
In addition: generally speaking, every time a request is made the key is reconstructed for that specific request anyway (as can be seen in keyservice/server.go). So there is little benefit to embedding the client into the key for re-usage purposes (other than maybe ensuring that the HTTP transport continues to be reused).
This has proven to be of importance when you want to integrate SOPS in another Go application while not relying on runtime environment variables, you can probably get an idea of what I am aiming at by looking at e.g. https://github.com/mozilla/sops/blob/develop/azkv/keysource.go#L97-L111 (or any of the other key source implementations).
Thank you for detailed feedback. I didn't understand that part and decided to skip it.
I think it is easy to fix and we will do that.
In addition: generally speaking, every time a request is made the key is reconstructed for that specific request anyway (as can be seen in keyservice/server.go). So there is little benefit to embedding the client into the key for re-usage purposes (other than maybe ensuring that the HTTP transport continues to be reused).
Does it mean that we should move client out of the Key structure?
@astreter cc
Does it mean that we should move
clientout of the Key structure?
Yes, I think it would be best to move the client out and replace it with authentication configuration.
@hiddeco Implemented ability to pass credentials programmatically and cleaned up tests a bit.
Client instance is created every time as you suggested. To implement tests properly I used GCP approach with passing grpcConn directly.
@hiddeco Hey. Small ping :) We fixed your comments and it will awesome if you could check them.
Hi, @ajvb I know you are busy, but could you please have a look at PR? I see that @hiddeco is busy, and maybe you have time for it :)
@hiddeco @ajvb
Hi, we implemented fixes that you requested and it would be great if you can make a review.
Have no rights to merge at present, which means my review wouldn't mean much until this is resolved. Which is something I am actively working on together with others.
