cilium: Adjust cilium filter build and implementation

Relates: envoyproxy/envoy#36187 Relates: envoyproxy/envoy#34815 Relates: envoyproxy/envoy#35694 Signed-off-by: Tam Mach <[email protected]>
cilium · Nov 19, 2024 · b499269 · b499269
1 parent f0225e4
commit b499269
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 52 deletions.
diff --git a/cilium/BUILD b/cilium/BUILD
@@ -129,12 +129,12 @@ envoy_cc_library(
         "websocket_config.h",
         "websocket_protocol.h",
     ],
-    external_deps = ["http_parser"],
     repository = "@envoy",
     deps = [
         "//cilium:accesslog_lib",
         "//cilium:socket_option_lib",
         "//cilium/api:websocket_cc_proto",
+        "@envoy//bazel/external/http_parser",
         "@envoy//envoy/common/crypto:crypto_interface",
         "@envoy//source/common/common:base64_lib",
         "@envoy//source/common/common:hex_lib",

diff --git a/cilium/grpc_subscription.cc b/cilium/grpc_subscription.cc
@@ -136,11 +136,11 @@ subscribe(const std::string& type_url, const LocalInfo::LocalInfo& local_info,
       std::make_unique<NopConfigValidatorsImpl>();
   auto factory_or_error = Config::Utility::factoryForGrpcApiConfigSource(
       cm.grpcAsyncClientManager(), api_config_source, scope, true, 0);
-  THROW_IF_STATUS_NOT_OK(factory_or_error, throw);
+  THROW_IF_NOT_OK_REF(factory_or_error.status());
 
   absl::StatusOr<Config::RateLimitSettings> rate_limit_settings_or_error =
       Config::Utility::parseRateLimitSettings(api_config_source);
-  THROW_IF_STATUS_NOT_OK(rate_limit_settings_or_error, throw);
+  THROW_IF_NOT_OK_REF(rate_limit_settings_or_error.status());
 
   Config::GrpcMuxContext grpc_mux_context{
       factory_or_error.value()->createUncachedRawAsyncClient(),

diff --git a/cilium/network_filter.cc b/cilium/network_filter.cc
@@ -215,7 +215,8 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream)
   if (go_parser_) {
     FilterResult res =
         go_parser_->OnIO(false, data, end_stream); // 'false' marks original direction data
-    ENVOY_CONN_LOG(trace, "cilium.network::onData: \'GoFilter::OnIO\' returned {}", conn, res);
+    ENVOY_CONN_LOG(trace, "cilium.network::onData: \'GoFilter::OnIO\' returned {}", conn,
+                   Envoy::Cilium::toString(res));
 
     if (res != FILTER_OK) {
       // Drop the connection due to an error
@@ -282,7 +283,7 @@ Network::FilterStatus Instance::onWrite(Buffer::Instance& data, bool end_stream)
     FilterResult res =
         go_parser_->OnIO(true, data, end_stream); // 'true' marks reverse direction data
     ENVOY_CONN_LOG(trace, "cilium.network::OnWrite: \'GoFilter::OnIO\' returned {}",
-                   callbacks_->connection(), res);
+                   callbacks_->connection(), Envoy::Cilium::toString(res));
 
     if (res != FILTER_OK) {
       // Drop the connection due to an error

diff --git a/cilium/network_policy.cc b/cilium/network_policy.cc
@@ -201,9 +201,10 @@ class HttpNetworkPolicyRule : public Logger::Loggable<Logger::Id::config> {
                 "Cilium L7 HttpNetworkPolicyRule(): HeaderMatch {}={} (match: {}, mismatch: {})",
                 header_match.name_.get(),
                 header_match.secret_ ? fmt::format("<SECRET {}>", header_match.secret_->name())
-                : header_match.value_.length() > 0 ? header_match.value_
-                                                   : "<PRESENT>",
-                header_match.match_action_, header_match.mismatch_action_);
+                : !header_match.value_.empty() ? header_match.value_
+                                               : "<PRESENT>",
+                cilium::HeaderMatch::MatchAction_Name(header_match.match_action_),
+                cilium::HeaderMatch::MismatchAction_Name(header_match.mismatch_action_));
     }
   }
 

diff --git a/cilium/proxylib.cc b/cilium/proxylib.cc
@@ -81,32 +81,6 @@ GoFilter::~GoFilter() {
   }
 }
 
