Skip to content

Commit 36171f3

Browse files
committed
fix: handle missing generic L7 metadata entries
Signed-off-by: immanuwell <pchpr.00@list.ru>
1 parent 9284145 commit 36171f3

4 files changed

Lines changed: 116 additions & 1 deletion

File tree

cilium/network_filter.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,11 @@ Network::FilterStatus Instance::onData(Buffer::Instance& data, bool end_stream)
312312
go_parser_->setOrigEndStream(end_stream);
313313
} else if (!l7proto_.empty()) {
314314
const auto& metadata = conn.streamInfo().dynamicMetadata();
315-
bool changed = log_entry_.updateFromMetadata(l7proto_, metadata.filter_metadata().at(l7proto_));
315+
const auto& filter_metadata = metadata.filter_metadata();
316+
const auto metadata_it = filter_metadata.find(l7proto_);
317+
const Protobuf::Struct empty_metadata;
318+
bool changed = log_entry_.updateFromMetadata(
319+
l7proto_, metadata_it != filter_metadata.end() ? metadata_it->second : empty_metadata);
316320

317321
// Policy may have changed since the connection was established, get fresh policy
318322
const auto policy_fs =

cilium/network_filter.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ namespace Envoy {
2727
namespace Filter {
2828
namespace CiliumL3 {
2929

30+
class NetworkFilterTestPeer;
31+
3032
/**
3133
* Shared configuration for Cilium network filter worker
3234
* Instances. Each new network connection (on each worker thread)
@@ -67,6 +69,8 @@ class Instance : public Network::Filter, Logger::Loggable<Logger::Id::filter> {
6769
Network::FilterStatus onWrite(Buffer::Instance&, bool end_stream) override;
6870

6971
private:
72+
friend class NetworkFilterTestPeer;
73+
7074
// helper to be used either directly from onNewConnection (no L7 LB),
7175
// or from upstream callback (l7 lb)
7276
bool enforceNetworkPolicy(const Cilium::CiliumPolicyFilterState* policy_fs,

tests/BUILD

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,17 @@ envoy_cc_test(
157157
],
158158
)
159159

160+
envoy_cc_test(
161+
name = "network_filter_test",
162+
srcs = ["network_filter_test.cc"],
163+
repository = "@envoy",
164+
deps = [
165+
"//cilium:network_filter_lib",
166+
"@envoy//test/mocks/network:network_mocks",
167+
"@envoy//test/mocks/server:listener_factory_context_mocks",
168+
],
169+
)
170+
160171
envoy_cc_test(
161172
name = "cilium_tcp_integration_test",
162173
srcs = ["cilium_tcp_integration_test.cc"],

tests/network_filter_test.cc

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#include <cstdint>
2+
#include <memory>
3+
#include <string>
4+
#include <utility>
5+
6+
#include <gmock/gmock.h>
7+
#include <gtest/gtest.h>
8+
9+
#include "cilium/accesslog.h"
10+
#include "cilium/api/network_filter.pb.h"
11+
#include "cilium/filter_state_cilium_policy.h"
12+
#include "cilium/network_filter.h"
13+
#include "cilium/network_policy.h"
14+
15+
#include "envoy/network/address.h"
16+
#include "envoy/network/filter.h"
17+
#include "envoy/stream_info/filter_state.h"
18+
#include "envoy/stream_info/stream_info.h"
19+
#include "envoy/upstream/host_description.h"
20+
21+
#include "source/common/buffer/buffer_impl.h"
22+
23+
#include "test/mocks/network/mocks.h"
24+
#include "test/mocks/server/listener_factory_context.h"
25+
26+
namespace Envoy {
27+
namespace Filter {
28+
namespace CiliumL3 {
29+
30+
class NetworkFilterTestPeer {
31+
public:
32+
static void setL7Proto(Instance& instance, std::string l7proto) {
33+
instance.l7proto_ = std::move(l7proto);
34+
}
35+
36+
static const Cilium::AccessLog::Entry& logEntry(const Instance& instance) {
37+
return instance.log_entry_;
38+
}
39+
};
40+
41+
} // namespace CiliumL3
42+
} // namespace Filter
43+
44+
namespace {
45+
46+
using testing::NiceMock;
47+
48+
class TestReadFilterCallbacks : public Network::MockReadFilterCallbacks {
49+
public:
50+
TestReadFilterCallbacks() {
51+
ON_CALL(*this, addUpstreamCallback(testing::_))
52+
.WillByDefault(testing::Invoke([](const Network::UpstreamCallback&) {}));
53+
ON_CALL(*this, iterateUpstreamCallbacks(testing::_, testing::_))
54+
.WillByDefault(testing::Return(true));
55+
}
56+
57+
MOCK_METHOD(void, addUpstreamCallback, (const Network::UpstreamCallback& cb), (override));
58+
MOCK_METHOD(bool, iterateUpstreamCallbacks,
59+
(Upstream::HostDescriptionConstSharedPtr, StreamInfo::StreamInfo&), (override));
60+
};
61+
62+
class DenyAllPolicyResolver : public Cilium::PolicyResolver {
63+
public:
64+
uint32_t resolvePolicyId(const Network::Address::Ip*) const override { return 123; }
65+
66+
const Cilium::PolicyInstance& getPolicy(const std::string&) const override {
67+
return Cilium::NetworkPolicyMap::getDenyAllPolicy();
68+
}
69+
70+
bool exists(const std::string&) const override { return true; }
71+
};
72+
73+
TEST(CiliumNetworkFilterTest, MissingMetadataNamespaceDoesNotCrash) {
74+
NiceMock<Server::Configuration::MockListenerFactoryContext> context;
75+
::cilium::NetworkFilter proto_config;
76+
auto config = std::make_shared<Filter::CiliumL3::Config>(proto_config, context);
77+
Filter::CiliumL3::Instance instance(config);
78+
79+
NiceMock<TestReadFilterCallbacks> callbacks;
80+
callbacks.connection_.stream_info_.filter_state_->setData(
81+
Cilium::CiliumPolicyFilterState::key(),
82+
std::make_shared<Cilium::CiliumPolicyFilterState>(
83+
0, 456, false, false, 80, std::string("pod"), std::string(""),
84+
std::make_shared<DenyAllPolicyResolver>(), 7, ""),
85+
StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Connection);
86+
instance.initializeReadFilterCallbacks(callbacks);
87+
Filter::CiliumL3::NetworkFilterTestPeer::setL7Proto(instance, "test.l7");
88+
89+
Buffer::OwnedImpl data("hello");
90+
EXPECT_NO_THROW(instance.onData(data, false));
91+
EXPECT_EQ(Filter::CiliumL3::NetworkFilterTestPeer::logEntry(instance).entry_.generic_l7().proto(),
92+
"test.l7");
93+
}
94+
95+
} // namespace
96+
} // namespace Envoy

0 commit comments

Comments
 (0)