From 42224fe45c70f9f2d540ba965f67f777eed261a0 Mon Sep 17 00:00:00 2001
From: Andy Lindeman <andy@lindeman.io>
Date: Thu, 2 Apr 2020 03:35:04 -0400
Subject: [PATCH] proxycfg: support path exposed with non-HTTP2 protocol
 (#7510)

If a proxied service is a gRPC or HTTP2 service, but a path is exposed
using the HTTP1 or TCP protocol, Envoy should not be configured with
`http2ProtocolOptions` for the cluster backing the path.

A situation where this comes up is a gRPC service whose healthcheck or
metrics route (e.g. for Prometheus) is an HTTP1 service running on
a different port. Previously, if these were exposed either using
`Expose: { Checks: true }` or `Expose: { Paths: ... }`, Envoy would
still be configured to communicate with the path over HTTP2, which would
not work properly.
---
 agent/proxycfg/testing.go                     | 30 ++++++++++
 agent/xds/clusters.go                         |  6 +-
 agent/xds/clusters_test.go                    |  4 ++
 ...expose-paths-grpc-new-cluster-http1.golden | 60 +++++++++++++++++++
 4 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 agent/xds/testdata/clusters/expose-paths-grpc-new-cluster-http1.golden

diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go
index e8ec4d0e6..8f5bb529e 100644
--- a/agent/proxycfg/testing.go
+++ b/agent/proxycfg/testing.go
@@ -1067,6 +1067,36 @@ func TestConfigSnapshotExposeConfig(t testing.T) *ConfigSnapshot {
 	}
 }
 
+func TestConfigSnapshotGRPCExposeHTTP1(t testing.T) *ConfigSnapshot {
+	return &ConfigSnapshot{
+		Kind:    structs.ServiceKindConnectProxy,
+		Service: "grpc-proxy",
+		ProxyID: structs.NewServiceID("grpc-proxy", nil),
+		Address: "1.2.3.4",
+		Port:    8080,
+		Proxy: structs.ConnectProxyConfig{
+			DestinationServiceName: "grpc",
+			DestinationServiceID:   "grpc",
+			LocalServicePort:       8080,
+			Config: map[string]interface{}{
+				"protocol": "grpc",
+			},
+			Expose: structs.ExposeConfig{
+				Checks: false,
+				Paths: []structs.ExposePath{
+					{
+						LocalPathPort: 8090,
+						Path:          "/healthz",
+						ListenerPort:  21500,
+						Protocol:      "http",
+					},
+				},
+			},
+		},
+		Datacenter: "dc1",
+	}
+}
+
 // ControllableCacheType is a cache.Type that simulates a typical blocking RPC
 // but lets us control the responses and when they are delivered easily.
 type ControllableCacheType struct {
diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go
index 3ff767e46..c51f7668b 100644
--- a/agent/xds/clusters.go
+++ b/agent/xds/clusters.go
@@ -229,7 +229,11 @@ func (s *Server) makeAppCluster(cfgSnap *proxycfg.ConfigSnapshot, name, pathProt
 			},
 		},
 	}
-	if cfg.Protocol == "http2" || cfg.Protocol == "grpc" || pathProtocol == "http2" {
+	protocol := pathProtocol
+	if protocol == "" {
+		protocol = cfg.Protocol
+	}
+	if protocol == "http2" || protocol == "grpc" {
 		c.Http2ProtocolOptions = &envoycore.Http2ProtocolOptions{}
 	}
 
diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go
index 3f586c1b6..431aff587 100644
--- a/agent/xds/clusters_test.go
+++ b/agent/xds/clusters_test.go
@@ -245,6 +245,10 @@ func TestClustersFromSnapshot(t *testing.T) {
 				}
 			},
 		},
+		{
+			name:   "expose-paths-grpc-new-cluster-http1",
+			create: proxycfg.TestConfigSnapshotGRPCExposeHTTP1,
+		},
 		{
 			name:   "mesh-gateway",
 			create: proxycfg.TestConfigSnapshotMeshGateway,
diff --git a/agent/xds/testdata/clusters/expose-paths-grpc-new-cluster-http1.golden b/agent/xds/testdata/clusters/expose-paths-grpc-new-cluster-http1.golden
new file mode 100644
index 000000000..5b3fc08e4
--- /dev/null
+++ b/agent/xds/testdata/clusters/expose-paths-grpc-new-cluster-http1.golden
@@ -0,0 +1,60 @@
+{
+  "versionInfo": "00000001",
+  "resources": [
+    {
+      "@type": "type.googleapis.com/envoy.api.v2.Cluster",
+      "name": "exposed_cluster_8090",
+      "type": "STATIC",
+      "connectTimeout": "5s",
+      "loadAssignment": {
+        "clusterName": "exposed_cluster_8090",
+        "endpoints": [
+          {
+            "lbEndpoints": [
+              {
+                "endpoint": {
+                  "address": {
+                    "socketAddress": {
+                      "address": "127.0.0.1",
+                      "portValue": 8090
+                    }
+                  }
+                }
+              }
+            ]
+          }
+        ]
+      }
+    },
+    {
+      "@type": "type.googleapis.com/envoy.api.v2.Cluster",
+      "name": "local_app",
+      "type": "STATIC",
+      "connectTimeout": "5s",
+      "loadAssignment": {
+        "clusterName": "local_app",
+        "endpoints": [
+          {
+            "lbEndpoints": [
+              {
+                "endpoint": {
+                  "address": {
+                    "socketAddress": {
+                      "address": "127.0.0.1",
+                      "portValue": 8080
+                    }
+                  }
+                }
+              }
+            ]
+          }
+        ]
+      },
+      "http2ProtocolOptions": {
+
+      }
+    }
+  ],
+  "typeUrl": "type.googleapis.com/envoy.api.v2.Cluster",
+  "nonce": "00000001"
+}
\ No newline at end of file