Skip to content

Commit

Permalink
Cleanup gateway vhost config gen (#12847)
Browse files Browse the repository at this point in the history 
* check match direction

* Cleanup http route generation

* undo pickMatching change

* golangbot comments

* address review comments

* fix validation bug

* gofmt

* check for intersection duplicates
  • Loading branch information
@frankbu @rshriram
frankbu authored and rshriram committed Apr 1, 2019
1 parent 67085ad commit 78f5101
Show file tree
Hide file tree
Showing 4 changed files with 222 additions and 56 deletions.
75 changes: 72 additions & 3 deletions pilot/pkg/model/service.go
View file
Expand Up @@ -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]) == "*"
Expand Down Expand Up @@ -639,6 +639,75 @@ func (h Hostnames) Swap(i, j int) {
h[i], h[j] = h[j], h[i]
}

func (h Hostnames) Contains(host Hostname) bool {
for _, hHost := range h {
if hHost == host {
return true
}
}
return false
}

// Intersection returns the subset of host names that are covered by both h 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([])
// Hostnames([]).Intersection(Hostnames(["bar.com"]) = Hostnames([])
func (h Hostnames) Intersection(other Hostnames) Hostnames {
result := make(Hostnames, 0, len(h))
for _, hHost := range h {
for _, oHost := range other {
if hHost.SubsetOf(oHost) {
if !result.Contains(hHost) {
result = append(result, hHost)
}
} else if oHost.SubsetOf(hHost) {
if !result.Contains(oHost) {
result = append(result, oHost)
}
}
}
}
return result
}

// 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/ or */.
// If not qualified or qualified with *, the host name is considered to be in every namespace.
// e.g.:
// HostnamesForNamespace(["ns1/foo.com","ns2/bar.com"], "ns1") = Hostnames(["foo.com"])
// HostnamesForNamespace(["ns1/foo.com","ns2/bar.com"], "ns3") = Hostnames([])
// HostnamesForNamespace(["ns1/foo.com","*/bar.com"], "ns1") = Hostnames(["foo.com","bar.com"])
// HostnamesForNamespace(["ns1/foo.com","*/bar.com"], "ns3") = Hostnames(["bar.com"])
// HostnamesForNamespace(["foo.com","ns2/bar.com"], "ns2") = Hostnames(["foo.com","bar.com"])
// HostnamesForNamespace(["foo.com","ns2/bar.com"], "ns3") = Hostnames(["foo.com"])
func HostnamesForNamespace(hosts []string, namespace string) Hostnames {
result := make(Hostnames, 0, len(hosts))
for _, host := range hosts {
if strings.Contains(host, "/") {
parts := strings.Split(host, "/")
if parts[0] != namespace && parts[0] != "*" {
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 {
Expand Down
114 changes: 114 additions & 0 deletions pilot/pkg/model/service_test.go
View file
Expand Up @@ -351,6 +351,120 @@ func BenchmarkMatch(b *testing.B) {
}
}

func TestHostnamesIntersection(t *testing.T) {
tests := []struct {
a, b, intersection Hostnames
}{
{
Hostnames{"foo,com"},
Hostnames{"bar.com"},
Hostnames{},
},
{
Hostnames{"foo.com", "bar.com"},
Hostnames{"bar.com"},
Hostnames{"bar.com"},
},
{
Hostnames{"foo.com", "bar.com"},
Hostnames{"*.com"},
Hostnames{"foo.com", "bar.com"},
},
{
Hostnames{"*.com"},
Hostnames{"foo.com", "bar.com"},
Hostnames{"foo.com", "bar.com"},
},
{
Hostnames{"foo.com", "*.net"},
Hostnames{"*.com", "bar.net"},
Hostnames{"foo.com", "bar.net"},
},
{
Hostnames{"foo.com", "*.net"},
Hostnames{"*.bar.net"},
Hostnames{"*.bar.net"},
},
{
Hostnames{"foo.com", "bar.net"},
Hostnames{"*"},
Hostnames{"foo.com", "bar.net"},
},
{
Hostnames{"foo.com"},
Hostnames{},
Hostnames{},
},
{
Hostnames{},
Hostnames{"bar.com"},
Hostnames{},
},
{
Hostnames{"*", "foo.com"},
Hostnames{"foo.com"},
Hostnames{"foo.com"},
},
}

for idx, tt := range tests {
t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
result := tt.a.Intersection(tt.b)
if !reflect.DeepEqual(result, tt.intersection) {
t.Fatalf("%v.Intersection(%v) = %v, want %v", tt.a, tt.b, result, tt.intersection)
}
})
}
}

func TestHostnamesForNamespace(t *testing.T) {
tests := []struct {
hosts []string
namespace string
want Hostnames
}{
{
[]string{"ns1/foo.com", "ns2/bar.com"},
"ns1",
Hostnames{"foo.com"},
},
{
[]string{"ns1/foo.com", "ns2/bar.com"},
"ns3",
Hostnames{},
},
{
[]string{"ns1/foo.com", "*/bar.com"},
"ns1",
Hostnames{"foo.com", "bar.com"},
},
{
[]string{"ns1/foo.com", "*/bar.com"},
"ns3",
Hostnames{"bar.com"},
},
{
[]string{"foo.com", "ns2/bar.com"},
"ns2",
Hostnames{"foo.com", "bar.com"},
},
{
[]string{"foo.com", "ns2/bar.com"},
"ns3",
Hostnames{"foo.com"},
},
}

for idx, tt := range tests {
t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) {
result := HostnamesForNamespace(tt.hosts, tt.namespace)
if !reflect.DeepEqual(result, tt.want) {
t.Fatalf("HostnamesForNamespace(%v, %v) = %v, want %v", tt.hosts, tt.namespace, result, tt.want)
}
})
}
}

func TestHostnamesSortOrder(t *testing.T) {
tests := []struct {
in, want Hostnames
Expand Down
4 changes: 0 additions & 4 deletions pilot/pkg/model/validation.go
View file
Expand Up @@ -446,10 +446,6 @@ func validateServer(server *networking.Server) (errs error) {
errs = appendErrors(errs, fmt.Errorf("server config must contain at least one host"))
} else {
for _, host := range server.Hosts {
// short name hosts are not allowed in gateways
if host != "*" && !strings.Contains(host, ".") {
errs = appendErrors(errs, fmt.Errorf("short names (non FQDN) are not allowed in Gateway server hosts"))
}
errs = appendErrors(errs, validateNamespaceSlashWildcardHostname(host, true))
}
}
Expand Down
85 changes: 36 additions & 49 deletions pilot/pkg/networking/core/v1alpha3/gateway.go
View file
Expand Up @@ -216,71 +216,57 @@ 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 {
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{
Expand All @@ -295,8 +281,9 @@ func (configgen *ConfigGeneratorImpl) buildGatewayHTTPRouteConfig(env *model.Env
},
},
},
})
}}
} else {
virtualHosts = make([]route.VirtualHost, 0, len(vHostDedupMap))
for _, v := range vHostDedupMap {
virtualHosts = append(virtualHosts, *v)
}
Expand Down

0 comments on commit 78f5101

Please sign in to comment.