Chiebot-Mirror
/
grpc
8
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

xds: Ignore HTTP filters if LDS resource is v2. (#25694)

Browse Source
pull/25701/head
Mark D. Roth 4 years ago committed by GitHub
parent e34b0a73c6
commit d2c2d66a03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 153 additions and 84 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 165
      src/core/ext/xds/xds_api.cc
  2. 72
      test/cpp/end2end/xds_end2end_test.cc

165
src/core/ext/xds/xds_api.cc
Unescape View File

@ -688,8 +688,13 @@ const char* kCdsV2TypeUrl = "type.googleapis.com/envoy.api.v2.Cluster";
const char* kEdsV2TypeUrl =
"type.googleapis.com/envoy.api.v2.ClusterLoadAssignment";
bool IsLds(absl::string_view type_url) {
return type_url == XdsApi::kLdsTypeUrl || type_url == kLdsV2TypeUrl;
bool IsLds(absl::string_view type_url, bool* is_v2 = nullptr) {
if (type_url == XdsApi::kLdsTypeUrl) return true;
if (type_url == kLdsV2TypeUrl) {
if (is_v2 != nullptr) *is_v2 = true;
return true;
}
return false;
}
bool IsRds(absl::string_view type_url) {
@ -1664,6 +1669,7 @@ grpc_error* HttpConnectionManagerParse(
bool is_client, const EncodingContext& context,
const envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager*
http_connection_manager_proto,
bool is_v2,
XdsApi::LdsUpdate::HttpConnectionManager* http_connection_manager) {
MaybeLogHttpConnectionManager(context, http_connection_manager_proto);
if (XdsTimeoutEnabled()) {
@ -1683,70 +1689,81 @@ grpc_error* HttpConnectionManagerParse(
}
}
// Parse filters.
if ((XdsSecurityEnabled() || XdsFaultInjectionEnabled()) && context.use_v3) {
size_t num_filters = 0;
const auto* http_filters =
envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_http_filters(
http_connection_manager_proto, &num_filters);
std::set<absl::string_view> names_seen;
for (size_t i = 0; i < num_filters; ++i) {
const auto* http_filter = http_filters[i];
absl::string_view name = UpbStringToAbsl(
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_name(
http_filter));
if (name.empty()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("empty filter name at index ", i).c_str());
}
if (names_seen.find(name) != names_seen.end()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("duplicate HTTP filter name: ", name).c_str());
}
names_seen.insert(name);
const bool is_optional =
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_is_optional(
http_filter);
const google_protobuf_Any* any =
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_typed_config(
http_filter);
if (any == nullptr) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("no filter config specified for filter name ", name)
.c_str());
}
absl::string_view filter_type;
grpc_error* error = ExtractHttpFilterTypeName(context, any, &filter_type);
if (error != GRPC_ERROR_NONE) return error;
const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType(filter_type);
if (filter_impl == nullptr) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("no filter registered for config type ", filter_type)
.c_str());
}
if ((is_client && !filter_impl->IsSupportedOnClients()) ||
(!is_client && !filter_impl->IsSupportedOnServers())) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrFormat("Filter %s is not supported on %s", filter_type,
is_client ? "clients" : "servers")
.c_str());
}
absl::StatusOr<XdsHttpFilterImpl::FilterConfig> filter_config =
filter_impl->GenerateFilterConfig(google_protobuf_Any_value(any),
context.arena);
if (!filter_config.ok()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat(
"filter config for type ", filter_type,
" failed to parse: ", filter_config.status().ToString())
.c_str());
if (XdsSecurityEnabled() || XdsFaultInjectionEnabled()) {
if (!is_v2) {
size_t num_filters = 0;
const auto* http_filters =
envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_http_filters(
http_connection_manager_proto, &num_filters);
std::set<absl::string_view> names_seen;
for (size_t i = 0; i < num_filters; ++i) {
const auto* http_filter = http_filters[i];
absl::string_view name = UpbStringToAbsl(
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_name(
http_filter));
if (name.empty()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("empty filter name at index ", i).c_str());
}
if (names_seen.find(name) != names_seen.end()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("duplicate HTTP filter name: ", name).c_str());
}
names_seen.insert(name);
const bool is_optional =
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_is_optional(
http_filter);
const google_protobuf_Any* any =
envoy_extensions_filters_network_http_connection_manager_v3_HttpFilter_typed_config(
http_filter);
if (any == nullptr) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("no filter config specified for filter name ", name)
.c_str());
}
absl::string_view filter_type;
grpc_error* error =
ExtractHttpFilterTypeName(context, any, &filter_type);
if (error != GRPC_ERROR_NONE) return error;
const XdsHttpFilterImpl* filter_impl =
XdsHttpFilterRegistry::GetFilterForType(filter_type);
if (filter_impl == nullptr) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("no filter registered for config type ", filter_type)
.c_str());
}
if ((is_client && !filter_impl->IsSupportedOnClients()) ||
(!is_client && !filter_impl->IsSupportedOnServers())) {
if (is_optional) continue;
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrFormat("Filter %s is not supported on %s", filter_type,
is_client ? "clients" : "servers")
.c_str());
}
absl::StatusOr<XdsHttpFilterImpl::FilterConfig> filter_config =
filter_impl->GenerateFilterConfig(google_protobuf_Any_value(any),
context.arena);
if (!filter_config.ok()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat(
"filter config for type ", filter_type,
" failed to parse: ", filter_config.status().ToString())
.c_str());
}
http_connection_manager->http_filters.emplace_back(
XdsApi::LdsUpdate::HttpConnectionManager::HttpFilter{
std::string(name), std::move(*filter_config)});
}
} else {
// If using a v2 config, we just hard-code a list containing only the
// router filter without actually looking at the config. This ensures
// that the right thing happens in the xds resolver without having
// to expose whether the resource we received was v2 or v3.
http_connection_manager->http_filters.emplace_back(
XdsApi::LdsUpdate::HttpConnectionManager::HttpFilter{
std::string(name), std::move(*filter_config)});
"router", {kXdsHttpRouterFilterConfigName, Json()}});
}
}
if (is_client) {
@ -1792,7 +1809,7 @@ grpc_error* HttpConnectionManagerParse(
grpc_error* LdsResponseParseClient(
const EncodingContext& context,
const envoy_config_listener_v3_ApiListener* api_listener,
const envoy_config_listener_v3_ApiListener* api_listener, bool is_v2,
XdsApi::LdsUpdate* lds_update) {
lds_update->type = XdsApi::LdsUpdate::ListenerType::kHttpApiListener;
const upb_strview encoded_api_listener = google_protobuf_Any_value(
@ -1805,7 +1822,7 @@ grpc_error* LdsResponseParseClient(
"Could not parse HttpConnectionManager config from ApiListener");
}
return HttpConnectionManagerParse(true /* is_client */, context,
http_connection_manager,
http_connection_manager, is_v2,
&lds_update->http_connection_manager);
}
@ -1926,7 +1943,7 @@ grpc_error* DownstreamTlsContextParse(
grpc_error* FilterChainParse(
const EncodingContext& context,
const envoy_config_listener_v3_FilterChain* filter_chain_proto,
const envoy_config_listener_v3_FilterChain* filter_chain_proto, bool is_v2,
XdsApi::LdsUpdate::FilterChain* filter_chain) {
grpc_error* error = GRPC_ERROR_NONE;
auto* filter_chain_match =
@ -1971,7 +1988,7 @@ grpc_error* FilterChainParse(
"typed_config");
}
error = HttpConnectionManagerParse(false /* is_client */, context,
http_connection_manager,
http_connection_manager, is_v2,
&filter_chain->http_connection_manager);
if (error != GRPC_ERROR_NONE) return error;
// Get the DownstreamTlsContext for the filter chain
@ -2013,7 +2030,7 @@ grpc_error* AddressParse(const envoy_config_core_v3_Address* address_proto,
grpc_error* LdsResponseParseServer(
const EncodingContext& context,
const envoy_config_listener_v3_Listener* listener,
const envoy_config_listener_v3_Listener* listener, bool is_v2,
XdsApi::LdsUpdate* lds_update) {
lds_update->type = XdsApi::LdsUpdate::ListenerType::kTcpListener;
grpc_error* error =
@ -2035,7 +2052,7 @@ grpc_error* LdsResponseParseServer(
lds_update->filter_chains.reserve(size);
for (size_t i = 0; i < size; i++) {
XdsApi::LdsUpdate::FilterChain filter_chain;
error = FilterChainParse(context, filter_chains[0], &filter_chain);
error = FilterChainParse(context, filter_chains[0], is_v2, &filter_chain);
if (error != GRPC_ERROR_NONE) return error;
lds_update->filter_chains.push_back(std::move(filter_chain));
}
@ -2043,7 +2060,8 @@ grpc_error* LdsResponseParseServer(
envoy_config_listener_v3_Listener_default_filter_chain(listener);
if (default_filter_chain != nullptr) {
XdsApi::LdsUpdate::FilterChain filter_chain;
error = FilterChainParse(context, default_filter_chain, &filter_chain);
error =
FilterChainParse(context, default_filter_chain, is_v2, &filter_chain);
if (error != GRPC_ERROR_NONE) return error;
lds_update->default_filter_chain = std::move(filter_chain);
}
@ -2068,7 +2086,8 @@ grpc_error* LdsResponseParse(
// Check the type_url of the resource.
absl::string_view type_url =
UpbStringToAbsl(google_protobuf_Any_type_url(resources[i]));
if (!IsLds(type_url)) {
bool is_v2 = false;
if (!IsLds(type_url, &is_v2)) {
errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("resource index ", i, ": Resource is not LDS.")
.c_str()));
@ -2125,9 +2144,9 @@ grpc_error* LdsResponseParse(
}
grpc_error* error = GRPC_ERROR_NONE;
if (api_listener != nullptr) {
error = LdsResponseParseClient(context, api_listener, &lds_update);
error = LdsResponseParseClient(context, api_listener, is_v2, &lds_update);
} else {
error = LdsResponseParseServer(context, listener, &lds_update);
error = LdsResponseParseServer(context, listener, is_v2, &lds_update);
}
if (error != GRPC_ERROR_NONE) {
errors.push_back(grpc_error_add_child(

72
test/cpp/end2end/xds_end2end_test.cc
Unescape View File

@ -1638,10 +1638,12 @@ class XdsEnd2endTest : public ::testing::TestWithParam<TestType> {
// Construct LDS resource.
default_listener_.set_name(kServerName);
HttpConnectionManager http_connection_manager;
auto* filter = http_connection_manager.add_http_filters();
filter->set_name("router");
filter->mutable_typed_config()->PackFrom(
envoy::extensions::filters::http::router::v3::Router());
if (!GetParam().use_v2()) {
auto* filter = http_connection_manager.add_http_filters();
filter->set_name("router");
filter->mutable_typed_config()->PackFrom(
envoy::extensions::filters::http::router::v3::Router());
}
default_listener_.mutable_api_listener()->mutable_api_listener()->PackFrom(
http_connection_manager);
// Construct RDS resource.
@ -3292,6 +3294,30 @@ TEST_P(LdsTest, HttpFiltersEnabled) {
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
// Tests that we ignore filters after the router filter.
TEST_P(LdsTest, IgnoresHttpFiltersAfterRouterFilter) {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION", "true");
SetNextResolutionForLbChannelAllBalancers();
auto listener = default_listener_;
HttpConnectionManager http_connection_manager;
listener.mutable_api_listener()->mutable_api_listener()->UnpackTo(
&http_connection_manager);
auto* filter = http_connection_manager.add_http_filters();
filter->set_name("unknown");
filter->mutable_typed_config()->set_type_url(
"grpc.testing.client_only_http_filter");
listener.mutable_api_listener()->mutable_api_listener()->PackFrom(
http_connection_manager);
SetListenerAndRouteConfiguration(0, listener, default_route_config_);
AdsServiceImpl::EdsResourceArgs args({
{"locality0", GetBackendPorts()},
});
balancers_[0]->ads_service()->SetEdsResource(
BuildEdsResource(args, DefaultEdsServiceName()));
WaitForAllBackends();
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
// Test that we fail RPCs if there is no router filter.
TEST_P(LdsTest, FailRpcsIfNoHttpRouterFilter) {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION", "true");
@ -3323,9 +3349,6 @@ TEST_P(LdsTest, FailRpcsIfNoHttpRouterFilter) {
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
// TODO(lidiz): As part of adding the fault injection filter, add a test
// that we ignore filters after the router filter.
// Test that we NACK empty filter names.
TEST_P(LdsTest, RejectsEmptyHttpFilterName) {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION", "true");
@ -3580,6 +3603,34 @@ TEST_P(LdsTest, IgnoresOptionalHttpFiltersNotSupportedOnClients) {
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
using LdsV2Test = LdsTest;
// Tests that we ignore the HTTP filter list in v2.
// TODO(roth): The test framework is not set up to allow us to test
// the server sending v2 resources when the client requests v3, so this
// just tests a pure v2 setup. When we have time, fix this.
TEST_P(LdsV2Test, IgnoresHttpFilters) {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION", "true");
auto listener = default_listener_;
HttpConnectionManager http_connection_manager;
listener.mutable_api_listener()->mutable_api_listener()->UnpackTo(
&http_connection_manager);
auto* filter = http_connection_manager.add_http_filters();
filter->set_name("unknown");
filter->mutable_typed_config()->PackFrom(Listener());
listener.mutable_api_listener()->mutable_api_listener()->PackFrom(
http_connection_manager);
SetListenerAndRouteConfiguration(0, listener, default_route_config_);
AdsServiceImpl::EdsResourceArgs args({
{"locality0", GetBackendPorts(0, 1)},
});
balancers_[0]->ads_service()->SetEdsResource(
BuildEdsResource(args, DefaultEdsServiceName()));
SetNextResolutionForLbChannelAllBalancers();
CheckRpcSendOk();
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
using LdsRdsTest = BasicTest;
// Tests that LDS client should send an ACK upon correct LDS response (with
@ -6338,10 +6389,6 @@ TEST_P(LdsRdsTest, RejectsUnparseableHttpFilterTypeInClusterWeight) {
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_FAULT_INJECTION");
}
// TODO(lidiz): As part of adding the fault injection filter, add tests
// for overriding filter configs in the typed_per_filter_config fields in
// each of VirtualHost, Route, and ClusterWeight.
using CdsTest = BasicTest;
// Tests that CDS client should send an ACK upon correct CDS response.
@ -9963,6 +10010,9 @@ INSTANTIATE_TEST_SUITE_P(XdsTest, SecureNamingTest,
// LDS depends on XdsResolver.
INSTANTIATE_TEST_SUITE_P(XdsTest, LdsTest, ::testing::Values(TestType()),
&TestTypeName);
INSTANTIATE_TEST_SUITE_P(XdsTest, LdsV2Test,
::testing::Values(TestType().set_use_v2()),
&TestTypeName);
// LDS/RDS commmon tests depend on XdsResolver.
INSTANTIATE_TEST_SUITE_P(

Write Preview
Loading…
Cancel
Save