Replace time.Before with time.Sub #213
Conversation
Also, calculate expiration before next-to-next cronjob run and rotate if required
There was a problem hiding this comment.
Your comment on the issue:
Also, we came across with a case while rotating root cert every month, helm chart calculated the cronStr as 0 0 */23 * * (i.e. cron gets executed every 1st and 23rd day of the month). Our certificate didn't get rotated and was due expiration on 7th day of the month.
This above cron string will run on at 00:00 on every 23rd of the month. If your cert is expiring on the 7th, it will detect that on previous month's 23 when the cron is run and the cert is expiring before the next cron run and it will rotate the certs
pkg/resource/tls_secret.go
Outdated
| // if cert is expiring before next-to-next run or within (next-to-next run + 1 hour) | ||
| if expiryTime.Sub(nextToNextRun) < 1*time.Hour { | ||
| return true, "Certificate about to expire, rotating certificate" | ||
| } |
There was a problem hiding this comment.
Why we need to check the next to next run + 1 hour, as we are already checking next run + 1 hour, the certificate expiring after next run + 1 hour will be rotated in the next run.
There was a problem hiding this comment.
@prafull01 your comment does make sense. I don't exactly remember the scenario since it was more than a month ago (should have documented the issue properly). I will remove this and update the unit test in next patch set.
There was a problem hiding this comment.
@prafull01 Apologies for addressing your comment late. I have just pushed a new patchset.
|
@prafull01 Could you please take a look at it again? |
Also, calculate expiration before next-to-next cronjob run and rotate if required
Fixes #212