Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/cc_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ jobs:
- env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
name: 'Run tests'
# Regression test using the new API listener. TODO(#2711) clean up.
run: |
cd mobile && ./bazelw test \
--action_env=LD_LIBRARY_PATH \
--test_output=all \
--copt=-DUSE_API_LISTENER \
$([ -z $GITHUB_TOKEN ] || echo "--config=remote-ci-linux") \
--remote_header="Authorization=Bearer $GITHUB_TOKEN" \
//test/cc/...
5 changes: 5 additions & 0 deletions envoy/server/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment thread
alyssawilk marked this conversation as resolved.
};

} // namespace Server
Expand Down
3 changes: 2 additions & 1 deletion mobile/envoy_build_config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ envoy_cc_library(
hdrs = ["extension_registry.h"],
repository = "@envoy",
deps = [
"@envoy//source/extensions/listener_managers/listener_manager:listener_manager_lib", #TODO(2711) remove
"extension_registry_platform_additions",
"@envoy//source/common/network:socket_lib",
"@envoy//source/common/router:upstream_codec_filter_lib",
Expand All @@ -27,7 +28,6 @@ envoy_cc_library(
"@envoy//source/extensions/filters/http/router:config",
"@envoy//source/extensions/filters/network/http_connection_manager:config",
"@envoy//source/extensions/http/header_formatters/preserve_case:config",
"@envoy//source/extensions/listener_managers/listener_manager:listener_manager_lib",
"@envoy//source/extensions/network/dns_resolver/getaddrinfo:config",
"@envoy//source/extensions/request_id/uuid:config",
"@envoy//source/extensions/stat_sinks/metrics_service:config",
Expand All @@ -44,6 +44,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",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to my other comment - should we include this dep only if the new compiler/build flag is set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends. if the long term plan is to have a bazel flag to have it optional, we can add it now. if long term we want to say E-M is a client library not a proxy, I'm inclined to not do the busy-work which I'd then have to unwind, and accept the additional include in the interim (we can remove it in-housE)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually having thought a bit more I think it is sensible to have a flag but I would like to add it when we flip the default, so have one for "enable TCP listener" where right now we have compile support for "enable API listener"

"@envoy_mobile//library/common/extensions/retry/options/network_configuration:config",
] + envoy_select_enable_http3(
[
Expand Down
2 changes: 2 additions & 0 deletions mobile/envoy_build_config/extension_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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 {
Expand Down Expand Up @@ -81,6 +82,7 @@ void ExtensionRegistry::registerFactories() {
Envoy::Network::forceRegisterGetAddrInfoDnsResolverFactory();
Envoy::Extensions::RequestId::forceRegisterUUIDRequestIDExtensionFactory();
Envoy::Server::forceRegisterDefaultListenerManagerFactoryImpl();
Envoy::Server::forceRegisterApiListenerManagerFactoryImpl();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we force register in here only if DUSE_API_LISTENER is set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to do that but I think it requires a bazel flag and additional plumbing to be useful given the library include


#ifdef ENVOY_ENABLE_QUIC
Quic::forceRegisterQuicServerTransportSocketConfigFactory();
Expand Down
2 changes: 2 additions & 0 deletions mobile/envoy_build_config/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 8 additions & 1 deletion mobile/library/common/engine_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@

namespace Envoy {

// Allows for using the new API listener when building with --copt=-DUSE_API_LISTENER
#ifdef USE_API_LISTENER
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<PlatformImpl>(), std::make_unique<Random::RandomGeneratorImpl>(),
nullptr) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -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<HttpApiListener>(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
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#pragma once

#include <memory>

#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<Logger::Id::config> {
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<std::reference_wrapper<Network::ListenerConfig>> listeners(ListenerState) override {
return {};
}
uint64_t numConnections() const override { return 0; }
bool removeListener(const std::string&) override { return true; }
void startWorkers(GuardDog&, std::function<void()> 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<ListenerManager>
createListenerManager(Instance& server, std::unique_ptr<ListenerComponentFactory>&&,
WorkerFactory&, bool, Quic::QuicStatNames&) override {
return std::make_unique<ApiListenerManagerImpl>(server);
}
std::string name() const override { return "envoy.listener_manager_impl.api"; }
};

DECLARE_FACTORY(ApiListenerManagerFactoryImpl);

} // namespace Server
} // namespace Envoy
1 change: 1 addition & 0 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 9 additions & 4 deletions source/server/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,16 @@ std::vector<std::string> 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<std::string> 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]);
Expand Down Expand Up @@ -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<std::string>& names) {
for (const auto& name : names) {
Expand Down
15 changes: 11 additions & 4 deletions source/server/options_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -31,8 +32,10 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
* @throw MalformedArgvException if something is wrong with the arguments (invalid flag or flag
* value). The caller should call exit(1) after any necessary cleanup.
*/
OptionsImpl(int argc, const char* const* argv, const HotRestartVersionCb& hot_restart_version_cb,
spdlog::level::level_enum default_log_level);
OptionsImpl(
int argc, const char* const* argv, const HotRestartVersionCb& hot_restart_version_cb,
spdlog::level::level_enum default_log_level,
absl::string_view listener_manager = Config::ServerExtensionValues::get().DEFAULT_LISTENER);

/**
* @throw NoServingException if Envoy has already done everything specified by the args (e.g.
Expand All @@ -41,8 +44,10 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
* @throw MalformedArgvException if something is wrong with the arguments (invalid flag or flag
* value). The caller should call exit(1) after any necessary cleanup.
*/
OptionsImpl(std::vector<std::string> args, const HotRestartVersionCb& hot_restart_version_cb,
spdlog::level::level_enum default_log_level);
OptionsImpl(
std::vector<std::string> 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.
Expand Down Expand Up @@ -157,6 +162,7 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
}
uint32_t count() const;
const std::string& socketPath() const override { return socket_path_; }
const std::string& listenerManager() const override { return listener_manager_; }
mode_t socketMode() const override { return socket_mode_; }

/**
Expand Down Expand Up @@ -217,6 +223,7 @@ class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::I
// enable_fine_grain_logging_.
bool enable_fine_grain_logging_ = false;
std::string socket_path_{"@envoy_domain_socket"};
std::string listener_manager_;
mode_t socket_mode_{0};
};

Expand Down
4 changes: 1 addition & 3 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -607,11 +607,9 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add
Network::SocketInterfaceSingleton::initialize(sock);
}
}

// Workers get created first so they register for thread local updates.
listener_manager_ =
Config::Utility::getAndCheckFactoryByName<ListenerManagerFactory>(
Config::ServerExtensionValues::get().DEFAULT_LISTENER)
Config::Utility::getAndCheckFactoryByName<ListenerManagerFactory>(options_.listenerManager())
.createListenerManager(*this, nullptr, worker_factory_,
bootstrap_.enable_dispatcher_stats(), quic_stat_names_);

Expand Down
1 change: 1 addition & 0 deletions test/mocks/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
4 changes: 4 additions & 0 deletions test/mocks/server/options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions test/mocks/server/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down