Document the TLS with ACME case #4654
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Oof this feels bad, perhaps a new boolean instead? Idk. This will work short term, but I think we should come up with a better way to do this before GA 😅 |
ldez
changed the title
ldez
changed the title
ldez
added
kind/enhancement
yeah, it's far from ideal. |
When we did the kubernetes crd implementation, we forgot to test the case for when one wants TLS, but handled with Let's Encrypt, i.e. without having to provide a Kubernetes Secret. We assumed it would be enough to provide (in YAML) a tls object with no field set, which would get us a non-nil IngressRouteSpec.TLS, allowing us to use IngressRouteSpec.TLS as the sentinel for whether TLS should be enabled. However, as IngressRouteSpec.TLS is a pointer, a tls object with no fields set will actually result in IngressRouteSpec.TLS being nil. This means at least one of the fields of the YAML tls object must exist. Therefore, we now use the secretName field value (for now, as it is the only field anyway) as the sentinel for whether Let's Encrypt should be used. This PR documents the above behavior, and adds a unit test for it. In addition, this PR fixes a related bug in the ACME provider: when a router is not configured with TLS enabled, the ACME provider does not try anymore to generate a (useless) certificate for the corresponding domain.
|
Maybe is just a visualisation issue, but the raw dashboard api seems to expect ❯ curl -sSL http://traefik-v2:8080/api/rawdata | jq . {
"ACME": {
"HTTP": {},
"TCP": {},
"TLSOptions": null,
"TLSStores": null
},
"kubernetescrd": {
"HTTP": {
"routers": {
"default/cats-fe7c78d104ccea9b6f28": {
"entryPoints": [
"web"
],
"middlewares": [
"default/rmpath"
],
"service": "default/cats-fe7c78d104ccea9b6f28",
"rule": "Path(`/cache`)"
},
... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
When we did the kubernetes crd implementation, we forgot to test the
case for when one wants TLS, but handled with Let's Encrypt, i.e.
without having to provide a Kubernetes Secret. We assumed it would be
enough to provide (in YAML) a tls object with no field set, which would
get us a non-nil IngressRouteSpec.TLS, allowing us to use
IngressRouteSpec.TLS as the sentinel for whether TLS should be enabled.
However, as IngressRouteSpec.TLS is a pointer, a tls object with no
fields set will actually result in IngressRouteSpec.TLS being nil. This
means at least one of the fields of the YAML tls object must exist.
Therefore, we now use the secretName field value (for now, as it is
the only field anyway) as the sentinel for whether Let's Encrypt should
be used.
This PR documents the above behaviour, and adds a unit test for it.
In addition, this PR fixes a related bug in the ACME provider: when a
router is not configured with TLS enabled, the ACME provider does not
try anymore to generate a (useless) certificate for the corresponding
domain.
Motivation
As described above, documenting this common case was needed.
More
Additional Notes
Co-authored-by: Julien Salleyron julien.salleyron@gmail.com