Add DYNAMIC_DNS resolution type for wildcard hosts #3565
Conversation
- Defines DELAYED_DNS Signed-off-by: Jackie Elliott <[email protected]>
Signed-off-by: Jackie Elliott <[email protected]>
|
Skipping CI for Draft Pull Request. |
istio-testing
added
the
do-not-merge/work-in-progress
|
😊 Welcome @jaellio! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
istio-testing
added
the
size/S
Signed-off-by: Jackie Elliott <[email protected]>
istio-testing
removed
the
do-not-merge/work-in-progress
validation Signed-off-by: Jackie Elliott <[email protected]>
|
Our current kubebuilder directive for wildcard hosts disallows setting any host to |
Signed-off-by: Jackie Elliott <[email protected]>
Signed-off-by: Jackie Elliott <[email protected]>
There was a problem hiding this comment.
LGTM; will approve after:
- Ian's comments/suggestions are addressed
- We have clarity on if we're using webhook validation or kubebuilder validation how this field is going to be used (I recommend webhook validation if we want to support this mode in sidecars later)
Signed-off-by: Jackie Elliott <[email protected]>
There was a problem hiding this comment.
Nice improvement. Thanks for fixing my weird "front-end" wording.
Maybe I'm being pedantic here but I'd like to see clearer delineation between what occurs during DNS proxy in ztunnel vs what DYNAMIC_DNS is intended to do. The two complement each other but should not be conflated.
Signed-off-by: Jackie Elliott <[email protected]>
| // the Host header or SNI to an IP address when handling traffic. This | ||
| // is particularly useful when multiple dns addresses can be represented | ||
| // by a single wildcard `host` entry without having to explicitly |
There was a problem hiding this comment.
Did we decide we only allow configuring this when using wildcards? The wording suggests it could be used elsewhere but I'm not sure if we strongly disallow that, or only don't recommend it.
There was a problem hiding this comment.
Personally, I'm in favor of a more narrow application and then expand it as we test
There was a problem hiding this comment.
I agree with being stricter in our official support. I believe in the POC I explicitly disallow non wildcard hosts with DYNAMIC_DNS resolution.
There was a problem hiding this comment.
I have this right now later in the comment
`DYNAMIC_DNS` is only supported for wildcard hosts
There was a problem hiding this comment.
My thinking is that "particularly useful" suggests there are other applications allowed, just we didn't think they would be so useful. We could change it to something like "This is required when using a wildcard host name..." which to my mind doesn't suggest other applications.
I may just be splitting hairs here though. It's likely clear enough and just my mistake reading sentences individually versus in full context.
There was a problem hiding this comment.
Removed "Particularly useful"
Signed-off-by: Jackie Elliott <[email protected]>
Part of istio/istio#54540