-
Notifications
You must be signed in to change notification settings - Fork 560
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
add domain name for virtual host match in envoy filters #3376
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Rama Chavali <[email protected]>
@@ -569,6 +569,9 @@ message EnvoyFilter { | |||
// registry. | |||
string name = 1; | |||
|
|||
// Match a specific domain name in a virtual host. | |||
string domain_name=3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the use case for using this vs name? Does it do wildcards or only exact match (like can I have a wildcard here to match a wildcard domain)? Does it consider port (I think we configure envoy to ignore port?)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case is - if we enable route collapse optimization with different domain names using same routes, we pick one route name and merge.
Yes. It can match wildcard names and port is not included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is the name not able to distinguish the VHost?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual Routes"
{
"version_info": "2024-11-01T23:45:05Z/17931",
"route_config": {
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "https.10443.xxxx",
"virtual_hosts": [
{
"name": "*.au:10443",
"domains": [
"*.au"
],
"routes": [..]
},
{
"name": "*.com:10443",
"domains": [
"*.com",
"*"
],
"routes": [..]
},
]
}
}
after merge
{
"version_info": "2024-11-01T23:45:05Z/17931",
"route_config": {
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "https.10443.xxxx",
"virtual_hosts": [
{
"name": "*.com:10443",
"domains": [
"*.au",
"*.com",
],
"routes": [..]
}
]
}
Now we can not match on Vs Host name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in this case, i put match.domainName = "*.au"
? And it will apply to *.com
as well though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add example. Release notes planning to add it for the implementation PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems only me is still confusing, Isn't name
already unique to distinguish the VirtualHost
I am not sure how https://github.com/istio/api/pull/3376/files#r1868857034 is related
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not. When we have routes collapsed for different domains, we pick one virtual host and combine domains - So it is not possible to uniquely identify a virtual host by name for a domain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@linsun added example. PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@linsun can you PTAL when you get chance?
Signed-off-by: Rama Chavali <[email protected]>
Signed-off-by: Rama Chavali <[email protected]>
No description provided.