diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index ffe6adc01102..7d49fd38f41d 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -102,8 +102,8 @@ void RdsRouteConfigSubscription::onConfigUpdate( return; } auto route_config = - MessageUtil::anyConvert(resources[0]); - MessageUtil::validate(route_config, validator_); + MessageUtil::anyConvertAndValidate(resources[0], + validator_); if (route_config.name() != route_config_name_) { throw EnvoyException(fmt::format("Unexpected RDS configuration (expecting {}): {}", route_config_name_, route_config.name())); diff --git a/source/common/router/route_config_update_receiver_impl.cc b/source/common/router/route_config_update_receiver_impl.cc index 5f7a716c0e80..bdd3a1e188dc 100644 --- a/source/common/router/route_config_update_receiver_impl.cc +++ b/source/common/router/route_config_update_receiver_impl.cc @@ -93,8 +93,8 @@ bool RouteConfigUpdateReceiverImpl::updateVhosts( continue; } envoy::config::route::v3::VirtualHost vhost = - MessageUtil::anyConvert(resource.resource()); - MessageUtil::validate(vhost, validation_visitor_); + MessageUtil::anyConvertAndValidate( + resource.resource(), validation_visitor_); auto found = vhosts.find(vhost.name()); if (found != vhosts.end()) { vhosts.erase(found); diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 05d8a69d0c73..c6ddf526349d 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -142,9 +142,8 @@ bool ScopedRdsConfigSubscription::addOrUpdateScopes( envoy::config::route::v3::ScopedRouteConfiguration scoped_route_config; try { scoped_route_config = - MessageUtil::anyConvert( - resource.resource()); - MessageUtil::validate(scoped_route_config, validation_visitor_); + MessageUtil::anyConvertAndValidate( + resource.resource(), validation_visitor_); const std::string scope_name = scoped_route_config.name(); if (!unique_resource_names.insert(scope_name).second) { throw EnvoyException( @@ -313,8 +312,8 @@ void ScopedRdsConfigSubscription::onConfigUpdate( for (const auto& resource_any : resources) { // Throws (thus rejects all) on any error. auto scoped_route = - MessageUtil::anyConvert(resource_any); - MessageUtil::validate(scoped_route, validation_visitor_); + MessageUtil::anyConvertAndValidate( + resource_any, validation_visitor_); const std::string scope_name = scoped_route.name(); auto scope_config_inserted = scoped_routes.try_emplace(scope_name, std::move(scoped_route)); if (!scope_config_inserted.second) { diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index 1a42193a3157..56f5a7194891 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -556,8 +556,8 @@ RtdsSubscription::RtdsSubscription( void RtdsSubscription::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string&) { validateUpdateSize(resources.size()); - auto runtime = MessageUtil::anyConvert(resources[0]); - MessageUtil::validate(runtime, validation_visitor_); + auto runtime = MessageUtil::anyConvertAndValidate( + resources[0], validation_visitor_); if (runtime.name() != resource_name_) { throw EnvoyException( fmt::format("Unexpected RTDS runtime (expecting {}): {}", resource_name_, runtime.name())); diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index a03b87cc1f7d..bc569a972f88 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -37,8 +37,8 @@ void SdsApi::onConfigUpdate(const Protobuf::RepeatedPtrField& const std::string& version_info) { validateUpdateSize(resources.size()); auto secret = - MessageUtil::anyConvert(resources[0]); - MessageUtil::validate(secret, validation_visitor_); + MessageUtil::anyConvertAndValidate( + resources[0], validation_visitor_); if (secret.name() != sds_config_name_) { throw EnvoyException( diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index 153f973b615a..e68a2c6f33b4 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -41,6 +41,7 @@ void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField clusters; for (const auto& cluster_blob : resources) { + // No validation needed here the overloaded call to onConfigUpdate validates. clusters.push_back(MessageUtil::anyConvert(cluster_blob)); clusters_to_remove.erase(clusters.back().name()); } @@ -78,8 +79,8 @@ void CdsApiImpl::onConfigUpdate( for (const auto& resource : added_resources) { envoy::config::cluster::v3::Cluster cluster; try { - cluster = MessageUtil::anyConvert(resource.resource()); - MessageUtil::validate(cluster, validation_visitor_); + cluster = MessageUtil::anyConvertAndValidate( + resource.resource(), validation_visitor_); if (!cluster_names.insert(cluster.name()).second) { // NOTE: at this point, the first of these duplicates has already been successfully applied. throw EnvoyException(fmt::format("duplicate cluster {} found", cluster.name())); diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index ceac2bd3e340..e8362e676cc3 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -115,8 +115,8 @@ void EdsClusterImpl::onConfigUpdate(const Protobuf::RepeatedPtrField(resources[0]); - MessageUtil::validate(cluster_load_assignment, validation_visitor_); + MessageUtil::anyConvertAndValidate( + resources[0], validation_visitor_); if (cluster_load_assignment.cluster_name() != cluster_name_) { throw EnvoyException(fmt::format("Unexpected EDS cluster (expecting {}): {}", cluster_name_, cluster_load_assignment.cluster_name())); diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index 80ba40685c6d..65f112495fb9 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -63,9 +63,8 @@ void LdsApiImpl::onConfigUpdate( for (const auto& resource : added_resources) { envoy::config::listener::v3::Listener listener; try { - listener = - MessageUtil::anyConvert(resource.resource()); - MessageUtil::validate(listener, validation_visitor_); + listener = MessageUtil::anyConvertAndValidate( + resource.resource(), validation_visitor_); if (!listener_names.insert(listener.name()).second) { // NOTE: at this point, the first of these duplicates has already been successfully applied. throw EnvoyException(fmt::format("duplicate listener {} found", listener.name())); @@ -108,6 +107,7 @@ void LdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField(listener_blob).name(); to_add->set_name(listener_name); diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 2adef91f0241..40b52415448e 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -6,6 +6,8 @@ #include "envoy/config/bootstrap/v3/bootstrap.pb.validate.h" #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/cluster/v3/cluster.pb.validate.h" +#include "envoy/config/cluster/v3/filter.pb.h" +#include "envoy/config/cluster/v3/filter.pb.validate.h" #include "envoy/type/v3/percent.pb.h" #include "common/common/base64.h" @@ -1107,6 +1109,16 @@ TEST_F(ProtobufUtilityTest, AnyConvertWrongType) { R"(Unable to unpack as google.protobuf.Timestamp: \[type.googleapis.com/google.protobuf.Duration\] .*)"); } +// Validated exception thrown when anyConvertAndValidate observes a PGV failures. +TEST_F(ProtobufUtilityTest, AnyConvertAndValidateFailedValidation) { + envoy::config::cluster::v3::Filter filter; + ProtobufWkt::Any source_any; + source_any.PackFrom(filter); + EXPECT_THROW(MessageUtil::anyConvertAndValidate( + source_any, ProtobufMessage::getStrictValidationVisitor()), + ProtoValidationException); +} + // MessageUtility::unpackTo() with the wrong type throws. TEST_F(ProtobufUtilityTest, UnpackToWrongType) { ProtobufWkt::Duration source_duration;