Skip to content

Commit d6076a6

Browse files
authored
network: add ext proc config check for processing mode and grpc services (envoyproxy#39284)
Throw in this PR, but I think a better way is to change the interface of `createFilterFactoryFromProtoTyped` to return `absl:status` to avoid throw for network filters. Commit Message: Additional Description: Risk Level: low Testing: Docs Changes: Release Notes: Platform Specific Features: --------- Signed-off-by: Boteng Yao <[email protected]>
1 parent 9ff0844 commit d6076a6

File tree

3 files changed

+67
-0
lines changed

3 files changed

+67
-0
lines changed

source/extensions/filters/network/ext_proc/config.cc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,34 @@ namespace Extensions {
1515
namespace NetworkFilters {
1616
namespace ExtProc {
1717

18+
namespace {
19+
20+
absl::Status verifyFilterConfig(
21+
const envoy::extensions::filters::network::ext_proc::v3::NetworkExternalProcessor& config) {
22+
if (!config.has_grpc_service()) {
23+
return absl::InvalidArgumentError("A grpc_service must be configured");
24+
}
25+
26+
if (config.processing_mode().process_read() ==
27+
envoy::extensions::filters::network::ext_proc::v3::ProcessingMode::SKIP &&
28+
config.processing_mode().process_write() ==
29+
envoy::extensions::filters::network::ext_proc::v3::ProcessingMode::SKIP) {
30+
return absl::InvalidArgumentError(
31+
"both read and write paths are skipped, at least one must be enabled.");
32+
}
33+
return absl::OkStatus();
34+
}
35+
36+
} // namespace
37+
1838
Network::FilterFactoryCb NetworkExtProcConfigFactory::createFilterFactoryFromProtoTyped(
1939
const envoy::extensions::filters::network::ext_proc::v3::NetworkExternalProcessor& proto_config,
2040
Server::Configuration::FactoryContext& context) {
41+
absl::Status result = verifyFilterConfig(proto_config);
42+
if (!result.ok()) {
43+
throw EnvoyException(std::string(result.message()));
44+
}
45+
2146
ConfigConstSharedPtr ext_proc_config = std::make_shared<const Config>(proto_config);
2247

2348
return [ext_proc_config, &context](Network::FilterManager& filter_manager) -> void {

test/extensions/filters/network/ext_proc/config_test.cc

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,47 @@ TEST(NetworkExtProcConfigTest, ConfigWithOptions) {
109109
cb(filter_manager);
110110
}
111111

112+
// Test the config without a gRPC service.
113+
TEST(NetworkExtProcConfigFactoryTest, MissingGrpcService) {
114+
envoy::extensions::filters::network::ext_proc::v3::NetworkExternalProcessor proto_config;
115+
116+
NiceMock<Server::Configuration::MockFactoryContext> context;
117+
NetworkExtProcConfigFactory factory;
118+
119+
EXPECT_THROW_WITH_MESSAGE(auto cb = factory.createFilterFactoryFromProto(proto_config, context),
120+
EnvoyException, "A grpc_service must be configured");
121+
}
122+
123+
// Test the config with both SKIP modes.
124+
TEST(NetworkExtProcConfigFactoryTest, BothModesSkipped) {
125+
envoy::extensions::filters::network::ext_proc::v3::NetworkExternalProcessor proto_config;
126+
proto_config.mutable_grpc_service()->mutable_envoy_grpc()->set_cluster_name("ext_proc_server");
127+
128+
auto* processing_mode = proto_config.mutable_processing_mode();
129+
processing_mode->set_process_read(
130+
envoy::extensions::filters::network::ext_proc::v3::ProcessingMode::SKIP);
131+
processing_mode->set_process_write(
132+
envoy::extensions::filters::network::ext_proc::v3::ProcessingMode::SKIP);
133+
134+
NiceMock<Server::Configuration::MockFactoryContext> context;
135+
NetworkExtProcConfigFactory factory;
136+
137+
EXPECT_THROW_WITH_MESSAGE(auto cb = factory.createFilterFactoryFromProto(proto_config, context),
138+
EnvoyException,
139+
"both read and write paths are skipped, at least one must be enabled.");
140+
}
141+
142+
// Test the configs with default processing modes.
143+
TEST(NetworkExtProcConfigFactoryTest, DefaultProcessingMode) {
144+
envoy::extensions::filters::network::ext_proc::v3::NetworkExternalProcessor proto_config;
145+
proto_config.mutable_grpc_service()->mutable_envoy_grpc()->set_cluster_name("ext_proc_server");
146+
147+
NiceMock<Server::Configuration::MockFactoryContext> context;
148+
NetworkExtProcConfigFactory factory;
149+
150+
EXPECT_NO_THROW(auto cb = factory.createFilterFactoryFromProto(proto_config, context));
151+
}
152+
112153
} // namespace
113154
} // namespace ExtProc
114155
} // namespace NetworkFilters

tools/code_format/config.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ paths:
192192
- source/extensions/filters/network/redis_proxy
193193
- source/extensions/filters/network/zookeeper_proxy
194194
- source/extensions/filters/network/ext_authz
195+
- source/extensions/filters/network/ext_proc
195196
- source/extensions/filters/network/mongo_proxy
196197
- source/extensions/filters/network/thrift_proxy
197198
- source/extensions/filters/network/generic_proxy

0 commit comments

Comments
 (0)