policy: Deduplicate SecretWatchers and TLS contexts

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<std::string>& removed_resources,
                               const std::string& system_version_info) override;
 
+  std::shared_ptr<NetworkPolicyMapImpl> sharedFromThis() {
+    return std::static_pointer_cast<NetworkPolicyMapImpl>(
+        ManagedGrpcSubscription::shared_from_this());
+  }
+
+  std::weak_ptr<NetworkPolicyMapImpl> 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<Server::Configuration::TransportSocketFactoryContextImpl>
       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<Logger::Id::config> {
       : 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<SecretWatcher>(
-          parent.transportFactoryContext(), parent.getConfigSource(), config.value_sds_secret());
+      secret_ = parent.getSecretWatcher(config.value_sds_secret());
     }
   }
 
@@ -565,7 +587,9 @@ class HeaderMatch : public Logger::Loggable<Logger::Id::config> {
   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<Logger::Id::config> {
@@ -797,14 +821,10 @@ class PortNetworkPolicyRule : public Logger::Loggable<Logger::Id::config> {
       remotes_.emplace(remote);
     }
     if (rule.has_downstream_tls_context()) {
-      auto config = rule.downstream_tls_context();
-      server_context_ = std::make_unique<DownstreamTLSContext>(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<UpstreamTLSContext>(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<NetworkPolicyMapImpl>(context, config_source, subscribe);
+  impl_ = std::make_shared<NetworkPolicyMapImpl>(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<NetworkPolicyDecoder>(); },
           config_source, context.serverFactoryContext(),
@@ -1958,6 +1980,8 @@ NetworkPolicyMapImpl::NetworkPolicyMapImpl(
       transport_factory_context_(
           std::make_shared<Server::Configuration::TransportSocketFactoryContextImpl>(
               context_, scope(), context_.messageValidationContext().dynamicValidationVisitor())),
+      policy_secret_cache_(transport_factory_context_, std::cref(getConfigSource()),
+                           policy_secret_cache_enabled),
       parked_init_manager_(std::make_unique<Init::ManagerImpl>("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();
     }
 

cilium/network_policy.h

@@ -197,7 +197,7 @@ class NetworkPolicyMap : public Singleton::Instance, public Logger::Loggable<Log
 public:
   NetworkPolicyMap(Server::Configuration::FactoryContext& context,
                    const envoy::config::core::v3::ConfigSource& config_source,
-                   bool subscribe = true);
+                   bool subscribe = true, bool policy_secret_cache_enabled = true);
   ~NetworkPolicyMap() override;
 
   bool exists(const std::string& endpoint_policy_name) const;

cilium/secret_watcher.cc

@@ -3,6 +3,9 @@
 #include <fmt/format.h>
 
 #include <atomic>
+#include <chrono>
+#include <functional>
+#include <memory>
 #include <string>
 #include <utility>
 
@@ -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 <typename Cache> 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 <typename Cache, typename Key, typename Factory>
+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 <typename Context>
+  using TLSContextMap =
+      absl::flat_hash_map<cilium::TLSContext, std::weak_ptr<Context>, MessageUtil, MessageUtil>;
+  using SecretWatcherMap = absl::flat_hash_map<std::string, std::weak_ptr<SecretWatcher>>;
+
+  Impl(std::shared_ptr<Server::Configuration::TransportSocketFactoryContext> context,
+       std::reference_wrapper<const envoy::config::core::v3::ConfigSource> 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<Server::Configuration::TransportSocketFactoryContext> context_;
+  std::reference_wrapper<const envoy::config::core::v3::ConfigSource> config_source_;
+  const bool caching_enabled_;
+  TLSContextMap<DownstreamTLSContext> downstream_cache_;
+  TLSContextMap<UpstreamTLSContext> upstream_cache_;
+  SecretWatcherMap secret_watcher_cache_;
+  MonotonicTime next_prune_time_;
+};
+
+PolicySecretCache::PolicySecretCache(
+    std::shared_ptr<Server::Configuration::TransportSocketFactoryContext> context,
+    std::reference_wrapper<const envoy::config::core::v3::ConfigSource> config_source,
+    bool caching_enabled)
+    : impl_(std::make_unique<Impl>(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<SecretWatcher>(
+                           *impl_->context_, impl_->config_source_.get(), sds_name);
+                     });
+}
+
 } // namespace Cilium
 } // namespace Envoy