From 52d687ad42ae14cf4b4e76f9909942ab5254ba18 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 25 May 2023 12:59:21 -0700 Subject: [PATCH] [xDS] second attempt: clean up cert provider factory and registry APIs (#33249) Original was #33226, reverted in #33248. --- CMakeLists.txt | 39 -- build_autogenerated.yaml | 17 +- src/core/BUILD | 26 +- .../ext/xds/certificate_provider_store.cc | 10 +- src/core/ext/xds/certificate_provider_store.h | 2 +- ...le_watcher_certificate_provider_factory.cc | 72 ++-- ...ile_watcher_certificate_provider_factory.h | 23 +- ...le_mesh_ca_certificate_provider_factory.cc | 258 ------------ ...gle_mesh_ca_certificate_provider_factory.h | 115 ------ .../certificate_provider_factory.h | 12 +- .../certificate_provider_registry.cc | 26 +- .../certificate_provider_registry.h | 22 +- .../certificate_provider_registry_test.cc | 10 +- test/core/xds/BUILD | 23 +- .../xds/certificate_provider_store_test.cc | 23 +- ...tcher_certificate_provider_factory_test.cc | 151 ++++--- ...sh_ca_certificate_provider_factory_test.cc | 367 ------------------ test/core/xds/xds_bootstrap_test.cc | 55 +-- test/cpp/end2end/xds/xds_end2end_test.cc | 18 +- tools/run_tests/generated/tests.json | 32 +- 20 files changed, 217 insertions(+), 1084 deletions(-) delete mode 100644 src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc delete mode 100644 src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h delete mode 100644 test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d00479153e..8de26a55fff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -980,7 +980,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx generic_end2end_test) add_dependencies(buildtests_cxx goaway_server_test) add_dependencies(buildtests_cxx google_c2p_resolver_test) - add_dependencies(buildtests_cxx google_mesh_ca_certificate_provider_factory_test) add_dependencies(buildtests_cxx graceful_shutdown_test) add_dependencies(buildtests_cxx grpc_alts_credentials_options_test) add_dependencies(buildtests_cxx grpc_audit_logging_test) @@ -11944,44 +11943,6 @@ target_link_libraries(google_c2p_resolver_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(google_mesh_ca_certificate_provider_factory_test - src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc - test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc - third_party/googletest/googletest/src/gtest-all.cc - third_party/googletest/googlemock/src/gmock-all.cc -) -target_compile_features(google_mesh_ca_certificate_provider_factory_test PUBLIC cxx_std_14) -target_include_directories(google_mesh_ca_certificate_provider_factory_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} - third_party/googletest/googletest/include - third_party/googletest/googletest - third_party/googletest/googlemock/include - third_party/googletest/googlemock - ${_gRPC_PROTO_GENS_DIR} -) - -target_link_libraries(google_mesh_ca_certificate_provider_factory_test - ${_gRPC_BASELIB_LIBRARIES} - ${_gRPC_PROTOBUF_LIBRARIES} - ${_gRPC_ZLIB_LIBRARIES} - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 9e6634b13b3..42a33a4ff24 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -3694,6 +3694,7 @@ libs: - src/core/lib/iomgr/wakeup_fd_pipe.h - src/core/lib/iomgr/wakeup_fd_posix.h - src/core/lib/json/json.h + - src/core/lib/json/json_args.h - src/core/lib/json/json_reader.h - src/core/lib/json/json_writer.h - src/core/lib/load_balancing/lb_policy.h @@ -5465,6 +5466,7 @@ targets: - test/core/xds/certificate_provider_store_test.cc deps: - grpc_test_util + uses_polling: false - name: cf_engine_test gtest: true build: test @@ -7110,6 +7112,7 @@ targets: - test/core/xds/file_watcher_certificate_provider_factory_test.cc deps: - grpc_test_util + uses_polling: false - name: filter_end2end_test gtest: true build: test @@ -7635,6 +7638,7 @@ targets: - src/core/lib/iomgr/wakeup_fd_pipe.h - src/core/lib/iomgr/wakeup_fd_posix.h - src/core/lib/json/json.h + - src/core/lib/json/json_args.h - src/core/lib/json/json_writer.h - src/core/lib/load_balancing/lb_policy.h - src/core/lib/load_balancing/lb_policy_factory.h @@ -8040,17 +8044,6 @@ targets: - test/core/util/fake_udp_and_tcp_server.cc deps: - grpc++_test_util -- name: google_mesh_ca_certificate_provider_factory_test - gtest: true - build: test - language: c++ - headers: - - src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h - src: - - src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc - - test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc - deps: - - grpc_test_util - name: graceful_shutdown_test gtest: true build: test @@ -13000,6 +12993,7 @@ targets: - test/core/xds/xds_bootstrap_test.cc deps: - grpc_test_util + uses_polling: false - name: xds_certificate_provider_test gtest: true build: test @@ -13009,6 +13003,7 @@ targets: - test/core/xds/xds_certificate_provider_test.cc deps: - grpc_test_util + uses_polling: false - name: xds_client_test gtest: true build: test diff --git a/src/core/BUILD b/src/core/BUILD index c09f3a86c0a..de30487abc7 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -2771,10 +2771,12 @@ grpc_cc_library( hdrs = [ "lib/security/certificate_provider/certificate_provider_factory.h", ], + external_deps = ["absl/strings"], deps = [ - "error", "json", + "json_args", "ref_counted", + "validation_errors", "//:alts_util", "//:gpr", "//:ref_counted_ptr", @@ -4245,28 +4247,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "grpc_google_mesh_ca_certificate_provider_factory", - srcs = [ - "ext/xds/google_mesh_ca_certificate_provider_factory.cc", - ], - hdrs = [ - "ext/xds/google_mesh_ca_certificate_provider_factory.h", - ], - language = "c++", - deps = [ - "certificate_provider_factory", - "error", - "grpc_tls_credentials", - "json", - "json_util", - "time", - "//:alts_util", - "//:gpr_platform", - "//:ref_counted_ptr", - ], -) - grpc_cc_library( name = "grpc_lb_policy_cds", srcs = [ diff --git a/src/core/ext/xds/certificate_provider_store.cc b/src/core/ext/xds/certificate_provider_store.cc index 1327e5a094d..b4a13067707 100644 --- a/src/core/ext/xds/certificate_provider_store.cc +++ b/src/core/ext/xds/certificate_provider_store.cc @@ -26,8 +26,6 @@ #include #include "src/core/lib/config/core_configuration.h" -#include "src/core/lib/gprpp/status_helper.h" -#include "src/core/lib/iomgr/error.h" #include "src/core/lib/security/certificate_provider/certificate_provider_registry.h" namespace grpc_core { @@ -46,7 +44,7 @@ CertificateProviderStore::PluginDefinition::JsonLoader(const JsonArgs&) { } void CertificateProviderStore::PluginDefinition::JsonPostLoad( - const Json& json, const JsonArgs&, ValidationErrors* errors) { + const Json& json, const JsonArgs& args, ValidationErrors* errors) { // Check that plugin is supported. CertificateProviderFactory* factory = nullptr; if (!plugin_name.empty()) { @@ -76,12 +74,8 @@ void CertificateProviderStore::PluginDefinition::JsonPostLoad( } if (factory == nullptr) return; // Use plugin to validate and parse config. - grpc_error_handle parse_error; config = factory->CreateCertificateProviderConfig( - Json::FromObject(std::move(config_json)), &parse_error); - if (!parse_error.ok()) { - errors->AddError(StatusToString(parse_error)); - } + Json::FromObject(std::move(config_json)), args, errors); } } diff --git a/src/core/ext/xds/certificate_provider_store.h b/src/core/ext/xds/certificate_provider_store.h index cee6e6e0a0a..24b172ac32d 100644 --- a/src/core/ext/xds/certificate_provider_store.h +++ b/src/core/ext/xds/certificate_provider_store.h @@ -55,7 +55,7 @@ class CertificateProviderStore RefCountedPtr config; static const JsonLoaderInterface* JsonLoader(const JsonArgs&); - void JsonPostLoad(const Json& json, const JsonArgs&, + void JsonPostLoad(const Json& json, const JsonArgs& args, ValidationErrors* errors); }; diff --git a/src/core/ext/xds/file_watcher_certificate_provider_factory.cc b/src/core/ext/xds/file_watcher_certificate_provider_factory.cc index fd76d83bd93..8053fc6e470 100644 --- a/src/core/ext/xds/file_watcher_certificate_provider_factory.cc +++ b/src/core/ext/xds/file_watcher_certificate_provider_factory.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -32,14 +33,13 @@ #include #include "src/core/lib/config/core_configuration.h" -#include "src/core/lib/json/json_util.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" namespace grpc_core { namespace { -const char* kFileWatcherPlugin = "file_watcher"; +constexpr absl::string_view kFileWatcherPlugin = "file_watcher"; } // namespace @@ -47,7 +47,7 @@ const char* kFileWatcherPlugin = "file_watcher"; // FileWatcherCertificateProviderFactory::Config // -const char* FileWatcherCertificateProviderFactory::Config::name() const { +absl::string_view FileWatcherCertificateProviderFactory::Config::name() const { return kFileWatcherPlugin; } @@ -71,58 +71,46 @@ std::string FileWatcherCertificateProviderFactory::Config::ToString() const { return absl::StrJoin(parts, ""); } -RefCountedPtr -FileWatcherCertificateProviderFactory::Config::Parse(const Json& config_json, - grpc_error_handle* error) { - auto config = MakeRefCounted(); - if (config_json.type() != Json::Type::kObject) { - *error = GRPC_ERROR_CREATE("error:config type should be OBJECT."); - return nullptr; - } - std::vector error_list; - ParseJsonObjectField(config_json.object(), "certificate_file", - &config->identity_cert_file_, &error_list, false); - ParseJsonObjectField(config_json.object(), "private_key_file", - &config->private_key_file_, &error_list, false); - if (config->identity_cert_file_.empty() != - config->private_key_file_.empty()) { - error_list.push_back(GRPC_ERROR_CREATE( +const JsonLoaderInterface* +FileWatcherCertificateProviderFactory::Config::JsonLoader(const JsonArgs&) { + static const auto* loader = + JsonObjectLoader() + .OptionalField("certificate_file", &Config::identity_cert_file_) + .OptionalField("private_key_file", &Config::private_key_file_) + .OptionalField("ca_certificate_file", &Config::root_cert_file_) + .OptionalField("refresh_interval", &Config::refresh_interval_) + .Finish(); + return loader; +} + +void FileWatcherCertificateProviderFactory::Config::JsonPostLoad( + const Json& json, const JsonArgs& /*args*/, ValidationErrors* errors) { + if ((json.object().find("certificate_file") == json.object().end()) != + (json.object().find("private_key_file") == json.object().end())) { + errors->AddError( "fields \"certificate_file\" and \"private_key_file\" must be both set " - "or both unset.")); + "or both unset"); } - ParseJsonObjectField(config_json.object(), "ca_certificate_file", - &config->root_cert_file_, &error_list, false); - if (config->identity_cert_file_.empty() && config->root_cert_file_.empty()) { - error_list.push_back(GRPC_ERROR_CREATE( - "At least one of \"certificate_file\" and \"ca_certificate_file\" must " - "be specified.")); - } - if (!ParseJsonObjectFieldAsDuration(config_json.object(), "refresh_interval", - &config->refresh_interval_, &error_list, - false)) { - config->refresh_interval_ = Duration::Minutes(10); // 10 minutes default - } - if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR( - "Error parsing file watcher certificate provider config", &error_list); - return nullptr; + if ((json.object().find("certificate_file") == json.object().end()) && + (json.object().find("ca_certificate_file") == json.object().end())) { + errors->AddError( + "at least one of \"certificate_file\" and \"ca_certificate_file\" must " + "be specified"); } - return config; } // // FileWatcherCertificateProviderFactory // -const char* FileWatcherCertificateProviderFactory::name() const { +absl::string_view FileWatcherCertificateProviderFactory::name() const { return kFileWatcherPlugin; } RefCountedPtr FileWatcherCertificateProviderFactory::CreateCertificateProviderConfig( - const Json& config_json, grpc_error_handle* error) { - return FileWatcherCertificateProviderFactory::Config::Parse(config_json, - error); + const Json& config_json, const JsonArgs& args, ValidationErrors* errors) { + return LoadFromJson>(config_json, args, errors); } RefCountedPtr @@ -130,7 +118,7 @@ FileWatcherCertificateProviderFactory::CreateCertificateProvider( RefCountedPtr config) { if (config->name() != name()) { gpr_log(GPR_ERROR, "Wrong config type Actual:%s vs Expected:%s", - config->name(), name()); + std::string(config->name()).c_str(), std::string(name()).c_str()); return nullptr; } auto* file_watcher_config = diff --git a/src/core/ext/xds/file_watcher_certificate_provider_factory.h b/src/core/ext/xds/file_watcher_certificate_provider_factory.h index 5e3b8dc10e2..7dd552c7c64 100644 --- a/src/core/ext/xds/file_watcher_certificate_provider_factory.h +++ b/src/core/ext/xds/file_watcher_certificate_provider_factory.h @@ -23,12 +23,16 @@ #include +#include "absl/strings/string_view.h" + #include #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" +#include "src/core/lib/json/json_args.h" +#include "src/core/lib/json/json_object_loader.h" #include "src/core/lib/security/certificate_provider/certificate_provider_factory.h" namespace grpc_core { @@ -38,10 +42,7 @@ class FileWatcherCertificateProviderFactory public: class Config : public CertificateProviderFactory::Config { public: - static RefCountedPtr Parse(const Json& config_json, - grpc_error_handle* error); - - const char* name() const override; + absl::string_view name() const override; std::string ToString() const override; @@ -55,18 +56,22 @@ class FileWatcherCertificateProviderFactory Duration refresh_interval() const { return refresh_interval_; } + static const JsonLoaderInterface* JsonLoader(const JsonArgs& args); + void JsonPostLoad(const Json& json, const JsonArgs& args, + ValidationErrors* errors); + private: std::string identity_cert_file_; std::string private_key_file_; std::string root_cert_file_; - Duration refresh_interval_; + Duration refresh_interval_ = Duration::Minutes(10); }; - const char* name() const override; + absl::string_view name() const override; RefCountedPtr - CreateCertificateProviderConfig(const Json& config_json, - grpc_error_handle* error) override; + CreateCertificateProviderConfig(const Json& config_json, const JsonArgs& args, + ValidationErrors* errors) override; RefCountedPtr CreateCertificateProvider( RefCountedPtr config) override; diff --git a/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc b/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc deleted file mode 100644 index 97ef65ef056..00000000000 --- a/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.cc +++ /dev/null @@ -1,258 +0,0 @@ -// -// -// Copyright 2020 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// - -#include - -#include "src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h" - -#include - -#include "src/core/lib/iomgr/error.h" -#include "src/core/lib/json/json_util.h" - -namespace grpc_core { - -namespace { - -const char* kMeshCaPlugin = "meshCA"; - -} // namespace - -// -// GoogleMeshCaCertificateProviderFactory::Config -// - -const char* GoogleMeshCaCertificateProviderFactory::Config::name() const { - return kMeshCaPlugin; -} - -std::string GoogleMeshCaCertificateProviderFactory::Config::ToString() const { - // TODO(yashykt): To be filled - return "{}"; -} - -std::vector -GoogleMeshCaCertificateProviderFactory::Config::ParseJsonObjectStsService( - const Json::Object& sts_service) { - std::vector error_list_sts_service; - if (!ParseJsonObjectField(sts_service, "token_exchange_service_uri", - &sts_config_.token_exchange_service_uri, - &error_list_sts_service, false)) { - sts_config_.token_exchange_service_uri = - "securetoken.googleapis.com"; // default - } - ParseJsonObjectField(sts_service, "resource", &sts_config_.resource, - &error_list_sts_service, false); - ParseJsonObjectField(sts_service, "audience", &sts_config_.audience, - &error_list_sts_service, false); - if (!ParseJsonObjectField(sts_service, "scope", &sts_config_.scope, - &error_list_sts_service, false)) { - sts_config_.scope = - "https://www.googleapis.com/auth/cloud-platform"; // default - } - ParseJsonObjectField(sts_service, "requested_token_type", - &sts_config_.requested_token_type, - &error_list_sts_service, false); - ParseJsonObjectField(sts_service, "subject_token_path", - &sts_config_.subject_token_path, - &error_list_sts_service); - ParseJsonObjectField(sts_service, "subject_token_type", - &sts_config_.subject_token_type, - &error_list_sts_service); - ParseJsonObjectField(sts_service, "actor_token_path", - &sts_config_.actor_token_path, &error_list_sts_service, - false); - ParseJsonObjectField(sts_service, "actor_token_type", - &sts_config_.actor_token_type, &error_list_sts_service, - false); - return error_list_sts_service; -} - -std::vector -GoogleMeshCaCertificateProviderFactory::Config::ParseJsonObjectCallCredentials( - const Json::Object& call_credentials) { - std::vector error_list_call_credentials; - const Json::Object* sts_service = nullptr; - if (ParseJsonObjectField(call_credentials, "sts_service", &sts_service, - &error_list_call_credentials)) { - std::vector error_list_sts_service = - ParseJsonObjectStsService(*sts_service); - if (!error_list_sts_service.empty()) { - error_list_call_credentials.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( - "field:sts_service", &error_list_sts_service)); - } - } - return error_list_call_credentials; -} - -std::vector -GoogleMeshCaCertificateProviderFactory::Config::ParseJsonObjectGoogleGrpc( - const Json::Object& google_grpc) { - std::vector error_list_google_grpc; - if (!ParseJsonObjectField(google_grpc, "target_uri", &endpoint_, - &error_list_google_grpc, false)) { - endpoint_ = "meshca.googleapis.com"; // Default target - } - const Json::Array* call_credentials_array = nullptr; - if (ParseJsonObjectField(google_grpc, "call_credentials", - &call_credentials_array, &error_list_google_grpc)) { - if (call_credentials_array->size() != 1) { - error_list_google_grpc.push_back(GRPC_ERROR_CREATE( - "field:call_credentials error:Need exactly one entry.")); - } else { - const Json::Object* call_credentials = nullptr; - if (ExtractJsonType((*call_credentials_array)[0], "call_credentials[0]", - &call_credentials, &error_list_google_grpc)) { - std::vector error_list_call_credentials = - ParseJsonObjectCallCredentials(*call_credentials); - if (!error_list_call_credentials.empty()) { - error_list_google_grpc.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( - "field:call_credentials", &error_list_call_credentials)); - } - } - } - } - - return error_list_google_grpc; -} - -std::vector -GoogleMeshCaCertificateProviderFactory::Config::ParseJsonObjectGrpcServices( - const Json::Object& grpc_service) { - std::vector error_list_grpc_services; - const Json::Object* google_grpc = nullptr; - if (ParseJsonObjectField(grpc_service, "google_grpc", &google_grpc, - &error_list_grpc_services)) { - std::vector error_list_google_grpc = - ParseJsonObjectGoogleGrpc(*google_grpc); - if (!error_list_google_grpc.empty()) { - error_list_grpc_services.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( - "field:google_grpc", &error_list_google_grpc)); - } - } - if (!ParseJsonObjectFieldAsDuration(grpc_service, "timeout", &timeout_, - &error_list_grpc_services, false)) { - timeout_ = Duration::Seconds(10); // 10sec default - } - return error_list_grpc_services; -} - -std::vector -GoogleMeshCaCertificateProviderFactory::Config::ParseJsonObjectServer( - const Json::Object& server) { - std::vector error_list_server; - std::string api_type; - if (ParseJsonObjectField(server, "api_type", &api_type, &error_list_server, - false)) { - if (api_type != "GRPC") { - error_list_server.push_back( - GRPC_ERROR_CREATE("field:api_type error:Only GRPC is supported")); - } - } - const Json::Array* grpc_services = nullptr; - if (ParseJsonObjectField(server, "grpc_services", &grpc_services, - &error_list_server)) { - if (grpc_services->size() != 1) { - error_list_server.push_back(GRPC_ERROR_CREATE( - "field:grpc_services error:Need exactly one entry")); - } else { - const Json::Object* grpc_service = nullptr; - if (ExtractJsonType((*grpc_services)[0], "grpc_services[0]", - &grpc_service, &error_list_server)) { - std::vector error_list_grpc_services = - ParseJsonObjectGrpcServices(*grpc_service); - if (!error_list_grpc_services.empty()) { - error_list_server.push_back(GRPC_ERROR_CREATE_FROM_VECTOR( - "field:grpc_services", &error_list_grpc_services)); - } - } - } - } - return error_list_server; -} - -RefCountedPtr -GoogleMeshCaCertificateProviderFactory::Config::Parse( - const Json& config_json, grpc_error_handle* error) { - auto config = - MakeRefCounted(); - if (config_json.type() != Json::Type::kObject) { - *error = GRPC_ERROR_CREATE("error:config type should be OBJECT."); - return nullptr; - } - std::vector error_list; - const Json::Object* server = nullptr; - if (ParseJsonObjectField(config_json.object(), "server", &server, - &error_list)) { - std::vector error_list_server = - config->ParseJsonObjectServer(*server); - if (!error_list_server.empty()) { - error_list.push_back( - GRPC_ERROR_CREATE_FROM_VECTOR("field:server", &error_list_server)); - } - } - if (!ParseJsonObjectFieldAsDuration( - config_json.object(), "certificate_lifetime", - &config->certificate_lifetime_, &error_list, false)) { - config->certificate_lifetime_ = Duration::Hours(24); // 24hrs default - } - if (!ParseJsonObjectFieldAsDuration( - config_json.object(), "renewal_grace_period", - &config->renewal_grace_period_, &error_list, false)) { - config->renewal_grace_period_ = Duration::Hours(12); // 12hrs default - } - std::string key_type; - if (ParseJsonObjectField(config_json.object(), "key_type", &key_type, - &error_list, false)) { - if (key_type != "RSA") { - error_list.push_back( - GRPC_ERROR_CREATE("field:key_type error:Only RSA is supported.")); - } - } - if (!ParseJsonObjectField(config_json.object(), "key_size", - &config->key_size_, &error_list, false)) { - config->key_size_ = 2048; // default 2048 bit key size - } - if (!ParseJsonObjectField(config_json.object(), "location", - &config->location_, &error_list, false)) { - // GCE/GKE Metadata server needs to be contacted to get the value. - } - if (!error_list.empty()) { - *error = GRPC_ERROR_CREATE_FROM_VECTOR( - "Error parsing google Mesh CA config", &error_list); - return nullptr; - } - return config; -} - -// -// GoogleMeshCaCertificateProviderFactory -// - -const char* GoogleMeshCaCertificateProviderFactory::name() const { - return kMeshCaPlugin; -} - -RefCountedPtr -GoogleMeshCaCertificateProviderFactory::CreateCertificateProviderConfig( - const Json& config_json, grpc_error_handle* error) { - return GoogleMeshCaCertificateProviderFactory::Config::Parse(config_json, - error); -} - -} // namespace grpc_core diff --git a/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h b/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h deleted file mode 100644 index eb5da1b9dec..00000000000 --- a/src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h +++ /dev/null @@ -1,115 +0,0 @@ -// -// -// Copyright 2020 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// - -#ifndef GRPC_SRC_CORE_EXT_XDS_GOOGLE_MESH_CA_CERTIFICATE_PROVIDER_FACTORY_H -#define GRPC_SRC_CORE_EXT_XDS_GOOGLE_MESH_CA_CERTIFICATE_PROVIDER_FACTORY_H - -#include - -#include - -#include -#include - -#include - -#include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/gprpp/time.h" -#include "src/core/lib/iomgr/error.h" -#include "src/core/lib/json/json.h" -#include "src/core/lib/security/certificate_provider/certificate_provider_factory.h" -#include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" - -namespace grpc_core { - -class GoogleMeshCaCertificateProviderFactory - : public CertificateProviderFactory { - public: - class Config : public CertificateProviderFactory::Config { - public: - struct StsConfig { - std::string token_exchange_service_uri; - std::string resource; - std::string audience; - std::string scope; - std::string requested_token_type; - std::string subject_token_path; - std::string subject_token_type; - std::string actor_token_path; - std::string actor_token_type; - }; - - const char* name() const override; - - std::string ToString() const override; - - const std::string& endpoint() const { return endpoint_; } - - const StsConfig& sts_config() const { return sts_config_; } - - Duration timeout() const { return timeout_; } - - Duration certificate_lifetime() const { return certificate_lifetime_; } - - Duration renewal_grace_period() const { return renewal_grace_period_; } - - uint32_t key_size() const { return key_size_; } - - const std::string& location() const { return location_; } - - static RefCountedPtr Parse(const Json& config_json, - grpc_error_handle* error); - - private: - // Helpers for parsing the config - std::vector ParseJsonObjectStsService( - const Json::Object& sts_service); - std::vector ParseJsonObjectCallCredentials( - const Json::Object& call_credentials); - std::vector ParseJsonObjectGoogleGrpc( - const Json::Object& google_grpc); - std::vector ParseJsonObjectGrpcServices( - const Json::Object& grpc_service); - std::vector ParseJsonObjectServer( - const Json::Object& server); - - std::string endpoint_; - StsConfig sts_config_; - Duration timeout_; - Duration certificate_lifetime_; - Duration renewal_grace_period_; - uint32_t key_size_; - std::string location_; - }; - - const char* name() const override; - - RefCountedPtr - CreateCertificateProviderConfig(const Json& config_json, - grpc_error_handle* error) override; - - RefCountedPtr CreateCertificateProvider( - RefCountedPtr /*config*/) override { - // TODO(yashykt) : To be implemented - return nullptr; - } -}; - -} // namespace grpc_core - -#endif // GRPC_SRC_CORE_EXT_XDS_GOOGLE_MESH_CA_CERTIFICATE_PROVIDER_FACTORY_H diff --git a/src/core/lib/security/certificate_provider/certificate_provider_factory.h b/src/core/lib/security/certificate_provider/certificate_provider_factory.h index fc5691d39f8..08f0e228859 100644 --- a/src/core/lib/security/certificate_provider/certificate_provider_factory.h +++ b/src/core/lib/security/certificate_provider/certificate_provider_factory.h @@ -23,12 +23,15 @@ #include +#include "absl/strings/string_view.h" + #include #include "src/core/lib/gprpp/ref_counted.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" +#include "src/core/lib/json/json_args.h" namespace grpc_core { @@ -43,7 +46,7 @@ class CertificateProviderFactory { // Name of the type of the CertificateProvider. Unique to each type of // config. - virtual const char* name() const = 0; + virtual absl::string_view name() const = 0; virtual std::string ToString() const = 0; }; @@ -51,10 +54,11 @@ class CertificateProviderFactory { virtual ~CertificateProviderFactory() = default; // Name of the plugin. - virtual const char* name() const = 0; + virtual absl::string_view name() const = 0; virtual RefCountedPtr CreateCertificateProviderConfig( - const Json& config_json, grpc_error_handle* error) = 0; + const Json& config_json, const JsonArgs& args, + ValidationErrors* errors) = 0; // Create a CertificateProvider instance from config. virtual RefCountedPtr diff --git a/src/core/lib/security/certificate_provider/certificate_provider_registry.cc b/src/core/lib/security/certificate_provider/certificate_provider_registry.cc index 86bee4bc477..ae29a85ee86 100644 --- a/src/core/lib/security/certificate_provider/certificate_provider_registry.cc +++ b/src/core/lib/security/certificate_provider/certificate_provider_registry.cc @@ -20,11 +20,8 @@ #include "src/core/lib/security/certificate_provider/certificate_provider_registry.h" -#include - -#include +#include #include -#include #include @@ -32,29 +29,22 @@ namespace grpc_core { void CertificateProviderRegistry::Builder::RegisterCertificateProviderFactory( std::unique_ptr factory) { + absl::string_view name = factory->name(); gpr_log(GPR_DEBUG, "registering certificate provider factory for \"%s\"", - factory->name()); - for (size_t i = 0; i < factories_.size(); ++i) { - GPR_ASSERT(strcmp(factories_[i]->name(), factory->name()) != 0); - } - factories_.push_back(std::move(factory)); + std::string(name).c_str()); + GPR_ASSERT(factories_.emplace(name, std::move(factory)).second); } CertificateProviderRegistry CertificateProviderRegistry::Builder::Build() { - CertificateProviderRegistry r; - r.factories_ = std::move(factories_); - return r; + return CertificateProviderRegistry(std::move(factories_)); } CertificateProviderFactory* CertificateProviderRegistry::LookupCertificateProviderFactory( absl::string_view name) const { - for (size_t i = 0; i < factories_.size(); ++i) { - if (name == factories_[i]->name()) { - return factories_[i].get(); - } - } - return nullptr; + auto it = factories_.find(name); + if (it == factories_.end()) return nullptr; + return it->second.get(); } } // namespace grpc_core diff --git a/src/core/lib/security/certificate_provider/certificate_provider_registry.h b/src/core/lib/security/certificate_provider/certificate_provider_registry.h index cf1d0b1accb..5cc05ae2a5e 100644 --- a/src/core/lib/security/certificate_provider/certificate_provider_registry.h +++ b/src/core/lib/security/certificate_provider/certificate_provider_registry.h @@ -21,8 +21,9 @@ #include +#include #include -#include +#include #include "absl/strings/string_view.h" @@ -32,20 +33,24 @@ namespace grpc_core { // Global registry for all the certificate provider plugins. class CertificateProviderRegistry { + private: + using FactoryMap = + std::map>; + public: class Builder { public: - // Register a provider with the registry. Can only be called after calling - // InitRegistry(). The key of the factory is extracted from factory - // parameter with method CertificateProviderFactory::name. If the same key - // is registered twice, an exception is raised. + // Register a provider with the registry. The key of the factory is + // extracted from factory parameter with method + // CertificateProviderFactory::name. The registry with a given name + // cannot be registered twice. void RegisterCertificateProviderFactory( std::unique_ptr factory); CertificateProviderRegistry Build(); private: - std::vector> factories_; + FactoryMap factories_; }; CertificateProviderRegistry(const CertificateProviderRegistry&) = delete; @@ -60,9 +65,10 @@ class CertificateProviderRegistry { absl::string_view name) const; private: - CertificateProviderRegistry() = default; + explicit CertificateProviderRegistry(FactoryMap factories) + : factories_(std::move(factories)) {} - std::vector> factories_; + FactoryMap factories_; }; } // namespace grpc_core diff --git a/test/core/security/certificate_provider_registry_test.cc b/test/core/security/certificate_provider_registry_test.cc index 4aeb4e2bc25..2fbf1cd4210 100644 --- a/test/core/security/certificate_provider_registry_test.cc +++ b/test/core/security/certificate_provider_registry_test.cc @@ -31,10 +31,11 @@ namespace { class FakeCertificateProviderFactory1 : public CertificateProviderFactory { public: - const char* name() const override { return "fake1"; } + absl::string_view name() const override { return "fake1"; } RefCountedPtr CreateCertificateProviderConfig( - const Json& /*config_json*/, grpc_error_handle* /*error*/) override { + const Json& /*config_json*/, const JsonArgs& /*args*/, + ValidationErrors* /*errors*/) override { return nullptr; } @@ -46,10 +47,11 @@ class FakeCertificateProviderFactory1 : public CertificateProviderFactory { class FakeCertificateProviderFactory2 : public CertificateProviderFactory { public: - const char* name() const override { return "fake2"; } + absl::string_view name() const override { return "fake2"; } RefCountedPtr CreateCertificateProviderConfig( - const Json& /*config_json*/, grpc_error_handle* /*error*/) override { + const Json& /*config_json*/, const JsonArgs& /*args*/, + ValidationErrors* /*errors*/) override { return nullptr; } diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index 1e07129a299..6750ed52dae 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -31,6 +31,8 @@ grpc_cc_test( "gtest", ], language = "C++", + uses_event_engine = False, + uses_polling = False, deps = [ "//:gpr", "//src/core:grpc_xds_client", @@ -43,6 +45,8 @@ grpc_cc_test( srcs = ["certificate_provider_store_test.cc"], external_deps = ["gtest"], language = "C++", + uses_event_engine = False, + uses_polling = False, deps = [ "//:gpr", "//:grpc", @@ -55,6 +59,8 @@ grpc_cc_test( srcs = ["file_watcher_certificate_provider_factory_test.cc"], external_deps = ["gtest"], language = "C++", + uses_event_engine = False, + uses_polling = False, deps = [ "//:gpr", "//:grpc", @@ -62,25 +68,14 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "google_mesh_ca_certificate_provider_factory_test", - srcs = ["google_mesh_ca_certificate_provider_factory_test.cc"], - external_deps = ["gtest"], - language = "C++", - deps = [ - "//:gpr", - "//:grpc", - "//src/core:grpc_google_mesh_ca_certificate_provider_factory", - "//test/core/util:grpc_test_util", - ], -) - grpc_cc_test( name = "xds_channel_stack_modifier_test", srcs = ["xds_channel_stack_modifier_test.cc"], external_deps = ["gtest"], language = "C++", tags = ["no_test_ios"], + uses_event_engine = False, + uses_polling = False, deps = [ "//:gpr", "//:grpc", @@ -97,6 +92,8 @@ grpc_cc_test( srcs = ["xds_certificate_provider_test.cc"], external_deps = ["gtest"], language = "C++", + uses_event_engine = False, + uses_polling = False, deps = [ "//:gpr", "//:grpc", diff --git a/test/core/xds/certificate_provider_store_test.cc b/test/core/xds/certificate_provider_store_test.cc index 9340c8ab7ae..53f5767e3bd 100644 --- a/test/core/xds/certificate_provider_store_test.cc +++ b/test/core/xds/certificate_provider_store_test.cc @@ -30,7 +30,6 @@ #include "src/core/lib/config/core_configuration.h" #include "src/core/lib/gprpp/unique_type_name.h" -#include "src/core/lib/iomgr/error.h" #include "test/core/util/test_config.h" namespace grpc_core { @@ -69,16 +68,17 @@ class FakeCertificateProviderFactory1 : public CertificateProviderFactory { public: class Config : public CertificateProviderFactory::Config { public: - const char* name() const override { return "fake1"; } + absl::string_view name() const override { return "fake1"; } std::string ToString() const override { return "{}"; } }; - const char* name() const override { return "fake1"; } + absl::string_view name() const override { return "fake1"; } RefCountedPtr CreateCertificateProviderConfig(const Json& /*config_json*/, - grpc_error_handle* /*error*/) override { + const JsonArgs& /*args*/, + ValidationErrors* /*errors*/) override { return MakeRefCounted(); } @@ -92,16 +92,17 @@ class FakeCertificateProviderFactory2 : public CertificateProviderFactory { public: class Config : public CertificateProviderFactory::Config { public: - const char* name() const override { return "fake2"; } + absl::string_view name() const override { return "fake2"; } std::string ToString() const override { return "{}"; } }; - const char* name() const override { return "fake2"; } + absl::string_view name() const override { return "fake2"; } RefCountedPtr CreateCertificateProviderConfig(const Json& /*config_json*/, - grpc_error_handle* /*error*/) override { + const JsonArgs& /*args*/, + ValidationErrors* /*errors*/) override { return MakeRefCounted(); } @@ -127,13 +128,13 @@ TEST_F(CertificateProviderStoreTest, Basic) { CertificateProviderStore::PluginDefinitionMap map = { {"fake_plugin_1", {"fake1", fake_factory_1->CreateCertificateProviderConfig( - Json::FromObject({}), nullptr)}}, + Json::FromObject({}), JsonArgs(), nullptr)}}, {"fake_plugin_2", {"fake2", fake_factory_2->CreateCertificateProviderConfig( - Json::FromObject({}), nullptr)}}, + Json::FromObject({}), JsonArgs(), nullptr)}}, {"fake_plugin_3", {"fake1", fake_factory_1->CreateCertificateProviderConfig( - Json::FromObject({}), nullptr)}}, + Json::FromObject({}), JsonArgs(), nullptr)}}, }; auto store = MakeOrphanable(std::move(map)); // Test for creating certificate providers with known plugin @@ -175,7 +176,7 @@ TEST_F(CertificateProviderStoreTest, Multithreaded) { CertificateProviderStore::PluginDefinitionMap map = { {"fake_plugin_1", {"fake1", fake_factory_1->CreateCertificateProviderConfig( - Json::FromObject({}), nullptr)}}}; + Json::FromObject({}), JsonArgs(), nullptr)}}}; auto store = MakeOrphanable(std::move(map)); // Test concurrent `CreateOrGetCertificateProvider()` with the same key. std::vector threads; diff --git a/test/core/xds/file_watcher_certificate_provider_factory_test.cc b/test/core/xds/file_watcher_certificate_provider_factory_test.cc index a68f5f82f39..6397f73827d 100644 --- a/test/core/xds/file_watcher_certificate_provider_factory_test.cc +++ b/test/core/xds/file_watcher_certificate_provider_factory_test.cc @@ -19,16 +19,15 @@ #include "src/core/ext/xds/file_watcher_certificate_provider_factory.h" #include +#include #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" -#include "gmock/gmock.h" #include "gtest/gtest.h" #include -#include "src/core/lib/gprpp/status_helper.h" #include "src/core/lib/json/json_reader.h" #include "test/core/util/test_config.h" @@ -41,6 +40,21 @@ const char* kPrivateKeyFile = "/path/to/private_key_file"; const char* kRootCertFile = "/path/to/root_cert_file"; const int kRefreshInterval = 400; +absl::StatusOr> +ParseConfig(absl::string_view json_string) { + auto json = JsonParse(json_string); + if (!json.ok()) return json.status(); + ValidationErrors errors; + auto config = + FileWatcherCertificateProviderFactory().CreateCertificateProviderConfig( + *json, JsonArgs(), &errors); + if (!errors.ok()) { + return errors.status(absl::StatusCode::kInvalidArgument, + "validation errors"); + } + return std::move(config); +} + TEST(FileWatcherConfigTest, Basic) { std::string json_str = absl::StrFormat( "{" @@ -50,16 +64,12 @@ TEST(FileWatcherConfigTest, Basic) { " \"refresh_interval\": \"%ds\"" "}", kIdentityCertFile, kPrivateKeyFile, kRootCertFile, kRefreshInterval); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_EQ(config->identity_cert_file(), kIdentityCertFile); - EXPECT_EQ(config->private_key_file(), kPrivateKeyFile); - EXPECT_EQ(config->root_cert_file(), kRootCertFile); - EXPECT_EQ(config->refresh_interval(), Duration::Seconds(kRefreshInterval)); + auto config = ParseConfig(json_str); + ASSERT_TRUE(config.ok()) << config.status(); + EXPECT_EQ((*config)->identity_cert_file(), kIdentityCertFile); + EXPECT_EQ((*config)->private_key_file(), kPrivateKeyFile); + EXPECT_EQ((*config)->root_cert_file(), kRootCertFile); + EXPECT_EQ((*config)->refresh_interval(), Duration::Seconds(kRefreshInterval)); } TEST(FileWatcherConfigTest, DefaultRefreshInterval) { @@ -70,16 +80,12 @@ TEST(FileWatcherConfigTest, DefaultRefreshInterval) { " \"ca_certificate_file\": \"%s\"" "}", kIdentityCertFile, kPrivateKeyFile, kRootCertFile); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_EQ(config->identity_cert_file(), kIdentityCertFile); - EXPECT_EQ(config->private_key_file(), kPrivateKeyFile); - EXPECT_EQ(config->root_cert_file(), kRootCertFile); - EXPECT_EQ(config->refresh_interval(), Duration::Seconds(600)); + auto config = ParseConfig(json_str); + ASSERT_TRUE(config.ok()) << config.status(); + EXPECT_EQ((*config)->identity_cert_file(), kIdentityCertFile); + EXPECT_EQ((*config)->private_key_file(), kPrivateKeyFile); + EXPECT_EQ((*config)->root_cert_file(), kRootCertFile); + EXPECT_EQ((*config)->refresh_interval(), Duration::Seconds(600)); } TEST(FileWatcherConfigTest, OnlyRootCertificatesFileProvided) { @@ -88,16 +94,12 @@ TEST(FileWatcherConfigTest, OnlyRootCertificatesFileProvided) { " \"ca_certificate_file\": \"%s\"" "}", kRootCertFile); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_TRUE(config->identity_cert_file().empty()); - EXPECT_TRUE(config->private_key_file().empty()); - EXPECT_EQ(config->root_cert_file(), kRootCertFile); - EXPECT_EQ(config->refresh_interval(), Duration::Seconds(600)); + auto config = ParseConfig(json_str); + ASSERT_TRUE(config.ok()) << config.status(); + EXPECT_EQ((*config)->identity_cert_file(), ""); + EXPECT_EQ((*config)->private_key_file(), ""); + EXPECT_EQ((*config)->root_cert_file(), kRootCertFile); + EXPECT_EQ((*config)->refresh_interval(), Duration::Seconds(600)); } TEST(FileWatcherConfigTest, OnlyIdenityCertificatesAndPrivateKeyProvided) { @@ -107,16 +109,12 @@ TEST(FileWatcherConfigTest, OnlyIdenityCertificatesAndPrivateKeyProvided) { " \"private_key_file\": \"%s\"" "}", kIdentityCertFile, kPrivateKeyFile); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_EQ(config->identity_cert_file(), kIdentityCertFile); - EXPECT_EQ(config->private_key_file(), kPrivateKeyFile); - EXPECT_TRUE(config->root_cert_file().empty()); - EXPECT_EQ(config->refresh_interval(), Duration::Seconds(600)); + auto config = ParseConfig(json_str); + ASSERT_TRUE(config.ok()) << config.status(); + EXPECT_EQ((*config)->identity_cert_file(), kIdentityCertFile); + EXPECT_EQ((*config)->private_key_file(), kPrivateKeyFile); + EXPECT_EQ((*config)->root_cert_file(), ""); + EXPECT_EQ((*config)->refresh_interval(), Duration::Seconds(600)); } TEST(FileWatcherConfigTest, WrongTypes) { @@ -127,19 +125,14 @@ TEST(FileWatcherConfigTest, WrongTypes) { " \"ca_certificate_file\": 123," " \"refresh_interval\": 123" "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT(StatusToString(error), - ::testing::ContainsRegex( - "field:certificate_file error:type should be STRING.*" - "field:private_key_file error:type should be STRING.*" - "field:ca_certificate_file error:type should be STRING.*" - "field:refresh_interval error:type should be STRING of the " - "form given by " - "google.proto.Duration.*")); + auto config = ParseConfig(json_str); + EXPECT_EQ(config.status().message(), + "validation errors: [" + "field:ca_certificate_file error:is not a string; " + "field:certificate_file error:is not a string; " + "field:private_key_file error:is not a string; " + "field:refresh_interval error:is not a string]") + << config.status(); } TEST(FileWatcherConfigTest, IdentityCertProvidedButPrivateKeyMissing) { @@ -148,45 +141,37 @@ TEST(FileWatcherConfigTest, IdentityCertProvidedButPrivateKeyMissing) { " \"certificate_file\": \"%s\"" "}", kIdentityCertFile); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT(StatusToString(error), - ::testing::ContainsRegex( - "fields \"certificate_file\" and \"private_key_file\" must " - "be both set or both unset.")); + auto config = ParseConfig(json_str); + EXPECT_EQ(config.status().message(), + "validation errors: [" + "field: error:fields \"certificate_file\" and " + "\"private_key_file\" must be both set or both unset]") + << config.status(); } TEST(FileWatcherConfigTest, PrivateKeyProvidedButIdentityCertMissing) { std::string json_str = absl::StrFormat( "{" + " \"ca_certificate_file\": \"%s\"," " \"private_key_file\": \"%s\"" "}", - kPrivateKeyFile); - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT(StatusToString(error), - ::testing::ContainsRegex( - "fields \"certificate_file\" and \"private_key_file\" must " - "be both set or both unset.")); + kRootCertFile, kPrivateKeyFile); + auto config = ParseConfig(json_str); + EXPECT_EQ(config.status().message(), + "validation errors: [" + "field: error:fields \"certificate_file\" and " + "\"private_key_file\" must be both set or both unset]") + << config.status(); } TEST(FileWatcherConfigTest, EmptyJsonObject) { std::string json_str = "{}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - FileWatcherCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex("At least one of \"certificate_file\" and " - "\"ca_certificate_file\" must be specified.")); + auto config = ParseConfig(json_str); + EXPECT_EQ(config.status().message(), + "validation errors: [" + "field: error:at least one of \"certificate_file\" and " + "\"ca_certificate_file\" must be specified]") + << config.status(); } } // namespace diff --git a/test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc b/test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc deleted file mode 100644 index 0559d83d710..00000000000 --- a/test/core/xds/google_mesh_ca_certificate_provider_factory_test.cc +++ /dev/null @@ -1,367 +0,0 @@ -// -// -// Copyright 2020 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// - -#include "src/core/ext/xds/google_mesh_ca_certificate_provider_factory.h" - -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -#include - -#include "src/core/lib/gprpp/status_helper.h" -#include "src/core/lib/gprpp/time.h" -#include "src/core/lib/json/json_reader.h" -#include "test/core/util/test_config.h" - -namespace grpc_core { -namespace testing { -namespace { - -TEST(GoogleMeshCaConfigTest, Basic) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"target_uri\": \"newmeshca.googleapis.com\"," - " \"channel_credentials\": { \"google_default\": {}}," - " \"call_credentials\": [{" - " \"sts_service\": {" - " \"token_exchange_service_uri\": " - "\"newsecuretoken.googleapis.com\"," - " \"resource\": \"newmeshca.googleapis.com\"," - " \"audience\": \"newmeshca.googleapis.com\"," - " \"scope\": " - "\"https://www.newgoogleapis.com/auth/cloud-platform\"," - " \"requested_token_type\": " - "\"urn:ietf:params:oauth:token-type:jwt\"," - " \"subject_token_path\": \"/etc/secret/sajwt.token\"," - " \"subject_token_type\": " - "\"urn:ietf:params:oauth:token-type:jwt\"," - " \"actor_token_path\": \"/etc/secret/sajwt.token\"," - " \"actor_token_type\": " - "\"urn:ietf:params:oauth:token-type:jwt\"" - " }" - " }]" - " }," - " \"timeout\": \"20s\"" - " }]" - " }," - " \"certificate_lifetime\": \"400s\"," - " \"renewal_grace_period\": \"100s\"," - " \"key_type\": \"RSA\"," - " \"key_size\": 1024," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_EQ(config->endpoint(), "newmeshca.googleapis.com"); - EXPECT_EQ(config->sts_config().token_exchange_service_uri, - "newsecuretoken.googleapis.com"); - EXPECT_EQ(config->sts_config().resource, "newmeshca.googleapis.com"); - EXPECT_EQ(config->sts_config().audience, "newmeshca.googleapis.com"); - EXPECT_EQ(config->sts_config().scope, - "https://www.newgoogleapis.com/auth/cloud-platform"); - EXPECT_EQ(config->sts_config().requested_token_type, - "urn:ietf:params:oauth:token-type:jwt"); - EXPECT_EQ(config->sts_config().subject_token_path, "/etc/secret/sajwt.token"); - EXPECT_EQ(config->sts_config().subject_token_type, - "urn:ietf:params:oauth:token-type:jwt"); - EXPECT_EQ(config->sts_config().actor_token_path, "/etc/secret/sajwt.token"); - EXPECT_EQ(config->sts_config().actor_token_type, - "urn:ietf:params:oauth:token-type:jwt"); - EXPECT_EQ(config->timeout(), Duration::Seconds(20)); - EXPECT_EQ(config->certificate_lifetime(), Duration::Seconds(400)); - EXPECT_EQ(config->renewal_grace_period(), Duration::Seconds(100)); - EXPECT_EQ(config->key_size(), 1024); - EXPECT_EQ(config->location(), - "https://container.googleapis.com/v1/project/test-project1/" - "locations/test-zone2/clusters/test-cluster3"); -} - -TEST(GoogleMeshCaConfigTest, Defaults) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"call_credentials\": [{" - " \"sts_service\": {" - " \"scope\": " - "\"https://www.googleapis.com/auth/cloud-platform\"," - " \"subject_token_path\": \"/etc/secret/sajwt.token\"," - " \"subject_token_type\": " - "\"urn:ietf:params:oauth:token-type:jwt\"" - " }" - " }]" - " }" - " }]" - " }," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - ASSERT_EQ(error, absl::OkStatus()) << StatusToString(error); - EXPECT_EQ(config->endpoint(), "meshca.googleapis.com"); - EXPECT_EQ(config->sts_config().token_exchange_service_uri, - "securetoken.googleapis.com"); - EXPECT_EQ(config->sts_config().resource, ""); - EXPECT_EQ(config->sts_config().audience, ""); - EXPECT_EQ(config->sts_config().scope, - "https://www.googleapis.com/auth/cloud-platform"); - EXPECT_EQ(config->sts_config().requested_token_type, ""); - EXPECT_EQ(config->sts_config().subject_token_path, "/etc/secret/sajwt.token"); - EXPECT_EQ(config->sts_config().subject_token_type, - "urn:ietf:params:oauth:token-type:jwt"); - EXPECT_EQ(config->sts_config().actor_token_path, ""); - EXPECT_EQ(config->sts_config().actor_token_type, ""); - EXPECT_EQ(config->timeout(), Duration::Seconds(10)); - EXPECT_EQ(config->certificate_lifetime(), Duration::Hours(24)); - EXPECT_EQ(config->renewal_grace_period(), Duration::Hours(12)); - EXPECT_EQ(config->key_size(), 2048); - EXPECT_EQ(config->location(), - "https://container.googleapis.com/v1/project/test-project1/" - "locations/test-zone2/clusters/test-cluster3"); -} - -TEST(GoogleMeshCaConfigTest, WrongExpectedValues) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"REST\"," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"call_credentials\": [{" - " \"sts_service\": {" - " \"scope\": " - "\"https://www.googleapis.com/auth/cloud-platform\"," - " \"subject_token_path\": \"/etc/secret/sajwt.token\"," - " \"subject_token_type\": " - "\"urn:ietf:params:oauth:token-type:jwt\"" - " }" - " }]" - " }" - " }]" - " }," - " \"key_type\": \"DSA\"," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex("field:api_type error:Only GRPC is supported.*" - "field:key_type error:Only RSA is supported")); -} - -TEST(GoogleMeshCaConfigTest, WrongTypes) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": 123," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"target_uri\": 123," - " \"call_credentials\": [{" - " \"sts_service\": {" - " \"token_exchange_service_uri\": 123," - " \"resource\": 123," - " \"audience\": 123," - " \"scope\": 123," - " \"requested_token_type\": 123," - " \"subject_token_path\": 123," - " \"subject_token_type\": 123," - " \"actor_token_path\": 123," - " \"actor_token_type\": 123" - " }" - " }]" - " }," - " \"timeout\": 20" - " }]" - " }," - " \"certificate_lifetime\": 400," - " \"renewal_grace_period\": 100," - " \"key_type\": 123," - " \"key_size\": \"1024A\"," - " \"location\": 123" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex( - "field:server.*field:api_type error:type should be STRING.*" - "field:grpc_services.*field:google_grpc.*field:target_uri " - "error:type should be STRING.*" - "field:call_credentials.*field:sts_service.*field:token_exchange_" - "service_uri error:type should be STRING.*" - "field:resource error:type should be STRING.*" - "field:audience error:type should be STRING.*" - "field:scope error:type should be STRING.*" - "field:requested_token_type error:type should be STRING.*" - "field:subject_token_path error:type should be STRING.*" - "field:subject_token_type error:type should be STRING.*" - "field:actor_token_path error:type should be STRING.*" - "field:actor_token_type error:type should be STRING.*" - "field:timeout error:type should be STRING of the form given by " - "google.proto.Duration.*" - "field:certificate_lifetime error:type should be STRING of the form " - "given by google.proto.Duration.*" - "field:renewal_grace_period error:type should be STRING of the form " - "given by google.proto.Duration..*" - "field:key_type error:type should be STRING.*" - "field:key_size error:failed to parse.*" - "field:location error:type should be STRING")); -} - -TEST(GoogleMeshCaConfigTest, GrpcServicesNotAnArray) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": 123" - " }," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex( - "field:server.*field:grpc_services error:type should be ARRAY")); -} - -TEST(GoogleMeshCaConfigTest, GoogleGrpcNotAnObject) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": [{" - " \"google_grpc\": 123" - " }]" - " }," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex("field:server.*field:grpc_services.*field:" - "google_grpc error:type should be OBJECT")); -} - -TEST(GoogleMeshCaConfigTest, CallCredentialsNotAnArray) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"call_credentials\": 123" - " }" - " }]" - " }," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT(StatusToString(error), - ::testing::ContainsRegex( - "field:server.*field:grpc_services.*field:google_grpc.*" - "field:call_credentials error:type should be ARRAY")); -} - -TEST(GoogleMeshCaConfigTest, StsServiceNotAnObject) { - const char* json_str = - "{" - " \"server\": {" - " \"api_type\": \"GRPC\"," - " \"grpc_services\": [{" - " \"google_grpc\": {" - " \"call_credentials\": [{" - " \"sts_service\": 123" - " }]" - " }" - " }]" - " }," - " \"location\": " - "\"https://container.googleapis.com/v1/project/test-project1/locations/" - "test-zone2/clusters/test-cluster3\"" - "}"; - auto json = JsonParse(json_str); - ASSERT_TRUE(json.ok()) << json.status(); - grpc_error_handle error; - auto config = - GoogleMeshCaCertificateProviderFactory::Config::Parse(*json, &error); - EXPECT_THAT( - StatusToString(error), - ::testing::ContainsRegex( - "field:server.*field:grpc_services.*field:google_grpc.*field:" - "call_credentials.*field:sts_service error:type should be OBJECT")); -} - -} // namespace -} // namespace testing -} // namespace grpc_core - -int main(int argc, char** argv) { - ::testing::InitGoogleTest(&argc, argv); - grpc::testing::TestEnvironment env(&argc, argv); - grpc_init(); - auto result = RUN_ALL_TESTS(); - grpc_shutdown(); - return result; -} diff --git a/test/core/xds/xds_bootstrap_test.cc b/test/core/xds/xds_bootstrap_test.cc index 86c0098fa64..41f6995146d 100644 --- a/test/core/xds/xds_bootstrap_test.cc +++ b/test/core/xds/xds_bootstrap_test.cc @@ -23,11 +23,9 @@ #include #include #include -#include #include "absl/status/status.h" #include "absl/status/statusor.h" -#include "absl/strings/numbers.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "gmock/gmock.h" @@ -37,7 +35,6 @@ #include #include #include -#include #include "src/core/ext/xds/certificate_provider_store.h" #include "src/core/ext/xds/xds_bootstrap_grpc.h" @@ -45,8 +42,9 @@ #include "src/core/lib/gpr/tmpfile.h" #include "src/core/lib/gprpp/env.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" -#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/gprpp/validation_errors.h" #include "src/core/lib/json/json.h" +#include "src/core/lib/json/json_args.h" #include "src/core/lib/json/json_object_loader.h" #include "src/core/lib/json/json_reader.h" #include "src/core/lib/json/json_writer.h" @@ -579,11 +577,9 @@ class FakeCertificateProviderFactory : public CertificateProviderFactory { public: class Config : public CertificateProviderFactory::Config { public: - explicit Config(int value) : value_(value) {} - int value() const { return value_; } - const char* name() const override { return "fake"; } + absl::string_view name() const override { return "fake"; } std::string ToString() const override { return absl::StrFormat( @@ -593,28 +589,23 @@ class FakeCertificateProviderFactory : public CertificateProviderFactory { value_); } + static const JsonLoaderInterface* JsonLoader(const JsonArgs&) { + static const auto* loader = JsonObjectLoader() + .OptionalField("value", &Config::value_) + .Finish(); + return loader; + } + private: int value_; }; - const char* name() const override { return "fake"; } + absl::string_view name() const override { return "fake"; } RefCountedPtr - CreateCertificateProviderConfig(const Json& config_json, - grpc_error_handle* error) override { - std::vector error_list; - EXPECT_EQ(config_json.type(), Json::Type::kObject); - auto it = config_json.object().find("value"); - if (it == config_json.object().end()) { - return MakeRefCounted(0); - } else if (it->second.type() != Json::Type::kNumber) { - *error = GRPC_ERROR_CREATE("field:config field:value not of type number"); - } else { - int value = 0; - EXPECT_TRUE(absl::SimpleAtoi(it->second.string(), &value)); - return MakeRefCounted(value); - } - return nullptr; + CreateCertificateProviderConfig(const Json& config_json, const JsonArgs& args, + ValidationErrors* errors) override { + return LoadFromJson>(config_json, args, errors); } RefCountedPtr CreateCertificateProvider( @@ -636,20 +627,16 @@ TEST(XdsBootstrapTest, CertificateProvidersFakePluginParsingError) { " \"fake_plugin\": {" " \"plugin_name\": \"fake\"," " \"config\": {" - " \"value\": \"10\"" + " \"value\": []" " }" " }" " }" "}"; auto bootstrap = GrpcXdsBootstrap::Create(json_str); - EXPECT_THAT( - // Explicit conversion to std::string to work around - // https://github.com/google/googletest/issues/3949. - std::string(bootstrap.status().message()), - ::testing::MatchesRegex( - "errors validating JSON: \\[" - "field:certificate_providers\\[\"fake_plugin\"\\].config " - "error:UNKNOWN:field:config field:value not of type number.*\\]")) + EXPECT_EQ(bootstrap.status().message(), + "errors validating JSON: [" + "field:certificate_providers[\"fake_plugin\"].config.value " + "error:is not a number]") << bootstrap.status(); } @@ -677,7 +664,7 @@ TEST(XdsBootstrapTest, CertificateProvidersFakePluginParsingSuccess) { const CertificateProviderStore::PluginDefinition& fake_plugin = bootstrap->certificate_providers().at("fake_plugin"); ASSERT_EQ(fake_plugin.plugin_name, "fake"); - ASSERT_STREQ(fake_plugin.config->name(), "fake"); + ASSERT_EQ(fake_plugin.config->name(), "fake"); ASSERT_EQ(static_cast>( fake_plugin.config) ->value(), @@ -705,7 +692,7 @@ TEST(XdsBootstrapTest, CertificateProvidersFakePluginEmptyConfig) { const CertificateProviderStore::PluginDefinition& fake_plugin = bootstrap->certificate_providers().at("fake_plugin"); ASSERT_EQ(fake_plugin.plugin_name, "fake"); - ASSERT_STREQ(fake_plugin.config->name(), "fake"); + ASSERT_EQ(fake_plugin.config->name(), "fake"); ASSERT_EQ(static_cast>( fake_plugin.config) ->value(), diff --git a/test/cpp/end2end/xds/xds_end2end_test.cc b/test/cpp/end2end/xds/xds_end2end_test.cc index 33c592275f8..a1843007939 100644 --- a/test/cpp/end2end/xds/xds_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_end2end_test.cc @@ -216,28 +216,30 @@ class FakeCertificateProviderFactory public: class Config : public grpc_core::CertificateProviderFactory::Config { public: - explicit Config(const char* name) : name_(name) {} + explicit Config(absl::string_view name) : name_(name) {} - const char* name() const override { return name_; } + absl::string_view name() const override { return name_; } std::string ToString() const override { return "{}"; } private: - const char* name_; + absl::string_view name_; }; FakeCertificateProviderFactory( - const char* name, + absl::string_view name, FakeCertificateProvider::CertDataMapWrapper* cert_data_map) : name_(name), cert_data_map_(cert_data_map) { GPR_ASSERT(cert_data_map != nullptr); } - const char* name() const override { return name_; } + absl::string_view name() const override { return name_; } grpc_core::RefCountedPtr - CreateCertificateProviderConfig(const grpc_core::Json& /*config_json*/, - grpc_error_handle* /*error*/) override { + CreateCertificateProviderConfig( + const grpc_core::Json& /*config_json*/, + const grpc_core::JsonArgs& /*args*/, + grpc_core::ValidationErrors* /*errors*/) override { return grpc_core::MakeRefCounted(name_); } @@ -251,7 +253,7 @@ class FakeCertificateProviderFactory } private: - const char* name_; + absl::string_view name_; FakeCertificateProvider::CertDataMapWrapper* cert_data_map_; }; diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index cad391c6d13..56187460fda 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -1343,7 +1343,7 @@ "posix", "windows" ], - "uses_polling": true + "uses_polling": false }, { "args": [], @@ -2959,7 +2959,7 @@ "posix", "windows" ], - "uses_polling": true + "uses_polling": false }, { "args": [], @@ -3291,30 +3291,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "google_mesh_ca_certificate_provider_factory_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false, @@ -8799,7 +8775,7 @@ "posix", "windows" ], - "uses_polling": true + "uses_polling": false }, { "args": [], @@ -8823,7 +8799,7 @@ "posix", "windows" ], - "uses_polling": true + "uses_polling": false }, { "args": [],