-namespace {
-
-const char* filter_strerror(FilterResult res) {
-  switch (res) {
-  case FILTER_OK:
-    return "No error";
-  case FILTER_PARSER_ERROR:
-    return "Parser error";
-  case FILTER_UNKNOWN_CONNECTION:
-    return "Unknown connection";
-  case FILTER_UNKNOWN_PARSER:
-    return "Unknown parser";
-  case FILTER_INVALID_ADDRESS:
-    return "Invalid address";
-  case FILTER_POLICY_DROP:
-    return "Connection rejected";
-  case FILTER_INVALID_INSTANCE:
-    return "Invalid proxylib instance";
-  case FILTER_UNKNOWN_ERROR:
-    break;
-  }
-  return "Unknown error";
-}
-
-} // namespace
-
 GoFilter::InstancePtr GoFilter::NewInstance(Network::Connection& conn, const std::string& go_proto,
                                             bool ingress, uint32_t src_id, uint32_t dst_id,
                                             const std::string& src_addr,
@@ -123,7 +97,7 @@ GoFilter::InstancePtr GoFilter::NewInstance(Network::Connection& conn, const std
       parser->connection_id_ = conn.id();
     } else {
       ENVOY_CONN_LOG(warn, "Cilium Network: Connection with parser \"{}\" rejected: {}", conn,
-                     go_proto, filter_strerror(res));
+                     go_proto, toString(res));
       parser.reset(nullptr);
     }
   }
@@ -240,8 +214,8 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e
     ENVOY_CONN_LOG(trace, "Cilium Network::OnIO: Calling go module with {} bytes of data", conn_,
                    total_length);
     res = (*parent_.go_on_data_)(connection_id_, reply, end_stream, &input_slices, &ops);
-    ENVOY_CONN_LOG(trace, "Cilium Network::OnIO: \'go_on_data\' returned {}, ops({})", conn_, res,
-                   ops.len());
+    ENVOY_CONN_LOG(trace, "Cilium Network::OnIO: \'go_on_data\' returned {}, ops({})", conn_,
+                   toString(res), ops.len());
     if (res == FILTER_OK) {
       // Process all returned filter operations.
       for (int i = 0; i < ops.len(); i++) {
@@ -257,7 +231,7 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e
         if (terminal_op_seen) {
           ENVOY_CONN_LOG(warn,
                          "Cilium Network::OnIO: Filter operation {} after "
-                         "terminal opertion.",
+                         "terminal operation.",
                          conn_, op);
           return FILTER_PARSER_ERROR;
         }
@@ -319,8 +293,7 @@ FilterResult GoFilter::Instance::OnIO(bool reply, Buffer::Instance& data, bool e
       }
     } else {
       // Close the connection an any error
-      ENVOY_CONN_LOG(warn, "Cilium Network::OnIO: FILTER_POLICY_DROP {}", conn_,
-                     filter_strerror(res));
+      ENVOY_CONN_LOG(warn, "Cilium Network::OnIO: FILTER_POLICY_DROP {}", conn_, toString(res));
       return FILTER_PARSER_ERROR;
     }
 

diff --git a/cilium/proxylib.h b/cilium/proxylib.h
@@ -38,6 +38,28 @@ template <typename T> struct GoSlice {
   GoInt cap_;
 };
 
+inline std::string toString(const FilterResult res) {
+  switch (res) {
+  case FILTER_OK:
+    return "No error";
+  case FILTER_PARSER_ERROR:
+    return "Parser error";
+  case FILTER_UNKNOWN_CONNECTION:
+    return "Unknown connection";
+  case FILTER_UNKNOWN_PARSER:
+    return "Unknown parser";
+  case FILTER_INVALID_ADDRESS:
+    return "Invalid address";
+  case FILTER_POLICY_DROP:
+    return "Connection rejected";
+  case FILTER_INVALID_INSTANCE:
+    return "Invalid proxylib instance";
+  case FILTER_UNKNOWN_ERROR:
+    break;
+  }
+  return "Unknown error";
+}
+
 // Slice that remembers the base pointer and that can be reset.
 // Note that these have more header data than GoSlices and therefore may not
 // used as array elements passed to Go!

diff --git a/cilium/secret_watcher.cc b/cilium/secret_watcher.cc
@@ -127,7 +127,7 @@ DownstreamTLSContext::DownstreamTLSContext(const NetworkPolicyMap& parent,
     server_names_.emplace_back(config.server_names(i));
   }
   auto server_config_or_error = Extensions::TransportSockets::Tls::ServerContextConfigImpl::create(
-      context_config, parent.transportFactoryContext());
+      context_config, parent.transportFactoryContext(), false);
   THROW_IF_NOT_OK(server_config_or_error.status());
   server_config_ = std::move(server_config_or_error.value());
 

