istio · rshriram · Jul 17, 2018 · Jul 17, 2018 · Jul 17, 2018 · Jul 17, 2018
diff --git a/pilot/pkg/config/monitor/monitor_test.go b/pilot/pkg/config/monitor/monitor_test.go
@@ -61,7 +61,7 @@ var updateConfigSet = []*model.Config{
 				{
 					Port: &networking.Port{
 						Number:   80,
-						Protocol: "HTTPS",
+						Protocol: "HTTP2",
 						Name:     "http",
 					},
 					Hosts: []string{"*.example.com"},
@@ -123,7 +123,7 @@ func TestMonitorForChange(t *testing.T) {
 		g.Expect(err).NotTo(gomega.HaveOccurred())
 
 		gateway := c[0].Spec.(*networking.Gateway)
-		if gateway.Servers[0].Port.Protocol != "HTTPS" {
+		if gateway.Servers[0].Port.Protocol != "HTTP2" {
 			return errors.New("Protocol has not been updated")
 		}
 

diff --git a/pilot/pkg/model/config.go b/pilot/pkg/model/config.go
@@ -605,6 +605,7 @@ func (store *istioConfigStore) VirtualServices(gateways map[string]bool) []Confi
 		return nil
 	}
 
+	// TODO Sort configs by creation time
 	out := make([]Config, 0)
 	for _, config := range configs {
 		rule := config.Spec.(*networking.VirtualService)

diff --git a/pilot/pkg/model/validation.go b/pilot/pkg/model/validation.go
@@ -411,7 +411,30 @@ func validateServer(server *networking.Server) (errs error) {
 			}
 		}
 	}
-	return appendErrors(errs, validateTLSOptions(server.Tls), validateServerPort(server.Port))
+	portErr := validateServerPort(server.Port)
+	if portErr != nil {
+		errs = appendErrors(errs, portErr)
+	}
+	errs = appendErrors(errs, validateTLSOptions(server.Tls))
+
+	// If port is HTTPS or TLS, make sure that server has TLS options
+	if portErr == nil {
+		protocol := ParseProtocol(server.Port.Protocol)
+		if protocol.IsTLS() && server.Tls == nil {
+			errs = appendErrors(errs, fmt.Errorf("server must have TLS settings for HTTPS/TLS protocols"))
+		} else if !protocol.IsTLS() && server.Tls != nil {
+			// only tls redirect is allowed if this is a HTTP server
+			if protocol.IsHTTP() {
+				if server.Tls.Mode != networking.Server_TLSOptions_PASSTHROUGH ||
+					server.Tls.CaCertificates != "" || server.Tls.PrivateKey != "" || server.Tls.ServerCertificate != "" {
+					errs = appendErrors(errs, fmt.Errorf("server cannot have TLS settings for plain text HTTP ports"))
+				}
+			} else {
+				errs = appendErrors(errs, fmt.Errorf("server cannot have TLS settings for non HTTPS/TLS ports"))
+			}
+		}
+	}
+	return errs
 }
 
 func validateServerPort(port *networking.Port) (errs error) {

diff --git a/pilot/pkg/model/validation_test.go b/pilot/pkg/model/validation_test.go
@@ -1132,6 +1132,28 @@ func TestValidateServer(t *testing.T) {
 				Tls:   &networking.Server_TLSOptions{Mode: networking.Server_TLSOptions_SIMPLE},
 			},
 			"TLS"},
+		{"no tls on HTTPS",
+			&networking.Server{
+				Hosts: []string{"foo.bar.com"},
+				Port:  &networking.Port{Number: 10000, Name: "https", Protocol: "https"},
+			},
+			"must have TLS"},
+		{"tls on HTTP",
+			&networking.Server{
+				Hosts: []string{"foo.bar.com"},
+				Port:  &networking.Port{Number: 10000, Name: "http", Protocol: "http"},
+				Tls:   &networking.Server_TLSOptions{Mode: networking.Server_TLSOptions_SIMPLE},
+			},
+			"cannot have TLS"},
+		{"tls redirect on HTTP",
+			&networking.Server{
+				Hosts: []string{"foo.bar.com"},
+				Port:  &networking.Port{Number: 10000, Name: "http", Protocol: "http"},
+				Tls: &networking.Server_TLSOptions{
+					HttpsRedirect: true,
+				},
+			},
+			""},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pilot/pkg/networking/core/v1alpha3/gateway.go b/pilot/pkg/networking/core/v1alpha3/gateway.go
@@ -162,7 +162,17 @@ func (configgen *ConfigGeneratorImpl) buildGatewayListeners(env *model.Environme
 		// we have some listeners to return, but we also have some errors; log them
 		log.Info(err.Error())
 	}
-	return listeners, nil
+
+	validatedListeners := make([]*xdsapi.Listener, 0, len(merged.Servers))
+	for _, l := range listeners {
+		if err := l.Validate(); err != nil {
+			log.Warnf("buildGatewayListeners: error validating listener %s: %v.. Skipping.", l.Name, err)
+			continue
+		}
+		validatedListeners = append(validatedListeners, l)
+	}
+
+	return validatedListeners, nil
 }
 
 func (configgen *ConfigGeneratorImpl) buildGatewayHTTPRouteConfig(env *model.Environment, node *model.Proxy,
@@ -196,11 +206,89 @@ func (configgen *ConfigGeneratorImpl) buildGatewayHTTPRouteConfig(env *model.Env
 		nameToServiceMap[svc.Hostname] = svc
 	}
 
-	routeCfg := configgen.buildGatewayInboundHTTPRouteConfig(env, node, nameToServiceMap, merged.Names, servers, routeName)
-	log.Debugf("Returning route config (%s) %v", routeName, routeCfg)
+	gatewayHosts := make(map[model.Hostname]bool)
+	tlsRedirect := make(map[model.Hostname]bool)
+
+	for _, server := range servers {
+		for _, host := range server.Hosts {
+			gatewayHosts[model.Hostname(host)] = true
+			if server.Tls != nil && server.Tls.HttpsRedirect {
+				tlsRedirect[model.Hostname(host)] = true
+			}
+		}
+	}
+
+	port := int(servers[0].Port.Number)
+	// NOTE: WE DO NOT SUPPORT two gateways on same workload binding to same virtual service
+	virtualServices := env.VirtualServices(merged.Names)
+	virtualHosts := make([]route.VirtualHost, 0, len(virtualServices))
+	for _, v := range virtualServices {
+		vs := v.Spec.(*networking.VirtualService)
+		matchingHosts := pickMatchingGatewayHosts(gatewayHosts, vs.Hosts)
+		if len(matchingHosts) == 0 {
+			log.Infof("%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(v, nameToServiceMap, port, nil, merged.Names, env.IstioConfigStore)
+		if err != nil {
+			log.Warnf("%s omitting routes for service %v due to error: %v", node.ID, v, err)
+			continue
+		}
+
+		for vsvcHost, gatewayHost := range matchingHosts {
+			host := route.VirtualHost{
+				Name:    fmt.Sprintf("%s:%d", v.Name, port),
+				Domains: []string{vsvcHost},
+				Routes:  routes,
+			}
+
+			if tlsRedirect[gatewayHost] {
+				host.RequireTls = route.VirtualHost_ALL
+			}
+			virtualHosts = append(virtualHosts, host)
+		}
+	}
+
+	if len(virtualHosts) == 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{
+			Name:    fmt.Sprintf("blackhole:%d", port),
+			Domains: []string{"*"},
+			Routes: []route.Route{
+				{
+					Match: route.RouteMatch{
+						PathSpecifier: &route.RouteMatch_Prefix{Prefix: "/"},
+					},
+					Action: &route.Route_DirectResponse{
+						DirectResponse: &route.DirectResponseAction{
+							Status: 404,
+						},
+					},
+				},
+			},
+		})
+	}
+	util.SortVirtualHosts(virtualHosts)
+
+	routeCfg := &xdsapi.RouteConfiguration{
+		Name:             routeName,
+		VirtualHosts:     virtualHosts,
+		ValidateClusters: boolFalse,
+	}
+	// call plugins
+	for _, p := range configgen.Plugins {
+		in := &plugin.InputParams{
+			ListenerProtocol: plugin.ListenerProtocolHTTP,
+			Env:              env,
+			Node:             node,
+		}
+		p.OnOutboundRouteConfiguration(in, routeCfg)
+	}
+
 	return routeCfg, nil
 }
 
