policy: Change policy resolver back to a shared pointer

Change policy resolver back to a shared pointer, and manage
NetworkPolicyMap destruction explicitly by splitting the map to singleton
and 'NetworkPolicyMapImpl' and by posting the 'NetworkPolicyMapImpl' to
the main thread for deletion.

NetworkPolicyMapImpl is managed via an unique_ptr, which required the
removal of use of 'shared_from_this', which was possible due to
simplified clean-up of the old map now that an atomic swap is used for
the policy map update.

This change allows long lived connections on a removed listener to keep
on functioning, including receiving policy updates via the singleton
policy map.

Signed-off-by: Jarno Rajahalme <[email protected]>

Author: jrajahalme

cilium/bpf_metadata.cc

@@ -190,11 +190,8 @@ createHostMap(Server::Configuration::ListenerFactoryContext& context) {
 std::shared_ptr<const Cilium::NetworkPolicyMap>
 createPolicyMap(Server::Configuration::FactoryContext& context, Cilium::CtMapSharedPtr& ct) {
   return context.serverFactoryContext().singletonManager().getTyped<const Cilium::NetworkPolicyMap>(
-      SINGLETON_MANAGER_REGISTERED_NAME(cilium_network_policy), [&context, &ct] {
-        auto map = std::make_shared<Cilium::NetworkPolicyMap>(context, ct);
-        map->startSubscription();
-        return map;
-      });
+      SINGLETON_MANAGER_REGISTERED_NAME(cilium_network_policy),
+      [&context, &ct] { return std::make_shared<Cilium::NetworkPolicyMap>(context, ct); });
 }
 
 } // namespace
@@ -530,7 +527,7 @@ Config::extractSocketMetadata(Network::ConnectionSocket& socket) {
       mark, ingress_source_identity, source_identity, is_ingress_, is_l7lb_, dip->port(),
       std::move(pod_ip), std::move(ingress_policy_name), std::move(src_address),
       std::move(source_addresses.ipv4_), std::move(source_addresses.ipv6_), std::move(dst_address),
-      weak_from_this(), proxy_id_, std::move(proxylib_l7proto), sni)};
+      shared_from_this(), proxy_id_, std::move(proxylib_l7proto), sni)};
 }
 
 Network::FilterStatus Instance::onAccept(Network::ListenerFilterCallbacks& cb) {

cilium/bpf_metadata.h

@@ -39,7 +39,7 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
                  Network::Address::InstanceConstSharedPtr source_address_ipv4,
                  Network::Address::InstanceConstSharedPtr source_address_ipv6,
                  Network::Address::InstanceConstSharedPtr original_dest_address,
-                 const std::weak_ptr<PolicyResolver>& policy_resolver, uint32_t proxy_id,
+                 const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id,
                  std::string&& proxylib_l7_proto, absl::string_view sni)
       : ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
         ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
