feat: add pkiSign function to use vault pki sign api endpoint
This pull requests intends to add a function to leverage the sign api endpoint in vault pki.
This can be really useful if one plans to use a large number of certificates with low TTL, where the usage of the issue endpoint can cause high load on the vault servers. By using the sign endpoint one can shift the heaviest part of the problem (the private key generation), from server side to client side.
hey guys, any chance to have a look on that? appreciate.
Hello team, could please someone have a look? It will be nice to have it soon. Thanks in advance!
Hi @n-marton , thanks a lot for opening the PR and your contribution. Your changes look great! I left some feedback. Also, we need unit tests added in here to ensure your new functions are covered in unit tests (to test they work in different use cases and edge cases and in case someone changes it in the future, unit tests catch errors): https://github.com/hashicorp/consul-template/blob/main/template/funcs_test.go
Hi @n-marton , thanks a lot for opening the PR and your contribution. Your changes look great! I left some feedback. Also, we need unit tests added in here to ensure your new functions are covered in unit tests (to test they work in different use cases and edge cases and in case someone changes it in the future, unit tests catch errors): https://github.com/hashicorp/consul-template/blob/main/template/funcs_test.go
hey @aslamovamir
thanks for getting back on this MR, I haven't wrote a test there because the cert issue func also did not have one, in fact that part is kind lacking heavily all together, however I am happy to write one for my new func, IF that is the only one thing we miss to merge this feature
Hi @n-marton , thanks a lot for opening the PR and your contribution. Your changes look great! I left some feedback. Also, we need unit tests added in here to ensure your new functions are covered in unit tests (to test they work in different use cases and edge cases and in case someone changes it in the future, unit tests catch errors): https://github.com/hashicorp/consul-template/blob/main/template/funcs_test.go
hey @aslamovamir
thanks for getting back on this MR, I haven't wrote a test there because the cert issue func also did not have one, in fact that part is kind lacking heavily all together, however I am happy to write one for my new func, IF that is the only one thing we miss to merge this feature
Hi @n-marton, yup I see - I honestly am not part of the codeowners for this repo or never contributed to it so did not know other functions also lack unit tests here. But would be great to have it written for yours if thats okay. All the rest looks good to me in your PR and can approve it once we have unit test coverage.