diff --git a/.github/workflows/android_tests.yml b/.github/workflows/android_tests.yml index 0ff44a0a79f4b..a004713f4d121 100644 --- a/.github/workflows/android_tests.yml +++ b/.github/workflows/android_tests.yml @@ -105,10 +105,12 @@ jobs: if: steps.should_run.outputs.run_ci_job == 'true' env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # This is run with ALLOW_TCP_LISTENERS due to the tests in test/kotlin/integration/proxying/ + # which use Envoy as an HTTP proxy rather than a mobile library. run: | cd mobile && ./bazelw test \ --test_output=all \ --build_tests_only \ $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-linux-clang") \ - --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ + --copt=-DALLOW_TCP_LISTENERS --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ //test/kotlin/... diff --git a/.github/workflows/ios_build.yml b/.github/workflows/ios_build.yml index b9f386421fc2a..e947b602907d6 100644 --- a/.github/workflows/ios_build.yml +++ b/.github/workflows/ios_build.yml @@ -213,21 +213,20 @@ jobs: $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \ --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ //examples/objective-c/hello_world:app - # TODO(jpsim): Re-enable running the app # Run the app in the background and redirect logs. - # - name: 'Run app' - # if: steps.should_run.outputs.run_ci_job == 'true' - # env: - # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - # run: | - # cd mobile && ./bazelw run \ - # --config=ios \ - # $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \ - # --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ - # //examples/objective-c/hello_world:app &> /tmp/envoy.log & - # - run: sed '/received headers with status 200/q' <(touch /tmp/envoy.log && tail -F /tmp/envoy.log) - # if: steps.should_run.outputs.run_ci_job == 'true' - # name: 'Check connectivity' - # - run: cat /tmp/envoy.log - # if: ${{ failure() || cancelled() }} - # name: 'Log app run' + - name: 'Run app' + if: steps.should_run.outputs.run_ci_job == 'true' + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + cd mobile && ./bazelw run \ + --config=ios \ + $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \ + --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ + //examples/objective-c/hello_world:app &> /tmp/envoy.log & + - run: sed '/received headers with status 200/q' <(touch /tmp/envoy.log && tail -F /tmp/envoy.log) + if: steps.should_run.outputs.run_ci_job == 'true' + name: 'Check connectivity' + - run: cat /tmp/envoy.log + if: ${{ failure() || cancelled() }} + name: 'Log app run' diff --git a/.github/workflows/ios_tests.yml b/.github/workflows/ios_tests.yml index d95e62750457b..7bc2176fb403d 100644 --- a/.github/workflows/ios_tests.yml +++ b/.github/workflows/ios_tests.yml @@ -23,12 +23,15 @@ jobs: if: steps.should_run.outputs.run_ci_job == 'true' env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # This is run with ALLOW_TCP_LISTENERS due to the few remaining tests which use + # TCP listeners. run: | cd mobile && ./bazelw test \ --experimental_ui_max_stdouterr_bytes=10485760 \ --test_output=all \ --config=ios \ --build_tests_only \ + --copt=-DALLOW_TCP_LISTENERS \ $([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-macos") \ --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ //test/swift/... diff --git a/envoy/server/options.h b/envoy/server/options.h index ab90efdd1f0eb..5da6b566e8538 100644 --- a/envoy/server/options.h +++ b/envoy/server/options.h @@ -266,6 +266,11 @@ class Options { * responsibility of the caller to handle the duplicates. */ virtual const Stats::TagVector& statsTags() const PURE; + + /** + * @return the type of listener manager to create. + */ + virtual const std::string& listenerManager() const PURE; }; } // namespace Server diff --git a/mobile/docs/root/intro/version_history.rst b/mobile/docs/root/intro/version_history.rst index bfb3119212f4c..89fb43189b8cc 100644 --- a/mobile/docs/root/intro/version_history.rst +++ b/mobile/docs/root/intro/version_history.rst @@ -11,6 +11,7 @@ Breaking changes: - iOS: remove experimental option to force all connections to use IPv6. - kotlin: always use ``getaddrinfo`` DNS resolver. Remove ``addDNSFallbackNameservers``, ``enableDNSFilterUnroutableFamilies``, and ``enableDNSUseSystemResolver`` methods from the Kotlin engine builder. (:issue:`#2618 <2618>`) - Envoy Mobile's release builds compile without admin support by default. (``--define=admin_functionality=disabled``) (:issue`#2693 <2693>`) +- Envoy Mobile no longer supports TCP listeners by default. This support can be restored by building with ``--copt=-DALLOW_TCP_LISTENERS``. (:issue`#24363 <24363>`) Bugfixes: diff --git a/mobile/envoy_build_config/BUILD b/mobile/envoy_build_config/BUILD index 3b10cbfec0113..24145b25892bd 100644 --- a/mobile/envoy_build_config/BUILD +++ b/mobile/envoy_build_config/BUILD @@ -43,6 +43,7 @@ envoy_cc_library( "@envoy_mobile//library/common/extensions/filters/http/platform_bridge:config", "@envoy_mobile//library/common/extensions/filters/http/route_cache_reset:config", "@envoy_mobile//library/common/extensions/filters/http/socket_tag:config", + "@envoy_mobile//library/common/extensions/listener_managers/api_listener_manager:api_listener_manager_lib", "@envoy_mobile//library/common/extensions/retry/options/network_configuration:config", ], ) diff --git a/mobile/envoy_build_config/extension_registry.cc b/mobile/envoy_build_config/extension_registry.cc index 73edc3769c996..2ec89d4480276 100644 --- a/mobile/envoy_build_config/extension_registry.cc +++ b/mobile/envoy_build_config/extension_registry.cc @@ -32,6 +32,7 @@ #include "library/common/extensions/filters/http/network_configuration/config.h" #include "library/common/extensions/filters/http/platform_bridge/config.h" #include "library/common/extensions/filters/http/route_cache_reset/config.h" +#include "library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h" #include "library/common/extensions/retry/options/network_configuration/config.h" namespace Envoy { @@ -75,6 +76,7 @@ void ExtensionRegistry::registerFactories() { Router::forceRegisterUpstreamCodecFilterFactory(); Envoy::Network::forceRegisterGetAddrInfoDnsResolverFactory(); Envoy::Extensions::RequestId::forceRegisterUUIDRequestIDExtensionFactory(); + Envoy::Server::forceRegisterApiListenerManagerFactoryImpl(); // TODO: add a "force initialize" function to the upstream code, or clean up the upstream code // in such a way that does not depend on the statically initialized variable. diff --git a/mobile/envoy_build_config/extensions_build_config.bzl b/mobile/envoy_build_config/extensions_build_config.bzl index 3b8e6cd4fa883..361156118fd65 100644 --- a/mobile/envoy_build_config/extensions_build_config.bzl +++ b/mobile/envoy_build_config/extensions_build_config.bzl @@ -29,6 +29,8 @@ EXTENSIONS = { "envoy.transport_sockets.tls": "//source/extensions/transport_sockets/tls:config", "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:config", "envoy_mobile.cert_validator.platform_bridge_cert_validator": "@envoy_mobile//library/common/extensions/cert_validator/platform_bridge:config", + "envoy.listener_manager_impl.api": "@envoy_mobile//library/common/extensions/listener_managers/api_listener_manager:api_listener_manager_lib", + } WINDOWS_EXTENSIONS = {} LEGACY_ALWAYSLINK = 1 diff --git a/mobile/library/common/engine_common.cc b/mobile/library/common/engine_common.cc index c11b166b57851..20dba96ad3988 100644 --- a/mobile/library/common/engine_common.cc +++ b/mobile/library/common/engine_common.cc @@ -5,10 +5,17 @@ namespace Envoy { +// Allows for legacy TCP listener support when building with --copt=-DALLOW_TCP_LISTENERS +#ifndef ALLOW_TCP_LISTENERS +const char* listener_type = "envoy.listener_manager_impl.api"; +#else +const char* listener_type = "envoy.listener_manager_impl.default"; +#endif + std::string hotRestartVersion(bool) { return "disabled"; } EngineCommon::EngineCommon(int argc, const char* const* argv) - : options_(argc, argv, &hotRestartVersion, spdlog::level::info), + : options_(argc, argv, &hotRestartVersion, spdlog::level::info, listener_type), base_(options_, real_time_system_, default_listener_hooks_, prod_component_factory_, std::make_unique(), std::make_unique(), nullptr) { diff --git a/mobile/library/common/extensions/listener_managers/api_listener_manager/BUILD b/mobile/library/common/extensions/listener_managers/api_listener_manager/BUILD new file mode 100644 index 0000000000000..1edcd81a93d25 --- /dev/null +++ b/mobile/library/common/extensions/listener_managers/api_listener_manager/BUILD @@ -0,0 +1,26 @@ +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_extension( + name = "api_listener_manager_lib", + srcs = [ + "api_listener_manager.cc", + ], + hdrs = [ + "api_listener_manager.h", + ], + repository = "@envoy", + deps = [ + "@envoy//envoy/server:instance_interface", + "@envoy//envoy/server:listener_manager_interface", + "@envoy//source/server:api_listener_lib", + "@envoy//source/server:listener_manager_factory_lib", + ], +) diff --git a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc new file mode 100644 index 0000000000000..22cd0f28fa7e1 --- /dev/null +++ b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.cc @@ -0,0 +1,52 @@ +#include "library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h" + +#include "envoy/config/listener/v3/listener.pb.h" +#include "envoy/network/listener.h" + +#include "source/common/common/assert.h" +#include "source/common/common/fmt.h" +#include "source/common/config/utility.h" +#include "source/server/api_listener_impl.h" + +namespace Envoy { +namespace Server { + +ApiListenerManagerImpl::ApiListenerManagerImpl(Instance& server) : server_(server) {} +bool ApiListenerManagerImpl::addOrUpdateListener( + const envoy::config::listener::v3::Listener& config, const std::string&, bool added_via_api) { + std::string name; + if (!config.name().empty()) { + name = config.name(); + } else { + // TODO (soulxu): The random uuid name is bad for logging. We can use listening addresses in + // the log to improve that. + name = server_.api().randomGenerator().uuid(); + } + + // TODO(junr03): currently only one ApiListener can be installed via bootstrap to avoid having to + // build a collection of listeners, and to have to be able to warm and drain the listeners. In the + // future allow multiple ApiListeners, and allow them to be created via LDS as well as bootstrap. + if (config.has_api_listener()) { + if (config.has_internal_listener()) { + throw EnvoyException(fmt::format( + "error adding listener named '{}': api_listener and internal_listener cannot be both set", + name)); + } + if (!api_listener_ && !added_via_api) { + // TODO(junr03): dispatch to different concrete constructors when there are other + // ApiListenerImplBase derived classes. + api_listener_ = std::make_unique(config, server_, config.name()); + return true; + } else { + ENVOY_LOG(warn, "listener {} can not be added because currently only one ApiListener is " + "allowed, and it can only be added via bootstrap configuration"); + return false; + } + } + return false; +} + +REGISTER_FACTORY(ApiListenerManagerFactoryImpl, ListenerManagerFactory); + +} // namespace Server +} // namespace Envoy diff --git a/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h new file mode 100644 index 0000000000000..2980ea8c819a0 --- /dev/null +++ b/mobile/library/common/extensions/listener_managers/api_listener_manager/api_listener_manager.h @@ -0,0 +1,60 @@ +#pragma once + +#include + +#include "envoy/server/instance.h" +#include "envoy/server/listener_manager.h" + +#include "source/server/listener_manager_factory.h" + +namespace Envoy { +namespace Server { + +/** + * Implementation of a lightweight ListenerManager for Envoy Mobile. + * This does not handle downstream TCP / UDP connections but only the API listener. + */ +class ApiListenerManagerImpl : public ListenerManager, Logger::Loggable { +public: + explicit ApiListenerManagerImpl(Instance& server); + + // Server::ListenerManager + bool addOrUpdateListener(const envoy::config::listener::v3::Listener& config, + const std::string& version_info, bool added_via_api) override; + void createLdsApi(const envoy::config::core::v3::ConfigSource&, + const xds::core::v3::ResourceLocator*) override {} + std::vector> listeners(ListenerState) override { + return {}; + } + uint64_t numConnections() const override { return 0; } + bool removeListener(const std::string&) override { return true; } + void startWorkers(GuardDog&, std::function callback) override { callback(); } + void stopListeners(StopListenersType) override {} + void stopWorkers() override {} + void beginListenerUpdate() override {} + void endListenerUpdate(FailureStates&&) override {} + bool isWorkerStarted() override { return true; } + Http::Context& httpContext() { return server_.httpContext(); } + ApiListenerOptRef apiListener() override { + return api_listener_ ? ApiListenerOptRef(std::ref(*api_listener_)) : absl::nullopt; + } + +private: + Instance& server_; + ApiListenerPtr api_listener_; +}; + +class ApiListenerManagerFactoryImpl : public ListenerManagerFactory { +public: + std::unique_ptr + createListenerManager(Instance& server, std::unique_ptr&&, + WorkerFactory&, bool, Quic::QuicStatNames&) override { + return std::make_unique(server); + } + std::string name() const override { return "envoy.listener_manager_impl.api"; } +}; + +DECLARE_FACTORY(ApiListenerManagerFactoryImpl); + +} // namespace Server +} // namespace Envoy diff --git a/source/server/BUILD b/source/server/BUILD index 8ac3b74a2d75c..0ac4f2684e457 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -505,6 +505,7 @@ envoy_cc_library( name = "listener_manager_factory_lib", hdrs = ["listener_manager_factory.h"], deps = [ + ":worker_lib", "//envoy/server:factory_context_interface", "//envoy/server:listener_manager_interface", "//source/common/quic:quic_stat_names_lib", diff --git a/source/server/options_impl.cc b/source/server/options_impl.cc index eae78c65d513c..76987f2df6d8c 100644 --- a/source/server/options_impl.cc +++ b/source/server/options_impl.cc @@ -36,12 +36,16 @@ std::vector toArgsVector(int argc, const char* const* argv) { OptionsImpl::OptionsImpl(int argc, const char* const* argv, const HotRestartVersionCb& hot_restart_version_cb, - spdlog::level::level_enum default_log_level) - : OptionsImpl(toArgsVector(argc, argv), hot_restart_version_cb, default_log_level) {} + spdlog::level::level_enum default_log_level, + absl::string_view listener_manager) + : OptionsImpl(toArgsVector(argc, argv), hot_restart_version_cb, default_log_level, + listener_manager) {} OptionsImpl::OptionsImpl(std::vector args, const HotRestartVersionCb& hot_restart_version_cb, - spdlog::level::level_enum default_log_level) { + spdlog::level::level_enum default_log_level, + absl::string_view listener_manager) + : listener_manager_(listener_manager) { std::string log_levels_string = fmt::format("Log levels: {}", allowedLogLevels()); log_levels_string += fmt::format("\nDefault is [{}]", spdlog::level::level_string_views[default_log_level]); @@ -442,7 +446,8 @@ Server::CommandLineOptionsPtr OptionsImpl::toCommandLineOptions() const { OptionsImpl::OptionsImpl(const std::string& service_cluster, const std::string& service_node, const std::string& service_zone, spdlog::level::level_enum log_level) : log_level_(log_level), service_cluster_(service_cluster), service_node_(service_node), - service_zone_(service_zone) {} + service_zone_(service_zone), + listener_manager_(Config::ServerExtensionValues::get().DEFAULT_LISTENER) {} void OptionsImpl::disableExtensions(const std::vector& names) { for (const auto& name : names) { diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 3c4426ade7aef..c9a325e708e5e 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -10,6 +10,7 @@ #include "envoy/server/options.h" #include "source/common/common/logger.h" +#include "source/common/config/well_known_names.h" #include "spdlog/spdlog.h" @@ -31,8 +32,10 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable args, const HotRestartVersionCb& hot_restart_version_cb, - spdlog::level::level_enum default_log_level); + OptionsImpl( + std::vector args, const HotRestartVersionCb& hot_restart_version_cb, + spdlog::level::level_enum default_log_level, + absl::string_view listener_manager = Config::ServerExtensionValues::get().DEFAULT_LISTENER); // Default constructor; creates "reasonable" defaults, but desired values should be set // explicitly. @@ -157,6 +162,7 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable( - Config::ServerExtensionValues::get().DEFAULT_LISTENER) + Config::Utility::getAndCheckFactoryByName(options_.listenerManager()) .createListenerManager(*this, nullptr, worker_factory_, bootstrap_.enable_dispatcher_stats(), quic_stat_names_); diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index 28311cf689862..dfc46a741c22a 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -51,6 +51,7 @@ envoy_cc_mock( hdrs = ["options.h"], deps = [ "//envoy/server:options_interface", + "//source/common/config:well_known_names", "@envoy_api//envoy/admin/v3:pkg_cc_proto", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", ], diff --git a/test/mocks/server/options.cc b/test/mocks/server/options.cc index d947cccf1c9e5..37ba32421140a 100644 --- a/test/mocks/server/options.cc +++ b/test/mocks/server/options.cc @@ -2,6 +2,8 @@ #include "envoy/admin/v3/server_info.pb.h" +#include "source/common/config/well_known_names.h" + #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -46,6 +48,8 @@ MockOptions::MockOptions(const std::string& config_path) : config_path_(config_p ON_CALL(*this, socketPath()).WillByDefault(ReturnRef(socket_path_)); ON_CALL(*this, socketMode()).WillByDefault(ReturnPointee(&socket_mode_)); ON_CALL(*this, statsTags()).WillByDefault(ReturnRef(stats_tags_)); + ON_CALL(*this, listenerManager()) + .WillByDefault(ReturnRef(Config::ServerExtensionValues::get().DEFAULT_LISTENER)); } MockOptions::~MockOptions() = default; diff --git a/test/mocks/server/options.h b/test/mocks/server/options.h index fb64f4f1b6642..8b6ef95596b81 100644 --- a/test/mocks/server/options.h +++ b/test/mocks/server/options.h @@ -53,6 +53,7 @@ class MockOptions : public Options { MOCK_METHOD(const std::string&, socketPath, (), (const)); MOCK_METHOD(mode_t, socketMode, (), (const)); MOCK_METHOD((const Stats::TagVector&), statsTags, (), (const)); + MOCK_METHOD(const std::string&, listenerManager, (), (const)); std::string config_path_; envoy::config::bootstrap::v3::Bootstrap config_proto_;