Reviewing 'minimum certificate duration' requirements and handling

[munnerz]

As per #3038 (comment), we should ensure that the 'minimum certificate duration' logic we have is a) needed and b) works properly.

/milestone Next
/priority important-longterm
/area api

[meyskens]

Tested on the latest alpha and we do seem to enforce it now :) I'll just blame an old cert-manager version with bad validation when I saw this before.

it's also quite a difficult thing for us to enforce given the CA can override the behaviour.
All of cert-manager code needs to tolerate any duration, and can't really rely on this being true.
That said, supposing a CA returns a Certificate valid for 1s, or a user requests a certificate valid for 1s, what would we really want to happen here? That's an excessively short duration, and will lead to cert-manager performing lots of certificate requests.
As a cluster administrator, I'd like to have some guard against this. Perhaps a flag on the controller which is a universal minimum value? (could be enforced by the webhook then?).

I think we should add a minimal duration check, well keep it as it seems to work. Just to prevent user error making 1m and thinking m is months.

How we do handle the case where a user doesn't explicitly set a duration, yet the CA still returns one valid for 1s? Should we go into that tight request loop despite the minimum duration not being met?

Adding a more reasonable edge case the user requests a longer valid cert than it receives and the duration happens to be 1 minute more than the renewBefore? Should we enforce a minimal duration on the CR?

Thinking even further.. what if a CA returns a certificate that is already expired?

How about we do a check on that, if so place the CR in a failed state so it is exponentially retried till the CA fixes their behavior?

[jetstack-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

[jetstack-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

[jetstack-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

[jetstack-bot]

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.