diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc index 97ea85ca4..dc7959c37 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -299,7 +299,8 @@ class NetworkPolicyMapImpl : public ManagedGrpcSubscription { public: friend class PortNetworkPolicyRule; NetworkPolicyMapImpl(Server::Configuration::FactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, bool subscribe); + const envoy::config::core::v3::ConfigSource& config_source, bool subscribe, + bool policy_secret_cache_enabled); ~NetworkPolicyMapImpl() override; // Config::SubscriptionCallbacks @@ -309,10 +310,29 @@ class NetworkPolicyMapImpl : public ManagedGrpcSubscription { const Protobuf::RepeatedPtrField& removed_resources, const std::string& system_version_info) override; + std::shared_ptr sharedFromThis() { + return std::static_pointer_cast( + ManagedGrpcSubscription::shared_from_this()); + } + + std::weak_ptr weakFromThis() { return sharedFromThis(); } + Server::Configuration::TransportSocketFactoryContext& transportFactoryContext() const { return *transport_factory_context_; } + DownstreamTLSContextSharedPtr getDownstreamTlsContext(const cilium::TLSContext& config) const { + return policy_secret_cache_.getOrCreateDownstream(config); + } + + UpstreamTLSContextSharedPtr getUpstreamTlsContext(const cilium::TLSContext& config) const { + return policy_secret_cache_.getOrCreateUpstream(config); + } + + SecretWatcherSharedPtr getSecretWatcher(const std::string& sds_name) const { + return policy_secret_cache_.getOrCreateSecretWatcher(sds_name); + } + Regex::Engine& regexEngine() const { return context_.regexEngine(); } void tlsWrapperMissingPolicyInc() const { stats_.tls_wrapper_missing_policy_.inc(); } @@ -402,6 +422,9 @@ class NetworkPolicyMapImpl : public ManagedGrpcSubscription { Init::TargetImpl init_target_; std::shared_ptr transport_factory_context_; + // Declared after transport_factory_context_ so that the cache, which retains a shared reference + // to the factory context, is destroyed first. + PolicySecretCache policy_secret_cache_; // Between policy updates, keep a dormant init manager installed so unexpected late init-target // registrations do not hit the listener's already-initialized manager. If it accumulates targets // while parked, log and rotate it out before making it active again. @@ -440,8 +463,7 @@ class HeaderMatch : public Logger::Loggable { : name_(config.name()), value_(config.value()), match_action_(config.match_action()), mismatch_action_(config.mismatch_action()) { if (!config.value_sds_secret().empty()) { - secret_ = std::make_unique( - parent.transportFactoryContext(), parent.getConfigSource(), config.value_sds_secret()); + secret_ = parent.getSecretWatcher(config.value_sds_secret()); } } @@ -565,7 +587,9 @@ class HeaderMatch : public Logger::Loggable { std::string value_; cilium::HeaderMatch::MatchAction match_action_; cilium::HeaderMatch::MismatchAction mismatch_action_; - SecretWatcherPtr secret_; + // Shared state contains only the SDS resource name and its atomically published value. Header + // matching behavior and the optional inline fallback remain local to this HeaderMatch. + SecretWatcherSharedPtr secret_; }; class HttpNetworkPolicyRule : public Logger::Loggable { @@ -797,14 +821,10 @@ class PortNetworkPolicyRule : public Logger::Loggable { remotes_.emplace(remote); } if (rule.has_downstream_tls_context()) { - auto config = rule.downstream_tls_context(); - server_context_ = std::make_unique(parent.transportFactoryContext(), - parent.getConfigSource(), config); + server_context_ = parent.getDownstreamTlsContext(rule.downstream_tls_context()); } if (rule.has_upstream_tls_context()) { - auto config = rule.upstream_tls_context(); - client_context_ = std::make_unique(parent.transportFactoryContext(), - parent.getConfigSource(), config); + client_context_ = parent.getUpstreamTlsContext(rule.upstream_tls_context()); } for (const auto& sni : rule.server_names()) { ENVOY_LOG(trace, "Cilium L7 PortNetworkPolicyRule(): {} SNI {} by rule {}", verdict_, sni, @@ -1894,9 +1914,10 @@ void ResourceMapOverlay::erasePolicyResource( // This is used directly for testing with a file-based subscription NetworkPolicyMap::NetworkPolicyMap(Server::Configuration::FactoryContext& context, const envoy::config::core::v3::ConfigSource& config_source, - bool subscribe) + bool subscribe, bool policy_secret_cache_enabled) : context_(context.serverFactoryContext()) { - impl_ = std::make_shared(context, config_source, subscribe); + impl_ = std::make_shared(context, config_source, subscribe, + policy_secret_cache_enabled); } NetworkPolicyMap::~NetworkPolicyMap() { @@ -1941,7 +1962,8 @@ NetworkPolicyMap::getPolicyInstanceShared(const std::string& endpoint_policy_nam NetworkPolicyMapImpl::NetworkPolicyMapImpl( Server::Configuration::FactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, bool subscribe) + const envoy::config::core::v3::ConfigSource& config_source, bool subscribe, + bool policy_secret_cache_enabled) : ManagedGrpcSubscription( NetworkPolicyTypeUrl, []() { return std::make_shared(); }, config_source, context.serverFactoryContext(), @@ -1958,6 +1980,8 @@ NetworkPolicyMapImpl::NetworkPolicyMapImpl( transport_factory_context_( std::make_shared( context_, scope(), context_.messageValidationContext().dynamicValidationVisitor())), + policy_secret_cache_(transport_factory_context_, std::cref(getConfigSource()), + policy_secret_cache_enabled), parked_init_manager_(std::make_unique("Cilium NetworkPolicyMap parked")), stats_{ALL_CILIUM_POLICY_COUNTERS(POOL_COUNTER(*policy_stats_scope_)) ALL_CILIUM_POLICY_GAUGES(POOL_GAUGE(*policy_stats_scope_))} { @@ -2107,9 +2131,18 @@ void NetworkPolicyMapImpl::scheduleDeferredDeletion(const PolicyMapSnapshot* old if (old_policy_map == nullptr) { return; } - runAfterAllThreads([old_policy_map]() { + const auto weak_this = weakFromThis(); + + runAfterAllThreads([old_policy_map, weak_this]() { // Clean-up in the main thread after all worker threads have scheduled. delete old_policy_map; + + // Prune only after the old policy snapshot has released its secret-derived resource + // references. This worker-quiescence completion callback already runs on the main dispatcher, + // outside the policy update call. + if (auto policy_map = weak_this.lock()) { + policy_map->policy_secret_cache_.prune(); + } }); } @@ -2148,6 +2181,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate( // so open it before the workers get a chance to enforce policy on the new IDs. if (is_new_stream) { ENVOY_LOG(info, "New NetworkPolicy stream {}", stream_generation); + policy_secret_cache_.reset(); reopenIpcache(); } @@ -2241,6 +2275,7 @@ absl::Status NetworkPolicyMapImpl::onConfigUpdate( // so open it before the workers get a chance to enforce policy on the new IDs. if (is_new_stream) { ENVOY_LOG(info, "New NetworkPolicy stream {}", stream_generation); + policy_secret_cache_.reset(); reopenIpcache(); } diff --git a/cilium/network_policy.h b/cilium/network_policy.h index 1c02f4d39..e6d096b80 100644 --- a/cilium/network_policy.h +++ b/cilium/network_policy.h @@ -197,7 +197,7 @@ class NetworkPolicyMap : public Singleton::Instance, public Logger::Loggable #include +#include +#include +#include #include #include @@ -17,9 +20,11 @@ #include "source/common/common/logger.h" #include "source/common/common/thread.h" #include "source/common/config/datasource.h" +#include "source/common/protobuf/utility.h" #include "source/common/tls/context_config_impl.h" #include "source/common/tls/server_context_config_impl.h" +#include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/synchronization/mutex.h" #include "cilium/api/npds.pb.h" @@ -103,7 +108,7 @@ TLSContext::TLSContext(Server::Configuration::TransportSocketFactoryContext& con namespace { -void setCommonConfig(const cilium::TLSContext config, +void setCommonConfig(const cilium::TLSContext& config, const envoy::config::core::v3::ConfigSource& config_source, envoy::extensions::transport_sockets::tls::v3::CommonTlsContext* tls_context) { if (!config.validation_context_sds_secret().empty()) { @@ -144,7 +149,7 @@ void setCommonConfig(const cilium::TLSContext config, DownstreamTLSContext::DownstreamTLSContext( Server::Configuration::TransportSocketFactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, const cilium::TLSContext config) + const envoy::config::core::v3::ConfigSource& config_source, const cilium::TLSContext& config) : TLSContext(context, "server") { // Server config always needs the TLS certificate to present to the client if (config.tls_sds_secret().empty() && config.certificate_chain().empty()) { @@ -194,7 +199,7 @@ DownstreamTLSContext::DownstreamTLSContext( UpstreamTLSContext::UpstreamTLSContext( Server::Configuration::TransportSocketFactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, cilium::TLSContext config) + const envoy::config::core::v3::ConfigSource& config_source, const cilium::TLSContext& config) : TLSContext(context, "client") { // Client context always needs the trusted CA for server certificate validation // TODO: Default to system default trusted CAs? @@ -240,5 +245,132 @@ UpstreamTLSContext::UpstreamTLSContext( } } +namespace { + +constexpr std::chrono::seconds PolicySecretCachePruneInterval{1}; + +template void pruneExpired(Cache& cache) { + for (auto it = cache.begin(); it != cache.end();) { + if (it->second.expired()) { + auto expired = it++; + cache.erase(expired); + } else { + ++it; + } + } +} + +template +auto getOrCreate(bool caching_enabled, Cache& cache, const Key& key, Factory&& factory) { + if (!caching_enabled) { + return factory(); + } + + auto it = cache.find(key); + if (it != cache.end()) { + if (auto cached = it->second.lock()) { + return cached; + } + cache.erase(it); + } + + auto value = factory(); + cache.emplace(key, value); + return value; +} + +} // namespace + +class PolicySecretCache::Impl { +public: + template + using TLSContextMap = + absl::flat_hash_map, MessageUtil, MessageUtil>; + using SecretWatcherMap = absl::flat_hash_map>; + + Impl(std::shared_ptr context, + std::reference_wrapper config_source, + bool caching_enabled) + : context_(std::move(context)), config_source_(config_source), + caching_enabled_(caching_enabled), + next_prune_time_(context_->serverFactoryContext().timeSource().monotonicTime() + + PolicySecretCachePruneInterval) {} + + void reset() { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + downstream_cache_.clear(); + upstream_cache_.clear(); + secret_watcher_cache_.clear(); + next_prune_time_ = context_->serverFactoryContext().timeSource().monotonicTime() + + PolicySecretCachePruneInterval; + } + + void prune() { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + if (!caching_enabled_) { + return; + } + const MonotonicTime now = context_->serverFactoryContext().timeSource().monotonicTime(); + if (now < next_prune_time_) { + return; + } + + // Advance the deadline before scanning so that frequent policy updates cannot trigger more + // than one complete cache scan per interval. + next_prune_time_ = now + PolicySecretCachePruneInterval; + pruneExpired(downstream_cache_); + pruneExpired(upstream_cache_); + pruneExpired(secret_watcher_cache_); + } + + std::shared_ptr context_; + std::reference_wrapper config_source_; + const bool caching_enabled_; + TLSContextMap downstream_cache_; + TLSContextMap upstream_cache_; + SecretWatcherMap secret_watcher_cache_; + MonotonicTime next_prune_time_; +}; + +PolicySecretCache::PolicySecretCache( + std::shared_ptr context, + std::reference_wrapper config_source, + bool caching_enabled) + : impl_(std::make_unique(std::move(context), config_source, caching_enabled)) {} + +PolicySecretCache::~PolicySecretCache() = default; + +void PolicySecretCache::reset() { impl_->reset(); } + +void PolicySecretCache::prune() { impl_->prune(); } + +DownstreamTLSContextSharedPtr +PolicySecretCache::getOrCreateDownstream(const cilium::TLSContext& config) const { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + return getOrCreate(impl_->caching_enabled_, impl_->downstream_cache_, config, [this, &config]() { + return DownstreamTLSContextSharedPtr( + new DownstreamTLSContext(*impl_->context_, impl_->config_source_.get(), config)); + }); +} + +UpstreamTLSContextSharedPtr +PolicySecretCache::getOrCreateUpstream(const cilium::TLSContext& config) const { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + return getOrCreate(impl_->caching_enabled_, impl_->upstream_cache_, config, [this, &config]() { + return UpstreamTLSContextSharedPtr( + new UpstreamTLSContext(*impl_->context_, impl_->config_source_.get(), config)); + }); +} + +SecretWatcherSharedPtr +PolicySecretCache::getOrCreateSecretWatcher(const std::string& sds_name) const { + ASSERT_IS_MAIN_OR_TEST_THREAD(); + return getOrCreate(impl_->caching_enabled_, impl_->secret_watcher_cache_, sds_name, + [this, &sds_name]() { + return std::make_shared( + *impl_->context_, impl_->config_source_.get(), sds_name); + }); +} + } // namespace Cilium } // namespace Envoy diff --git a/cilium/secret_watcher.h b/cilium/secret_watcher.h index ffa90b91c..877454e5d 100644 --- a/cilium/secret_watcher.h +++ b/cilium/secret_watcher.h @@ -26,6 +26,8 @@ namespace Envoy { namespace Cilium { +class PolicySecretCache; + // Facility for SDS config override for testing using GetSdsConfigFunc = std::function; @@ -53,7 +55,7 @@ class SecretWatcher : public Logger::Loggable { Secret::GenericSecretConfigProviderSharedPtr secret_provider_; Envoy::Common::CallbackHandlePtr update_secret_; }; -using SecretWatcherPtr = std::unique_ptr; +using SecretWatcherSharedPtr = std::shared_ptr; // private base class for the common bits class TLSContext : public Logger::Loggable { @@ -72,9 +74,6 @@ class TLSContext : public Logger::Loggable { class DownstreamTLSContext : protected TLSContext { public: - DownstreamTLSContext(Server::Configuration::TransportSocketFactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, - const cilium::TLSContext config); ~DownstreamTLSContext() { manager_.removeContext(server_context_); } const Ssl::ContextConfig& getTlsContextConfig() const { return *server_config_; } @@ -85,6 +84,12 @@ class DownstreamTLSContext : protected TLSContext { } private: + friend class PolicySecretCache; + + DownstreamTLSContext(Server::Configuration::TransportSocketFactoryContext& context, + const envoy::config::core::v3::ConfigSource& config_source, + const cilium::TLSContext& config); + Ssl::ServerContextConfigPtr server_config_; std::vector server_names_; Ssl::ServerContextSharedPtr server_context_ ABSL_GUARDED_BY(ssl_context_mutex_); @@ -93,9 +98,6 @@ using DownstreamTLSContextSharedPtr = std::shared_ptr; class UpstreamTLSContext : protected TLSContext { public: - UpstreamTLSContext(Server::Configuration::TransportSocketFactoryContext& context, - const envoy::config::core::v3::ConfigSource& config_source, - cilium::TLSContext config); ~UpstreamTLSContext() { manager_.removeContext(client_context_); } const Ssl::ContextConfig& getTlsContextConfig() const { return *client_config_; } @@ -105,10 +107,54 @@ class UpstreamTLSContext : protected TLSContext { } private: + friend class PolicySecretCache; + + UpstreamTLSContext(Server::Configuration::TransportSocketFactoryContext& context, + const envoy::config::core::v3::ConfigSource& config_source, + const cilium::TLSContext& config); + Ssl::ClientContextConfigPtr client_config_; Ssl::ClientContextSharedPtr client_context_ ABSL_GUARDED_BY(ssl_context_mutex_); }; using UpstreamTLSContextSharedPtr = std::shared_ptr; +// Main-thread-only cache of secret-derived resources constructed for NetworkPolicy rules. The +// cache owns the factory context and observes the active NPDS config source for the lifetime of its +// owning NetworkPolicyMapImpl. +class PolicySecretCache { +public: + PolicySecretCache( + std::shared_ptr context, + std::reference_wrapper config_source, + bool caching_enabled); + ~PolicySecretCache(); + + PolicySecretCache(const PolicySecretCache&) = delete; + PolicySecretCache& operator=(const PolicySecretCache&) = delete; + + // Must only be called from Envoy's main thread (or the designated test thread). + // Clears all resources cached for the previous NPDS stream generation. + void reset(); + + // Must only be called from Envoy's main thread (or the designated test thread), after a policy + // update. Pruning is internally rate-limited. + void prune(); + + // Must only be called from Envoy's main thread (or the designated test thread). + DownstreamTLSContextSharedPtr getOrCreateDownstream(const cilium::TLSContext& config) const; + + // Must only be called from Envoy's main thread (or the designated test thread). + UpstreamTLSContextSharedPtr getOrCreateUpstream(const cilium::TLSContext& config) const; + + // Must only be called from Envoy's main thread (or the designated test thread). + // Within one NPDS stream, the SDS resource name uniquely identifies the effective config source + // and generic secret value observed by all HeaderMatches using that name. + SecretWatcherSharedPtr getOrCreateSecretWatcher(const std::string& sds_name) const; + +private: + class Impl; + std::unique_ptr impl_; +}; + } // namespace Cilium } // namespace Envoy diff --git a/tests/BUILD b/tests/BUILD index be476c796..194a7fa3e 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -1,5 +1,7 @@ load( "@envoy//bazel:envoy_build_system.bzl", + "envoy_benchmark_test", + "envoy_cc_benchmark_binary", "envoy_cc_test", "envoy_cc_test_library", "envoy_package", @@ -134,6 +136,31 @@ envoy_cc_test( ], ) +envoy_cc_benchmark_binary( + name = "cilium_network_policy_benchmark", + srcs = ["cilium_network_policy_benchmark.cc"], + data = ["@envoy//test/config/integration/certs:upstreamcacert.pem"], + external_deps = ["bazel_runfiles"], + repository = "@envoy", + deps = [ + ":bpf_metadata_lib", + ":cilium_test_peer_lib", + "//cilium:network_policy_lib", + "@benchmark", + "@envoy//source/common/memory:stats_lib", + "@envoy//source/common/secret:sds_api_lib", + "@envoy//test/mocks/secret:secret_mocks", + "@envoy//test/mocks/server:factory_context_mocks", + "@envoy//test/test_common:environment_lib", + ], +) + +envoy_benchmark_test( + name = "cilium_network_policy_benchmark_test", + benchmark_binary = "cilium_network_policy_benchmark", + repository = "@envoy", +) + envoy_cc_test( name = "bpf_metadata_config_test", srcs = ["bpf_metadata_config_test.cc"], diff --git a/tests/cilium_network_policy_benchmark.cc b/tests/cilium_network_policy_benchmark.cc new file mode 100644 index 000000000..725915f7c --- /dev/null +++ b/tests/cilium_network_policy_benchmark.cc @@ -0,0 +1,256 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "envoy/common/optref.h" +#include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/extensions/transport_sockets/tls/v3/secret.pb.h" +#include "envoy/init/manager.h" +#include "envoy/server/factory_context.h" +#include "envoy/service/discovery/v3/discovery.pb.h" + +#include "source/common/config/decoded_resource_impl.h" +#include "source/common/memory/stats.h" +#include "source/common/secret/sds_api.h" // NOLINT + +#include "test/mocks/secret/mocks.h" +#include "test/mocks/server/factory_context.h" +#include "test/test_common/environment.h" + +#include "absl/strings/str_cat.h" +#include "benchmark/benchmark.h" +#include "cilium/api/npds.pb.h" +#include "cilium/network_policy.h" +#include "tests/cilium_test_peer.h" +#include "tools/cpp/runfiles/runfiles.h" + +namespace Envoy { +namespace Cilium { + +// Cilium XDS API config source. Used for all Cilium XDS. +extern const envoy::config::core::v3::ConfigSource CILIUM_XDS_API_CONFIG; + +namespace { +using testing::_; +using testing::Invoke; +using testing::NiceMock; +using testing::ReturnRef; + +constexpr uint64_t kPolicyCount = 1000; +constexpr char kSharedValidationContextSdsSecret[] = "benchmark-shared-ca"; +constexpr char kSharedHeaderSdsSecret[] = "benchmark-shared-header"; +constexpr char kSdsVersion[] = "benchmark-secret-version"; +constexpr size_t kHeaderSecretValueSize = 256; + +envoy::service::discovery::v3::DiscoveryResponse makeNpdsResponse(bool use_header_secret) { + envoy::service::discovery::v3::DiscoveryResponse response; + response.set_version_info("benchmark-version"); + + for (uint64_t index = 0; index < kPolicyCount; ++index) { + cilium::NetworkPolicy policy; + policy.set_endpoint_id(index + 1); + policy.add_endpoint_ips(absl::StrCat("10.0.", index / 250, ".", index % 250 + 1)); + + auto* port_policy = use_header_secret ? policy.add_ingress_per_port_policies() + : policy.add_egress_per_port_policies(); + port_policy->set_port(use_header_secret ? 80 : 443); + auto* rule = port_policy->add_rules(); + rule->add_remote_policies(42); + if (use_header_secret) { + auto* header_match = rule->mutable_http_rules()->add_http_rules()->add_header_matches(); + header_match->set_name("x-benchmark-secret"); + header_match->set_value_sds_secret(kSharedHeaderSdsSecret); + } else { + rule->mutable_upstream_tls_context()->set_validation_context_sds_secret( + kSharedValidationContextSdsSecret); + } + + response.add_resources()->PackFrom(policy); + } + return response; +} + +void processNpdsUpdateWith1000Policies(benchmark::State& state, bool policy_secret_cache_enabled, + bool use_header_secret) { + NiceMock factory_context; + NiceMock secret_manager; + ON_CALL(factory_context.server_factory_context_, secretManager()) + .WillByDefault(ReturnRef(secret_manager)); + + std::string runfiles_error; + std::error_code executable_path_error; + const std::filesystem::path executable_path = + std::filesystem::read_symlink("/proc/self/exe", executable_path_error); + if (executable_path_error) { + state.SkipWithError(executable_path_error.message()); + return; + } + std::unique_ptr runfiles( + bazel::tools::cpp::runfiles::Runfiles::Create(executable_path.string(), &runfiles_error)); + if (runfiles == nullptr) { + state.SkipWithError(runfiles_error); + return; + } + TestEnvironment::setRunfiles(runfiles.get()); + const std::string trusted_ca = TestEnvironment::readFileToStringForTest( + TestEnvironment::runfilesPath("test/config/integration/certs/upstreamcacert.pem")); + + // Model an SDS resource that has already delivered the shared CA. The test policy contains only + // the SDS resource name; the CA payload is owned once by this shared dynamic provider. + auto validation_context_provider = Secret::CertificateValidationContextSdsApi::create( + factory_context.server_factory_context_, CILIUM_XDS_API_CONFIG, + kSharedValidationContextSdsSecret, []() {}, false); + auto validation_context_secret = + std::make_unique(); + validation_context_secret->set_name(kSharedValidationContextSdsSecret); + validation_context_secret->mutable_validation_context()->mutable_trusted_ca()->set_inline_string( + trusted_ca); + Config::DecodedResourcesWrapper decoded_sds_resources; + decoded_sds_resources.pushBack(std::make_unique( + std::move(validation_context_secret), kSharedValidationContextSdsSecret, + std::vector{}, kSdsVersion)); + const auto sds_status = static_cast(*validation_context_provider) + .onConfigUpdate(decoded_sds_resources.refvec_, kSdsVersion); + if (!sds_status.ok()) { + state.SkipWithError(sds_status.ToString()); + return; + } + + ON_CALL(secret_manager, findOrCreateCertificateValidationContextProvider(_, _, _, _)) + .WillByDefault( + Invoke([validation_context_provider]( + const envoy::config::core::v3::ConfigSource&, const std::string&, + Server::Configuration::ServerFactoryContext&, Init::Manager& init_manager) { + // SecretManagerImpl adds the shared provider target to each caller's init manager even + // when the provider already exists. + init_manager.add(*validation_context_provider->initTarget()); + return validation_context_provider; + })); + + // Use a synthetic, non-credential value representative of an authorization-style HTTP header. + // This is intentionally much smaller than the CA payload used by the TLS context scenario. + const std::string header_secret_value(kHeaderSecretValueSize, 'x'); + auto generic_secret_provider = Secret::GenericSecretSdsApi::create( + factory_context.server_factory_context_, CILIUM_XDS_API_CONFIG, kSharedHeaderSdsSecret, + []() {}, false); + auto generic_secret = std::make_unique(); + generic_secret->set_name(kSharedHeaderSdsSecret); + generic_secret->mutable_generic_secret()->mutable_secret()->set_inline_string( + header_secret_value); + Config::DecodedResourcesWrapper decoded_generic_secret_resources; + decoded_generic_secret_resources.pushBack(std::make_unique( + std::move(generic_secret), kSharedHeaderSdsSecret, std::vector{}, kSdsVersion)); + const auto generic_secret_status = + static_cast(*generic_secret_provider) + .onConfigUpdate(decoded_generic_secret_resources.refvec_, kSdsVersion); + if (!generic_secret_status.ok()) { + state.SkipWithError(generic_secret_status.ToString()); + return; + } + + ON_CALL(secret_manager, findOrCreateGenericSecretProvider(_, _, _, _)) + .WillByDefault(Invoke([generic_secret_provider](const envoy::config::core::v3::ConfigSource&, + const std::string&, + Server::Configuration::ServerFactoryContext&, + OptRef init_manager) { + if (init_manager.has_value()) { + init_manager->add(*generic_secret_provider->initTarget()); + } else { + generic_secret_provider->start(); + } + return generic_secret_provider; + })); + + const auto response = makeNpdsResponse(use_header_secret); + NetworkPolicyDecoder decoder; + auto decoded_resources_or_error = Config::DecodedResourcesWrapper::create( + decoder, response.resources(), response.version_info()); + if (!decoded_resources_or_error.ok()) { + state.SkipWithError(decoded_resources_or_error.status().ToString()); + return; + } + auto decoded_resources = std::move(decoded_resources_or_error.value()); + uint64_t total_retained_memory = 0; + auto& config_tracker_callbacks = + factory_context.server_factory_context_.admin_.config_tracker_.config_tracker_callbacks_; + + // Exclude one-time TLS/library initialization from the retained-memory comparison and warm the + // same code path before Google Benchmark calibrates the measured iterations. + { + config_tracker_callbacks.clear(); + auto policy_map = std::make_shared(factory_context, CILIUM_XDS_API_CONFIG, + false, policy_secret_cache_enabled); + CiliumTestPeer::resetStream(*policy_map); + const auto warmup_status = + CiliumTestPeer::subscriptionCallbacks(*policy_map) + .onConfigUpdate(decoded_resources->refvec_, response.version_info()); + if (!warmup_status.ok()) { + state.SkipWithError(warmup_status.ToString()); + return; + } + } + + for (auto _ : state) { // NOLINT: Silences warning about dead store. + // Recreate the policy map outside the measured interval so that every iteration measures a + // first update, without whole-policy reuse from a previous iteration. The memory counter is + // retained live heap allocated by the update; response construction and decoding are excluded. + state.PauseTiming(); + // MockConfigTracker does not remove callback keys when its EntryOwner is destroyed. + config_tracker_callbacks.clear(); + auto policy_map = std::make_shared(factory_context, CILIUM_XDS_API_CONFIG, + false, policy_secret_cache_enabled); + CiliumTestPeer::resetStream(*policy_map); + auto& callbacks = CiliumTestPeer::subscriptionCallbacks(*policy_map); + const uint64_t memory_before = Memory::Stats::totalCurrentlyAllocated(); + state.ResumeTiming(); + + const auto status = + callbacks.onConfigUpdate(decoded_resources->refvec_, response.version_info()); + benchmark::DoNotOptimize(status); + + state.PauseTiming(); + const uint64_t memory_after = Memory::Stats::totalCurrentlyAllocated(); + policy_map.reset(); + if (!status.ok()) { + state.ResumeTiming(); + state.SkipWithError(status.ToString()); + return; + } + if (memory_after < memory_before) { + state.ResumeTiming(); + state.SkipWithError("allocator reported less live memory after the NPDS update"); + return; + } + total_retained_memory += memory_after - memory_before; + state.ResumeTiming(); + } + + const double average_retained_memory = + static_cast(total_retained_memory) / state.iterations(); + state.counters["retained_memory_bytes"] = benchmark::Counter( + average_retained_memory, benchmark::Counter::kDefaults, benchmark::Counter::kIs1024); + state.counters["retained_memory_per_policy_bytes"] = + benchmark::Counter(average_retained_memory / kPolicyCount, benchmark::Counter::kDefaults, + benchmark::Counter::kIs1024); + state.SetItemsProcessed(static_cast(state.iterations()) * kPolicyCount); +} + +BENCHMARK_CAPTURE(processNpdsUpdateWith1000Policies, TlsContextCacheEnabled, true, false) + ->Unit(benchmark::kMillisecond); +BENCHMARK_CAPTURE(processNpdsUpdateWith1000Policies, TlsContextCacheDisabled, false, false) + ->Unit(benchmark::kMillisecond); +BENCHMARK_CAPTURE(processNpdsUpdateWith1000Policies, HeaderSecretCacheEnabled, true, true) + ->Unit(benchmark::kMillisecond); +BENCHMARK_CAPTURE(processNpdsUpdateWith1000Policies, HeaderSecretCacheDisabled, false, true) + ->Unit(benchmark::kMillisecond); + +} // namespace +} // namespace Cilium +} // namespace Envoy diff --git a/tests/cilium_network_policy_test.cc b/tests/cilium_network_policy_test.cc index 201871615..a313d55f5 100644 --- a/tests/cilium_network_policy_test.cc +++ b/tests/cilium_network_policy_test.cc @@ -255,6 +255,23 @@ class CiliumNetworkPolicyTest : public ::testing::Test { return tlsAllowed(false, pod_ip, remote_id, port, sni, tls_socket_required, raw_socket_allowed); } + const Ssl::ContextConfig* tlsContextConfig(const PolicyInstance& policy, bool ingress, + uint64_t remote_id, uint16_t port) { + auto port_policy = policy.findPortPolicy(ingress, port); + const Ssl::ContextConfig* config = nullptr; + bool raw_socket_allowed = false; + if (ingress) { + static_cast( + port_policy.getServerTlsContext(proxy_id_, remote_id, "", config, raw_socket_allowed)); + } else { + static_cast( + port_policy.getClientTlsContext(proxy_id_, remote_id, "", config, raw_socket_allowed)); + } + EXPECT_FALSE(raw_socket_allowed); + EXPECT_NE(config, nullptr); + return config; + } + std::string updatesRejectedStatName() { return CiliumTestPeer::policyStats(*policy_map_).updates_rejected_.name(); } @@ -2808,6 +2825,162 @@ TEST_F(CiliumNetworkPolicyTest, HttpPolicyUpdateToMissingSDS) { EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{":path", "/notallowed"}})); } +TEST_F(CiliumNetworkPolicyTest, TlsContextsAreDeduplicatedWithinStream) { + EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.3" ] + endpoint_id: 42 + ingress_per_port_policies: + - port: 443 + rules: + - remote_policies: [ 43 ] + downstream_tls_context: + tls_sds_secret: "shared-cert" + validation_context_sds_secret: "shared-ca" + egress_per_port_policies: + - port: 443 + rules: + - remote_policies: [ 43 ] + upstream_tls_context: + tls_sds_secret: "shared-cert" + validation_context_sds_secret: "shared-ca" +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.4" ] + endpoint_id: 44 + ingress_per_port_policies: + - port: 443 + rules: + - remote_policies: [ 43 ] + downstream_tls_context: + tls_sds_secret: "shared-cert" + validation_context_sds_secret: "shared-ca" + egress_per_port_policies: + - port: 443 + rules: + - remote_policies: [ 43 ] + upstream_tls_context: + tls_sds_secret: "shared-cert" + validation_context_sds_secret: "shared-ca" +)EOF")); + + const auto& first = policy_map_->getPolicyInstance("10.1.2.3", false); + const auto& second = policy_map_->getPolicyInstance("10.1.2.4", false); + + const auto* first_downstream = tlsContextConfig(first, true, 43, 443); + const auto* second_downstream = tlsContextConfig(second, true, 43, 443); + const auto* first_upstream = tlsContextConfig(first, false, 43, 443); + const auto* second_upstream = tlsContextConfig(second, false, 43, 443); + + EXPECT_EQ(first_downstream, second_downstream); + EXPECT_EQ(first_upstream, second_upstream); + EXPECT_NE(first_downstream, first_upstream); +} + +TEST_F(CiliumNetworkPolicyTest, SecretWatchersAreDeduplicatedWithinStream) { + EXPECT_CALL(secret_manager_, findOrCreateGenericSecretProvider(_, "shared-header-secret", _, _)); + + EXPECT_NO_THROW(updateFromYaml(R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.3" ] + endpoint_id: 42 + ingress_per_port_policies: + - port: 80 + rules: + - remote_policies: [ 43 ] + http_rules: + http_rules: + - header_matches: + - name: "x-shared-secret" + value: "first-header-value" + value_sds_secret: "shared-header-secret" +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.4" ] + endpoint_id: 44 + ingress_per_port_policies: + - port: 80 + rules: + - remote_policies: [ 43 ] + http_rules: + http_rules: + - header_matches: + - name: "x-shared-secret" + value: "second-header-value" + value_sds_secret: "shared-header-secret" +)EOF")); + + // The missing shared SDS value leaves each HeaderMatch's own inline fallback in effect. + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{"x-shared-secret", "first-header-value"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.3", 43, 80, {{"x-shared-secret", "second-header-value"}})); + EXPECT_TRUE(ingressAllowed("10.1.2.4", 43, 80, {{"x-shared-secret", "second-header-value"}})); + EXPECT_FALSE(ingressAllowed("10.1.2.4", 43, 80, {{"x-shared-secret", "first-header-value"}})); +} + +TEST_F(CiliumNetworkPolicyTest, SecretWatcherCacheIsResetForNewStream) { + EXPECT_CALL(secret_manager_, findOrCreateGenericSecretProvider(_, "header-secret", _, _)) + .Times(2); + + const auto policy_yaml = R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.3" ] + endpoint_id: 42 + ingress_per_port_policies: + - port: 80 + rules: + - remote_policies: [ 43 ] + http_rules: + http_rules: + - header_matches: + - name: "x-secret" + value: "header-value" + value_sds_secret: "header-secret" +)EOF"; + + EXPECT_NO_THROW(updateFromYaml(policy_yaml)); + auto old_policy = CiliumTestPeer::policyInstanceShared(*policy_map_, "10.1.2.3"); + ASSERT_NE(old_policy, nullptr); + + CiliumTestPeer::resetStream(*policy_map_); + EXPECT_NO_THROW(updateFromYaml(policy_yaml)); + + Http::TestRequestHeaderMapImpl old_headers{{"x-secret", "header-value"}}; + Cilium::AccessLog::Entry old_log_entry; + EXPECT_TRUE(old_policy->allowed(true, proxy_id_, 43, 80, old_headers, old_log_entry)); + EXPECT_TRUE(ingressAllowed("10.1.2.3", 43, 80, {{"x-secret", "header-value"}})); +} + +TEST_F(CiliumNetworkPolicyTest, TlsContextCacheIsResetForNewStream) { + const auto policy_yaml = R"EOF(version_info: "1" +resources: +- "@type": type.googleapis.com/cilium.NetworkPolicy + endpoint_ips: [ "10.1.2.3" ] + endpoint_id: 42 + ingress_per_port_policies: + - port: 443 + rules: + - remote_policies: [ 43 ] + downstream_tls_context: + tls_sds_secret: "shared-cert" +)EOF"; + + EXPECT_NO_THROW(updateFromYaml(policy_yaml)); + auto old_policy = CiliumTestPeer::policyInstanceShared(*policy_map_, "10.1.2.3"); + ASSERT_NE(old_policy, nullptr); + const auto* old_config = tlsContextConfig(*old_policy, true, 43, 443); + + CiliumTestPeer::resetStream(*policy_map_); + EXPECT_NO_THROW(updateFromYaml(policy_yaml)); + + const auto& new_policy = policy_map_->getPolicyInstance("10.1.2.3", false); + const auto* new_config = tlsContextConfig(new_policy, true, 43, 443); + EXPECT_NE(old_config, new_config); + + // Resetting the cache must not invalidate contexts retained by the old policy map. + EXPECT_EQ(old_config, tlsContextConfig(*old_policy, true, 43, 443)); +} + TEST_F(CiliumNetworkPolicyTest, TlsPolicyUpdate) { bool tls_socket_required; bool raw_socket_allowed;