From a96ae699f7b3283ecbb12b43259635005d03e3d1 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 30 Jan 2020 07:56:49 -0800 Subject: [PATCH] Convert xds bootstrap code to use new JSON API --- .../ext/filters/client_channel/xds/xds_api.cc | 73 +-- .../client_channel/xds/xds_bootstrap.cc | 475 +++++------------- .../client_channel/xds/xds_bootstrap.h | 57 +-- .../filters/client_channel/xds/xds_channel.cc | 4 +- .../client_channel/xds/xds_channel_secure.cc | 12 +- .../filters/client_channel/xds/xds_client.cc | 2 +- src/core/lib/iomgr/load_file.cc | 1 + src/core/lib/json/json.h | 1 + .../core/client_channel/xds_bootstrap_test.cc | 286 ++++------- 9 files changed, 300 insertions(+), 611 deletions(-) diff --git a/src/core/ext/filters/client_channel/xds/xds_api.cc b/src/core/ext/filters/client_channel/xds/xds_api.cc index dc7d10a7f4c..a51256594fb 100644 --- a/src/core/ext/filters/client_channel/xds/xds_api.cc +++ b/src/core/ext/filters/client_channel/xds/xds_api.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -106,10 +107,10 @@ bool XdsDropConfig::ShouldDrop(const std::string** category_name) const { namespace { void PopulateMetadataValue(upb_arena* arena, google_protobuf_Value* value_pb, - const XdsBootstrap::MetadataValue& value); + const Json& value); void PopulateListValue(upb_arena* arena, google_protobuf_ListValue* list_value, - const std::vector& values) { + const Json::Array& values) { for (const auto& value : values) { auto* value_pb = google_protobuf_ListValue_add_values(list_value, arena); PopulateMetadataValue(arena, value_pb, value); @@ -117,13 +118,12 @@ void PopulateListValue(upb_arena* arena, google_protobuf_ListValue* list_value, } void PopulateMetadata(upb_arena* arena, google_protobuf_Struct* metadata_pb, - const std::map& metadata) { + const Json::Object& metadata) { for (const auto& p : metadata) { google_protobuf_Struct_FieldsEntry* field = google_protobuf_Struct_add_fields(metadata_pb, arena); - google_protobuf_Struct_FieldsEntry_set_key(field, - upb_strview_makez(p.first)); + google_protobuf_Struct_FieldsEntry_set_key( + field, upb_strview_makez(p.first.c_str())); google_protobuf_Value* value = google_protobuf_Struct_FieldsEntry_mutable_value(field, arena); PopulateMetadataValue(arena, value, p.second); @@ -131,31 +131,35 @@ void PopulateMetadata(upb_arena* arena, google_protobuf_Struct* metadata_pb, } void PopulateMetadataValue(upb_arena* arena, google_protobuf_Value* value_pb, - const XdsBootstrap::MetadataValue& value) { - switch (value.type) { - case XdsBootstrap::MetadataValue::Type::MD_NULL: + const Json& value) { + switch (value.type()) { + case Json::Type::JSON_NULL: google_protobuf_Value_set_null_value(value_pb, 0); break; - case XdsBootstrap::MetadataValue::Type::DOUBLE: - google_protobuf_Value_set_number_value(value_pb, value.double_value); + case Json::Type::NUMBER: + google_protobuf_Value_set_number_value( + value_pb, strtod(value.string_value().c_str(), nullptr)); break; - case XdsBootstrap::MetadataValue::Type::STRING: + case Json::Type::STRING: google_protobuf_Value_set_string_value( - value_pb, upb_strview_makez(value.string_value)); + value_pb, upb_strview_makez(value.string_value().c_str())); break; - case XdsBootstrap::MetadataValue::Type::BOOL: - google_protobuf_Value_set_bool_value(value_pb, value.bool_value); + case Json::Type::JSON_TRUE: + google_protobuf_Value_set_bool_value(value_pb, true); break; - case XdsBootstrap::MetadataValue::Type::STRUCT: { + case Json::Type::JSON_FALSE: + google_protobuf_Value_set_bool_value(value_pb, false); + break; + case Json::Type::OBJECT: { google_protobuf_Struct* struct_value = google_protobuf_Value_mutable_struct_value(value_pb, arena); - PopulateMetadata(arena, struct_value, value.struct_value); + PopulateMetadata(arena, struct_value, value.object_value()); break; } - case XdsBootstrap::MetadataValue::Type::LIST: { + case Json::Type::ARRAY: { google_protobuf_ListValue* list_value = google_protobuf_Value_mutable_list_value(value_pb, arena); - PopulateListValue(arena, list_value, value.list_value); + PopulateListValue(arena, list_value, value.array_value()); break; } } @@ -164,33 +168,34 @@ void PopulateMetadataValue(upb_arena* arena, google_protobuf_Value* value_pb, void PopulateNode(upb_arena* arena, const XdsBootstrap::Node* node, const char* build_version, envoy_api_v2_core_Node* node_msg) { if (node != nullptr) { - if (node->id != nullptr) { - envoy_api_v2_core_Node_set_id(node_msg, upb_strview_makez(node->id)); + if (!node->id.empty()) { + envoy_api_v2_core_Node_set_id(node_msg, + upb_strview_makez(node->id.c_str())); } - if (node->cluster != nullptr) { - envoy_api_v2_core_Node_set_cluster(node_msg, - upb_strview_makez(node->cluster)); + if (!node->cluster.empty()) { + envoy_api_v2_core_Node_set_cluster( + node_msg, upb_strview_makez(node->cluster.c_str())); } - if (!node->metadata.empty()) { + if (!node->metadata.object_value().empty()) { google_protobuf_Struct* metadata = envoy_api_v2_core_Node_mutable_metadata(node_msg, arena); - PopulateMetadata(arena, metadata, node->metadata); + PopulateMetadata(arena, metadata, node->metadata.object_value()); } - if (node->locality_region != nullptr || node->locality_zone != nullptr || - node->locality_subzone != nullptr) { + if (!node->locality_region.empty() || !node->locality_zone.empty() || + !node->locality_subzone.empty()) { envoy_api_v2_core_Locality* locality = envoy_api_v2_core_Node_mutable_locality(node_msg, arena); - if (node->locality_region != nullptr) { + if (!node->locality_region.empty()) { envoy_api_v2_core_Locality_set_region( - locality, upb_strview_makez(node->locality_region)); + locality, upb_strview_makez(node->locality_region.c_str())); } - if (node->locality_zone != nullptr) { + if (!node->locality_zone.empty()) { envoy_api_v2_core_Locality_set_zone( - locality, upb_strview_makez(node->locality_zone)); + locality, upb_strview_makez(node->locality_zone.c_str())); } - if (node->locality_subzone != nullptr) { + if (!node->locality_subzone.empty()) { envoy_api_v2_core_Locality_set_sub_zone( - locality, upb_strview_makez(node->locality_subzone)); + locality, upb_strview_makez(node->locality_subzone.c_str())); } } } diff --git a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc index 1be01a0dca0..13ca99637b8 100644 --- a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc +++ b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc @@ -39,86 +39,54 @@ std::unique_ptr XdsBootstrap::ReadFromFile(grpc_error** error) { grpc_slice contents; *error = grpc_load_file(path.get(), /*add_null_terminator=*/true, &contents); if (*error != GRPC_ERROR_NONE) return nullptr; - return grpc_core::MakeUnique(contents, error); + Json json = Json::Parse(StringViewFromSlice(contents), error); + grpc_slice_unref_internal(contents); + if (*error != GRPC_ERROR_NONE) return nullptr; + return grpc_core::MakeUnique(std::move(json), error); } -XdsBootstrap::XdsBootstrap(grpc_slice contents, grpc_error** error) - : contents_(contents) { - tree_ = grpc_json_parse_string_with_len( - reinterpret_cast(GPR_SLICE_START_PTR(contents_)), - GPR_SLICE_LENGTH(contents_)); - if (tree_ == nullptr) { - *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "failed to parse bootstrap file JSON"); - return; - } - if (tree_->type != GRPC_JSON_OBJECT || tree_->key != nullptr) { +XdsBootstrap::XdsBootstrap(Json json, grpc_error** error) { + if (json.type() != Json::Type::OBJECT) { *error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "malformed JSON in bootstrap file"); return; } InlinedVector error_list; - bool seen_xds_servers = false; - bool seen_node = false; - for (grpc_json* child = tree_->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - } else if (strcmp(child->key, "xds_servers") == 0) { - if (child->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"xds_servers\" field is not an array")); - } - if (seen_xds_servers) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"xds_servers\" field")); - } - seen_xds_servers = true; - grpc_error* parse_error = ParseXdsServerList(child); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); - } else if (strcmp(child->key, "node") == 0) { - if (child->type != GRPC_JSON_OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"node\" field is not an object")); - } - if (seen_node) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"node\" field")); - } - seen_node = true; - grpc_error* parse_error = ParseNode(child); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); - } - } - if (!seen_xds_servers) { + auto it = json.mutable_object()->find("xds_servers"); + if (it == json.mutable_object()->end()) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "\"xds_servers\" field not present")); + } else if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"xds_servers\" field is not an array")); + } else { + grpc_error* parse_error = ParseXdsServerList(&it->second); + if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); + } + it = json.mutable_object()->find("node"); + if (it != json.mutable_object()->end()) { + if (it->second.type() != Json::Type::OBJECT) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"node\" field is not an object")); + } else { + grpc_error* parse_error = ParseNode(&it->second); + if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); + } } *error = GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing xds bootstrap file", &error_list); } -XdsBootstrap::~XdsBootstrap() { - grpc_json_destroy(tree_); - grpc_slice_unref_internal(contents_); -} - -grpc_error* XdsBootstrap::ParseXdsServerList(grpc_json* json) { +grpc_error* XdsBootstrap::ParseXdsServerList(Json* json) { InlinedVector error_list; size_t idx = 0; - for (grpc_json *child = json->child; child != nullptr; - child = child->next, ++idx) { - if (child->key != nullptr) { - char* msg; - gpr_asprintf(&msg, "array element %" PRIuPTR " key is not null", idx); - error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); - } - if (child->type != GRPC_JSON_OBJECT) { + for (Json& child : *json->mutable_array()) { + if (child.type() != Json::Type::OBJECT) { char* msg; gpr_asprintf(&msg, "array element %" PRIuPTR " is not an object", idx); error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); } else { - grpc_error* parse_error = ParseXdsServer(child, idx); + grpc_error* parse_error = ParseXdsServer(&child, idx); if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } } @@ -126,42 +94,29 @@ grpc_error* XdsBootstrap::ParseXdsServerList(grpc_json* json) { &error_list); } -grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json, size_t idx) { +grpc_error* XdsBootstrap::ParseXdsServer(Json* json, size_t idx) { InlinedVector error_list; servers_.emplace_back(); XdsServer& server = servers_[servers_.size() - 1]; - bool seen_channel_creds = false; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - } else if (strcmp(child->key, "server_uri") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"server_uri\" field is not a string")); - } - if (server.server_uri != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"server_uri\" field")); - } - server.server_uri = child->value; - } else if (strcmp(child->key, "channel_creds") == 0) { - if (child->type != GRPC_JSON_ARRAY) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"channel_creds\" field is not an array")); - } - if (seen_channel_creds) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"channel_creds\" field")); - } - seen_channel_creds = true; - grpc_error* parse_error = ParseChannelCredsArray(child, &server); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); - } - } - if (server.server_uri == nullptr) { + auto it = json->mutable_object()->find("server_uri"); + if (it == json->mutable_object()->end()) { error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( "\"server_uri\" field not present")); + } else if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"server_uri\" field is not a string")); + } else { + server.server_uri = std::move(*it->second.mutable_string_value()); + } + it = json->mutable_object()->find("channel_creds"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::ARRAY) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"channel_creds\" field is not an array")); + } else { + grpc_error* parse_error = ParseChannelCredsArray(&it->second, &server); + if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); + } } // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error // string is not static in this case. @@ -176,23 +131,17 @@ grpc_error* XdsBootstrap::ParseXdsServer(grpc_json* json, size_t idx) { return error; } -grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json, +grpc_error* XdsBootstrap::ParseChannelCredsArray(Json* json, XdsServer* server) { InlinedVector error_list; - size_t idx = 0; - for (grpc_json *child = json->child; child != nullptr; - child = child->next, ++idx) { - if (child->key != nullptr) { + for (size_t i = 0; i < json->mutable_array()->size(); ++i) { + Json& child = json->mutable_array()->at(i); + if (child.type() != Json::Type::OBJECT) { char* msg; - gpr_asprintf(&msg, "array element %" PRIuPTR " key is not null", idx); - error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); - } - if (child->type != GRPC_JSON_OBJECT) { - char* msg; - gpr_asprintf(&msg, "array element %" PRIuPTR " is not an object", idx); + gpr_asprintf(&msg, "array element %" PRIuPTR " is not an object", i); error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); } else { - grpc_error* parse_error = ParseChannelCreds(child, idx, server); + grpc_error* parse_error = ParseChannelCreds(&child, i, server); if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } } @@ -200,38 +149,31 @@ grpc_error* XdsBootstrap::ParseChannelCredsArray(grpc_json* json, &error_list); } -grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx, +grpc_error* XdsBootstrap::ParseChannelCreds(Json* json, size_t idx, XdsServer* server) { InlinedVector error_list; ChannelCreds channel_creds; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - } else if (strcmp(child->key, "type") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"type\" field is not a string")); - } - if (channel_creds.type != nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"type\" field")); - } - channel_creds.type = child->value; - } else if (strcmp(child->key, "config") == 0) { - if (child->type != GRPC_JSON_OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"config\" field is not an object")); - } - if (channel_creds.config != nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"config\" field")); - } - channel_creds.config = child; + auto it = json->mutable_object()->find("type"); + if (it == json->mutable_object()->end()) { + error_list.push_back( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("\"type\" field not present")); + } else if (it->second.type() != Json::Type::STRING) { + error_list.push_back( + GRPC_ERROR_CREATE_FROM_STATIC_STRING("\"type\" field is not a string")); + } else { + channel_creds.type = std::move(*it->second.mutable_string_value()); + } + it = json->mutable_object()->find("config"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::OBJECT) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"config\" field is not an object")); + } else { + channel_creds.config = std::move(it->second); } } - if (channel_creds.type != nullptr) { - server->channel_creds.push_back(channel_creds); + if (!channel_creds.type.empty()) { + server->channel_creds.emplace_back(std::move(channel_creds)); } // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error // string is not static in this case. @@ -246,242 +188,81 @@ grpc_error* XdsBootstrap::ParseChannelCreds(grpc_json* json, size_t idx, return error; } -grpc_error* XdsBootstrap::ParseNode(grpc_json* json) { +grpc_error* XdsBootstrap::ParseNode(Json* json) { InlinedVector error_list; node_ = grpc_core::MakeUnique(); - bool seen_metadata = false; - bool seen_locality = false; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { + auto it = json->mutable_object()->find("id"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::STRING) { error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - } else if (strcmp(child->key, "id") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"id\" field is not a string")); - } - if (node_->id != nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"id\" field")); - } - node_->id = child->value; - } else if (strcmp(child->key, "cluster") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"cluster\" field is not a string")); - } - if (node_->cluster != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"cluster\" field")); - } - node_->cluster = child->value; - } else if (strcmp(child->key, "locality") == 0) { - if (child->type != GRPC_JSON_OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"locality\" field is not an object")); - } - if (seen_locality) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"locality\" field")); - } - seen_locality = true; - grpc_error* parse_error = ParseLocality(child); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); - } else if (strcmp(child->key, "metadata") == 0) { - if (child->type != GRPC_JSON_OBJECT) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"metadata\" field is not an object")); - } - if (seen_metadata) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"metadata\" field")); - } - seen_metadata = true; - InlinedVector parse_errors = - ParseMetadataStruct(child, &node_->metadata); - if (!parse_errors.empty()) { - grpc_error* parse_error = GRPC_ERROR_CREATE_FROM_VECTOR( - "errors parsing \"metadata\" object", &parse_errors); - error_list.push_back(parse_error); - } + GRPC_ERROR_CREATE_FROM_STATIC_STRING("\"id\" field is not a string")); + } else { + node_->id = std::move(*it->second.mutable_string_value()); } } - return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"node\" object", - &error_list); -} - -grpc_error* XdsBootstrap::ParseLocality(grpc_json* json) { - InlinedVector error_list; - node_->locality_region = nullptr; - node_->locality_zone = nullptr; - node_->locality_subzone = nullptr; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - } else if (strcmp(child->key, "region") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"region\" field is not a string")); - } - if (node_->locality_region != nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"region\" field")); - } - node_->locality_region = child->value; - } else if (strcmp(child->key, "zone") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"zone\" field is not a string")); - } - if (node_->locality_zone != nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("duplicate \"zone\" field")); - } - node_->locality_zone = child->value; - } else if (strcmp(child->key, "subzone") == 0) { - if (child->type != GRPC_JSON_STRING) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "\"subzone\" field is not a string")); - } - if (node_->locality_subzone != nullptr) { - error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "duplicate \"subzone\" field")); - } - node_->locality_subzone = child->value; + it = json->mutable_object()->find("cluster"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"cluster\" field is not a string")); + } else { + node_->cluster = std::move(*it->second.mutable_string_value()); } } - return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"locality\" object", - &error_list); -} - -InlinedVector XdsBootstrap::ParseMetadataStruct( - grpc_json* json, - std::map* result) { - InlinedVector error_list; - for (grpc_json* child = json->child; child != nullptr; child = child->next) { - if (child->key == nullptr) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_STATIC_STRING("JSON key is null")); - continue; + it = json->mutable_object()->find("locality"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::OBJECT) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"locality\" field is not an object")); + } else { + grpc_error* parse_error = ParseLocality(&it->second); + if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } - if (result->find(child->key) != result->end()) { - char* msg; - gpr_asprintf(&msg, "duplicate metadata key \"%s\"", child->key); - error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); - gpr_free(msg); + } + it = json->mutable_object()->find("metadata"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::OBJECT) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"metadata\" field is not an object")); + } else { + node_->metadata = std::move(it->second); } - MetadataValue& value = (*result)[child->key]; - grpc_error* parse_error = ParseMetadataValue(child, 0, &value); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } - return error_list; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"node\" object", + &error_list); } -InlinedVector XdsBootstrap::ParseMetadataList( - grpc_json* json, std::vector* result) { +grpc_error* XdsBootstrap::ParseLocality(Json* json) { InlinedVector error_list; - size_t idx = 0; - for (grpc_json *child = json->child; child != nullptr; - child = child->next, ++idx) { - if (child->key != nullptr) { - char* msg; - gpr_asprintf(&msg, "JSON key is non-null for index %" PRIuPTR, idx); - error_list.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg)); - gpr_free(msg); + auto it = json->mutable_object()->find("region"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"region\" field is not a string")); + } else { + node_->locality_region = std::move(*it->second.mutable_string_value()); } - result->emplace_back(); - grpc_error* parse_error = ParseMetadataValue(child, idx, &result->back()); - if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error); } - return error_list; -} - -grpc_error* XdsBootstrap::ParseMetadataValue(grpc_json* json, size_t idx, - MetadataValue* result) { - grpc_error* error = GRPC_ERROR_NONE; - auto context_func = [json, idx]() { - char* context; - if (json->key != nullptr) { - gpr_asprintf(&context, "key \"%s\"", json->key); + it = json->mutable_object()->find("zone"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"zone\" field is not a string")); } else { - gpr_asprintf(&context, "index %" PRIuPTR, idx); + node_->locality_zone = std::move(*it->second.mutable_string_value()); } - return context; - }; - switch (json->type) { - case GRPC_JSON_STRING: - result->type = MetadataValue::Type::STRING; - result->string_value = json->value; - break; - case GRPC_JSON_NUMBER: - result->type = MetadataValue::Type::DOUBLE; - errno = 0; // To distinguish error. - result->double_value = strtod(json->value, nullptr); - if (errno != 0) { - char* context = context_func(); - char* msg; - gpr_asprintf(&msg, "error parsing numeric value for %s: \"%s\"", - context, json->value); - error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); - gpr_free(context); - gpr_free(msg); - } - break; - case GRPC_JSON_TRUE: - result->type = MetadataValue::Type::BOOL; - result->bool_value = true; - break; - case GRPC_JSON_FALSE: - result->type = MetadataValue::Type::BOOL; - result->bool_value = false; - break; - case GRPC_JSON_NULL: - result->type = MetadataValue::Type::MD_NULL; - break; - case GRPC_JSON_ARRAY: { - result->type = MetadataValue::Type::LIST; - InlinedVector error_list = - ParseMetadataList(json, &result->list_value); - if (!error_list.empty()) { - // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error - // string is not static in this case. - char* context = context_func(); - char* msg; - gpr_asprintf(&msg, "errors parsing struct for %s", context); - error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); - gpr_free(context); - gpr_free(msg); - for (size_t i = 0; i < error_list.size(); ++i) { - error = grpc_error_add_child(error, error_list[i]); - } - } - break; - } - case GRPC_JSON_OBJECT: { - result->type = MetadataValue::Type::STRUCT; - InlinedVector error_list = - ParseMetadataStruct(json, &result->struct_value); - if (!error_list.empty()) { - // Can't use GRPC_ERROR_CREATE_FROM_VECTOR() here, because the error - // string is not static in this case. - char* context = context_func(); - char* msg; - gpr_asprintf(&msg, "errors parsing struct for %s", context); - error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg); - gpr_free(context); - gpr_free(msg); - for (size_t i = 0; i < error_list.size(); ++i) { - error = grpc_error_add_child(error, error_list[i]); - GRPC_ERROR_UNREF(error_list[i]); - } - } - break; + } + it = json->mutable_object()->find("subzone"); + if (it != json->mutable_object()->end()) { + if (it->second.type() != Json::Type::STRING) { + error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "\"subzone\" field is not a string")); + } else { + node_->locality_subzone = std::move(*it->second.mutable_string_value()); } - default: - break; } - return error; + return GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing \"locality\" object", + &error_list); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/xds/xds_bootstrap.h b/src/core/ext/filters/client_channel/xds/xds_bootstrap.h index 5176ac749ee..26599aa9404 100644 --- a/src/core/ext/filters/client_channel/xds/xds_bootstrap.h +++ b/src/core/ext/filters/client_channel/xds/xds_bootstrap.h @@ -19,6 +19,8 @@ #include +#include +#include #include #include @@ -33,33 +35,22 @@ namespace grpc_core { class XdsBootstrap { public: - struct MetadataValue { - enum class Type { MD_NULL, DOUBLE, STRING, BOOL, STRUCT, LIST }; - Type type = Type::MD_NULL; - // TODO(roth): Once we can use C++17, these can be in a std::variant. - double double_value; - const char* string_value; - bool bool_value; - std::map struct_value; - std::vector list_value; - }; - struct Node { - const char* id = nullptr; - const char* cluster = nullptr; - const char* locality_region = nullptr; - const char* locality_zone = nullptr; - const char* locality_subzone = nullptr; - std::map metadata; + std::string id; + std::string cluster; + std::string locality_region; + std::string locality_zone; + std::string locality_subzone; + Json metadata; }; struct ChannelCreds { - const char* type = nullptr; - grpc_json* config = nullptr; + std::string type; + Json config; }; struct XdsServer { - const char* server_uri = nullptr; + std::string server_uri; InlinedVector channel_creds; }; @@ -68,8 +59,7 @@ class XdsBootstrap { static std::unique_ptr ReadFromFile(grpc_error** error); // Do not instantiate directly -- use ReadFromFile() above instead. - XdsBootstrap(grpc_slice contents, grpc_error** error); - ~XdsBootstrap(); + XdsBootstrap(Json json, grpc_error** error); // TODO(roth): We currently support only one server. Fix this when we // add support for fallback for the xds channel. @@ -77,23 +67,12 @@ class XdsBootstrap { const Node* node() const { return node_.get(); } private: - grpc_error* ParseXdsServerList(grpc_json* json); - grpc_error* ParseXdsServer(grpc_json* json, size_t idx); - grpc_error* ParseChannelCredsArray(grpc_json* json, XdsServer* server); - grpc_error* ParseChannelCreds(grpc_json* json, size_t idx, XdsServer* server); - grpc_error* ParseNode(grpc_json* json); - grpc_error* ParseLocality(grpc_json* json); - - InlinedVector ParseMetadataStruct( - grpc_json* json, - std::map* result); - InlinedVector ParseMetadataList( - grpc_json* json, std::vector* result); - grpc_error* ParseMetadataValue(grpc_json* json, size_t idx, - MetadataValue* result); - - grpc_slice contents_; - grpc_json* tree_ = nullptr; + grpc_error* ParseXdsServerList(Json* json); + grpc_error* ParseXdsServer(Json* json, size_t idx); + grpc_error* ParseChannelCredsArray(Json* json, XdsServer* server); + grpc_error* ParseChannelCreds(Json* json, size_t idx, XdsServer* server); + grpc_error* ParseNode(Json* json); + grpc_error* ParseLocality(Json* json); InlinedVector servers_; std::unique_ptr node_; diff --git a/src/core/ext/filters/client_channel/xds/xds_channel.cc b/src/core/ext/filters/client_channel/xds/xds_channel.cc index e8ba3706f13..d30cee047c9 100644 --- a/src/core/ext/filters/client_channel/xds/xds_channel.cc +++ b/src/core/ext/filters/client_channel/xds/xds_channel.cc @@ -31,8 +31,8 @@ grpc_channel_args* ModifyXdsChannelArgs(grpc_channel_args* args) { grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap, const grpc_channel_args& args) { if (!bootstrap.server().channel_creds.empty()) return nullptr; - return grpc_insecure_channel_create(bootstrap.server().server_uri, &args, - nullptr); + return grpc_insecure_channel_create(bootstrap.server().server_uri.c_str(), + &args, nullptr); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/xds/xds_channel_secure.cc b/src/core/ext/filters/client_channel/xds/xds_channel_secure.cc index 9a752fe5826..56a55096de7 100644 --- a/src/core/ext/filters/client_channel/xds/xds_channel_secure.cc +++ b/src/core/ext/filters/client_channel/xds/xds_channel_secure.cc @@ -69,12 +69,10 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap, RefCountedPtr creds_to_unref; if (!bootstrap.server().channel_creds.empty()) { for (size_t i = 0; i < bootstrap.server().channel_creds.size(); ++i) { - if (strcmp(bootstrap.server().channel_creds[i].type, "google_default") == - 0) { + if (bootstrap.server().channel_creds[i].type == "google_default") { creds = grpc_google_default_credentials_create(); break; - } else if (strcmp(bootstrap.server().channel_creds[i].type, "fake") == - 0) { + } else if (bootstrap.server().channel_creds[i].type == "fake") { creds = grpc_fake_transport_security_credentials_create(); break; } @@ -85,15 +83,15 @@ grpc_channel* CreateXdsChannel(const XdsBootstrap& bootstrap, creds = grpc_channel_credentials_find_in_args(&args); if (creds == nullptr) { // Built with security but parent channel is insecure. - return grpc_insecure_channel_create(bootstrap.server().server_uri, &args, - nullptr); + return grpc_insecure_channel_create(bootstrap.server().server_uri.c_str(), + &args, nullptr); } } const char* arg_to_remove = GRPC_ARG_CHANNEL_CREDENTIALS; grpc_channel_args* new_args = grpc_channel_args_copy_and_remove(&args, &arg_to_remove, 1); grpc_channel* channel = grpc_secure_channel_create( - creds, bootstrap.server().server_uri, new_args, nullptr); + creds, bootstrap.server().server_uri.c_str(), new_args, nullptr); grpc_channel_args_destroy(new_args); return channel; } diff --git a/src/core/ext/filters/client_channel/xds/xds_client.cc b/src/core/ext/filters/client_channel/xds/xds_client.cc index 7e3c7ba04ec..ad3fcad37c9 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.cc +++ b/src/core/ext/filters/client_channel/xds/xds_client.cc @@ -1735,7 +1735,7 @@ XdsClient::XdsClient(Combiner* combiner, grpc_pollset_set* interested_parties, } if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) { gpr_log(GPR_INFO, "[xds_client %p: creating channel to %s", this, - bootstrap_->server().server_uri); + bootstrap_->server().server_uri.c_str()); } chand_ = MakeOrphanable( Ref(DEBUG_LOCATION, "XdsClient+ChannelState"), channel_args); diff --git a/src/core/lib/iomgr/load_file.cc b/src/core/lib/iomgr/load_file.cc index f6431d0f1ca..4bed6275dca 100644 --- a/src/core/lib/iomgr/load_file.cc +++ b/src/core/lib/iomgr/load_file.cc @@ -53,6 +53,7 @@ grpc_error* grpc_load_file(const char* filename, int add_null_terminator, gpr_malloc(contents_size + (add_null_terminator ? 1 : 0))); bytes_read = fread(contents, 1, contents_size, file); if (bytes_read < contents_size) { + gpr_free(contents); error = GRPC_OS_ERROR(errno, "fread"); GPR_ASSERT(ferror(file)); goto end; diff --git a/src/core/lib/json/json.h b/src/core/lib/json/json.h index de3c2aa1a50..dfa6eb02ae3 100644 --- a/src/core/lib/json/json.h +++ b/src/core/lib/json/json.h @@ -163,6 +163,7 @@ class Json { // Accessor methods. Type type() const { return type_; } const std::string& string_value() const { return string_value_; } + std::string* mutable_string_value() { return &string_value_; } const Object& object_value() const { return object_value_; } Object* mutable_object() { return &object_value_; } const Array& array_value() const { return array_value_; } diff --git a/test/core/client_channel/xds_bootstrap_test.cc b/test/core/client_channel/xds_bootstrap_test.cc index fa35a68b720..071e519a31d 100644 --- a/test/core/client_channel/xds_bootstrap_test.cc +++ b/test/core/client_channel/xds_bootstrap_test.cc @@ -36,7 +36,7 @@ void VerifyRegexMatch(grpc_error* error, const std::regex& e) { } TEST(XdsBootstrapTest, Basic) { - const char* json = + const char* json_str = "{" " \"xds_servers\": [" " {" @@ -70,99 +70,46 @@ TEST(XdsBootstrapTest, Basic) { " \"ignore\": {}" " }," " \"metadata\": {" - " \"null\": null," - " \"string\": \"quux\"," - " \"double\": 123.4," - " \"bool\": true," - " \"struct\": {" - " \"whee\": 0" - " }," - " \"list\": [1, 2, 3]" + " \"foo\": 1," + " \"bar\": 2" " }," " \"ignore\": \"whee\"" " }," " \"ignore\": {}" "}"; - grpc_slice slice = grpc_slice_from_copied_string(json); grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); - EXPECT_STREQ(bootstrap.server().server_uri, "fake:///lb"); + EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); ASSERT_EQ(bootstrap.server().channel_creds.size(), 1); - EXPECT_STREQ(bootstrap.server().channel_creds[0].type, "fake"); - EXPECT_EQ(bootstrap.server().channel_creds[0].config, nullptr); + EXPECT_EQ(bootstrap.server().channel_creds[0].type, "fake"); + EXPECT_EQ(bootstrap.server().channel_creds[0].config.type(), + Json::Type::JSON_NULL); ASSERT_NE(bootstrap.node(), nullptr); - EXPECT_STREQ(bootstrap.node()->id, "foo"); - EXPECT_STREQ(bootstrap.node()->cluster, "bar"); - EXPECT_STREQ(bootstrap.node()->locality_region, "milky_way"); - EXPECT_STREQ(bootstrap.node()->locality_zone, "sol_system"); - EXPECT_STREQ(bootstrap.node()->locality_subzone, "earth"); - EXPECT_THAT( - bootstrap.node()->metadata, - ::testing::ElementsAre( - ::testing::Pair( - ::testing::StrEq("bool"), - ::testing::AllOf( - ::testing::Field(&XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::BOOL), - ::testing::Field(&XdsBootstrap::MetadataValue::bool_value, - true))), - ::testing::Pair( - ::testing::StrEq("double"), - ::testing::AllOf( - ::testing::Field(&XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::DOUBLE), - ::testing::Field(&XdsBootstrap::MetadataValue::double_value, - 123.4))), - ::testing::Pair( - ::testing::StrEq("list"), - ::testing::Field(&XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::LIST)), - ::testing::Pair(::testing::StrEq("null"), - ::testing::AllOf(::testing::Field( - &XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::MD_NULL))), - ::testing::Pair( - ::testing::StrEq("string"), - ::testing::AllOf( - ::testing::Field(&XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::STRING), - ::testing::Field(&XdsBootstrap::MetadataValue::string_value, - ::testing::StrEq("quux")))), - ::testing::Pair( - ::testing::StrEq("struct"), - ::testing::AllOf( - ::testing::Field(&XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::STRUCT), - ::testing::Field( - &XdsBootstrap::MetadataValue::struct_value, - ::testing::ElementsAre(::testing::Pair( - ::testing::StrEq("whee"), - ::testing::AllOf( - ::testing::Field( - &XdsBootstrap::MetadataValue::type, - XdsBootstrap::MetadataValue::Type::DOUBLE), - ::testing::Field( - &XdsBootstrap::MetadataValue::double_value, - 0))))))))); - // TODO(roth): Once our InlinedVector<> implementation supports - // iteration, replace this by using ElementsAre() in the statement above. - auto it = bootstrap.node()->metadata.find("list"); - ASSERT_TRUE(it != bootstrap.node()->metadata.end()); - ASSERT_EQ(it->second.list_value.size(), 3); - EXPECT_EQ(it->second.list_value[0].type, - XdsBootstrap::MetadataValue::Type::DOUBLE); - EXPECT_EQ(it->second.list_value[0].double_value, 1); - EXPECT_EQ(it->second.list_value[1].type, - XdsBootstrap::MetadataValue::Type::DOUBLE); - EXPECT_EQ(it->second.list_value[1].double_value, 2); - EXPECT_EQ(it->second.list_value[2].type, - XdsBootstrap::MetadataValue::Type::DOUBLE); - EXPECT_EQ(it->second.list_value[2].double_value, 3); + EXPECT_EQ(bootstrap.node()->id, "foo"); + EXPECT_EQ(bootstrap.node()->cluster, "bar"); + EXPECT_EQ(bootstrap.node()->locality_region, "milky_way"); + EXPECT_EQ(bootstrap.node()->locality_zone, "sol_system"); + EXPECT_EQ(bootstrap.node()->locality_subzone, "earth"); + ASSERT_EQ(bootstrap.node()->metadata.type(), Json::Type::OBJECT); + EXPECT_THAT(bootstrap.node()->metadata.object_value(), + ::testing::ElementsAre( + ::testing::Pair( + ::testing::Eq("bar"), + ::testing::AllOf( + ::testing::Property(&Json::type, Json::Type::NUMBER), + ::testing::Property(&Json::string_value, "2"))), + ::testing::Pair( + ::testing::Eq("foo"), + ::testing::AllOf( + ::testing::Property(&Json::type, Json::Type::NUMBER), + ::testing::Property(&Json::string_value, "1"))))); } TEST(XdsBootstrapTest, ValidWithoutChannelCredsAndNode) { - const char* json = + const char* json_str = "{" " \"xds_servers\": [" " {" @@ -170,93 +117,89 @@ TEST(XdsBootstrapTest, ValidWithoutChannelCredsAndNode) { " }" " ]" "}"; - grpc_slice slice = grpc_slice_from_copied_string(json); grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); - EXPECT_EQ(error, GRPC_ERROR_NONE); - EXPECT_STREQ(bootstrap.server().server_uri, "fake:///lb"); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); + EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + EXPECT_EQ(bootstrap.server().server_uri, "fake:///lb"); EXPECT_EQ(bootstrap.server().channel_creds.size(), 0); EXPECT_EQ(bootstrap.node(), nullptr); } -TEST(XdsBootstrapTest, InvalidJson) { - grpc_slice slice = grpc_slice_from_copied_string(""); - grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); - gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); - ASSERT_TRUE(error != GRPC_ERROR_NONE); - std::regex e(std::string("failed to parse bootstrap file JSON")); - VerifyRegexMatch(error, e); -} - -TEST(XdsBootstrapTest, MalformedJson) { - grpc_slice slice = grpc_slice_from_copied_string("\"foo\""); +TEST(XdsBootstrapTest, MissingXdsServers) { grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse("{}", &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); - std::regex e(std::string("malformed JSON in bootstrap file")); + std::regex e(std::string("\"xds_servers\" field not present")); VerifyRegexMatch(error, e); } -TEST(XdsBootstrapTest, MissingXdsServers) { - grpc_slice slice = grpc_slice_from_copied_string("{}"); +TEST(XdsBootstrapTest, TopFieldsWrongTypes) { + const char* json_str = + "{" + " \"xds_servers\":1," + " \"node\":1" + "}"; grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); - std::regex e(std::string("\"xds_servers\" field not present")); + std::regex e( + std::string("\"xds_servers\" field is not an array(.*)" + "\"node\" field is not an object")); VerifyRegexMatch(error, e); } -TEST(XdsBootstrapTest, BadXdsServers) { - grpc_slice slice = grpc_slice_from_copied_string( +TEST(XdsBootstrapTest, XdsServerMissingServerUri) { + const char* json_str = "{" - " \"xds_servers\":1," " \"xds_servers\":[{}]" - "}"); + "}"; grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("\"xds_servers\" field is not an array(.*)" - "duplicate \"xds_servers\" field(.*)" - "errors parsing \"xds_servers\" array(.*)" + std::string("errors parsing \"xds_servers\" array(.*)" "errors parsing index 0(.*)" "\"server_uri\" field not present")); VerifyRegexMatch(error, e); } -TEST(XdsBootstrapTest, BadXdsServerContents) { - grpc_slice slice = grpc_slice_from_copied_string( +TEST(XdsBootstrapTest, XdsServerUriAndCredsWrongTypes) { + const char* json_str = "{" " \"xds_servers\":[" " {" " \"server_uri\":1," - " \"server_uri\":\"foo\"," - " \"channel_creds\":1," - " \"channel_creds\":{}" + " \"channel_creds\":1" " }" " ]" - "}"); + "}"; grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( std::string("errors parsing \"xds_servers\" array(.*)" "errors parsing index 0(.*)" "\"server_uri\" field is not a string(.*)" - "duplicate \"server_uri\" field(.*)" - "\"channel_creds\" field is not an array(.*)" - "\"channel_creds\" field is not an array(.*)" - "duplicate \"channel_creds\" field(.*)")); + "\"channel_creds\" field is not an array")); VerifyRegexMatch(error, e); } -TEST(XdsBootstrapTest, BadChannelCredsContents) { - grpc_slice slice = grpc_slice_from_copied_string( +TEST(XdsBootstrapTest, ChannelCredsFieldsWrongTypes) { + const char* json_str = "{" " \"xds_servers\":[" " {" @@ -264,16 +207,16 @@ TEST(XdsBootstrapTest, BadChannelCredsContents) { " \"channel_creds\":[" " {" " \"type\":0," - " \"type\":\"fake\"," - " \"config\":1," - " \"config\":{}" + " \"config\":1" " }" " ]" " }" " ]" - "}"); + "}"; grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( @@ -282,80 +225,61 @@ TEST(XdsBootstrapTest, BadChannelCredsContents) { "errors parsing \"channel_creds\" array(.*)" "errors parsing index 0(.*)" "\"type\" field is not a string(.*)" - "duplicate \"type\" field(.*)" - "\"config\" field is not an object(.*)" - "duplicate \"config\" field")); + "\"config\" field is not an object")); VerifyRegexMatch(error, e); } -// under TSAN, ASAN and UBSAN, bazel RBE can suffer from a std::regex -// stackoverflow bug if the analyzed string is too long (> ~2000 characters). As -// this test is only single-thread and deterministic, it is safe to just disable -// it under TSAN and ASAN until -// https://github.com/GoogleCloudPlatform/layer-definitions/issues/591 -// is resolved. The risk for UBSAN problem also doesn't seem to be very high. -#ifndef GRPC_ASAN -#ifndef GRPC_TSAN -#ifndef GRPC_UBSAN - -TEST(XdsBootstrapTest, BadNode) { - grpc_slice slice = grpc_slice_from_copied_string( +TEST(XdsBootstrapTest, NodeFieldsWrongTypes) { + const char* json_str = "{" - " \"node\":1," " \"node\":{" " \"id\":0," - " \"id\":\"foo\"," " \"cluster\":0," - " \"cluster\":\"foo\"," " \"locality\":0," + " \"metadata\":0" + " }" + "}"; + grpc_error* error = GRPC_ERROR_NONE; + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); + gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); + ASSERT_TRUE(error != GRPC_ERROR_NONE); + std::regex e( + std::string("errors parsing \"node\" object(.*)" + "\"id\" field is not a string(.*)" + "\"cluster\" field is not a string(.*)" + "\"locality\" field is not an object(.*)" + "\"metadata\" field is not an object")); + VerifyRegexMatch(error, e); +} + +TEST(XdsBootstrapTest, LocalityFieldsWrongType) { + const char* json_str = + "{" + " \"node\":{" " \"locality\":{" " \"region\":0," - " \"region\":\"foo\"," " \"zone\":0," - " \"zone\":\"foo\"," - " \"subzone\":0," - " \"subzone\":\"foo\"" - " }," - " \"metadata\":0," - " \"metadata\":{" - " \"foo\":0," - " \"foo\":\"whee\"," - " \"foo\":\"whee2\"" + " \"subzone\":0" " }" " }" - "}"); + "}"; grpc_error* error = GRPC_ERROR_NONE; - grpc_core::XdsBootstrap bootstrap(slice, &error); + Json json = Json::Parse(json_str, &error); + ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_string(error); + grpc_core::XdsBootstrap bootstrap(std::move(json), &error); gpr_log(GPR_ERROR, "%s", grpc_error_string(error)); ASSERT_TRUE(error != GRPC_ERROR_NONE); std::regex e( - std::string("\"node\" field is not an object(.*)" - "duplicate \"node\" field(.*)" - "errors parsing \"node\" object(.*)" - "\"id\" field is not a string(.*)" - "duplicate \"id\" field(.*)" - "\"cluster\" field is not a string(.*)" - "duplicate \"cluster\" field(.*)" - "\"locality\" field is not an object(.*)" - "duplicate \"locality\" field(.*)" + std::string("errors parsing \"node\" object(.*)" "errors parsing \"locality\" object(.*)" "\"region\" field is not a string(.*)" - "duplicate \"region\" field(.*)" "\"zone\" field is not a string(.*)" - "duplicate \"zone\" field(.*)" - "\"subzone\" field is not a string(.*)" - "duplicate \"subzone\" field(.*)" - "\"metadata\" field is not an object(.*)" - "duplicate \"metadata\" field(.*)" - "errors parsing \"metadata\" object(.*)" - "duplicate metadata key \"foo\"")); + "\"subzone\" field is not a string")); VerifyRegexMatch(error, e); } -#endif -#endif -#endif - } // namespace testing } // namespace grpc_core