From fdec21589cf8e9ecc089068ebf556490938da78c Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Wed, 2 Mar 2022 01:48:12 -0500 Subject: [PATCH] test: removing a bunch of direct runtime singleton access (#19993) Signed-off-by: Alyssa Wilk Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> --- test/benchmark/main.cc | 7 ++---- test/common/common/dns_utils_test.cc | 9 +++---- test/common/common/regex_test.cc | 9 +++---- .../delta_subscription_state_old_test.cc | 4 +-- .../config/subscription_factory_impl_test.cc | 3 +-- test/common/event/dispatcher_impl_test.cc | 3 +-- test/common/event/file_event_impl_test.cc | 2 -- .../grpc/async_client_manager_impl_test.cc | 2 +- test/common/http/conn_manager_utility_test.cc | 15 +++++------ test/common/http/header_map_impl_fuzz_test.cc | 7 +++--- test/common/http/header_map_impl_test.cc | 4 +-- test/common/http/http1/codec_impl_test.cc | 6 ----- test/common/http/http2/conn_pool_test.cc | 2 -- test/common/network/connection_impl_test.cc | 5 ++-- test/common/network/listener_impl_test.cc | 15 ++++------- test/common/protobuf/utility_test.cc | 25 ++++++++----------- test/common/router/router_test.cc | 4 +-- .../upstream/cluster_manager_impl_test.cc | 3 +-- test/common/upstream/eds_speed_test.cc | 2 +- test/common/upstream/eds_test.cc | 8 +++--- .../dns_cache_impl_test.cc | 6 ++--- .../filters/http/buffer/buffer_filter_test.cc | 3 --- .../filters/http/ext_authz/ext_authz_test.cc | 2 +- .../filters/http/wasm/wasm_filter_test.cc | 12 --------- .../network/common/redis/fault_test.cc | 5 ---- test/server/connection_handler_test.cc | 4 +-- test/server/listener_manager_impl_test.cc | 20 ++++++--------- .../test_common/simulated_time_system_test.cc | 2 -- test/test_common/test_runtime.h | 12 +++++---- 29 files changed, 68 insertions(+), 133 deletions(-) diff --git a/test/benchmark/main.cc b/test/benchmark/main.cc index 69c117d12bbf..17780ea14d20 100644 --- a/test/benchmark/main.cc +++ b/test/benchmark/main.cc @@ -82,15 +82,12 @@ int main(int argc, char** argv) { skip_expensive_benchmarks = skip_switch.getValue(); + TestScopedRuntime runtime; // Initialize scoped_runtime if a runtime_feature argument is present. This // allows benchmarks to use their own scoped_runtime in case no runtime flag is // passed as an argument. - std::unique_ptr scoped_runtime = nullptr; const auto& runtime_features_args = runtime_features.getValue(); for (const absl::string_view runtime_feature_arg : runtime_features_args) { - if (scoped_runtime == nullptr) { - scoped_runtime = std::make_unique(); - } // Make sure the argument contains a single ":" character. const std::vector runtime_feature_split = absl::StrSplit(runtime_feature_arg, ':'); if (runtime_feature_split.size() != 2) { @@ -102,7 +99,7 @@ int main(int argc, char** argv) { } const auto feature_name = runtime_feature_split[0]; const auto feature_val = runtime_feature_split[1]; - Runtime::LoaderSingleton::getExisting()->mergeValues({{feature_name, feature_val}}); + runtime.mergeValues({{feature_name, feature_val}}); } if (skip_expensive_benchmarks) { diff --git a/test/common/common/dns_utils_test.cc b/test/common/common/dns_utils_test.cc index 3ea82592e71b..784a817fc17e 100644 --- a/test/common/common/dns_utils_test.cc +++ b/test/common/common/dns_utils_test.cc @@ -11,8 +11,7 @@ namespace { TEST(DnsUtils, LegacyGenerateTest) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.allow_multiple_dns_addresses", "false"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "false"}}); std::list responses = TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"}); @@ -23,8 +22,7 @@ TEST(DnsUtils, LegacyGenerateTest) { TEST(DnsUtils, MultipleGenerateTest) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); std::list responses = TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"}); @@ -37,8 +35,7 @@ TEST(DnsUtils, MultipleGenerateTest) { TEST(DnsUtils, ListChanged) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); Network::Address::InstanceConstSharedPtr address1 = Network::Utility::parseInternetAddress("10.0.0.1"); diff --git a/test/common/common/regex_test.cc b/test/common/common/regex_test.cc index b6d36c3cd44d..7330b1050c79 100644 --- a/test/common/common/regex_test.cc +++ b/test/common/common/regex_test.cc @@ -73,8 +73,7 @@ TEST(Utility, ParseRegex) { // The deprecated field codepath precedes any runtime settings. { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"re2.max_program_size.error_level", "3"}}); + scoped_runtime.mergeValues({{"re2.max_program_size.error_level", "3"}}); envoy::type::matcher::v3::RegexMatcher matcher; matcher.set_regex("/asdf/.*"); matcher.mutable_google_re2()->mutable_max_program_size()->set_value(1); @@ -90,8 +89,7 @@ TEST(Utility, ParseRegex) { // Verify that an exception is thrown for the error level max program size. { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"re2.max_program_size.error_level", "1"}}); + scoped_runtime.mergeValues({{"re2.max_program_size.error_level", "1"}}); envoy::type::matcher::v3::RegexMatcher matcher; matcher.set_regex("/asdf/.*"); matcher.mutable_google_re2(); @@ -127,8 +125,7 @@ TEST(Utility, ParseRegex) { // Verify that a warning is logged for the warn level max program size. { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"re2.max_program_size.warn_level", "1"}}); + scoped_runtime.mergeValues({{"re2.max_program_size.warn_level", "1"}}); envoy::type::matcher::v3::RegexMatcher matcher; matcher.set_regex("/asdf/.*"); matcher.mutable_google_re2(); diff --git a/test/common/config/delta_subscription_state_old_test.cc b/test/common/config/delta_subscription_state_old_test.cc index 8e61015e7130..fa564c6ca21a 100644 --- a/test/common/config/delta_subscription_state_old_test.cc +++ b/test/common/config/delta_subscription_state_old_test.cc @@ -38,8 +38,8 @@ class OldDeltaSubscriptionStateTestBase : public testing::Test { // Disable the explicit wildcard resource feature, so OldDeltaSubscriptionState will be picked // up. { - TestScopedRuntime scoped_runtime_; - Runtime::LoaderSingleton::getExisting()->mergeValues({ + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues({ {"envoy.restart_features.explicit_wildcard_resource", "false"}, }); state_ = std::make_unique(type_url, callbacks_, diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc index c27f407b1a7a..334b39c5805d 100644 --- a/test/common/config/subscription_factory_impl_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -80,8 +80,7 @@ class SubscriptionFactoryTestUnifiedOrLegacyMux public: SubscriptionFactoryTestUnifiedOrLegacyMux() { if (GetParam() == LegacyOrUnified::Unified) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.unified_mux", "true"}}); + scoped_runtime_.mergeValues({{"envoy.reloadable_features.unified_mux", "true"}}); } } diff --git a/test/common/event/dispatcher_impl_test.cc b/test/common/event/dispatcher_impl_test.cc index 7538a8c495c0..a7c84bb3a588 100644 --- a/test/common/event/dispatcher_impl_test.cc +++ b/test/common/event/dispatcher_impl_test.cc @@ -14,10 +14,10 @@ #include "source/common/stats/isolated_store_impl.h" #include "test/mocks/common.h" +#include "test/mocks/event/mocks.h" #include "test/mocks/server/watch_dog.h" #include "test/mocks/stats/mocks.h" #include "test/test_common/simulated_time_system.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -907,7 +907,6 @@ class TimerImplTest : public testing::Test { requested_advance_ = absl::ZeroDuration(); } - TestScopedRuntime scoped_runtime_; absl::Duration requested_advance_ = absl::ZeroDuration(); std::vector> check_callbacks_; bool in_event_loop_{}; diff --git a/test/common/event/file_event_impl_test.cc b/test/common/event/file_event_impl_test.cc index d3a842ada010..0f1c7194eb72 100644 --- a/test/common/event/file_event_impl_test.cc +++ b/test/common/event/file_event_impl_test.cc @@ -8,7 +8,6 @@ #include "test/mocks/common.h" #include "test/test_common/environment.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -72,7 +71,6 @@ class FileEventImplActivateTest : public testing::TestWithParammergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.enable_grpc_async_client_cache", "false"}}); envoy::config::core::v3::GrpcService grpc_service; grpc_service.mutable_envoy_grpc()->set_cluster_name("foo"); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index b96f2056d1d2..4ccaddffb23a 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -240,8 +240,7 @@ TEST_F(ConnectionManagerUtilityTest, UseRemoteAddressWhenNotLocalHostRemoteAddre TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfUrlInvalid) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -254,8 +253,7 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfUrlInvalid) { TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfMultipleEntriesAreFound) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -269,8 +267,7 @@ TEST_F(ConnectionManagerUtilityTest, RemoveRefererIfMultipleEntriesAreFound) { TEST_F(ConnectionManagerUtilityTest, ValidRefererPassesSanitization) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.sanitize_http_header_referer", "true"}}); connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress( std::make_shared("10.0.0.1")); @@ -1970,7 +1967,7 @@ TEST_F(ConnectionManagerUtilityTest, RejectPathWithFragmentByDefault) { TEST_F(ConnectionManagerUtilityTest, DropFragmentFromPathWithOverride) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}}); TestRequestHeaderMapImpl header_map{{":path", "/foo/bar#boom"}}; @@ -2003,9 +2000,9 @@ TEST_F(ConnectionManagerUtilityTest, DropFragmentFromPathWithOverride) { TEST_F(ConnectionManagerUtilityTest, KeepFragmentFromPathWithBothOverrides) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}}); - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.http_strip_fragment_from_path_unsafe_if_disabled", "false"}}); TestRequestHeaderMapImpl header_map{{":path", "/foo/bar#boom"}}; diff --git a/test/common/http/header_map_impl_fuzz_test.cc b/test/common/http/header_map_impl_fuzz_test.cc index d769eb225879..62fc452a420d 100644 --- a/test/common/http/header_map_impl_fuzz_test.cc +++ b/test/common/http/header_map_impl_fuzz_test.cc @@ -17,12 +17,11 @@ namespace Envoy { // Fuzz the header map implementation. DEFINE_PROTO_FUZZER(const test::common::http::HeaderMapImplFuzzTestCase& input) { - TestScopedRuntime runtime; + TestScopedRuntime scoped_runtime; // Set the lazy header-map threshold if found. if (input.has_config()) { - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.http.headermap.lazy_map_min_size", - absl::StrCat(input.config().lazy_map_min_size())}}); + scoped_runtime.mergeValues({{"envoy.http.headermap.lazy_map_min_size", + absl::StrCat(input.config().lazy_map_min_size())}}); } auto header_map = Http::RequestHeaderMapImpl::create(); diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 1b95f4b49ad1..57561ec98ead 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -372,7 +372,7 @@ class HeaderMapImplTest : public testing::TestWithParam { public: HeaderMapImplTest() { // Set the lazy map threshold using the test parameter. - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_.mergeValues( {{"envoy.http.headermap.lazy_map_min_size", absl::StrCat(GetParam())}}); } @@ -380,7 +380,7 @@ class HeaderMapImplTest : public testing::TestWithParam { return absl::StrCat(params.param); } - TestScopedRuntime runtime; + TestScopedRuntime scoped_runtime_; }; INSTANTIATE_TEST_SUITE_P(HeaderMapThreshold, HeaderMapImplTest, diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 825b6431c335..36726e28f325 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -1118,10 +1118,6 @@ TEST_F(Http1ServerConnectionImplTest, HostHeaderTranslation) { // Ensures that requests with invalid HTTP header values are properly rejected // when the runtime guard is enabled for the feature. TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) { - TestScopedRuntime scoped_runtime; - // When the runtime-guarded feature is enabled, invalid header values - // should result in a rejection. - initialize(); MockRequestDecoder decoder; @@ -1211,8 +1207,6 @@ TEST_F(Http1ServerConnectionImplTest, HeaderNameWithUnderscoreCauseRequestReject } TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) { - TestScopedRuntime scoped_runtime; - initialize(); MockRequestDecoder decoder; diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index 9e186ed3e924..97baf19266e6 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -1443,7 +1443,6 @@ TEST_F(Http2ConnPoolImplTest, PreconnectWithoutMultiplexing) { } TEST_F(Http2ConnPoolImplTest, IncreaseCapacityWithSettingsFrame) { - TestScopedRuntime scoped_runtime; cluster_->http2_options_.mutable_max_concurrent_streams()->set_value(100); ON_CALL(*cluster_, perUpstreamPreconnectRatio).WillByDefault(Return(1)); @@ -1486,7 +1485,6 @@ TEST_F(Http2ConnPoolImplTest, IncreaseCapacityWithSettingsFrame) { } TEST_F(Http2ConnPoolImplTest, DisconnectWithNegativeCapacity) { - TestScopedRuntime scoped_runtime; cluster_->http2_options_.mutable_max_concurrent_streams()->set_value(6); ON_CALL(*cluster_, perUpstreamPreconnectRatio).WillByDefault(Return(1)); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 344f14f4ab35..ef046c48d51c 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -3030,9 +3030,8 @@ class InternalClientConnectionImplTest : public testing::Test { TEST_F(InternalClientConnectionImplTest, CannotCreateConnectionToInternalAddressWithInternalAddressEnabled) { - auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.internal_address", "true"}}); + auto runtime = std::make_unique(); + runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}}); const Network::SocketInterface* sock_interface = Network::socketInterface( "envoy.extensions.network.socket_interface.default_socket_interface"); diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 15beded05f3b..fde37f55c4d0 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -124,8 +124,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { // Required to manipulate runtime values when there is no test server. TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"overload.global_downstream_max_connections", "2"}}); + scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "2"}}); auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; @@ -161,8 +160,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { EXPECT_EQ(2, server_connections.size()); // Let's increase the allowed connections and try sending more connections. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"overload.global_downstream_max_connections", "3"}}); + scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "3"}}); initiate_connections(5); EXPECT_CALL(listener_callbacks, onReject(TcpListenerCallbacks::RejectCause::GlobalCxLimit)) .Times(4); @@ -171,8 +169,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { EXPECT_EQ(3, server_connections.size()); // Clear the limit and verify there's no longer a limit. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"overload.global_downstream_max_connections", ""}}); + scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", ""}}); initiate_connections(10); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -185,16 +182,14 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { conn->close(ConnectionCloseType::NoFlush); } - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"overload.global_downstream_max_connections", ""}}); + scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", ""}}); } TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { // Required to manipulate runtime values when there is no test server. TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"overload.global_downstream_max_connections", "1"}}); + scoped_runtime.mergeValues({{"overload.global_downstream_max_connections", "1"}}); auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 644a92d57312..7d259f621c86 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -1725,7 +1725,7 @@ TEST_F(DeprecatedFieldsTest, IndividualFieldDeprecatedEmitsCrash) { base.set_is_deprecated("foo"); // Non-fatal checks for a deprecated field should throw an exception if the // runtime flag is enabled.. - Runtime::LoaderSingleton::getExisting()->mergeValues({ + mergeValues({ {"envoy.features.fail_on_any_deprecated_feature", "true"}, }); EXPECT_THROW_WITH_REGEX( @@ -1756,9 +1756,8 @@ TEST_F(DeprecatedFieldsTest, // The config will be rejected, so the feature will not be used. // Now create a new snapshot with this feature allowed. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated_fatal", - "True "}}); + mergeValues({{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated_fatal", + "True "}}); // Now the same deprecation check should only trigger a warning. EXPECT_LOG_CONTAINS( @@ -1780,8 +1779,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(IndividualFieldDisallowedWi // The config will be rejected, so the feature will not be used. // Now create a new snapshot with this all features allowed. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.features.enable_all_deprecated_features", "true"}}); + mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}}); // Now the same deprecation check should only trigger a warning. EXPECT_LOG_CONTAINS( @@ -1800,7 +1798,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) { checkForDeprecation(base)); // Now create a new snapshot with this feature disallowed. - Runtime::LoaderSingleton::getExisting()->mergeValues( + mergeValues( {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated", " false"}}); EXPECT_THROW_WITH_REGEX( @@ -1809,8 +1807,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) { // Verify that even when the enable_all_deprecated_features is enabled the // feature is disallowed. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.features.enable_all_deprecated_features", "true"}}); + mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}}); EXPECT_THROW_WITH_REGEX( checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException, @@ -1915,7 +1912,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault) envoy::test::deprecation_test::Base base; base.mutable_enum_container(); - Runtime::LoaderSingleton::getExisting()->mergeValues( + mergeValues( {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_DEFAULT", "false"}}); // Make sure this is set up right. @@ -1925,8 +1922,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault) // Verify that even when the enable_all_deprecated_features is enabled the // enum is disallowed. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.features.enable_all_deprecated_features", "true"}}); + mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}}); EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), Envoy::ProtobufMessage::DeprecatedProtoFieldException, @@ -1942,7 +1938,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnum)) { Envoy::ProtobufMessage::DeprecatedProtoFieldException, "Using deprecated value DEPRECATED_FATAL"); - Runtime::LoaderSingleton::getExisting()->mergeValues( + mergeValues( {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_FATAL", "true"}}); EXPECT_LOG_CONTAINS( @@ -1963,8 +1959,7 @@ TEST_F(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnumGlobalOverride)) { Envoy::ProtobufMessage::DeprecatedProtoFieldException, "Using deprecated value DEPRECATED_FATAL"); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.features.enable_all_deprecated_features", "true"}}); + mergeValues({{"envoy.features.enable_all_deprecated_features", "true"}}); EXPECT_LOG_CONTAINS( "warning", diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index dd74cba502a9..4a8f08bf4864 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -4324,7 +4324,7 @@ TEST_F(RouterTest, InternalRedirectStripsFragment) { TEST_F(RouterTest, InternalRedirectKeepsFragmentWithOveride) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.http_reject_path_with_fragment", "false"}}); enableRedirects(); default_request_headers_.setForwardedProto("http"); @@ -5914,7 +5914,7 @@ TEST_F(RouterTest, ExpectedUpstreamTimeoutUpdatedDuringRetries) { TEST_F(RouterTest, ExpectedUpstreamTimeoutNotUpdatedDuringRetriesWhenRuntimeGuardDisabled) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.update_expected_rq_timeout_on_retry", "false"}}); auto retry_options_predicate = std::make_shared(); diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index cfec77e8f83f..bbe5a9d7fd23 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -5380,8 +5380,7 @@ TEST_F(ClusterManagerImplTest, ConnPoolsNotDrainedOnHostSetChange) { TEST_F(ClusterManagerImplTest, ConnPoolsIdleDeleted) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.conn_pool_delete_when_idle", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.conn_pool_delete_when_idle", "true"}}); const std::string yaml = R"EOF( static_resources: diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index c3d6a43e27ef..64a2ebffd463 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -45,6 +45,7 @@ class EdsSpeedTest { type_url_("type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"), subscription_stats_(Config::Utility::generateStats(stats_)), api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()) { + TestDeprecatedV2Api::allowDeprecatedV2(); if (use_unified_mux_) { grpc_mux_.reset(new Config::XdsMux::GrpcMuxSotw( std::unique_ptr(async_client_), dispatcher_, @@ -150,7 +151,6 @@ class EdsSpeedTest { num_hosts); } - TestDeprecatedV2Api _deprecated_v2_api_; State& state_; bool use_unified_mux_; const std::string type_url_; diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index c57da0bb13e1..863901a2f9e6 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -127,7 +127,7 @@ class EdsTest : public testing::Test { Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, stats_, singleton_manager_, tls_, validation_visitor_, *api_, options_); - cluster_ = std::make_shared(eds_cluster_, runtime_, factory_context, + cluster_ = std::make_shared(eds_cluster_, runtime_.loader(), factory_context, std::move(scope), false); EXPECT_EQ(initialize_phase, cluster_->initializePhase()); eds_callbacks_ = cm_.subscription_factory_.callbacks_; @@ -154,7 +154,7 @@ class EdsTest : public testing::Test { EdsClusterImplSharedPtr cluster_; Config::SubscriptionCallbacks* eds_callbacks_{}; NiceMock random_; - NiceMock runtime_; + TestScopedRuntime runtime_; NiceMock local_info_; NiceMock admin_; Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()}; @@ -1520,8 +1520,7 @@ TEST_F(EdsTest, EndpointLocalityUpdated) { // Unlike EndpointLocalityUpdated, runtime feature flag is disabled this time and then it is // verified that locality update does not happen on eds cluster endpoints. TEST_F(EdsTest, EndpointLocalityNotUpdatedIfFixDisabled) { - TestScopedRuntime runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + runtime_.mergeValues( {{"envoy.reloadable_features.support_locality_update_on_eds_cluster_endpoints", "false"}}); envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare"); @@ -1579,7 +1578,6 @@ TEST_F(EdsTest, EndpointLocalityNotUpdatedIfFixDisabled) { EXPECT_EQ("hello", locality.zone()); EXPECT_EQ("world", locality.sub_zone()); } - Runtime::LoaderSingleton::clear(); } // Validate that onConfigUpdate() does not propagate locality weights to the host set when diff --git a/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc b/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc index c53b7a8342ce..f836716e882c 100644 --- a/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc +++ b/test/extensions/common/dynamic_forward_proxy/dns_cache_impl_test.cc @@ -1182,8 +1182,7 @@ TEST(UtilityTest, PrepareDnsRefreshStrategy) { TEST_F(DnsCacheImplTest, ResolveSuccessWithCaching) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); auto* time_source = new NiceMock(); context_.dispatcher_.time_system_.reset(time_source); @@ -1299,8 +1298,7 @@ TEST_F(DnsCacheImplTest, ResolveSuccessWithCaching) { TEST_F(DnsCacheImplTest, CacheLoad) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.allow_multiple_dns_addresses", "true"}}); auto* time_source = new NiceMock(); context_.dispatcher_.time_system_.reset(time_source); diff --git a/test/extensions/filters/http/buffer/buffer_filter_test.cc b/test/extensions/filters/http/buffer/buffer_filter_test.cc index fd672c624299..85d3976b19ca 100644 --- a/test/extensions/filters/http/buffer/buffer_filter_test.cc +++ b/test/extensions/filters/http/buffer/buffer_filter_test.cc @@ -11,7 +11,6 @@ #include "test/mocks/buffer/mocks.h" #include "test/mocks/http/mocks.h" #include "test/test_common/printers.h" -#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -40,8 +39,6 @@ class BufferFilterTest : public testing::Test { NiceMock callbacks_; BufferFilterConfigSharedPtr config_; BufferFilter filter_; - // Create a runtime loader, so that tests can manually manipulate runtime guarded features. - TestScopedRuntime scoped_runtime; }; TEST_F(BufferFilterTest, HeaderOnlyRequest) { diff --git a/test/extensions/filters/http/ext_authz/ext_authz_test.cc b/test/extensions/filters/http/ext_authz/ext_authz_test.cc index 314a1c9f328a..00591fcebfa7 100644 --- a/test/extensions/filters/http/ext_authz/ext_authz_test.cc +++ b/test/extensions/filters/http/ext_authz/ext_authz_test.cc @@ -1691,7 +1691,7 @@ TEST_P(HttpFilterTestParam, NoRoute) { // `envoy.reloadable_features.http_ext_authz_do_not_skip_direct_response_and_redirect` is false. TEST_P(HttpFilterTestParam, NoRouteWithSkipAuth) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.http_ext_authz_do_not_skip_direct_response_and_redirect", "false"}}); EXPECT_CALL(*decoder_filter_callbacks_.route_, routeEntry()).WillOnce(Return(nullptr)); diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index b6652a14964d..c867272aec80 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -6,7 +6,6 @@ #include "test/extensions/common/wasm/wasm_runtime.h" #include "test/mocks/network/connection.h" #include "test/mocks/router/mocks.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/wasm_base.h" using testing::Eq; @@ -940,7 +939,6 @@ TEST_P(WasmHttpFilterTest, GrpcCall) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1018,7 +1016,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallBadCall) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1060,7 +1057,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallFailure) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1150,7 +1146,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallCancel) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1209,7 +1204,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallClose) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1268,7 +1262,6 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) { proto_or_cluster.push_back("grpc_call_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; setupTest(id); setupFilter(); @@ -1372,7 +1365,6 @@ TEST_P(WasmHttpFilterTest, GrpcStream) { proto_or_cluster.push_back("grpc_stream_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; Grpc::RawAsyncStreamCallbacks* callbacks = nullptr; setupGrpcStreamTest(callbacks, id); @@ -1434,7 +1426,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseLocal) { proto_or_cluster.push_back("grpc_stream_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; Grpc::RawAsyncStreamCallbacks* callbacks = nullptr; setupGrpcStreamTest(callbacks, id); @@ -1495,7 +1486,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseRemote) { proto_or_cluster.push_back("grpc_stream_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; Grpc::RawAsyncStreamCallbacks* callbacks = nullptr; setupGrpcStreamTest(callbacks, id); @@ -1555,7 +1545,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCancel) { proto_or_cluster.push_back("grpc_stream_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; Grpc::RawAsyncStreamCallbacks* callbacks = nullptr; setupGrpcStreamTest(callbacks, id); @@ -1606,7 +1595,6 @@ TEST_P(WasmHttpFilterTest, GrpcStreamOpenAtShutdown) { proto_or_cluster.push_back("grpc_stream_proto"); } for (const auto& id : proto_or_cluster) { - TestScopedRuntime scoped_runtime; Grpc::RawAsyncStreamCallbacks* callbacks = nullptr; setupGrpcStreamTest(callbacks, id); diff --git a/test/extensions/filters/network/common/redis/fault_test.cc b/test/extensions/filters/network/common/redis/fault_test.cc index da80a05d7a9f..f8c67a2130b7 100644 --- a/test/extensions/filters/network/common/redis/fault_test.cc +++ b/test/extensions/filters/network/common/redis/fault_test.cc @@ -92,7 +92,6 @@ TEST_F(FaultTest, NoFaults) { RedisProxy redis_config; auto* faults = redis_config.mutable_faults(); - TestScopedRuntime scoped_runtime; FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults); const Fault* fault_ptr = fault_manager.getFaultForCommand("get"); @@ -104,7 +103,6 @@ TEST_F(FaultTest, SingleCommandFaultNotEnabled) { auto* faults = redis_config.mutable_faults(); createCommandFault(faults->Add(), "get", 0, 0, FractionalPercent::HUNDRED, RUNTIME_KEY); - TestScopedRuntime scoped_runtime; FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults); EXPECT_CALL(random_, random()).WillOnce(Return(0)); @@ -120,7 +118,6 @@ TEST_F(FaultTest, SingleCommandFault) { auto* faults = redis_config.mutable_faults(); createCommandFault(faults->Add(), "ttl", 0, 5000, FractionalPercent::TEN_THOUSAND, RUNTIME_KEY); - TestScopedRuntime scoped_runtime; FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults); EXPECT_CALL(random_, random()).WillOnce(Return(1)); @@ -136,7 +133,6 @@ TEST_F(FaultTest, SingleCommandFaultWithNoDefaultValueOrRuntimeValue) { auto* faults = redis_config.mutable_faults(); createCommandFault(faults->Add(), "ttl", 0, absl::nullopt, absl::nullopt, absl::nullopt); - TestScopedRuntime scoped_runtime; FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults); EXPECT_CALL(random_, random()).WillOnce(Return(1)); @@ -154,7 +150,6 @@ TEST_F(FaultTest, MultipleFaults) { createCommandFault(faults->Add(), "get", 0, 25, FractionalPercent::HUNDRED, RUNTIME_KEY); createAllKeyFault(faults->Add(), 2, 25, FractionalPercent::HUNDRED, absl::nullopt); - TestScopedRuntime scoped_runtime; FaultManagerImpl fault_manager = FaultManagerImpl(random_, runtime_, *faults); const Fault* fault_ptr; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 3313b1118166..9f86c5c76e42 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -982,7 +982,7 @@ TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListener) { } TEST_F(ConnectionHandlerTest, EnsureNotMatchStoppedAnyAddressListenerOnOldBehavior) { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_.mergeValues( {{"envoy.reloadable_features.listener_wildcard_match_ip_family", "false"}}); Network::TcpListenerCallbacks* listener_callbacks; @@ -1103,7 +1103,7 @@ TEST_F(ConnectionHandlerTest, FallbackToWildcardListener) { } TEST_F(ConnectionHandlerTest, OldBehaviorWildcardListener) { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_.mergeValues( {{"envoy.reloadable_features.listener_wildcard_match_ip_family", "false"}}); Network::TcpListenerCallbacks* listener_callbacks1; diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 2f9c18052998..5d67dca2f1da 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -547,10 +547,9 @@ bind_to_port: false } TEST_F(ListenerManagerImplTest, UnsupportedInternalListener) { - auto scoped_runtime_guard = std::make_unique(); + auto scoped_runtime = std::make_unique(); // Workaround of triggering death at windows platform. - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.internal_address", "false"}}); + scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "false"}}); const std::string yaml = R"EOF( name: "foo" @@ -565,9 +564,8 @@ TEST_F(ListenerManagerImplTest, UnsupportedInternalListener) { } TEST_F(ListenerManagerImplTest, RejectListenerWithSocketAddressWithInternalListenerConfig) { - auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.internal_address", "true"}}); + auto scoped_runtime = std::make_unique(); + scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}}); const std::string yaml = R"EOF( name: "foo" @@ -587,9 +585,8 @@ TEST_F(ListenerManagerImplTest, RejectListenerWithSocketAddressWithInternalListe } TEST_F(ListenerManagerImplTest, RejectTcpOptionsWithInternalListenerConfig) { - auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.internal_address", "true"}}); + auto scoped_runtime = std::make_unique(); + scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}}); const std::string yaml = R"EOF( name: "foo" @@ -4874,9 +4871,8 @@ name: test_api_listener_2 } TEST_F(ListenerManagerImplWithRealFiltersTest, AddOrUpdateInternalListener) { - auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.internal_address", "true"}}); + auto scoped_runtime = std::make_unique(); + scoped_runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}}); time_system_.setSystemTime(std::chrono::milliseconds(1001001001001)); InSequence s; diff --git a/test/test_common/simulated_time_system_test.cc b/test/test_common/simulated_time_system_test.cc index 422669b0725e..03720a5a767b 100644 --- a/test/test_common/simulated_time_system_test.cc +++ b/test/test_common/simulated_time_system_test.cc @@ -6,7 +6,6 @@ #include "test/mocks/common.h" #include "test/mocks/event/mocks.h" #include "test/test_common/simulated_time_system.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "event2/event.h" @@ -59,7 +58,6 @@ class SimulatedTimeSystemTest : public testing::Test { base_scheduler_.run(Dispatcher::RunType::NonBlock); } - TestScopedRuntime scoped_runtime_; Event::MockDispatcher dispatcher_; LibeventScheduler base_scheduler_; SimulatedTimeSystem time_system_; diff --git a/test/test_common/test_runtime.h b/test/test_common/test_runtime.h index a55c526b0683..d763309c3be7 100644 --- a/test/test_common/test_runtime.h +++ b/test/test_common/test_runtime.h @@ -3,11 +3,8 @@ // As long as this class is in scope one can do runtime feature overrides: // // TestScopedRuntime scoped_runtime; -// Runtime::LoaderSingleton::getExisting()->mergeValues( +// scoped_runtime.mergeValues( // {{"envoy.reloadable_features.test_feature_true", "false"}}); -// -// As long as a TestScopedRuntime exists, Runtime::LoaderSingleton::getExisting()->mergeValues() -// can safely be called to override runtime values. #pragma once @@ -42,6 +39,10 @@ class TestScopedRuntime { Runtime::Loader& loader() { return *Runtime::LoaderSingleton::getExisting(); } + void mergeValues(const absl::node_hash_map& values) { + loader().mergeValues(values); + } + ~TestScopedRuntime() { Runtime::RuntimeFeatures features; features.restoreDefaults(); @@ -60,7 +61,8 @@ class TestScopedRuntime { class TestDeprecatedV2Api : public TestScopedRuntime { public: - TestDeprecatedV2Api() { + TestDeprecatedV2Api() { allowDeprecatedV2(); } + static void allowDeprecatedV2() { Runtime::LoaderSingleton::getExisting()->mergeValues({ {"envoy.test_only.broken_in_production.enable_deprecated_v2_api", "true"}, {"envoy.features.enable_all_deprecated_features", "true"},