Remove validation of TunnelSettings.Protocol for empty string (istio#…

…40102) Signed-off-by: Jacek Ewertowski <jewertow@redhat.com>
stevenctl · Jul 25, 2022 · 1aca7a6 · 1aca7a6
1 parent e0110ff
commit 1aca7a6
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 19 deletions.
diff --git a/pilot/pkg/networking/core/v1alpha3/networkfilter_test.go b/pilot/pkg/networking/core/v1alpha3/networkfilter_test.go
@@ -165,6 +165,15 @@ func TestBuildOutboundNetworkFiltersTunnelingConfig(t *testing.T) {
 			},
 		},
 	}
+	tunnelingEnabledWithoutProtocol := &networking.DestinationRule{
+		Host: "tunnel-proxy.com",
+		TrafficPolicy: &networking.TrafficPolicy{
+			Tunnel: &networking.TrafficPolicy_TunnelSettings{
+				TargetHost: "example.com",
+				TargetPort: 8443,
+			},
+		},
+	}
 	tunnelingEnabledForSubset := &networking.DestinationRule{
 		Host: "tunnel-proxy.com",
 		Subsets: []*networking.Subset{
@@ -236,6 +245,15 @@ func TestBuildOutboundNetworkFiltersTunnelingConfig(t *testing.T) {
 				usePost:  false,
 			},
 		},
+		{
+			name:              "tunneling_config should be applied with disabled usePost property when tunneling settings does not specify protocol",
+			routeDestinations: tunnelProxyDestination,
+			destinationRule:   tunnelingEnabledWithoutProtocol,
+			expectedTunnelingConfig: &tunnelingConfig{
+				hostname: "example.com:8443",
+				usePost:  false,
+			},
+		},
 		{
 			name:              "tunneling_config should be applied when destination rule has specified tunnel settings and the target host is an IPv4 address",
 			routeDestinations: tunnelProxyDestination,

diff --git a/pkg/config/validation/validation.go b/pkg/config/validation/validation.go
@@ -1344,10 +1344,7 @@ func validateTunnelSettings(tunnel *networking.TrafficPolicy_TunnelSettings) (er
 	if tunnel == nil {
 		return
 	}
-	if tunnel.Protocol == "" {
-		errs = appendErrors(errs, fmt.Errorf("tunnel protocol must be specified"))
-	}
-	if tunnel.Protocol != "CONNECT" && tunnel.Protocol != "POST" {
+	if tunnel.Protocol != "" && tunnel.Protocol != "CONNECT" && tunnel.Protocol != "POST" {
 		errs = appendErrors(errs, fmt.Errorf("tunnel protocol must be \"CONNECT\" or \"POST\""))
 	}
 	fqdnErr := ValidateFQDN(tunnel.TargetHost)

diff --git a/pkg/config/validation/validation_test.go b/pkg/config/validation/validation_test.go
@@ -2290,7 +2290,16 @@ func TestValidateDestinationWithInheritance(t *testing.T) {
 				},
 			},
 		}, valid: true},
-		{name: "global tunnel settings with connect protocol", in: &networking.DestinationRule{
+		{name: "global tunnel settings without protocol", in: &networking.DestinationRule{
+			Host: "tunnel-proxy.com",
+			TrafficPolicy: &networking.TrafficPolicy{
+				Tunnel: &networking.TrafficPolicy_TunnelSettings{
+					TargetHost: "example.com",
+					TargetPort: 80,
+				},
+			},
+		}, valid: true},
+		{name: "global tunnel settings with CONNECT protocol", in: &networking.DestinationRule{
 			Host: "tunnel-proxy.com",
 			TrafficPolicy: &networking.TrafficPolicy{
 				Tunnel: &networking.TrafficPolicy_TunnelSettings{
@@ -2300,7 +2309,7 @@ func TestValidateDestinationWithInheritance(t *testing.T) {
 				},
 			},
 		}, valid: true},
-		{name: "global tunnel settings with post protocol", in: &networking.DestinationRule{
+		{name: "global tunnel settings with POST protocol", in: &networking.DestinationRule{
 			Host: "tunnel-proxy.com",
 			TrafficPolicy: &networking.TrafficPolicy{
 				Tunnel: &networking.TrafficPolicy_TunnelSettings{
@@ -2310,7 +2319,21 @@ func TestValidateDestinationWithInheritance(t *testing.T) {
 				},
 			},
 		}, valid: true},
-		{name: "subset tunnel settings with connect protocol", in: &networking.DestinationRule{
+		{name: "subset tunnel settings without protocol", in: &networking.DestinationRule{
+			Host: "tunnel-proxy.com",
+			Subsets: []*networking.Subset{
+				{
+					Name: "reviews-80",
+					TrafficPolicy: &networking.TrafficPolicy{
+						Tunnel: &networking.TrafficPolicy_TunnelSettings{
+							TargetHost: "example.com",
+							TargetPort: 80,
+						},
+					},
+				},
+			},
+		}, valid: true},
+		{name: "subset tunnel settings with CONNECT protocol", in: &networking.DestinationRule{
 			Host: "tunnel-proxy.com",
 			Subsets: []*networking.Subset{
 				{
@@ -2325,7 +2348,7 @@ func TestValidateDestinationWithInheritance(t *testing.T) {
 				},
 			},
 		}, valid: true},
-		{name: "subset tunnel settings with post protocol", in: &networking.DestinationRule{
+		{name: "subset tunnel settings with POST protocol", in: &networking.DestinationRule{
 			Host: "tunnel-proxy.com",
 			Subsets: []*networking.Subset{
 				{
@@ -2449,15 +2472,6 @@ func TestValidateDestinationWithInheritance(t *testing.T) {
 				},
 			},
 		}, valid: false},
-		{name: "tunnel settings without required protocol", in: &networking.DestinationRule{
-			Host: "tunnel-proxy.com",
-			TrafficPolicy: &networking.TrafficPolicy{
-				Tunnel: &networking.TrafficPolicy_TunnelSettings{
-					TargetHost: "example.com",
-					TargetPort: 80,
-				},
-			},
-		}, valid: false},
 		{name: "tunnel settings without required target host", in: &networking.DestinationRule{
 			Host: "tunnel-proxy.com",
 			TrafficPolicy: &networking.TrafficPolicy{

diff --git a/tests/integration/pilot/testdata/tunneling/destination-rule.tmpl.yaml b/tests/integration/pilot/testdata/tunneling/destination-rule.tmpl.yaml
@@ -8,13 +8,11 @@ spec:
   - name: external-svc-tcp
     trafficPolicy:
       tunnel:
-        protocol: CONNECT
         targetHost: external.{{ .externalNamespace }}
         targetPort: {{ .externalSvcTcpPort }}
   - name: external-svc-tls
     trafficPolicy:
       tunnel:
-        protocol: CONNECT
         targetHost: external.{{ .externalNamespace }}
         targetPort: {{ .externalSvcTlsPort }}
 ---