From 0090d86cc8bb32f4afde7f97dc5688a7ab20e011 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Thu, 12 Sep 2024 22:56:04 +0800 Subject: [PATCH 01/17] local rate limit: add new rate_limits api to the filter's api Signed-off-by: wangbaiping --- .../extensions/common/ratelimit/v3/BUILD | 3 + .../common/ratelimit/v3/ratelimit.proto | 251 ++++++++++++++++++ .../local_ratelimit/v3/local_rate_limit.proto | 10 + 3 files changed, 264 insertions(+) diff --git a/api/envoy/extensions/common/ratelimit/v3/BUILD b/api/envoy/extensions/common/ratelimit/v3/BUILD index ef19132f9180..a8033f668ebb 100644 --- a/api/envoy/extensions/common/ratelimit/v3/BUILD +++ b/api/envoy/extensions/common/ratelimit/v3/BUILD @@ -6,6 +6,9 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/config/core/v3:pkg", + "//envoy/config/route/v3:pkg", + "//envoy/type/metadata/v3:pkg", "//envoy/type/v3:pkg", "@com_github_cncf_xds//udpa/annotations:pkg", ], diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index 73d729adc269..30bddbc87ecd 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -4,6 +4,11 @@ package envoy.extensions.common.ratelimit.v3; import "envoy/type/v3/ratelimit_unit.proto"; import "envoy/type/v3/token_bucket.proto"; +import "envoy/config/route/v3/route_components.proto"; +import "google/protobuf/wrappers.proto"; +import "envoy/config/core/v3/extension.proto"; +import "envoy/config/core/v3/base.proto"; +import "envoy/type/metadata/v3/metadata.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; @@ -142,3 +147,249 @@ message LocalRateLimitDescriptor { // about local cluster. message LocalClusterRateLimit { } + +// Rate limit config that used to generate a list of descriptor entries (a list of key/value pairs) +// based on the request context. The generated entries will be used to much a specific rate limit +// rule. +// Take the local rate limit extension as an example, the generated entries will be used to find a +// ``LocalRateLimitDescriptor`` which contains a token bucket. And the token bucket will be used to +// limit the request rate. +// [#not-implemented-hide:] +message RateLimitConfig { + // [#next-free-field: 11] + message Action { + // The following descriptor entry is appended to the descriptor: + // + // .. code-block:: cpp + // + // ("source_cluster", "") + // + // is derived from the :option:`--service-cluster` option. + message SourceCluster { + } + + // The following descriptor entry is appended to the descriptor: + // + // .. code-block:: cpp + // + // ("destination_cluster", "") + // + // Once a request matches against a route table rule, a routed cluster is determined by one of + // the following :ref:`route table configuration ` + // settings: + // + // * :ref:`cluster ` indicates the upstream cluster + // to route to. + // * :ref:`weighted_clusters ` + // chooses a cluster randomly from a set of clusters with attributed weight. + // * :ref:`cluster_header ` indicates which + // header in the request contains the target cluster. + message DestinationCluster { + } + + // The following descriptor entry is appended when a header contains a key that matches the + // ``header_name``: + // + // .. code-block:: cpp + // + // ("", "") + message RequestHeaders { + // The header name to be queried from the request headers. The header’s + // value is used to populate the value of the descriptor entry for the + // descriptor_key. + string header_name = 1 + [(validate.rules).string = {min_len: 1 well_known_regex: HTTP_HEADER_NAME strict: false}]; + + // The key to use in the descriptor entry. + string descriptor_key = 2 [(validate.rules).string = {min_len: 1}]; + + // If set to true, Envoy skips the descriptor while calling rate limiting service + // when header is not present in the request. By default it skips calling the + // rate limiting service if this header is not present in the request. + bool skip_if_absent = 3; + } + + // The following descriptor entry is appended to the descriptor and is populated using the + // trusted address from :ref:`x-forwarded-for `: + // + // .. code-block:: cpp + // + // ("remote_address", "") + message RemoteAddress { + } + + // The following descriptor entry is appended to the descriptor and is populated using the + // masked address from :ref:`x-forwarded-for `: + // + // .. code-block:: cpp + // + // ("masked_remote_address", "") + message MaskedRemoteAddress { + // Length of prefix mask len for IPv4 (e.g. 0, 32). + // Defaults to 32 when unset. + // For example, trusted address from x-forwarded-for is ``192.168.1.1``, + // the descriptor entry is ("masked_remote_address", "192.168.1.1/32"); + // if mask len is 24, the descriptor entry is ("masked_remote_address", "192.168.1.0/24"). + google.protobuf.UInt32Value v4_prefix_mask_len = 1 [(validate.rules).uint32 = {lte: 32}]; + + // Length of prefix mask len for IPv6 (e.g. 0, 128). + // Defaults to 128 when unset. + // For example, trusted address from x-forwarded-for is ``2001:abcd:ef01:2345:6789:abcd:ef01:234``, + // the descriptor entry is ("masked_remote_address", "2001:abcd:ef01:2345:6789:abcd:ef01:234/128"); + // if mask len is 64, the descriptor entry is ("masked_remote_address", "2001:abcd:ef01:2345::/64"). + google.protobuf.UInt32Value v6_prefix_mask_len = 2 [(validate.rules).uint32 = {lte: 128}]; + } + + // The following descriptor entry is appended to the descriptor: + // + // .. code-block:: cpp + // + // ("generic_key", "") + message GenericKey { + // The value to use in the descriptor entry. + string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; + + // An optional key to use in the descriptor entry. If not set it defaults + // to 'generic_key' as the descriptor key. + string descriptor_key = 2; + } + + // The following descriptor entry is appended to the descriptor: + // + // .. code-block:: cpp + // + // ("header_match", "") + message HeaderValueMatch { + // The key to use in the descriptor entry. Defaults to ``header_match``. + string descriptor_key = 4; + + // The value to use in the descriptor entry. + string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; + + // If set to true, the action will append a descriptor entry when the + // request matches the headers. If set to false, the action will append a + // descriptor entry when the request does not match the headers. The + // default value is true. + google.protobuf.BoolValue expect_match = 2; + + // Specifies a set of headers that the rate limit action should match + // on. The action will check the request’s headers against all the + // specified headers in the config. A match will happen if all the + // headers in the config are present in the request with the same values + // (or based on presence if the value field is not in the config). + repeated config.route.v3.HeaderMatcher headers = 3 [(validate.rules).repeated = {min_items: 1}]; + } + + // The following descriptor entry is appended when the metadata contains a key value: + // + // .. code-block:: cpp + // + // ("", "") + // [#next-free-field: 6] + message MetaData { + enum Source { + // Query :ref:`dynamic metadata ` + DYNAMIC = 0; + + // Query :ref:`route entry metadata ` + ROUTE_ENTRY = 1; + } + + // The key to use in the descriptor entry. + string descriptor_key = 1 [(validate.rules).string = {min_len: 1}]; + + // Metadata struct that defines the key and path to retrieve the string value. A match will + // only happen if the value in the metadata is of type string. + type.metadata.v3.MetadataKey metadata_key = 2 [(validate.rules).message = {required: true}]; + + // An optional value to use if ``metadata_key`` is empty. If not set and + // no value is present under the metadata_key then ``skip_if_absent`` is followed to + // skip calling the rate limiting service or skip the descriptor. + string default_value = 3; + + // Source of metadata + Source source = 4 [(validate.rules).enum = {defined_only: true}]; + + // If set to true, Envoy skips the descriptor while calling rate limiting service + // when ``metadata_key`` is empty and ``default_value`` is not set. By default it skips calling the + // rate limiting service in that case. + bool skip_if_absent = 5; + } + + // The following descriptor entry is appended to the descriptor: + // + // .. code-block:: cpp + // + // ("query_match", "") + message QueryParameterValueMatch { + // The key to use in the descriptor entry. Defaults to ``query_match``. + string descriptor_key = 4; + + // The value to use in the descriptor entry. + string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; + + // If set to true, the action will append a descriptor entry when the + // request matches the headers. If set to false, the action will append a + // descriptor entry when the request does not match the headers. The + // default value is true. + google.protobuf.BoolValue expect_match = 2; + + // Specifies a set of query parameters that the rate limit action should match + // on. The action will check the request’s query parameters against all the + // specified query parameters in the config. A match will happen if all the + // query parameters in the config are present in the request with the same values + // (or based on presence if the value field is not in the config). + repeated config.route.v3.QueryParameterMatcher query_parameters = 3 + [(validate.rules).repeated = {min_items: 1}]; + } + + oneof action_specifier { + option (validate.required) = true; + + // Rate limit on source cluster. + SourceCluster source_cluster = 1; + + // Rate limit on destination cluster. + DestinationCluster destination_cluster = 2; + + // Rate limit on request headers. + RequestHeaders request_headers = 3; + + // Rate limit on remote address. + RemoteAddress remote_address = 4; + + // Rate limit on a generic key. + GenericKey generic_key = 5; + + // Rate limit on the existence of request headers. + HeaderValueMatch header_value_match = 6; + + // Rate limit on metadata. + MetaData metadata = 7; + + // Rate limit descriptor extension. See the rate limit descriptor extensions documentation. + // + // :ref:`HTTP matching input functions ` are + // permitted as descriptor extensions. The input functions are only + // looked up if there is no rate limit descriptor extension matching + // the type URL. + // + // [#extension-category: envoy.rate_limit_descriptors] + config.core.v3.TypedExtensionConfig extension = 8; + + // Rate limit on masked remote address. + MaskedRemoteAddress masked_remote_address = 9; + + // Rate limit on the existence of query parameters. + QueryParameterValueMatch query_parameter_value_match = 10; + } + } + + // A list of actions that are to be applied for this rate limit configuration. + // Order matters as the actions are processed sequentially and the descriptor + // is composed by appending descriptor entries in that sequence. If an action + // cannot append a descriptor entry, no descriptor is generated for the + // configuration. See :ref:`composing actions + // ` for additional documentation. + repeated Action actions = 1 [(validate.rules).repeated = {min_items: 1}]; +} diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index a32475f352f3..697200738cf5 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -147,4 +147,14 @@ message LocalRateLimit { // of the default ``UNAVAILABLE`` gRPC code for a rate limited gRPC call. The // HTTP code will be 200 for a gRPC response. bool rate_limited_as_resource_exhausted = 15; + + // Rate limit configuration that used to generate a list of description entries based on + // the request context. And the generated entries will be used to find one or multiple matched rate + // limit rule from the ``descriptors``. + // If this is set, then + // :ref:`VirtualHost.rate_limits` or + // :ref:`RouteAction.rate_limits` fields + // will be ignored. + // [#not-implemented-hide:] + repeated common.ratelimit.v3.RateLimitConfig rate_limits = 17; } From 111aaf799744603bc6106f8c2923608ed8974cb2 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Fri, 13 Sep 2024 14:12:04 +0800 Subject: [PATCH 02/17] address format Signed-off-by: wangbaiping --- .../extensions/common/ratelimit/v3/ratelimit.proto | 11 ++++++----- .../http/local_ratelimit/v3/local_rate_limit.proto | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index 30bddbc87ecd..82941960a24b 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -2,13 +2,13 @@ syntax = "proto3"; package envoy.extensions.common.ratelimit.v3; +import "envoy/config/core/v3/extension.proto"; +import "envoy/config/route/v3/route_components.proto"; +import "envoy/type/metadata/v3/metadata.proto"; import "envoy/type/v3/ratelimit_unit.proto"; import "envoy/type/v3/token_bucket.proto"; -import "envoy/config/route/v3/route_components.proto"; + import "google/protobuf/wrappers.proto"; -import "envoy/config/core/v3/extension.proto"; -import "envoy/config/core/v3/base.proto"; -import "envoy/type/metadata/v3/metadata.proto"; import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; @@ -277,7 +277,8 @@ message RateLimitConfig { // specified headers in the config. A match will happen if all the // headers in the config are present in the request with the same values // (or based on presence if the value field is not in the config). - repeated config.route.v3.HeaderMatcher headers = 3 [(validate.rules).repeated = {min_items: 1}]; + repeated config.route.v3.HeaderMatcher headers = 3 + [(validate.rules).repeated = {min_items: 1}]; } // The following descriptor entry is appended when the metadata contains a key value: diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index 697200738cf5..fbf9f71b645a 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -22,7 +22,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Local Rate limit :ref:`configuration overview `. // [#extension: envoy.filters.http.local_ratelimit] -// [#next-free-field: 17] +// [#next-free-field: 18] message LocalRateLimit { // The human readable prefix to use when emitting stats. string stat_prefix = 1 [(validate.rules).string = {min_len: 1}]; From 68df70cb1dbb9b1a0746eadda1aef5bbadf195a3 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Wed, 18 Sep 2024 21:21:57 +0800 Subject: [PATCH 03/17] basic rate limit Signed-off-by: wangbaiping --- .../common/ratelimit/v3/ratelimit.proto | 2 +- .../local_ratelimit/v3/local_rate_limit.proto | 2 +- source/common/router/BUILD | 1 + source/common/router/router_ratelimit.cc | 38 +- source/common/router/router_ratelimit.h | 35 +- .../filters/common/ratelimit_config/BUILD | 23 + .../ratelimit_config/ratelimit_config.cc | 116 ++ .../ratelimit_config/ratelimit_config.h | 56 + .../filters/http/local_ratelimit/config.cc | 11 +- .../http/local_ratelimit/local_ratelimit.cc | 25 +- .../http/local_ratelimit/local_ratelimit.h | 21 +- .../filters/common/ratelimit_config/BUILD | 36 + .../ratelimit_config/ratelimit_config_test.cc | 1107 +++++++++++++++++ .../ratelimit_config_test.proto | 10 + 14 files changed, 1457 insertions(+), 26 deletions(-) create mode 100644 source/extensions/filters/common/ratelimit_config/BUILD create mode 100644 source/extensions/filters/common/ratelimit_config/ratelimit_config.cc create mode 100644 source/extensions/filters/common/ratelimit_config/ratelimit_config.h create mode 100644 test/extensions/filters/common/ratelimit_config/BUILD create mode 100644 test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc create mode 100644 test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index 82941960a24b..09a32a752d01 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -155,7 +155,7 @@ message LocalClusterRateLimit { // ``LocalRateLimitDescriptor`` which contains a token bucket. And the token bucket will be used to // limit the request rate. // [#not-implemented-hide:] -message RateLimitConfig { +message RateLimitPolicy { // [#next-free-field: 11] message Action { // The following descriptor entry is appended to the descriptor: diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index fbf9f71b645a..474f773ac46b 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -156,5 +156,5 @@ message LocalRateLimit { // :ref:`RouteAction.rate_limits` fields // will be ignored. // [#not-implemented-hide:] - repeated common.ratelimit.v3.RateLimitConfig rate_limits = 17; + repeated common.ratelimit.v3.RateLimitPolicy rate_limits = 17; } diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 70200e05eaff..c2ff0159041d 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -433,6 +433,7 @@ envoy_cc_library( "//source/common/protobuf:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", ], ) diff --git a/source/common/router/router_ratelimit.cc b/source/common/router/router_ratelimit.cc index db2dcc45e298..d5fc5376820e 100644 --- a/source/common/router/router_ratelimit.cc +++ b/source/common/router/router_ratelimit.cc @@ -8,6 +8,7 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "source/common/common/assert.h" #include "source/common/common/empty_string.h" #include "source/common/config/metadata.h" @@ -191,6 +192,16 @@ MetaDataAction::MetaDataAction(const envoy::config::route::v3::RateLimit::Action default_value_(action.default_value()), source_(action.source()), skip_if_absent_(action.skip_if_absent()) {} +MetaDataAction::MetaDataAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MetaData& action) + : metadata_key_(action.metadata_key()), descriptor_key_(action.descriptor_key()), + default_value_(action.default_value()), + source_(action.source() == envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action:: + MetaData::ROUTE_ENTRY + ? envoy::config::route::v3::RateLimit::Action::MetaData::ROUTE_ENTRY + : envoy::config::route::v3::RateLimit::Action::MetaData::DYNAMIC), + skip_if_absent_(action.skip_if_absent()) {} + MetaDataAction::MetaDataAction( const envoy::config::route::v3::RateLimit::Action::DynamicMetaData& action) : metadata_key_(action.metadata_key()), descriptor_key_(action.descriptor_key()), @@ -238,6 +249,15 @@ HeaderValueMatchAction::HeaderValueMatchAction( expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers(), context)) {} +HeaderValueMatchAction::HeaderValueMatchAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::HeaderValueMatch& + action, + Server::Configuration::CommonFactoryContext& context) + : descriptor_value_(action.descriptor_value()), + descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "header_match"), + expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), + action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers(), context)) {} + bool HeaderValueMatchAction::populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string&, const Http::RequestHeaderMap& headers, @@ -256,7 +276,18 @@ QueryParameterValueMatchAction::QueryParameterValueMatchAction( : descriptor_value_(action.descriptor_value()), descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "query_match"), expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), - action_query_parameters_(buildQueryParameterMatcherVector(action, context)) {} + action_query_parameters_( + buildQueryParameterMatcherVector(action.query_parameters(), context)) {} + +QueryParameterValueMatchAction::QueryParameterValueMatchAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action:: + QueryParameterValueMatch& action, + Server::Configuration::CommonFactoryContext& context) + : descriptor_value_(action.descriptor_value()), + descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "query_match"), + expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), + action_query_parameters_( + buildQueryParameterMatcherVector(action.query_parameters(), context)) {} bool QueryParameterValueMatchAction::populateDescriptor( RateLimit::DescriptorEntry& descriptor_entry, const std::string&, @@ -274,10 +305,11 @@ bool QueryParameterValueMatchAction::populateDescriptor( std::vector QueryParameterValueMatchAction::buildQueryParameterMatcherVector( - const envoy::config::route::v3::RateLimit::Action::QueryParameterValueMatch& action, + const Protobuf::RepeatedPtrField& + query_parameters, Server::Configuration::CommonFactoryContext& context) { std::vector ret; - for (const auto& query_parameter : action.query_parameters()) { + for (const auto& query_parameter : query_parameters) { ret.push_back(std::make_unique(query_parameter, context)); } return ret; diff --git a/source/common/router/router_ratelimit.h b/source/common/router/router_ratelimit.h index b069af8d7ed6..3a87e8a204b3 100644 --- a/source/common/router/router_ratelimit.h +++ b/source/common/router/router_ratelimit.h @@ -16,6 +16,7 @@ #include "source/common/network/cidr_range.h" #include "source/common/protobuf/utility.h" #include "source/common/router/config_utility.h" +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "absl/types/optional.h" @@ -72,6 +73,12 @@ class RequestHeadersAction : public RateLimit::DescriptorProducer { : header_name_(action.header_name()), descriptor_key_(action.descriptor_key()), skip_if_absent_(action.skip_if_absent()) {} + RequestHeadersAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::RequestHeaders& + action) + : header_name_(action.header_name()), descriptor_key_(action.descriptor_key()), + skip_if_absent_(action.skip_if_absent()) {} + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -106,6 +113,12 @@ class MaskedRemoteAddressAction : public RateLimit::DescriptorProducer { : v4_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v4_prefix_mask_len, 32)), v6_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v6_prefix_mask_len, 128)) {} + MaskedRemoteAddressAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MaskedRemoteAddress& + action) + : v4_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v4_prefix_mask_len, 32)), + v6_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v6_prefix_mask_len, 128)) {} + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -127,6 +140,12 @@ class GenericKeyAction : public RateLimit::DescriptorProducer { descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "generic_key") {} + GenericKeyAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::GenericKey& action) + : descriptor_value_(action.descriptor_value()), + descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() + : "generic_key") {} + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -146,6 +165,10 @@ class MetaDataAction : public RateLimit::DescriptorProducer { MetaDataAction(const envoy::config::route::v3::RateLimit::Action::MetaData& action); // for maintaining backward compatibility with the deprecated DynamicMetaData action MetaDataAction(const envoy::config::route::v3::RateLimit::Action::DynamicMetaData& action); + + MetaDataAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MetaData& action); + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -169,6 +192,11 @@ class HeaderValueMatchAction : public RateLimit::DescriptorProducer { const envoy::config::route::v3::RateLimit::Action::HeaderValueMatch& action, Server::Configuration::CommonFactoryContext& context); + HeaderValueMatchAction( + const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::HeaderValueMatch& + action, + Server::Configuration::CommonFactoryContext& context); + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -191,6 +219,10 @@ class QueryParameterValueMatchAction : public RateLimit::DescriptorProducer { const envoy::config::route::v3::RateLimit::Action::QueryParameterValueMatch& action, Server::Configuration::CommonFactoryContext& context); + QueryParameterValueMatchAction(const envoy::extensions::common::ratelimit::v3::RateLimitPolicy:: + Action::QueryParameterValueMatch& action, + Server::Configuration::CommonFactoryContext& context); + // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -198,7 +230,8 @@ class QueryParameterValueMatchAction : public RateLimit::DescriptorProducer { const StreamInfo::StreamInfo& info) const override; std::vector buildQueryParameterMatcherVector( - const envoy::config::route::v3::RateLimit::Action::QueryParameterValueMatch& action, + const Protobuf::RepeatedPtrField& + query_parameters, Server::Configuration::CommonFactoryContext& context); private: diff --git a/source/extensions/filters/common/ratelimit_config/BUILD b/source/extensions/filters/common/ratelimit_config/BUILD new file mode 100644 index 000000000000..718178d55f42 --- /dev/null +++ b/source/extensions/filters/common/ratelimit_config/BUILD @@ -0,0 +1,23 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_library( + name = "ratelimit_config_lib", + srcs = ["ratelimit_config.cc"], + hdrs = ["ratelimit_config.h"], + external_deps = [ + "abseil_strings", + "abseil_inlined_vector", + ], + deps = [ + "//envoy/ratelimit:ratelimit_interface", + "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc new file mode 100644 index 000000000000..e363157ed262 --- /dev/null +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -0,0 +1,116 @@ +#include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" +#include "ratelimit_config.h" +#include "source/common/config/utility.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RateLimit { + +RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, + Server::Configuration::CommonFactoryContext& context, + absl::Status& creation_status) { + for (const ProtoRateLimit::Action& action : config.actions()) { + switch (action.action_specifier_case()) { + case ProtoRateLimit::Action::ActionSpecifierCase::kSourceCluster: + actions_.emplace_back(new Envoy::Router::SourceClusterAction()); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kDestinationCluster: + actions_.emplace_back(new Envoy::Router::DestinationClusterAction()); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kRequestHeaders: + actions_.emplace_back(new Envoy::Router::RequestHeadersAction(action.request_headers())); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kRemoteAddress: + actions_.emplace_back(new Envoy::Router::RemoteAddressAction()); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kGenericKey: + actions_.emplace_back(new Envoy::Router::GenericKeyAction(action.generic_key())); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kMetadata: + actions_.emplace_back(new Envoy::Router::MetaDataAction(action.metadata())); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kHeaderValueMatch: + actions_.emplace_back( + new Envoy::Router::HeaderValueMatchAction(action.header_value_match(), context)); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kExtension: { + ProtobufMessage::ValidationVisitor& validator = context.messageValidationVisitor(); + auto* factory = + Envoy::Config::Utility::getFactory( + action.extension()); + if (!factory) { + creation_status = absl::InvalidArgumentError( + fmt::format("rate limit action: {} is not supported", action.extension().name())); + return; + } + auto message = Envoy::Config::Utility::translateAnyToFactoryConfig( + action.extension().typed_config(), validator, *factory); + Envoy::RateLimit::DescriptorProducerPtr producer = + factory->createDescriptorProducerFromProto(*message, context); + if (producer) { + actions_.emplace_back(std::move(producer)); + } else { + creation_status = absl::InvalidArgumentError( + absl::StrCat("rate limit descriptor extension failed: ", action.extension().name())); + return; + } + break; + } + case ProtoRateLimit::Action::ActionSpecifierCase::kMaskedRemoteAddress: + actions_.emplace_back(new Router::MaskedRemoteAddressAction(action.masked_remote_address())); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::kQueryParameterValueMatch: + actions_.emplace_back(new Router::QueryParameterValueMatchAction( + action.query_parameter_value_match(), context)); + break; + case ProtoRateLimit::Action::ActionSpecifierCase::ACTION_SPECIFIER_NOT_SET: + PANIC_DUE_TO_CORRUPT_ENUM; + } + } +} + +void RateLimitPolicy::populateDescriptors(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& stream_info, + const std::string& local_service_cluster, + RateLimitDescriptors& descriptors) const { + Envoy::RateLimit::LocalDescriptor descriptor; + for (const Envoy::RateLimit::DescriptorProducerPtr& action : actions_) { + Envoy::RateLimit::DescriptorEntry entry; + if (!action->populateDescriptor(entry, local_service_cluster, headers, stream_info)) { + return; + } + if (!entry.key_.empty()) { + descriptor.entries_.emplace_back(std::move(entry)); + } + } + descriptors.emplace_back(std::move(descriptor)); +} + +RateLimitConfig::RateLimitConfig(const Protobuf::RepeatedPtrField& configs, + Server::Configuration::CommonFactoryContext& context, + absl::Status& creation_status) { + for (const ProtoRateLimit& config : configs) { + auto descriptor_generator = std::make_unique(config, context, creation_status); + if (!creation_status.ok()) { + return; + } + rate_limit_policies_.emplace_back(std::move(descriptor_generator)); + } +} + +void RateLimitConfig::populateDescriptors(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& stream_info, + const std::string& local_service_cluster, + RateLimitDescriptors& descriptors) const { + for (const auto& generator : rate_limit_policies_) { + generator->populateDescriptors(headers, stream_info, local_service_cluster, descriptors); + } +} + +} // namespace RateLimit +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h new file mode 100644 index 000000000000..ee4b00b4759b --- /dev/null +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h @@ -0,0 +1,56 @@ +#pragma once + +#include "envoy/ratelimit/ratelimit.h" +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" +#include "absl/container/inlined_vector.h" +#include "google/protobuf/repeated_ptr_field.h" +#include "source/common/router/router_ratelimit.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RateLimit { + +using ProtoRateLimit = envoy::extensions::common::ratelimit::v3::RateLimitPolicy; +using RateLimitDescriptors = std::vector; + +class RateLimitPolicy { +public: + RateLimitPolicy(const ProtoRateLimit& config, + Server::Configuration::CommonFactoryContext& context, + absl::Status& creation_status); + + void populateDescriptors(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info, + const std::string& local_service_cluster, + RateLimitDescriptors& descriptors) const; + +private: + std::vector actions_; +}; + +class RateLimitConfig { +public: + RateLimitConfig(const Protobuf::RepeatedPtrField& configs, + Server::Configuration::CommonFactoryContext& context, + absl::Status& creation_status); + + bool empty() const { return rate_limit_policies_.empty(); } + + size_t size() const { return rate_limit_policies_.size(); } + + void populateDescriptors(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info, + const std::string& local_service_cluster, + RateLimitDescriptors& descriptors) const; + +private: + std::vector> rate_limit_policies_; +}; + +} // namespace RateLimit +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/http/local_ratelimit/config.cc b/source/extensions/filters/http/local_ratelimit/config.cc index e0990cf4598f..2228619acc75 100644 --- a/source/extensions/filters/http/local_ratelimit/config.cc +++ b/source/extensions/filters/http/local_ratelimit/config.cc @@ -15,12 +15,9 @@ namespace LocalRateLimitFilter { Http::FilterFactoryCb LocalRateLimitFilterConfig::createFilterFactoryFromProtoTyped( const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& proto_config, const std::string&, Server::Configuration::FactoryContext& context) { - auto& server_context = context.serverFactoryContext(); - FilterConfigSharedPtr filter_config = std::make_shared( - proto_config, server_context.localInfo(), server_context.mainThreadDispatcher(), - server_context.clusterManager(), server_context.singletonManager(), context.scope(), - server_context.runtime()); + FilterConfigSharedPtr filter_config = + std::make_shared(proto_config, context.serverFactoryContext(), context.scope()); return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamFilter(std::make_shared(filter_config)); }; @@ -30,9 +27,7 @@ Router::RouteSpecificFilterConfigConstSharedPtr LocalRateLimitFilterConfig::createRouteSpecificFilterConfigTyped( const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& proto_config, Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) { - return std::make_shared( - proto_config, context.localInfo(), context.mainThreadDispatcher(), context.clusterManager(), - context.singletonManager(), context.scope(), context.runtime(), true); + return std::make_shared(proto_config, context, context.scope(), true); } /** diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index 8356fa49b652..59e5521aaeb4 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -23,10 +23,8 @@ const std::string& PerConnectionRateLimiter::key() { FilterConfig::FilterConfig( const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& config, - const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Upstream::ClusterManager& cm, Singleton::Manager& singleton_manager, Stats::Scope& scope, - Runtime::Loader& runtime, const bool per_route) - : dispatcher_(dispatcher), status_(toErrorCode(config.status().code())), + Server::Configuration::CommonFactoryContext& context, Stats::Scope& scope, const bool per_route) + : dispatcher_(context.mainThreadDispatcher()), status_(toErrorCode(config.status().code())), stats_(generateStats(config.stat_prefix(), scope)), fill_interval_(std::chrono::milliseconds( PROTOBUF_GET_MS_OR_DEFAULT(config.token_bucket(), fill_interval, 0))), @@ -38,7 +36,7 @@ FilterConfig::FilterConfig( config.has_always_consume_default_token_bucket() ? config.always_consume_default_token_bucket().value() : true), - local_info_(local_info), runtime_(runtime), + local_info_(context.localInfo()), runtime_(context.runtime()), filter_enabled_( config.has_filter_enabled() ? absl::optional( @@ -73,20 +71,25 @@ FilterConfig::FilterConfig( throw EnvoyException("local rate limit token bucket must be set for per filter configs"); } + absl::Status creation_status; + rate_limit_config_ = std::make_unique( + config.rate_limits(), context, creation_status); + THROW_IF_NOT_OK_REF(creation_status); + Filters::Common::LocalRateLimit::ShareProviderSharedPtr share_provider; if (config.has_local_cluster_rate_limit()) { if (rate_limit_per_connection_) { throw EnvoyException("local_cluster_rate_limit is set and " "local_rate_limit_per_downstream_connection is set to true"); } - if (!cm.localClusterName().has_value()) { + if (!context.clusterManager().localClusterName().has_value()) { throw EnvoyException("local_cluster_rate_limit is set but no local cluster name is present"); } // If the local cluster name is set then the relevant cluster must exist or the cluster // manager will fail to initialize. share_provider_manager_ = Filters::Common::LocalRateLimit::ShareProviderManager::singleton( - dispatcher, cm, singleton_manager); + dispatcher_, context.clusterManager(), context.singletonManager()); if (!share_provider_manager_) { throw EnvoyException("local_cluster_rate_limit is set but no local cluster is present"); } @@ -95,7 +98,7 @@ FilterConfig::FilterConfig( } rate_limiter_ = std::make_unique( - fill_interval_, max_tokens_, tokens_per_fill_, dispatcher, descriptors_, + fill_interval_, max_tokens_, tokens_per_fill_, dispatcher_, descriptors_, always_consume_default_token_bucket_, std::move(share_provider)); } @@ -137,7 +140,11 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::RequestHeaderMap& headers, std::vector descriptors; if (used_config_->hasDescriptors()) { - populateDescriptors(descriptors, headers); + if (used_config_->hasRateLimitConfigs()) { + used_config_->populateDescriptors(headers, decoder_callbacks_->streamInfo(), descriptors); + } else { + populateDescriptors(descriptors, headers); + } } if (ENVOY_LOG_CHECK_LEVEL(debug)) { diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h index d23f7228f03c..2c3125dd0000 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h @@ -19,6 +19,7 @@ #include "source/common/runtime/runtime_protos.h" #include "source/extensions/filters/common/local_ratelimit/local_ratelimit_impl.h" #include "source/extensions/filters/common/ratelimit/ratelimit.h" +#include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" #include "source/extensions/filters/http/common/pass_through_filter.h" namespace Envoy { @@ -72,9 +73,8 @@ class PerConnectionRateLimiter : public StreamInfo::FilterState::Object { class FilterConfig : public Router::RouteSpecificFilterConfig { public: FilterConfig(const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& config, - const LocalInfo::LocalInfo& local_info, Event::Dispatcher& dispatcher, - Upstream::ClusterManager& cm, Singleton::Manager& singleton_manager, - Stats::Scope& scope, Runtime::Loader& runtime, bool per_route = false); + Server::Configuration::CommonFactoryContext& context, Stats::Scope& scope, + const bool per_route = false); ~FilterConfig() override { // Ensure that the LocalRateLimiterImpl instance will be destroyed on the thread where its inner // timer is created and running. @@ -113,6 +113,20 @@ class FilterConfig : public Router::RouteSpecificFilterConfig { return rate_limited_grpc_status_; } + bool hasRateLimitPerConnection() const { return rate_limit_per_connection_; } + + bool hasRateLimitConfigs() const { + ASSERT(rate_limit_config_ != nullptr); + return !rate_limit_config_->empty(); + } + + void populateDescriptors(const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info, + Filters::Common::RateLimit::RateLimitDescriptors& descriptors) const { + ASSERT(rate_limit_config_ != nullptr); + rate_limit_config_->populateDescriptors(headers, info, local_info_.clusterName(), descriptors); + } + private: friend class FilterTest; @@ -150,6 +164,7 @@ class FilterConfig : public Router::RouteSpecificFilterConfig { const bool enable_x_rate_limit_headers_; const envoy::extensions::common::ratelimit::v3::VhRateLimitsOptions vh_rate_limits_; const absl::optional rate_limited_grpc_status_; + std::unique_ptr rate_limit_config_; }; using FilterConfigSharedPtr = std::shared_ptr; diff --git a/test/extensions/filters/common/ratelimit_config/BUILD b/test/extensions/filters/common/ratelimit_config/BUILD new file mode 100644 index 000000000000..f0a0761af836 --- /dev/null +++ b/test/extensions/filters/common/ratelimit_config/BUILD @@ -0,0 +1,36 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", + "envoy_proto_library", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_proto_library( + name = "ratelimit_config_test_proto", + srcs = ["ratelimit_config_test.proto"], + deps = [ + "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg", + ], +) + +envoy_cc_test( + name = "ratelimit_config_test", + srcs = ["ratelimit_config_test.cc"], + deps = [ + ":ratelimit_config_test_proto_cc_proto", + "//source/common/http:header_map_lib", + "//source/common/protobuf:utility_lib", + "//source/common/router:config_lib", + "//source/extensions/filters/common/ratelimit_config:ratelimit_config_lib", + "//test/mocks/http:http_mocks", + "//test/mocks/ratelimit:ratelimit_mocks", + "//test/mocks/router:router_mocks", + "//test/mocks/server:instance_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/route/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc new file mode 100644 index 000000000000..b566f31a21b9 --- /dev/null +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -0,0 +1,1107 @@ +#include +#include +#include + +#include "envoy/config/route/v3/route.pb.h" + +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" +#include "source/common/http/header_map_impl.h" +#include "source/common/network/address_impl.h" +#include "source/common/protobuf/utility.h" +#include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" + +#include "test/mocks/http/mocks.h" +#include "test/mocks/ratelimit/mocks.h" +#include "test/mocks/router/mocks.h" +#include "test/mocks/server/instance.h" +#include "test/test_common/printers.h" +#include "test/test_common/utility.h" +#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.h" +#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.validate.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::NiceMock; + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RateLimit { +namespace { + +envoy::extensions::common::ratelimit::v3::RateLimitPolicy +parseRateLimitFromV3Yaml(const std::string& yaml_string) { + envoy::extensions::common::ratelimit::v3::RateLimitPolicy rate_limit; + TestUtility::loadFromYaml(yaml_string, rate_limit); + TestUtility::validate(rate_limit); + return rate_limit; +} + +TEST(BadRateLimitConfiguration, MissingActions) { + EXPECT_THROW_WITH_REGEX(parseRateLimitFromV3Yaml("{}"), EnvoyException, + "value must contain at least"); +} + +TEST(BadRateLimitConfiguration, ActionsMissingRequiredFields) { + const std::string yaml_one = R"EOF( +actions: +- request_headers: {} + )EOF"; + + EXPECT_THROW_WITH_REGEX(parseRateLimitFromV3Yaml(yaml_one), EnvoyException, + "value length must be at least"); + + const std::string yaml_two = R"EOF( +actions: +- request_headers: + header_name: test + )EOF"; + + EXPECT_THROW_WITH_REGEX(parseRateLimitFromV3Yaml(yaml_two), EnvoyException, + "value length must be at least"); + + const std::string yaml_three = R"EOF( +actions: +- request_headers: + descriptor_key: test + )EOF"; + + EXPECT_THROW_WITH_REGEX(parseRateLimitFromV3Yaml(yaml_three), EnvoyException, + "value length must be at least"); +} + +class RateLimitConfigTest : public testing::Test { +public: + void setupTest(const std::string& yaml) { + test::extensions::filters::common::ratelimit_config::TestRateLimitConfig proto_config; + TestUtility::loadFromYaml(yaml, proto_config); + config_ = std::make_unique( + proto_config.rate_limits(), factory_context_, creation_status_); + stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + } + + NiceMock factory_context_; + ProtobufMessage::NullValidationVisitorImpl any_validation_visitor_; + absl::Status creation_status_{}; + std::unique_ptr config_; + Http::TestRequestHeaderMapImpl header_; + Network::Address::InstanceConstSharedPtr default_remote_address_{ + new Network::Address::Ipv4Instance("10.0.0.1")}; + NiceMock stream_info_; +}; + +TEST_F(RateLimitConfigTest, EmptyRateLimit) { + const std::string yaml = R"EOF( +rate_limits: [] + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); + setupTest(yaml); + + EXPECT_TRUE(config_->empty()); +} + +TEST_F(RateLimitConfigTest, SinglePolicy) { + const std::string yaml = R"EOF( + rate_limits: + - actions: + - remote_address: {} + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); + setupTest(yaml); + + EXPECT_EQ(1U, config_->size()); + + std::vector descriptors; + config_->populateDescriptors(header_, stream_info_, "", descriptors); + EXPECT_THAT(std::vector({{{{"remote_address", "10.0.0.1"}}}}), + testing::ContainerEq(descriptors)); +} + +TEST_F(RateLimitConfigTest, MultiplePoliciesAndMultipleActions) { + const std::string yaml = R"EOF( + rate_limits: + - actions: + - remote_address: {} + - destination_cluster: {} + - actions: + - destination_cluster: {} + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2test"}, {}); + setupTest(yaml); + + std::vector descriptors; + + EXPECT_THAT(std::vector( + {Envoy::RateLimit::LocalDescriptor{ + {{"remote_address", "10.0.0.1"}, {"destination_cluster", "www2test"}}}, + Envoy::RateLimit::LocalDescriptor{{{"destination_cluster", "www2test"}}}}), + testing::ContainerEq(descriptors)); +} + +class RateLimitPolicyTest : public testing::Test { +public: + void setupTest(const std::string& yaml) { + absl::Status creation_status; + rate_limit_entry_ = std::make_unique(parseRateLimitFromV3Yaml(yaml), + factory_context_, creation_status); + THROW_IF_NOT_OK(creation_status); // NOLINT + descriptors_.clear(); + stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + } + + NiceMock factory_context_; + std::unique_ptr rate_limit_entry_; + Http::TestRequestHeaderMapImpl header_; + std::shared_ptr route_{new NiceMock()}; + + std::vector descriptors_; + Network::Address::InstanceConstSharedPtr default_remote_address_{ + new Network::Address::Ipv4Instance("10.0.0.1")}; + NiceMock stream_info_; +}; + +class RateLimitPolicyIpv6Test : public testing::Test { +public: + void setupTest(const std::string& yaml) { + absl::Status creation_status; + rate_limit_entry_ = std::make_unique(parseRateLimitFromV3Yaml(yaml), + factory_context_, creation_status); + THROW_IF_NOT_OK(creation_status); // NOLINT + descriptors_.clear(); + stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + } + + NiceMock factory_context_; + std::unique_ptr rate_limit_entry_; + Http::TestRequestHeaderMapImpl header_; + std::vector descriptors_; + std::shared_ptr route_{new NiceMock()}; + + Network::Address::InstanceConstSharedPtr default_remote_address_{ + new Network::Address::Ipv6Instance("2001:abcd:ef01:2345:6789:abcd:ef01:234")}; + NiceMock stream_info_; +}; + +TEST_F(RateLimitPolicyTest, RemoteAddress) { + const std::string yaml = R"EOF( +actions: +- remote_address: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"remote_address", "10.0.0.1"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MaskedRemoteAddressIpv4Default) { + const std::string yaml = R"EOF( +actions: +- masked_remote_address: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector( + {{{{"masked_remote_address", "10.0.0.1/32"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MaskedRemoteAddressIpv4) { + const std::string yaml = R"EOF( +actions: +- masked_remote_address: + v4_prefix_mask_len: 16 + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector( + {{{{"masked_remote_address", "10.0.0.0/16"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyIpv6Test, MaskedRemoteAddressIpv6Default) { + const std::string yaml = R"EOF( +actions: +- masked_remote_address: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector( + {{{{"masked_remote_address", "2001:abcd:ef01:2345:6789:abcd:ef01:234/128"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyIpv6Test, MaskedRemoteAddressIpv6) { + const std::string yaml = R"EOF( +actions: +- masked_remote_address: + v6_prefix_mask_len: 64 + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector( + {{{{"masked_remote_address", "2001:abcd:ef01:2345::/64"}}}}), + testing::ContainerEq(descriptors_)); +} + +// Verify no descriptor is emitted if remote is a pipe. +TEST_F(RateLimitPolicyTest, PipeAddress) { + const std::string yaml = R"EOF( +actions: +- remote_address: {} + )EOF"; + + setupTest(yaml); + + stream_info_.downstream_connection_info_provider_->setRemoteAddress( + *Network::Address::PipeInstance::create("/hello")); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, SourceService) { + const std::string yaml = R"EOF( +actions: +- source_cluster: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector({{{{"source_cluster", "service_cluster"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, DestinationService) { + const std::string yaml = R"EOF( +actions: +- destination_cluster: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector({{{{"destination_cluster", "fake_cluster"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, RequestHeaders) { + const std::string yaml = R"EOF( +actions: +- request_headers: + header_name: x-header-name + descriptor_key: my_header_name + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector({{{{"my_header_name", "test_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +// Validate that a descriptor is added if the missing request header +// has skip_if_absent set to true +TEST_F(RateLimitPolicyTest, RequestHeadersWithSkipIfAbsent) { + const std::string yaml = R"EOF( +actions: +- request_headers: + header_name: x-header-name + descriptor_key: my_header_name + skip_if_absent: false +- request_headers: + header_name: x-header + descriptor_key: my_header + skip_if_absent: true + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector({{{{"my_header_name", "test_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +// Tests if the descriptors are added if one of the headers is missing +// and skip_if_absent is set to default value which is false +TEST_F(RateLimitPolicyTest, RequestHeadersWithDefaultSkipIfAbsent) { + const std::string yaml = R"EOF( +actions: +- request_headers: + header_name: x-header-name + descriptor_key: my_header_name + skip_if_absent: false +- request_headers: + header_name: x-header + descriptor_key: my_header + skip_if_absent: false + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-test", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, RequestHeadersNoMatch) { + const std::string yaml = R"EOF( +actions: +- request_headers: + header_name: x-header + descriptor_key: my_header_name + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, RateLimitKey) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_value: fake_key + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"generic_key", "fake_key"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, GenericKeyWithSetDescriptorKey) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_key: fake_key + descriptor_value: fake_value + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, GenericKeyWithEmptyDescriptorKey) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_key: "" + descriptor_value: fake_value + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"generic_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, DEPRECATED_FEATURE_TEST(DynamicMetaDataMatch)) { + const std::string yaml = R"EOF( +actions: +- dynamic_metadata: + descriptor_key: fake_key + default_value: fake_value + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MetaDataMatchDynamicSourceByDefault) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + default_value: fake_value + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MetaDataMatchDynamicSource) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + default_value: fake_value + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + source: DYNAMIC + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MetaDataMatchRouteEntrySource) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + default_value: fake_value + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + source: ROUTE_ENTRY + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, route_->metadata_); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), + testing::ContainerEq(descriptors_)); +} + +// Tests that the default_value is used in the descriptor when the metadata_key is empty. +TEST_F(RateLimitPolicyTest, MetaDataNoMatchWithDefaultValue) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + default_value: fake_value + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + another_key: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MetaDataNoMatch) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + another_key: + prop: foo + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, MetaDataEmptyValue) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: "" + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +// Tests that no descriptors are generated when both the metadata_key and default_value are empty. +TEST_F(RateLimitPolicyTest, MetaDataAndDefaultValueEmpty) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_key: fake_key + descriptor_value: fake_value +- metadata: + descriptor_key: fake_key + default_value: "" + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + another_key: + prop: "" + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +// Tests that no descriptor is generated when both the metadata_key and default_value are empty, +// and skip_if_absent is set to true. +TEST_F(RateLimitPolicyTest, MetaDataAndDefaultValueEmptySkipIfAbsent) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_key: fake_key + descriptor_value: fake_value +- metadata: + descriptor_key: fake_key + default_value: "" + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + skip_if_absent: true + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + another_key: + prop: "" + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, MetaDataNonStringNoMatch) { + const std::string yaml = R"EOF( +actions: +- metadata: + descriptor_key: fake_key + metadata_key: + key: 'envoy.xxx' + path: + - key: test + - key: prop + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + envoy.xxx: + test: + prop: + foo: bar + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, HeaderValueMatch) { + const std::string yaml = R"EOF( +actions: +- header_value_match: + descriptor_value: fake_value + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"header_match", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, HeaderValueMatchDescriptorKey) { + const std::string yaml = R"EOF( +actions: +- header_value_match: + descriptor_key: fake_key + descriptor_value: fake_value + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, HeaderValueMatchNoMatch) { + const std::string yaml = R"EOF( +actions: +- header_value_match: + descriptor_value: fake_value + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "not_same_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, HeaderValueMatchHeadersNotPresent) { + const std::string yaml = R"EOF( +actions: +- header_value_match: + descriptor_value: fake_value + expect_match: false + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "not_same_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"header_match", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, HeaderValueMatchHeadersPresent) { + const std::string yaml = R"EOF( +actions: +- header_value_match: + descriptor_value: fake_value + expect_match: false + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, QueryParameterValueMatch) { + const std::string yaml = R"EOF( +actions: +- query_parameter_value_match: + descriptor_value: fake_value + query_parameters: + - name: x-parameter-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{":path", "/?x-parameter-name=test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"query_match", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, QueryParameterValueMatchDescriptorKey) { + const std::string yaml = R"EOF( +actions: +- query_parameter_value_match: + descriptor_key: fake_key + descriptor_value: fake_value + query_parameters: + - name: x-parameter-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{":path", "/?x-parameter-name=test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, QueryParameterValueMatchNoMatch) { + const std::string yaml = R"EOF( +actions: +- query_parameter_value_match: + descriptor_value: fake_value + query_parameters: + - name: x-parameter-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{":path", "/?x-parameter-name=not_same_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, QueryParameterValueMatchExpectNoMatch) { + const std::string yaml = R"EOF( +actions: +- query_parameter_value_match: + descriptor_value: fake_value + expect_match: false + query_parameters: + - name: x-parameter-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{":path", "/?x-parameter-name=not_same_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_THAT(std::vector({{{{"query_match", "fake_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, QueryParameterValueMatchExpectNoMatchFailed) { + const std::string yaml = R"EOF( +actions: +- query_parameter_value_match: + descriptor_value: fake_value + expect_match: false + query_parameters: + - name: x-parameter-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + Http::TestRequestHeaderMapImpl header{{":path", "/?x-parameter-name=test_value"}}; + + rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, CompoundActions) { + const std::string yaml = R"EOF( +actions: +- destination_cluster: {} +- source_cluster: {} + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector( + {{{{"destination_cluster", "fake_cluster"}, {"source_cluster", "service_cluster"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, CompoundActionsNoDescriptor) { + const std::string yaml = R"EOF( +actions: +- destination_cluster: {} +- header_value_match: + descriptor_value: fake_value + headers: + - name: x-header-name + string_match: + exact: test_value + )EOF"; + + setupTest(yaml); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideNotFound) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_value: limited_fake_key +limit: + dynamic_metadata: + metadata_key: + key: unknown.key + path: + - key: test + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + test.filter.key: + test: + requests_per_unit: 42 + unit: HOUR + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + EXPECT_THAT( + std::vector({{{{"generic_key", "limited_fake_key"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideWrongType) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_value: limited_fake_key +limit: + dynamic_metadata: + metadata_key: + key: test.filter.key + path: + - key: test + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + test.filter.key: + test: some_string + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + EXPECT_THAT( + std::vector({{{{"generic_key", "limited_fake_key"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideWrongUnit) { + const std::string yaml = R"EOF( +actions: +- generic_key: + descriptor_value: limited_fake_key +limit: + dynamic_metadata: + metadata_key: + key: test.filter.key + path: + - key: test + )EOF"; + + setupTest(yaml); + + std::string metadata_yaml = R"EOF( +filter_metadata: + test.filter.key: + test: + requests_per_unit: 42 + unit: NOT_A_UNIT + )EOF"; + + TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); + rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + EXPECT_THAT( + std::vector({{{{"generic_key", "limited_fake_key"}}}}), + testing::ContainerEq(descriptors_)); +} + +const std::string RequestHeaderMatchInputDescriptor = R"EOF( +actions: +- extension: + name: my_header_name + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: x-header-name + )EOF"; + +TEST_F(RateLimitPolicyTest, RequestMatchInput) { + setupTest(RequestHeaderMatchInputDescriptor); + Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_THAT( + std::vector({{{{"my_header_name", "test_value"}}}}), + testing::ContainerEq(descriptors_)); +} + +TEST_F(RateLimitPolicyTest, RequestMatchInputEmpty) { + setupTest(RequestHeaderMatchInputDescriptor); + Http::TestRequestHeaderMapImpl header{{"x-header-name", ""}}; + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_FALSE(descriptors_.empty()); +} + +TEST_F(RateLimitPolicyTest, RequestMatchInputSkip) { + setupTest(RequestHeaderMatchInputDescriptor); + + rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + + EXPECT_TRUE(descriptors_.empty()); +} + +} // namespace +} // namespace RateLimit +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto new file mode 100644 index 000000000000..7ab7fcbbfb7e --- /dev/null +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; + +package test.extensions.filters.common.ratelimit_config; + +import "envoy/extensions/common/ratelimit/v3/ratelimit.proto"; + + +message TestRateLimitConfig { + repeated envoy.extensions.common.ratelimit.v3.RateLimitPolicy rate_limits = 1; +} From cddefa73cd425ac9db23823f6c327d887a12be2b Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Thu, 19 Sep 2024 00:32:38 +0800 Subject: [PATCH 04/17] complete the implementation Signed-off-by: wangbaiping --- .../filters/common/ratelimit_config/BUILD | 7 +- .../ratelimit_config/ratelimit_config.cc | 63 ++++- .../filters/http/local_ratelimit/BUILD | 1 + .../ratelimit_config/ratelimit_config_test.cc | 226 +++++------------- .../filters/http/local_ratelimit/BUILD | 3 +- .../http/local_ratelimit/filter_test.cc | 86 ++++++- 6 files changed, 197 insertions(+), 189 deletions(-) diff --git a/source/extensions/filters/common/ratelimit_config/BUILD b/source/extensions/filters/common/ratelimit_config/BUILD index 718178d55f42..7f407b6a2ba5 100644 --- a/source/extensions/filters/common/ratelimit_config/BUILD +++ b/source/extensions/filters/common/ratelimit_config/BUILD @@ -12,12 +12,11 @@ envoy_cc_library( name = "ratelimit_config_lib", srcs = ["ratelimit_config.cc"], hdrs = ["ratelimit_config.h"], - external_deps = [ - "abseil_strings", - "abseil_inlined_vector", - ], deps = [ "//envoy/ratelimit:ratelimit_interface", + "//source/common/router:router_ratelimit_lib", + "@com_google_absl//absl/container:inlined_vector", + "@com_google_absl//absl/strings", "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index e363157ed262..b3ff2b2899ad 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -2,12 +2,58 @@ #include "ratelimit_config.h" #include "source/common/config/utility.h" +#include "source/common/config/utility.h" +#include "source/common/http/matching/data_impl.h" +#include "source/common/matcher/matcher.h" + namespace Envoy { namespace Extensions { namespace Filters { namespace Common { namespace RateLimit { +namespace { + +class MatchInputRateLimitDescriptor : public Envoy::RateLimit::DescriptorProducer { +public: + MatchInputRateLimitDescriptor(const std::string& descriptor_key, + Matcher::DataInputPtr&& data_input) + : descriptor_key_(descriptor_key), data_input_(std::move(data_input)) {} + + // Ratelimit::DescriptorProducer + bool populateDescriptor(Envoy::RateLimit::DescriptorEntry& entry, const std::string&, + const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info) const override { + Http::Matching::HttpMatchingDataImpl data(info); + data.onRequestHeaders(headers); + const auto result = data_input_->get(data); + if (result.data_availability_ != + Matcher::DataInputGetResult::DataAvailability::AllDataAvailable || + !absl::holds_alternative(result.data_)) { + return false; + } + if (absl::string_view value = absl::get(result.data_); !value.empty()) { + std::cout << value << std::endl; + entry = {descriptor_key_, std::string(value)}; + } + return true; + } + +private: + const std::string descriptor_key_; + Matcher::DataInputPtr data_input_; +}; + +class DataInputValidator : public Matcher::MatchTreeValidationVisitor { +public: + absl::Status performDataInputValidation(const Matcher::DataInputFactory&, + absl::string_view) override { + return absl::OkStatus(); + } +}; + +} // namespace + RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, Server::Configuration::CommonFactoryContext& context, absl::Status& creation_status) { @@ -41,9 +87,18 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, Envoy::Config::Utility::getFactory( action.extension()); if (!factory) { - creation_status = absl::InvalidArgumentError( - fmt::format("rate limit action: {} is not supported", action.extension().name())); - return; + // If no descriptor extension is found, fallback to using HTTP matcher + // input functions. Note that if the same extension name or type was + // dual registered as an extension descriptor and an HTTP matcher input + // function, the descriptor extension takes priority. + DataInputValidator validation_visitor; + Matcher::MatchInputFactory input_factory(validator, + validation_visitor); + Matcher::DataInputFactoryCb data_input_cb = + input_factory.createDataInput(action.extension()); + actions_.emplace_back(std::make_unique( + action.extension().name(), data_input_cb())); + break; } auto message = Envoy::Config::Utility::translateAnyToFactoryConfig( action.extension().typed_config(), validator, *factory); @@ -53,7 +108,7 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, actions_.emplace_back(std::move(producer)); } else { creation_status = absl::InvalidArgumentError( - absl::StrCat("rate limit descriptor extension failed: ", action.extension().name())); + absl::StrCat("Rate limit descriptor extension failed: ", action.extension().name())); return; } break; diff --git a/source/extensions/filters/http/local_ratelimit/BUILD b/source/extensions/filters/http/local_ratelimit/BUILD index 88d4042e9809..f20a2b04118e 100644 --- a/source/extensions/filters/http/local_ratelimit/BUILD +++ b/source/extensions/filters/http/local_ratelimit/BUILD @@ -27,6 +27,7 @@ envoy_cc_library( "//source/common/runtime:runtime_lib", "//source/extensions/filters/common/local_ratelimit:local_ratelimit_lib", "//source/extensions/filters/common/ratelimit:ratelimit_lib", + "//source/extensions/filters/common/ratelimit_config:ratelimit_config_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", "//source/extensions/filters/http/common:ratelimit_headers_lib", "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index b566f31a21b9..324085851a87 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -5,6 +5,8 @@ #include "envoy/config/route/v3/route.pb.h" #include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.validate.h" +#include "google/protobuf/struct.pb.h" #include "source/common/http/header_map_impl.h" #include "source/common/network/address_impl.h" #include "source/common/protobuf/utility.h" @@ -80,13 +82,15 @@ class RateLimitConfigTest : public testing::Test { config_ = std::make_unique( proto_config.rate_limits(), factory_context_, creation_status_); stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + ON_CALL(Const(stream_info_), route()).WillByDefault(testing::Return(route_)); } NiceMock factory_context_; ProtobufMessage::NullValidationVisitorImpl any_validation_visitor_; absl::Status creation_status_{}; std::unique_ptr config_; - Http::TestRequestHeaderMapImpl header_; + Http::TestRequestHeaderMapImpl headers_; + std::shared_ptr route_{new NiceMock()}; Network::Address::InstanceConstSharedPtr default_remote_address_{ new Network::Address::Ipv4Instance("10.0.0.1")}; NiceMock stream_info_; @@ -116,7 +120,7 @@ TEST_F(RateLimitConfigTest, SinglePolicy) { EXPECT_EQ(1U, config_->size()); std::vector descriptors; - config_->populateDescriptors(header_, stream_info_, "", descriptors); + config_->populateDescriptors(headers_, stream_info_, "", descriptors); EXPECT_THAT(std::vector({{{{"remote_address", "10.0.0.1"}}}}), testing::ContainerEq(descriptors)); } @@ -131,15 +135,16 @@ TEST_F(RateLimitConfigTest, MultiplePoliciesAndMultipleActions) { - destination_cluster: {} )EOF"; - factory_context_.cluster_manager_.initializeClusters({"www2test"}, {}); setupTest(yaml); std::vector descriptors; + config_->populateDescriptors(headers_, stream_info_, "", descriptors); + EXPECT_THAT(std::vector( {Envoy::RateLimit::LocalDescriptor{ - {{"remote_address", "10.0.0.1"}, {"destination_cluster", "www2test"}}}, - Envoy::RateLimit::LocalDescriptor{{{"destination_cluster", "www2test"}}}}), + {{"remote_address", "10.0.0.1"}, {"destination_cluster", "fake_cluster"}}}, + Envoy::RateLimit::LocalDescriptor{{{"destination_cluster", "fake_cluster"}}}}), testing::ContainerEq(descriptors)); } @@ -152,11 +157,12 @@ class RateLimitPolicyTest : public testing::Test { THROW_IF_NOT_OK(creation_status); // NOLINT descriptors_.clear(); stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + ON_CALL(Const(stream_info_), route()).WillByDefault(testing::Return(route_)); } NiceMock factory_context_; std::unique_ptr rate_limit_entry_; - Http::TestRequestHeaderMapImpl header_; + Http::TestRequestHeaderMapImpl headers_; std::shared_ptr route_{new NiceMock()}; std::vector descriptors_; @@ -174,11 +180,12 @@ class RateLimitPolicyIpv6Test : public testing::Test { THROW_IF_NOT_OK(creation_status); // NOLINT descriptors_.clear(); stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); + ON_CALL(Const(stream_info_), route()).WillByDefault(testing::Return(route_)); } NiceMock factory_context_; std::unique_ptr rate_limit_entry_; - Http::TestRequestHeaderMapImpl header_; + Http::TestRequestHeaderMapImpl headers_; std::vector descriptors_; std::shared_ptr route_{new NiceMock()}; @@ -195,7 +202,7 @@ TEST_F(RateLimitPolicyTest, RemoteAddress) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"remote_address", "10.0.0.1"}}}}), testing::ContainerEq(descriptors_)); @@ -209,7 +216,7 @@ TEST_F(RateLimitPolicyTest, MaskedRemoteAddressIpv4Default) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector( {{{{"masked_remote_address", "10.0.0.1/32"}}}}), @@ -225,7 +232,7 @@ TEST_F(RateLimitPolicyTest, MaskedRemoteAddressIpv4) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector( {{{{"masked_remote_address", "10.0.0.0/16"}}}}), @@ -240,7 +247,7 @@ TEST_F(RateLimitPolicyIpv6Test, MaskedRemoteAddressIpv6Default) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector( {{{{"masked_remote_address", "2001:abcd:ef01:2345:6789:abcd:ef01:234/128"}}}}), @@ -256,7 +263,7 @@ TEST_F(RateLimitPolicyIpv6Test, MaskedRemoteAddressIpv6) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector( {{{{"masked_remote_address", "2001:abcd:ef01:2345::/64"}}}}), @@ -274,7 +281,7 @@ TEST_F(RateLimitPolicyTest, PipeAddress) { stream_info_.downstream_connection_info_provider_->setRemoteAddress( *Network::Address::PipeInstance::create("/hello")); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -287,7 +294,7 @@ TEST_F(RateLimitPolicyTest, SourceService) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector({{{{"source_cluster", "service_cluster"}}}}), @@ -302,7 +309,7 @@ TEST_F(RateLimitPolicyTest, DestinationService) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector({{{{"destination_cluster", "fake_cluster"}}}}), @@ -318,9 +325,9 @@ TEST_F(RateLimitPolicyTest, RequestHeaders) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector({{{{"my_header_name", "test_value"}}}}), @@ -343,9 +350,9 @@ TEST_F(RateLimitPolicyTest, RequestHeadersWithSkipIfAbsent) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector({{{{"my_header_name", "test_value"}}}}), @@ -370,7 +377,7 @@ TEST_F(RateLimitPolicyTest, RequestHeadersWithDefaultSkipIfAbsent) { setupTest(yaml); Http::TestRequestHeaderMapImpl header{{"x-header-test", "test_value"}}; - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -384,9 +391,9 @@ TEST_F(RateLimitPolicyTest, RequestHeadersNoMatch) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -400,7 +407,7 @@ TEST_F(RateLimitPolicyTest, RateLimitKey) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"generic_key", "fake_key"}}}}), testing::ContainerEq(descriptors_)); @@ -416,7 +423,7 @@ TEST_F(RateLimitPolicyTest, GenericKeyWithSetDescriptorKey) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -432,41 +439,12 @@ TEST_F(RateLimitPolicyTest, GenericKeyWithEmptyDescriptorKey) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"generic_key", "fake_value"}}}}), testing::ContainerEq(descriptors_)); } -TEST_F(RateLimitPolicyTest, DEPRECATED_FEATURE_TEST(DynamicMetaDataMatch)) { - const std::string yaml = R"EOF( -actions: -- dynamic_metadata: - descriptor_key: fake_key - default_value: fake_value - metadata_key: - key: 'envoy.xxx' - path: - - key: test - - key: prop - )EOF"; - - setupTest(yaml); - - std::string metadata_yaml = R"EOF( -filter_metadata: - envoy.xxx: - test: - prop: foo - )EOF"; - - TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); - - EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), - testing::ContainerEq(descriptors_)); -} - TEST_F(RateLimitPolicyTest, MetaDataMatchDynamicSourceByDefault) { const std::string yaml = R"EOF( actions: @@ -490,7 +468,7 @@ TEST_F(RateLimitPolicyTest, MetaDataMatchDynamicSourceByDefault) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), testing::ContainerEq(descriptors_)); @@ -520,7 +498,7 @@ TEST_F(RateLimitPolicyTest, MetaDataMatchDynamicSource) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), testing::ContainerEq(descriptors_)); @@ -551,7 +529,7 @@ TEST_F(RateLimitPolicyTest, MetaDataMatchRouteEntrySource) { TestUtility::loadFromYaml(metadata_yaml, route_->metadata_); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "foo"}}}}), testing::ContainerEq(descriptors_)); @@ -581,7 +559,7 @@ TEST_F(RateLimitPolicyTest, MetaDataNoMatchWithDefaultValue) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -609,7 +587,7 @@ TEST_F(RateLimitPolicyTest, MetaDataNoMatch) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -636,7 +614,7 @@ TEST_F(RateLimitPolicyTest, MetaDataEmptyValue) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -668,7 +646,7 @@ TEST_F(RateLimitPolicyTest, MetaDataAndDefaultValueEmpty) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -702,7 +680,7 @@ TEST_F(RateLimitPolicyTest, MetaDataAndDefaultValueEmptySkipIfAbsent) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -731,7 +709,7 @@ TEST_F(RateLimitPolicyTest, MetaDataNonStringNoMatch) { )EOF"; TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -748,9 +726,9 @@ TEST_F(RateLimitPolicyTest, HeaderValueMatch) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"header_match", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -769,9 +747,9 @@ TEST_F(RateLimitPolicyTest, HeaderValueMatchDescriptorKey) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"fake_key", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -789,9 +767,9 @@ TEST_F(RateLimitPolicyTest, HeaderValueMatchNoMatch) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "not_same_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "not_same_value"); - rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -809,9 +787,9 @@ TEST_F(RateLimitPolicyTest, HeaderValueMatchHeadersNotPresent) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "not_same_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "not_same_value"); - rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_THAT(std::vector({{{{"header_match", "fake_value"}}}}), testing::ContainerEq(descriptors_)); @@ -830,9 +808,9 @@ TEST_F(RateLimitPolicyTest, HeaderValueMatchHeadersPresent) { )EOF"; setupTest(yaml); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header, stream_info_, "", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "", descriptors_); EXPECT_TRUE(descriptors_.empty()); } @@ -947,7 +925,7 @@ TEST_F(RateLimitPolicyTest, CompoundActions) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector( @@ -969,99 +947,11 @@ TEST_F(RateLimitPolicyTest, CompoundActionsNoDescriptor) { setupTest(yaml); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_TRUE(descriptors_.empty()); } -TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideNotFound) { - const std::string yaml = R"EOF( -actions: -- generic_key: - descriptor_value: limited_fake_key -limit: - dynamic_metadata: - metadata_key: - key: unknown.key - path: - - key: test - )EOF"; - - setupTest(yaml); - - std::string metadata_yaml = R"EOF( -filter_metadata: - test.filter.key: - test: - requests_per_unit: 42 - unit: HOUR - )EOF"; - - TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); - EXPECT_THAT( - std::vector({{{{"generic_key", "limited_fake_key"}}}}), - testing::ContainerEq(descriptors_)); -} - -TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideWrongType) { - const std::string yaml = R"EOF( -actions: -- generic_key: - descriptor_value: limited_fake_key -limit: - dynamic_metadata: - metadata_key: - key: test.filter.key - path: - - key: test - )EOF"; - - setupTest(yaml); - - std::string metadata_yaml = R"EOF( -filter_metadata: - test.filter.key: - test: some_string - )EOF"; - - TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); - EXPECT_THAT( - std::vector({{{{"generic_key", "limited_fake_key"}}}}), - testing::ContainerEq(descriptors_)); -} - -TEST_F(RateLimitPolicyTest, DynamicMetadataRateLimitOverrideWrongUnit) { - const std::string yaml = R"EOF( -actions: -- generic_key: - descriptor_value: limited_fake_key -limit: - dynamic_metadata: - metadata_key: - key: test.filter.key - path: - - key: test - )EOF"; - - setupTest(yaml); - - std::string metadata_yaml = R"EOF( -filter_metadata: - test.filter.key: - test: - requests_per_unit: 42 - unit: NOT_A_UNIT - )EOF"; - - TestUtility::loadFromYaml(metadata_yaml, stream_info_.dynamicMetadata()); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "", descriptors_); - EXPECT_THAT( - std::vector({{{{"generic_key", "limited_fake_key"}}}}), - testing::ContainerEq(descriptors_)); -} - const std::string RequestHeaderMatchInputDescriptor = R"EOF( actions: - extension: @@ -1073,9 +963,9 @@ const std::string RequestHeaderMatchInputDescriptor = R"EOF( TEST_F(RateLimitPolicyTest, RequestMatchInput) { setupTest(RequestHeaderMatchInputDescriptor); - Http::TestRequestHeaderMapImpl header{{"x-header-name", "test_value"}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_THAT( std::vector({{{{"my_header_name", "test_value"}}}}), @@ -1084,9 +974,9 @@ TEST_F(RateLimitPolicyTest, RequestMatchInput) { TEST_F(RateLimitPolicyTest, RequestMatchInputEmpty) { setupTest(RequestHeaderMatchInputDescriptor); - Http::TestRequestHeaderMapImpl header{{"x-header-name", ""}}; + headers_.setCopy(Http::LowerCaseString("x-header-name"), ""); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_FALSE(descriptors_.empty()); } @@ -1094,7 +984,7 @@ TEST_F(RateLimitPolicyTest, RequestMatchInputEmpty) { TEST_F(RateLimitPolicyTest, RequestMatchInputSkip) { setupTest(RequestHeaderMatchInputDescriptor); - rate_limit_entry_->populateDescriptors(header_, stream_info_, "service_cluster", descriptors_); + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); EXPECT_TRUE(descriptors_.empty()); } diff --git a/test/extensions/filters/http/local_ratelimit/BUILD b/test/extensions/filters/http/local_ratelimit/BUILD index a120d6875c55..aacdf5d3147f 100644 --- a/test/extensions/filters/http/local_ratelimit/BUILD +++ b/test/extensions/filters/http/local_ratelimit/BUILD @@ -21,8 +21,7 @@ envoy_extension_cc_test( "//source/extensions/filters/http/local_ratelimit:local_ratelimit_lib", "//test/common/stream_info:test_util", "//test/mocks/http:http_mocks", - "//test/mocks/local_info:local_info_mocks", - "//test/mocks/upstream:cluster_manager_mocks", + "//test/mocks/server:server_mocks", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/extensions/filters/http/local_ratelimit/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/local_ratelimit/filter_test.cc b/test/extensions/filters/http/local_ratelimit/filter_test.cc index 4495dcaf7a6a..2de6b7bf4a4a 100644 --- a/test/extensions/filters/http/local_ratelimit/filter_test.cc +++ b/test/extensions/filters/http/local_ratelimit/filter_test.cc @@ -3,9 +3,8 @@ #include "source/common/singleton/manager_impl.h" #include "source/extensions/filters/http/local_ratelimit/local_ratelimit.h" +#include "test/mocks/server/mocks.h" #include "test/mocks/http/mocks.h" -#include "test/mocks/local_info/mocks.h" -#include "test/mocks/upstream/cluster_manager.h" #include "test/test_common/test_runtime.h" #include "test/test_common/thread_factory_for_test.h" @@ -64,12 +63,12 @@ class FilterTest : public testing::Test { void setupPerRoute(const std::string& yaml, const bool enabled = true, const bool enforced = true, const bool per_route = false) { EXPECT_CALL( - runtime_.snapshot_, + factory_context_.runtime_loader_.snapshot_, featureEnabled(absl::string_view("test_enabled"), testing::Matcher(Percent(100)))) .WillRepeatedly(testing::Return(enabled)); EXPECT_CALL( - runtime_.snapshot_, + factory_context_.runtime_loader_.snapshot_, featureEnabled(absl::string_view("test_enforced"), testing::Matcher(Percent(100)))) .WillRepeatedly(testing::Return(enforced)); @@ -80,8 +79,7 @@ class FilterTest : public testing::Test { envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit config; TestUtility::loadFromYaml(yaml, config); config_ = - std::make_shared(config, local_info_, dispatcher_, cm_, singleton_manager_, - *stats_.rootScope(), runtime_, per_route); + std::make_shared(config, factory_context_, *stats_.rootScope(), per_route); filter_ = std::make_shared(config_); filter_->setDecoderFilterCallbacks(decoder_callbacks_); @@ -103,10 +101,8 @@ class FilterTest : public testing::Test { testing::NiceMock decoder_callbacks_; testing::NiceMock decoder_callbacks_2_; NiceMock dispatcher_; - NiceMock runtime_; - NiceMock local_info_; - NiceMock cm_; - Singleton::ManagerImpl singleton_manager_; + + NiceMock factory_context_; std::shared_ptr config_; std::shared_ptr filter_; @@ -115,7 +111,7 @@ class FilterTest : public testing::Test { TEST_F(FilterTest, Runtime) { setup(fmt::format(config_yaml, "false", "1", "false", "\"OFF\""), false, false); - EXPECT_EQ(&runtime_, &(config_->runtime())); + EXPECT_EQ(&factory_context_.runtime_loader_, &(config_->runtime())); } TEST_F(FilterTest, ToErrorCode) { @@ -400,6 +396,56 @@ enable_x_ratelimit_headers: {} stage: {} )"; +static constexpr absl::string_view inlined_descriptor_config_yaml = R"( +stat_prefix: test +token_bucket: + max_tokens: {} + tokens_per_fill: 1 + fill_interval: 60s +filter_enabled: + runtime_key: test_enabled + default_value: + numerator: 100 + denominator: HUNDRED +filter_enforced: + runtime_key: test_enforced + default_value: + numerator: 100 + denominator: HUNDRED +response_headers_to_add: + - append_action: OVERWRITE_IF_EXISTS_OR_ADD + header: + key: x-test-rate-limit + value: 'true' +enable_x_ratelimit_headers: {} +descriptors: +- entries: + - key: hello + value: world + - key: foo + value: bar + token_bucket: + max_tokens: 10 + tokens_per_fill: 10 + fill_interval: 60s +- entries: + - key: foo2 + value: bar2 + token_bucket: + max_tokens: {} + tokens_per_fill: 1 + fill_interval: 60s +rate_limits: +- actions: + - header_value_match: + descriptor_key: foo2 + descriptor_value: bar2 + headers: + - name: x-header-name + string_match: + exact: test_value + )"; + static constexpr absl::string_view consume_default_token_config_yaml = R"( stat_prefix: test token_bucket: @@ -938,6 +984,24 @@ TEST_F(DescriptorFilterTest, IncludeVirtualHostRateLimitsSetTrue) { EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); } +TEST_F(DescriptorFilterTest, UseInlinedRateLimitConfig) { + setUpTest(fmt::format(inlined_descriptor_config_yaml, "10", R"("OFF")", "1")); + + auto headers = Http::TestRequestHeaderMapImpl(); + // Requests will not be blocked because the requests don't match any descriptor and + // the global token bucket has enough tokens. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + + headers.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); + + // Only one request is allowd in 60s for the matched request. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); + EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::TooManyRequests, _, _, _, _)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers, false)); +} + } // namespace LocalRateLimitFilter } // namespace HttpFilters } // namespace Extensions From 5c01cd5e7fa008f06f671b2772c87ce4529de3b6 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sat, 21 Sep 2024 17:45:48 +0800 Subject: [PATCH 05/17] fix format Signed-off-by: wangbaiping --- source/common/router/router_ratelimit.cc | 2 +- source/common/router/router_ratelimit.h | 2 +- .../filters/common/ratelimit_config/ratelimit_config.cc | 3 +-- .../filters/common/ratelimit_config/ratelimit_config.h | 7 ++++--- .../filters/http/local_ratelimit/local_ratelimit.h | 2 -- test/extensions/filters/common/ratelimit_config/BUILD | 1 + .../common/ratelimit_config/ratelimit_config_test.cc | 7 +++---- .../common/ratelimit_config/ratelimit_config_test.proto | 3 +-- .../extensions/filters/http/local_ratelimit/filter_test.cc | 4 ++-- 9 files changed, 14 insertions(+), 17 deletions(-) diff --git a/source/common/router/router_ratelimit.cc b/source/common/router/router_ratelimit.cc index d5fc5376820e..974039066d23 100644 --- a/source/common/router/router_ratelimit.cc +++ b/source/common/router/router_ratelimit.cc @@ -7,8 +7,8 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" - #include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" + #include "source/common/common/assert.h" #include "source/common/common/empty_string.h" #include "source/common/config/metadata.h" diff --git a/source/common/router/router_ratelimit.h b/source/common/router/router_ratelimit.h index 3a87e8a204b3..a4d61574bed9 100644 --- a/source/common/router/router_ratelimit.h +++ b/source/common/router/router_ratelimit.h @@ -7,6 +7,7 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" +#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "envoy/ratelimit/ratelimit.h" #include "envoy/router/router.h" #include "envoy/router/router_ratelimit.h" @@ -16,7 +17,6 @@ #include "source/common/network/cidr_range.h" #include "source/common/protobuf/utility.h" #include "source/common/router/config_utility.h" -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "absl/types/optional.h" diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index b3ff2b2899ad..984c73adb569 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -1,6 +1,5 @@ -#include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" #include "ratelimit_config.h" -#include "source/common/config/utility.h" +#include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" #include "source/common/config/utility.h" #include "source/common/http/matching/data_impl.h" diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h index ee4b00b4759b..63ec2a238b07 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h @@ -1,11 +1,12 @@ #pragma once -#include "envoy/ratelimit/ratelimit.h" #include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" -#include "absl/container/inlined_vector.h" -#include "google/protobuf/repeated_ptr_field.h" +#include "envoy/ratelimit/ratelimit.h" + #include "source/common/router/router_ratelimit.h" +#include "absl/container/inlined_vector.h" + namespace Envoy { namespace Extensions { namespace Filters { diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h index 2c3125dd0000..3df0bd94e94d 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h @@ -113,8 +113,6 @@ class FilterConfig : public Router::RouteSpecificFilterConfig { return rate_limited_grpc_status_; } - bool hasRateLimitPerConnection() const { return rate_limit_per_connection_; } - bool hasRateLimitConfigs() const { ASSERT(rate_limit_config_ != nullptr); return !rate_limit_config_->empty(); diff --git a/test/extensions/filters/common/ratelimit_config/BUILD b/test/extensions/filters/common/ratelimit_config/BUILD index f0a0761af836..a116b9578a02 100644 --- a/test/extensions/filters/common/ratelimit_config/BUILD +++ b/test/extensions/filters/common/ratelimit_config/BUILD @@ -32,5 +32,6 @@ envoy_cc_test( "//test/mocks/server:instance_mocks", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index 324085851a87..e4b3baa8e501 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -3,23 +3,22 @@ #include #include "envoy/config/route/v3/route.pb.h" - #include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.validate.h" -#include "google/protobuf/struct.pb.h" + #include "source/common/http/header_map_impl.h" #include "source/common/network/address_impl.h" #include "source/common/protobuf/utility.h" #include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" +#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.h" +#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.validate.h" #include "test/mocks/http/mocks.h" #include "test/mocks/ratelimit/mocks.h" #include "test/mocks/router/mocks.h" #include "test/mocks/server/instance.h" #include "test/test_common/printers.h" #include "test/test_common/utility.h" -#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.h" -#include "test/extensions/filters/common/ratelimit_config/ratelimit_config_test.pb.validate.h" #include "gmock/gmock.h" #include "gtest/gtest.h" diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto index 7ab7fcbbfb7e..caaa2d3ecdb6 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto @@ -4,7 +4,6 @@ package test.extensions.filters.common.ratelimit_config; import "envoy/extensions/common/ratelimit/v3/ratelimit.proto"; - message TestRateLimitConfig { - repeated envoy.extensions.common.ratelimit.v3.RateLimitPolicy rate_limits = 1; + repeated envoy.extensions.common.ratelimit.v3.RateLimitPolicy rate_limits = 1; } diff --git a/test/extensions/filters/http/local_ratelimit/filter_test.cc b/test/extensions/filters/http/local_ratelimit/filter_test.cc index 2de6b7bf4a4a..cfecb31560b0 100644 --- a/test/extensions/filters/http/local_ratelimit/filter_test.cc +++ b/test/extensions/filters/http/local_ratelimit/filter_test.cc @@ -3,8 +3,8 @@ #include "source/common/singleton/manager_impl.h" #include "source/extensions/filters/http/local_ratelimit/local_ratelimit.h" -#include "test/mocks/server/mocks.h" #include "test/mocks/http/mocks.h" +#include "test/mocks/server/mocks.h" #include "test/test_common/test_runtime.h" #include "test/test_common/thread_factory_for_test.h" @@ -996,7 +996,7 @@ TEST_F(DescriptorFilterTest, UseInlinedRateLimitConfig) { headers.setCopy(Http::LowerCaseString("x-header-name"), "test_value"); - // Only one request is allowd in 60s for the matched request. + // Only one request is allowed in 60s for the matched request. EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(headers, false)); EXPECT_CALL(decoder_callbacks_, sendLocalReply(Http::Code::TooManyRequests, _, _, _, _)); EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_->decodeHeaders(headers, false)); From 26efa0944efedc98d7ac7c2ef98460966dc459be Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sat, 21 Sep 2024 18:03:09 +0800 Subject: [PATCH 06/17] add change log Signed-off-by: wangbaiping --- changelogs/current.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index bfc2be2fc518..2705d56e247d 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -353,5 +353,13 @@ new_features: change: | Added two new methods ``oidsPeerCertificate()`` and ``oidsLocalCertificate()`` to SSL connection object API :ref:`SSL connection info object `. +- area: local_ratelimit + change: | + Add the :ref:`rate_limits + ` + field to generate rate limit descriptors. If this field is set, the + :ref:`VirtualHost.rate_limits` or + :ref:`RouteAction.rate_limits` fields + will be ignored. deprecated: From 36381316fd22deb257c10b1d682a4b34b0e1e9f2 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sat, 21 Sep 2024 22:27:02 +0800 Subject: [PATCH 07/17] fix docs Signed-off-by: wangbaiping --- .../filters/http/local_ratelimit/v3/local_rate_limit.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index 474f773ac46b..0a22b1b207d6 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -155,6 +155,5 @@ message LocalRateLimit { // :ref:`VirtualHost.rate_limits` or // :ref:`RouteAction.rate_limits` fields // will be ignored. - // [#not-implemented-hide:] repeated common.ratelimit.v3.RateLimitPolicy rate_limits = 17; } From a57356049bc09af626a8af357c444ea50de1bb83 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sun, 22 Sep 2024 16:36:38 +0800 Subject: [PATCH 08/17] fix docs Signed-off-by: wangbaiping --- api/envoy/extensions/common/ratelimit/v3/ratelimit.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index 09a32a752d01..c7d487c0da50 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -154,7 +154,6 @@ message LocalClusterRateLimit { // Take the local rate limit extension as an example, the generated entries will be used to find a // ``LocalRateLimitDescriptor`` which contains a token bucket. And the token bucket will be used to // limit the request rate. -// [#not-implemented-hide:] message RateLimitPolicy { // [#next-free-field: 11] message Action { From 21a569cba41e7536e5da01888c608b325fa36396 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sun, 22 Sep 2024 20:08:13 +0800 Subject: [PATCH 09/17] remove new rate limit policy and use the previous route ratelimit Signed-off-by: wangbaiping --- .../extensions/common/ratelimit/v3/BUILD | 3 - .../common/ratelimit/v3/ratelimit.proto | 249 ------------------ .../filters/http/local_ratelimit/v3/BUILD | 1 + .../local_ratelimit/v3/local_rate_limit.proto | 13 +- source/common/router/BUILD | 1 - source/common/router/router_ratelimit.cc | 84 ++---- source/common/router/router_ratelimit.h | 58 ++-- .../filters/common/ratelimit_config/BUILD | 2 +- .../ratelimit_config/ratelimit_config.cc | 52 +--- .../ratelimit_config/ratelimit_config.h | 4 +- .../filters/common/ratelimit_config/BUILD | 3 +- .../ratelimit_config/ratelimit_config_test.cc | 9 +- .../ratelimit_config_test.proto | 4 +- 13 files changed, 72 insertions(+), 411 deletions(-) diff --git a/api/envoy/extensions/common/ratelimit/v3/BUILD b/api/envoy/extensions/common/ratelimit/v3/BUILD index a8033f668ebb..ef19132f9180 100644 --- a/api/envoy/extensions/common/ratelimit/v3/BUILD +++ b/api/envoy/extensions/common/ratelimit/v3/BUILD @@ -6,9 +6,6 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ - "//envoy/config/core/v3:pkg", - "//envoy/config/route/v3:pkg", - "//envoy/type/metadata/v3:pkg", "//envoy/type/v3:pkg", "@com_github_cncf_xds//udpa/annotations:pkg", ], diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index c7d487c0da50..37270c378ded 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -2,9 +2,6 @@ syntax = "proto3"; package envoy.extensions.common.ratelimit.v3; -import "envoy/config/core/v3/extension.proto"; -import "envoy/config/route/v3/route_components.proto"; -import "envoy/type/metadata/v3/metadata.proto"; import "envoy/type/v3/ratelimit_unit.proto"; import "envoy/type/v3/token_bucket.proto"; @@ -147,249 +144,3 @@ message LocalRateLimitDescriptor { // about local cluster. message LocalClusterRateLimit { } - -// Rate limit config that used to generate a list of descriptor entries (a list of key/value pairs) -// based on the request context. The generated entries will be used to much a specific rate limit -// rule. -// Take the local rate limit extension as an example, the generated entries will be used to find a -// ``LocalRateLimitDescriptor`` which contains a token bucket. And the token bucket will be used to -// limit the request rate. -message RateLimitPolicy { - // [#next-free-field: 11] - message Action { - // The following descriptor entry is appended to the descriptor: - // - // .. code-block:: cpp - // - // ("source_cluster", "") - // - // is derived from the :option:`--service-cluster` option. - message SourceCluster { - } - - // The following descriptor entry is appended to the descriptor: - // - // .. code-block:: cpp - // - // ("destination_cluster", "") - // - // Once a request matches against a route table rule, a routed cluster is determined by one of - // the following :ref:`route table configuration ` - // settings: - // - // * :ref:`cluster ` indicates the upstream cluster - // to route to. - // * :ref:`weighted_clusters ` - // chooses a cluster randomly from a set of clusters with attributed weight. - // * :ref:`cluster_header ` indicates which - // header in the request contains the target cluster. - message DestinationCluster { - } - - // The following descriptor entry is appended when a header contains a key that matches the - // ``header_name``: - // - // .. code-block:: cpp - // - // ("", "") - message RequestHeaders { - // The header name to be queried from the request headers. The header’s - // value is used to populate the value of the descriptor entry for the - // descriptor_key. - string header_name = 1 - [(validate.rules).string = {min_len: 1 well_known_regex: HTTP_HEADER_NAME strict: false}]; - - // The key to use in the descriptor entry. - string descriptor_key = 2 [(validate.rules).string = {min_len: 1}]; - - // If set to true, Envoy skips the descriptor while calling rate limiting service - // when header is not present in the request. By default it skips calling the - // rate limiting service if this header is not present in the request. - bool skip_if_absent = 3; - } - - // The following descriptor entry is appended to the descriptor and is populated using the - // trusted address from :ref:`x-forwarded-for `: - // - // .. code-block:: cpp - // - // ("remote_address", "") - message RemoteAddress { - } - - // The following descriptor entry is appended to the descriptor and is populated using the - // masked address from :ref:`x-forwarded-for `: - // - // .. code-block:: cpp - // - // ("masked_remote_address", "") - message MaskedRemoteAddress { - // Length of prefix mask len for IPv4 (e.g. 0, 32). - // Defaults to 32 when unset. - // For example, trusted address from x-forwarded-for is ``192.168.1.1``, - // the descriptor entry is ("masked_remote_address", "192.168.1.1/32"); - // if mask len is 24, the descriptor entry is ("masked_remote_address", "192.168.1.0/24"). - google.protobuf.UInt32Value v4_prefix_mask_len = 1 [(validate.rules).uint32 = {lte: 32}]; - - // Length of prefix mask len for IPv6 (e.g. 0, 128). - // Defaults to 128 when unset. - // For example, trusted address from x-forwarded-for is ``2001:abcd:ef01:2345:6789:abcd:ef01:234``, - // the descriptor entry is ("masked_remote_address", "2001:abcd:ef01:2345:6789:abcd:ef01:234/128"); - // if mask len is 64, the descriptor entry is ("masked_remote_address", "2001:abcd:ef01:2345::/64"). - google.protobuf.UInt32Value v6_prefix_mask_len = 2 [(validate.rules).uint32 = {lte: 128}]; - } - - // The following descriptor entry is appended to the descriptor: - // - // .. code-block:: cpp - // - // ("generic_key", "") - message GenericKey { - // The value to use in the descriptor entry. - string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; - - // An optional key to use in the descriptor entry. If not set it defaults - // to 'generic_key' as the descriptor key. - string descriptor_key = 2; - } - - // The following descriptor entry is appended to the descriptor: - // - // .. code-block:: cpp - // - // ("header_match", "") - message HeaderValueMatch { - // The key to use in the descriptor entry. Defaults to ``header_match``. - string descriptor_key = 4; - - // The value to use in the descriptor entry. - string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; - - // If set to true, the action will append a descriptor entry when the - // request matches the headers. If set to false, the action will append a - // descriptor entry when the request does not match the headers. The - // default value is true. - google.protobuf.BoolValue expect_match = 2; - - // Specifies a set of headers that the rate limit action should match - // on. The action will check the request’s headers against all the - // specified headers in the config. A match will happen if all the - // headers in the config are present in the request with the same values - // (or based on presence if the value field is not in the config). - repeated config.route.v3.HeaderMatcher headers = 3 - [(validate.rules).repeated = {min_items: 1}]; - } - - // The following descriptor entry is appended when the metadata contains a key value: - // - // .. code-block:: cpp - // - // ("", "") - // [#next-free-field: 6] - message MetaData { - enum Source { - // Query :ref:`dynamic metadata ` - DYNAMIC = 0; - - // Query :ref:`route entry metadata ` - ROUTE_ENTRY = 1; - } - - // The key to use in the descriptor entry. - string descriptor_key = 1 [(validate.rules).string = {min_len: 1}]; - - // Metadata struct that defines the key and path to retrieve the string value. A match will - // only happen if the value in the metadata is of type string. - type.metadata.v3.MetadataKey metadata_key = 2 [(validate.rules).message = {required: true}]; - - // An optional value to use if ``metadata_key`` is empty. If not set and - // no value is present under the metadata_key then ``skip_if_absent`` is followed to - // skip calling the rate limiting service or skip the descriptor. - string default_value = 3; - - // Source of metadata - Source source = 4 [(validate.rules).enum = {defined_only: true}]; - - // If set to true, Envoy skips the descriptor while calling rate limiting service - // when ``metadata_key`` is empty and ``default_value`` is not set. By default it skips calling the - // rate limiting service in that case. - bool skip_if_absent = 5; - } - - // The following descriptor entry is appended to the descriptor: - // - // .. code-block:: cpp - // - // ("query_match", "") - message QueryParameterValueMatch { - // The key to use in the descriptor entry. Defaults to ``query_match``. - string descriptor_key = 4; - - // The value to use in the descriptor entry. - string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; - - // If set to true, the action will append a descriptor entry when the - // request matches the headers. If set to false, the action will append a - // descriptor entry when the request does not match the headers. The - // default value is true. - google.protobuf.BoolValue expect_match = 2; - - // Specifies a set of query parameters that the rate limit action should match - // on. The action will check the request’s query parameters against all the - // specified query parameters in the config. A match will happen if all the - // query parameters in the config are present in the request with the same values - // (or based on presence if the value field is not in the config). - repeated config.route.v3.QueryParameterMatcher query_parameters = 3 - [(validate.rules).repeated = {min_items: 1}]; - } - - oneof action_specifier { - option (validate.required) = true; - - // Rate limit on source cluster. - SourceCluster source_cluster = 1; - - // Rate limit on destination cluster. - DestinationCluster destination_cluster = 2; - - // Rate limit on request headers. - RequestHeaders request_headers = 3; - - // Rate limit on remote address. - RemoteAddress remote_address = 4; - - // Rate limit on a generic key. - GenericKey generic_key = 5; - - // Rate limit on the existence of request headers. - HeaderValueMatch header_value_match = 6; - - // Rate limit on metadata. - MetaData metadata = 7; - - // Rate limit descriptor extension. See the rate limit descriptor extensions documentation. - // - // :ref:`HTTP matching input functions ` are - // permitted as descriptor extensions. The input functions are only - // looked up if there is no rate limit descriptor extension matching - // the type URL. - // - // [#extension-category: envoy.rate_limit_descriptors] - config.core.v3.TypedExtensionConfig extension = 8; - - // Rate limit on masked remote address. - MaskedRemoteAddress masked_remote_address = 9; - - // Rate limit on the existence of query parameters. - QueryParameterValueMatch query_parameter_value_match = 10; - } - } - - // A list of actions that are to be applied for this rate limit configuration. - // Order matters as the actions are processed sequentially and the descriptor - // is composed by appending descriptor entries in that sequence. If an action - // cannot append a descriptor entry, no descriptor is generated for the - // configuration. See :ref:`composing actions - // ` for additional documentation. - repeated Action actions = 1 [(validate.rules).repeated = {min_items: 1}]; -} diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/BUILD b/api/envoy/extensions/filters/http/local_ratelimit/v3/BUILD index 1ef2f0c9bf47..ac9fd7c8abe8 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/BUILD +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/BUILD @@ -7,6 +7,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/config/core/v3:pkg", + "//envoy/config/route/v3:pkg", "//envoy/extensions/common/ratelimit/v3:pkg", "//envoy/type/v3:pkg", "@com_github_cncf_xds//udpa/annotations:pkg", diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index 0a22b1b207d6..2f70155ee33b 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.extensions.filters.http.local_ratelimit.v3; import "envoy/config/core/v3/base.proto"; +import "envoy/config/route/v3/route_components.proto"; import "envoy/extensions/common/ratelimit/v3/ratelimit.proto"; import "envoy/type/v3/http_status.proto"; import "envoy/type/v3/token_bucket.proto"; @@ -155,5 +156,15 @@ message LocalRateLimit { // :ref:`VirtualHost.rate_limits` or // :ref:`RouteAction.rate_limits` fields // will be ignored. - repeated common.ratelimit.v3.RateLimitPolicy rate_limits = 17; + // + // .. note:: + // Not all configuration fields of + // :ref:`rate limit config ` is supported at here. + // Following fields are not supported: + // + // 1. :ref:`rate limit stage `. + // 2. :ref:`dynamic metadata `. + // 3. :ref:`disable_key `. + // 4. :ref:`override limit `. + repeated config.route.v3.RateLimit rate_limits = 17; } diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 23da53f05940..6f86c998503e 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -431,7 +431,6 @@ envoy_cc_library( "//source/common/protobuf:utility_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", ], ) diff --git a/source/common/router/router_ratelimit.cc b/source/common/router/router_ratelimit.cc index 974039066d23..77c285a85a24 100644 --- a/source/common/router/router_ratelimit.cc +++ b/source/common/router/router_ratelimit.cc @@ -7,14 +7,11 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "source/common/common/assert.h" #include "source/common/common/empty_string.h" #include "source/common/config/metadata.h" #include "source/common/config/utility.h" -#include "source/common/http/matching/data_impl.h" -#include "source/common/matcher/matcher.h" #include "source/common/protobuf/utility.h" namespace Envoy { @@ -40,44 +37,24 @@ bool populateDescriptor(const std::vector& act return result; } -class RateLimitDescriptorValidationVisitor - : public Matcher::MatchTreeValidationVisitor { -public: - absl::Status performDataInputValidation(const Matcher::DataInputFactory&, - absl::string_view) override { - return absl::OkStatus(); +} // namespace + +// Ratelimit::DescriptorProducer +bool MatchInputRateLimitDescriptor::populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, + const std::string&, + const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info) const { + Http::Matching::HttpMatchingDataImpl data(info); + data.onRequestHeaders(headers); + auto result = data_input_->get(data); + if (!absl::holds_alternative(result.data_)) { + return false; } -}; - -class MatchInputRateLimitDescriptor : public RateLimit::DescriptorProducer { -public: - MatchInputRateLimitDescriptor(const std::string& descriptor_key, - Matcher::DataInputPtr&& data_input) - : descriptor_key_(descriptor_key), data_input_(std::move(data_input)) {} - - // Ratelimit::DescriptorProducer - bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string&, - const Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& info) const override { - Http::Matching::HttpMatchingDataImpl data(info); - data.onRequestHeaders(headers); - auto result = data_input_->get(data); - if (absl::holds_alternative(result.data_)) { - return false; - } - const std::string& str = absl::get(result.data_); - if (!str.empty()) { - descriptor_entry = {descriptor_key_, str}; - } - return true; + if (absl::string_view str = absl::get(result.data_); !str.empty()) { + descriptor_entry = {descriptor_key_, std::string(str)}; } - -private: - const std::string descriptor_key_; - Matcher::DataInputPtr data_input_; -}; - -} // namespace + return true; +} const uint64_t RateLimitPolicyImpl::MAX_STAGE_NUMBER = 10UL; @@ -192,16 +169,6 @@ MetaDataAction::MetaDataAction(const envoy::config::route::v3::RateLimit::Action default_value_(action.default_value()), source_(action.source()), skip_if_absent_(action.skip_if_absent()) {} -MetaDataAction::MetaDataAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MetaData& action) - : metadata_key_(action.metadata_key()), descriptor_key_(action.descriptor_key()), - default_value_(action.default_value()), - source_(action.source() == envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action:: - MetaData::ROUTE_ENTRY - ? envoy::config::route::v3::RateLimit::Action::MetaData::ROUTE_ENTRY - : envoy::config::route::v3::RateLimit::Action::MetaData::DYNAMIC), - skip_if_absent_(action.skip_if_absent()) {} - MetaDataAction::MetaDataAction( const envoy::config::route::v3::RateLimit::Action::DynamicMetaData& action) : metadata_key_(action.metadata_key()), descriptor_key_(action.descriptor_key()), @@ -249,15 +216,6 @@ HeaderValueMatchAction::HeaderValueMatchAction( expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers(), context)) {} -HeaderValueMatchAction::HeaderValueMatchAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::HeaderValueMatch& - action, - Server::Configuration::CommonFactoryContext& context) - : descriptor_value_(action.descriptor_value()), - descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "header_match"), - expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), - action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers(), context)) {} - bool HeaderValueMatchAction::populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string&, const Http::RequestHeaderMap& headers, @@ -279,16 +237,6 @@ QueryParameterValueMatchAction::QueryParameterValueMatchAction( action_query_parameters_( buildQueryParameterMatcherVector(action.query_parameters(), context)) {} -QueryParameterValueMatchAction::QueryParameterValueMatchAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action:: - QueryParameterValueMatch& action, - Server::Configuration::CommonFactoryContext& context) - : descriptor_value_(action.descriptor_value()), - descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "query_match"), - expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), - action_query_parameters_( - buildQueryParameterMatcherVector(action.query_parameters(), context)) {} - bool QueryParameterValueMatchAction::populateDescriptor( RateLimit::DescriptorEntry& descriptor_entry, const std::string&, const Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo&) const { diff --git a/source/common/router/router_ratelimit.h b/source/common/router/router_ratelimit.h index a4d61574bed9..3fb5149a4cc2 100644 --- a/source/common/router/router_ratelimit.h +++ b/source/common/router/router_ratelimit.h @@ -7,13 +7,14 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/route/v3/route_components.pb.h" -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" #include "envoy/ratelimit/ratelimit.h" #include "envoy/router/router.h" #include "envoy/router/router_ratelimit.h" #include "source/common/config/metadata.h" #include "source/common/http/header_utility.h" +#include "source/common/http/matching/data_impl.h" +#include "source/common/matcher/matcher.h" #include "source/common/network/cidr_range.h" #include "source/common/protobuf/utility.h" #include "source/common/router/config_utility.h" @@ -73,12 +74,6 @@ class RequestHeadersAction : public RateLimit::DescriptorProducer { : header_name_(action.header_name()), descriptor_key_(action.descriptor_key()), skip_if_absent_(action.skip_if_absent()) {} - RequestHeadersAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::RequestHeaders& - action) - : header_name_(action.header_name()), descriptor_key_(action.descriptor_key()), - skip_if_absent_(action.skip_if_absent()) {} - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -113,12 +108,6 @@ class MaskedRemoteAddressAction : public RateLimit::DescriptorProducer { : v4_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v4_prefix_mask_len, 32)), v6_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v6_prefix_mask_len, 128)) {} - MaskedRemoteAddressAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MaskedRemoteAddress& - action) - : v4_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v4_prefix_mask_len, 32)), - v6_prefix_mask_len_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, v6_prefix_mask_len, 128)) {} - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -140,12 +129,6 @@ class GenericKeyAction : public RateLimit::DescriptorProducer { descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() : "generic_key") {} - GenericKeyAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::GenericKey& action) - : descriptor_value_(action.descriptor_value()), - descriptor_key_(!action.descriptor_key().empty() ? action.descriptor_key() - : "generic_key") {} - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -166,9 +149,6 @@ class MetaDataAction : public RateLimit::DescriptorProducer { // for maintaining backward compatibility with the deprecated DynamicMetaData action MetaDataAction(const envoy::config::route::v3::RateLimit::Action::DynamicMetaData& action); - MetaDataAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::MetaData& action); - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -192,11 +172,6 @@ class HeaderValueMatchAction : public RateLimit::DescriptorProducer { const envoy::config::route::v3::RateLimit::Action::HeaderValueMatch& action, Server::Configuration::CommonFactoryContext& context); - HeaderValueMatchAction( - const envoy::extensions::common::ratelimit::v3::RateLimitPolicy::Action::HeaderValueMatch& - action, - Server::Configuration::CommonFactoryContext& context); - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -219,10 +194,6 @@ class QueryParameterValueMatchAction : public RateLimit::DescriptorProducer { const envoy::config::route::v3::RateLimit::Action::QueryParameterValueMatch& action, Server::Configuration::CommonFactoryContext& context); - QueryParameterValueMatchAction(const envoy::extensions::common::ratelimit::v3::RateLimitPolicy:: - Action::QueryParameterValueMatch& action, - Server::Configuration::CommonFactoryContext& context); - // Ratelimit::DescriptorProducer bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string& local_service_cluster, @@ -241,6 +212,31 @@ class QueryParameterValueMatchAction : public RateLimit::DescriptorProducer { const std::vector action_query_parameters_; }; +class RateLimitDescriptorValidationVisitor + : public Matcher::MatchTreeValidationVisitor { +public: + absl::Status performDataInputValidation(const Matcher::DataInputFactory&, + absl::string_view) override { + return absl::OkStatus(); + } +}; + +class MatchInputRateLimitDescriptor : public RateLimit::DescriptorProducer { +public: + MatchInputRateLimitDescriptor(const std::string& descriptor_key, + Matcher::DataInputPtr&& data_input) + : descriptor_key_(descriptor_key), data_input_(std::move(data_input)) {} + + // Ratelimit::DescriptorProducer + bool populateDescriptor(RateLimit::DescriptorEntry& descriptor_entry, const std::string&, + const Http::RequestHeaderMap& headers, + const StreamInfo::StreamInfo& info) const override; + +private: + const std::string descriptor_key_; + Matcher::DataInputPtr data_input_; +}; + /* * Implementation of RateLimitPolicyEntry that holds the action for the configuration. */ diff --git a/source/extensions/filters/common/ratelimit_config/BUILD b/source/extensions/filters/common/ratelimit_config/BUILD index 7f407b6a2ba5..a89b1d3676af 100644 --- a/source/extensions/filters/common/ratelimit_config/BUILD +++ b/source/extensions/filters/common/ratelimit_config/BUILD @@ -17,6 +17,6 @@ envoy_cc_library( "//source/common/router:router_ratelimit_lib", "@com_google_absl//absl/container:inlined_vector", "@com_google_absl//absl/strings", - "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", + "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index 984c73adb569..42a572fd9211 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -1,4 +1,3 @@ -#include "ratelimit_config.h" #include "source/extensions/filters/common/ratelimit_config/ratelimit_config.h" #include "source/common/config/utility.h" @@ -11,48 +10,6 @@ namespace Filters { namespace Common { namespace RateLimit { -namespace { - -class MatchInputRateLimitDescriptor : public Envoy::RateLimit::DescriptorProducer { -public: - MatchInputRateLimitDescriptor(const std::string& descriptor_key, - Matcher::DataInputPtr&& data_input) - : descriptor_key_(descriptor_key), data_input_(std::move(data_input)) {} - - // Ratelimit::DescriptorProducer - bool populateDescriptor(Envoy::RateLimit::DescriptorEntry& entry, const std::string&, - const Http::RequestHeaderMap& headers, - const StreamInfo::StreamInfo& info) const override { - Http::Matching::HttpMatchingDataImpl data(info); - data.onRequestHeaders(headers); - const auto result = data_input_->get(data); - if (result.data_availability_ != - Matcher::DataInputGetResult::DataAvailability::AllDataAvailable || - !absl::holds_alternative(result.data_)) { - return false; - } - if (absl::string_view value = absl::get(result.data_); !value.empty()) { - std::cout << value << std::endl; - entry = {descriptor_key_, std::string(value)}; - } - return true; - } - -private: - const std::string descriptor_key_; - Matcher::DataInputPtr data_input_; -}; - -class DataInputValidator : public Matcher::MatchTreeValidationVisitor { -public: - absl::Status performDataInputValidation(const Matcher::DataInputFactory&, - absl::string_view) override { - return absl::OkStatus(); - } -}; - -} // namespace - RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, Server::Configuration::CommonFactoryContext& context, absl::Status& creation_status) { @@ -90,12 +47,12 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, // input functions. Note that if the same extension name or type was // dual registered as an extension descriptor and an HTTP matcher input // function, the descriptor extension takes priority. - DataInputValidator validation_visitor; + Router::RateLimitDescriptorValidationVisitor validation_visitor; Matcher::MatchInputFactory input_factory(validator, validation_visitor); Matcher::DataInputFactoryCb data_input_cb = input_factory.createDataInput(action.extension()); - actions_.emplace_back(std::make_unique( + actions_.emplace_back(std::make_unique( action.extension().name(), data_input_cb())); break; } @@ -119,8 +76,11 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, actions_.emplace_back(new Router::QueryParameterValueMatchAction( action.query_parameter_value_match(), context)); break; + case ProtoRateLimit::Action::ActionSpecifierCase::kDynamicMetadata: case ProtoRateLimit::Action::ActionSpecifierCase::ACTION_SPECIFIER_NOT_SET: - PANIC_DUE_TO_CORRUPT_ENUM; + creation_status = absl::InvalidArgumentError(fmt::format( + "Unsupported rate limit action: {}", static_cast(action.action_specifier_case()))); + return; } } } diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h index 63ec2a238b07..e044edb216f5 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h @@ -1,6 +1,6 @@ #pragma once -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" +#include "envoy/config/route/v3/route_components.pb.h" #include "envoy/ratelimit/ratelimit.h" #include "source/common/router/router_ratelimit.h" @@ -13,7 +13,7 @@ namespace Filters { namespace Common { namespace RateLimit { -using ProtoRateLimit = envoy::extensions::common::ratelimit::v3::RateLimitPolicy; +using ProtoRateLimit = envoy::config::route::v3::RateLimit; using RateLimitDescriptors = std::vector; class RateLimitPolicy { diff --git a/test/extensions/filters/common/ratelimit_config/BUILD b/test/extensions/filters/common/ratelimit_config/BUILD index a116b9578a02..2f2c365fc7f8 100644 --- a/test/extensions/filters/common/ratelimit_config/BUILD +++ b/test/extensions/filters/common/ratelimit_config/BUILD @@ -13,7 +13,7 @@ envoy_proto_library( name = "ratelimit_config_test_proto", srcs = ["ratelimit_config_test.proto"], deps = [ - "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg", + "@envoy_api//envoy/config/route/v3:pkg", ], ) @@ -32,6 +32,5 @@ envoy_cc_test( "//test/mocks/server:instance_mocks", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/common/ratelimit/v3:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index e4b3baa8e501..41fdb0f17af1 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -3,8 +3,8 @@ #include #include "envoy/config/route/v3/route.pb.h" -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.h" -#include "envoy/extensions/common/ratelimit/v3/ratelimit.pb.validate.h" +#include "envoy/config/route/v3/route_components.pb.h" +#include "envoy/config/route/v3/route_components.pb.validate.h" #include "source/common/http/header_map_impl.h" #include "source/common/network/address_impl.h" @@ -32,9 +32,8 @@ namespace Common { namespace RateLimit { namespace { -envoy::extensions::common::ratelimit::v3::RateLimitPolicy -parseRateLimitFromV3Yaml(const std::string& yaml_string) { - envoy::extensions::common::ratelimit::v3::RateLimitPolicy rate_limit; +ProtoRateLimit parseRateLimitFromV3Yaml(const std::string& yaml_string) { + ProtoRateLimit rate_limit; TestUtility::loadFromYaml(yaml_string, rate_limit); TestUtility::validate(rate_limit); return rate_limit; diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto index caaa2d3ecdb6..08d48af9cb86 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.proto @@ -2,8 +2,8 @@ syntax = "proto3"; package test.extensions.filters.common.ratelimit_config; -import "envoy/extensions/common/ratelimit/v3/ratelimit.proto"; +import "envoy/config/route/v3/route_components.proto"; message TestRateLimitConfig { - repeated envoy.extensions.common.ratelimit.v3.RateLimitPolicy rate_limits = 1; + repeated envoy.config.route.v3.RateLimit rate_limits = 1; } From cd7fa928b58f19d449b78d5cf558d70c61b2c813 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sun, 22 Sep 2024 20:28:56 +0800 Subject: [PATCH 10/17] fix docs label Signed-off-by: wangbaiping --- .../filters/http/local_ratelimit/v3/local_rate_limit.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index 2f70155ee33b..de16d01134d5 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -159,7 +159,7 @@ message LocalRateLimit { // // .. note:: // Not all configuration fields of - // :ref:`rate limit config ` is supported at here. + // :ref:`rate limit config ` is supported at here. // Following fields are not supported: // // 1. :ref:`rate limit stage `. From 4a776f16e9d75f09ae7f8816db50296e35f04e16 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sun, 22 Sep 2024 20:30:33 +0800 Subject: [PATCH 11/17] resolve warn Signed-off-by: wangbaiping --- api/envoy/extensions/common/ratelimit/v3/ratelimit.proto | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto index 37270c378ded..73d729adc269 100644 --- a/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto +++ b/api/envoy/extensions/common/ratelimit/v3/ratelimit.proto @@ -5,8 +5,6 @@ package envoy.extensions.common.ratelimit.v3; import "envoy/type/v3/ratelimit_unit.proto"; import "envoy/type/v3/token_bucket.proto"; -import "google/protobuf/wrappers.proto"; - import "udpa/annotations/status.proto"; import "udpa/annotations/versioning.proto"; import "validate/validate.proto"; From 6677aa41f8e4e49f86406bee1c46f8ff4dbe9927 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Mon, 23 Sep 2024 14:16:56 +0800 Subject: [PATCH 12/17] fix coverage Signed-off-by: wangbaiping --- .../ratelimit_config/ratelimit_config.cc | 3 +- .../filters/common/ratelimit_config/BUILD | 1 + .../ratelimit_config/ratelimit_config_test.cc | 87 ++++++++++++++++++- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index 42a572fd9211..302d8f98b123 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -76,8 +76,7 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, actions_.emplace_back(new Router::QueryParameterValueMatchAction( action.query_parameter_value_match(), context)); break; - case ProtoRateLimit::Action::ActionSpecifierCase::kDynamicMetadata: - case ProtoRateLimit::Action::ActionSpecifierCase::ACTION_SPECIFIER_NOT_SET: + default: creation_status = absl::InvalidArgumentError(fmt::format( "Unsupported rate limit action: {}", static_cast(action.action_specifier_case()))); return; diff --git a/test/extensions/filters/common/ratelimit_config/BUILD b/test/extensions/filters/common/ratelimit_config/BUILD index 2f2c365fc7f8..bffabf03a6cd 100644 --- a/test/extensions/filters/common/ratelimit_config/BUILD +++ b/test/extensions/filters/common/ratelimit_config/BUILD @@ -30,6 +30,7 @@ envoy_cc_test( "//test/mocks/ratelimit:ratelimit_mocks", "//test/mocks/router:router_mocks", "//test/mocks/server:instance_mocks", + "//test/test_common:registry_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index 41fdb0f17af1..d5cde6188462 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -18,6 +18,7 @@ #include "test/mocks/router/mocks.h" #include "test/mocks/server/instance.h" #include "test/test_common/printers.h" +#include "test/test_common/registry.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -94,6 +95,37 @@ class RateLimitConfigTest : public testing::Test { NiceMock stream_info_; }; +TEST_F(RateLimitConfigTest, NoAction) { + { + const std::string yaml = R"EOF( +actions: +- {} + )EOF"; + + ProtoRateLimit rate_limit; + TestUtility::loadFromYaml(yaml, rate_limit); + + absl::Status creation_status; + RateLimitPolicy policy(rate_limit, factory_context_, creation_status); + + EXPECT_TRUE(absl::StartsWith(creation_status.message(), "Unsupported rate limit action:")); + } + + { + const std::string yaml = R"EOF( + rate_limits: + - actions: + - remote_address: {} + - {} + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); + setupTest(yaml); + + EXPECT_TRUE(absl::StartsWith(creation_status_.message(), "Unsupported rate limit action:")); + } +} + TEST_F(RateLimitConfigTest, EmptyRateLimit) { const std::string yaml = R"EOF( rate_limits: [] @@ -149,10 +181,8 @@ TEST_F(RateLimitConfigTest, MultiplePoliciesAndMultipleActions) { class RateLimitPolicyTest : public testing::Test { public: void setupTest(const std::string& yaml) { - absl::Status creation_status; rate_limit_entry_ = std::make_unique(parseRateLimitFromV3Yaml(yaml), - factory_context_, creation_status); - THROW_IF_NOT_OK(creation_status); // NOLINT + factory_context_, creation_status_); descriptors_.clear(); stream_info_.downstream_connection_info_provider_->setRemoteAddress(default_remote_address_); ON_CALL(Const(stream_info_), route()).WillByDefault(testing::Return(route_)); @@ -160,6 +190,7 @@ class RateLimitPolicyTest : public testing::Test { NiceMock factory_context_; std::unique_ptr rate_limit_entry_; + absl::Status creation_status_{}; Http::TestRequestHeaderMapImpl headers_; std::shared_ptr route_{new NiceMock()}; @@ -987,6 +1018,56 @@ TEST_F(RateLimitPolicyTest, RequestMatchInputSkip) { EXPECT_TRUE(descriptors_.empty()); } +class ExtensionDescriptorFactory : public Envoy::RateLimit::DescriptorProducerFactory { +public: + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique(); + } + std::string name() const override { return "test.descriptor_producer"; } + + Envoy::RateLimit::DescriptorProducerPtr + createDescriptorProducerFromProto(const Protobuf::Message&, + Server::Configuration::CommonFactoryContext&) override { + return return_valid_producer_ ? std::make_unique() : nullptr; + } + bool return_valid_producer_{true}; +}; + +TEST_F(RateLimitPolicyTest, ExtensionDescriptorProducer) { + const std::string ExtensionDescriptor = R"EOF( +actions: +- extension: + name: test.descriptor_producer + typed_config: + "@type": type.googleapis.com/google.protobuf.Struct + value: + key: value + )EOF"; + + { + ExtensionDescriptorFactory factory; + Registry::InjectFactory registration(factory); + + setupTest(ExtensionDescriptor); + + rate_limit_entry_->populateDescriptors(headers_, stream_info_, "service_cluster", descriptors_); + EXPECT_THAT( + std::vector({{{{"source_cluster", "service_cluster"}}}}), + testing::ContainerEq(descriptors_)); + } + + { + ExtensionDescriptorFactory factory; + factory.return_valid_producer_ = false; + Registry::InjectFactory registration(factory); + + setupTest(ExtensionDescriptor); + + EXPECT_TRUE( + absl::StartsWith(creation_status_.message(), "Rate limit descriptor extension failed:")); + } +} + } // namespace } // namespace RateLimit } // namespace Common From 4ad79d23c7fbdcbdfc9164a798cd45ac7f21a3ed Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Wed, 25 Sep 2024 23:45:15 +0800 Subject: [PATCH 13/17] address comment and add warning Signed-off-by: wangbaiping --- .../ratelimit_config/ratelimit_config.cc | 18 +++++++++++++--- .../ratelimit_config/ratelimit_config.h | 8 +++---- .../http/local_ratelimit/local_ratelimit.cc | 10 +++++++++ .../http/local_ratelimit/local_ratelimit.h | 3 ++- .../ratelimit_config/ratelimit_config_test.cc | 21 +++++++++++++++++++ 5 files changed, 52 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index 302d8f98b123..f244349d94b6 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -12,7 +12,18 @@ namespace RateLimit { RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, Server::Configuration::CommonFactoryContext& context, - absl::Status& creation_status) { + absl::Status& creation_status, bool no_limit) { + if (config.has_stage() || !config.disable_key().empty()) { + ENVOY_LOG(warn, "'stage' field and 'disable_key' field are not supported"); + } + + if (config.has_limit()) { + if (no_limit) { + ENVOY_LOG(warn, "'limit' field is only supported in filter that calls remote rate " + "limit service."); + } + } + for (const ProtoRateLimit::Action& action : config.actions()) { switch (action.action_specifier_case()) { case ProtoRateLimit::Action::ActionSpecifierCase::kSourceCluster: @@ -103,9 +114,10 @@ void RateLimitPolicy::populateDescriptors(const Http::RequestHeaderMap& headers, RateLimitConfig::RateLimitConfig(const Protobuf::RepeatedPtrField& configs, Server::Configuration::CommonFactoryContext& context, - absl::Status& creation_status) { + absl::Status& creation_status, bool no_limit) { for (const ProtoRateLimit& config : configs) { - auto descriptor_generator = std::make_unique(config, context, creation_status); + auto descriptor_generator = + std::make_unique(config, context, creation_status, no_limit); if (!creation_status.ok()) { return; } diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h index e044edb216f5..743e69360c47 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.h +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.h @@ -16,11 +16,11 @@ namespace RateLimit { using ProtoRateLimit = envoy::config::route::v3::RateLimit; using RateLimitDescriptors = std::vector; -class RateLimitPolicy { +class RateLimitPolicy : Logger::Loggable { public: RateLimitPolicy(const ProtoRateLimit& config, Server::Configuration::CommonFactoryContext& context, - absl::Status& creation_status); + absl::Status& creation_status, bool no_limit = true); void populateDescriptors(const Http::RequestHeaderMap& headers, const StreamInfo::StreamInfo& info, @@ -31,11 +31,11 @@ class RateLimitPolicy { std::vector actions_; }; -class RateLimitConfig { +class RateLimitConfig : Logger::Loggable { public: RateLimitConfig(const Protobuf::RepeatedPtrField& configs, Server::Configuration::CommonFactoryContext& context, - absl::Status& creation_status); + absl::Status& creation_status, bool no_limit = true); bool empty() const { return rate_limit_policies_.empty(); } diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index 59e5521aaeb4..7ef4d3cceba4 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -76,6 +76,16 @@ FilterConfig::FilterConfig( config.rate_limits(), context, creation_status); THROW_IF_NOT_OK_REF(creation_status); + if (rate_limit_config_->empty()) { + if (!config.descriptors().empty()) { + ENVOY_LOG_FIRST_N( + warn, 20, + "'descriptors' are set for local rate limit filter but no 'rate_limits' " + "are configured in the filter config. Please use the 'rate_limits' field " + "in filter config to instead of the 'rate_limits' field in the route config."); + } + } + Filters::Common::LocalRateLimit::ShareProviderSharedPtr share_provider; if (config.has_local_cluster_rate_limit()) { if (rate_limit_per_connection_) { diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h index 3df0bd94e94d..2b3af4ec7fb8 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.h +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.h @@ -70,7 +70,8 @@ class PerConnectionRateLimiter : public StreamInfo::FilterState::Object { /** * Global configuration for the HTTP local rate limit filter. */ -class FilterConfig : public Router::RouteSpecificFilterConfig { +class FilterConfig : public Router::RouteSpecificFilterConfig, + Logger::Loggable { public: FilterConfig(const envoy::extensions::filters::http::local_ratelimit::v3::LocalRateLimit& config, Server::Configuration::CommonFactoryContext& context, Stats::Scope& scope, diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index d5cde6188462..fd9f6f2b88e2 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -95,6 +95,27 @@ class RateLimitConfigTest : public testing::Test { NiceMock stream_info_; }; +TEST_F(RateLimitConfigTest, MeaninglessAndForCoverage) { + { + const std::string yaml = R"EOF( + rate_limits: + - actions: + - remote_address: {} + stage: 2 + disable_key: anything + limit: + dynamic_metadata: + metadata_key: + key: key + path: + - key: key + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); + setupTest(yaml); + } +} + TEST_F(RateLimitConfigTest, NoAction) { { const std::string yaml = R"EOF( From 135975e64588c666a5fe0e47fa456e486ee72f01 Mon Sep 17 00:00:00 2001 From: code Date: Thu, 3 Oct 2024 07:13:30 +0800 Subject: [PATCH 14/17] Update api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto Co-authored-by: Matt Klein Signed-off-by: code --- .../filters/http/local_ratelimit/v3/local_rate_limit.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto index de16d01134d5..82e38ed91d5a 100644 --- a/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto +++ b/api/envoy/extensions/filters/http/local_ratelimit/v3/local_rate_limit.proto @@ -149,8 +149,8 @@ message LocalRateLimit { // HTTP code will be 200 for a gRPC response. bool rate_limited_as_resource_exhausted = 15; - // Rate limit configuration that used to generate a list of description entries based on - // the request context. And the generated entries will be used to find one or multiple matched rate + // Rate limit configuration that is used to generate a list of descriptor entries based on + // the request context. The generated entries will be used to find one or multiple matched rate // limit rule from the ``descriptors``. // If this is set, then // :ref:`VirtualHost.rate_limits` or From 3d5429a771f473dbbf028e5ce0c40a3d021998f8 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Thu, 3 Oct 2024 21:35:17 +0800 Subject: [PATCH 15/17] address comments Signed-off-by: wangbaiping --- .../ratelimit_config/ratelimit_config.cc | 8 +++--- .../http/local_ratelimit/local_ratelimit.cc | 8 +++--- .../ratelimit_config/ratelimit_config_test.cc | 26 ++++++++++++++++++- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc index f244349d94b6..c3fec0f4f691 100644 --- a/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc +++ b/source/extensions/filters/common/ratelimit_config/ratelimit_config.cc @@ -14,13 +14,15 @@ RateLimitPolicy::RateLimitPolicy(const ProtoRateLimit& config, Server::Configuration::CommonFactoryContext& context, absl::Status& creation_status, bool no_limit) { if (config.has_stage() || !config.disable_key().empty()) { - ENVOY_LOG(warn, "'stage' field and 'disable_key' field are not supported"); + creation_status = + absl::InvalidArgumentError("'stage' field and 'disable_key' field are not supported"); + return; } if (config.has_limit()) { if (no_limit) { - ENVOY_LOG(warn, "'limit' field is only supported in filter that calls remote rate " - "limit service."); + creation_status = absl::InvalidArgumentError("'limit' field is not supported"); + return; } } diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index 7ef4d3cceba4..ef693206b97e 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -80,9 +80,11 @@ FilterConfig::FilterConfig( if (!config.descriptors().empty()) { ENVOY_LOG_FIRST_N( warn, 20, - "'descriptors' are set for local rate limit filter but no 'rate_limits' " - "are configured in the filter config. Please use the 'rate_limits' field " - "in filter config to instead of the 'rate_limits' field in the route config."); + "'descriptors' is set but no valid 'rate_limits' in LocalRateLimit config. Note that " + "'descriptors' only makes sense when 'rate_limits' in LocalRateLimit config or route " + "config is specified. And please take the 'rate_limits' in the LocalRateLimit config as " + "priority because the 'rate_limits' in the route config may be ignored by the filter in " + "the future."); } } diff --git a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc index fd9f6f2b88e2..aea1832038bc 100644 --- a/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc +++ b/test/extensions/filters/common/ratelimit_config/ratelimit_config_test.cc @@ -95,7 +95,7 @@ class RateLimitConfigTest : public testing::Test { NiceMock stream_info_; }; -TEST_F(RateLimitConfigTest, MeaninglessAndForCoverage) { +TEST_F(RateLimitConfigTest, DisableKeyIsNotAllowed) { { const std::string yaml = R"EOF( rate_limits: @@ -113,6 +113,30 @@ TEST_F(RateLimitConfigTest, MeaninglessAndForCoverage) { factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); setupTest(yaml); + EXPECT_FALSE(creation_status_.ok()); + EXPECT_EQ(creation_status_.message(), + "'stage' field and 'disable_key' field are not supported"); + } +} + +TEST_F(RateLimitConfigTest, LimitIsNotAllowed) { + { + const std::string yaml = R"EOF( + rate_limits: + - actions: + - remote_address: {} + limit: + dynamic_metadata: + metadata_key: + key: key + path: + - key: key + )EOF"; + + factory_context_.cluster_manager_.initializeClusters({"www2"}, {}); + setupTest(yaml); + EXPECT_FALSE(creation_status_.ok()); + EXPECT_EQ(creation_status_.message(), "'limit' field is not supported"); } } From 3b8d8b7b3ef170d3a08b61ea56fe27f87f4c110a Mon Sep 17 00:00:00 2001 From: code Date: Sat, 5 Oct 2024 13:16:06 +0800 Subject: [PATCH 16/17] Update source/extensions/filters/http/local_ratelimit/local_ratelimit.cc Co-authored-by: Matt Klein Signed-off-by: code --- .../filters/http/local_ratelimit/local_ratelimit.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index ef693206b97e..158af7a34a32 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -80,11 +80,7 @@ FilterConfig::FilterConfig( if (!config.descriptors().empty()) { ENVOY_LOG_FIRST_N( warn, 20, - "'descriptors' is set but no valid 'rate_limits' in LocalRateLimit config. Note that " - "'descriptors' only makes sense when 'rate_limits' in LocalRateLimit config or route " - "config is specified. And please take the 'rate_limits' in the LocalRateLimit config as " - "priority because the 'rate_limits' in the route config may be ignored by the filter in " - "the future."); + "'descriptors' is set but only used by route configuration. Please configure the local rate limit filter using the embedded 'rate_limits' field as route level configuration for local rate limits will be ignored in the future."); } } From e9893ed6dbdbd5ed3e675e65f259485c7688b2c2 Mon Sep 17 00:00:00 2001 From: wangbaiping Date: Sat, 5 Oct 2024 13:17:45 +0800 Subject: [PATCH 17/17] fix format Signed-off-by: wangbaiping --- .../filters/http/local_ratelimit/local_ratelimit.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc index 158af7a34a32..0e12da02bfdd 100644 --- a/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc +++ b/source/extensions/filters/http/local_ratelimit/local_ratelimit.cc @@ -80,7 +80,9 @@ FilterConfig::FilterConfig( if (!config.descriptors().empty()) { ENVOY_LOG_FIRST_N( warn, 20, - "'descriptors' is set but only used by route configuration. Please configure the local rate limit filter using the embedded 'rate_limits' field as route level configuration for local rate limits will be ignored in the future."); + "'descriptors' is set but only used by route configuration. Please configure the local " + "rate limit filter using the embedded 'rate_limits' field as route configuration for " + "local rate limits will be ignored in the future."); } }