@@ -118,7 +118,7 @@ struct SocketMetadata : public Logger::Loggable<Logger::Id::filter> {
   uint32_t proxy_id_;
   std::string proxylib_l7_proto_;
   std::string sni_;
-  std::weak_ptr<PolicyResolver> policy_resolver_;
+  const PolicyResolverSharedPtr policy_resolver_;
 
   uint32_t mark_;
 

cilium/filter_state_cilium_policy.cc

@@ -26,19 +26,12 @@ bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& co
                                                    /* OUT */ bool& use_proxy_lib,
                                                    /* OUT */ std::string& l7_proto,
                                                    /* INOUT */ AccessLog::Entry& log_entry) const {
-  const auto resolver = policy_resolver_.lock();
   use_proxy_lib = false;
   l7_proto = "";
 
-  if (!resolver) {
-    // No policy resolver
-    ENVOY_CONN_LOG(debug, "No policy resolver", conn);
-    return false;
-  }
-
   // enforce pod policy first, if any
   if (pod_ip_.length() > 0) {
-    const auto& policy = resolver->getPolicy(pod_ip_);
+    const auto& policy = policy_resolver_->getPolicy(pod_ip_);
     auto remote_id = ingress_ ? source_identity_ : destination_identity;
     auto port = ingress_ ? port_ : destination_port;
 
@@ -57,7 +50,7 @@ bool CiliumPolicyFilterState::enforceNetworkPolicy(const Network::Connection& co
   // enforce Ingress policy 2nd, if any
   if (ingress_policy_name_.length() > 0) {
     log_entry.entry_.set_policy_name(ingress_policy_name_);
-    const auto& policy = resolver->getPolicy(ingress_policy_name_);
+    const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_);
 
     // Enforce ingress policy for Ingress, on the original destination port
     if (ingress_source_identity_ != 0) {
@@ -93,14 +86,6 @@ bool CiliumPolicyFilterState::enforceHTTPPolicy(const Network::Connection& conn,
                                                 uint16_t destination_port,
                                                 /* INOUT */ Http::RequestHeaderMap& headers,
                                                 /* INOUT */ AccessLog::Entry& log_entry) const {
-  const auto resolver = policy_resolver_.lock();
-
-  if (!resolver) {
-    // No policy resolver
-    ENVOY_CONN_LOG(debug, "No policy resolver", conn);
-    return false;
-  }
-
   // enforce pod policy first, if any.
   // - ingress enforcement in downstream
   // - egress enforcement in upstream
@@ -109,7 +94,7 @@ bool CiliumPolicyFilterState::enforceHTTPPolicy(const Network::Connection& conn,
   // - is_l7lb_: ingress_ == is_downstream
   // - !is_l7lb_: is_downstream
   if (pod_ip_.length() > 0 && (is_l7lb_ ? is_downstream == ingress_ : is_downstream)) {
-    const auto& policy = resolver->getPolicy(pod_ip_);
+    const auto& policy = policy_resolver_->getPolicy(pod_ip_);
     auto remote_id = ingress_ ? source_identity_ : destination_identity;
     auto port = ingress_ ? port_ : destination_port;
     if (!policy.allowed(ingress_, proxy_id_, remote_id, port, headers, log_entry)) {
@@ -122,7 +107,7 @@ bool CiliumPolicyFilterState::enforceHTTPPolicy(const Network::Connection& conn,
   // enforce Ingress policy 2nd, if any, always on the upstream
   if (!is_downstream && ingress_policy_name_.length() > 0) {
     log_entry.entry_.set_policy_name(ingress_policy_name_);
-    const auto& policy = resolver->getPolicy(ingress_policy_name_);
+    const auto& policy = policy_resolver_->getPolicy(ingress_policy_name_);
 
     // Enforce ingress policy for Ingress, on the original destination port
     if (ingress_source_identity_ != 0) {

cilium/filter_state_cilium_policy.h

@@ -16,7 +16,6 @@
 #include "absl/strings/string_view.h"
 #include "cilium/accesslog.h"
 #include "cilium/network_policy.h"
-#include "cilium/policy_id.h"
 
 namespace Envoy {
 namespace Cilium {
@@ -28,6 +27,7 @@ class PolicyResolver {
   virtual uint32_t resolvePolicyId(const Network::Address::Ip*) const PURE;
   virtual const PolicyInstance& getPolicy(const std::string&) const PURE;
 };
+using PolicyResolverSharedPtr = std::shared_ptr<PolicyResolver>;
 
 // FilterState that holds relevant connection & policy information that can be retrieved
 // by the Cilium network- and HTTP policy filters via filter state.
@@ -37,7 +37,7 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
   CiliumPolicyFilterState(uint32_t ingress_source_identity, uint32_t source_identity, bool ingress,
                           bool l7lb, uint16_t port, std::string&& pod_ip,
                           std::string&& ingress_policy_name,
-                          const std::weak_ptr<PolicyResolver>& policy_resolver, uint32_t proxy_id,
+                          const PolicyResolverSharedPtr& policy_resolver, uint32_t proxy_id,
                           absl::string_view sni)
       : ingress_source_identity_(ingress_source_identity), source_identity_(source_identity),
         ingress_(ingress), is_l7lb_(l7lb), port_(port), pod_ip_(std::move(pod_ip)),
@@ -51,20 +51,10 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
   }
 
   uint32_t resolvePolicyId(const Network::Address::Ip* ip) const {
-    const auto resolver = policy_resolver_.lock();
-    if (resolver) {
-      return resolver->resolvePolicyId(ip);
-    }
-    return Cilium::ID::WORLD; // default to WORLD policy ID if resolver is no longer available
+    return policy_resolver_->resolvePolicyId(ip);
   }
 
-  const PolicyInstance& getPolicy() const {
-    const auto resolver = policy_resolver_.lock();
-    if (resolver) {
-      return resolver->getPolicy(pod_ip_);
-    }
-    return NetworkPolicyMap::getDenyAllPolicy();
-  }
+  const PolicyInstance& getPolicy() const { return policy_resolver_->getPolicy(pod_ip_); }
 
   bool enforceNetworkPolicy(const Network::Connection& conn, uint32_t destination_identity,
                             uint16_t destination_port, const absl::string_view sni,
@@ -95,7 +85,7 @@ class CiliumPolicyFilterState : public StreamInfo::FilterState::Object,
   std::string sni_;
 
 private:
-  const std::weak_ptr<PolicyResolver> policy_resolver_;
+  const PolicyResolverSharedPtr policy_resolver_;
 };
 } // namespace Cilium
 } // namespace Envoy