diff --git a/DEPRECATED.md b/DEPRECATED.md index 63fcfa9b89ec6..0a6f54c846cf2 100644 --- a/DEPRECATED.md +++ b/DEPRECATED.md @@ -29,9 +29,6 @@ A logged warning is expected for each deprecated item that is in deprecation win * Setting hosts via `hosts` field in `Cluster` is deprecated. Use `load_assignment` instead. * Use of `response_headers_to_*` and `request_headers_to_add` are deprecated at the `RouteAction` level. Please use the configuration options at the `Route` level. -* Use of `runtime` in `RouteMatch`, found in - [route.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/api/v2/route/route.proto). - Set the `runtime_fraction` field instead. * Use of the string `user` field in `Authenticated` in [rbac.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/rbac/v2alpha/rbac.proto) is deprecated in favor of the new `StringMatcher` based `principal_name` field. diff --git a/api/envoy/api/v2/core/BUILD b/api/envoy/api/v2/core/BUILD index 45251aebb4ba5..71a8d33f59d35 100644 --- a/api/envoy/api/v2/core/BUILD +++ b/api/envoy/api/v2/core/BUILD @@ -37,15 +37,11 @@ api_proto_library_internal( visibility = [ ":friends", ], - deps = [ - "//envoy/type:percent", - ], ) api_go_proto_library( name = "base", proto = ":base", - deps = ["//envoy/type:percent_go_proto"], ) api_proto_library_internal( diff --git a/api/envoy/api/v2/core/base.proto b/api/envoy/api/v2/core/base.proto index 14c9a89a76a4b..40204e38d87c7 100644 --- a/api/envoy/api/v2/core/base.proto +++ b/api/envoy/api/v2/core/base.proto @@ -9,8 +9,6 @@ import "google/protobuf/wrappers.proto"; import "validate/validate.proto"; import "gogoproto/gogo.proto"; -import "envoy/type/percent.proto"; - option (gogoproto.equal_all) = true; // [#protodoc-title: Common types] @@ -218,13 +216,3 @@ message SocketOption { SocketState state = 6 [(validate.rules).message.required = true, (validate.rules).enum.defined_only = true]; } - -// Runtime derived FractionalPercent with defaults for when the numerator or denominator is not -// specified via a runtime key. -message RuntimeFractionalPercent { - // Default value if the runtime value's for the numerator/denominator keys are not available. - envoy.type.FractionalPercent default_value = 1 [(validate.rules).message.required = true]; - - // Runtime key for a YAML representation of a FractionalPercent. - string runtime_key = 2; -} diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 9c9a9793f07cc..51fb5ce5f91c6 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -291,33 +291,16 @@ message RouteMatch { // is true. google.protobuf.BoolValue case_sensitive = 4; - oneof runtime_specifier { - // Indicates that the route should additionally match on a runtime key. An integer between - // 0-100. Every time the route is considered for a match, a random number between 0-99 is - // selected. If the number is <= the value found in the key (checked first) or, if the key is - // not present, the default value, the route is a match (assuming everything also about the - // route matches). A runtime route configuration can be used to roll out route changes in a - // gradual manner without full code/config deploys. Refer to the :ref:`traffic shifting - // ` docs for additional - // documentation. - // - // .. attention:: - // - // **This field is deprecated**. Set the - // :ref:`runtime_fraction` field instead. - core.RuntimeUInt32 runtime = 5 [deprecated = true]; - - // Indicates that the route should additionally match on a runtime key. Every time the route - // is considered for a match, it must also fall under the percentage of matches indicated by - // this field. For some fraction N/D, a random number in the range [0,D) is selected. If the - // number is <= the value of the numberator N, or if the key is not present, the default - // value, the router continues to evaluate the remaining match criteria. A runtime_fraction - // route configuration can be used to roll out route changes in a gradual manner (with more - // granularity than the deprecated runtime field) without full code/config deploys. Refer to - // the :ref:`traffic shifting ` docs - // for additional documentation. - core.RuntimeFractionalPercent runtime_fraction = 8; - } + // Indicates that the route should additionally match on a runtime key. An + // integer between 0-100. Every time the route is considered for a match, a + // random number between 0-99 is selected. If the number is <= the value found + // in the key (checked first) or, if the key is not present, the default + // value, the route is a match (assuming everything also about the route + // matches). A runtime route configuration can be used to roll out route changes in a + // gradual manner without full code/config deploys. Refer to the + // :ref:`traffic shifting ` docs + // for additional documentation. + core.RuntimeUInt32 runtime = 5; // Specifies a set of headers that the route should match on. The router will // check the request’s headers against all the specified headers in the route diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc index 09db698dbcff3..163b695cf0253 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -113,15 +113,14 @@ bool RuntimeFilter::evaluate(const RequestInfo::RequestInfo&, const Http::HeaderEntry* uuid = request_header.RequestId(); uint64_t random_value; if (use_independent_randomness_ || uuid == nullptr || - !UuidUtils::uuidModBy( - uuid->value().c_str(), random_value, - ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_.denominator()))) { + !UuidUtils::uuidModBy(uuid->value().c_str(), random_value, + ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_))) { random_value = random_.random(); } return runtime_.snapshot().featureEnabled( runtime_key_, percent_.numerator(), random_value, - ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_.denominator())); + ProtobufPercentHelper::fractionalPercentDenominatorToInt(percent_)); } OperatorFilter::OperatorFilter(const Protobuf::RepeatedPtrField< diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index d8a3ab138f509..b2b5b82faeaad 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -20,9 +20,8 @@ uint64_t convertPercent(double percent, uint64_t max_value) { return max_value * (percent / 100.0); } -uint64_t fractionalPercentDenominatorToInt( - const envoy::type::FractionalPercent::DenominatorType& denominator) { - switch (denominator) { +uint64_t fractionalPercentDenominatorToInt(const envoy::type::FractionalPercent& percent) { + switch (percent.denominator()) { case envoy::type::FractionalPercent::HUNDRED: return 100; case envoy::type::FractionalPercent::TEN_THOUSAND: diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index f34975f90e14f..b615010f3c000 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -68,11 +68,10 @@ uint64_t convertPercent(double percent, uint64_t max_value); /** * Convert a fractional percent denominator enum into an integer. - * @param denominator supplies denominator to convert. + * @param percent supplies percent to convert. * @return the converted denominator. */ -uint64_t fractionalPercentDenominatorToInt( - const envoy::type::FractionalPercent::DenominatorType& denominator); +uint64_t fractionalPercentDenominatorToInt(const envoy::type::FractionalPercent& percent); } // namespace ProtobufPercentHelper } // namespace Envoy diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 5de127f5d5882..61294100ef5b1 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -11,7 +11,6 @@ #include "envoy/http/header_map.h" #include "envoy/runtime/runtime.h" -#include "envoy/type/percent.pb.validate.h" #include "envoy/upstream/cluster_manager.h" #include "envoy/upstream/upstream.h" @@ -19,7 +18,6 @@ #include "common/common/empty_string.h" #include "common/common/fmt.h" #include "common/common/hash.h" -#include "common/common/logger.h" #include "common/common/utility.h" #include "common/config/metadata.h" #include "common/config/rds_json.h" @@ -28,7 +26,6 @@ #include "common/http/headers.h" #include "common/http/utility.h" #include "common/http/websocket/ws_handler_impl.h" -#include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" #include "common/router/retry_state_impl.h" @@ -255,7 +252,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, timeout_(PROTOBUF_GET_MS_OR_DEFAULT(route.route(), timeout, DEFAULT_ROUTE_TIMEOUT_MS)), idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), idle_timeout)), max_grpc_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), max_grpc_timeout)), - loader_(factory_context.runtime()), runtime_(loadRuntimeData(route.match())), + runtime_(loadRuntimeData(route.match())), loader_(factory_context.runtime()), host_redirect_(route.redirect().host_redirect()), path_redirect_(route.redirect().path_redirect()), https_redirect_(route.redirect().https_redirect()), @@ -343,11 +340,8 @@ bool RouteEntryImplBase::matchRoute(const Http::HeaderMap& headers, uint64_t ran bool matches = true; if (runtime_) { - matches &= random_value % runtime_->denominator_val_ < runtime_->numerator_val_; - if (!matches) { - // No need to waste further cycles calculating a route match. - return false; - } + matches &= loader_.snapshot().featureEnabled(runtime_.value().key_, runtime_.value().default_, + random_value); } if (match_grpc_) { @@ -409,36 +403,14 @@ void RouteEntryImplBase::finalizeResponseHeaders( absl::optional RouteEntryImplBase::loadRuntimeData(const envoy::api::v2::route::RouteMatch& route_match) { absl::optional runtime; - RuntimeData runtime_data; - - if (route_match.runtime_specifier_case() == envoy::api::v2::route::RouteMatch::kRuntimeFraction) { - envoy::type::FractionalPercent fractional_percent; - const std::string& fraction_yaml = - loader_.snapshot().get(route_match.runtime_fraction().runtime_key()); - - try { - MessageUtil::loadFromYamlAndValidate(fraction_yaml, fractional_percent); - } catch (const EnvoyException& ex) { - ENVOY_LOG(error, "failed to parse string value for runtime key {}: {}", - route_match.runtime_fraction().runtime_key(), ex.what()); - fractional_percent = route_match.runtime_fraction().default_value(); - } - - runtime_data.numerator_val_ = fractional_percent.numerator(); - runtime_data.denominator_val_ = - ProtobufPercentHelper::fractionalPercentDenominatorToInt(fractional_percent.denominator()); - } else if (route_match.runtime_specifier_case() == envoy::api::v2::route::RouteMatch::kRuntime) { - // For backwards compatibility, the deprecated 'runtime' field must be converted to a - // RuntimeData format with a variable denominator type. The 'runtime' field assumes a percentage - // (0-100), so the hard-coded denominator value reflects this. - runtime_data.denominator_val_ = 100; - runtime_data.numerator_val_ = loader_.snapshot().getInteger( - route_match.runtime().runtime_key(), route_match.runtime().default_value()); - } else { - return runtime; + if (route_match.has_runtime()) { + RuntimeData data; + data.key_ = route_match.runtime().runtime_key(); + data.default_ = route_match.runtime().default_value(); + runtime = data; } - return runtime_data; + return runtime; } void RouteEntryImplBase::finalizePathHeader(Http::HeaderMap& headers, diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 6d195632eb720..6678fa22ac5af 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -281,8 +281,7 @@ class RouteEntryImplBase : public RouteEntry, public DirectResponseEntry, public Route, public PathMatchCriterion, - public std::enable_shared_from_this, - Logger::Loggable { + public std::enable_shared_from_this { public: /** * @throw EnvoyException with reason if the route configuration contains any errors @@ -377,8 +376,8 @@ class RouteEntryImplBase : public RouteEntry, private: struct RuntimeData { - uint64_t numerator_val_{}; - uint64_t denominator_val_{}; + std::string key_{}; + uint64_t default_{}; }; class DynamicRouteEntry : public RouteEntry, public Route { @@ -509,7 +508,8 @@ class RouteEntryImplBase : public RouteEntry, typedef std::shared_ptr WeightedClusterEntrySharedPtr; - absl::optional loadRuntimeData(const envoy::api::v2::route::RouteMatch& route); + static absl::optional + loadRuntimeData(const envoy::api::v2::route::RouteMatch& route); static std::multimap parseOpaqueConfig(const envoy::api::v2::route::Route& route); @@ -530,8 +530,8 @@ class RouteEntryImplBase : public RouteEntry, const std::chrono::milliseconds timeout_; const absl::optional idle_timeout_; const absl::optional max_grpc_timeout_; - Runtime::Loader& loader_; const absl::optional runtime_; + Runtime::Loader& loader_; const std::string host_redirect_; const std::string path_redirect_; const bool https_redirect_; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 1d9a6fe87e425..5301565895612 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -134,14 +134,13 @@ bool FaultFilter::isDelayEnabled() { bool enabled = config_->runtime().snapshot().featureEnabled( DELAY_PERCENT_KEY, fault_settings_->delayPercentage().numerator(), config_->randomGenerator().random(), - ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->delayPercentage().denominator())); + ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->delayPercentage())); if (!downstream_cluster_delay_percent_key_.empty()) { enabled |= config_->runtime().snapshot().featureEnabled( downstream_cluster_delay_percent_key_, fault_settings_->delayPercentage().numerator(), config_->randomGenerator().random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->delayPercentage().denominator())); + fault_settings_->delayPercentage())); } return enabled; } @@ -150,14 +149,13 @@ bool FaultFilter::isAbortEnabled() { bool enabled = config_->runtime().snapshot().featureEnabled( ABORT_PERCENT_KEY, fault_settings_->abortPercentage().numerator(), config_->randomGenerator().random(), - ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->abortPercentage().denominator())); + ProtobufPercentHelper::fractionalPercentDenominatorToInt(fault_settings_->abortPercentage())); if (!downstream_cluster_abort_percent_key_.empty()) { enabled |= config_->runtime().snapshot().featureEnabled( downstream_cluster_abort_percent_key_, fault_settings_->abortPercentage().numerator(), config_->randomGenerator().random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_settings_->abortPercentage().denominator())); + fault_settings_->abortPercentage())); } return enabled; } diff --git a/source/extensions/filters/network/mongo_proxy/proxy.cc b/source/extensions/filters/network/mongo_proxy/proxy.cc index 3c01ced80cf4d..4b72b96a0e62f 100644 --- a/source/extensions/filters/network/mongo_proxy/proxy.cc +++ b/source/extensions/filters/network/mongo_proxy/proxy.cc @@ -326,7 +326,7 @@ absl::optional ProxyFilter::delayDuration() { fault_config_->delayPercentage().numerator(), generator_.random(), ProtobufPercentHelper::fractionalPercentDenominatorToInt( - fault_config_->delayPercentage().denominator()))) { + fault_config_->delayPercentage()))) { return result; } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index b3da16ec6c977..219e9c57a7a5d 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -1971,104 +1971,16 @@ TEST(RouteMatcherTest, Runtime) { Runtime::MockSnapshot snapshot; ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); - EXPECT_CALL(snapshot, getInteger("some_key", 50)) - .Times(testing::AtLeast(1)) - .WillRepeatedly(Return(42)); TestConfigImpl config(parseRouteConfigurationFromJson(json), factory_context, true); + EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 10)).WillOnce(Return(true)); EXPECT_EQ("something_else", - config.route(genHeaders("www.lyft.com", "/", "GET"), 41)->routeEntry()->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 10)->routeEntry()->clusterName()); + EXPECT_CALL(snapshot, featureEnabled("some_key", 50, 20)).WillOnce(Return(false)); EXPECT_EQ("www2", - config.route(genHeaders("www.lyft.com", "/", "GET"), 43)->routeEntry()->clusterName()); -} - -TEST(RouteMatcherTest, FractionalRuntime) { - std::string yaml = R"EOF( -virtual_hosts: - - name: "www2" - domains: ["www.lyft.com"] - routes: - - match: - prefix: "/" - runtime_fraction: - default_value: - numerator: 50 - denominator: MILLION - runtime_key: "bogus_key" - route: - cluster: "something_else" - - match: - prefix: "/" - route: - cluster: "www2" - )EOF"; - - NiceMock factory_context; - Runtime::MockSnapshot snapshot; - ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); - - const std::string runtime_fraction = R"EOF( - numerator: 42 - denominator: HUNDRED - )EOF"; - EXPECT_CALL(snapshot, get("bogus_key")) - .Times(testing::AtLeast(1)) - .WillRepeatedly(ReturnRef(runtime_fraction)); - - TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, false); - - EXPECT_EQ( - "something_else", - config.route(genHeaders("www.lyft.com", "/foo", "GET"), 41)->routeEntry()->clusterName()); - - EXPECT_EQ( - "www2", - config.route(genHeaders("www.lyft.com", "/foo", "GET"), 43)->routeEntry()->clusterName()); -} - -TEST(RouteMatcherTest, FractionalRuntimeDefault) { - std::string yaml = R"EOF( -virtual_hosts: - - name: "www2" - domains: ["www.lyft.com"] - routes: - - match: - prefix: "/" - runtime_fraction: - default_value: - numerator: 50 - denominator: MILLION - runtime_key: "bogus_key" - route: - cluster: "something_else" - - match: - prefix: "/" - route: - cluster: "www2" - )EOF"; - - NiceMock factory_context; - Runtime::MockSnapshot snapshot; - ON_CALL(factory_context.runtime_loader_, snapshot()).WillByDefault(ReturnRef(snapshot)); - - const std::string runtime_fraction = R"EOF( - this string is nonsense and should fail parsing - )EOF"; - EXPECT_CALL(snapshot, get("bogus_key")) - .Times(testing::AtLeast(1)) - .WillRepeatedly(ReturnRef(runtime_fraction)); - - TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context, false); - - EXPECT_EQ( - "something_else", - config.route(genHeaders("www.lyft.com", "/foo", "GET"), 49)->routeEntry()->clusterName()); - - EXPECT_EQ( - "www2", - config.route(genHeaders("www.lyft.com", "/foo", "GET"), 51)->routeEntry()->clusterName()); + config.route(genHeaders("www.lyft.com", "/", "GET"), 20)->routeEntry()->clusterName()); } TEST(RouteMatcherTest, ShadowClusterNotFound) {