Skip to content

Commit e89daf0

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

7 files changed

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

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,130 @@ 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(MutationUtils::protoToHeaders(proto_headers, headers, checker, rejections),
937+
HasStatus(absl::StatusCode::kInvalidArgument, "header_mutation_set_headers_failed"));
938+
}
939+
940+
// 2. CheckResult::IGNORE for set_headers in protoToHeaders
941+
{
942+
HeaderMutationRules rules_ignore;
943+
rules_ignore.mutable_disallow_all()->set_value(true);
944+
rules_ignore.mutable_disallow_is_error()->set_value(false);
945+
Checker checker_ignore(rules_ignore, regex_engine_);
946+
envoy::config::core::v3::HeaderMap proto_headers;
947+
auto* h = proto_headers.add_headers();
948+
h->set_key("x-new-header-2");
949+
h->set_raw_value("value");
950+
EXPECT_CALL(rejections, inc());
951+
EXPECT_OK(MutationUtils::protoToHeaders(proto_headers, headers, checker_ignore, rejections));
952+
EXPECT_TRUE(headers.get(LowerCaseString("x-new-header-2")).empty());
953+
}
954+
}
955+
832956
} // namespace
833957
} // namespace ExternalProcessing
834958
} // namespace HttpFilters

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
#include "source/extensions/http/ext_proc/response_processors/save_processing_response/save_processing_response.h"
2+
#include "test/mocks/stream_info/mocks.h"
3+
#include "test/test_common/utility.h"
4+
#include "gtest/gtest.h"
5+
6+
namespace Envoy {
7+
namespace Http {
8+
namespace ExternalProcessing {
9+
namespace {
10+
11+
using envoy::service::ext_proc::v3::ProcessingResponse;
12+
using envoy::service::ext_proc::v3::StreamedImmediateResponse;
13+
14+
TEST(SaveProcessingResponseTest, SaveStreamingImmediateResponse) {
15+
envoy::extensions::http::ext_proc::response_processors::save_processing_response::v3::
16+
SaveProcessingResponse config;
17+
config.mutable_save_immediate_response()->set_save_response(true);
18+
19+
SaveProcessingResponse processor(config);
20+
21+
StreamedImmediateResponse response;
22+
response.mutable_headers_response()->set_end_of_stream(true);
23+
24+
NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
25+
26+
processor.afterProcessingStreamingImmediateResponse(response, absl::OkStatus(), stream_info);
27+
28+
auto filter_state = stream_info.filterState()->getDataReadOnly<SaveProcessingResponseFilterState>(
29+
SaveProcessingResponseFilterState::kFilterStateName);
30+
ASSERT_NE(filter_state, nullptr);
31+
EXPECT_EQ(filter_state->response->processing_response.response_case(),
32+
ProcessingResponse::ResponseCase::kStreamedImmediateResponse);
33+
EXPECT_TRUE(TestUtility::protoEqual(filter_state->response->processing_response.streamed_immediate_response(), response));
34+
}
35+
36+
TEST(SaveProcessingResponseTest, SaveStreamingImmediateResponseOnError) {
37+
envoy::extensions::http::ext_proc::response_processors::save_processing_response::v3::
38+
SaveProcessingResponse config;
39+
config.mutable_save_immediate_response()->set_save_response(true);
40+
config.mutable_save_immediate_response()->set_save_on_error(true);
41+
42+
SaveProcessingResponse processor(config);
43+
44+
StreamedImmediateResponse response;
45+
response.mutable_headers_response()->set_end_of_stream(true);
46+
47+
NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
48+
49+
auto error_status = absl::InternalError("test error");
50+
processor.afterProcessingStreamingImmediateResponse(response, error_status, stream_info);
51+
52+
auto filter_state = stream_info.filterState()->getDataReadOnly<SaveProcessingResponseFilterState>(
53+
SaveProcessingResponseFilterState::kFilterStateName);
54+
ASSERT_NE(filter_state, nullptr);
55+
EXPECT_EQ(filter_state->response->processing_status, error_status);
56+
}
57+
58+
TEST(SaveProcessingResponseTest, DontSaveStreamingImmediateResponseOnError) {
59+
envoy::extensions::http::ext_proc::response_processors::save_processing_response::v3::
60+
SaveProcessingResponse config;
61+
config.mutable_save_immediate_response()->set_save_response(true);
62+
config.mutable_save_immediate_response()->set_save_on_error(false);
63+
64+
SaveProcessingResponse processor(config);
65+
66+
StreamedImmediateResponse response;
67+
NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
68+
69+
processor.afterProcessingStreamingImmediateResponse(response, absl::InternalError("test error"), stream_info);
70+
71+
auto filter_state = stream_info.filterState()->getDataReadOnly<SaveProcessingResponseFilterState>(
72+
SaveProcessingResponseFilterState::kFilterStateName);
73+
EXPECT_EQ(filter_state, nullptr);
74+
}
75+
76+
} // namespace
77+
} // namespace ExternalProcessing
78+
} // namespace Http
79+
} // namespace Envoy

0 commit comments

Comments
 (0)