search

jetstack / kube-lego

DEPRECATED: Automatically request certificates for Kubernetes Ingress resources from Let's Encrypt
Apache License 2.0
2.16k stars 269 forks source link

Pass the ingress object, not the function to renew #339

Closed carsonoid closed 6 years ago

carsonoid commented 6 years ago

This fixes auto-renewal of certificates and exposes any future errors.

jetstack-bot commented 6 years ago

Hi @carsonoid. Thanks for your PR.

I'm waiting for a jetstack member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
carsonoid commented 6 years ago

Fixes #338

I know that kube-lego is deprecated. But this is a major feature that was broken by the 0.1.6 release.

munnerz commented 6 years ago

Nice find. Thanks very much.

@simonswine could you take a quick look at this and merge? Given the issue at hand, I think this warrants a new patch release.

We don't have test cases for renewal, so this was not picked up.

/assign @simonswine

carsonoid commented 6 years ago

@simonswine can I get a review on this? Not having auto-renewal working is a big issue for us. :)

simonswine commented 6 years ago

/assign @joshvanl

/ok-to-test

simonswine commented 6 years ago

Thanks for this fix @carsonoid, my colleague tries to reproduce the problem, verify your fix and finnally get a new version release

killianbrackey commented 6 years ago

Thanks for the quick fix @carsonoid. @simonswine do you anticipate the patch will be merged up soon here? I am able to reproduce @carsonoid's findings and will be building my own image if you are unsure when the patch may be released as I have a couple of certificates that expire in a week or so here 👍 Happy to help reproduce or test as well.

JoshVanL commented 6 years ago

Hi @carlossg, I was able to reproduce the problem. Removing the work queue rate limiter and adding your PR changes, kube-lego was auto renewing as expected.

@simonswine @munnerz /lgtm

munnerz commented 6 years ago

/approve

simonswine commented 6 years ago

/approve