From 439eccdd80f7930aa3c44a774816e8c5cb6feb0f Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 8 Jul 2022 13:21:05 -0700 Subject: [PATCH 1/4] Respect http2 protocol for upstreams of terminating gateways --- agent/proxycfg/testing_terminating_gateway.go | 119 ++++++++++++ agent/xds/clusters.go | 13 ++ agent/xds/clusters_test.go | 8 + ...teway-http2-upstream-subsets.latest.golden | 181 ++++++++++++++++++ ...ating-gateway-http2-upstream.latest.golden | 65 +++++++ 5 files changed, 386 insertions(+) create mode 100644 agent/xds/testdata/clusters/terminating-gateway-http2-upstream-subsets.latest.golden create mode 100644 agent/xds/testdata/clusters/terminating-gateway-http2-upstream.latest.golden diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go index bac411fff8..c9060df922 100644 --- a/agent/proxycfg/testing_terminating_gateway.go +++ b/agent/proxycfg/testing_terminating_gateway.go @@ -633,6 +633,125 @@ func TestConfigSnapshotTerminatingGatewaySNI(t testing.T) *ConfigSnapshot { }) } +func TestConfigSnapshotTerminatingGatewayHTTP2(t testing.T) *ConfigSnapshot { + web := structs.NewServiceName("web", nil) + + return TestConfigSnapshotTerminatingGateway(t, false, nil, []UpdateEvent{ + { + CorrelationID: gatewayServicesWatchID, + Result: &structs.IndexedGatewayServices{ + Services: []*structs.GatewayService{ + { + Service: web, + CAFile: "ca.cert.pem", + Protocol: "http2", + }, + }, + }, + }, + { + CorrelationID: serviceConfigIDPrefix + web.String(), + Result: &structs.ServiceConfigResponse{ + ProxyConfig: map[string]interface{}{"protocol": "http2"}, + }, + }, + { + CorrelationID: externalServiceIDPrefix + web.String(), + Result: &structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + ID: "external", + Node: "external", + Address: "web.external.service", + Datacenter: "dc1", + }, + Service: &structs.NodeService{ + Service: "web", + Port: 9090, + }, + }, + }, + }, + }, + }) +} + +func TestConfigSnapshotTerminatingGatewaySubsetsHTTP2(t testing.T) *ConfigSnapshot { + web := structs.NewServiceName("web", nil) + + return TestConfigSnapshotTerminatingGateway(t, false, nil, []UpdateEvent{ + { + CorrelationID: serviceResolverIDPrefix + web.String(), + Result: &structs.ConfigEntryResponse{ + Entry: &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "web", + Subsets: map[string]structs.ServiceResolverSubset{ + "v1": { + Filter: "Service.Meta.version == 1", + }, + "v2": { + Filter: "Service.Meta.version == 2", + }, + }, + }, + }, + }, + { + CorrelationID: gatewayServicesWatchID, + Result: &structs.IndexedGatewayServices{ + Services: []*structs.GatewayService{ + { + Service: web, + CAFile: "ca.cert.pem", + Protocol: "http2", + }, + }, + }, + }, + { + CorrelationID: serviceConfigIDPrefix + web.String(), + Result: &structs.ServiceConfigResponse{ + ProxyConfig: map[string]interface{}{"protocol": "http2"}, + }, + }, + { + CorrelationID: externalServiceIDPrefix + web.String(), + Result: &structs.IndexedCheckServiceNodes{ + Nodes: []structs.CheckServiceNode{ + { + Node: &structs.Node{ + ID: "external", + Node: "external", + Address: "web.external.service", + Datacenter: "dc1", + }, + Service: &structs.NodeService{ + Service: "web", + Port: 9090, + Meta: map[string]string{"version": "1"}, + }, + }, + { + Node: &structs.Node{ + ID: "external2", + Node: "external2", + Address: "web.external2.service", + Datacenter: "dc1", + }, + Service: &structs.NodeService{ + Service: "web", + Port: 9091, + Meta: map[string]string{"version": "2"}, + }, + }, + }, + }, + }, + }) +} + func TestConfigSnapshotTerminatingGatewayHostnameSubsets(t testing.T) *ConfigSnapshot { var ( api = structs.NewServiceName("api", nil) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 3f2efdef7e..75f0b2344d 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -425,6 +425,14 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( } clusters = append(clusters, cluster) + gatewaySvc, ok := cfgSnap.TerminatingGateway.GatewayServices[svc] + isHTTP2 := ok && gatewaySvc.Protocol == "http2" + if isHTTP2 { + if err := s.setHttp2ProtocolOptions(cluster); err != nil { + return nil, err + } + } + // If there is a service-resolver for this service then also setup a cluster for each subset for name, subset := range resolver.Subsets { subsetHostnameEndpoints, err := s.filterSubsetEndpoints(&subset, hostnameEndpoints) @@ -444,6 +452,11 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( if err := s.injectGatewayServiceAddons(cfgSnap, cluster, svc, loadBalancer); err != nil { return nil, err } + if isHTTP2 { + if err := s.setHttp2ProtocolOptions(cluster); err != nil { + return nil, err + } + } clusters = append(clusters, cluster) } } diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go index b3e85486b7..49d333750c 100644 --- a/agent/xds/clusters_test.go +++ b/agent/xds/clusters_test.go @@ -589,6 +589,14 @@ func TestClustersFromSnapshot(t *testing.T) { name: "terminating-gateway-sni", create: proxycfg.TestConfigSnapshotTerminatingGatewaySNI, }, + { + name: "terminating-gateway-http2-upstream", + create: proxycfg.TestConfigSnapshotTerminatingGatewayHTTP2, + }, + { + name: "terminating-gateway-http2-upstream-subsets", + create: proxycfg.TestConfigSnapshotTerminatingGatewaySubsetsHTTP2, + }, { name: "terminating-gateway-ignore-extra-resolvers", create: proxycfg.TestConfigSnapshotTerminatingGatewayIgnoreExtraResolvers, diff --git a/agent/xds/testdata/clusters/terminating-gateway-http2-upstream-subsets.latest.golden b/agent/xds/testdata/clusters/terminating-gateway-http2-upstream-subsets.latest.golden new file mode 100644 index 0000000000..953a207e68 --- /dev/null +++ b/agent/xds/testdata/clusters/terminating-gateway-http2-upstream-subsets.latest.golden @@ -0,0 +1,181 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "v1.web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "v1.web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "web.external.service", + "portValue": 9090 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "typedExtensionProtocolOptions": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicitHttpConfig": { + "http2ProtocolOptions": { + + } + } + } + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + } + } + } + } + } + }, + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "v2.web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "v2.web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "web.external2.service", + "portValue": 9091 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "typedExtensionProtocolOptions": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicitHttpConfig": { + "http2ProtocolOptions": { + + } + } + } + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + } + } + } + } + } + }, + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "web.external.service", + "portValue": 9090 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "typedExtensionProtocolOptions": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicitHttpConfig": { + "http2ProtocolOptions": { + + } + } + } + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + } + } + } + } + } + } + ], + "typeUrl": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/clusters/terminating-gateway-http2-upstream.latest.golden b/agent/xds/testdata/clusters/terminating-gateway-http2-upstream.latest.golden new file mode 100644 index 0000000000..9a8e2bdf43 --- /dev/null +++ b/agent/xds/testdata/clusters/terminating-gateway-http2-upstream.latest.golden @@ -0,0 +1,65 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "name": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "LOGICAL_DNS", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "web.external.service", + "portValue": 9090 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, + "typedExtensionProtocolOptions": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicitHttpConfig": { + "http2ProtocolOptions": { + + } + } + } + }, + "dnsRefreshRate": "10s", + "dnsLookupFamily": "V4_ONLY", + "outlierDetection": { + + }, + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext", + "commonTlsContext": { + "tlsParams": { + + }, + "validationContext": { + "trustedCa": { + "filename": "ca.cert.pem" + } + } + } + } + } + } + ], + "typeUrl": "type.googleapis.com/envoy.config.cluster.v3.Cluster", + "nonce": "00000001" +} \ No newline at end of file From 608d0fe2c14db060f1dec799fa6db9e8459d5e60 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 8 Jul 2022 15:23:00 -0700 Subject: [PATCH 2/4] Add changelog note --- .changelog/13699.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/13699.txt diff --git a/.changelog/13699.txt b/.changelog/13699.txt new file mode 100644 index 0000000000..64cde8d97c --- /dev/null +++ b/.changelog/13699.txt @@ -0,0 +1,3 @@ +```release-note:bug +xds: Fix a bug where terminating gateway upstream clusters weren't configured properly when the service protocol was `http2`. +``` From 7162e3bde24b3e911853f860ab35b3603208defe Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 12 Jul 2022 14:38:44 -0700 Subject: [PATCH 3/4] Enable http2 options for grpc protocol --- agent/xds/clusters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 75f0b2344d..44f06984de 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -426,7 +426,7 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( clusters = append(clusters, cluster) gatewaySvc, ok := cfgSnap.TerminatingGateway.GatewayServices[svc] - isHTTP2 := ok && gatewaySvc.Protocol == "http2" + isHTTP2 := ok && (gatewaySvc.Protocol == "http2" || gatewaySvc.Protocol == "grpc") if isHTTP2 { if err := s.setHttp2ProtocolOptions(cluster); err != nil { return nil, err From 7d0c692374afd7b7df8938e850ef1a63228ee555 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 12 Jul 2022 16:09:35 -0700 Subject: [PATCH 4/4] Use protocol from resolved config entry, not gateway service --- agent/proxycfg/testing_terminating_gateway.go | 10 ++++------ agent/xds/clusters.go | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/agent/proxycfg/testing_terminating_gateway.go b/agent/proxycfg/testing_terminating_gateway.go index c9060df922..00771433b5 100644 --- a/agent/proxycfg/testing_terminating_gateway.go +++ b/agent/proxycfg/testing_terminating_gateway.go @@ -642,9 +642,8 @@ func TestConfigSnapshotTerminatingGatewayHTTP2(t testing.T) *ConfigSnapshot { Result: &structs.IndexedGatewayServices{ Services: []*structs.GatewayService{ { - Service: web, - CAFile: "ca.cert.pem", - Protocol: "http2", + Service: web, + CAFile: "ca.cert.pem", }, }, }, @@ -703,9 +702,8 @@ func TestConfigSnapshotTerminatingGatewaySubsetsHTTP2(t testing.T) *ConfigSnapsh Result: &structs.IndexedGatewayServices{ Services: []*structs.GatewayService{ { - Service: web, - CAFile: "ca.cert.pem", - Protocol: "http2", + Service: web, + CAFile: "ca.cert.pem", }, }, }, diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 44f06984de..ed634ce713 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -425,8 +425,18 @@ func (s *ResourceGenerator) makeGatewayServiceClusters( } clusters = append(clusters, cluster) - gatewaySvc, ok := cfgSnap.TerminatingGateway.GatewayServices[svc] - isHTTP2 := ok && (gatewaySvc.Protocol == "http2" || gatewaySvc.Protocol == "grpc") + svcConfig, ok := cfgSnap.TerminatingGateway.ServiceConfigs[svc] + isHTTP2 := false + if ok { + upstreamCfg, err := structs.ParseUpstreamConfig(svcConfig.ProxyConfig) + if err != nil { + // Don't hard fail on a config typo, just warn. The parse func returns + // default config if there is an error so it's safe to continue. + s.Logger.Warn("failed to parse", "upstream", svc, "error", err) + } + isHTTP2 = upstreamCfg.Protocol == "http2" || upstreamCfg.Protocol == "grpc" + } + if isHTTP2 { if err := s.setHttp2ProtocolOptions(cluster); err != nil { return nil, err