diff --git a/.changelog/13127.txt b/.changelog/13127.txt new file mode 100644 index 000000000..144cd2de5 --- /dev/null +++ b/.changelog/13127.txt @@ -0,0 +1,3 @@ +```release-note:bug +fix a bug that caused an error when creating `grpc` or `http2` ingress gateway listeners with multiple services +``` diff --git a/agent/proxycfg/testing_ingress_gateway.go b/agent/proxycfg/testing_ingress_gateway.go index 3512ec0f9..6846bb8a3 100644 --- a/agent/proxycfg/testing_ingress_gateway.go +++ b/agent/proxycfg/testing_ingress_gateway.go @@ -800,6 +800,109 @@ func TestConfigSnapshotIngress_HTTPMultipleServices(t testing.T) *ConfigSnapshot }) } +func TestConfigSnapshotIngress_GRPCMultipleServices(t testing.T) *ConfigSnapshot { + // We do not add baz/qux here so that we test the chain.IsDefault() case + entries := []structs.ConfigEntry{ + &structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + Config: map[string]interface{}{ + "protocol": "http", + }, + }, + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "foo", + ConnectTimeout: 22 * time.Second, + }, + &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "bar", + ConnectTimeout: 22 * time.Second, + }, + } + + var ( + foo = structs.NewServiceName("foo", nil) + fooUID = NewUpstreamIDFromServiceName(foo) + fooChain = discoverychain.TestCompileConfigEntries(t, "foo", "default", "default", "dc1", connect.TestClusterID+".consul", nil, entries...) + + bar = structs.NewServiceName("bar", nil) + barUID = NewUpstreamIDFromServiceName(bar) + barChain = discoverychain.TestCompileConfigEntries(t, "bar", "default", "default", "dc1", connect.TestClusterID+".consul", nil, entries...) + ) + + require.False(t, fooChain.Default) + require.False(t, barChain.Default) + + return TestConfigSnapshotIngressGateway(t, false, "http", "default", nil, func(entry *structs.IngressGatewayConfigEntry) { + entry.Listeners = []structs.IngressListener{ + { + Port: 8080, + Protocol: "grpc", + Services: []structs.IngressService{ + { + Name: "foo", + Hosts: []string{ + "test1.example.com", + "test2.example.com", + "test2.example.com:8080", + }, + }, + {Name: "bar"}, + }, + }, + } + }, []UpdateEvent{ + { + CorrelationID: gatewayServicesWatchID, + Result: &structs.IndexedGatewayServices{ + Services: []*structs.GatewayService{ + { + Service: foo, + Port: 8080, + Protocol: "grpc", + Hosts: []string{ + "test1.example.com", + "test2.example.com", + "test2.example.com:8080", + }, + }, + { + Service: bar, + Port: 8080, + Protocol: "grpc", + }, + }, + }, + }, + { + CorrelationID: "discovery-chain:" + fooUID.String(), + Result: &structs.DiscoveryChainResponse{ + Chain: fooChain, + }, + }, + { + CorrelationID: "upstream-target:" + fooChain.ID() + ":" + fooUID.String(), + Result: &structs.IndexedCheckServiceNodes{ + Nodes: TestUpstreamNodes(t, "foo"), + }, + }, + { + CorrelationID: "discovery-chain:" + barUID.String(), + Result: &structs.DiscoveryChainResponse{ + Chain: barChain, + }, + }, + { + CorrelationID: "upstream-target:" + barChain.ID() + ":" + barUID.String(), + Result: &structs.IndexedCheckServiceNodes{ + Nodes: TestUpstreamNodes(t, "bar"), + }, + }, + }) +} + func TestConfigSnapshotIngress_MultipleListenersDuplicateService(t testing.T) *ConfigSnapshot { var ( foo = structs.NewServiceName("foo", nil) diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go index 4aadd8ef9..0f8917098 100644 --- a/agent/structs/config_entry_gateways.go +++ b/agent/structs/config_entry_gateways.go @@ -275,8 +275,8 @@ func (e *IngressGatewayConfigEntry) Validate() error { } // Validate that http features aren't being used with tcp or another non-supported protocol. - if listener.Protocol != "http" && len(listener.Services) > 1 { - return fmt.Errorf("Multiple services per listener are only supported for protocol = 'http' (listener on port %d)", + if !IsProtocolHTTPLike(listener.Protocol) && len(listener.Services) > 1 { + return fmt.Errorf("Multiple services per listener are only supported for L7 protocols, 'http', 'grpc' and 'http2' (listener on port %d)", listener.Port) } diff --git a/agent/structs/config_entry_gateways_test.go b/agent/structs/config_entry_gateways_test.go index 3afc7cb03..c494979c2 100644 --- a/agent/structs/config_entry_gateways_test.go +++ b/agent/structs/config_entry_gateways_test.go @@ -135,6 +135,26 @@ func TestIngressGatewayConfigEntry(t *testing.T) { validateErr: "Wildcard service name is only valid for protocol", }, "http features: multiple services": { + entry: &IngressGatewayConfigEntry{ + Kind: "ingress-gateway", + Name: "ingress-web", + Listeners: []IngressListener{ + { + Port: 1111, + Protocol: "http", + Services: []IngressService{ + { + Name: "db1", + }, + { + Name: "db2", + }, + }, + }, + }, + }, + }, + "http features: multiple services on tcp listener": { entry: &IngressGatewayConfigEntry{ Kind: "ingress-gateway", Name: "ingress-web", @@ -153,7 +173,7 @@ func TestIngressGatewayConfigEntry(t *testing.T) { }, }, }, - validateErr: "Multiple services per listener are only supported for protocol", + validateErr: "Multiple services per listener are only supported for L7", }, // ========================== "tcp listener requires a defined service": { diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index 6f556e3f1..8fb1cbddc 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -680,6 +680,10 @@ func TestListenersFromSnapshot(t *testing.T) { name: "ingress-http-multiple-services", create: proxycfg.TestConfigSnapshotIngress_HTTPMultipleServices, }, + { + name: "ingress-grpc-multiple-services", + create: proxycfg.TestConfigSnapshotIngress_GRPCMultipleServices, + }, { name: "terminating-gateway-no-api-cert", create: func(t testinf.T) *proxycfg.ConfigSnapshot { diff --git a/agent/xds/routes_test.go b/agent/xds/routes_test.go index 501d05cb2..80a75ef1b 100644 --- a/agent/xds/routes_test.go +++ b/agent/xds/routes_test.go @@ -142,6 +142,10 @@ func TestRoutesFromSnapshot(t *testing.T) { name: "ingress-http-multiple-services", create: proxycfg.TestConfigSnapshotIngress_HTTPMultipleServices, }, + { + name: "ingress-grpc-multiple-services", + create: proxycfg.TestConfigSnapshotIngress_GRPCMultipleServices, + }, { name: "ingress-with-chain-and-router-header-manip", create: func(t testinf.T) *proxycfg.ConfigSnapshot { diff --git a/agent/xds/testdata/listeners/ingress-grpc-multiple-services.latest.golden b/agent/xds/testdata/listeners/ingress-grpc-multiple-services.latest.golden new file mode 100644 index 000000000..6d1f1790c --- /dev/null +++ b/agent/xds/testdata/listeners/ingress-grpc-multiple-services.latest.golden @@ -0,0 +1,65 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.listener.v3.Listener", + "name": "grpc:1.2.3.4:8080", + "address": { + "socketAddress": { + "address": "1.2.3.4", + "portValue": 8080 + } + }, + "filterChains": [ + { + "filters": [ + { + "name": "envoy.filters.network.http_connection_manager", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager", + "statPrefix": "ingress_upstream_8080", + "rds": { + "configSource": { + "ads": { + + }, + "resourceApiVersion": "V3" + }, + "routeConfigName": "8080" + }, + "httpFilters": [ + { + "name": "envoy.filters.http.grpc_stats", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.filters.http.grpc_stats.v3.FilterConfig", + "statsForAllMethods": true + } + }, + { + "name": "envoy.filters.http.grpc_http1_bridge", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.filters.http.grpc_http1_bridge.v3.Config" + } + }, + { + "name": "envoy.filters.http.router", + "typedConfig": { + "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router" + } + } + ], + "tracing": { + "randomSampling": {} + }, + "http2ProtocolOptions": {} + } + } + ] + } + ], + "trafficDirection": "OUTBOUND" + } + ], + "typeUrl": "type.googleapis.com/envoy.config.listener.v3.Listener", + "nonce": "00000001" +} \ No newline at end of file diff --git a/agent/xds/testdata/routes/ingress-grpc-multiple-services.latest.golden b/agent/xds/testdata/routes/ingress-grpc-multiple-services.latest.golden new file mode 100644 index 000000000..2822f903d --- /dev/null +++ b/agent/xds/testdata/routes/ingress-grpc-multiple-services.latest.golden @@ -0,0 +1,50 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "name": "8080", + "virtualHosts": [ + { + "name": "foo", + "domains": [ + "test1.example.com", + "test2.example.com", + "test2.example.com:8080", + "test1.example.com:8080" + ], + "routes": [ + { + "match": { + "prefix": "/" + }, + "route": { + "cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + }, + { + "name": "bar", + "domains": [ + "bar.ingress.*", + "bar.ingress.*:8080" + ], + "routes": [ + { + "match": { + "prefix": "/" + }, + "route": { + "cluster": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + } + ], + "validateClusters": true + } + ], + "typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration", + "nonce": "00000001" +} \ No newline at end of file