From a3faf4d532f3a985db7224229251fe0a8949d76f Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 20 Aug 2026 09:04:44 +0200 Subject: [PATCH] policy: Deduplicate SecretWatchers and TLS Contexts Network policies may contain hundreds or thousands of HeaderMatch or TLS rules referring to a small set of Secrets. Each reference previously constructed a separate SecretWatcher or Envoy TLS ContextConfigImpl, including its SDS provider callbacks and other context state. Add separate weak caches for SecretWatchers and upstream and downstream TLS contexts, keyed by the SDS name or Cilium TLSContext protobuf. Reuse a live context when an equivalent policy configuration is encountered. Reset the caches together with the policy maps after an NPDS stream restart, preserving the existing agent-restart behavior. Prune expired cache entries asynchronously after policy updates and worker quiescence, keeping pruning off the policy-update hot path. Benchmark an NPDS update containing 1,000 policies that reference the same SDS-derived CA validation context. The CA is delivered through a real dynamic SDS provider rather than embedded in the policy TLS contexts. Optimized benchmark results (3 repetitions): CPU time Retained heap Heap/policy TLS context cache enabled 1.47 ms 1.581 MiB 1.619 KiB TLS context cache disabled 3.95 ms 4.990 MiB 5.109 KiB This reduces CPU time by approximately 63% and retained heap by 68%, saving about 3.41 MiB per 1,000-policy update and increasing throughput by 2.68x. The benchmark uses Envoy's small, single-certificate test CA. Production SDS resources commonly contain larger CA bundles comparable to those shipped with operating systems, so the benchmark likely understates the memory impact of retaining duplicate certificate-related context state. Benchmark results for deduplication of SecretWatchers on HeaderMatches are more modest (duplicating header sized secrets carries less overhead), but are still positive around 20-25% for CPU and memory. Signed-off-by: Jarno Rajahalme --- cilium/network_policy.cc | 63 ++++-- cilium/network_policy.h | 2 +- cilium/secret_watcher.cc | 138 +++++++++++- cilium/secret_watcher.h | 60 +++++- tests/BUILD | 27 +++ tests/cilium_network_policy_benchmark.cc | 256 +++++++++++++++++++++++ tests/cilium_network_policy_test.cc | 173 +++++++++++++++ 7 files changed, 694 insertions(+), 25 deletions(-) create mode 100644 tests/cilium_network_policy_benchmark.cc 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;