From b499269ba2dbaa59d7b2bd6019299fe8f7320333 Mon Sep 17 00:00:00 2001 From: Tam Mach Date: Tue, 19 Nov 2024 20:53:38 +1100 Subject: [PATCH] cilium: Adjust cilium filter build and implementation Relates: https://github.com/envoyproxy/envoy/pull/36187 Relates: https://github.com/envoyproxy/envoy/pull/34815 Relates: https://github.com/envoyproxy/envoy/pull/35694 Signed-off-by: Tam Mach --- cilium/BUILD | 2 +- cilium/grpc_subscription.cc | 4 +-- cilium/network_filter.cc | 5 +-- cilium/network_policy.cc | 7 +++-- cilium/proxylib.cc | 37 +++-------------------- cilium/proxylib.h | 22 ++++++++++++++ cilium/secret_watcher.cc | 2 +- cilium/websocket_codec.cc | 2 +- cilium/websocket_config.cc | 2 +- tests/cilium_tls_http_integration_test.cc | 2 +- tests/cilium_tls_tcp_integration_test.cc | 2 +- tests/uds_server.cc | 15 ++++----- 12 files changed, 50 insertions(+), 52 deletions(-) diff --git a/cilium/BUILD b/cilium/BUILD index 66ee680a0..b007aaa25 100644 --- 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 index 0325519b5..2869aa531 100644 --- 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(); 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 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 index db67badbc..c9b770760 100644 --- 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 index 2387b4876..e7e95a899 100644 --- a/cilium/network_policy.cc +++ b/cilium/network_policy.cc @@ -201,9 +201,10 @@ class HttpNetworkPolicyRule : public Logger::Loggable { "Cilium L7 HttpNetworkPolicyRule(): HeaderMatch {}={} (match: {}, mismatch: {})", header_match.name_.get(), header_match.secret_ ? fmt::format("", header_match.secret_->name()) - : header_match.value_.length() > 0 ? header_match.value_ - : "", - header_match.match_action_, header_match.mismatch_action_); + : !header_match.value_.empty() ? header_match.value_ + : "", + 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 index d29f2be43..82c203d8c 100644 --- 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 index 9219887e5..a67ab9f3e 100644 --- a/cilium/proxylib.h +++ b/cilium/proxylib.h @@ -38,6 +38,28 @@ template 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 index 475899853..7921ac1b1 100644 --- 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 index a3da68bcd..142211c13 100644 --- a/cilium/websocket_codec.cc +++ b/cilium/websocket_codec.cc @@ -78,7 +78,7 @@ class HttpParser : public Logger::Loggable { 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(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 index 9342b288c..26e5fb708 100644 --- 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 index 1d4dda595..c590c1457 100644 --- 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 index c8b86b837..f1bbaabff 100644 --- 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 index 07e25aca1..563fd24ee 100644 --- a/tests/uds_server.cc +++ b/tests/uds_server.cc @@ -44,7 +44,7 @@ UDSServer::UDSServer(const std::string& path, std::function= 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(&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);