-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Cleanup gateway vhost config gen #12847
Changes from 3 commits
10927d9
feae87c
0b23369
22d5e36
a94471f
7865c22
53ab249
80122be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -543,10 +543,10 @@ type ServiceDiscovery interface { | |
// Hostname("foo.com").Matches("foo.com") = true | ||
// Hostname("foo.com").Matches("bar.com") = false | ||
// Hostname("*.com").Matches("foo.com") = true | ||
// Hostname("*.com").Matches("bar.com") = true | ||
// Hostname("bar.com").Matches("*.com") = true | ||
// Hostname("*.foo.com").Matches("foo.com") = false | ||
// Hostname("*").Matches("foo.com") = true | ||
// Hostname("*").Matches("*.com") = true | ||
// Hostname("*").Matches("foo.com") = true | ||
// Hostname("*").Matches("*.com") = true | ||
func (h Hostname) Matches(o Hostname) bool { | ||
hWildcard := len(h) > 0 && string(h[0]) == "*" | ||
oWildcard := len(o) > 0 && string(o[0]) == "*" | ||
|
@@ -639,6 +639,55 @@ func (h Hostnames) Swap(i, j int) { | |
h[i], h[j] = h[j], h[i] | ||
} | ||
|
||
// Intersection returns the subset of host names that are covered by both hosts and other. | ||
// e.g.: | ||
// Hostnames(["foo.com","bar.com"]).Intersection(Hostnames(["*.com"])) = Hostnames(["foo.com","bar.com"]) | ||
// Hostnames(["foo.com","*.net"]).Intersection(Hostnames(["*.com","bar.net"])) = Hostnames(["foo.com","bar.net"]) | ||
// Hostnames(["foo.com","*.net"]).Intersection(Hostnames(["*.bar.net"])) = Hostnames(["*.bar.net"]) | ||
// Hostnames(["foo.com"]).Intersection(Hostnames(["bar.com"])) = Hostnames([]) | ||
func (hosts Hostnames) Intersection(other Hostnames) Hostnames { | ||
result := make(Hostnames, 0, len(hosts)) | ||
for _, hHost := range hosts { | ||
for _, oHost := range other { | ||
if hHost.SubsetOf(oHost) { | ||
result = append(result, hHost) | ||
} else if oHost.SubsetOf(hHost) { | ||
result = append(result, oHost) | ||
} | ||
} | ||
} | ||
return result | ||
} | ||
rshriram marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// StringsToHostnames converts a slice of host name strings to type Hostnames. | ||
func StringsToHostnames(hosts []string) Hostnames { | ||
result := make(Hostnames, 0, len(hosts)) | ||
for _, host := range hosts { | ||
result = append(result, Hostname(host)) | ||
} | ||
return result | ||
} | ||
|
||
// HostnamesForNamespace returns the subset of hosts that are in the specified namespace. | ||
// The list of hosts contains host names optionally qualified with namespace/. | ||
// If not qualified, a host name is considered to be in any namespace. | ||
// NOTE: only expecting ns/host and no ./* or */host. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. server host could definitely be of the form */host |
||
func HostnamesForNamespace(hosts []string, namespace string) Hostnames { | ||
result := make(Hostnames, 0, len(hosts)) | ||
for _, host := range hosts { | ||
if strings.Contains(host, "/") { | ||
parts := strings.Split(string(host), "/") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unnecessary conversion (from |
||
if parts[0] != namespace { | ||
continue | ||
} | ||
//strip the namespace | ||
host = parts[1] | ||
} | ||
result = append(result, Hostname(host)) | ||
} | ||
return result | ||
} | ||
|
||
// SubsetOf is true if the label has identical values for the keys | ||
func (l Labels) SubsetOf(that Labels) bool { | ||
for k, v := range l { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -216,71 +216,58 @@ func (configgen *ConfigGeneratorImpl) buildGatewayHTTPRouteConfig(env *model.Env | |
} | ||
|
||
servers := merged.ServersByRouteName[routeName] | ||
port := int(servers[0].Port.Number) // all these servers are for the same routeName, and therefore same port | ||
|
||
nameToServiceMap := make(map[model.Hostname]*model.Service, len(services)) | ||
for _, svc := range services { | ||
nameToServiceMap[svc.Hostname] = svc | ||
} | ||
|
||
gatewaysForVSSelection := make(map[string]bool) | ||
gatewayHosts := make(map[model.Hostname]bool) | ||
tlsRedirect := make(map[model.Hostname]bool) | ||
|
||
vHostDedupMap := make(map[model.Hostname]*route.VirtualHost) | ||
for _, server := range servers { | ||
// collect all the owning gateway names of these servers | ||
// we will only select virtual services pertaining to these gateways | ||
gatewaysForVSSelection[merged.GatewayNameForServer[server]] = true | ||
for _, host := range server.Hosts { | ||
gatewayHosts[model.Hostname(host)] = true | ||
if server.Tls != nil && server.Tls.HttpsRedirect { | ||
tlsRedirect[model.Hostname(host)] = true | ||
} | ||
} | ||
} | ||
gatewayName := merged.GatewayNameForServer[server] | ||
virtualServices := push.VirtualServices(node, map[string]bool{gatewayName: true}) | ||
for _, virtualService := range virtualServices { | ||
virtualServiceHosts := model.StringsToHostnames(virtualService.Spec.(*networking.VirtualService).Hosts) | ||
serverHosts := model.HostnamesForNamespace(server.Hosts, virtualService.Namespace) | ||
|
||
port := int(servers[0].Port.Number) | ||
// NOTE: WE DO NOT SUPPORT two gateways on same workload binding to same virtual service | ||
virtualServices := push.VirtualServices(node, gatewaysForVSSelection) | ||
vHostDedupMap := make(map[string]*route.VirtualHost) | ||
// We have two cases here: | ||
// 1. virtualService hosts are 1.foo.com, 2.foo.com, 3.foo.com and server hosts are ns/*.foo.com | ||
// 2. virtualService hosts are *.foo.com, and server hosts are ns/1.foo.com, ns/2.foo.com, ns/3.foo.com | ||
intersectingHosts := serverHosts.Intersection(virtualServiceHosts) | ||
if len(intersectingHosts) == 0 { | ||
log.Debugf("%s virtual service %q has no matching hosts for gateways %v server %d", node.ID, virtualService.Name, gatewayName, port) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove this.. this will be super noisy when debugging pilot |
||
continue | ||
} | ||
|
||
for _, v := range virtualServices { | ||
// We have two cases here: | ||
// 1. virtualService hosts are 1.foo.com, 2.foo.com, 3.foo.com and gateway's hosts are ns/*.foo.com | ||
// 2. virtualService hosts are *.foo.com, and gateway's hosts are ns/1.foo.com, ns/2.foo.com, ns/3.foo.com | ||
// The logic below only handles case 1. | ||
// TODO: handle case 2 | ||
matchingHosts := pickMatchingGatewayHosts(gatewayHosts, v) | ||
if len(matchingHosts) == 0 { | ||
log.Debugf("%s omitting virtual service %q because its hosts don't match gateways %v server %d", node.ID, v.Name, gateways, port) | ||
continue | ||
} | ||
routes, err := istio_route.BuildHTTPRoutesForVirtualService(node, push, v, nameToServiceMap, port, nil, gatewaysForVSSelection) | ||
if err != nil { | ||
log.Debugf("%s omitting routes for service %v due to error: %v", node.ID, v, err) | ||
continue | ||
} | ||
routes, err := istio_route.BuildHTTPRoutesForVirtualService(node, push, virtualService, nameToServiceMap, port, nil, map[string]bool{gatewayName: true}) | ||
if err != nil { | ||
log.Debugf("%s omitting routes for service %v due to error: %v", node.ID, virtualService, err) | ||
continue | ||
} | ||
|
||
for vsvcHost, gatewayHost := range matchingHosts { | ||
if currentVhost, exists := vHostDedupMap[vsvcHost]; exists { | ||
currentVhost.Routes = istio_route.CombineVHostRoutes(currentVhost.Routes, routes) | ||
} else { | ||
newVhost := &route.VirtualHost{ | ||
Name: fmt.Sprintf("%s:%d", vsvcHost, port), | ||
Domains: []string{vsvcHost, fmt.Sprintf("%s:%d", vsvcHost, port)}, | ||
Routes: routes, | ||
} | ||
if tlsRedirect[gatewayHost] { | ||
newVhost.RequireTls = route.VirtualHost_ALL | ||
for _, host := range intersectingHosts { | ||
if vHost, exists := vHostDedupMap[host]; exists { | ||
vHost.Routes = istio_route.CombineVHostRoutes(vHost.Routes, routes) | ||
} else { | ||
newVHost := &route.VirtualHost{ | ||
Name: fmt.Sprintf("%s:%d", host, port), | ||
Domains: []string{string(host), fmt.Sprintf("%s:%d", host, port)}, | ||
Routes: routes, | ||
} | ||
if server.Tls != nil && server.Tls.HttpsRedirect { | ||
newVHost.RequireTls = route.VirtualHost_ALL | ||
} | ||
vHostDedupMap[host] = newVHost | ||
} | ||
vHostDedupMap[vsvcHost] = newVhost | ||
} | ||
} | ||
} | ||
|
||
virtualHosts := make([]route.VirtualHost, 0, len(virtualServices)) | ||
var virtualHosts []route.VirtualHost | ||
if len(vHostDedupMap) == 0 { | ||
log.Warnf("constructed http route config for port %d with no vhosts; Setting up a default 404 vhost", port) | ||
virtualHosts = append(virtualHosts, route.VirtualHost{ | ||
virtualHosts = []route.VirtualHost{route.VirtualHost{ | ||
Name: fmt.Sprintf("blackhole:%d", port), | ||
Domains: []string{"*"}, | ||
Routes: []route.Route{ | ||
|
@@ -295,8 +282,9 @@ func (configgen *ConfigGeneratorImpl) buildGatewayHTTPRouteConfig(env *model.Env | |
}, | ||
}, | ||
}, | ||
}) | ||
}} | ||
} else { | ||
virtualHosts = make([]route.VirtualHost, 0, len(vHostDedupMap)) | ||
for _, v := range vHostDedupMap { | ||
virtualHosts = append(virtualHosts, *v) | ||
} | ||
|
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.
receiver name hosts should be consistent with previous receiver name h for Hostnames (from
golint
)