diff --git a/cilium/websocket_codec.cc b/cilium/websocket_codec.cc
@@ -78,7 +78,7 @@ class HttpParser : public Logger::Loggable<Logger::Id::filter> {
 
     ssize_t rc = http_parser_execute(&parser_, &settings, msg.data(), msg.length());
     ENVOY_LOG(trace, "websocket: http_parser parsed {} chars, error code: {}", rc,
-              HTTP_PARSER_ERRNO(&parser_));
+              static_cast<int>(HTTP_PARSER_ERRNO(&parser_)));
 
     // Errors in parsing HTTP.
     if (HTTP_PARSER_ERRNO(&parser_) != HPE_OK) {

diff --git a/cilium/websocket_config.cc b/cilium/websocket_config.cc
@@ -59,7 +59,7 @@ Config::Config(Server::Configuration::FactoryContext& context, bool client,
   x_rid_config.mutable_typed_config()->PackFrom(
       envoy::extensions::request_id::uuid::v3::UuidRequestIdConfig());
   auto extension_or_error = Http::RequestIDExtensionFactory::fromProto(x_rid_config, context);
-  THROW_IF_STATUS_NOT_OK(extension_or_error, throw);
+  THROW_IF_NOT_OK_REF(extension_or_error.status());
   request_id_extension_ = extension_or_error.value();
 
   // Base64 encode the given/expected key, if any.

diff --git a/tests/cilium_tls_http_integration_test.cc b/tests/cilium_tls_http_integration_test.cc
@@ -253,7 +253,7 @@ class CiliumHttpTLSIntegrationTest : public CiliumHttpIntegrationTest {
 
     auto server_config_or_error =
         Extensions::TransportSockets::Tls::ServerContextConfigImpl::create(tls_context,
-                                                                           factory_context_);
+                                                                           factory_context_, false);
     THROW_IF_NOT_OK(server_config_or_error.status());
     auto cfg = std::move(server_config_or_error.value());
 

diff --git a/tests/cilium_tls_tcp_integration_test.cc b/tests/cilium_tls_tcp_integration_test.cc
@@ -111,7 +111,7 @@ class CiliumTLSIntegrationTest : public CiliumTcpIntegrationTest {
         fmt::format("test/config/integration/certs/{}key.pem", upstream_cert_name_)));
 
     auto cfg_or_error = Extensions::TransportSockets::Tls::ServerContextConfigImpl::create(
-        tls_context, factory_context_);
+        tls_context, factory_context_, false);
     THROW_IF_NOT_OK(cfg_or_error.status());
     auto cfg = std::move(cfg_or_error.value());
 

diff --git a/tests/uds_server.cc b/tests/uds_server.cc
@@ -44,7 +44,7 @@ UDSServer::UDSServer(const std::string& path, std::function<void(const std::stri
     return;
   }
 
-  ENVOY_LOG(trace, "Starting unix domain socket server thread fd: {}", fd_);
+  ENVOY_LOG(trace, "Starting unix domain socket server thread fd: {}", fd_.load());
 
   thread_ = Thread::threadFactoryForTest().createThread([this]() { threadRoutine(); });
 }
@@ -72,32 +72,33 @@ void UDSServer::Close() {
 
 void UDSServer::threadRoutine() {
   while (fd_ >= 0) {
-    ENVOY_LOG(debug, "Unix domain socket server thread started on fd: {}", fd_);
+    ENVOY_LOG(debug, "Unix domain socket server thread started on fd: {}", fd_.load());
     // Accept a new connection
     struct sockaddr_un addr;
     socklen_t addr_len = sizeof(addr);
-    ENVOY_LOG(trace, "Unix domain socket server blocking accept on fd: {}", fd_);
+    ENVOY_LOG(trace, "Unix domain socket server blocking accept on fd: {}", fd_.load());
     fd2_ = ::accept(fd_, reinterpret_cast<sockaddr*>(&addr), &addr_len);
     if (fd2_ < 0) {
       if (errno == EINVAL) {
         return; // fd_ was closed
       }
-      ENVOY_LOG(warn, "Unix domain socket server accept on fd {} failed: {}", fd_,
+      ENVOY_LOG(warn, "Unix domain socket server accept on fd {} failed: {}", fd_.load(),
                 Envoy::errorDetails(errno));
       continue;
     }
     char buf[8192];
     while (true) {
-      ENVOY_LOG(trace, "Unix domain socket server blocking recv on fd: {}", fd2_);
+      ENVOY_LOG(trace, "Unix domain socket server blocking recv on fd: {}", fd2_.load());
       ssize_t received = ::recv(fd2_, buf, sizeof(buf), 0);
       if (received < 0) {
         if (errno == EINTR)
           continue;
-        ENVOY_LOG(warn, "Unix domain socket server recv on fd {} failed: {}", fd2_,
+        ENVOY_LOG(warn, "Unix domain socket server recv on fd {} failed: {}", fd2_.load(),
                   Envoy::errorDetails(errno));
         break;
       } else if (received == 0) {
-        ENVOY_LOG(trace, "Unix domain socket server client on fd {} has closed socket", fd2_);
+        ENVOY_LOG(trace, "Unix domain socket server client on fd {} has closed socket",
+                  fd2_.load());
         break;
       } else {
         std::string data(buf, received);