cilium: Adjust custom filter implementation

Relates: envoyproxy/envoy#30765 Relates: envoyproxy/envoy#31189 Signed-off-by: Tam Mach <[email protected]>
sayboras · Mar 23, 2024 · eb35be3 · eb35be3
1 parent c5a4c70
commit eb35be3
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 53 deletions.
diff --git a/cilium/bpf_metadata.cc b/cilium/bpf_metadata.cc
@@ -83,17 +83,17 @@ namespace {
 
 std::shared_ptr<const Cilium::PolicyHostMap>
 createHostMap(Server::Configuration::ListenerFactoryContext& context) {
-  return context.singletonManager().getTyped<const Cilium::PolicyHostMap>(
+  return context.serverFactoryContext().singletonManager().getTyped<const Cilium::PolicyHostMap>(
       SINGLETON_MANAGER_REGISTERED_NAME(cilium_host_map), [&context] {
-        auto map = std::make_shared<Cilium::PolicyHostMap>(context);
-        map->startSubscription(context);
+        auto map = std::make_shared<Cilium::PolicyHostMap>(context.serverFactoryContext());
+        map->startSubscription(context.serverFactoryContext());
         return map;
       });
 }
 
 std::shared_ptr<const Cilium::NetworkPolicyMap>
 createPolicyMap(Server::Configuration::FactoryContext& context, Cilium::CtMapSharedPtr& ct) {
-  return context.singletonManager().getTyped<const Cilium::NetworkPolicyMap>(
+  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(context);
@@ -135,13 +135,13 @@ Config::Config(const ::cilium::BpfMetadata& config,
   // configured
   std::string bpf_root = config.bpf_root();
   if (bpf_root.length() > 0) {
-    ct_maps_ = context.singletonManager().getTyped<Cilium::CtMap>(
+    ct_maps_ = context.serverFactoryContext().singletonManager().getTyped<Cilium::CtMap>(
         SINGLETON_MANAGER_REGISTERED_NAME(cilium_bpf_conntrack), [&bpf_root] {
           // Even if opening the global maps fail, local maps may still succeed
           // later.
           return std::make_shared<Cilium::CtMap>(bpf_root);
         });
-    ipcache_ = context.singletonManager().getTyped<Cilium::IPCache>(
+    ipcache_ = context.serverFactoryContext().singletonManager().getTyped<Cilium::IPCache>(
         SINGLETON_MANAGER_REGISTERED_NAME(cilium_ipcache), [&bpf_root] {
           auto ipcache = std::make_shared<Cilium::IPCache>(bpf_root);
           if (!ipcache->Open()) {

diff --git a/cilium/l7policy.cc b/cilium/l7policy.cc
@@ -3,15 +3,12 @@
 #include <string>
 
 #include "envoy/registry/registry.h"
-#include "envoy/singleton/manager.h"
 
 #include "source/common/buffer/buffer_impl.h"
-#include "source/common/common/enum_to_int.h"
 #include "source/common/config/utility.h"
 #include "source/common/http/header_map_impl.h"
 #include "source/common/http/utility.h"
 #include "source/common/network/upstream_server_name.h"
-#include "source/common/network/upstream_subject_alt_names.h"
 
 #include "cilium/api/l7policy.pb.validate.h"
 #include "cilium/network_policy.h"
@@ -22,7 +19,7 @@ namespace Cilium {
 
 class ConfigFactory : public Server::Configuration::NamedHttpFilterConfigFactory {
 public:
-  Http::FilterFactoryCb
+  absl::StatusOr<Http::FilterFactoryCb>
   createFilterFactoryFromProto(const Protobuf::Message& proto_config, const std::string&,
                                Server::Configuration::FactoryContext& context) override {
     auto config = std::make_shared<Cilium::Config>(
@@ -48,8 +45,8 @@ REGISTER_FACTORY(ConfigFactory, Server::Configuration::NamedHttpFilterConfigFact
 
 Config::Config(const std::string& access_log_path, const std::string& denied_403_body,
                Server::Configuration::FactoryContext& context)
-    : time_source_(context.timeSource()), stats_{ALL_CILIUM_STATS(
-                                              POOL_COUNTER_PREFIX(context.scope(), "cilium"))},
+    : time_source_(context.serverFactoryContext().timeSource()),
+      stats_{ALL_CILIUM_STATS(POOL_COUNTER_PREFIX(context.scope(), "cilium"))},
       denied_403_body_(denied_403_body), access_log_(nullptr) {
   if (access_log_path.length()) {
     access_log_ = AccessLog::Open(access_log_path);

diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc
@@ -56,7 +56,7 @@ namespace CiliumL3 {
 
 Config::Config(const ::cilium::NetworkFilter& config,
                Server::Configuration::FactoryContext& context)
-    : time_source_(context.timeSource()), access_log_(nullptr) {
+    : time_source_(context.serverFactoryContext().timeSource()), access_log_(nullptr) {
   const auto& access_log_path = config.access_log_path();
   if (access_log_path.length()) {
     access_log_ = Cilium::AccessLog::Open(access_log_path);

diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc
@@ -1063,28 +1063,30 @@ class PolicyInstanceImpl : public PolicyInstance {
 // Common base constructor
 // This is used directly for testing with a file-based subscription
 NetworkPolicyMap::NetworkPolicyMap(Server::Configuration::FactoryContext& context)
-    : tls_map_(context.threadLocal()),
-      local_ip_str_(context.localInfo().address()->ip()->addressAsString()),
+    : tls_map_(context.serverFactoryContext().threadLocal()),
+      local_ip_str_(context.serverFactoryContext().localInfo().address()->ip()->addressAsString()),
       name_(fmt::format("cilium.policymap.{}.{}.", local_ip_str_, ++instance_id_)),
-      scope_(context.serverScope().createScope(name_)),
+      scope_(context.serverFactoryContext().serverScope().createScope(name_)),
       init_target_(fmt::format("Cilium Network Policy subscription start"),
                    [this]() { subscription_->start({}); }),
       transport_factory_context_(
           std::make_shared<Server::Configuration::TransportSocketFactoryContextImpl>(
-              context.getServerFactoryContext(),
+              context.serverFactoryContext(),
               context.getTransportSocketFactoryContext().sslContextManager(), *scope_,
-              context.getServerFactoryContext().clusterManager(),
-              context.messageValidationContext().dynamicValidationVisitor())) {
+              context.serverFactoryContext().clusterManager(),
+              context.serverFactoryContext()
+                  .messageValidationContext()
+                  .dynamicValidationVisitor())) {
   // Use listener init manager for the first initialization
   transport_factory_context_->setInitManager(context.initManager());
   context.initManager().add(init_target_);
 
   ENVOY_LOG(trace, "NetworkPolicyMap({}) created.", name_);
   tls_map_.set([&](Event::Dispatcher&) { return std::make_shared<ThreadLocalPolicyMap>(); });
 
-  if (context.admin().has_value()) {
+  if (context.serverFactoryContext().admin().has_value()) {
     ENVOY_LOG(debug, "Registering NetworkPolicies to config tracker");
-    config_tracker_entry_ = context.admin()->getConfigTracker().add(
+    config_tracker_entry_ = context.serverFactoryContext().admin()->getConfigTracker().add(
         "networkpolicies", [this](const Matchers::StringMatcher& name_matcher) {
           return dumpNetworkPolicyConfigs(name_matcher);
         });
@@ -1104,9 +1106,11 @@ NetworkPolicyMap::NetworkPolicyMap(Server::Configuration::FactoryContext& contex
 // pointer is formed by the caller of the constructor, hence this
 // can't be called from the constructor!
 void NetworkPolicyMap::startSubscription(Server::Configuration::FactoryContext& context) {
-  subscription_ = subscribe("type.googleapis.com/cilium.NetworkPolicy", context.localInfo(),
-                            context.clusterManager(), context.mainThreadDispatcher(),
-                            context.api().randomGenerator(), *scope_, *this,
+  subscription_ = subscribe("type.googleapis.com/cilium.NetworkPolicy",
+                            context.serverFactoryContext().localInfo(),
+                            context.serverFactoryContext().clusterManager(),
+                            context.serverFactoryContext().mainThreadDispatcher(),
+                            context.serverFactoryContext().api().randomGenerator(), *scope_, *this,
                             std::make_shared<NetworkPolicyDecoder>());
 }
 

diff --git a/cilium/websocket_config.cc b/cilium/websocket_config.cc
@@ -47,14 +47,14 @@ Config::Config(Server::Configuration::FactoryContext& context, bool client,
                const std::string& key, const std::string& version, const std::string& origin,
                const ProtobufWkt::Duration& handshake_timeout,
                const ProtobufWkt::Duration& ping_interval, bool ping_when_idle)
-    : time_source_(context.timeSource()),
-      dispatcher_(context.mainThreadDispatcher()), stats_{ALL_WEBSOCKET_STATS(POOL_COUNTER_PREFIX(
-                                                       context.scope(), "websocket"))},
-      random_(context.api().randomGenerator()), client_(client), host_(absl::AsciiStrToLower(host)),
-      path_(absl::AsciiStrToLower(path)), key_(key), version_(absl::AsciiStrToLower(version)),
-      origin_(absl::AsciiStrToLower(origin)), handshake_timeout_(std::chrono::seconds(5)),
-      ping_interval_(std::chrono::milliseconds(0)), ping_when_idle_(ping_when_idle),
-      access_log_(nullptr) {
+    : time_source_(context.serverFactoryContext().timeSource()),
+      dispatcher_(context.serverFactoryContext().mainThreadDispatcher()),
+      stats_{ALL_WEBSOCKET_STATS(POOL_COUNTER_PREFIX(context.scope(), "websocket"))},
+      random_(context.serverFactoryContext().api().randomGenerator()), client_(client),
+      host_(absl::AsciiStrToLower(host)), path_(absl::AsciiStrToLower(path)), key_(key),
+      version_(absl::AsciiStrToLower(version)), origin_(absl::AsciiStrToLower(origin)),
+      handshake_timeout_(std::chrono::seconds(5)), ping_interval_(std::chrono::milliseconds(0)),
+      ping_when_idle_(ping_when_idle), access_log_(nullptr) {
   envoy::extensions::filters::network::http_connection_manager::v3::RequestIDExtension x_rid_config;
   x_rid_config.mutable_typed_config()->PackFrom(
       envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig());

diff --git a/tests/accesslog_server.cc b/tests/accesslog_server.cc
@@ -3,16 +3,10 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <sys/socket.h>
-#include <sys/un.h>
 #include <unistd.h>
 
 #include <string>
 
-#include "source/common/common/lock_guard.h"
-#include "source/common/common/utility.h"
-
-#include "test/test_common/thread_factory_for_test.h"
-
 namespace Envoy {
 
 AccessLogServer::AccessLogServer(const std::string path)

diff --git a/tests/accesslog_server.h b/tests/accesslog_server.h
@@ -5,9 +5,6 @@
 #include <string>
 #include <vector>
 
-#include "source/common/common/logger.h"
-#include "source/common/common/thread.h"
-
 #include "test/test_common/utility.h"
 
 #include "absl/synchronization/mutex.h"

diff --git a/tests/bpf_metadata.cc b/tests/bpf_metadata.cc
@@ -28,20 +28,22 @@ namespace {
 
 std::shared_ptr<const Cilium::PolicyHostMap>
 createHostMap(const std::string& config, Server::Configuration::ListenerFactoryContext& context) {
-  return context.singletonManager().getTyped<const Cilium::PolicyHostMap>(
+  return context.serverFactoryContext().singletonManager().getTyped<const Cilium::PolicyHostMap>(
       "cilium_host_map_singleton", [&config, &context] {
         std::string path = TestEnvironment::writeStringToFileForTest("host_map.yaml", config);
         ENVOY_LOG_MISC(debug, "Loading Cilium Host Map from file \'{}\' instead of using gRPC",
                        path);
 
-        Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(path, context.api());
+        Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(
+            path, context.serverFactoryContext().api());
         Envoy::Config::SubscriptionStats stats =
             Envoy::Config::Utility::generateStats(context.scope());
-        auto map = std::make_shared<Cilium::PolicyHostMap>(context);
+        auto map = std::make_shared<Cilium::PolicyHostMap>(context.serverFactoryContext());
         auto subscription = std::make_unique<Envoy::Config::FilesystemSubscriptionImpl>(
-            context.mainThreadDispatcher(), Envoy::Config::makePathConfigSource(path), *map,
+            context.serverFactoryContext().mainThreadDispatcher(),
+            Envoy::Config::makePathConfigSource(path), *map,
             std::make_shared<Cilium::PolicyHostDecoder>(), stats,
-            ProtobufMessage::getNullValidationVisitor(), context.api());
+            ProtobufMessage::getNullValidationVisitor(), context.serverFactoryContext().api());
         map->startSubscription(std::move(subscription));
         return map;
       });
@@ -51,15 +53,16 @@ std::shared_ptr<const Cilium::NetworkPolicyMap>
 createPolicyMap(const std::string& config,
                 const std::vector<std::pair<std::string, std::string>>& secret_configs,
                 Server::Configuration::FactoryContext& context) {
-  return context.singletonManager().getTyped<const Cilium::NetworkPolicyMap>(
+  return context.serverFactoryContext().singletonManager().getTyped<const Cilium::NetworkPolicyMap>(
       "cilium_network_policy_singleton", [&config, &secret_configs, &context] {
         if (secret_configs.size() > 0) {
           for (auto sds_pair : secret_configs) {
             auto& name = sds_pair.first;
             auto& sds_config = sds_pair.second;
             std::string sds_path = TestEnvironment::writeStringToFileForTest(
                 fmt::sprintf("secret-%s.yaml", name), sds_config);
-            Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(sds_path, context.api());
+            Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(
+                sds_path, context.serverFactoryContext().api());
           }
           Cilium::setSDSConfigFunc(
               [](const std::string& name) -> envoy::config::core::v3::ConfigSource {
@@ -79,14 +82,16 @@ createPolicyMap(const std::string& config,
                        "Loading Cilium Network Policy from file \'{}\' instead "
                        "of using gRPC",
                        policy_path);
-        Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(policy_path, context.api());
+        Envoy::Config::Utility::checkFilesystemSubscriptionBackingPath(
+            policy_path, context.serverFactoryContext().api());
         Envoy::Config::SubscriptionStats stats =
             Envoy::Config::Utility::generateStats(context.scope());
         auto map = std::make_shared<Cilium::NetworkPolicyMap>(context);
         auto subscription = std::make_unique<Envoy::Config::FilesystemSubscriptionImpl>(
-            context.mainThreadDispatcher(), Envoy::Config::makePathConfigSource(policy_path), *map,
+            context.serverFactoryContext().mainThreadDispatcher(),
+            Envoy::Config::makePathConfigSource(policy_path), *map,
             std::make_shared<Cilium::NetworkPolicyDecoder>(), stats,
-            ProtobufMessage::getNullValidationVisitor(), context.api());
+            ProtobufMessage::getNullValidationVisitor(), context.serverFactoryContext().api());
         map->startSubscription(std::move(subscription));
         return map;
       });

diff --git a/tests/metadata_config_test.cc b/tests/metadata_config_test.cc
@@ -26,9 +26,10 @@ class MetadataConfigTest : public testing::Test {
 
     ON_CALL(context_, getTransportSocketFactoryContext())
         .WillByDefault(ReturnRef(transport_socket_factory_context_));
-    ON_CALL(context_, api()).WillByDefault(testing::ReturnRef(*api_));
 
-    ON_CALL(context_.dispatcher_, createFilesystemWatcher_())
+    ON_CALL(context_.server_factory_context_, api()).WillByDefault(testing::ReturnRef(*api_));
+
+    ON_CALL(context_.server_factory_context_.dispatcher_, createFilesystemWatcher_())
         .WillByDefault(Invoke([]() -> Filesystem::Watcher* {
           auto watcher = new Filesystem::MockWatcher();
           EXPECT_CALL(*watcher, addWatch(_, Filesystem::Watcher::Events::MovedTo, _))