cockroachdb / cockroachdb/helm-charts
cert rotation not happening when expiryTime and cron scheduled run are equal
- Dominant language
- Go
- Stars
- 105
- Forks
- 152
- Avg merge
- 4d 23h
- Merged PRs (30d)
- 5
Description
[`Before()` function ](https://pkg.go.dev/time#Time.Before), that we are using [here](https://github.com/cockroachdb/helm-charts/blob/c1bd1b706b450a54f5ab2c09558db9bdb7f3af50/pkg/resource/tls_secret.go#L161), compares if a time has happened before the time provided as the argument and returns false if both times are equal.
https://github.com/cockroachdb/helm-charts/blob/c1bd1b706b450a54f5ab2c09558db9bdb7f3af50/pkg/resource/tls_secret.go#L159-L163
We need to trigger cert rotation before/equals/close to actual cert expiration. For that we can compare `expiryTime` with `nextRun` using [`Sub() function`](https://pkg.go.dev/time#Time.Sub).
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. Rotation didn't happen by the cronjob that ran on the 1st day of the month and the next run is in 23 days while the certificate is expiring in 6 days. So, to avoid this issue, we should also calculate if the cert is expiring before next-to-next cronjob run.
Jira issue: HELM-28
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in pkg/resource/tls_secret.go around lines 159-163 and read how expiryTime is compared with the scheduled run. Trace the cron calculation and certificate rotation path, then check existing tests for this resource. Done means rotation is triggered when expiry is equal to or before the next relevant cron runs, including the next-to-next run case.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, helm
- Domain
- devops, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100