feat: add csr-domain-name config option
#381
Conversation
DnPlas
force-pushed
the
KF-5337-domain-name-config
branch
from
932e70a to
ba6278d
Compare
DnPlas
changed the title
Having the domain-name config option will allow users to specify the domain name they'd like to use when integrating with TLS certificate operators. This feature expands the support for integrating with TLS certificate providers that cannot issue signed certificates on a CSR that only contains an IP address (like we used to do). This commit also adds some test coverage to test the recently added code. Fixes #379
DnPlas
force-pushed
the
KF-5337-domain-name-config
branch
from
ba6278d to
606107c
Compare
There was a problem hiding this comment.
Thank you @DnPlas. I'm adding what we discussed in Matrix:
- Have you tested this? Should a reviewer test this too? If yes, how?
- Do we need to update docs for this and how it's supposed to be used?
In general the changes make sense to me and this is some good work. I have some thoughts on the cert_subject property, but let's see what you think as well.
|
I expect the CI to fail until we merge #382 |
|
Can we line up the name of the config option with that of Traefik etc? (I think they use external_hostname or something like that?) |
Hey @jnsgruk the reason why I did not want to use the same name as them is because I could see us having a more explicit configuration for the external hostname (the name users will reach on their browsers), but that seems out of scope of this PR. wdyt? EDIT: or we could not have the option at all and use our "automatic" discovery and keep it implicit in istio. I will explore this option a bit more. |
This sounds like a better plan to me :) |
Yeah, definitely. Will update the PR shortly. |
DnPlas
changed the title
cert_subject from the ingress gateway address
DnPlas
changed the title
cert_subject from the ingress gateway addresscsr-domain-name config option
|
Thanks @ca-scribner for the review, this is ready for another pass. |
feat: enable csr-domain-name config option so istio-pilot can use it on CSRs The istio-pilot charm already has a mechanism in place to discover the ingress gateway address from the `Service`, but it is limited to only returning IP addresses, which not all TLS certificate providers accept as a valid cert subject. Having the domain-name config option will allow users to specify the domain name they'd like to use when integrating with TLS certificate operators. This feature expands the support for integrating with TLS certificate providers that cannot issue signed certificates on a CSR that only contains an IP address (like we used to do). This commit also adds some test coverage to test the recently added code. Fixes #379
feat: enable csr-domain-name config option so istio-pilot can use it on CSRs The istio-pilot charm already has a mechanism in place to discover the ingress gateway address from the `Service`, but it is limited to only returning IP addresses, which not all TLS certificate providers accept as a valid cert subject. Having the domain-name config option will allow users to specify the domain name they'd like to use when integrating with TLS certificate operators. This feature expands the support for integrating with TLS certificate providers that cannot issue signed certificates on a CSR that only contains an IP address (like we used to do). This commit also adds some test coverage to test the recently added code. Fixes #379
feat: add `csr-domain-name` config option (#381)
feat: enable csr-domain-name config option so istio-pilot can use it on CSRs
The istio-pilot charm already has a mechanism in place to discover the ingress gateway address from the
Service, but it is limited to only returning IP addresses, which not all TLS certificate providers accept as a valid cert subject. Having the domain-name config option will allow users to specify the domain name they'd like to use when integrating with TLS certificate operators. This feature expands the support for integrating with TLS certificate providers that cannot issue signed certificates on a CSR that only contains an IP address (like we used to do).This commit also adds some test coverage to test the recently added code.
Fixes #379