Cherry picking bug fixes from master into release-1.0 (#6834)

* fix bug so that destination.service.** attributes are collected (#6801)

* remove unnecessary generated attributes finding. (#6785)

* modify docker template files for proxyv2 (#6790)

* Long-running testing improvements (#6800)

* Add values for config map settings, including access log.
More docs.

* Updates and improvements for the stress-testing configs.

* Add values for config map settings, including access log. (#6797)

* Add values for config map settings, including access log.
More docs.

* Updates and improvements for the stress-testing configs.

* Address review comments

* Merged wrong files

* Add the setup helm file - this change now depend on the previous PR.

* Sync with remote, remove accidentally added files.

* Another accidental file

* SNI routing from sidecar to gateway with virtual services (#6402)

* quick sni matching 1st pass with no refactoring of existing code

* use shriram's api sha

* quick pass at using tls block

* add some validation

* copyright

* fix lint + remove deadcode

* rename protocol tcp_tls -> tls

* update back to istio/api master

* remove accidentally added test file

* add tls block to gateway logic

* add todos

* basic sni wildcard implementation

* add tcp, fix problems with rbac, matching

* better tcp + tls validation

* address code review comments

* remove out of date comment

* update comments

* fix compile error

* use tcp proxy in tcp routing

* add tcp routing e2e test

* add forgotten vs config file + update description of test

* Comments, bug fixes

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* cleanup gateway tcp test

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* moving networking test yamls

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* tcp/tls tests

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* yaml fixes

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* fix file switcheroo

* port matches

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* fix authN plugin overwriting TLS context

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* more tests - route via egress gateway

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* yaml fixes

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* initialize prom variables

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* split tests

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* final test fix hopefully

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

* revert gateway tweaks

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

install/consul/templates/istio.yaml.tmpl

@@ -57,6 +57,8 @@ services:
         aliases:
           - istio-pilot
     expose:
+      - "15003"
+      - "15005"
       - "15007"
     ports:
       - "8081:15007"

install/eureka/templates/istio.yaml.tmpl

@@ -39,6 +39,8 @@ services:
   istio-pilot:
     image: {PILOT_HUB}/pilot:{PILOT_TAG}
     expose:
+      - "15003"
+      - "15005"
       - "15007"
     ports:
       - "8081:15007"

install/kubernetes/helm/istio-remote/templates/sidecar-injector-configmap.yaml

@@ -11,7 +11,7 @@ metadata:
     istio: sidecar-injector
 data:
   config: |-
-    policy: {{ .Values.global.proxy.policy }}
+    policy: {{ .Values.global.proxy.autoInject }}
     template: |-
       initContainers:
       - name: istio-init

install/kubernetes/helm/istio-remote/values.yaml

@@ -37,7 +37,9 @@ global:
     #     Redirect only selected ports:            --includeInboundPorts="80,8080"
     includeInboundPorts: "*"
     excludeInboundPorts: ""
-    policy: enabled
+
+    # This controls the 'policy' in the sidecar injector.
+    autoInject: enabled
 
     # Sets the destination Statsd in envoy (the value of the "--statsdUdpAddress" proxy argument
     # would be <host>:<port>).

install/kubernetes/helm/istio/templates/configmap.yaml

@@ -12,25 +12,21 @@ data:
   mesh: |-
     # Set the following variable to true to disable policy checks by the Mixer.
     # Note that metrics will still be reported to the Mixer.
-    disablePolicyChecks: false
+    disablePolicyChecks: {{ .Values.global.disablePolicyChecks }}
+
     # Set enableTracing to false to disable request tracing.
-    enableTracing: true
+    enableTracing: {{ .Values.global.enableTracing }}
+
     # Set accessLogFile to empty string to disable access log.
-    accessLogFile: "/dev/stdout"
+    accessLogFile: "{{ .Values.global.proxy.accessLogFile }}"
     #
     # To disable the mixer completely (including metrics), comment out
     # the following lines
+    # Deprecated: mixer is using EDS
     mixerCheckServer: istio-policy.{{ .Release.Namespace }}.svc.cluster.local:15004
     mixerReportServer: istio-telemetry.{{ .Release.Namespace }}.svc.cluster.local:15004
     # This is the ingress service name, update if you used a different name
     ingressService: istio-ingress
-    #
-    # Along with discoveryRefreshDelay, this setting determines how
-    # frequently should Envoy fetch and update its internal configuration
-    # from istio Pilot. Lower refresh delay results in higher CPU
-    # utilization and potential performance loss in exchange for faster
-    # convergence. Tweak this value according to your setup.
-    rdsRefreshDelay: {{ .Values.global.refreshInterval }}
 
     # Unix Domain Socket through which envoy communicates with NodeAgent SDS to get
     # key/cert for mTLS. Use secret-mount files instead of SDS if set to empty. 
@@ -41,10 +37,6 @@ data:
 
     #
     defaultConfig:
-      # NOTE: If you change any values in this section, make sure to make
-      # the same changes in start up args in istio-ingress pods.
-      # See rdsRefreshDelay for explanation about this setting.
-      discoveryRefreshDelay: {{ .Values.global.refreshInterval }}
       #
       # TCP connection timeout between Envoy & the application, and between Envoys.
       connectTimeout: 10s

install/kubernetes/helm/istio/templates/sidecar-injector-configmap.yaml

@@ -11,7 +11,7 @@ metadata:
     istio: sidecar-injector
 data:
   config: |-
-    policy: {{ .Values.global.proxy.policy }}
+    policy: {{ .Values.global.proxy.autoInject }}
     template: |-
       initContainers:
       - name: istio-init

install/kubernetes/helm/istio/values.yaml

@@ -8,19 +8,20 @@ global:
   # Default repository for Istio images.
   # Releases are published to docker hub under 'istio' project.
   # Daily builds from prow are on gcr.io, and nightly builds from circle on
-  # docker.io/istionightly
-  hub: docker.io/istio
+  hub: docker.io/istionightly
 
   # Default tag for Istio images.
-  # Should track latest released version in the branch.
-  tag: 0.8.latest
+  tag: nightly-master
+
   proxy:
     image: proxyv2
 
-    # istio-sidecar-injector configmap stores configuration for sidecar injection.
-    # This config map is used by istioctl kube-inject and the injector webhook.
+    # Configures the access log for each sidecar. Setting it to an empty string will
+    # disable access log for sidecar.
+    accessLogFile: "/dev/stdout"
+
+    # If set, newly injected sidecars will have core dumps enabled.
     enableCoreDump: false
-    replicaCount: 1
 
     # istio egress capture whitelist
     # https://istio.io/docs/tasks/traffic-management/egress.html#calling-external-services-directly
@@ -37,7 +38,9 @@ global:
     #     Redirect only selected ports:            --includeInboundPorts="80,8080"
     includeInboundPorts: "*"
     excludeInboundPorts: ""
-    policy: enabled
+
+    # This controls the 'policy' in the sidecar injector.
+    autoInject: enabled
 
     # Sets the destination Statsd in envoy (the value of the "--statsdUdpAddress" proxy argument
     # would be <host>:<port>).
@@ -64,6 +67,13 @@ global:
   # propagated, not recommended for tests.
   controlPlaneSecurityEnabled: false
 
+  # disablePolicyChecks disables mixer policy checks.
+  # Will set the value with same name in istio config map - pilot needs to be restarted to take effect.
+  disablePolicyChecks: false
+
+  # EnableTracing sets the value with same name in istio config map, requires pilot restart to take effect.
+  enableTracing: true
+
   # Default mtls policy. If true, mtls between services will be enabled by default.
   mtls:
     # Default setting for service-to-service mtls. Can be set explicitly using
@@ -151,7 +161,15 @@ gateways:
       istio: ingressgateway
     replicaCount: 1
     autoscaleMin: 1
-    autoscaleMax: 1
+    autoscaleMax: 5
+    resources: {}
+      # limits:
+      #  cpu: 100m
+      #  memory: 128Mi
+      #requests:
+      #  cpu: 1800m
+      #  memory: 256Mi
+
     loadBalancerIP: ""
     serviceAnnotations: {}
     type: LoadBalancer #change to NodePort, ClusterIP or LoadBalancer if need be

mixer/template/sample/template.gen.go

@@ -436,7 +436,7 @@ var (
 				outBag := newWrapperAttrBag(
 					func(name string) (value interface{}, found bool) {
 						field := strings.TrimPrefix(name, fullOutName)
-						if len(field) != len(name) && out.WasSet(field) {
+						if len(field) != len(name) {
 							switch field {
 
 							case "int64Primitive":

mixer/template/template.gen.go

@@ -327,7 +327,7 @@ var (
 				outBag := newWrapperAttrBag(
 					func(name string) (value interface{}, found bool) {
 						field := strings.TrimPrefix(name, fullOutName)
-						if len(field) != len(name) && out.WasSet(field) {
+						if len(field) != len(name) {
 							switch field {
 
 							case "source_pod_ip":

mixer/test/spyAdapter/template/template.gen.go

@@ -236,7 +236,7 @@ var (
 				outBag := newWrapperAttrBag(
 					func(name string) (value interface{}, found bool) {
 						field := strings.TrimPrefix(name, fullOutName)
-						if len(field) != len(name) && out.WasSet(field) {
+						if len(field) != len(name) {
 							switch field {
 
 							case "int64Primitive":

mixer/tools/codegen/pkg/bootstrapgen/template/bootstrapTemplate.go

@@ -338,7 +338,7 @@ var (
             outBag := newWrapperAttrBag(
                 func(name string) (value interface{}, found bool) {
                     field := strings.TrimPrefix(name, fullOutName)
-                    if len(field) != len(name) && out.WasSet(field) {
+                    if len(field) != len(name) {
                         switch field {
                             {{range .OutputTemplateMessage.Fields}}
                             case "{{.ProtoName}}":

mixer/tools/codegen/pkg/bootstrapgen/testdata/template.gen.go.golden

@@ -464,7 +464,7 @@ var (
 				outBag := newWrapperAttrBag(
 					func(name string) (value interface{}, found bool) {
 						field := strings.TrimPrefix(name, fullOutName)
-						if len(field) != len(name) && out.WasSet(field) {
+						if len(field) != len(name) {
 							switch field {
 
 							case "int64Primitive":

pilot/pkg/model/service.go

@@ -143,9 +143,9 @@ const (
 	// ProtocolTCP declares the the port uses TCP.
 	// This is the default protocol for a service port.
 	ProtocolTCP Protocol = "TCP"
-	// ProtocolTCPTLS declares that the port carries TLS traffic on top of TCP
-	// TLS traffic is assumed to contain SNI as part of the handshake
-	ProtocolTCPTLS Protocol = "TCP_TLS"
+	// ProtocolTLS declares that the port carries TLS traffic.
+	// TLS traffic is assumed to contain SNI as part of the handshake.
+	ProtocolTLS Protocol = "TLS"
 	// ProtocolUDP declares that the port uses UDP.
 	// Note that UDP protocol is not currently supported by the proxy.
 	ProtocolUDP Protocol = "UDP"
@@ -204,8 +204,8 @@ func ParseProtocol(s string) Protocol {
 		return ProtocolHTTP2
 	case "https":
 		return ProtocolHTTPS
-	case "tcp_tls":
-		return ProtocolTCPTLS
+	case "tls":
+		return ProtocolTLS
 	case "mongo":
 		return ProtocolMongo
 	case "redis":
@@ -238,7 +238,7 @@ func (p Protocol) IsHTTP() bool {
 // IsTCP is true for protocols that use TCP as transport protocol
 func (p Protocol) IsTCP() bool {
 	switch p {
-	case ProtocolTCP, ProtocolHTTPS, ProtocolTCPTLS, ProtocolMongo, ProtocolRedis:
+	case ProtocolTCP, ProtocolHTTPS, ProtocolTLS, ProtocolMongo, ProtocolRedis:
 		return true
 	default:
 		return false
@@ -248,7 +248,7 @@ func (p Protocol) IsTCP() bool {
 // IsTLS is true for protocols on top of TLS (e.g. HTTPS)
 func (p Protocol) IsTLS() bool {
 	switch p {
-	case ProtocolHTTPS, ProtocolTCPTLS:
+	case ProtocolHTTPS, ProtocolTLS:
 		return true
 	default:
 		return false
@@ -539,7 +539,7 @@ func (h Hostnames) Swap(i, j int) {
 	h[i], h[j] = h[j], h[i]
 }
 
-// SubsetOf is true if the tag has identical values for the keys
+// SubsetOf is true if the label has identical values for the keys
 func (l Labels) SubsetOf(that Labels) bool {
 	for k, v := range l {
 		if that[k] != v {

pilot/pkg/model/validation.go

@@ -1958,14 +1958,81 @@ func ValidateVirtualService(name, namespace string, msg proto.Message) (errs err
 		}
 	}
 
-	if len(virtualService.Http) == 0 && len(virtualService.Tcp) == 0 {
-		errs = appendErrors(errs, fmt.Errorf("http or tcp must be provided in virtual service"))
+	if len(virtualService.Http) == 0 && len(virtualService.Tcp) == 0 && len(virtualService.Tls) == 0 {
+		errs = appendErrors(errs, errors.New("http, tcp or tls must be provided in virtual service"))
 	}
 	for _, httpRoute := range virtualService.Http {
 		errs = appendErrors(errs, validateHTTPRoute(httpRoute))
 	}
-	// TODO: validate TCP
+	for _, tlsRoute := range virtualService.Tls {
+		errs = appendErrors(errs, validateTLSRoute(tlsRoute))
+	}
+	for _, tcpRoute := range virtualService.Tcp {
+		errs = appendErrors(errs, validateTCPRoute(tcpRoute))
+	}
+
+	return
+}
+
+func validateTLSRoute(tls *networking.TLSRoute) (errs error) {
+	if tls == nil {
+		return nil
+	}
+
+	if len(tls.Match) == 0 {
+		errs = appendErrors(errs, errors.New("TLS route must have at least one match condition"))
+	}
+	for _, match := range tls.Match {
+		errs = appendErrors(errs, validateTLSMatch(match))
+	}
+	if len(tls.Route) != 1 {
+		errs = appendErrors(errs, errors.New("TLS route must have exactly one destination"))
+	}
+	errs = appendErrors(errs, validateDestinationWeights(tls.Route))
+	return
+}
+
+func validateTLSMatch(match *networking.TLSMatchAttributes) (errs error) {
+	if len(match.SniHosts) == 0 {
+		errs = appendErrors(errs, fmt.Errorf("TLS match must have at least one SNI host"))
+	}
+	if match.DestinationSubnet != "" {
+		errs = appendErrors(errs, ValidateIPv4Subnet(match.DestinationSubnet))
+	}
+	if match.Port != 0 {
+		errs = appendErrors(errs, ValidatePort(int(match.Port)))
+	}
+	errs = appendErrors(errs, Labels(match.SourceLabels).Validate())
+	errs = appendErrors(errs, validateGatewayNames(match.Gateways))
+	return
+}
+
+func validateTCPRoute(tcp *networking.TCPRoute) (errs error) {
+	if tcp == nil {
+		return nil
+	}
+	for _, match := range tcp.Match {
+		errs = appendErrors(errs, validateTCPMatch(match))
+	}
+	if len(tcp.Route) != 1 {
+		errs = appendErrors(errs, errors.New("TLS route must have exactly one destination"))
+	}
+	errs = appendErrors(errs, validateDestinationWeights(tcp.Route))
+	return
+}
 
+func validateTCPMatch(match *networking.L4MatchAttributes) (errs error) {
+	if match.DestinationSubnet != "" {
+		errs = appendErrors(errs, ValidateIPv4Subnet(match.DestinationSubnet))
+	}
+	if match.SourceSubnet != "" {
+		errs = appendErrors(errs, ValidateIPv4Subnet(match.SourceSubnet))
+	}
+	if match.Port != 0 {
+		errs = appendErrors(errs, ValidatePort(int(match.Port)))
+	}
+	errs = appendErrors(errs, Labels(match.SourceLabels).Validate())
+	errs = appendErrors(errs, validateGatewayNames(match.Gateways))
 	return
 }
 
@@ -2013,29 +2080,46 @@ func validateHTTPRoute(http *networking.HTTPRoute) (errs error) {
 			errs = appendErrors(errs, ValidateHTTPHeaderName(name))
 		}
 
-		// TODO: validate match.Port
+		if match.Port != 0 {
+			errs = appendErrors(errs, ValidatePort(int(match.Port)))
+		}
 		errs = appendErrors(errs, Labels(match.SourceLabels).Validate())
+		errs = appendErrors(errs, validateGatewayNames(match.Gateways))
 	}
 	errs = appendErrors(errs, validateDestination(http.Mirror))
 	errs = appendErrors(errs, validateHTTPRedirect(http.Redirect))
 	errs = appendErrors(errs, validateHTTPRetry(http.Retries))
 	errs = appendErrors(errs, validateHTTPRewrite(http.Rewrite))
+	errs = appendErrors(errs, validateDestinationWeights(http.Route))
+	if http.Timeout != nil {
+		errs = appendErrors(errs, ValidateDurationGogo(http.Timeout))
+	}
+
+	return
+}
+
+func validateGatewayNames(gateways []string) (errs error) {
+	for _, gateway := range gateways {
+		if err := ValidateFQDN(gateway); err != nil {
+			errs = appendErrors(errs, err)
+		}
+	}
+	return
+}
+
+func validateDestinationWeights(weights []*networking.DestinationWeight) (errs error) {
 	var totalWeight int32
-	for _, route := range http.Route {
-		if route.Destination == nil {
+	for _, weight := range weights {
+		if weight.Destination == nil {
 			errs = multierror.Append(errs, errors.New("destination is required"))
 		}
-		errs = appendErrors(errs, validateDestination(route.Destination))
-		errs = appendErrors(errs, ValidatePercent(route.Weight))
-		totalWeight += route.Weight
+		errs = appendErrors(errs, validateDestination(weight.Destination))
+		errs = appendErrors(errs, ValidatePercent(weight.Weight))
+		totalWeight += weight.Weight
 	}
-	if len(http.Route) > 1 && totalWeight > 100 {
+	if len(weights) > 1 && totalWeight > 100 {
 		errs = appendErrors(errs, fmt.Errorf("total destination weight %v > 100", totalWeight))
 	}
-	if http.Timeout != nil {
-		errs = appendErrors(errs, ValidateDurationGogo(http.Timeout))
-	}
-
 	return
 }