+// to process HTTP and HTTPS servers
 func (configgen *ConfigGeneratorImpl) createGatewayHTTPFilterChainOpts(
 	env *model.Environment, node *model.Proxy, push *model.PushStatus, servers []*networking.Server, gatewayNames map[string]bool) []*filterChainOpts {
 
@@ -218,17 +306,15 @@ func (configgen *ConfigGeneratorImpl) createGatewayHTTPFilterChainOpts(
 	httpListeners := make([]*filterChainOpts, 0, len(servers))
 	// Are we processing plaintext servers or TLS servers?
 	// If plain text, we have to combine all servers into a single listener
-	if model.ParseProtocol(servers[0].Port.Protocol) == model.ProtocolHTTP {
+	if model.ParseProtocol(servers[0].Port.Protocol).IsHTTP() {
 		rdsName := model.GatewayRDSRouteName(servers[0])
-		routeCfg := configgen.buildGatewayInboundHTTPRouteConfig(env, node, nameToServiceMap, gatewayNames, servers, rdsName)
 		o := &filterChainOpts{
 			// This works because we validate that only HTTPS servers can have same port but still different port names
 			// and that no two non-HTTPS servers can be on same port or share port names.
 			// Validation is done per gateway and also during merging
 			sniHosts:   nil,
 			tlsContext: nil,
 			httpOpts: &httpListenerOpts{
-				routeConfig:      routeCfg,
 				rds:              rdsName,
 				useRemoteAddress: true,
 				direction:        http_conn.EGRESS, // viewed as from gateway to internal
@@ -237,21 +323,14 @@ func (configgen *ConfigGeneratorImpl) createGatewayHTTPFilterChainOpts(
 		httpListeners = append(httpListeners, o)
 	} else {
 		// Build a filter chain for each TLS server
-		for i, server := range servers {
-			rdsName := model.GatewayRDSRouteName(server)
-			routeCfg := configgen.buildGatewayInboundHTTPRouteConfig(env, node, nameToServiceMap, gatewayNames, []*networking.Server{server}, rdsName)
-			if routeCfg == nil {
-				log.Warnf("omitting HTTP listeners for port %d filter chain %d due to no routes", server.Port, i)
-				continue
-			}
+		for _, server := range servers {
 			o := &filterChainOpts{
 				// This works because we validate that only HTTPS servers can have same port but still different port names
 				// and that no two non-HTTPS servers can be on same port or share port names.
 				// Validation is done per gateway and also during merging
 				sniHosts:   getSNIHosts(server),
 				tlsContext: buildGatewayListenerTLSContext(server),
 				httpOpts: &httpListenerOpts{
-					routeConfig:      routeCfg,
 					rds:              model.GatewayRDSRouteName(server),
 					useRemoteAddress: true,
 					direction:        http_conn.EGRESS, // viewed as from gateway to internal
@@ -314,97 +393,6 @@ func buildGatewayListenerTLSContext(server *networking.Server) *auth.DownstreamT
 	}
 }
 
-// TODO: Once RDS is permanent, merge this function with buildGatewayHTTPRouteConfig
-func (configgen *ConfigGeneratorImpl) buildGatewayInboundHTTPRouteConfig(
-	env *model.Environment,
-	node *model.Proxy,
-	svcs map[model.Hostname]*model.Service,
-	gateways map[string]bool,
-	servers []*networking.Server,
-	routeName string) *xdsapi.RouteConfiguration {
-
-	gatewayHosts := make(map[model.Hostname]bool)
-	tlsRedirect := make(map[model.Hostname]bool)
-
-	for _, server := range servers {
-		for _, host := range server.Hosts {
-			gatewayHosts[model.Hostname(host)] = true
-			if server.Tls != nil && server.Tls.HttpsRedirect {
-				tlsRedirect[model.Hostname(host)] = true
-			}
-		}
-	}
-
-	port := int(servers[0].Port.Number)
-	// NOTE: WE DO NOT SUPPORT two gateways on same workload binding to same virtual service
-	virtualServices := env.VirtualServices(gateways)
-	virtualHosts := make([]route.VirtualHost, 0, len(virtualServices))
-	for _, v := range virtualServices {
-		vs := v.Spec.(*networking.VirtualService)
-		matchingHosts := pickMatchingGatewayHosts(gatewayHosts, vs.Hosts)
-		if len(matchingHosts) == 0 {
-			log.Infof("%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(v, svcs, port, nil, gateways, env.IstioConfigStore)
-		if err != nil {
-			log.Warnf("%s omitting routes for service %v due to error: %v", node.ID, v, err)
-			continue
-		}
-
-		for vsvcHost, gatewayHost := range matchingHosts {
-			host := route.VirtualHost{
-				Name:    fmt.Sprintf("%s:%d", v.Name, port),
-				Domains: []string{vsvcHost},
-				Routes:  routes,
-			}
-
-			if tlsRedirect[gatewayHost] {
-				host.RequireTls = route.VirtualHost_ALL
-			}
-			virtualHosts = append(virtualHosts, host)
-		}
-	}
-
-	if len(virtualHosts) == 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{
-			Name:    fmt.Sprintf("blackhole:%d", port),
-			Domains: []string{"*"},
-			Routes: []route.Route{
-				{
-					Match: route.RouteMatch{
-						PathSpecifier: &route.RouteMatch_Prefix{Prefix: "/"},
-					},
-					Action: &route.Route_DirectResponse{
-						DirectResponse: &route.DirectResponseAction{
-							Status: 404,
-						},
-					},
-				},
-			},
-		})
-	}
-	util.SortVirtualHosts(virtualHosts)
-
-	out := &xdsapi.RouteConfiguration{
-		Name:             routeName,
-		VirtualHosts:     virtualHosts,
-		ValidateClusters: boolFalse,
-	}
-	// call plugins
-	for _, p := range configgen.Plugins {
-		in := &plugin.InputParams{
-			ListenerProtocol: plugin.ListenerProtocolHTTP,
-			Env:              env,
-			Node:             node,
-		}
-		p.OnOutboundRouteConfiguration(in, out)
-	}
-
-	return out
-}
-
 func createGatewayTCPFilterChainOpts(
 	env *model.Environment, servers []*networking.Server, gatewayNames map[string]bool) []*filterChainOpts {
 

diff --git a/pilot/pkg/networking/core/v1alpha3/listener.go b/pilot/pkg/networking/core/v1alpha3/listener.go
@@ -193,8 +193,6 @@ func (configgen *ConfigGeneratorImpl) buildSidecarListeners(env *model.Environme
 			protocol:       model.ProtocolHTTP,
 			filterChainOpts: []*filterChainOpts{{
 				httpOpts: &httpListenerOpts{
-					routeConfig: configgen.buildSidecarOutboundHTTPRouteConfig(env, node, proxyInstances,
-						services, RDSHttpProxy),
 					rds:              RDSHttpProxy,
 					useRemoteAddress: useRemoteAddress,
 					direction:        traceOperation,
@@ -470,9 +468,7 @@ func (configgen *ConfigGeneratorImpl) buildSidecarOutboundListeners(env *model.E
 				listenerOpts.protocol = servicePort.Protocol
 				listenerOpts.filterChainOpts = []*filterChainOpts{{
 					httpOpts: &httpListenerOpts{
-						rds: fmt.Sprintf("%d", servicePort.Port),
-						routeConfig: configgen.buildSidecarOutboundHTTPRouteConfig(
-							env, node, proxyInstances, services, fmt.Sprintf("%d", servicePort.Port)),
+						rds:              fmt.Sprintf("%d", servicePort.Port),
 						useRemoteAddress: useRemoteAddress,
 						direction:        operation,
 					},