Repository: kumahq/kuma
Pull request #1714 (2021-03-22 08:29:48)

fix(kuma-cp) pick HTTP health checker version depending on service protocol
### Summary

When configuring `HealthCheck.Http` Kuma implied it always HTTP2. That didn't work well with `httpbin.org`. 

### Full changelog

The current fix takes into account `protocol` which was set on the Service. But the fix has shortcomings. It doesn't allow creating an HTTP2 health checker for traffic that is treated as TCP or HTTP1.

Alternatively, we can introduce a new field `version` in `HealthCheck.Http` section:

```yaml
type: HealthCheck
mesh: default
name: internalhttpbin-check
sources: 
  - match: 
      kuma.io/service: '*'
destinations: 
  - match: 
      kuma.io/service: externalhttpbin
conf: 
  interval: 10s
  timeout: 2s
  unhealthyThreshold: 1
  healthyThreshold: 1
  healthyPanicThreshold: 0
  http: 
    version: v1
    path: /
```

### Issues resolved

Fix https://github.com/kumahq/kuma/issues/1700

### Documentation

- [ ] Link to the website [documentation PR](https://github.com/kumahq/kuma-website/pull/XXX)



Patch:
diff --git a/api/go.mod b/api/go.mod
index 7068f9ec4..7c3289906 100644
--- a/api/go.mod
+++ b/api/go.mod
@@ -4,10 +4,10 @@ go 1.15
 
 require (
 	github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad
-	github.com/envoyproxy/protoc-gen-validate v0.4.1
+	github.com/envoyproxy/protoc-gen-validate v0.5.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/golang/protobuf v1.4.3
-	github.com/onsi/ginkgo v1.15.1
+	github.com/onsi/ginkgo v1.15.2
 	github.com/onsi/gomega v1.11.0
 	github.com/pkg/errors v0.9.1
 	google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013
diff --git a/api/go.sum b/api/go.sum
index fe0f22a8b..a44dcf2c9 100644
--- a/api/go.sum
+++ b/api/go.sum
@@ -11,8 +11,8 @@ github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.m
 github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad h1:EmNYJhPYy0pOFjCx2PrgtaBXmee0iUX9hLlxE1xHOJE=
 github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
-github.com/envoyproxy/protoc-gen-validate v0.4.1 h1:7dLaJvASGRD7X49jSCSXXHwKPm0ZN9r9kJD+p+vS7dM=
-github.com/envoyproxy/protoc-gen-validate v0.4.1/go.mod h1:E+IEazqdaWv3FrnGtZIu3b9fPFMK8AzeTTrk9SfVwWs=
+github.com/envoyproxy/protoc-gen-validate v0.5.0 h1:+ltcA/hEtDde/2Z+9EvwamSsAzPNE6z17oSOChokJzU=
+github.com/envoyproxy/protoc-gen-validate v0.5.0/go.mod h1:xL5IroIBOR+aTp0IZk48epGwBV3+LcuaosPL0pr0hE0=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
@@ -47,8 +47,8 @@ github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
 github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
-github.com/onsi/ginkgo v1.15.1 h1:DsXNrKujDlkMS9Rsxmd+Fg7S6Kc5lhE+qX8tY6laOxc=
-github.com/onsi/ginkgo v1.15.1/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
+github.com/onsi/ginkgo v1.15.2 h1:l77YT15o814C2qVL47NOyjV/6RbaP7kKdrvZnxQ3Org=
+github.com/onsi/ginkgo v1.15.2/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
 github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo=
 github.com/onsi/gomega v1.11.0 h1:+CqWgvj0OZycCaqclBD1pxKHAU+tOkHmQIWvDHq2aug=
diff --git a/go.mod b/go.mod
index f5c043198..428ff5ab5 100644
--- a/go.mod
+++ b/go.mod
@@ -11,7 +11,7 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/emicklei/go-restful v2.15.0+incompatible
 	github.com/envoyproxy/go-control-plane v0.9.9-0.20210107210257-6e771204fae6
-	github.com/envoyproxy/protoc-gen-validate v0.4.1
+	github.com/envoyproxy/protoc-gen-validate v0.5.0
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-errors/errors v1.0.2-0.20180813162953-d98b870cc4e0
 	github.com/go-kit/kit v0.10.0
@@ -29,7 +29,7 @@ require (
 	github.com/kumahq/kuma/pkg/transparentproxy/istio v0.0.0-00010101000000-000000000000
 	github.com/lib/pq v1.9.0
 	github.com/miekg/dns v1.1.40
-	github.com/onsi/ginkgo v1.15.1
+	github.com/onsi/ginkgo v1.15.2
 	github.com/onsi/gomega v1.11.0
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/pkg/errors v0.9.1
diff --git a/go.sum b/go.sum
index 67f5e3cf1..98ba85ead 100644
--- a/go.sum
+++ b/go.sum
@@ -351,8 +351,9 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.m
 github.com/envoyproxy/go-control-plane v0.9.9-0.20210107210257-6e771204fae6 h1:DZNZDoFHT7Q2zxdwbHkt2HFexENxiXTNd44M9pertiw=
 github.com/envoyproxy/go-control-plane v0.9.9-0.20210107210257-6e771204fae6/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
-github.com/envoyproxy/protoc-gen-validate v0.4.1 h1:7dLaJvASGRD7X49jSCSXXHwKPm0ZN9r9kJD+p+vS7dM=
 github.com/envoyproxy/protoc-gen-validate v0.4.1/go.mod h1:E+IEazqdaWv3FrnGtZIu3b9fPFMK8AzeTTrk9SfVwWs=
+github.com/envoyproxy/protoc-gen-validate v0.5.0 h1:+ltcA/hEtDde/2Z+9EvwamSsAzPNE6z17oSOChokJzU=
+github.com/envoyproxy/protoc-gen-validate v0.5.0/go.mod h1:xL5IroIBOR+aTp0IZk48epGwBV3+LcuaosPL0pr0hE0=
 github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5/go.mod h1:a2zkGnVExMxdzMo3M0Hi/3sEU+cWnZpSni0O6/Yb/P0=
 github.com/evanphx/json-patch v0.0.0-20200808040245-162e5629780b/go.mod h1:NAJj0yf/KaRKURN6nyi7A9IZydMivZEm9oQLWNjfKDc=
 github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
@@ -890,8 +891,9 @@ github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+
 github.com/onsi/ginkgo v1.12.0/go.mod h1:oUhWkIvk5aDxtKvDDuw8gItl8pKl42LzjC9KZE0HfGg=
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
 github.com/onsi/ginkgo v1.13.0/go.mod h1:+REjRxOmWfHCjfv9TTWB1jD1Frx4XydAD3zm1lskyM0=
-github.com/onsi/ginkgo v1.15.1 h1:DsXNrKujDlkMS9Rsxmd+Fg7S6Kc5lhE+qX8tY6laOxc=
 github.com/onsi/ginkgo v1.15.1/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
+github.com/onsi/ginkgo v1.15.2 h1:l77YT15o814C2qVL47NOyjV/6RbaP7kKdrvZnxQ3Org=
+github.com/onsi/ginkgo v1.15.2/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
 github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.3.0/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
diff --git a/pkg/plugins/resources/k8s/native/go.mod b/pkg/plugins/resources/k8s/native/go.mod
index d65019ed8..373c7cc5b 100644
--- a/pkg/plugins/resources/k8s/native/go.mod
+++ b/pkg/plugins/resources/k8s/native/go.mod
@@ -6,7 +6,7 @@ require (
 	github.com/go-logr/logr v0.1.0
 	github.com/golang/protobuf v1.4.3
 	github.com/kumahq/kuma/api v0.0.0-00010101000000-000000000000
-	github.com/onsi/ginkgo v1.15.1
+	github.com/onsi/ginkgo v1.15.2
 	github.com/onsi/gomega v1.11.0
 	github.com/pkg/errors v0.9.1
 	golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb
diff --git a/pkg/plugins/resources/k8s/native/go.sum b/pkg/plugins/resources/k8s/native/go.sum
index 93d08feba..f21952608 100644
--- a/pkg/plugins/resources/k8s/native/go.sum
+++ b/pkg/plugins/resources/k8s/native/go.sum
@@ -246,8 +246,9 @@ github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
-github.com/onsi/ginkgo v1.15.1 h1:DsXNrKujDlkMS9Rsxmd+Fg7S6Kc5lhE+qX8tY6laOxc=
 github.com/onsi/ginkgo v1.15.1/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
+github.com/onsi/ginkgo v1.15.2 h1:l77YT15o814C2qVL47NOyjV/6RbaP7kKdrvZnxQ3Org=
+github.com/onsi/ginkgo v1.15.2/go.mod h1:Dd6YFfwBW84ETqqtL0CPyPXillHgY6XhQH3uuCCTr/o=
 github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
diff --git a/pkg/xds/envoy/clusters/configurers.go b/pkg/xds/envoy/clusters/configurers.go
index 42d1516a8..07f47447f 100644
--- a/pkg/xds/envoy/clusters/configurers.go
+++ b/pkg/xds/envoy/clusters/configurers.go
@@ -95,13 +95,15 @@ func EdsCluster(name string) ClusterBuilderOpt {
 	})
 }
 
-func HealthCheck(healthCheck *mesh_core.HealthCheckResource) ClusterBuilderOpt {
+func HealthCheck(protocol mesh_core.Protocol, healthCheck *mesh_core.HealthCheckResource) ClusterBuilderOpt {
 	return ClusterBuilderOptFunc(func(config *ClusterBuilderConfig) {
 		config.AddV2(&v2.HealthCheckConfigurer{
 			HealthCheck: healthCheck,
+			Protocol:    protocol,
 		})
 		config.AddV3(&v3.HealthCheckConfigurer{
 			HealthCheck: healthCheck,
+			Protocol:    protocol,
 		})
 	})
 }
diff --git a/pkg/xds/envoy/clusters/v2/health_check_configurer.go b/pkg/xds/envoy/clusters/v2/health_check_configurer.go
index d87714f8e..8414b8874 100644
--- a/pkg/xds/envoy/clusters/v2/health_check_configurer.go
+++ b/pkg/xds/envoy/clusters/v2/health_check_configurer.go
@@ -17,6 +17,7 @@ import (
 
 type HealthCheckConfigurer struct {
 	HealthCheck *mesh_core.HealthCheckResource
+	Protocol    mesh_core.Protocol
 }
 
 var _ ClusterConfigurer = &HealthCheckConfigurer{}
@@ -75,6 +76,7 @@ func tcpHealthCheck(
 }
 
 func httpHealthCheck(
+	protocol mesh_core.Protocol,
 	httpConf *mesh_proto.HealthCheck_Conf_Http,
 ) *envoy_core.HealthCheck_HttpHealthCheck_ {
 	var expectedStatuses []*envoy_type.Int64Range
@@ -85,11 +87,16 @@ func httpHealthCheck(
 		)
 	}
 
+	codecClientType := envoy_type.CodecClientType_HTTP1
+	if protocol == mesh_core.ProtocolHTTP2 {
+		codecClientType = envoy_type.CodecClientType_HTTP2
+	}
+
 	httpHealthCheck := envoy_core.HealthCheck_HttpHealthCheck{
 		Path:                httpConf.Path,
 		RequestHeadersToAdd: mapHttpHeaders(httpConf.RequestHeadersToAdd),
 		ExpectedStatuses:    expectedStatuses,
-		CodecClientType:     envoy_type.CodecClientType_HTTP2,
+		CodecClientType:     codecClientType,
 	}
 
 	return &envoy_core.HealthCheck_HttpHealthCheck_{
@@ -159,7 +166,7 @@ func (e *HealthCheckConfigurer) Configure(cluster *envoy_api.Cluster) error {
 	}
 
 	if http := activeChecks.GetHttp(); http != nil {
-		healthCheck.HealthChecker = httpHealthCheck(http)
+		healthCheck.HealthChecker = httpHealthCheck(e.Protocol, http)
 	}
 
 	cluster.HealthChecks = append(cluster.HealthChecks, &healthCheck)
diff --git a/pkg/xds/envoy/clusters/v3/health_check_configurer.go b/pkg/xds/envoy/clusters/v3/health_check_configurer.go
index 8c0e05c23..b14824bff 100644
--- a/pkg/xds/envoy/clusters/v3/health_check_configurer.go
+++ b/pkg/xds/envoy/clusters/v3/health_check_configurer.go
@@ -17,6 +17,7 @@ import (
 
 type HealthCheckConfigurer struct {
 	HealthCheck *mesh_core.HealthCheckResource
+	Protocol    mesh_core.Protocol
 }
 
 var _ ClusterConfigurer = &HealthCheckConfigurer{}
@@ -75,6 +76,7 @@ func tcpHealthCheck(
 }
 
 func httpHealthCheck(
+	protocol mesh_core.Protocol,
 	httpConf *mesh_proto.HealthCheck_Conf_Http,
 ) *envoy_core.HealthCheck_HttpHealthCheck_ {
 	var expectedStatuses []*envoy_type.Int64Range
@@ -85,11 +87,16 @@ func httpHealthCheck(
 		)
 	}
 
+	codecClientType := envoy_type.CodecClientType_HTTP1
+	if protocol == mesh_core.ProtocolHTTP2 {
+		codecClientType = envoy_type.CodecClientType_HTTP2
+	}
+
 	httpHealthCheck := envoy_core.HealthCheck_HttpHealthCheck{
 		Path:                httpConf.Path,
 		RequestHeadersToAdd: mapHttpHeaders(httpConf.RequestHeadersToAdd),
 		ExpectedStatuses:    expectedStatuses,
-		CodecClientType:     envoy_type.CodecClientType_HTTP2,
+		CodecClientType:     codecClientType,
 	}
 
 	return &envoy_core.HealthCheck_HttpHealthCheck_{
@@ -161,7 +168,7 @@ func (e *HealthCheckConfigurer) Configure(cluster *envoy_cluster.Cluster) error
 	}
 
 	if http := activeChecks.GetHttp(); http != nil {
-		healthCheck.HealthChecker = httpHealthCheck(http)
+		healthCheck.HealthChecker = httpHealthCheck(e.Protocol, http)
 	}
 
 	cluster.HealthChecks = append(cluster.HealthChecks, &healthCheck)
diff --git a/pkg/xds/generator/outbound_proxy_generator.go b/pkg/xds/generator/outbound_proxy_generator.go
index c025678c0..fee0d58b7 100644
--- a/pkg/xds/generator/outbound_proxy_generator.go
+++ b/pkg/xds/generator/outbound_proxy_generator.go
@@ -171,7 +171,7 @@ func (o OutboundProxyGenerator) generateCDS(ctx xds_context.Context, proxy *mode
 			Configure(envoy_clusters.Timeout(protocol, clusters.Get(clusterName).Timeout())).
 			Configure(envoy_clusters.LbSubset(o.lbSubsets(tags))).
 			Configure(envoy_clusters.OutlierDetection(circuitBreaker)).
-			Configure(envoy_clusters.HealthCheck(healthCheck))
+			Configure(envoy_clusters.HealthCheck(protocol, healthCheck))
 
 		if clusters.Get(clusterName).HasExternalService() {
 			edsClusterBuilder.


Test patch:
diff --git a/pkg/xds/envoy/clusters/v2/health_check_configurer_test.go b/pkg/xds/envoy/clusters/v2/health_check_configurer_test.go
index 955a70cc9..4dc2258f0 100644
--- a/pkg/xds/envoy/clusters/v2/health_check_configurer_test.go
+++ b/pkg/xds/envoy/clusters/v2/health_check_configurer_test.go
@@ -31,7 +31,7 @@ var _ = Describe("HealthCheckConfigurer", func() {
 			// when
 			cluster, err := clusters.NewClusterBuilder(envoy.APIV2).
 				Configure(clusters.EdsCluster(given.clusterName)).
-				Configure(clusters.HealthCheck(given.healthCheck)).
+				Configure(clusters.HealthCheck(mesh_core.ProtocolHTTP, given.healthCheck)).
 				Configure(clusters.Timeout(mesh_core.ProtocolTCP, &mesh_proto.Timeout_Conf{ConnectTimeout: durationpb.New(5 * time.Second)})).
 				Build()
 
@@ -220,7 +220,6 @@ var _ = Describe("HealthCheckConfigurer", func() {
             - healthyThreshold: 2
               interval: 5s
               httpHealthCheck:
-                codecClientType: HTTP2
                 expectedStatuses:
                 - end: "201"
                   start: "200"
diff --git a/pkg/xds/envoy/clusters/v3/health_check_configurer_test.go b/pkg/xds/envoy/clusters/v3/health_check_configurer_test.go
index e364d1195..7c49045f6 100644
--- a/pkg/xds/envoy/clusters/v3/health_check_configurer_test.go
+++ b/pkg/xds/envoy/clusters/v3/health_check_configurer_test.go
@@ -31,7 +31,7 @@ var _ = Describe("HealthCheckConfigurer", func() {
 			// when
 			cluster, err := clusters.NewClusterBuilder(envoy.APIV3).
 				Configure(clusters.EdsCluster(given.clusterName)).
-				Configure(clusters.HealthCheck(given.healthCheck)).
+				Configure(clusters.HealthCheck(mesh_core.ProtocolHTTP, given.healthCheck)).
 				Configure(clusters.Timeout(mesh_core.ProtocolTCP, &mesh_proto.Timeout_Conf{ConnectTimeout: durationpb.New(5 * time.Second)})).
 				Build()
 
@@ -225,7 +225,6 @@ var _ = Describe("HealthCheckConfigurer", func() {
             - healthyThreshold: 2
               interval: 5s
               httpHealthCheck:
-                codecClientType: HTTP2
                 expectedStatuses:
                 - end: "201"
                   start: "200"