Skip to content

Commit

Permalink
test: removing a bunch of direct runtime singleton access (envoyproxy…
Browse files Browse the repository at this point in the history
…#19993)

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>
  • Loading branch information
alyssawilk authored and rexnp committed Mar 2, 2022
1 parent f176746 commit fdec215
Show file tree
Hide file tree
Showing 29 changed files with 68 additions and 133 deletions.
7 changes: 2 additions & 5 deletions test/benchmark/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestScopedRuntime> 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<TestScopedRuntime>();
}
// Make sure the argument contains a single ":" character.
const std::vector<std::string> runtime_feature_split = absl::StrSplit(runtime_feature_arg, ':');
if (runtime_feature_split.size() != 2) {
Expand All @@ -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) {
Expand Down
9 changes: 3 additions & 6 deletions test/common/common/dns_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Network::DnsResponse> responses =
TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"});
Expand All @@ -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<Network::DnsResponse> responses =
TestUtility::makeDnsResponse({"10.0.0.1", "10.0.0.2"});
Expand All @@ -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");
Expand Down
9 changes: 3 additions & 6 deletions test/common/common/regex_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions test/common/config/delta_subscription_state_old_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Envoy::Config::DeltaSubscriptionState>(type_url, callbacks_,
Expand Down
3 changes: 1 addition & 2 deletions test/common/config/subscription_factory_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"}});
}
}

Expand Down
3 changes: 1 addition & 2 deletions test/common/event/dispatcher_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -907,7 +907,6 @@ class TimerImplTest : public testing::Test {
requested_advance_ = absl::ZeroDuration();
}

TestScopedRuntime scoped_runtime_;
absl::Duration requested_advance_ = absl::ZeroDuration();
std::vector<std::function<void()>> check_callbacks_;
bool in_event_loop_{};
Expand Down
2 changes: 0 additions & 2 deletions test/common/event/file_event_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -72,7 +71,6 @@ class FileEventImplActivateTest : public testing::TestWithParam<Network::Address

protected:
Api::OsSysCalls& os_sys_calls_;
TestScopedRuntime scoped_runtime_;
};

INSTANTIATE_TEST_SUITE_P(IpVersions, FileEventImplActivateTest,
Expand Down
2 changes: 1 addition & 1 deletion test/common/grpc/async_client_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcOk) {

TEST_F(AsyncClientManagerImplTest, DisableRawAsyncClientCache) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
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");
Expand Down
15 changes: 6 additions & 9 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Network::Address::Ipv4Instance>("10.0.0.1"));
Expand All @@ -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<Network::Address::Ipv4Instance>("10.0.0.1"));
Expand All @@ -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<Network::Address::Ipv4Instance>("10.0.0.1"));
Expand Down Expand Up @@ -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"}};
Expand Down Expand Up @@ -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"}};
Expand Down
7 changes: 3 additions & 4 deletions test/common/http/header_map_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions test/common/http/header_map_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,15 +372,15 @@ class HeaderMapImplTest : public testing::TestWithParam<uint32_t> {
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())}});
}

static std::string testParamsToString(const ::testing::TestParamInfo<uint32_t>& params) {
return absl::StrCat(params.param);
}

TestScopedRuntime runtime;
TestScopedRuntime scoped_runtime_;
};

INSTANTIATE_TEST_SUITE_P(HeaderMapThreshold, HeaderMapImplTest,
Expand Down
6 changes: 0 additions & 6 deletions test/common/http/http1/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1211,8 +1207,6 @@ TEST_F(Http1ServerConnectionImplTest, HeaderNameWithUnderscoreCauseRequestReject
}

TEST_F(Http1ServerConnectionImplTest, HeaderInvalidAuthority) {
TestScopedRuntime scoped_runtime;

initialize();

MockRequestDecoder decoder;
Expand Down
2 changes: 0 additions & 2 deletions test/common/http/http2/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down
5 changes: 2 additions & 3 deletions test/common/network/connection_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3030,9 +3030,8 @@ class InternalClientConnectionImplTest : public testing::Test {

TEST_F(InternalClientConnectionImplTest,
CannotCreateConnectionToInternalAddressWithInternalAddressEnabled) {
auto scoped_runtime_guard = std::make_unique<TestScopedRuntime>();
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.internal_address", "true"}});
auto runtime = std::make_unique<TestScopedRuntime>();
runtime->mergeValues({{"envoy.reloadable_features.internal_address", "true"}});

const Network::SocketInterface* sock_interface = Network::socketInterface(
"envoy.extensions.network.socket_interface.default_socket_interface");
Expand Down
15 changes: 5 additions & 10 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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::TcpListenSocketImmediateListen>(
Network::Test::getCanonicalLoopbackAddress(version_));
Network::MockTcpListenerCallbacks listener_callbacks;
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand All @@ -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::TcpListenSocketImmediateListen>(
Network::Test::getCanonicalLoopbackAddress(version_));
Network::MockTcpListenerCallbacks listener_callbacks;
Expand Down
Loading

0 comments on commit fdec215

Please sign in to comment.