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

added letsencrypt with http challenge configuration for traefik #37

Merged
merged 4 commits into from
Feb 3, 2022

Conversation

owngr
Copy link
Contributor

@owngr owngr commented Feb 1, 2022

I added two optional variables traefik_acme_tls and traefik_acme_email to enable TLS with letsencrypt, this will directly create a prod acme. I chose the http Challenge as it isn't dependent on the DNS provider.

I noticed afterwards that some people already changed their config the same way after I finished the modification #18 so I don't know if my PR is really relevant, please remove it if it isn't how you want to do it.

@mnencia
Copy link
Contributor

mnencia commented Feb 2, 2022

I think you should also allow overriding the --certificatesresolvers.letsencrypt.acme.caServer parameter because it's better to use the staging server when doing experiments. Otherwise, you could get temporarily suspended due to production rate limits.

- "--certificatesresolvers.le.acme.httpchallenge=true"
- "--certificatesresolvers.le.acme.httpchallenge.entrypoint=web"
- "--certificatesresolvers.le.acme.email=${traefik_acme_email}"
- "--certificatesresolvers.le.acme.storage=/data/acme.json"
Copy link
Collaborator

@mysticaltech mysticaltech Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just use acme.json instead of /data/acme.json, both will work definitely but the former is how they do it on one Traefik example.

Anyways, this file will not be saved permanently, so if the pod restarts it will reissue a certificate (according to this article). But I believe that's not a problem.

I think a way to have the file saved permanently would be to let cert-manager do the certificate fetching and storing as a Kubernetes TLS secret, then they can be used inside of an ingress definition like the certificates we add manually.

Copy link
Contributor Author

@owngr owngr Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mysticaltech Actually I started with acme.json but this didn't work, I had the following error on the traffic pod when I runned it:
is skipped from the resolvers list because: unable to get ACME account: open acme.json: read-only file system

I don't know really know why but storing it in the /data emptydir volume fixed it

@mysticaltech
Copy link
Collaborator

Love this proposal @Olivierwenger, thanks! It's definitely useful to have the option to configure it out of the box.

@mysticaltech
Copy link
Collaborator

mysticaltech commented Feb 2, 2022

I think you should also allow overriding the --certificatesresolvers.letsencrypt.acme.caServer parameter because it's better to use the staging server when doing experiments. Otherwise, you could get temporarily suspended due to production rate limits.

@mnencia Indeed, having the option to start with staging would be good. And later one can re-apply the generated HelmChartConfig without the line in question:

- "--certificatesresolvers.letsencrypt.acme.caserver=https://acme-staging-v02.api.letsencrypt.org/directory"

@mysticaltech
Copy link
Collaborator

@Olivierwenger I can add the staging option later no worries, as soon as you confirm that all works, we can merge this.

@owngr
Copy link
Contributor Author

owngr commented Feb 2, 2022

@mysticaltech I tested @mnencia proposed changes and it works. I just force pushed to change the terraform.tfvars.example accordingly.
I didn't add the the staging option.
Thanks for your help

@mysticaltech
Copy link
Collaborator

@Olivierwenger The terraform fmt -check -diff command is failing. Probably you just need to run terraform fmt to align the formatting and style and push again, please.

@owngr
Copy link
Contributor Author

owngr commented Feb 3, 2022

@mysticaltech done, it should work now

@mysticaltech mysticaltech merged commit 87e6ac4 into kube-hetzner:master Feb 3, 2022
@owngr owngr deleted the feature/add-tls branch February 3, 2022 08:35
@mysticaltech
Copy link
Collaborator

Awesome! Thanks again for this nice contribution @Olivierwenger! 🙏

@TimHeckel
Copy link
Contributor

TimHeckel commented Feb 12, 2022

Hey this is great - is there any way some documentation could be added to readme on how to use this though? Ideally, how do you create the Ingress after these options are enabled -- both for subdomains (e.g., blog.mysite.com) as well as the Ingress for the root (mysite.com)? @Olivierwenger

@owngr
Copy link
Contributor Author

owngr commented Feb 14, 2022

@TimHeckel Yep I'll try to provide an example to test it. Would it be better to use Ingress or IngressRoute ?

@TimHeckel
Copy link
Contributor

Hi Oliver - for my money I think Ingress, just cause people may want to swap Traefik out for something, but if IngressRoute is what's working - then by all means just use that. Thanks!

@mysticaltech
Copy link
Collaborator

Both should work folks, it's just a matter of taste IMHO. Indeed, using Ingress definitions is more portable, as ingress routes are specific to Traefik.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants