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

Delete Certificate when owning ingress no longer requires it #912

Closed
timuthy opened this issue Sep 16, 2018 · 5 comments · Fixed by #1705
Closed

Delete Certificate when owning ingress no longer requires it #912

timuthy opened this issue Sep 16, 2018 · 5 comments · Fixed by #1705
Assignees
Labels
area/ingress-shim Indicates a PR or issue relates to the ingress-shim 'auto-certificate' component help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@timuthy
Copy link
Contributor

timuthy commented Sep 16, 2018

Is your feature request related to a problem? Please describe.
The Ingress Shim creates a Certificate resource for each element in the TLS list of an Ingress resource. When a element is subsequently deleted from this TLS list, the corresponding Certificate resource is not cleaned up.

Consequences: Although the certificate is not required any more, Cert-Manager still manages its complete lifecycle. If this happens frequently the work queue will finally contain a lot of irrelevant elements that cause evitable processing time and memory usage. Moreover, in combination with Let's Encrypt it affects the rate limit.

Describe the solution you'd like
As soon as the TLS list of an Ingress is modified, Cert-Manager should check if it once has created Certificate resources which got unreferenced through this change, i.e. the control loop should delete this Certificate resource.

/kind feature

@jetstack-bot jetstack-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 16, 2018
@timuthy
Copy link
Contributor Author

timuthy commented Sep 25, 2018

Proposal:

For an updated Ingress resource the Ingress Shim controller gets corresponding Certificates (by owner reference or namespace / name) and checks if hosts in Ingress and DNSNames in Certificate still match.

In case of a mismatch DNSNames of the Certificate is updated or the resource is deleted entirely.

timuthy added a commit to timuthy/cert-broker that referenced this issue Oct 15, 2018
Cleaning up stale Certificates and Secrets must be delegated to
Cert-Manager

cert-manager/cert-manager#912
cert-manager/cert-manager#897

As long as this is not implemented, Certificates and Secrets got stale
remain in the control cluster.
timuthy added a commit to timuthy/cert-broker that referenced this issue Oct 15, 2018
Cleaning up stale Certificates and Secrets must be delegated to
Cert-Manager

cert-manager/cert-manager#912
cert-manager/cert-manager#897

As long as this is not implemented, Certificates and Secrets got stale
remain in the control cluster.
@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 Dec 24, 2018
@munnerz munnerz added this to the v0.7 milestone Jan 16, 2019
@munnerz
Copy link
Member

munnerz commented Jan 16, 2019

Scheduled in for v0.7 - it'd be great to get ingress-shim updated to handle this properly 😄

/remove-lifecycle stale

@jetstack-bot jetstack-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 16, 2019
@munnerz munnerz added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Feb 7, 2019
@munnerz munnerz removed this from the v0.7 milestone Feb 7, 2019
@munnerz munnerz changed the title Clean up Certificate when TLS element is deleted from Ingress Delete Certificate when owning ingress no longer requires it Feb 7, 2019
@munnerz munnerz added the area/ingress-shim Indicates a PR or issue relates to the ingress-shim 'auto-certificate' component label Feb 8, 2019
@MikeBlomm
Copy link

MikeBlomm commented Apr 2, 2019

@munnerz What is the latest status of this proposal? Seems like a great feature to me.

@cheukwing
Copy link
Contributor

@MikeBlomm I'm going to try to implement this

/assign
/lifecycle active

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ingress-shim Indicates a PR or issue relates to the ingress-shim 'auto-certificate' component help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/active priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants