Skip to content

Commit 71026e1

Browse files
etruong42etruong42
committed
Improve ext_proc code coverage
Signed-off-by: Ethan Truong <ethantruong@google.com>
1 parent 8db119b commit 71026e1

7 files changed

Lines changed: 506 additions & 0 deletions

File tree

‎test/extensions/filters/http/ext_proc/BUILD‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,33 @@ envoy_extension_cc_test(
345345
],
346346
)
347347

348+
envoy_extension_cc_test(
349+
name = "save_processing_response_test",
350+
size = "small",
351+
srcs = ["save_processing_response_test.cc"],
352+
extension_names = ["envoy.filters.http.ext_proc"],
353+
rbe_pool = "6gig",
354+
tags = ["skip_on_windows"],
355+
deps = [
356+
"//source/extensions/http/ext_proc/response_processors/save_processing_response:save_processing_response_lib",
357+
"//test/mocks/stream_info:stream_info_mocks",
358+
"//test/test_common:utility_lib",
359+
],
360+
)
361+
362+
envoy_extension_cc_test(
363+
name = "logging_info_test",
364+
size = "small",
365+
srcs = ["logging_info_test.cc"],
366+
extension_names = ["envoy.filters.http.ext_proc"],
367+
rbe_pool = "6gig",
368+
tags = ["skip_on_windows"],
369+
deps = [
370+
"//source/extensions/filters/http/ext_proc",
371+
"//test/test_common:utility_lib",
372+
],
373+
)
374+
348375
envoy_extension_cc_test(
349376
name = "mutation_utils_test",
350377
size = "small",

‎test/extensions/filters/http/ext_proc/filter_test.cc‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5139,6 +5139,39 @@ TEST_F(HttpFilterTest, SaveProcessingResponseHeaders) {
51395139
EXPECT_EQ(1, config_->stats().streams_closed_.value());
51405140
}
51415141

5142+
TEST_F(HttpFilterTest, OverrideMessageTimeout) {
5143+
initialize(R"EOF(
5144+
grpc_service:
5145+
envoy_grpc:
5146+
cluster_name: "ext_proc_server"
5147+
max_message_timeout: 10s
5148+
)EOF");
5149+
5150+
EXPECT_EQ(FilterHeadersStatus::StopIteration, filter_->decodeHeaders(request_headers_, false));
5151+
5152+
// Receive override_message_timeout
5153+
auto response = std::make_unique<ProcessingResponse>();
5154+
response->mutable_override_message_timeout()->set_seconds(5);
5155+
5156+
stream_callbacks_->onReceiveMessage(std::move(response));
5157+
5158+
EXPECT_EQ(1, config_->stats().override_message_timeout_received_.value());
5159+
5160+
// Test invalid timeout (too large)
5161+
response = std::make_unique<ProcessingResponse>();
5162+
response->mutable_override_message_timeout()->set_seconds(20);
5163+
stream_callbacks_->onReceiveMessage(std::move(response));
5164+
EXPECT_EQ(1, config_->stats().override_message_timeout_ignored_.value());
5165+
5166+
// Test invalid timeout (too small)
5167+
response = std::make_unique<ProcessingResponse>();
5168+
response->mutable_override_message_timeout()->set_nanos(0);
5169+
stream_callbacks_->onReceiveMessage(std::move(response));
5170+
EXPECT_EQ(2, config_->stats().override_message_timeout_ignored_.value());
5171+
5172+
filter_->onDestroy();
5173+
}
5174+
51425175
TEST_F(HttpFilterTest, OnProcessingResponseBodies) {
51435176
TestOnProcessingResponseFactory factory;
51445177
Envoy::Registry::InjectFactory<OnProcessingResponseFactory> registration(factory);

‎test/extensions/filters/http/ext_proc/logging_info_test.cc‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
#include "source/extensions/filters/http/ext_proc/ext_proc.h"
2+
3+
#include "test/test_common/utility.h"
4+
5+
#include "gtest/gtest.h"
6+
7+
namespace Envoy {
8+
namespace Extensions {
9+
namespace HttpFilters {
10+
namespace ExternalProcessing {
11+
namespace {
12+
13+
using envoy::config::core::v3::TrafficDirection;
14+
using Filters::Common::ProcessingEffect::Effect;
15+
16+
class ExtProcLoggingInfoTest : public ::testing::Test {
17+
public:
18+
ExtProcLoggingInfoTest() : logging_info_(filter_metadata_) {}
19+
20+
Protobuf::Struct filter_metadata_;
21+
ExtProcLoggingInfo logging_info_;
22+
};
23+
24+
TEST_F(ExtProcLoggingInfoTest, SerializeEmpty) {
25+
auto proto = logging_info_.serializeAsProto();
26+
EXPECT_TRUE(proto != nullptr);
27+
28+
auto str = logging_info_.serializeAsString();
29+
EXPECT_TRUE(str.has_value());
30+
EXPECT_EQ(str.value(), "bs:0,br:0,os:0");
31+
}
32+
33+
TEST_F(ExtProcLoggingInfoTest, RecordAndSerialize) {
34+
logging_info_.recordGrpcCall(std::chrono::microseconds(100), Grpc::Status::Ok,
35+
ProcessorState::CallbackState::HeadersCallback,
36+
TrafficDirection::INBOUND);
37+
logging_info_.recordGrpcCall(std::chrono::microseconds(200), Grpc::Status::Aborted,
38+
ProcessorState::CallbackState::TrailersCallback,
39+
TrafficDirection::INBOUND);
40+
logging_info_.recordGrpcCall(std::chrono::microseconds(300), Grpc::Status::Ok,
41+
ProcessorState::CallbackState::BufferedBodyCallback,
42+
TrafficDirection::INBOUND);
43+
logging_info_.recordGrpcCall(std::chrono::microseconds(400), Grpc::Status::Ok,
44+
ProcessorState::CallbackState::BufferedBodyCallback,
45+
TrafficDirection::INBOUND);
46+
47+
logging_info_.recordGrpcCall(std::chrono::microseconds(500), Grpc::Status::Ok,
48+
ProcessorState::CallbackState::HeadersCallback,
49+
TrafficDirection::OUTBOUND);
50+
logging_info_.recordGrpcCall(std::chrono::microseconds(600), Grpc::Status::Ok,
51+
ProcessorState::CallbackState::TrailersCallback,
52+
TrafficDirection::OUTBOUND);
53+
logging_info_.recordGrpcCall(std::chrono::microseconds(700), Grpc::Status::Ok,
54+
ProcessorState::CallbackState::BufferedBodyCallback,
55+
TrafficDirection::OUTBOUND);
56+
57+
logging_info_.setBytesSent(1000);
58+
logging_info_.setBytesReceived(2000);
59+
logging_info_.recordGrpcStatusBeforeFirstCall(Grpc::Status::Internal);
60+
logging_info_.setFailedOpen();
61+
logging_info_.setReceivedImmediateResponse();
62+
63+
logging_info_.recordProcessingEffect(ProcessorState::CallbackState::HeadersCallback,
64+
TrafficDirection::INBOUND, Effect::MutationApplied);
65+
logging_info_.recordProcessingEffect(ProcessorState::CallbackState::TrailersCallback,
66+
TrafficDirection::INBOUND, Effect::MutationFailed);
67+
logging_info_.recordProcessingEffect(ProcessorState::CallbackState::BufferedBodyCallback,
68+
TrafficDirection::INBOUND, Effect::None);
69+
70+
auto proto = logging_info_.serializeAsProto();
71+
ASSERT_TRUE(proto != nullptr);
72+
const auto& fields = dynamic_cast<const Protobuf::Struct&>(*proto).fields();
73+
74+
EXPECT_EQ(fields.at("request_header_latency_us").number_value(), 100);
75+
EXPECT_EQ(fields.at("request_trailer_latency_us").number_value(), 200);
76+
EXPECT_EQ(fields.at("request_body_call_count").number_value(), 2);
77+
EXPECT_EQ(fields.at("request_body_total_latency_us").number_value(), 700);
78+
EXPECT_EQ(fields.at("request_body_max_latency_us").number_value(), 400);
79+
80+
EXPECT_EQ(fields.at("bytes_sent").number_value(), 1000);
81+
EXPECT_EQ(fields.at("bytes_received").number_value(), 2000);
82+
EXPECT_TRUE(fields.at("failed_open").bool_value());
83+
EXPECT_TRUE(fields.at("received_immediate_response").bool_value());
84+
EXPECT_EQ(fields.at("grpc_status_before_first_call").number_value(),
85+
static_cast<int>(Grpc::Status::Internal));
86+
87+
auto str = logging_info_.serializeAsString();
88+
EXPECT_TRUE(str.has_value());
89+
// `rh:100:0,rb:2:700:0,rt:200:10,sh:500:0,sb:1:700:0,st:600:0,bs:1000,br:2000,os:13`
90+
EXPECT_THAT(str.value(), testing::HasSubstr("rh:100:0"));
91+
EXPECT_THAT(str.value(), testing::HasSubstr("rb:2:700:0"));
92+
EXPECT_THAT(str.value(), testing::HasSubstr("rt:200:10"));
93+
EXPECT_THAT(str.value(), testing::HasSubstr("bs:1000"));
94+
EXPECT_THAT(str.value(), testing::HasSubstr("br:2000"));
95+
EXPECT_THAT(str.value(), testing::HasSubstr("os:13"));
96+
}
97+
98+
TEST_F(ExtProcLoggingInfoTest, GetField) {
99+
logging_info_.recordGrpcCall(std::chrono::microseconds(100), Grpc::Status::Ok,
100+
ProcessorState::CallbackState::HeadersCallback,
101+
TrafficDirection::INBOUND);
102+
logging_info_.setBytesSent(1000);
103+
logging_info_.setFailedOpen();
104+
105+
EXPECT_EQ(absl::get<int64_t>(logging_info_.getField("request_header_latency_us")), 100);
106+
EXPECT_EQ(absl::get<int64_t>(logging_info_.getField("bytes_sent")), 1000);
107+
EXPECT_EQ(absl::get<int64_t>(logging_info_.getField("failed_open")), 1);
108+
EXPECT_TRUE(absl::holds_alternative<absl::monostate>(logging_info_.getField("non_existent")));
109+
}
110+
111+
} // namespace
112+
} // namespace ExternalProcessing
113+
} // namespace HttpFilters
114+
} // namespace Extensions
115+
} // namespace Envoy

‎test/extensions/filters/http/ext_proc/mutation_utils_test.cc‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,131 @@ TEST_F(MutationUtilsTest, InvalidStatusRejected) {
829829
HasStatus(absl::StatusCode::kInvalidArgument, "header_mutation_set_headers_failed"));
830830
}
831831

832+
TEST_F(MutationUtilsTest, TestApplyMutationsFailures) {
833+
Http::TestRequestHeaderMapImpl headers{
834+
{":method", "GET"},
835+
{"x-remove-me", "yes"},
836+
{"x-keep-me", "yes"},
837+
};
838+
839+
HeaderMutationRules rules;
840+
rules.mutable_disallow_is_error()->set_value(true);
841+
rules.mutable_disallow_all()->set_value(true);
842+
Checker checker(rules, regex_engine_);
843+
Envoy::Stats::MockCounter rejections;
844+
Effect effect = Effect::None;
845+
846+
// 1. CheckResult::FAIL for remove_headers
847+
{
848+
envoy::service::ext_proc::v3::HeaderMutation mutation;
849+
mutation.add_remove_headers("x-remove-me");
850+
EXPECT_CALL(rejections, inc());
851+
EXPECT_THAT(
852+
MutationUtils::applyHeaderMutations(mutation, headers, false, checker, rejections, effect),
853+
HasStatus(absl::StatusCode::kInvalidArgument, "header_mutation_remove_headers_failed"));
854+
EXPECT_THAT(effect, Effect::MutationFailed);
855+
}
856+
857+
// 2. CheckResult::FAIL for set_headers
858+
{
859+
envoy::service::ext_proc::v3::HeaderMutation mutation;
860+
auto* s = mutation.add_set_headers();
861+
s->mutable_header()->set_key("x-new-header");
862+
s->mutable_header()->set_raw_value("value");
863+
EXPECT_CALL(rejections, inc());
864+
EXPECT_THAT(
865+
MutationUtils::applyHeaderMutations(mutation, headers, false, checker, rejections, effect),
866+
HasStatus(absl::StatusCode::kInvalidArgument, "header_mutation_set_headers_failed"));
867+
EXPECT_THAT(effect, Effect::MutationFailed);
868+
}
869+
870+
// 3. CheckResult::IGNORE for remove_headers
871+
{
872+
HeaderMutationRules rules_ignore;
873+
rules_ignore.mutable_disallow_all()->set_value(true);
874+
rules_ignore.mutable_disallow_is_error()->set_value(false);
875+
Checker checker_ignore(rules_ignore, regex_engine_);
876+
envoy::service::ext_proc::v3::HeaderMutation mutation;
877+
mutation.add_remove_headers("x-remove-me");
878+
EXPECT_CALL(rejections, inc());
879+
EXPECT_OK(MutationUtils::applyHeaderMutations(mutation, headers, false, checker_ignore,
880+
rejections, effect));
881+
EXPECT_THAT(effect, Effect::None);
882+
EXPECT_EQ(headers.get(LowerCaseString("x-remove-me"))[0]->value(), "yes");
883+
}
884+
885+
// 4. CheckResult::IGNORE for set_headers
886+
{
887+
HeaderMutationRules rules_ignore;
888+
rules_ignore.mutable_disallow_all()->set_value(true);
889+
rules_ignore.mutable_disallow_is_error()->set_value(false);
890+
Checker checker_ignore(rules_ignore, regex_engine_);
891+
envoy::service::ext_proc::v3::HeaderMutation mutation;
892+
auto* s = mutation.add_set_headers();
893+
s->mutable_header()->set_key("x-new-header");
894+
s->mutable_header()->set_raw_value("value");
895+
EXPECT_CALL(rejections, inc());
896+
EXPECT_OK(MutationUtils::applyHeaderMutations(mutation, headers, false, checker_ignore,
897+
rejections, effect));
898+
EXPECT_THAT(effect, Effect::None);
899+
EXPECT_TRUE(headers.get(LowerCaseString("x-new-header")).empty());
900+
}
901+
902+
// 5. Special handling for :method when replacing_message is true
903+
{
904+
HeaderMutationRules rules_method;
905+
rules_method.mutable_disallow_all()->set_value(true);
906+
rules_method.mutable_disallow_is_error()->set_value(true);
907+
Checker checker_method(rules_method, regex_engine_);
908+
envoy::service::ext_proc::v3::HeaderMutation mutation;
909+
auto* s = mutation.add_set_headers();
910+
s->mutable_header()->set_key(":method");
911+
s->mutable_header()->set_raw_value("POST");
912+
// replacing_message = true should allow :method even if disallowed by rules
913+
EXPECT_OK(MutationUtils::applyHeaderMutations(mutation, headers, true, checker_method,
914+
rejections, effect));
915+
EXPECT_THAT(effect, Effect::MutationApplied);
916+
EXPECT_EQ(headers.Method()->value(), "POST");
917+
}
918+
}
919+
920+
TEST_F(MutationUtilsTest, TestProtoToHeadersFailures) {
921+
Http::TestResponseHeaderMapImpl headers;
922+
Envoy::Stats::MockCounter rejections;
923+
924+
HeaderMutationRules rules;
925+
rules.mutable_disallow_all()->set_value(true);
926+
rules.mutable_disallow_is_error()->set_value(true);
927+
Checker checker(rules, regex_engine_);
928+
929+
// 1. CheckResult::FAIL for set_headers in protoToHeaders
930+
{
931+
envoy::config::core::v3::HeaderMap proto_headers;
932+
auto* h = proto_headers.add_headers();
933+
h->set_key("x-new-header");
934+
h->set_raw_value("value");
935+
EXPECT_CALL(rejections, inc());
936+
EXPECT_THAT(
937+
MutationUtils::protoToHeaders(proto_headers, headers, checker, rejections),
938+
HasStatus(absl::StatusCode::kInvalidArgument, "header_mutation_set_headers_failed"));
939+
}
940+
941+
// 2. CheckResult::IGNORE for set_headers in protoToHeaders
942+
{
943+
HeaderMutationRules rules_ignore;
944+
rules_ignore.mutable_disallow_all()->set_value(true);
945+
rules_ignore.mutable_disallow_is_error()->set_value(false);
946+
Checker checker_ignore(rules_ignore, regex_engine_);
947+
envoy::config::core::v3::HeaderMap proto_headers;
948+
auto* h = proto_headers.add_headers();
949+
h->set_key("x-new-header-2");
950+
h->set_raw_value("value");
951+
EXPECT_CALL(rejections, inc());
952+
EXPECT_OK(MutationUtils::protoToHeaders(proto_headers, headers, checker_ignore, rejections));
953+
EXPECT_TRUE(headers.get(LowerCaseString("x-new-header-2")).empty());
954+
}
955+
}
956+
832957
} // namespace
833958
} // namespace ExternalProcessing
834959
} // namespace HttpFilters

0 commit comments

Comments
 (0)