fix: cert-manager duration expects XhYmZs format

[gozer]

It accepts shorter format like Yh, but the mutating webhook, if
installed/enabled will convert it to the full format, causing potential
endless reconciliation loops with tools like ArgoCD

Fixes #190

[llamerada-jp]

if installed/enabled will convert it to the full format, causing potential endless reconciliation loops with tools like ArgoCD

The reverse is also true, If there is a mutating webhook that converts to a shorter format, The change o this PR can cause endless reconcilation loop. I have checked and argocd works fine with both 87600h and 87600h0m0s descriptions. According to upstream's comment, the problem was fixed at v2.10 or later.
Would you please review the mutating webhook settings first, and if you can't avoid the problem, change it so that it can handle both formats.

[gozer]

The reverse is also true, If there is a mutating webhook that converts to a shorter format, The change to this PR can cause endless reconcilation loop. I have checked and argocd works fine with both 87600h and 87600h0m0s descriptions.

The issue isn't with ArgoCD itself, but rather with cert-manager itself, it requires durations to be in that format, and it's cert-manager's own mutating webhook controller that makes this conversion transparently.

So, technically, a Certificate spec.duration field needs to be in this format to be correct, this this PR

[llamerada-jp]

Thanks for letting me know.

I tried to check the output format of cert-manager for the review, but I could not see the above behavior in my environment. It may be a difference in versions, so please tell me which version you tested. Or do I need any options for cert-manager?

[gozer]

Thanks for letting me know.

No worries!

I tried to check the output format of cert-manager for the review, but I could not see the above behavior in my environment. It may be a difference in versions, so please tell me which version you tested. Or do I need any options for cert-manager?

The validating/mutating webhook portion of cert-manager is an optional component.

I've seen it with cert-manager 1.13.3 as of now.

[llamerada-jp]

I checked my environment, cert-manager is 1.14.4, and the webhook is enabled. Does your environment set additional options for webhooks?

I checked the cert-manager code, but could only find a webhook for the CertificateResuest custom resource and could not found logic for Certificate custom resource.
https://github.com/cert-manager/cert-manager/tree/master/internal/webhook/admission/certificaterequest

The validating/mutating webhook portion of cert-manager is an optional component.

Does this mean it is a external product?

[iflan7744]

@gozer I agree with your change, and I am also experiencing this issue when installing the chart via ArgoCD. One small suggestion: instead of hardcoding, it would be nice if we can call it via values.yaml, allowing people to modify it based on their requirements.

webhook:
  pvcMutatingWebhook:
    enabled: true
  certificate:
    generate: true
    caCertDuration: 87600h  # 10 years
    certDuration: 8760h     # 1 year

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity for 30 days. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[gozer]

@gozer I agree with your change, and I am also experiencing this issue when installing the chart via ArgoCD. One small suggestion: instead of hardcoding, it would be nice if we can call it via values.yaml, allowing people to modify it based on their requirements.

Thanks! I didn't have time to dig down precisely where it's coming from so I can offer a reproductible test case. At least I am not alone.

And yes, making it a value makes a whole lot more sense. I'll fix the PR.

[toshipp]

@gozer
Please check and answer @llamerada-jp's question. We can not approve this unless it is reasonable.