From 3cb70566a907423795b8e218111f2efc8acb52c2 Mon Sep 17 00:00:00 2001 From: Andrew Stucki Date: Fri, 9 Jun 2023 08:22:32 -0400 Subject: [PATCH] [API Gateway] Fix rate limiting for API gateways (#17631) * [API Gateway] Fix rate limiting for API gateways * Add changelog * Fix failing unit tests * Fix operator usage tests for api package --- .changelog/17631.txt | 3 + agent/consul/state/state_store_test.go | 19 +++++- agent/consul/state/usage.go | 1 + agent/consul/state/usage_test.go | 15 +++- .../usagemetrics/usagemetrics_oss_test.go | 68 ++++++++++++++++--- agent/operator_endpoint_oss_test.go | 1 + agent/structs/testing_catalog.go | 8 +++ api/operator_usage_test.go | 1 + 8 files changed, 104 insertions(+), 12 deletions(-) create mode 100644 .changelog/17631.txt diff --git a/.changelog/17631.txt b/.changelog/17631.txt new file mode 100644 index 000000000000..b24b7461ec6e --- /dev/null +++ b/.changelog/17631.txt @@ -0,0 +1,3 @@ +```release-note:bug +gateways: Fixed a bug where API gateways were not being taken into account in determining xDS rate limits. +``` diff --git a/agent/consul/state/state_store_test.go b/agent/consul/state/state_store_test.go index dfe8c988b329..fef750253272 100644 --- a/agent/consul/state/state_store_test.go +++ b/agent/consul/state/state_store_test.go @@ -203,11 +203,27 @@ func testRegisterConnectService(t *testing.T, s *Store, idx uint64, nodeID, serv }) } +func testRegisterAPIService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { + testRegisterGatewayService(t, s, structs.ServiceKindAPIGateway, idx, nodeID, serviceID) +} + +func testRegisterTerminatingService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { + testRegisterGatewayService(t, s, structs.ServiceKindTerminatingGateway, idx, nodeID, serviceID) +} + func testRegisterIngressService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { + testRegisterGatewayService(t, s, structs.ServiceKindIngressGateway, idx, nodeID, serviceID) +} + +func testRegisterMeshService(t *testing.T, s *Store, idx uint64, nodeID, serviceID string) { + testRegisterGatewayService(t, s, structs.ServiceKindMeshGateway, idx, nodeID, serviceID) +} + +func testRegisterGatewayService(t *testing.T, s *Store, kind structs.ServiceKind, idx uint64, nodeID, serviceID string) { svc := &structs.NodeService{ ID: serviceID, Service: serviceID, - Kind: structs.ServiceKindIngressGateway, + Kind: kind, Address: "1.1.1.1", Port: 1111, } @@ -227,6 +243,7 @@ func testRegisterIngressService(t *testing.T, s *Store, idx uint64, nodeID, serv t.Fatalf("bad service: %#v", result) } } + func testRegisterCheck(t *testing.T, s *Store, idx uint64, nodeID string, serviceID string, checkID types.CheckID, state string) { testRegisterCheckWithPartition(t, s, idx, diff --git a/agent/consul/state/usage.go b/agent/consul/state/usage.go index 8e455241c299..b3794105508a 100644 --- a/agent/consul/state/usage.go +++ b/agent/consul/state/usage.go @@ -25,6 +25,7 @@ var allConnectKind = []string{ string(structs.ServiceKindIngressGateway), string(structs.ServiceKindMeshGateway), string(structs.ServiceKindTerminatingGateway), + string(structs.ServiceKindAPIGateway), connectNativeInstancesTable, } diff --git a/agent/consul/state/usage_test.go b/agent/consul/state/usage_test.go index 6ccaddb9f7e8..68844ebc1140 100644 --- a/agent/consul/state/usage_test.go +++ b/agent/consul/state/usage_test.go @@ -179,16 +179,25 @@ func TestStateStore_Usage_ServiceUsage(t *testing.T) { testRegisterConnectNativeService(t, s, 13, "node1", "service-native") testRegisterConnectNativeService(t, s, 14, "node2", "service-native") testRegisterConnectNativeService(t, s, 15, "node2", "service-native-1") + testRegisterIngressService(t, s, 16, "node1", "ingress") + testRegisterMeshService(t, s, 17, "node1", "mesh") + testRegisterTerminatingService(t, s, 18, "node1", "terminating") + testRegisterAPIService(t, s, 19, "node1", "api") + testRegisterAPIService(t, s, 20, "node2", "api") ws := memdb.NewWatchSet() idx, usage, err := s.ServiceUsage(ws) require.NoError(t, err) - require.Equal(t, idx, uint64(15)) - require.Equal(t, 5, usage.Services) - require.Equal(t, 8, usage.ServiceInstances) + require.Equal(t, idx, uint64(20)) + require.Equal(t, 9, usage.Services) + require.Equal(t, 13, usage.ServiceInstances) require.Equal(t, 2, usage.ConnectServiceInstances[string(structs.ServiceKindConnectProxy)]) require.Equal(t, 3, usage.ConnectServiceInstances[connectNativeInstancesTable]) require.Equal(t, 6, usage.BillableServiceInstances) + require.Equal(t, 2, usage.ConnectServiceInstances[string(structs.ServiceKindAPIGateway)]) + require.Equal(t, 1, usage.ConnectServiceInstances[string(structs.ServiceKindIngressGateway)]) + require.Equal(t, 1, usage.ConnectServiceInstances[string(structs.ServiceKindTerminatingGateway)]) + require.Equal(t, 1, usage.ConnectServiceInstances[string(structs.ServiceKindMeshGateway)]) testRegisterSidecarProxy(t, s, 16, "node2", "service2") diff --git a/agent/consul/usagemetrics/usagemetrics_oss_test.go b/agent/consul/usagemetrics/usagemetrics_oss_test.go index 1781dca4c50e..8a4b511577c8 100644 --- a/agent/consul/usagemetrics/usagemetrics_oss_test.go +++ b/agent/consul/usagemetrics/usagemetrics_oss_test.go @@ -149,6 +149,22 @@ var baseCases = map[string]testCase{ {Name: "kind", Value: "ingress-gateway"}, }, }, + "consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=api-gateway": { // Legacy + Name: "consul.usage.test.consul.state.connect_instances", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + }, + "consul.usage.test.state.connect_instances;datacenter=dc1;kind=api-gateway": { + Name: "consul.usage.test.state.connect_instances", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + }, "consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=mesh-gateway": { // Legacy Name: "consul.usage.test.consul.state.connect_instances", Value: 0, @@ -624,6 +640,22 @@ var baseCases = map[string]testCase{ {Name: "kind", Value: "ingress-gateway"}, }, }, + "consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=api-gateway": { // Legacy + Name: "consul.usage.test.consul.state.connect_instances", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + }, + "consul.usage.test.state.connect_instances;datacenter=dc1;kind=api-gateway": { + Name: "consul.usage.test.state.connect_instances", + Value: 0, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + }, "consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=mesh-gateway": { // Legacy Name: "consul.usage.test.consul.state.connect_instances", Value: 0, @@ -1127,6 +1159,9 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { require.NoError(t, s.EnsureNode(3, &structs.Node{Node: "baz", Address: "127.0.0.2"})) require.NoError(t, s.EnsureNode(4, &structs.Node{Node: "qux", Address: "127.0.0.3"})) + apigw := structs.TestNodeServiceAPIGateway(t) + apigw.ID = "api-gateway" + mgw := structs.TestNodeServiceMeshGateway(t) mgw.ID = "mesh-gateway" @@ -1141,16 +1176,17 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { require.NoError(t, s.EnsureRegistration(10, structs.TestRegisterIngressGateway(t))) require.NoError(t, s.EnsureService(11, "foo", mgw)) require.NoError(t, s.EnsureService(12, "foo", tgw)) - require.NoError(t, s.EnsureService(13, "bar", &structs.NodeService{ID: "db-native", Service: "db", Tags: nil, Address: "", Port: 5000, Connect: structs.ServiceConnect{Native: true}})) - require.NoError(t, s.EnsureConfigEntry(14, &structs.IngressGatewayConfigEntry{ + require.NoError(t, s.EnsureService(13, "foo", apigw)) + require.NoError(t, s.EnsureService(14, "bar", &structs.NodeService{ID: "db-native", Service: "db", Tags: nil, Address: "", Port: 5000, Connect: structs.ServiceConnect{Native: true}})) + require.NoError(t, s.EnsureConfigEntry(15, &structs.IngressGatewayConfigEntry{ Kind: structs.IngressGateway, Name: "foo", })) - require.NoError(t, s.EnsureConfigEntry(15, &structs.IngressGatewayConfigEntry{ + require.NoError(t, s.EnsureConfigEntry(16, &structs.IngressGatewayConfigEntry{ Kind: structs.IngressGateway, Name: "bar", })) - require.NoError(t, s.EnsureConfigEntry(16, &structs.IngressGatewayConfigEntry{ + require.NoError(t, s.EnsureConfigEntry(17, &structs.IngressGatewayConfigEntry{ Kind: structs.IngressGateway, Name: "baz", })) @@ -1191,22 +1227,22 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { } nodesAndSvcsCase.expectedGauges["consul.usage.test.consul.state.services;datacenter=dc1"] = metrics.GaugeValue{ // Legacy Name: "consul.usage.test.consul.state.services", - Value: 7, + Value: 8, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, } nodesAndSvcsCase.expectedGauges["consul.usage.test.state.services;datacenter=dc1"] = metrics.GaugeValue{ Name: "consul.usage.test.state.services", - Value: 7, + Value: 8, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, } nodesAndSvcsCase.expectedGauges["consul.usage.test.consul.state.service_instances;datacenter=dc1"] = metrics.GaugeValue{ // Legacy Name: "consul.usage.test.consul.state.service_instances", - Value: 9, + Value: 10, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, } nodesAndSvcsCase.expectedGauges["consul.usage.test.state.service_instances;datacenter=dc1"] = metrics.GaugeValue{ Name: "consul.usage.test.state.service_instances", - Value: 9, + Value: 10, Labels: []metrics.Label{{Name: "datacenter", Value: "dc1"}}, } nodesAndSvcsCase.expectedGauges["consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=connect-proxy"] = metrics.GaugeValue{ // Legacy @@ -1257,6 +1293,22 @@ func TestUsageReporter_emitServiceUsage_OSS(t *testing.T) { {Name: "kind", Value: "ingress-gateway"}, }, } + nodesAndSvcsCase.expectedGauges["consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=api-gateway"] = metrics.GaugeValue{ // Legacy + Name: "consul.usage.test.consul.state.connect_instances", + Value: 1, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + } + nodesAndSvcsCase.expectedGauges["consul.usage.test.state.connect_instances;datacenter=dc1;kind=api-gateway"] = metrics.GaugeValue{ + Name: "consul.usage.test.state.connect_instances", + Value: 1, + Labels: []metrics.Label{ + {Name: "datacenter", Value: "dc1"}, + {Name: "kind", Value: "api-gateway"}, + }, + } nodesAndSvcsCase.expectedGauges["consul.usage.test.consul.state.connect_instances;datacenter=dc1;kind=mesh-gateway"] = metrics.GaugeValue{ // Legacy Name: "consul.usage.test.consul.state.connect_instances", Value: 1, diff --git a/agent/operator_endpoint_oss_test.go b/agent/operator_endpoint_oss_test.go index ea144cffa692..90f0e7d0cd14 100644 --- a/agent/operator_endpoint_oss_test.go +++ b/agent/operator_endpoint_oss_test.go @@ -56,6 +56,7 @@ func TestOperator_Usage(t *testing.T) { Services: 5, ServiceInstances: 6, ConnectServiceInstances: map[string]int{ + "api-gateway": 0, "connect-native": 1, "connect-proxy": 1, "ingress-gateway": 0, diff --git a/agent/structs/testing_catalog.go b/agent/structs/testing_catalog.go index 0e560b3906ff..9e72aebc7745 100644 --- a/agent/structs/testing_catalog.go +++ b/agent/structs/testing_catalog.go @@ -174,6 +174,14 @@ func TestNodeServiceMeshGateway(t testing.T) *NodeService { ServiceAddress{Address: "198.18.4.5", Port: 443}) } +func TestNodeServiceAPIGateway(t testing.T) *NodeService { + return &NodeService{ + Kind: ServiceKindAPIGateway, + Service: "api-gateway", + Address: "1.1.1.1", + } +} + func TestNodeServiceTerminatingGateway(t testing.T, address string) *NodeService { return &NodeService{ Kind: ServiceKindTerminatingGateway, diff --git a/api/operator_usage_test.go b/api/operator_usage_test.go index a276682e07de..77b15fcdb586 100644 --- a/api/operator_usage_test.go +++ b/api/operator_usage_test.go @@ -56,6 +56,7 @@ func TestAPI_OperatorUsage(t *testing.T) { require.Equal(t, 4, usage.Usage["dc1"].Services) require.Equal(t, 5, usage.Usage["dc1"].ServiceInstances) require.Equal(t, map[string]int{ + "api-gateway": 0, "connect-native": 1, "connect-proxy": 1, "ingress-gateway": 0,