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

Settings tls-acme to false should remove existing Certificate objects that are not Ready #227

Closed
seanhamlin opened this issue Mar 1, 2022 · 4 comments · Fixed by #231
Closed
Labels
2-build-deploy bug Something isn't working

Comments

@seanhamlin
Copy link
Contributor

Describe the bug

If you set tls-acme to True by mistake, e.g. due to the DNS mapping to a CDN that strips Lets Encrypt challenges (e.g. Akamai). This creates a pod that will run in the namespace trying to validate the HTTP challenge.

If you then realise your mistake, and correct the .lagoon.yml to have tls-acme set to False, this will not clean up the existing Lets Encrypt challenge. Leaving you with manual intervention required to clean up the Certificate object.

To Reproduce

Steps to reproduce the behavior:

  1. Set tls-acme to True on a domain in .lagoon.yml
  2. Point DNS for the domain to a CDN that strips the Lets Encrypt challenges (e.g. Akamai)
  3. Correct the .lagoon.yml to have tls-acme set to False
  4. Notice challenge pod remains

Expected behavior

Lagoon should delete all Certificate objects in the namespace if they are not Ready in status.

Additional context

Potentially related to uselagoon/lagoon#2795

@seanhamlin seanhamlin added bug Something isn't working 2-build-deploy labels Mar 1, 2022
@shreddedbacon
Copy link
Member

shreddedbacon commented Mar 1, 2022

We could probably have a build run the kubectl delete certificate custom-ingress.fullname-tls

But we should also probably then tell the Ingress to not set the tls spec if tls-acme: false (https://github.com/uselagoon/lagoon/blob/v2.3.0/images/kubectl-build-deploy-dind/helmcharts/custom-ingress/templates/ingress.yaml#L57-L60). Setting this tells the ingress controller that there is a certificate available, but if it never generates the ingress controller still tries to check the certificate and fails, which seems like extra work for the controller when it doesn't need to do it at all.

@seanhamlin
Copy link
Contributor Author

This just occurred on a large cluster, where there where 50 certificate objects lying around (and 50 acme pods)

@shreddedbacon
Copy link
Member

Need to fully understand what implications there will be for deleting the Certificate and associated resources if it is set to false.

Might there be a case where we don't want to delete those resources?

@smlx
Copy link
Member

smlx commented Jul 8, 2022

Might there be a case where we don't want to delete those resources?

I can't see any problem with this. It won't actually remove the TLS certificate from the ingress. It just means that the ACME challenges will no longer be generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-build-deploy bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants