feat(azurekv): add expiration time to azure kv secret#4709
feat(azurekv): add expiration time to azure kv secret#4709hjoshi123 wants to merge 9 commits intoexternal-secrets:mainfrom
Conversation
29e118f to
77539be
Compare
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
f34bb85 to
32f5960
Compare
|
Wonderful ! |
| expiryDuration := time.Now().Add(time.Hour) | ||
| expiryTime := date.UnixTime(expiryDuration) | ||
| createdTime := date.UnixTime(time.Now()) |
There was a problem hiding this comment.
can we add custom checks to verify if these values are actually present on the outputs for this test?
There was a problem hiding this comment.
yup will add that to the test
There was a problem hiding this comment.
@hjoshi123 Can you point me to the code section that actually verifies that these values are set correctly? 🤔 I don't see that.
There was a problem hiding this comment.
This is still unanswered. :)
There was a problem hiding this comment.
oh hey sorry I missed the thread.. I did add a test here..
setSecretWithAttributes := func(smtc *secretManagerTestCase) {
expiryDuration := time.Now().Add(time.Hour)
expiryTime := date.UnixTime(expiryDuration)
createdTime := date.UnixTime(time.Now())
smtc.ref.MetadataPolicy = esv1.ExternalSecretMetadataPolicyFetch
smtc.secretOutput = keyvault.SecretBundle{
Value: &secretString,
Attributes: &keyvault.SecretAttributes{
Expires: &expiryTime,
Created: &createdTime,
Updated: &createdTime,
},
}
smtc.expectedData["expires"] = []byte(time.Unix(int64(expiryTime.Duration().Seconds()), 0).String())
smtc.expectedData["created"] = []byte(time.Unix(int64(createdTime.Duration().Seconds()), 0).String())
smtc.expectedData["updated"] = []byte(time.Unix(int64(createdTime.Duration().Seconds()), 0).String())
}its in the secret manager test case.. sorry for the delay in reply
|
code itself looks good, but tests do not seem to be testing the feature itself :) |
|
@gusfcarvalho the test I added checks for expiry.. I will add checks for other fields too.. what other tests do you think I should add? |
|
that one is the only test case that I think it is worth to add - to test out these fields will now be there. |
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
c31033b to
9fcfaba
Compare
|
@gusfcarvalho does this look good now? |
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Signed-off-by: Hemant Joshi <mail2hemantjoshi@pm.me>
|
@Skarlso @gusfcarvalho is there any updates I should do for the above? |
|
Hey @hjoshi123. Could you please take care of the new Sonar issue that popped up? :) Thanks. Sorry, I was on vacation. |
Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>
…hi123/external-secrets into feat/azure-keyvault-expiration-time
|
|
@Skarlso no issues. Sorry for the delay and the ping too. I think the sonarr qube thing is fixed now |
|
@Skarlso Has the issue been resolved? I can also help with this PR |
|
Sorry, was on vacation again.. :) I'll take a look at this today. |
|
Skarlso
left a comment
There was a problem hiding this comment.
Ah, please fix the CI failures. :)
|
Hi @hjoshi123, |
|
We waited for a longer period of time now on this, so I'm going to close it. @bsvicente feel free to open a separate PR if you are up for it as a continuation of this one. :) Thanks! |
Closes external-secrets#4697. When MetadataPolicy is set to Fetch, secret/cert/key attributes (expires, created, updated, notBefore) are now included alongside tags in the metadata response as RFC3339 timestamps. Also fixes the gocritic sloppyReassign lint errors that blocked PR external-secrets#4709. Signed-off-by: Murali <muraliavarma@gmail.com>
|
Hi @Skarlso , I tried to take the work from @hjoshi123 over the finish line in a separate PR here if you think it is worth looking at. It is my first contribution to the repo so I apologize if I am missing something obvious in the code or with the process. Thank you! |
Closes external-secrets#4697. When MetadataPolicy is set to Fetch, secret/cert/key attributes (expires, created, updated, notBefore) are now included alongside tags in the metadata response as RFC3339 timestamps. Also fixes the gocritic sloppyReassign lint errors that blocked PR external-secrets#4709. Signed-off-by: Murali <muraliavarma@gmail.com>
Problem Statement
What is the problem you're trying to solve?
Adding expiration date along with other available attributes to azure keyvault metadata. Previously this was allowed only for tags.
Related Issue
Fixes #4697
Proposed Changes
How do you like to solve the issue and why?
Checklist
git commit --signoffmake testmake reviewable