Handle cert renewal correctly

[irbekrm]

See #3897 for context.

This PR changes cert's renewBefore period to be calculated as min(renewBefore, cert duration / 3) where renewBefore is either cert.spec.renewBefore or 30d.

It fixes a bug, where when cert's duration is very slightly longer than the renewBefore period, we enter a renewal loop.
This has so far hit a number of Vault issuer's users.

The bug:

See the below section of cert status:

nextPrivateKeySecretName: example-com-l2g4c
  notAfter: "2021-06-06T09:06:50Z"
  notBefore: "2021-05-07T09:06:20Z"
  renewalTime: "2021-05-07T09:06:50Z"
  revision: 185

See these functions where we calculate renewal time.

This cert has defaultRenewBefore=30d, actualDuration=30d30s, which means that we return 30d as the renewBefore time here.
We then calculate renewal time here.
In this case the renewal time it will be cert.spec.NotAfter - 30d resulting in renewal time 30s from now and we start renewing every 30s (when I tested this, every cert issued by Vault had the duration with the extra 30s).

This particular case would have also been solved by rounding up the cert duration, but it still seems like min(renewBefore, cert duration / 3) is a more sane formula.

Fixes cert renewal bug for Vault certificates with duration close to renewBefore period. Thanks to @andreas-p for raising the issue.

This PR is based on #4031 , but also refactors the renewal functionality so that these kind of bugs can be more easily spotted in future. I have added two test cases for these error cases and tested with an actual Vault instance.
Fixes #3897
Signed-off-by: irbekrm irbekrm@gmail.com

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [irbekrm]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[irbekrm]

/kind bug

[james-w]

/lgtm

Thanks! This fixes the bug and also makes the code and tests clearer.