aureq / cert-manager-webhook-ovh

OVH Webhook for Cert Manager
https://aureq.github.io/cert-manager-webhook-ovh/
Apache License 2.0
80 stars 14 forks source link

Fix pki duration #10

Closed orblazer closed 1 year ago

orblazer commented 1 year ago

Hello, This PR fix pki certificate duration by removing useless minutes and seconds.

Why ? Because if we use argoCD or other look like that, said the resources is "Out of Sync" because argocd-controller remove 0m0s in that resources.

aureq commented 1 year ago

@orblazer do you have a reference that you could share please?

orblazer commented 1 year ago

@aureq reference of what ? how argocd change that value ? if it is, i don't know, i have just see this issue in my argocd instance.

If that way is not great, i think we could simply allow change that values.

aureq commented 1 year ago

@orblazer Looks like another user is having issues with ArgoCD. I think the next release will probably allow for custom values but likely default to the longer format.

TheMrPuffin commented 1 year ago

I'm currently experiencing the issue described by @orblazer but in reverse. If the minutes and seconds are not included in the template for the certificate, ArgoCD will be out of sync because certificate objects in the K8s cluster will contain the minutes and seconds but the manifest set by the helm template will not.

For reference I’ve seen this on ArgoCD v2.5.3 on a K8s version 1.25.4.

Sounds like adding custom values would be better in the long run.

TheMrPuffin commented 1 year ago

Also the version of the chart included in the release 0.4.0 of this repository includes the duration values without the minutes and seconds. I was trying to use the version as it was available on a gh-page but bumped into the same issue I was having above.

This PR would revert the fixing changes which I assume is coming in the upcoming release.

aureq commented 1 year ago

@TheMrPuffin Good catch. Something slipped up there. It should be shipped that way already.

aureq commented 1 year ago

@TheMrPuffin @orblazer could you folks give a go with v0.4.1 please? For now, the duration is not configurable but I'd like to see how it behaves for both of you.

TheMrPuffin commented 1 year ago

@aureq Just upgraded to v0.4.1 and works as expected. My ArgoCD is now showing the certificate is now synced between manifest and the created object, so all good! :)

aureq commented 1 year ago

I'll make this one as closed since it appears to be working fine and no other issues on the topic have been reported. Thank you both