From 1c7b8e6729b5a785697477d60448bb08fc01909d Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 8 Jul 2024 12:54:54 +0000 Subject: [PATCH 1/3] Envoy Mobile: making exception-free build the default Signed-off-by: Alyssa Wilk --- .github/config.yml | 31 ------- .../workflows/mobile-compile_time_options.yml | 84 ------------------- bazel/envoy_build_system.bzl | 2 + bazel/envoy_select.bzl | 7 ++ .../generic_proxy/filters/network/test/BUILD | 1 + mobile/.bazelrc | 3 + .../filters/http/platform_bridge/BUILD | 2 +- .../stat_sinks/metrics_service/BUILD | 2 +- mobile/test/common/integration/BUILD | 2 +- .../filter_chain_manager_impl.cc | 12 +-- source/common/listener_manager/lds_api.cc | 4 +- .../common/listener_manager/listener_impl.cc | 81 ++++++++++-------- .../listener_manager/listener_manager_impl.cc | 51 +++++------ source/common/network/listen_socket_impl.cc | 8 ++ .../quic_server_transport_socket_factory.cc | 6 +- source/common/tls/server_context_impl.cc | 12 +-- source/server/overload_manager_impl.cc | 32 +++---- test/config_test/config_test.cc | 15 ++-- .../filters/listener/local_ratelimit/BUILD | 1 + test/integration/BUILD | 84 +++++++++++++------ test/integration/base_integration_test.cc | 8 +- test/integration/server.cc | 14 ++-- test/integration/server.h | 10 +-- test/mocks/runtime/mocks.h | 4 +- test/test_common/BUILD | 5 +- test/test_common/environment.cc | 20 +++++ test/test_common/network_utility.cc | 2 +- 27 files changed, 242 insertions(+), 261 deletions(-) delete mode 100644 .github/workflows/mobile-compile_time_options.yml diff --git a/.github/config.yml b/.github/config.yml index 189bbc77eae1..24f45f1b1dfb 100644 --- a/.github/config.yml +++ b/.github/config.yml @@ -48,11 +48,6 @@ checks: required: true on-run: - mobile-cc - mobile-compile-time-options: - name: Mobile/Compile time options - on-run: - - mobile-compile-time-cc - - mobile-compile-time-options mobile-coverage: name: Mobile/Coverage required: true @@ -217,32 +212,6 @@ run: - test/integration/* - test/mocks/**/* - test/test_common/**/* - mobile-compile-time-cc: - paths: - - .bazelrc - - .bazelversion - - .github/config.yml - - api/**/* - - bazel/external/quiche.BUILD - - bazel/repository_locations.bzl - - envoy/**/* - - mobile/.bazelrc - - mobile/**/* - - source/**/* - - test/config/**/* - - test/integration/* - - test/mocks/**/* - - test/test_common/**/* - mobile-compile-time-options: - paths: - - .bazelrc - - .bazelversion - - .github/config.yml - - bazel/external/quiche.BUILD - - bazel/repository_locations.bzl - - mobile/.bazelrc - - mobile/**/* - - tools/code_format/check_format.py mobile-coverage: paths: - .bazelrc diff --git a/.github/workflows/mobile-compile_time_options.yml b/.github/workflows/mobile-compile_time_options.yml deleted file mode 100644 index 6ad3c5b7ead1..000000000000 --- a/.github/workflows/mobile-compile_time_options.yml +++ /dev/null @@ -1,84 +0,0 @@ -name: Mobile/Compile time options - -permissions: - contents: read - -on: - workflow_run: - workflows: - - Request - types: - - completed - -concurrency: - group: ${{ github.head_ref || github.run_id }}-${{ github.workflow }} - cancel-in-progress: true - - -jobs: - load: - secrets: - app-key: ${{ secrets.ENVOY_CI_APP_KEY }} - app-id: ${{ secrets.ENVOY_CI_APP_ID }} - lock-app-key: ${{ secrets.ENVOY_CI_MUTEX_APP_KEY }} - lock-app-id: ${{ secrets.ENVOY_CI_MUTEX_APP_ID }} - permissions: - actions: read - contents: read - packages: read - pull-requests: read - if: ${{ github.event.workflow_run.conclusion == 'success' }} - uses: ./.github/workflows/_load.yml - with: - check-name: mobile-compile-time-options - - cc: - permissions: - contents: read - packages: read - uses: ./.github/workflows/_mobile_container_ci.yml - if: ${{ fromJSON(needs.load.outputs.request).run.mobile-compile-time-cc }} - needs: load - with: - args: ${{ matrix.args }} - command: ./bazelw - entrypoint: ${{ matrix.entrypoint }} - request: ${{ needs.load.outputs.request }} - target: ${{ matrix.target }} - timeout-minutes: 120 - strategy: - fail-fast: false - matrix: - include: - - name: Running C++ build with exceptions disabled - target: cc-no-build-exceptions - args: >- - build - --config=mobile-remote-ci-cc-no-exceptions - //test/performance:test_binary_size //library/cc/... - - name: Running C++ tests with full protos enabled - target: cc-tests-full-protos-enabled - args: >- - test - --config=mobile-remote-ci-cc-full-protos-enabled - //test/common/... //test/cc/... - - request: - secrets: - app-id: ${{ secrets.ENVOY_CI_APP_ID }} - app-key: ${{ secrets.ENVOY_CI_APP_KEY }} - permissions: - actions: read - contents: read - pull-requests: read - if: >- - ${{ always() - && github.event.workflow_run.conclusion == 'success' - && (fromJSON(needs.load.outputs.request).run.mobile-compile-time-options - || fromJSON(needs.load.outputs.request).run.mobile-compile-time-cc) }} - needs: - - load - - cc - uses: ./.github/workflows/_finish.yml - with: - needs: ${{ toJSON(needs) }} diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index 6aa07fe19544..4a373b5f2687 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -35,6 +35,7 @@ load( _envoy_select_boringssl = "envoy_select_boringssl", _envoy_select_disable_exceptions = "envoy_select_disable_exceptions", _envoy_select_disable_logging = "envoy_select_disable_logging", + _envoy_select_enable_exceptions = "envoy_select_enable_exceptions", _envoy_select_enable_http3 = "envoy_select_enable_http3", _envoy_select_enable_http_datagrams = "envoy_select_enable_http_datagrams", _envoy_select_enable_yaml = "envoy_select_enable_yaml", @@ -239,6 +240,7 @@ envoy_select_google_grpc = _envoy_select_google_grpc envoy_select_enable_http3 = _envoy_select_enable_http3 envoy_select_enable_yaml = _envoy_select_enable_yaml envoy_select_disable_exceptions = _envoy_select_disable_exceptions +envoy_select_enable_exceptions = _envoy_select_enable_exceptions envoy_select_hot_restart = _envoy_select_hot_restart envoy_select_enable_http_datagrams = _envoy_select_enable_http_datagrams envoy_select_signal_trace = _envoy_select_signal_trace diff --git a/bazel/envoy_select.bzl b/bazel/envoy_select.bzl index d4190d0a0b70..549794a392aa 100644 --- a/bazel/envoy_select.bzl +++ b/bazel/envoy_select.bzl @@ -87,6 +87,13 @@ def envoy_select_disable_exceptions(xs, repository = ""): "//conditions:default": [], }) +# Selects the given values if exceptions are enabled in the current build. +def envoy_select_enable_exceptions(xs, repository = ""): + return select({ + repository + "//bazel:disable_exceptions": [], + "//conditions:default": xs, + }) + # Selects the given values if HTTP datagram support is enabled in the current build. def envoy_select_enable_http_datagrams(xs, repository = ""): return select({ diff --git a/contrib/generic_proxy/filters/network/test/BUILD b/contrib/generic_proxy/filters/network/test/BUILD index e57a54439024..9c6e7e566c6e 100644 --- a/contrib/generic_proxy/filters/network/test/BUILD +++ b/contrib/generic_proxy/filters/network/test/BUILD @@ -103,6 +103,7 @@ envoy_cc_test( "//source/extensions/transport_sockets/raw_buffer:config", "//test/common/upstream:utility_lib", "//test/integration:base_integration_test_lib", + "//test/integration:common_extensions_lib", "//test/mocks/server:factory_context_mocks", "//test/mocks/upstream:cluster_info_mocks", "//test/test_common:registry_lib", diff --git a/mobile/.bazelrc b/mobile/.bazelrc index 27f0a09ba005..b99513a0b26c 100644 --- a/mobile/.bazelrc +++ b/mobile/.bazelrc @@ -31,6 +31,8 @@ build --experimental_repository_downloader_retries=2 build --define=google_grpc=disabled build --define=envoy_yaml=disabled build --define=envoy_full_protos=disabled +build --define envoy_exceptions=disabled +build: --copt=-fno-exceptions # We don't have a ton of Swift in Envoy Mobile, so always build with WMO # This also helps work around a bug in rules_swift: https://github.com/bazelbuild/rules_swift/issues/949 @@ -242,6 +244,7 @@ test:mobile-remote-ci-android --config=mobile-test-android build:mobile-remote-ci-cc --config=mobile-remote-ci test:mobile-remote-ci-cc --action_env=LD_LIBRARY_PATH +# TODO(alyssar) remove in a follow-up PR build:mobile-remote-ci-cc-no-exceptions --config=mobile-remote-ci-cc build:mobile-remote-ci-cc-no-exceptions --define envoy_exceptions=disabled build:mobile-remote-ci-cc-no-exceptions --copt=-fno-exceptions diff --git a/mobile/test/common/extensions/filters/http/platform_bridge/BUILD b/mobile/test/common/extensions/filters/http/platform_bridge/BUILD index ecec01f8d2b1..846f182f30c0 100644 --- a/mobile/test/common/extensions/filters/http/platform_bridge/BUILD +++ b/mobile/test/common/extensions/filters/http/platform_bridge/BUILD @@ -36,7 +36,7 @@ envoy_extension_cc_test( "//library/common/api:external_api_lib", "//library/common/extensions/filters/http/platform_bridge:config", "//library/common/extensions/filters/http/platform_bridge:filter_cc_proto", - "@envoy//test/integration:http_integration_lib", + "@envoy//test/integration:http_integration_lib_light", "@envoy//test/mocks/server:factory_context_mocks", "@envoy//test/test_common:simulated_time_system_lib", "@envoy//test/test_common:utility_lib", diff --git a/mobile/test/common/extensions/stat_sinks/metrics_service/BUILD b/mobile/test/common/extensions/stat_sinks/metrics_service/BUILD index aa4a795148a8..0e23af06dbb1 100644 --- a/mobile/test/common/extensions/stat_sinks/metrics_service/BUILD +++ b/mobile/test/common/extensions/stat_sinks/metrics_service/BUILD @@ -32,7 +32,7 @@ envoy_extension_cc_test( "//library/common/extensions/stat_sinks/metrics_service:config", "//library/common/extensions/stat_sinks/metrics_service:config_proto_cc_proto", "//library/common/extensions/stat_sinks/metrics_service:service_cc_proto", - "@envoy//test/integration:http_integration_lib", + "@envoy//test/integration:http_integration_lib_light", "@envoy//test/mocks/grpc:grpc_mocks", "@envoy//test/mocks/local_info:local_info_mocks", "@envoy//test/mocks/thread_local:thread_local_mocks", diff --git a/mobile/test/common/integration/BUILD b/mobile/test/common/integration/BUILD index d11a7e2b8d96..47a8feb38749 100644 --- a/mobile/test/common/integration/BUILD +++ b/mobile/test/common/integration/BUILD @@ -53,7 +53,7 @@ envoy_cc_test_library( "//library/common/http:header_utility_lib", "//library/common/types:c_types_lib", "@envoy//test/common/http:common_lib", - "@envoy//test/integration:http_integration_lib", + "@envoy//test/integration:http_integration_lib_light", "@envoy//test/test_common:utility_lib", ], ) diff --git a/source/common/listener_manager/filter_chain_manager_impl.cc b/source/common/listener_manager/filter_chain_manager_impl.cc index 88f01d1577c7..a0c29fdd6883 100644 --- a/source/common/listener_manager/filter_chain_manager_impl.cc +++ b/source/common/listener_manager/filter_chain_manager_impl.cc @@ -138,7 +138,7 @@ void FilterChainManagerImpl::addFilterChains( for (const auto& filter_chain : filter_chain_span) { const auto& filter_chain_match = filter_chain->filter_chain_match(); if (!filter_chain_match.address_suffix().empty() || filter_chain_match.has_suffix_len()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "error adding listener '{}': filter chain '{}' contains " "unimplemented fields", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()), filter_chain->name())); @@ -157,7 +157,7 @@ void FilterChainManagerImpl::addFilterChains( MessageUtil::getJsonStringFromMessageOrError(filter_chain_match, false)); #endif - throw EnvoyException(error_msg); + throwEnvoyExceptionOrPanic(error_msg); } filter_chains.insert({filter_chain_match, filter_chain->name()}); } @@ -175,14 +175,14 @@ void FilterChainManagerImpl::addFilterChains( // If using the matcher, require usage of "name" field and skip building the index. if (filter_chain_matcher) { if (filter_chain->name().empty()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "error adding listener '{}': \"name\" field is required when using a listener matcher", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()))); } auto [_, inserted] = filter_chains_by_name.try_emplace(filter_chain->name(), filter_chain_impl); if (!inserted) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "error adding listener '{}': \"name\" field is duplicated with value '{}'", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()), filter_chain->name())); } @@ -213,7 +213,7 @@ void FilterChainManagerImpl::addFilterChains( // Reject partial wildcards, we don't match on them. for (const auto& server_name : filter_chain_match.server_names()) { if (absl::StrContains(server_name, '*') && !isWildcardServerName(server_name)) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error adding listener '{}': partial wildcards are not supported in " "\"server_names\"", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()))); @@ -442,7 +442,7 @@ void FilterChainManagerImpl::addFilterChainForSourcePorts( // If we got here and found already configured branch, then it means that this FilterChainMatch // is a duplicate, and that there is some overlap in the repeated fields with already processed // FilterChainMatches. - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error adding listener '{}': multiple filter chains with " "overlapping matching rules are defined", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()))); diff --git a/source/common/listener_manager/lds_api.cc b/source/common/listener_manager/lds_api.cc index f77333ea0cce..87cbb28dc252 100644 --- a/source/common/listener_manager/lds_api.cc +++ b/source/common/listener_manager/lds_api.cc @@ -110,9 +110,7 @@ LdsApiImpl::onConfigUpdate(const std::vector& added_ } } END_TRY - catch (const EnvoyException& e) { - onError(e.what()); - } + CATCH(EnvoyException & e, { onError(e.what()); }) } listener_manager_.endListenerUpdate(std::move(failure_state)); diff --git a/source/common/listener_manager/listener_impl.cc b/source/common/listener_manager/listener_impl.cc index c951a931bd08..dde659392823 100644 --- a/source/common/listener_manager/listener_impl.cc +++ b/source/common/listener_manager/listener_impl.cc @@ -161,7 +161,12 @@ Network::SocketSharedPtr ListenSocketFactoryImpl::createListenSocketAndApplyOpti fmt::format("{}: Setting socket options {}", listener_name_, ok ? "succeeded" : "failed"); if (!ok) { ENVOY_LOG(warn, "{}", message); +#ifdef ENVOY_DISABLE_EXCEPTIONS + PANIC(message); +#else throw Network::SocketOptionException(message); +#endif + } else { ENVOY_LOG(debug, "{}", message); } @@ -190,13 +195,18 @@ void ListenSocketFactoryImpl::doFinalPreWorkerInit() { auto listen_and_apply_options = [](Envoy::Network::SocketSharedPtr socket, int tcp_backlog_size) { const auto rc = socket->ioHandle().listen(tcp_backlog_size); if (rc.return_value_ != 0) { - throw EnvoyException(fmt::format("cannot listen() errno={}", rc.errno_)); + throwEnvoyExceptionOrPanic(fmt::format("cannot listen() errno={}", rc.errno_)); } if (!Network::Socket::applyOptions(socket->options(), *socket, envoy::config::core::v3::SocketOption::STATE_LISTENING)) { - throw Network::SocketOptionException( + std::string message = fmt::format("cannot set post-listen socket option on socket: {}", - socket->connectionInfoProvider().localAddress()->asString())); + socket->connectionInfoProvider().localAddress()->asString()); +#ifdef ENVOY_DISABLE_EXCEPTIONS + PANIC(message); +#else + throw Network::SocketOptionException(message); +#endif } }; // On all platforms we should listen on the first socket. @@ -321,7 +331,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, for (auto i = 0; i < config.additional_addresses_size(); i++) { if (socket_type_ != Network::Utility::protobufAddressSocketType(config.additional_addresses(i).address())) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("listener {}: has different socket type. The listener only " "support same socket type for all the addresses.", name_)); @@ -452,7 +462,7 @@ void ListenerImpl::checkIpv4CompatAddress(const Network::Address::InstanceConstS (address->ip()->version() != Network::Address::IpVersion::v6 || (!address->ip()->isAnyAddress() && address->ip()->ipv6()->v4CompatibleAddress() == nullptr))) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Only IPv6 address '::' or valid IPv4-mapped IPv6 address can set ipv4_compat: {}", address->asStringView())); } @@ -461,17 +471,17 @@ void ListenerImpl::checkIpv4CompatAddress(const Network::Address::InstanceConstS void ListenerImpl::validateConfig() { if (mptcp_enabled_) { if (socket_type_ != Network::Socket::Type::Stream) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("listener {}: enable_mptcp can only be used with TCP listeners", name_)); } for (auto& address : addresses_) { if (address->type() != Network::Address::Type::Ip) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("listener {}: enable_mptcp can only be used with IP addresses", name_)); } } if (!Api::OsSysCallsSingleton::get().supportsMptcp()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "listener {}: enable_mptcp is set but MPTCP is not supported by the operating system", name_)); } @@ -489,9 +499,10 @@ void ListenerImpl::buildAccessLog(const envoy::config::listener::v3::Listener& c void ListenerImpl::buildInternalListener(const envoy::config::listener::v3::Listener& config) { if (config.has_internal_listener()) { if (config.has_address() || !config.additional_addresses().empty()) { - throw EnvoyException(fmt::format("error adding listener '{}': address should not be used " - "when an internal listener config is provided", - name_)); + throwEnvoyExceptionOrPanic( + fmt::format("error adding listener '{}': address should not be used " + "when an internal listener config is provided", + name_)); } if ((config.has_connection_balance_config() && config.connection_balance_config().has_exact_balance()) || @@ -500,11 +511,11 @@ void ListenerImpl::buildInternalListener(const envoy::config::listener::v3::List || (config.has_freebind() && config.freebind().value()) || config.has_tcp_backlog_size() || config.has_tcp_fast_open_queue_length() || (config.has_transparent() && config.transparent().value())) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "error adding listener named '{}': has unsupported tcp listener feature", name_)); } if (!config.socket_options().empty()) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error adding listener named '{}': does not support socket option", name_)); } std::shared_ptr internal_listener_registry = @@ -521,7 +532,7 @@ void ListenerImpl::buildInternalListener(const envoy::config::listener::v3::List "type.googleapis.com/" "envoy.extensions.bootstrap.internal_listener.v3.InternalListener"; })) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "error adding listener named '{}': InternalListener bootstrap extension is mandatory", name_)); } @@ -535,9 +546,10 @@ void ListenerImpl::buildInternalListener(const envoy::config::listener::v3::List [](const envoy::config::listener::v3::AdditionalAddress& proto_address) { return proto_address.address().has_envoy_internal_address(); })) { - throw EnvoyException(fmt::format("error adding listener named '{}': use internal_listener " - "field instead of address for internal listeners", - name_)); + throwEnvoyExceptionOrPanic( + fmt::format("error adding listener named '{}': use internal_listener " + "field instead of address for internal listeners", + name_)); } } @@ -561,10 +573,11 @@ void ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Li return; } if (!reuse_port_ && concurrency > 1) { - throw EnvoyException("Listening on UDP when concurrency is > 1 without the SO_REUSEPORT " - "socket option results in " - "unstable packet proxying. Configure the reuse_port listener option or " - "set concurrency = 1."); + throwEnvoyExceptionOrPanic( + "Listening on UDP when concurrency is > 1 without the SO_REUSEPORT " + "socket option results in " + "unstable packet proxying. Configure the reuse_port listener option or " + "set concurrency = 1."); } udp_listener_config_ = std::make_shared(config.udp_listener_config()); @@ -578,8 +591,8 @@ void ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Li if (config.udp_listener_config().has_quic_options()) { #ifdef ENVOY_ENABLE_QUIC if (config.has_connection_balance_config()) { - throw EnvoyException("connection_balance_config is configured for QUIC listener which " - "doesn't work with connection balancer."); + throwEnvoyExceptionOrPanic("connection_balance_config is configured for QUIC listener which " + "doesn't work with connection balancer."); } udp_listener_config_->listener_factory_ = std::make_unique( config.udp_listener_config().quic_options(), concurrency, quic_stat_names_, @@ -596,7 +609,7 @@ void ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Li } #endif #else - throw EnvoyException("QUIC is configured but not enabled in the build."); + throwEnvoyExceptionOrPanic("QUIC is configured but not enabled in the build."); #endif } else { udp_listener_config_->listener_factory_ = @@ -693,7 +706,7 @@ void ListenerImpl::validateFilterChains(const envoy::config::listener::v3::Liste !udp_listener_config_->listener_factory_->isTransportConnectionless())) { // If we got here, this is a tcp listener or connection-oriented udp listener, so ensure there // is a filter chain specified - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error adding listener '{}': no filter chains specified", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()))); } else if (udp_listener_config_ != nullptr && @@ -702,7 +715,7 @@ void ListenerImpl::validateFilterChains(const envoy::config::listener::v3::Liste if (anyFilterChain(config, [](const auto& filter_chain) { return !filter_chain.has_transport_socket(); })) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error adding listener '{}': no transport socket " "specified for connection oriented UDP listener", absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()))); @@ -711,10 +724,11 @@ void ListenerImpl::validateFilterChains(const envoy::config::listener::v3::Liste udp_listener_config_ != nullptr && udp_listener_config_->listener_factory_->isTransportConnectionless()) { - throw EnvoyException(fmt::format("error adding listener '{}': {} filter chain(s) specified for " - "connection-less UDP listener.", - absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()), - config.filter_chains_size())); + throwEnvoyExceptionOrPanic( + fmt::format("error adding listener '{}': {} filter chain(s) specified for " + "connection-less UDP listener.", + absl::StrJoin(addresses_, ",", Network::AddressStrFormatter()), + config.filter_chains_size())); } } @@ -760,8 +774,9 @@ void ListenerImpl::buildConnectionBalancer(const envoy::config::listener::v3::Li Envoy::Registry::FactoryRegistry::getFactoryByType( connection_balance_library_type); if (factory == nullptr) { - throw EnvoyException(fmt::format("Didn't find a registered implementation for type: '{}'", - connection_balance_library_type)); + throwEnvoyExceptionOrPanic( + fmt::format("Didn't find a registered implementation for type: '{}'", + connection_balance_library_type)); } connection_balancers_.emplace( address.asString(), @@ -770,7 +785,7 @@ void ListenerImpl::buildConnectionBalancer(const envoy::config::listener::v3::Li break; } case envoy::config::listener::v3::Listener_ConnectionBalanceConfig::BALANCE_TYPE_NOT_SET: { - throw EnvoyException("No valid balance type for connection balance"); + throwEnvoyExceptionOrPanic("No valid balance type for connection balance"); } } } else { diff --git a/source/common/listener_manager/listener_manager_impl.cc b/source/common/listener_manager/listener_manager_impl.cc index 8484eec9022a..701dd97d389d 100644 --- a/source/common/listener_manager/listener_manager_impl.cc +++ b/source/common/listener_manager/listener_manager_impl.cc @@ -144,7 +144,7 @@ ProdListenerComponentFactory::createListenerFilterFactoryListImpl( const auto& name = proto_config.name(); if (config_discovery.apply_default_config_without_warming() && !config_discovery.has_default_config()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Error: listener filter config {} applied without warming but has no default config.", name)); } @@ -154,7 +154,7 @@ ProdListenerComponentFactory::createListenerFilterFactoryListImpl( Registry::FactoryRegistry:: getFactoryByType(factory_type_url); if (factory == nullptr) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Error: no listener factory found for a required type URL {}.", factory_type_url)); } } @@ -197,9 +197,9 @@ ProdListenerComponentFactory::createUdpListenerFilterFactoryListImpl( static_cast(proto_config.typed_config()))); if (proto_config.config_type_case() == envoy::config::listener::v3::ListenerFilter::ConfigTypeCase::kConfigDiscovery) { - throw EnvoyException(fmt::format("UDP listener filter: {} is configured with " - "unsupported dynamic configuration", - proto_config.name())); + throwEnvoyExceptionOrPanic(fmt::format("UDP listener filter: {} is configured with " + "unsupported dynamic configuration", + proto_config.name())); return ret; } // Now see if there is a factory that will accept the config. @@ -234,7 +234,7 @@ ProdListenerComponentFactory::createQuicListenerFilterFactoryListImpl( const std::string& name = proto_config.name(); if (config_discovery.apply_default_config_without_warming() && !config_discovery.has_default_config()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Error: listener filter config {} applied without warming but has no default config.", name)); } @@ -242,7 +242,7 @@ ProdListenerComponentFactory::createQuicListenerFilterFactoryListImpl( absl::string_view factory_type_url = TypeUtil::typeUrlToDescriptorFullName(type_url); if (Registry::FactoryRegistry:: getFactoryByType(factory_type_url) == nullptr) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Error: no listener factory found for a required type URL {}.", factory_type_url)); } } @@ -291,7 +291,7 @@ Network::SocketSharedPtr ProdListenerComponentFactory::createListenSocket( // This could be implemented in the future, since Unix domain sockets // support SOCK_DGRAM, but there would need to be a way to specify it in // envoy.api.v2.core.Pipe. - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("socket type {} not supported for pipes", toString(socket_type))); } const std::string addr = fmt::format("unix://{}", address->asString()); @@ -494,7 +494,7 @@ ListenerManagerImpl::addOrUpdateListener(const envoy::config::listener::v3::List return addOrUpdateListenerInternal(config, version_info, added_via_api, name); } END_TRY - catch (const EnvoyException& e) { + CATCH(const EnvoyException& e, { if (it == error_state_tracker_.end()) { it = error_state_tracker_.emplace(name, std::make_unique()).first; } @@ -503,7 +503,7 @@ ListenerManagerImpl::addOrUpdateListener(const envoy::config::listener::v3::List it->second->set_details(e.what()); it->second->mutable_failed_configuration()->PackFrom(config); return absl::InvalidArgumentError(e.what()); - } + }) error_state_tracker_.erase(it); return false; } @@ -512,15 +512,15 @@ void ListenerManagerImpl::setupSocketFactoryForListener(ListenerImpl& new_listen const ListenerImpl& existing_listener) { bool same_socket_options = true; if (new_listener.reusePort() != existing_listener.reusePort()) { - throw EnvoyException(fmt::format("Listener {}: reuse port cannot be changed during an update", - new_listener.name())); + throwEnvoyExceptionOrPanic(fmt::format( + "Listener {}: reuse port cannot be changed during an update", new_listener.name())); } same_socket_options = existing_listener.socketOptionsEqual(new_listener); if (!same_socket_options && new_listener.reusePort() == false) { - throw EnvoyException(fmt::format("Listener {}: doesn't support update any socket options " - "when the reuse port isn't enabled", - new_listener.name())); + throwEnvoyExceptionOrPanic(fmt::format("Listener {}: doesn't support update any socket options " + "when the reuse port isn't enabled", + new_listener.name())); } if (!(existing_listener.hasCompatibleAddress(new_listener) && same_socket_options)) { @@ -736,11 +736,11 @@ bool ListenerManagerImpl::doFinalPreWorkerListenerInit(ListenerImpl& listener) { return true; } END_TRY - catch (EnvoyException& e) { + CATCH(EnvoyException & e, { ENVOY_LOG(error, "final pre-worker listener init for listener '{}' failed: {}", listener.name(), e.what()); return false; - } + }); } void ListenerManagerImpl::addListenerToWorker(Worker& worker, @@ -1070,10 +1070,11 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF #if defined(ENVOY_ENABLE_QUIC) if (is_quic && dynamic_cast(&config_factory) == nullptr) { - throw EnvoyException(fmt::format("error building filter chain for quic listener: wrong " - "transport socket config specified for quic transport socket: " - "{}. \nUse QuicDownstreamTransport instead.", - transport_socket.DebugString())); + throwEnvoyExceptionOrPanic( + fmt::format("error building filter chain for quic listener: wrong " + "transport socket config specified for quic transport socket: " + "{}. \nUse QuicDownstreamTransport instead.", + transport_socket.DebugString())); } const std::string hcm_str = "type.googleapis.com/" @@ -1082,7 +1083,7 @@ Network::DrainableFilterChainSharedPtr ListenerFilterChainFactoryBuilder::buildF (filter_chain.filters().empty() || filter_chain.filters(filter_chain.filters().size() - 1).typed_config().type_url() != hcm_str)) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("error building network filter chain for quic listener: requires " "http_connection_manager filter to be last in the chain.")); } @@ -1120,7 +1121,7 @@ void ListenerManagerImpl::setNewOrDrainingSocketFactory(const std::string& name, fmt::format("error adding listener: '{}' has duplicate address '{}' as existing listener", name, absl::StrJoin(listener.addresses(), ",", Network::AddressStrFormatter())); ENVOY_LOG(warn, "{}", message); - throw EnvoyException(message); + throwEnvoyExceptionOrPanic(message); } // Search through draining listeners to see if there is a listener that has a socket factory for @@ -1183,12 +1184,12 @@ void ListenerManagerImpl::createListenSocketFactory(ListenerImpl& listener) { } } END_TRY - catch (const EnvoyException& e) { + CATCH(const EnvoyException& e, { ENVOY_LOG(error, "listener '{}' failed to bind or apply socket options: {}", listener.name(), e.what()); incListenerCreateFailureStat(); throw e; - } + }); } void ListenerManagerImpl::maybeCloseSocketsForListener(ListenerImpl& listener) { diff --git a/source/common/network/listen_socket_impl.cc b/source/common/network/listen_socket_impl.cc index 121473fdd492..1de9779bd24d 100644 --- a/source/common/network/listen_socket_impl.cc +++ b/source/common/network/listen_socket_impl.cc @@ -27,7 +27,11 @@ Api::SysCallIntResult ListenSocketImpl::bind(Network::Address::InstanceConstShar const std::string error = fmt::format("cannot bind '{}': {}", connection_info_provider_->localAddress()->asString(), errorDetails(result.errno_)); +#ifdef ENVOY_DISABLE_EXCEPTIONS + PANIC(error); +#else throw SocketBindException(error, result.errno_); +#endif } return {0, 0}; } @@ -35,7 +39,11 @@ Api::SysCallIntResult ListenSocketImpl::bind(Network::Address::InstanceConstShar void ListenSocketImpl::setListenSocketOptions(const Network::Socket::OptionsSharedPtr& options) { if (!Network::Socket::applyOptions(options, *this, envoy::config::core::v3::SocketOption::STATE_PREBIND)) { +#ifdef ENVOY_DISABLE_EXCEPTIONS + PANIC("ListenSocket: Setting socket options failed"); +#else throw SocketOptionException("ListenSocket: Setting socket options failed"); +#endif } } diff --git a/source/common/quic/quic_server_transport_socket_factory.cc b/source/common/quic/quic_server_transport_socket_factory.cc index d8743ef100e4..edb90204870e 100644 --- a/source/common/quic/quic_server_transport_socket_factory.cc +++ b/source/common/quic/quic_server_transport_socket_factory.cc @@ -55,7 +55,7 @@ void initializeQuicCertAndKey(Ssl::TlsContext& context, std::istringstream pem_stream(cert_str); auto pem_result = quic::ReadNextPemMessage(&pem_stream); if (pem_result.status != quic::PemReadResult::Status::kOk) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( "Error loading certificate in QUIC context: error from ReadNextPemMessage"); } chain.push_back(std::move(pem_result.contents)); @@ -77,7 +77,7 @@ void initializeQuicCertAndKey(Ssl::TlsContext& context, bssl::UniquePtr pub_key(X509_get_pubkey(context.cert_chain_.get())); int sign_alg = deduceSignatureAlgorithmFromPublicKey(pub_key.get(), &error_details); if (sign_alg == 0) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( absl::StrCat("Failed to deduce signature algorithm from public key: ", error_details)); } @@ -88,7 +88,7 @@ void initializeQuicCertAndKey(Ssl::TlsContext& context, std::unique_ptr pem_key = std::make_unique(std::move(privateKey)); if (pem_key == nullptr) { - throw EnvoyException("Failed to load QUIC private key."); + throwEnvoyExceptionOrPanic("Failed to load QUIC private key."); } context.quic_private_key_ = std::move(pem_key); diff --git a/source/common/tls/server_context_impl.cc b/source/common/tls/server_context_impl.cc index 5cb1facf7d4d..9dc43f2c62ad 100644 --- a/source/common/tls/server_context_impl.cc +++ b/source/common/tls/server_context_impl.cc @@ -102,7 +102,7 @@ ServerContextImpl::ServerContextImpl(Stats::Scope& scope, return; } if (config.tlsCertificates().empty() && !config.capabilities().provides_certificates) { - throw EnvoyException("Server TlsCertificates must have a certificate specified"); + throwEnvoyExceptionOrPanic("Server TlsCertificates must have a certificate specified"); } for (auto& ctx : tls_contexts_) { @@ -188,17 +188,17 @@ ServerContextImpl::ServerContextImpl(Stats::Scope& scope, auto& ocsp_resp_bytes = tls_certificates[i].get().ocspStaple(); if (ocsp_resp_bytes.empty()) { if (ctx.is_must_staple_) { - throw EnvoyException("OCSP response is required for must-staple certificate"); + throwEnvoyExceptionOrPanic("OCSP response is required for must-staple certificate"); } if (ocsp_staple_policy_ == Ssl::ServerContextConfig::OcspStaplePolicy::MustStaple) { - throw EnvoyException("Required OCSP response is missing from TLS context"); + throwEnvoyExceptionOrPanic("Required OCSP response is missing from TLS context"); } } else { auto response_or_error = Ocsp::OcspResponseWrapperImpl::create(ocsp_resp_bytes, factory_context_.timeSource()); THROW_IF_STATUS_NOT_OK(response_or_error, throw); if (!response_or_error.value()->matchesCertificate(*ctx.cert_chain_)) { - throw EnvoyException("OCSP response does not match its TLS certificate"); + throwEnvoyExceptionOrPanic("OCSP response does not match its TLS certificate"); } ctx.ocsp_response_ = std::move(response_or_error.value()); } @@ -291,7 +291,7 @@ ServerContextImpl::generateHashForSessionContextId(const std::vector= saturated_threshold_) { - throw EnvoyException("scaling_threshold must be less than saturation_threshold"); + throwEnvoyExceptionOrPanic("scaling_threshold must be less than saturation_threshold"); } } @@ -90,7 +90,7 @@ TriggerPtr createTriggerFromConfig(const envoy::config::overload::v3::Trigger& t trigger = std::make_unique(trigger_config.scaled()); break; case envoy::config::overload::v3::Trigger::TriggerOneofCase::TRIGGER_ONEOF_NOT_SET: - throw EnvoyException(absl::StrCat("action not set for trigger ", trigger_config.name())); + throwEnvoyExceptionOrPanic(absl::StrCat("action not set for trigger ", trigger_config.name())); } return trigger; @@ -130,7 +130,7 @@ Event::ScaledTimerType parseTimerType( case Config::TRANSPORT_SOCKET_CONNECT: return Event::ScaledTimerType::TransportSocketConnectTimeout; default: - throw EnvoyException(fmt::format("Unknown timer type {}", config_timer_type)); + throwEnvoyExceptionOrPanic(fmt::format("Unknown timer type {}", config_timer_type)); } } @@ -156,8 +156,8 @@ parseTimerMinimums(const ProtobufWkt::Any& typed_config, auto [_, inserted] = timer_map.insert(std::make_pair(timer_type, minimum)); UNREFERENCED_PARAMETER(_); if (!inserted) { - throw EnvoyException(fmt::format("Found duplicate entry for timer type {}", - Config::TimerType_Name(scale_timer.timer()))); + throwEnvoyExceptionOrPanic(fmt::format("Found duplicate entry for timer type {}", + Config::TimerType_Name(scale_timer.timer()))); } } @@ -274,7 +274,7 @@ OverloadAction::OverloadAction(const envoy::config::overload::v3::OverloadAction for (const auto& trigger_config : config.triggers()) { if (!triggers_.try_emplace(trigger_config.name(), createTriggerFromConfig(trigger_config)) .second) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( absl::StrCat("Duplicate trigger resource for overload action ", config.name())); } } @@ -322,7 +322,7 @@ LoadShedPointImpl::LoadShedPointImpl(const envoy::config::overload::v3::LoadShed for (const auto& trigger_config : config.triggers()) { if (!triggers_.try_emplace(trigger_config.name(), createTriggerFromConfig(trigger_config)) .second) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( absl::StrCat("Duplicate trigger resource for LoadShedPoint ", config.name())); } } @@ -416,7 +416,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S result = resources_.try_emplace(name, name, std::move(monitor), *this, stats_scope).second; } if (!result) { - throw EnvoyException(absl::StrCat("Duplicate resource monitor ", name)); + throwEnvoyExceptionOrPanic(absl::StrCat("Duplicate resource monitor ", name)); } } @@ -429,7 +429,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S const auto& well_known_actions = OverloadActionNames::get().WellKnownActions; if (std::find(well_known_actions.begin(), well_known_actions.end(), name) == well_known_actions.end()) { - throw EnvoyException(absl::StrCat("Unknown Overload Manager Action ", name)); + throwEnvoyExceptionOrPanic(absl::StrCat("Unknown Overload Manager Action ", name)); } // TODO: use in place construction once https://github.com/abseil/abseil-cpp/issues/388 is @@ -439,7 +439,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S // an invalid free. auto result = actions_.try_emplace(symbol, OverloadAction(action, stats_scope)); if (!result.second) { - throw EnvoyException(absl::StrCat("Duplicate overload action ", name)); + throwEnvoyExceptionOrPanic(absl::StrCat("Duplicate overload action ", name)); } if (name == OverloadActionNames::get().ReduceTimeouts) { @@ -447,12 +447,12 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S parseTimerMinimums(action.typed_config(), validation_visitor)); } else if (name == OverloadActionNames::get().ResetStreams) { if (!config.has_buffer_factory_config()) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("Overload action \"{}\" requires buffer_factory_config.", name)); } makeCounter(api.rootScope(), OverloadActionStatsNames::get().ResetStreamsCount); } else if (action.has_typed_config()) { - throw EnvoyException(fmt::format( + throwEnvoyExceptionOrPanic(fmt::format( "Overload action \"{}\" has an unexpected value for the typed_config field", name)); } @@ -464,7 +464,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S if (resources_.find(resource) == resources_.end() && proactive_resource_it == OverloadProactiveResources::get().proactive_action_name_to_resource_.end()) { - throw EnvoyException( + throwEnvoyExceptionOrPanic( fmt::format("Unknown trigger resource {} for overload action {}", resource, name)); } resource_to_actions_.insert(std::make_pair(resource, symbol)); @@ -475,8 +475,8 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S for (const auto& point : config.loadshed_points()) { for (const auto& trigger : point.triggers()) { if (!resources_.contains(trigger.name())) { - throw EnvoyException(fmt::format("Unknown trigger resource {} for loadshed point {}", - trigger.name(), point.name())); + throwEnvoyExceptionOrPanic(fmt::format("Unknown trigger resource {} for loadshed point {}", + trigger.name(), point.name())); } } @@ -485,7 +485,7 @@ OverloadManagerImpl::OverloadManagerImpl(Event::Dispatcher& dispatcher, Stats::S std::make_unique(point, api.rootScope(), api.randomGenerator())); if (!result.second) { - throw EnvoyException(absl::StrCat("Duplicate loadshed point ", point.name())); + throwEnvoyExceptionOrPanic(absl::StrCat("Duplicate loadshed point ", point.name())); } } } diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index bf12f5417c77..6fdc89db3ac0 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -40,7 +40,7 @@ namespace ConfigTest { namespace { // asConfigYaml returns a new config that empties the configPath() and populates configYaml() -OptionsImpl asConfigYaml(const OptionsImpl& src, Api::Api& api) { +OptionsImplBase asConfigYaml(const OptionsImplBase& src, Api::Api& api) { return Envoy::Server::createTestOptionsImpl( "", api.fileSystem().fileReadToEnd(src.configPath()).value(), src.localAddressIpVersion()); } @@ -58,7 +58,8 @@ static std::vector unsuported_win32_configs = { class ConfigTest { public: - ConfigTest(OptionsImpl& options) : api_(Api::createApiForTest(time_system_)), options_(options) { + ConfigTest(OptionsImplBase& options) + : api_(Api::createApiForTest(time_system_)), options_(options) { ON_CALL(*server_.server_factory_context_, api()).WillByDefault(ReturnRef(server_.api_)); ON_CALL(server_, options()).WillByDefault(ReturnRef(options_)); ON_CALL(server_, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); @@ -174,7 +175,7 @@ class ConfigTest { NiceMock server_; Server::ServerFactoryContextImpl server_factory_context_{server_}; NiceMock ssl_context_manager_; - OptionsImpl& options_; + OptionsImplBase& options_; std::unique_ptr cluster_manager_factory_; std::unique_ptr> component_factory_ptr_{ std::make_unique>()}; @@ -212,8 +213,8 @@ void testMerge() { ] } })EOF"; - OptionsImpl options(Server::createTestOptionsImpl("envoyproxy_io_proxy.yaml", overlay, - Network::Address::IpVersion::v6)); + OptionsImplBase options(Server::createTestOptionsImpl("envoyproxy_io_proxy.yaml", overlay, + Network::Address::IpVersion::v6)); envoy::config::bootstrap::v3::Bootstrap bootstrap; ASSERT_TRUE(Server::InstanceUtil::loadBootstrapConfig( bootstrap, options, ProtobufMessage::getStrictValidationVisitor(), *api) @@ -238,7 +239,7 @@ uint32_t run(const std::string& directory) { [filename](const absl::string_view& s) { return filename.find(std::string(s)) != std::string::npos; }) == unsuported_win32_configs.end()) { - OptionsImpl options( + OptionsImplBase options( Envoy::Server::createTestOptionsImpl(filename, "", Network::Address::IpVersion::v6)); ConfigTest test1(options); envoy::config::bootstrap::v3::Bootstrap bootstrap; @@ -246,7 +247,7 @@ uint32_t run(const std::string& directory) { bootstrap, options, ProtobufMessage::getStrictValidationVisitor(), *api) .ok()); ENVOY_LOG_MISC(info, "testing {} as yaml.", filename); - OptionsImpl config = asConfigYaml(options, *api); + OptionsImplBase config = asConfigYaml(options, *api); ConfigTest test2(config); } num_tested++; diff --git a/test/extensions/filters/listener/local_ratelimit/BUILD b/test/extensions/filters/listener/local_ratelimit/BUILD index 9581be70cf29..fe20789f4eeb 100644 --- a/test/extensions/filters/listener/local_ratelimit/BUILD +++ b/test/extensions/filters/listener/local_ratelimit/BUILD @@ -39,6 +39,7 @@ envoy_extension_cc_test( "//source/extensions/filters/network/tcp_proxy:config", "//source/extensions/transport_sockets/raw_buffer:config", "//test/integration:base_integration_test_lib", + "//test/integration:common_extensions_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto", diff --git a/test/integration/BUILD b/test/integration/BUILD index 162e3109e9b1..cd6cfabcfc61 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -924,8 +924,10 @@ envoy_cc_test_library( ], ) +# The light verson of this library allows Envoy mobile e2e tests to pull minimal +# dependencies, so keep as few libraries as possible in scope of the excepton-free builds. envoy_cc_test_library( - name = "http_integration_lib", + name = "http_integration_lib_light", srcs = [ "http_integration.cc", ], @@ -936,7 +938,7 @@ envoy_cc_test_library( "//test/config/integration/certs", ], deps = [ - ":integration_lib", + ":integration_lib_light", ":test_host_predicate_lib", "//envoy/event:timer_interface", "//source/common/common:thread_annotations", @@ -946,6 +948,22 @@ envoy_cc_test_library( "//source/extensions/filters/network/http_connection_manager:config", "//test/common/http/http2:http2_frame", "//test/common/upstream:utility_lib", + "//test/mocks/upstream:cluster_info_mocks", + "//test/test_common:registry_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", + ], +) + +envoy_cc_test_library( + name = "http_integration_lib", + deps = [ + ":common_extensions_lib", + ":http_integration_lib_light", + ":integration_lib", + ":test_host_predicate_lib", "//test/integration/filters:add_body_filter_config_lib", "//test/integration/filters:add_trailers_filter_config_lib", "//test/integration/filters:call_decodedata_once_filter_config_lib", @@ -956,12 +974,6 @@ envoy_cc_test_library( "//test/integration/filters:passthrough_filter_config_lib", "//test/integration/filters:pause_filter_lib", "//test/integration/filters:wait_for_whole_request_and_response_config_lib", - "//test/mocks/upstream:cluster_info_mocks", - "//test/test_common:registry_lib", - "//test/test_common:utility_lib", - "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", - "@envoy_api//envoy/extensions/transport_sockets/quic/v3:pkg_cc_proto", ], ) @@ -1101,6 +1113,24 @@ envoy_cc_test_library( ]), ) +envoy_cc_test_library( + name = "common_extensions_lib", + deps = [ + "//source/common/http:rds_lib", + "//source/common/router:scoped_rds_lib", + "//source/extensions/clusters/eds:eds_lib", + "//source/extensions/clusters/static:static_cluster_lib", + "//source/extensions/config_subscription/grpc:grpc_collection_subscription_lib", + "//source/extensions/config_subscription/grpc:grpc_subscription_lib", + "//source/extensions/load_balancing_policies/cluster_provided:config", + "//source/extensions/load_balancing_policies/least_request:config", + "//source/extensions/load_balancing_policies/maglev:config", + "//source/extensions/load_balancing_policies/random:config", + "//source/extensions/load_balancing_policies/round_robin:config", + "//source/extensions/network/dns_resolver/cares:config", + ], +) + envoy_cc_test_library( name = "base_integration_test_lib", srcs = [ @@ -1116,20 +1146,9 @@ envoy_cc_test_library( ":utility_lib", "//source/common/common:thread_lib", "//source/common/config:api_version_lib", - "//source/common/http:rds_lib", "//source/common/tls:context_config_lib", "//source/common/tls:context_lib", "//source/common/tls:ssl_socket_lib", - "//source/extensions/clusters/eds:eds_lib", - "//source/extensions/clusters/static:static_cluster_lib", - "//source/extensions/config_subscription/grpc:grpc_collection_subscription_lib", - "//source/extensions/config_subscription/grpc:grpc_subscription_lib", - "//source/extensions/load_balancing_policies/cluster_provided:config", - "//source/extensions/load_balancing_policies/least_request:config", - "//source/extensions/load_balancing_policies/maglev:config", - "//source/extensions/load_balancing_policies/random:config", - "//source/extensions/load_balancing_policies/round_robin:config", - "//source/extensions/network/dns_resolver/cares:config", "//source/extensions/request_id/uuid:config", "//source/server:process_context_lib", "//source/server:proto_descriptors_lib", @@ -1211,7 +1230,7 @@ envoy_cc_test_library( "//source/server:drain_manager_lib", "//source/server:hot_restart_nop_lib", "//source/server:listener_hooks_lib", - "//source/server:options_lib", + "//source/server:options_base", "//source/server:process_context_lib", "//source/server:server_lib", "//test/common/upstream:utility_lib", @@ -1237,8 +1256,10 @@ envoy_cc_test_library( ], ) +# The light verson of this library allows Envoy mobile e2e tests to pull minimal +# dependencies, so keep as few libraries as possible in scope of the excepton-free builds. envoy_cc_test_library( - name = "integration_lib", + name = "integration_lib_light", hdrs = [ "integration.h", ], @@ -1275,7 +1296,6 @@ envoy_cc_test_library( "//source/common/http:codec_client_lib", "//source/common/http:header_map_lib", "//source/common/http:headers_lib", - "//source/common/http:rds_lib", "//source/common/http/http1:codec_lib", "//source/common/http/http2:codec_lib", "//source/common/listener_manager:connection_handler_lib", @@ -1283,7 +1303,6 @@ envoy_cc_test_library( "//source/common/network:filter_lib", "//source/common/network:listen_socket_lib", "//source/common/network:utility_lib", - "//source/common/router:scoped_rds_lib", "//source/common/runtime:runtime_lib", "//source/common/stats:isolated_store_lib", "//source/common/stats:thread_local_store_lib", @@ -1291,10 +1310,6 @@ envoy_cc_test_library( "//source/common/tls:server_context_lib", "//source/common/upstream:upstream_includes", "//source/common/upstream:upstream_lib", - "//source/extensions/access_loggers/file:config", - "//source/extensions/health_checkers/grpc:health_checker_lib", - "//source/extensions/health_checkers/http:health_checker_lib", - "//source/extensions/health_checkers/tcp:health_checker_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:drain_manager_lib", "//source/server:hot_restart_nop_lib", @@ -1318,6 +1333,21 @@ envoy_cc_test_library( ], ) +envoy_cc_test_library( + name = "integration_lib", + hdrs = [ + "integration.h", + ], + deps = [ + ":common_extensions_lib", + ":integration_lib_light", + "//source/extensions/access_loggers/file:config", + "//source/extensions/health_checkers/grpc:health_checker_lib", + "//source/extensions/health_checkers/http:health_checker_lib", + "//source/extensions/health_checkers/tcp:health_checker_lib", + ], +) + envoy_cc_test_library( name = "integration_test_lib", hdrs = ["integration_test.h"], diff --git a/test/integration/base_integration_test.cc b/test/integration/base_integration_test.cc index 6558342b0e40..ce040b423743 100644 --- a/test/integration/base_integration_test.cc +++ b/test/integration/base_integration_test.cc @@ -693,16 +693,18 @@ AssertionResult BaseIntegrationTest::waitForPortAvailable(uint32_t port, std::chrono::milliseconds timeout) { Event::TestTimeSystem::RealTimeBound bound(timeout); while (bound.withinBound()) { - try { + TRY_NEEDS_AUDIT { Network::TcpListenSocket give_me_a_name( Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(version_), port), nullptr, true); return AssertionSuccess(); - } catch (const EnvoyException&) { + } + END_TRY + CATCH(const EnvoyException&, { // The nature of this function requires using a real sleep here. timeSystem().realSleepDoNotUseWithoutScrutiny(std::chrono::milliseconds(100)); - } + }); } return AssertionFailure() << "Timeout waiting for port availability"; diff --git a/test/integration/server.cc b/test/integration/server.cc index 25884fdd29b1..92a7986c28f3 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -13,7 +13,7 @@ #include "source/common/thread_local/thread_local_impl.h" #include "source/server/hot_restart_nop_impl.h" #include "source/server/instance_impl.h" -#include "source/server/options_impl.h" +#include "source/server/options_impl_base.h" #include "source/server/process_context_impl.h" #include "test/integration/utility.h" @@ -27,7 +27,7 @@ namespace Envoy { namespace Server { -OptionsImpl +OptionsImplBase createTestOptionsImpl(const std::string& config_path, const std::string& config_yaml, Network::Address::IpVersion ip_version, FieldValidationConfig validation_config, uint32_t concurrency, @@ -38,7 +38,11 @@ createTestOptionsImpl(const std::string& config_path, const std::string& config_ const std::string service_cluster = use_bootstrap_node_metadata ? "" : "cluster_name"; const std::string service_node = use_bootstrap_node_metadata ? "" : "node_name"; const std::string service_zone = use_bootstrap_node_metadata ? "" : "zone_name"; - OptionsImpl test_options(service_cluster, service_node, service_zone, spdlog::level::info); + OptionsImplBase test_options; + test_options.setServiceClusterName(service_cluster); + test_options.setServiceNodeName(service_node); + test_options.setServiceZone(service_zone); + test_options.setLogLevel(spdlog::level::info); test_options.setConfigPath(config_path); test_options.setConfigYaml(config_yaml); @@ -196,7 +200,7 @@ void IntegrationTestServer::threadRoutine( ProcessObjectOptRef process_object, Server::FieldValidationConfig validation_config, uint32_t concurrency, std::chrono::seconds drain_time, Server::DrainStrategy drain_strategy, Buffer::WatermarkFactorySharedPtr watermark_factory, bool use_bootstrap_node_metadata) { - OptionsImpl options(Server::createTestOptionsImpl( + OptionsImplBase options(Server::createTestOptionsImpl( config_path_, "", version, validation_config, concurrency, drain_time, drain_strategy, use_bootstrap_node_metadata, std::move(config_proto_))); Thread::MutexBasicLockable lock; @@ -224,7 +228,7 @@ IntegrationTestServerImpl::IntegrationTestServerImpl( } void IntegrationTestServerImpl::createAndRunEnvoyServer( - OptionsImpl& options, Event::TimeSystem& time_system, + OptionsImplBase& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, Thread::BasicLockable& access_log_lock, Server::ComponentFactory& component_factory, Random::RandomGeneratorPtr&& random_generator, ProcessObjectOptRef process_object, diff --git a/test/integration/server.h b/test/integration/server.h index dcb4ec69d8b1..e80c6ebe6280 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -19,7 +19,7 @@ #include "source/common/stats/allocator_impl.h" #include "source/server/drain_manager_impl.h" #include "source/server/listener_hooks.h" -#include "source/server/options_impl.h" +#include "source/server/options_impl_base.h" #include "source/server/server.h" #include "test/integration/server_stats.h" @@ -39,8 +39,8 @@ struct FieldValidationConfig { bool ignore_unknown_dynamic_fields = false; }; -// Create OptionsImpl structures suitable for tests. Disables hot restart. -OptionsImpl createTestOptionsImpl( +// Create OptionsImplBase structures suitable for tests. Disables hot restart. +OptionsImplBase createTestOptionsImpl( const std::string& config_path, const std::string& config_yaml, Network::Address::IpVersion ip_version, FieldValidationConfig validation_config = FieldValidationConfig(), uint32_t concurrency = 1, @@ -560,7 +560,7 @@ class IntegrationTestServer : public Logger::Loggable, // functions server(), statStore(), and adminAddress() may be called, but before the server // has been started. // The subclass is also responsible for tearing down this server in its destructor. - virtual void createAndRunEnvoyServer(OptionsImpl& options, Event::TimeSystem& time_system, + virtual void createAndRunEnvoyServer(OptionsImplBase& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, Thread::BasicLockable& access_log_lock, Server::ComponentFactory& component_factory, @@ -635,7 +635,7 @@ class IntegrationTestServerImpl : public IntegrationTestServer { } private: - void createAndRunEnvoyServer(OptionsImpl& options, Event::TimeSystem& time_system, + void createAndRunEnvoyServer(OptionsImplBase& options, Event::TimeSystem& time_system, Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, Thread::BasicLockable& access_log_lock, Server::ComponentFactory& component_factory, diff --git a/test/mocks/runtime/mocks.h b/test/mocks/runtime/mocks.h index 9bb7e6c09c89..020c6b344617 100644 --- a/test/mocks/runtime/mocks.h +++ b/test/mocks/runtime/mocks.h @@ -27,8 +27,8 @@ class MockSnapshot : public Snapshot { } else if (default_value == 100) { return true; } else { - throw std::invalid_argument("Not implemented yet. You may want to set expectation of mocked " - "featureEnabled() instead."); + PANIC("Not implemented yet. You may want to set expectation of mocked" + "featureEnabled() instead."); } } diff --git a/test/test_common/BUILD b/test/test_common/BUILD index 260abad9c73f..61fdf06171c8 100644 --- a/test/test_common/BUILD +++ b/test/test_common/BUILD @@ -5,6 +5,7 @@ load( "envoy_cc_test", "envoy_cc_test_library", "envoy_package", + "envoy_select_enable_exceptions", "envoy_select_signal_trace", ) @@ -31,8 +32,10 @@ envoy_cc_test_library( "//source/common/filesystem:filesystem_lib", "//source/common/json:json_loader_lib", "//source/common/network:utility_lib", + "//source/server:options_base", + ] + envoy_select_signal_trace(["//source/common/signal:sigaction_lib"]) + envoy_select_enable_exceptions([ "//source/server:options_lib", - ] + envoy_select_signal_trace(["//source/common/signal:sigaction_lib"]), + ]), ) envoy_cc_test_library( diff --git a/test/test_common/environment.cc b/test/test_common/environment.cc index 0e6659c24569..7b9bc502511f 100644 --- a/test/test_common/environment.cc +++ b/test/test_common/environment.cc @@ -22,7 +22,11 @@ #include "source/common/signal/signal_action.h" #endif +#ifdef ENVOY_DISABLE_EXCEPTIONS +#include "source/server/options_impl_base.h" +#else #include "source/server/options_impl.h" +#endif #include "test/test_common/file_system_for_test.h" #include "test/test_common/network_utility.h" @@ -261,8 +265,24 @@ std::vector TestEnvironment::getSpdLoggersForTest() { } Server::Options& TestEnvironment::getOptions() { +#ifdef ENVOY_DISABLE_EXCEPTIONS + // tclap, used for command line parsing, throws exceptions. While Envoy won't + // do full command line parsing in exception-free mode, handling -l trace is a + // nice-to-have for Envoy mobile e2e tests. + static OptionsImplBase* options = new OptionsImplBase(); + for (int i = 0; i < argc_; ++i) { + if (absl::StartsWith(argv_[i], "-l ")) { + static absl::StatusOr level = + OptionsImplBase::parseAndValidateLogLevel(argv_[i] + 3); + if (level.status().ok()) { + options->setLogLevel(*level); + } + } + } +#else static OptionsImpl* options = new OptionsImpl( argc_, argv_, [](bool) { return "1"; }, spdlog::level::err); +#endif return *options; } diff --git a/test/test_common/network_utility.cc b/test/test_common/network_utility.cc index 485a214f6755..e6462138cad5 100644 --- a/test/test_common/network_utility.cc +++ b/test/test_common/network_utility.cc @@ -174,7 +174,7 @@ bindFreeLoopbackPort(Address::IpVersion version, Socket::Type type, bool reuse_p std::string msg = fmt::format("bind failed for address {} with error: {} ({})", addr->asString(), errorDetails(result.errno_), result.errno_); ADD_FAILURE() << msg; - throw EnvoyException(msg); + throwEnvoyExceptionOrPanic(msg); } return std::make_pair(sock->connectionInfoProvider().localAddress(), std::move(sock)); From e5d3a966f67c3dbdac161b8d1ef9bc9c3bca48ec Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 8 Jul 2024 19:41:35 +0000 Subject: [PATCH 2/3] unit test fix Signed-off-by: Alyssa Wilk --- test/server/server_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/server_test.cc b/test/server/server_test.cc index fb37a78db4f4..0b741c9d63be 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1679,7 +1679,7 @@ TEST_P(ServerInstanceImplTest, CallbacksStatsSinkTest) { // Validate that disabled extension is reflected in the list of Node extensions. TEST_P(ServerInstanceImplTest, DisabledExtension) { - OptionsImpl::disableExtensions({"envoy.filters.http/envoy.filters.http.buffer"}); + OptionsImplBase::disableExtensions({"envoy.filters.http/envoy.filters.http.buffer"}); initialize("test/server/test_data/server/node_bootstrap.yaml"); bool disabled_filter_found = false; for (const auto& extension : server_->localInfo().node().extensions()) { From fe5885ec83e22cd00a3c3a0f767c3bc2620c93d3 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 8 Jul 2024 20:44:50 +0000 Subject: [PATCH 3/3] third time's the charm Signed-off-by: Alyssa Wilk --- source/server/admin/server_info_handler.cc | 5 ++++- test/extensions/filters/listener/http_inspector/BUILD | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/source/server/admin/server_info_handler.cc b/source/server/admin/server_info_handler.cc index d13bc54eb5ef..7b5df185304c 100644 --- a/source/server/admin/server_info_handler.cc +++ b/source/server/admin/server_info_handler.cc @@ -85,7 +85,10 @@ Http::Code ServerInfoHandler::handlerServerInfo(Http::ResponseHeaderMap& headers server_info.mutable_uptime_all_epochs()->set_seconds(uptime_all_epochs); envoy::admin::v3::CommandLineOptions* command_line_options = server_info.mutable_command_line_options(); - *command_line_options = *server_.options().toCommandLineOptions(); + Server::CommandLineOptionsPtr options = server_.options().toCommandLineOptions(); + if (options) { + *command_line_options = *options; + } server_info.mutable_node()->MergeFrom(server_.localInfo().node()); response.add(MessageUtil::getJsonStringFromMessageOrError(server_info, true, true)); headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.Json); diff --git a/test/extensions/filters/listener/http_inspector/BUILD b/test/extensions/filters/listener/http_inspector/BUILD index ad5ebbbc2c93..7e5eef4d8c42 100644 --- a/test/extensions/filters/listener/http_inspector/BUILD +++ b/test/extensions/filters/listener/http_inspector/BUILD @@ -71,6 +71,7 @@ envoy_extension_cc_test( "//source/extensions/filters/network/tcp_proxy:config", "//source/extensions/transport_sockets/raw_buffer:config", "//test/integration:base_integration_test_lib", + "//test/integration:common_extensions_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/access_loggers/file/v3:pkg_cc_proto",