Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature request: Add custom renewal time for certificates #437

Closed
transfaeries opened this issue Apr 6, 2018 · 6 comments
Closed

feature request: Add custom renewal time for certificates #437

transfaeries opened this issue Apr 6, 2018 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@transfaeries
Copy link

It'd be useful to give people more control over when certificates renew rather than a constant 30 days before expiry. Shorter times would be useful for testing, and in general people or companies could choose a different interval to match their own schedule.

At the moment the time to renewal is set as a constant in pkg/controller/certificates/sync.go

const renewBefore = time.Hour * 24 * 30

It subtracts this number from the expiry date and renews if the result is less than 0

durationUntilExpiry := cert.NotAfter.Sub(time.Now())
renewIn := durationUntilExpiry - renewBefore
if renewIn <= 0 {
	err := c.renew(ctx, i, crtCopy)
	...
 }

I propose that the user should be allowed to provide their own renewIn as an annotation on the ingress or a field in the certificate issue. In the form of a string "00d00h00m".

The tricky thing is, since as of now the code relies on a fixed timestamp on the certificate to count down the hours until renewal. A variable renewal time would have to be stored somewhere. Possibly as a field on the certificate resource.

Basically the program would take this string, convert it into a duration, subtract that duration from the expiry date, and store this as a renewBefore. So for instance if the user provides 15d00h00m. For a certificate that expires in 90 days renewBefore should be set to
90 days - 15 days = 75 days. So we'd store 75*time.Hour*24 + 0*time.Hour + 0*time.Minute or as a string 75d00h00m

So then every time the controller checks if it should renew instead of subtracting the constant time.Hour * 24 * 30 it would get the custom duration and subtract that.

I've started writing some of this code, but I'm having trouble figuring out a) how to read from an annotation and pass it onto pkg/controller/certificates/sync.go b) how to read and write from the certificate resource for pkg/controller/certificates/sync.go

I also wonder if other people might not have a better way to do this. This is just what I figured would be one way to do it that leaves most of the existing code untouched.

/kind feature

@jetstack-bot jetstack-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 6, 2018
@munnerz
Copy link
Member

munnerz commented Apr 6, 2018

#292 adds this feature, although it is set on a per-issuer basis (i.e. we add a renewBefore and duration field to the issuer, and all certificates issued by that issuer will have the specified renewBefore/duration)

@munnerz
Copy link
Member

munnerz commented Apr 6, 2018

@transfaeries
Copy link
Author

wow, thanks for the mega quick reply. I'll look through that Pull Request and see if it has what I need.

@retest-bot
Copy link

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 jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 5, 2018
@munnerz
Copy link
Member

munnerz commented Jul 25, 2018

/remove-lifecycle stale
/lifecycle frozen

@jetstack-bot jetstack-bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 25, 2018
@munnerz
Copy link
Member

munnerz commented Jan 10, 2019

This has been completed in #893 😄

@munnerz munnerz closed this as completed Jan 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

4 participants