diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a0c582e245..9661e4592eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20654,6 +20654,10 @@ endif() if(gRPC_BUILD_TESTS) add_executable(xds_common_types_test + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/reflection/v1alpha/reflection.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/reflection/v1alpha/reflection.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/reflection/v1alpha/reflection.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/reflection/v1alpha/reflection.grpc.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/base.pb.h @@ -20678,7 +20682,20 @@ add_executable(xds_common_types_test ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/tls.grpc.pb.cc ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/tls.pb.h ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/tls.grpc.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/typed_struct.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/typed_struct.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/typed_struct.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/typed_struct.grpc.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/udpa_typed_struct.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/udpa_typed_struct.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/udpa_typed_struct.pb.h + ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/udpa_typed_struct.grpc.pb.h test/core/xds/xds_common_types_test.cc + test/cpp/util/cli_call.cc + test/cpp/util/cli_credentials.cc + test/cpp/util/proto_file_parser.cc + test/cpp/util/proto_reflection_descriptor_database.cc + test/cpp/util/service_describer.cc third_party/googletest/googletest/src/gtest-all.cc third_party/googletest/googlemock/src/gmock-all.cc ) @@ -20705,6 +20722,8 @@ target_include_directories(xds_common_types_test target_link_libraries(xds_common_types_test ${_gRPC_PROTOBUF_LIBRARIES} ${_gRPC_ALLTARGETS_LIBRARIES} + absl::flags + grpc++ grpc_test_util ) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 9317e873831..cef6b0d4bd1 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -11089,16 +11089,32 @@ targets: gtest: true build: test language: c++ - headers: [] + headers: + - test/cpp/util/cli_call.h + - test/cpp/util/cli_credentials.h + - test/cpp/util/config_grpc_cli.h + - test/cpp/util/proto_file_parser.h + - test/cpp/util/proto_reflection_descriptor_database.h + - test/cpp/util/service_describer.h src: + - src/proto/grpc/reflection/v1alpha/reflection.proto - src/proto/grpc/testing/xds/v3/base.proto - src/proto/grpc/testing/xds/v3/extension.proto - src/proto/grpc/testing/xds/v3/percent.proto - src/proto/grpc/testing/xds/v3/regex.proto - src/proto/grpc/testing/xds/v3/string.proto - src/proto/grpc/testing/xds/v3/tls.proto + - src/proto/grpc/testing/xds/v3/typed_struct.proto + - src/proto/grpc/testing/xds/v3/udpa_typed_struct.proto - test/core/xds/xds_common_types_test.cc + - test/cpp/util/cli_call.cc + - test/cpp/util/cli_credentials.cc + - test/cpp/util/proto_file_parser.cc + - test/cpp/util/proto_reflection_descriptor_database.cc + - test/cpp/util/service_describer.cc deps: + - absl/flags:flag + - grpc++ - grpc_test_util uses_polling: false - name: xds_core_end2end_test diff --git a/src/core/ext/xds/xds_cluster_specifier_plugin.cc b/src/core/ext/xds/xds_cluster_specifier_plugin.cc index 3fbf773adff..769097a9e69 100644 --- a/src/core/ext/xds/xds_cluster_specifier_plugin.cc +++ b/src/core/ext/xds/xds_cluster_specifier_plugin.cc @@ -26,6 +26,7 @@ #include "absl/status/status.h" #include "absl/strings/str_cat.h" +#include "absl/types/variant.h" #include "upb/json_encode.h" #include "upb/status.h" #include "upb/upb.hpp" @@ -50,10 +51,15 @@ void XdsRouteLookupClusterSpecifierPlugin::PopulateSymtab( absl::StatusOr XdsRouteLookupClusterSpecifierPlugin::GenerateLoadBalancingPolicyConfig( - upb_StringView serialized_plugin_config, upb_Arena* arena, - upb_DefPool* symtab) const { + XdsExtension extension, upb_Arena* arena, upb_DefPool* symtab) const { + absl::string_view* serialized_plugin_config = + absl::get_if(&extension.value); + if (serialized_plugin_config == nullptr) { + return absl::InvalidArgumentError("could not parse plugin config"); + } const auto* specifier = grpc_lookup_v1_RouteLookupClusterSpecifier_parse( - serialized_plugin_config.data, serialized_plugin_config.size, arena); + serialized_plugin_config->data(), serialized_plugin_config->size(), + arena); if (specifier == nullptr) { return absl::InvalidArgumentError("Could not parse plugin config"); } diff --git a/src/core/ext/xds/xds_cluster_specifier_plugin.h b/src/core/ext/xds/xds_cluster_specifier_plugin.h index 068ef71cff0..2d6d6605675 100644 --- a/src/core/ext/xds/xds_cluster_specifier_plugin.h +++ b/src/core/ext/xds/xds_cluster_specifier_plugin.h @@ -26,7 +26,8 @@ #include "absl/strings/string_view.h" #include "upb/arena.h" #include "upb/def.h" -#include "upb/upb.h" + +#include "src/core/ext/xds/xds_common_types.h" namespace grpc_core { @@ -39,8 +40,7 @@ class XdsClusterSpecifierPluginImpl { // Returns the LB policy config in JSON form. virtual absl::StatusOr GenerateLoadBalancingPolicyConfig( - upb_StringView serialized_plugin_config, upb_Arena* arena, - upb_DefPool* symtab) const = 0; + XdsExtension extension, upb_Arena* arena, upb_DefPool* symtab) const = 0; }; class XdsRouteLookupClusterSpecifierPlugin @@ -48,7 +48,7 @@ class XdsRouteLookupClusterSpecifierPlugin void PopulateSymtab(upb_DefPool* symtab) const override; absl::StatusOr GenerateLoadBalancingPolicyConfig( - upb_StringView serialized_plugin_config, upb_Arena* arena, + XdsExtension extension, upb_Arena* arena, upb_DefPool* symtab) const override; }; diff --git a/src/core/ext/xds/xds_common_types.cc b/src/core/ext/xds/xds_common_types.cc index 0685889b800..0af88958858 100644 --- a/src/core/ext/xds/xds_common_types.cc +++ b/src/core/ext/xds/xds_common_types.cc @@ -35,8 +35,13 @@ #include "envoy/type/matcher/v3/regex.upb.h" #include "envoy/type/matcher/v3/string.upb.h" #include "google/protobuf/any.upb.h" +#include "google/protobuf/struct.upb.h" +#include "google/protobuf/struct.upbdefs.h" #include "google/protobuf/wrappers.upb.h" -#include "upb/upb.h" +#include "upb/arena.h" +#include "upb/json_encode.h" +#include "upb/status.h" +#include "upb/upb.hpp" #include "xds/type/v3/typed_struct.upb.h" #include "src/core/ext/xds/certificate_provider_store.h" @@ -403,30 +408,94 @@ CommonTlsContext CommonTlsContext::Parse( return common_tls_context; } -absl::StatusOr ExtractExtensionTypeName( +// +// ExtractXdsExtension +// + +namespace { + +absl::StatusOr ParseProtobufStructToJson( const XdsResourceType::DecodeContext& context, - const google_protobuf_Any* any) { - ExtractExtensionTypeNameResult result; - result.type = UpbStringToAbsl(google_protobuf_Any_type_url(any)); - if (result.type == "type.googleapis.com/xds.type.v3.TypedStruct" || - result.type == "type.googleapis.com/udpa.type.v1.TypedStruct") { - upb_StringView any_value = google_protobuf_Any_value(any); - result.typed_struct = xds_type_v3_TypedStruct_parse( - any_value.data, any_value.size, context.arena); - if (result.typed_struct == nullptr) { - return absl::InvalidArgumentError( - "could not parse TypedStruct from extension"); - } - result.type = - UpbStringToAbsl(xds_type_v3_TypedStruct_type_url(result.typed_struct)); - } - size_t pos = result.type.rfind('/'); - if (pos == absl::string_view::npos || pos == result.type.size() - 1) { + const google_protobuf_Struct* resource) { + upb::Status status; + const auto* msg_def = google_protobuf_Struct_getmsgdef(context.symtab); + size_t json_size = upb_JsonEncode(resource, msg_def, context.symtab, 0, + nullptr, 0, status.ptr()); + if (json_size == static_cast(-1)) { return absl::InvalidArgumentError( - absl::StrCat("Invalid type_url ", result.type)); + absl::StrCat("error encoding google::Protobuf::Struct as JSON: ", + upb_Status_ErrorMessage(status.ptr()))); + } + void* buf = upb_Arena_Malloc(context.arena, json_size + 1); + upb_JsonEncode(resource, msg_def, context.symtab, 0, + reinterpret_cast(buf), json_size + 1, status.ptr()); + auto json = Json::Parse(reinterpret_cast(buf)); + if (!json.ok()) { + // This should never happen. + return absl::InternalError( + absl::StrCat("error parsing JSON form of google::Protobuf::Struct " + "produced by upb library: ", + json.status().ToString())); + } + return std::move(*json); +} + +} // namespace + +absl::optional ExtractXdsExtension( + const XdsResourceType::DecodeContext& context, + const google_protobuf_Any* any, ValidationErrors* errors) { + if (any == nullptr) { + errors->AddError("field not present"); + return absl::nullopt; + } + XdsExtension extension; + auto strip_type_prefix = [&]() { + ValidationErrors::ScopedField field(errors, ".type_url"); + if (extension.type.empty()) { + errors->AddError("field not present"); + return; + } + size_t pos = extension.type.rfind('/'); + if (pos == absl::string_view::npos || pos == extension.type.size() - 1) { + errors->AddError(absl::StrCat("invalid value \"", extension.type, "\"")); + } else { + extension.type = extension.type.substr(pos + 1); + } + }; + extension.type = UpbStringToAbsl(google_protobuf_Any_type_url(any)); + strip_type_prefix(); + extension.validation_fields.emplace_back( + errors, absl::StrCat(".value[", extension.type, "]")); + absl::string_view any_value = UpbStringToAbsl(google_protobuf_Any_value(any)); + if (extension.type == "xds.type.v3.TypedStruct" || + extension.type == "udpa.type.v1.TypedStruct") { + const auto* typed_struct = xds_type_v3_TypedStruct_parse( + any_value.data(), any_value.size(), context.arena); + if (typed_struct == nullptr) { + errors->AddError("could not parse"); + return absl::nullopt; + } + extension.type = + UpbStringToAbsl(xds_type_v3_TypedStruct_type_url(typed_struct)); + strip_type_prefix(); + extension.validation_fields.emplace_back( + errors, absl::StrCat(".value[", extension.type, "]")); + auto* protobuf_struct = xds_type_v3_TypedStruct_value(typed_struct); + if (protobuf_struct == nullptr) { + extension.value = Json::Object(); // Default to empty object. + } else { + auto json = ParseProtobufStructToJson(context, protobuf_struct); + if (!json.ok()) { + errors->AddError(json.status().message()); + return absl::nullopt; + } + extension.value = std::move(*json); + } + } else { + extension.value = any_value; } - result.type = result.type.substr(pos + 1); - return result; + return std::move(extension); } } // namespace grpc_core diff --git a/src/core/ext/xds/xds_common_types.h b/src/core/ext/xds/xds_common_types.h index 79029b9cad1..c90001d1d8d 100644 --- a/src/core/ext/xds/xds_common_types.h +++ b/src/core/ext/xds/xds_common_types.h @@ -22,16 +22,17 @@ #include #include -#include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" +#include "absl/types/variant.h" #include "envoy/extensions/transport_sockets/tls/v3/tls.upb.h" #include "google/protobuf/any.upb.h" #include "google/protobuf/duration.upb.h" -#include "xds/type/v3/typed_struct.upb.h" #include "src/core/ext/xds/xds_resource_type.h" #include "src/core/lib/gprpp/time.h" #include "src/core/lib/gprpp/validation_errors.h" +#include "src/core/lib/json/json.h" #include "src/core/lib/matchers/matchers.h" namespace grpc_core { @@ -87,14 +88,20 @@ struct CommonTlsContext { ValidationErrors* errors); }; -struct ExtractExtensionTypeNameResult { +struct XdsExtension { + // The type, either from the top level or from inside the TypedStruct. absl::string_view type; - xds_type_v3_TypedStruct* typed_struct = nullptr; + // A Json object for a TypedStruct, or the serialized config otherwise. + absl::variant + value; + // Validation fields that need to stay in scope until we're done + // processing the extension. + std::vector validation_fields; }; -absl::StatusOr ExtractExtensionTypeName( +absl::optional ExtractXdsExtension( const XdsResourceType::DecodeContext& context, - const google_protobuf_Any* any); + const google_protobuf_Any* any, ValidationErrors* errors); } // namespace grpc_core diff --git a/src/core/ext/xds/xds_http_fault_filter.cc b/src/core/ext/xds/xds_http_fault_filter.cc index 164dec91c44..37ea234b2ce 100644 --- a/src/core/ext/xds/xds_http_fault_filter.cc +++ b/src/core/ext/xds/xds_http_fault_filter.cc @@ -28,6 +28,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "absl/types/variant.h" #include "envoy/extensions/filters/common/fault/v3/fault.upb.h" #include "envoy/extensions/filters/http/fault/v3/fault.upb.h" #include "envoy/extensions/filters/http/fault/v3/fault.upbdefs.h" @@ -74,9 +75,9 @@ uint32_t GetDenominator(const envoy_type_v3_FractionalPercent* fraction) { } absl::StatusOr ParseHttpFaultIntoJson( - upb_StringView serialized_http_fault, upb_Arena* arena) { + absl::string_view serialized_http_fault, upb_Arena* arena) { auto* http_fault = envoy_extensions_filters_http_fault_v3_HTTPFault_parse( - serialized_http_fault.data, serialized_http_fault.size, arena); + serialized_http_fault.data(), serialized_http_fault.size(), arena); if (http_fault == nullptr) { return absl::InvalidArgumentError( "could not parse fault injection filter config"); @@ -184,10 +185,16 @@ void XdsHttpFaultFilter::PopulateSymtab(upb_DefPool* symtab) const { } absl::StatusOr -XdsHttpFaultFilter::GenerateFilterConfig( - upb_StringView serialized_filter_config, upb_Arena* arena) const { +XdsHttpFaultFilter::GenerateFilterConfig(XdsExtension extension, + upb_Arena* arena) const { + absl::string_view* serialized_filter_config = + absl::get_if(&extension.value); + if (serialized_filter_config == nullptr) { + return absl::InvalidArgumentError( + "could not parse fault injection filter config"); + } absl::StatusOr parse_result = - ParseHttpFaultIntoJson(serialized_filter_config, arena); + ParseHttpFaultIntoJson(*serialized_filter_config, arena); if (!parse_result.ok()) { return parse_result.status(); } @@ -195,11 +202,11 @@ XdsHttpFaultFilter::GenerateFilterConfig( } absl::StatusOr -XdsHttpFaultFilter::GenerateFilterConfigOverride( - upb_StringView serialized_filter_config, upb_Arena* arena) const { +XdsHttpFaultFilter::GenerateFilterConfigOverride(XdsExtension extension, + upb_Arena* arena) const { // HTTPFault filter has the same message type in HTTP connection manager's // filter config and in overriding filter config field. - return GenerateFilterConfig(serialized_filter_config, arena); + return GenerateFilterConfig(std::move(extension), arena); } const grpc_channel_filter* XdsHttpFaultFilter::channel_filter() const { diff --git a/src/core/ext/xds/xds_http_fault_filter.h b/src/core/ext/xds/xds_http_fault_filter.h index 59ba533cc82..f075f6ad8c3 100644 --- a/src/core/ext/xds/xds_http_fault_filter.h +++ b/src/core/ext/xds/xds_http_fault_filter.h @@ -22,8 +22,8 @@ #include "absl/status/statusor.h" #include "upb/arena.h" #include "upb/def.h" -#include "upb/upb.h" +#include "src/core/ext/xds/xds_common_types.h" #include "src/core/ext/xds/xds_http_filters.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" @@ -39,11 +39,11 @@ class XdsHttpFaultFilter : public XdsHttpFilterImpl { // Overrides the GenerateFilterConfig method absl::StatusOr GenerateFilterConfig( - upb_StringView serialized_filter_config, upb_Arena* arena) const override; + XdsExtension extension, upb_Arena* arena) const override; // Overrides the GenerateFilterConfigOverride method absl::StatusOr GenerateFilterConfigOverride( - upb_StringView serialized_filter_config, upb_Arena* arena) const override; + XdsExtension extension, upb_Arena* arena) const override; // Overrides the channel_filter method const grpc_channel_filter* channel_filter() const override; diff --git a/src/core/ext/xds/xds_http_filters.cc b/src/core/ext/xds/xds_http_filters.cc index 3528a827c64..a700dd597b6 100644 --- a/src/core/ext/xds/xds_http_filters.cc +++ b/src/core/ext/xds/xds_http_filters.cc @@ -24,6 +24,7 @@ #include #include "absl/status/status.h" +#include "absl/types/variant.h" #include "envoy/extensions/filters/http/router/v3/router.upb.h" #include "envoy/extensions/filters/http/router/v3/router.upbdefs.h" @@ -44,10 +45,14 @@ class XdsHttpRouterFilter : public XdsHttpFilterImpl { } absl::StatusOr GenerateFilterConfig( - upb_StringView serialized_filter_config, - upb_Arena* arena) const override { + XdsExtension extension, upb_Arena* arena) const override { + absl::string_view* serialized_filter_config = + absl::get_if(&extension.value); + if (serialized_filter_config == nullptr) { + return absl::InvalidArgumentError("could not parse router filter config"); + } if (envoy_extensions_filters_http_router_v3_Router_parse( - serialized_filter_config.data, serialized_filter_config.size, + serialized_filter_config->data(), serialized_filter_config->size(), arena) == nullptr) { return absl::InvalidArgumentError("could not parse router filter config"); } @@ -55,8 +60,7 @@ class XdsHttpRouterFilter : public XdsHttpFilterImpl { } absl::StatusOr GenerateFilterConfigOverride( - upb_StringView /*serialized_filter_config*/, - upb_Arena* /*arena*/) const override { + XdsExtension /*extension*/, upb_Arena* /*arena*/) const override { return absl::InvalidArgumentError( "router filter does not support config override"); } diff --git a/src/core/ext/xds/xds_http_filters.h b/src/core/ext/xds/xds_http_filters.h index c231e3f1e5a..3f5dd4724af 100644 --- a/src/core/ext/xds/xds_http_filters.h +++ b/src/core/ext/xds/xds_http_filters.h @@ -28,8 +28,8 @@ #include "absl/strings/string_view.h" #include "upb/arena.h" #include "upb/def.h" -#include "upb/upb.h" +#include "src/core/ext/xds/xds_common_types.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" #include "src/core/lib/json/json.h" @@ -75,12 +75,12 @@ class XdsHttpFilterImpl { // Generates a Config from the xDS filter config proto. // Used for the top-level config in the HCM HTTP filter list. virtual absl::StatusOr GenerateFilterConfig( - upb_StringView serialized_filter_config, upb_Arena* arena) const = 0; + XdsExtension extension, upb_Arena* arena) const = 0; // Generates a Config from the xDS filter config proto. // Used for the typed_per_filter_config override in VirtualHost and Route. virtual absl::StatusOr GenerateFilterConfigOverride( - upb_StringView serialized_filter_config, upb_Arena* arena) const = 0; + XdsExtension extension, upb_Arena* arena) const = 0; // C-core channel filter implementation. virtual const grpc_channel_filter* channel_filter() const = 0; diff --git a/src/core/ext/xds/xds_http_rbac_filter.cc b/src/core/ext/xds/xds_http_rbac_filter.cc index 2da65813547..6f2d722357c 100644 --- a/src/core/ext/xds/xds_http_rbac_filter.cc +++ b/src/core/ext/xds/xds_http_rbac_filter.cc @@ -32,6 +32,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" +#include "absl/types/variant.h" #include "envoy/config/core/v3/address.upb.h" #include "envoy/config/rbac/v3/rbac.upb.h" #include "envoy/config/route/v3/route_components.upb.h" @@ -43,6 +44,7 @@ #include "envoy/type/matcher/v3/string.upb.h" #include "envoy/type/v3/range.upb.h" #include "google/protobuf/wrappers.upb.h" +#include "upb/upb.h" #include "src/core/ext/filters/rbac/rbac_filter.h" #include "src/core/ext/filters/rbac/rbac_service_config_parser.h" @@ -497,16 +499,22 @@ void XdsHttpRbacFilter::PopulateSymtab(upb_DefPool* symtab) const { } absl::StatusOr -XdsHttpRbacFilter::GenerateFilterConfig(upb_StringView serialized_filter_config, +XdsHttpRbacFilter::GenerateFilterConfig(XdsExtension extension, upb_Arena* arena) const { - absl::StatusOr rbac_json; + absl::string_view* serialized_filter_config = + absl::get_if(&extension.value); + if (serialized_filter_config == nullptr) { + return absl::InvalidArgumentError( + "could not parse HTTP RBAC filter config"); + } auto* rbac = envoy_extensions_filters_http_rbac_v3_RBAC_parse( - serialized_filter_config.data, serialized_filter_config.size, arena); + serialized_filter_config->data(), serialized_filter_config->size(), + arena); if (rbac == nullptr) { return absl::InvalidArgumentError( "could not parse HTTP RBAC filter config"); } - rbac_json = ParseHttpRbacToJson(rbac); + absl::StatusOr rbac_json = ParseHttpRbacToJson(rbac); if (!rbac_json.ok()) { return rbac_json.status(); } @@ -514,11 +522,17 @@ XdsHttpRbacFilter::GenerateFilterConfig(upb_StringView serialized_filter_config, } absl::StatusOr -XdsHttpRbacFilter::GenerateFilterConfigOverride( - upb_StringView serialized_filter_config, upb_Arena* arena) const { +XdsHttpRbacFilter::GenerateFilterConfigOverride(XdsExtension extension, + upb_Arena* arena) const { + absl::string_view* serialized_filter_config = + absl::get_if(&extension.value); + if (serialized_filter_config == nullptr) { + return absl::InvalidArgumentError("could not parse RBACPerRoute"); + } auto* rbac_per_route = envoy_extensions_filters_http_rbac_v3_RBACPerRoute_parse( - serialized_filter_config.data, serialized_filter_config.size, arena); + serialized_filter_config->data(), serialized_filter_config->size(), + arena); if (rbac_per_route == nullptr) { return absl::InvalidArgumentError("could not parse RBACPerRoute"); } diff --git a/src/core/ext/xds/xds_http_rbac_filter.h b/src/core/ext/xds/xds_http_rbac_filter.h index ec8857b21df..89602a3c61b 100644 --- a/src/core/ext/xds/xds_http_rbac_filter.h +++ b/src/core/ext/xds/xds_http_rbac_filter.h @@ -22,8 +22,8 @@ #include "absl/status/statusor.h" #include "upb/arena.h" #include "upb/def.h" -#include "upb/upb.h" +#include "src/core/ext/xds/xds_common_types.h" #include "src/core/ext/xds/xds_http_filters.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/channel_fwd.h" @@ -38,10 +38,10 @@ class XdsHttpRbacFilter : public XdsHttpFilterImpl { void PopulateSymtab(upb_DefPool* symtab) const override; absl::StatusOr GenerateFilterConfig( - upb_StringView serialized_filter_config, upb_Arena* arena) const override; + XdsExtension extension, upb_Arena* arena) const override; absl::StatusOr GenerateFilterConfigOverride( - upb_StringView serialized_filter_config, upb_Arena* arena) const override; + XdsExtension extension, upb_Arena* arena) const override; const grpc_channel_filter* channel_filter() const override; diff --git a/src/core/ext/xds/xds_lb_policy_registry.cc b/src/core/ext/xds/xds_lb_policy_registry.cc index ac91fa63ea8..3f0fb25e150 100644 --- a/src/core/ext/xds/xds_lb_policy_registry.cc +++ b/src/core/ext/xds/xds_lb_policy_registry.cc @@ -27,22 +27,20 @@ #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" +#include "absl/types/optional.h" +#include "absl/types/variant.h" #include "envoy/config/core/v3/extension.upb.h" #include "envoy/extensions/load_balancing_policies/ring_hash/v3/ring_hash.upb.h" #include "envoy/extensions/load_balancing_policies/wrr_locality/v3/wrr_locality.upb.h" #include "google/protobuf/any.upb.h" -#include "google/protobuf/struct.upb.h" -#include "google/protobuf/struct.upbdefs.h" #include "google/protobuf/wrappers.upb.h" -#include "upb/arena.h" -#include "upb/json_encode.h" -#include "upb/status.h" -#include "upb/upb.hpp" -#include "xds/type/v3/typed_struct.upb.h" + +#include #include "src/core/ext/xds/upb_utils.h" #include "src/core/ext/xds/xds_common_types.h" #include "src/core/lib/config/core_configuration.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/load_balancing/lb_policy_registry.h" namespace grpc_core { @@ -151,32 +149,6 @@ class WrrLocalityLbPolicyConfigFactory } }; -absl::StatusOr ParseStructToJson( - const XdsResourceType::DecodeContext& context, - const google_protobuf_Struct* resource) { - upb::Status status; - const auto* msg_def = google_protobuf_Struct_getmsgdef(context.symtab); - size_t json_size = upb_JsonEncode(resource, msg_def, context.symtab, 0, - nullptr, 0, status.ptr()); - if (json_size == static_cast(-1)) { - return absl::InvalidArgumentError( - absl::StrCat("Error parsing google::Protobuf::Struct: ", - upb_Status_ErrorMessage(status.ptr()))); - } - void* buf = upb_Arena_Malloc(context.arena, json_size + 1); - upb_JsonEncode(resource, msg_def, context.symtab, 0, - reinterpret_cast(buf), json_size + 1, status.ptr()); - auto json = Json::Parse(reinterpret_cast(buf)); - if (!json.ok()) { - // This should not happen - return absl::InternalError( - absl::StrCat("Error parsing JSON form of google::Protobuf::Struct " - "produced by upb library: ", - json.status().ToString())); - } - return std::move(*json); -} - } // namespace // @@ -215,16 +187,18 @@ absl::StatusOr XdsLbPolicyRegistry::ConvertXdsLbPolicyConfig( "Error parsing LoadBalancingPolicy::Policy - Missing " "TypedExtensionConfig::typed_config field"); } - auto type = ExtractExtensionTypeName(context, typed_config); - if (!type.ok()) { - return absl::InvalidArgumentError(absl::StrCat( + ValidationErrors errors; + auto extension = ExtractXdsExtension(context, typed_config, &errors); + if (!errors.ok()) { + return errors.status( "Error parsing " - "LoadBalancingPolicy::Policy::TypedExtensionConfig::typed_config: ", - type.status().message())); + "LoadBalancingPolicy::Policy::TypedExtensionConfig::typed_config"); } + GPR_ASSERT(extension.has_value()); absl::string_view value = UpbStringToAbsl(google_protobuf_Any_value(typed_config)); - auto config_factory_it = Get()->policy_config_factories_.find(type->type); + auto config_factory_it = + Get()->policy_config_factories_.find(extension->type); if (config_factory_it != Get()->policy_config_factories_.end()) { policy = config_factory_it->second->ConvertXdsLbPolicyConfig( context, value, recursion_depth); @@ -235,29 +209,16 @@ absl::StatusOr XdsLbPolicyRegistry::ConvertXdsLbPolicyConfig( "typed_config to JSON: ", policy.status().message())); } - } else if (type->typed_struct != nullptr) { + } else if (absl::holds_alternative(extension->value)) { // Custom lb policy config - std::string custom_type = std::string(type->type); if (!CoreConfiguration::Get() .lb_policy_registry() - .LoadBalancingPolicyExists(custom_type.c_str(), nullptr)) { + .LoadBalancingPolicyExists(extension->type, nullptr)) { // Skip unsupported custom lb policy. continue; } - // Convert typed struct to json. - auto value = xds_type_v3_TypedStruct_value(type->typed_struct); - if (value == nullptr) { - policy = Json::Object{{std::move(custom_type), Json() /* null */}}; - } else { - auto parsed_value = ParseStructToJson(context, value); - if (!parsed_value.ok()) { - return absl::InvalidArgumentError(absl::StrCat( - "Error parsing LoadBalancingPolicy: Custom Policy: ", custom_type, - ": ", parsed_value.status().message())); - } - policy = - Json::Object{{std::move(custom_type), *(std::move(parsed_value))}}; - } + Json* json = absl::get_if(&extension->value); + policy = Json::Object{{std::string(extension->type), std::move(*json)}}; } else { // Unsupported type. Skipping entry. continue; diff --git a/src/core/ext/xds/xds_listener.cc b/src/core/ext/xds/xds_listener.cc index 2c199920844..dbdcbe7cfea 100644 --- a/src/core/ext/xds/xds_listener.cc +++ b/src/core/ext/xds/xds_listener.cc @@ -358,18 +358,24 @@ HttpConnectionManagerParse( } continue; } - auto filter_type = ExtractExtensionTypeName(context, any); - if (!filter_type.ok()) { - errors.emplace_back(absl::StrCat("filter name ", name, ": ", - filter_type.status().message())); + ValidationErrors validation_errors; + ValidationErrors::ScopedField field( + &validation_errors, + absl::StrCat(".http_filters[", i, "].typed_config")); + auto extension = ExtractXdsExtension(context, any, &validation_errors); + if (!validation_errors.ok()) { + errors.emplace_back( + validation_errors.status(absl::StrCat("filter name ", name)) + .message()); continue; } + GPR_ASSERT(extension.has_value()); const XdsHttpFilterImpl* filter_impl = - XdsHttpFilterRegistry::GetFilterForType(filter_type->type); + XdsHttpFilterRegistry::GetFilterForType(extension->type); if (filter_impl == nullptr) { if (!is_optional) { errors.emplace_back(absl::StrCat( - "no filter registered for config type ", filter_type->type)); + "no filter registered for config type ", extension->type)); } continue; } @@ -377,17 +383,17 @@ HttpConnectionManagerParse( (!is_client && !filter_impl->IsSupportedOnServers())) { if (!is_optional) { errors.emplace_back(absl::StrFormat( - "Filter %s is not supported on %s", filter_type->type, + "Filter %s is not supported on %s", extension->type, is_client ? "clients" : "servers")); } continue; } absl::StatusOr filter_config = - filter_impl->GenerateFilterConfig(google_protobuf_Any_value(any), + filter_impl->GenerateFilterConfig(std::move(*extension), context.arena); if (!filter_config.ok()) { errors.emplace_back(absl::StrCat( - "filter config for type ", filter_type->type, + "filter config for type ", extension->type, " failed to parse: ", StatusToString(filter_config.status()))); continue; } diff --git a/src/core/ext/xds/xds_route_config.cc b/src/core/ext/xds/xds_route_config.cc index 6c18584548d..0db5fc8cace 100644 --- a/src/core/ext/xds/xds_route_config.cc +++ b/src/core/ext/xds/xds_route_config.cc @@ -349,39 +349,47 @@ ClusterSpecifierPluginParse( envoy_config_route_v3_RouteConfiguration_cluster_specifier_plugins( route_config, &num_cluster_specifier_plugins); for (size_t i = 0; i < num_cluster_specifier_plugins; ++i) { - const envoy_config_core_v3_TypedExtensionConfig* extension = + const envoy_config_core_v3_TypedExtensionConfig* typed_extension_config = envoy_config_route_v3_ClusterSpecifierPlugin_extension( cluster_specifier_plugin[i]); std::string name = UpbStringToStdString( - envoy_config_core_v3_TypedExtensionConfig_name(extension)); + envoy_config_core_v3_TypedExtensionConfig_name(typed_extension_config)); if (cluster_specifier_plugin_map.find(name) != cluster_specifier_plugin_map.end()) { return absl::InvalidArgumentError(absl::StrCat( "Duplicated definition of cluster_specifier_plugin ", name)); } const google_protobuf_Any* any = - envoy_config_core_v3_TypedExtensionConfig_typed_config(extension); + envoy_config_core_v3_TypedExtensionConfig_typed_config( + typed_extension_config); if (any == nullptr) { return absl::InvalidArgumentError( "Could not obtrain TypedExtensionConfig for plugin config."); } - auto plugin_type = ExtractExtensionTypeName(context, any); - if (!plugin_type.ok()) return plugin_type.status(); + ValidationErrors validation_errors; + ValidationErrors::ScopedField field( + &validation_errors, absl::StrCat(".cluster_specifier_plugins[", i, + "].extension.typed_config")); + auto extension = ExtractXdsExtension(context, any, &validation_errors); + if (!validation_errors.ok()) { + return validation_errors.status("could not determine extension type"); + } + GPR_ASSERT(extension.has_value()); bool is_optional = envoy_config_route_v3_ClusterSpecifierPlugin_is_optional( cluster_specifier_plugin[i]); const XdsClusterSpecifierPluginImpl* cluster_specifier_plugin_impl = - XdsClusterSpecifierPluginRegistry::GetPluginForType(plugin_type->type); + XdsClusterSpecifierPluginRegistry::GetPluginForType(extension->type); std::string lb_policy_config; if (cluster_specifier_plugin_impl == nullptr) { if (!is_optional) { return absl::InvalidArgumentError(absl::StrCat( - "Unknown ClusterSpecifierPlugin type ", plugin_type->type)); + "Unknown ClusterSpecifierPlugin type ", extension->type)); } // Optional plugin, leave lb_policy_config empty. } else { auto config = cluster_specifier_plugin_impl->GenerateLoadBalancingPolicyConfig( - google_protobuf_Any_value(any), context.arena, context.symtab); + std::move(*extension), context.arena, context.symtab); if (!config.ok()) return config.status(); lb_policy_config = std::move(*config); } @@ -625,21 +633,27 @@ ParseTypedPerFilterConfig( absl::StrCat("no filter config specified for filter name ", key)); } } - auto type = ExtractExtensionTypeName(context, any); - if (!type.ok()) return type.status(); + ValidationErrors errors; + ValidationErrors::ScopedField field( + &errors, absl::StrCat(".typed_per_filter_config[", key, "]")); + auto extension = ExtractXdsExtension(context, any, &errors); + if (!errors.ok()) { + return errors.status("could not determine extension type"); + } + GPR_ASSERT(extension.has_value()); const XdsHttpFilterImpl* filter_impl = - XdsHttpFilterRegistry::GetFilterForType(type->type); + XdsHttpFilterRegistry::GetFilterForType(extension->type); if (filter_impl == nullptr) { if (is_optional) continue; - return absl::InvalidArgumentError( - absl::StrCat("no filter registered for config type ", type->type)); + return absl::InvalidArgumentError(absl::StrCat( + "no filter registered for config type ", extension->type)); } absl::StatusOr filter_config = - filter_impl->GenerateFilterConfigOverride( - google_protobuf_Any_value(any), context.arena); + filter_impl->GenerateFilterConfigOverride(std::move(*extension), + context.arena); if (!filter_config.ok()) { return absl::InvalidArgumentError( - absl::StrCat("filter config for type ", type->type, + absl::StrCat("filter config for type ", extension->type, " failed to parse: ", filter_config.status().message())); } typed_per_filter_config[std::string(key)] = std::move(*filter_config); diff --git a/src/core/lib/gprpp/validation_errors.h b/src/core/lib/gprpp/validation_errors.h index e100b95b8e0..c9783b6003c 100644 --- a/src/core/lib/gprpp/validation_errors.h +++ b/src/core/lib/gprpp/validation_errors.h @@ -21,6 +21,7 @@ #include #include +#include #include #include "absl/status/status.h" @@ -69,7 +70,23 @@ class ValidationErrors { : errors_(errors) { errors_->PushField(field_name); } - ~ScopedField() { errors_->PopField(); } + + // Not copyable. + ScopedField(const ScopedField& other) = delete; + ScopedField& operator=(const ScopedField& other) = delete; + + // Movable. + ScopedField(ScopedField&& other) noexcept + : errors_(std::exchange(other.errors_, nullptr)) {} + ScopedField& operator=(ScopedField&& other) noexcept { + if (errors_ != nullptr) errors_->PopField(); + errors_ = std::exchange(other.errors_, nullptr); + return *this; + } + + ~ScopedField() { + if (errors_ != nullptr) errors_->PopField(); + } private: ValidationErrors* errors_; diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index b73cdcac2ff..0581ade8c8e 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -167,7 +167,10 @@ grpc_cc_test( "//:grpc", "//:grpc_xds_client", "//src/proto/grpc/testing/xds/v3:tls_proto", + "//src/proto/grpc/testing/xds/v3:typed_struct_proto", + "//src/proto/grpc/testing/xds/v3:udpa_typed_struct_proto", "//test/core/util:grpc_test_util", + "//test/cpp/util:grpc_cli_utils", ], ) diff --git a/test/core/xds/xds_common_types_test.cc b/test/core/xds/xds_common_types_test.cc index 13b1f864cd2..c706fcf1337 100644 --- a/test/core/xds/xds_common_types_test.cc +++ b/test/core/xds/xds_common_types_test.cc @@ -16,17 +16,20 @@ #include "src/core/ext/xds/xds_common_types.h" +#include #include #include #include #include +#include #include #include "absl/status/status.h" #include "absl/status/statusor.h" #include "envoy/extensions/transport_sockets/tls/v3/tls.upb.h" #include "gmock/gmock.h" +#include "google/protobuf/any.upb.h" #include "google/protobuf/duration.upb.h" #include "gtest/gtest.h" #include "re2/re2.h" @@ -36,6 +39,7 @@ #include #include +#include "src/core/ext/xds/upb_utils.h" #include "src/core/ext/xds/xds_bootstrap.h" #include "src/core/ext/xds/xds_bootstrap_grpc.h" #include "src/core/ext/xds/xds_client.h" @@ -44,15 +48,18 @@ #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" #include "src/core/lib/gprpp/validation_errors.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/matchers/matchers.h" #include "src/proto/grpc/testing/xds/v3/regex.pb.h" #include "src/proto/grpc/testing/xds/v3/string.pb.h" #include "src/proto/grpc/testing/xds/v3/tls.pb.h" +#include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h" +#include "src/proto/grpc/testing/xds/v3/udpa_typed_struct.pb.h" #include "test/core/util/test_config.h" +#include "test/cpp/util/config_grpc_cli.h" using CommonTlsContextProto = envoy::extensions::transport_sockets::tls::v3::CommonTlsContext; +using xds::type::v3::TypedStruct; namespace grpc_core { namespace testing { @@ -69,7 +76,6 @@ class XdsCommonTypesTest : public ::testing::Test { upb_arena_.ptr()} {} static RefCountedPtr MakeXdsClient() { - grpc_error_handle error; auto bootstrap = GrpcXdsBootstrap::Create( "{\n" " \"xds_servers\": [\n" @@ -521,6 +527,321 @@ TEST_F(CommonTlsConfigTest, ValidationContextUnsupportedFields) { << common_tls_context.status(); } +// +// ExtractXdsExtension() tests +// + +using ExtractXdsExtensionTest = XdsCommonTypesTest; + +TEST_F(ExtractXdsExtensionTest, Basic) { + constexpr absl::string_view kTypeUrl = "type.googleapis.com/MyType"; + constexpr absl::string_view kValue = "foobar"; + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url(any_proto, StdStringToUpbString(kTypeUrl)); + google_protobuf_Any_set_value(any_proto, StdStringToUpbString(kValue)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); + ASSERT_TRUE(extension.has_value()); + EXPECT_EQ(extension->type, "MyType"); + ASSERT_TRUE(absl::holds_alternative(extension->value)); + EXPECT_EQ(absl::get(extension->value), kValue); +} + +TEST_F(ExtractXdsExtensionTest, TypedStruct) { + TypedStruct typed_struct; + typed_struct.set_type_url("type.googleapis.com/MyType"); + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].set_string_value("bar"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); + ASSERT_TRUE(extension.has_value()); + EXPECT_EQ(extension->type, "MyType"); + ASSERT_TRUE(absl::holds_alternative(extension->value)); + EXPECT_EQ(absl::get(extension->value).Dump(), "{\"foo\":\"bar\"}"); +} + +TEST_F(ExtractXdsExtensionTest, UdpaTypedStruct) { + udpa::type::v1::TypedStruct typed_struct; + typed_struct.set_type_url("type.googleapis.com/MyType"); + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].set_string_value("bar"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); + ASSERT_TRUE(extension.has_value()); + EXPECT_EQ(extension->type, "MyType"); + ASSERT_TRUE(absl::holds_alternative(extension->value)); + EXPECT_EQ(absl::get(extension->value).Dump(), "{\"foo\":\"bar\"}"); +} + +TEST_F(ExtractXdsExtensionTest, TypedStructWithoutValue) { + TypedStruct typed_struct; + typed_struct.set_type_url("type.googleapis.com/MyType"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); + ASSERT_TRUE(extension.has_value()); + EXPECT_EQ(extension->type, "MyType"); + ASSERT_TRUE(absl::holds_alternative(extension->value)); + EXPECT_EQ(absl::get(extension->value).Dump(), "{}"); +} + +TEST_F(ExtractXdsExtensionTest, TypedStructJsonConversion) { + TypedStruct typed_struct; + ASSERT_TRUE(grpc::protobuf::TextFormat::ParseFromString( + R"pb( + type_url: "type.googleapis.com/envoy.ExtensionType" + value { + fields { + key: "key" + value { null_value: NULL_VALUE } + } + fields { + key: "number" + value { number_value: 123 } + } + fields { + key: "string" + value { string_value: "value" } + } + fields { + key: "struct" + value { + struct_value { + fields { + key: "key" + value { null_value: NULL_VALUE } + } + } + } + } + fields { + key: "list" + value { + list_value { + values { null_value: NULL_VALUE } + values { number_value: 234 } + } + } + } + } + )pb", + &typed_struct)); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_TRUE(errors.ok()) << errors.status("unexpected errors"); + ASSERT_TRUE(extension.has_value()); + EXPECT_EQ(extension->type, "envoy.ExtensionType"); + ASSERT_TRUE(absl::holds_alternative(extension->value)); + EXPECT_EQ(absl::get(extension->value).Dump(), + "{" + "\"key\":null," + "\"list\":[null,234]," + "\"number\":123," + "\"string\":\"value\"," + "\"struct\":{\"key\":null}" + "}"); +} + +TEST_F(ExtractXdsExtensionTest, FieldMissing) { + ValidationErrors errors; + ValidationErrors::ScopedField field(&errors, "any"); + auto extension = ExtractXdsExtension(decode_context_, nullptr, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [field:any error:field not present]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypeUrlMissing) { + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [field:type_url error:field not present]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypedStructTypeUrlMissing) { + TypedStruct typed_struct; + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].set_string_value("bar"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:value[xds.type.v3.TypedStruct].type_url " + "error:field not present]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypeUrlNoSlash) { + constexpr absl::string_view kTypeUrl = "MyType"; + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url(any_proto, StdStringToUpbString(kTypeUrl)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:type_url error:invalid value \"MyType\"]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypedStructTypeUrlNoSlash) { + TypedStruct typed_struct; + typed_struct.set_type_url("MyType"); + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].set_string_value("bar"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:value[xds.type.v3.TypedStruct].type_url " + "error:invalid value \"MyType\"]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypeUrlNothingAfterSlash) { + constexpr absl::string_view kTypeUrl = "type.googleapi.com/"; + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url(any_proto, StdStringToUpbString(kTypeUrl)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:type_url error:invalid value \"type.googleapi.com/\"]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypedStructTypeUrlNothingAfterSlash) { + TypedStruct typed_struct; + typed_struct.set_type_url("type.googleapis.com/"); + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].set_string_value("bar"); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:value[xds.type.v3.TypedStruct].type_url " + "error:invalid value \"type.googleapis.com/\"]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypedStructParseFailure) { + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + std::string serialized_type_struct("\0", 1); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_type_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:value[xds.type.v3.TypedStruct] error:could not parse]") + << status; +} + +TEST_F(ExtractXdsExtensionTest, TypedStructWithInvalidProtobufStruct) { + TypedStruct typed_struct; + typed_struct.set_type_url("type.googleapis.com/xds.MyType"); + auto* fields = typed_struct.mutable_value()->mutable_fields(); + (*fields)["foo"].mutable_list_value()->add_values(); + std::string serialized_typed_struct = typed_struct.SerializeAsString(); + google_protobuf_Any* any_proto = google_protobuf_Any_new(upb_arena_.ptr()); + google_protobuf_Any_set_type_url( + any_proto, StdStringToUpbString(absl::string_view( + "type.googleapis.com/xds.type.v3.TypedStruct"))); + google_protobuf_Any_set_value(any_proto, + StdStringToUpbString(serialized_typed_struct)); + ValidationErrors errors; + auto extension = ExtractXdsExtension(decode_context_, any_proto, &errors); + ASSERT_FALSE(errors.ok()); + absl::Status status = errors.status("validation errors"); + EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument); + EXPECT_EQ(status.message(), + "validation errors: [" + "field:value[xds.type.v3.TypedStruct].value[xds.MyType] " + "error:error encoding google::Protobuf::Struct as JSON: " + "No value set in Value proto]") + << status; +} + } // namespace } // namespace testing } // namespace grpc_core diff --git a/test/core/xds/xds_lb_policy_registry_test.cc b/test/core/xds/xds_lb_policy_registry_test.cc index 5b08dc48a0a..bf9334514d8 100644 --- a/test/core/xds/xds_lb_policy_registry_test.cc +++ b/test/core/xds/xds_lb_policy_registry_test.cc @@ -299,7 +299,7 @@ TEST(XdsLbPolicyRegistryTest, CustomLbPolicy) { auto result = ConvertXdsPolicy(policy); EXPECT_TRUE(result.ok()); EXPECT_EQ(result->size(), 1); - EXPECT_EQ((*result)[0], Json::Parse("{\"test.CustomLb\": null}").value()); + EXPECT_EQ((*result)[0], Json::Parse("{\"test.CustomLb\": {}}").value()); } TEST(XdsLbPolicyRegistryTest, CustomLbPolicyUdpaTyped) { @@ -312,7 +312,7 @@ TEST(XdsLbPolicyRegistryTest, CustomLbPolicyUdpaTyped) { auto result = ConvertXdsPolicy(policy); EXPECT_TRUE(result.ok()); EXPECT_EQ(result->size(), 1); - EXPECT_EQ((*result)[0], Json::Parse("{\"test.CustomLb\": null}").value()); + EXPECT_EQ((*result)[0], Json::Parse("{\"test.CustomLb\": {}}").value()); } TEST(XdsLbPolicyRegistryTest, UnsupportedCustomTypeError) { @@ -338,11 +338,13 @@ TEST(XdsLbPolicyRegistryTest, CustomTypeInvalidUrlMissingSlash) { typed_struct); auto result = ConvertXdsPolicy(policy); EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument); - EXPECT_THAT( - std::string(result.status().message()), - ::testing::HasSubstr("Error parsing " - "LoadBalancingPolicy::Policy::TypedExtensionConfig::" - "typed_config: Invalid type_url test.UnknownLb")); + EXPECT_EQ(result.status().message(), + "Error parsing " + "LoadBalancingPolicy::Policy::TypedExtensionConfig::" + "typed_config: [" + "field:value[xds.type.v3.TypedStruct].type_url " + "error:invalid value \"test.UnknownLb\"]") + << result.status(); } TEST(XdsLbPolicyRegistryTest, CustomTypeInvalidUrlEmptyType) { @@ -354,11 +356,13 @@ TEST(XdsLbPolicyRegistryTest, CustomTypeInvalidUrlEmptyType) { typed_struct); auto result = ConvertXdsPolicy(policy); EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument); - EXPECT_THAT( - std::string(result.status().message()), - ::testing::HasSubstr("Error parsing " - "LoadBalancingPolicy::Policy::TypedExtensionConfig::" - "typed_config: Invalid type_url myorg/")); + EXPECT_EQ(result.status().message(), + "Error parsing " + "LoadBalancingPolicy::Policy::TypedExtensionConfig::" + "typed_config: " + "[field:value[xds.type.v3.TypedStruct].type_url " + "error:invalid value \"myorg/\"]") + << result.status(); } TEST(XdsLbPolicyRegistryTest, CustomLbPolicyJsonConversion) { @@ -437,11 +441,13 @@ TEST(XdsLbPolicyRegistryTest, CustomLbPolicyListError) { typed_struct); auto result = ConvertXdsPolicy(policy); EXPECT_EQ(result.status().code(), absl::StatusCode::kInvalidArgument); - EXPECT_THAT(std::string(result.status().message()), - ::testing::HasSubstr( - "Error parsing LoadBalancingPolicy: Custom Policy: " - "test.CustomLb: Error parsing google::Protobuf::Struct: No " - "value set in Value proto")); + EXPECT_EQ(result.status().message(), + "Error parsing " + "LoadBalancingPolicy::Policy::TypedExtensionConfig::typed_config: [" + "field:value[xds.type.v3.TypedStruct].value[test.CustomLb] " + "error:error encoding google::Protobuf::Struct as JSON: " + "No value set in Value proto]") + << result.status(); } TEST(XdsLbPolicyRegistryTest, UnsupportedBuiltInTypeSkipped) { diff --git a/test/cpp/end2end/xds/no_op_http_filter.h b/test/cpp/end2end/xds/no_op_http_filter.h index 179e0eda882..35b3a96919f 100644 --- a/test/cpp/end2end/xds/no_op_http_filter.h +++ b/test/cpp/end2end/xds/no_op_http_filter.h @@ -29,17 +29,18 @@ class NoOpHttpFilter : public grpc_core::XdsHttpFilterImpl { supported_on_servers_(supported_on_servers), is_terminal_filter_(is_terminal_filter) {} - void PopulateSymtab(upb_DefPool* /* symtab */) const override {} + void PopulateSymtab(upb_DefPool* /*symtab*/) const override {} absl::StatusOr - GenerateFilterConfig(upb_StringView /* serialized_filter_config */, - upb_Arena* /* arena */) const override { + GenerateFilterConfig(grpc_core::XdsExtension /*extension*/, + upb_Arena* /*arena*/) const override { return grpc_core::XdsHttpFilterImpl::FilterConfig{name_, grpc_core::Json()}; } absl::StatusOr - GenerateFilterConfigOverride(upb_StringView /*serialized_filter_config*/, - upb_Arena* /*arena*/) const override { + GenerateFilterConfigOverride( + grpc_core::XdsExtension /*serialized_filter_config*/, + upb_Arena* /*arena*/) const override { return grpc_core::XdsHttpFilterImpl::FilterConfig{name_, grpc_core::Json()}; }