diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 5aa0f35ed991..f9e95c6a7f4e 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -159,14 +159,15 @@ class RouteActionValidationVisitor } }; -const envoy::config::route::v3::WeightedCluster::ClusterWeight& validateWeightedClusterSpecifier( +absl::Status validateWeightedClusterSpecifier( const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster) { if (!cluster.name().empty() && !cluster.cluster_header().empty()) { - throwEnvoyExceptionOrPanic("Only one of name or cluster_header can be specified"); + return absl::InvalidArgumentError("Only one of name or cluster_header can be specified"); } else if (cluster.name().empty() && cluster.cluster_header().empty()) { - throwEnvoyExceptionOrPanic("At least one of name or cluster_header need to be specified"); + return absl::InvalidArgumentError( + "At least one of name or cluster_header need to be specified"); } - return cluster; + return absl::OkStatus(); } // Returns a vector of header parsers, sorted by specificity. The `specificity_ascend` parameter @@ -261,13 +262,17 @@ RetryPolicyImpl::create(const envoy::config::route::v3::RetryPolicy& retry_polic ProtobufMessage::ValidationVisitor& validation_visitor, Upstream::RetryExtensionFactoryContext& factory_context, Server::Configuration::CommonFactoryContext& common_context) { - return std::unique_ptr( - new RetryPolicyImpl(retry_policy, validation_visitor, factory_context, common_context)); + absl::Status creation_status = absl::OkStatus(); + auto ret = std::unique_ptr(new RetryPolicyImpl( + retry_policy, validation_visitor, factory_context, common_context, creation_status)); + RETURN_IF_NOT_OK(creation_status); + return ret; } RetryPolicyImpl::RetryPolicyImpl(const envoy::config::route::v3::RetryPolicy& retry_policy, ProtobufMessage::ValidationVisitor& validation_visitor, Upstream::RetryExtensionFactoryContext& factory_context, - Server::Configuration::CommonFactoryContext& common_context) + Server::Configuration::CommonFactoryContext& common_context, + absl::Status& creation_status) : retriable_headers_(Http::HeaderUtility::buildHeaderMatcherVector( retry_policy.retriable_headers(), common_context)), retriable_request_headers_(Http::HeaderUtility::buildHeaderMatcherVector( @@ -332,8 +337,9 @@ RetryPolicyImpl::RetryPolicyImpl(const envoy::config::route::v3::RetryPolicy& re } if ((*max_interval_).count() < (*base_interval_).count()) { - throwEnvoyExceptionOrPanic( + creation_status = absl::InvalidArgumentError( "retry_policy.max_interval must greater than or equal to the base_interval"); + return; } } } @@ -644,8 +650,10 @@ RouteEntryImplBase::RouteEntryImplBase(const CommonVirtualHostSharedPtr& vhost, std::vector weighted_clusters; weighted_clusters.reserve(route.route().weighted_clusters().clusters().size()); for (const auto& cluster : route.route().weighted_clusters().clusters()) { - auto cluster_entry = std::make_unique( - this, runtime_key_prefix + "." + cluster.name(), factory_context, validator, cluster); + auto cluster_entry = THROW_OR_RETURN_VALUE( + WeightedClusterEntry::create(this, runtime_key_prefix + "." + cluster.name(), + factory_context, validator, cluster), + std::unique_ptr); weighted_clusters.emplace_back(std::move(cluster_entry)); total_weight += weighted_clusters.back()->clusterWeight(); if (total_weight > std::numeric_limits::max()) { @@ -1475,13 +1483,24 @@ const Envoy::Config::TypedMetadata& RouteEntryImplBase::typedMetadata() const { : DefaultRouteMetadataPack::get().typed_metadata_; } +absl::StatusOr> +RouteEntryImplBase::WeightedClusterEntry::create( + const RouteEntryImplBase* parent, const std::string& runtime_key, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator, + const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster) { + RETURN_IF_NOT_OK(validateWeightedClusterSpecifier(cluster)); + return std::unique_ptr( + new WeightedClusterEntry(parent, runtime_key, factory_context, validator, cluster)); +} + RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry( const RouteEntryImplBase* parent, const std::string& runtime_key, Server::Configuration::ServerFactoryContext& factory_context, ProtobufMessage::ValidationVisitor& validator, const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster) - : DynamicRouteEntry(parent, nullptr, validateWeightedClusterSpecifier(cluster).name()), - runtime_key_(runtime_key), loader_(factory_context.runtime()), + : DynamicRouteEntry(parent, nullptr, cluster.name()), runtime_key_(runtime_key), + loader_(factory_context.runtime()), cluster_weight_(PROTOBUF_GET_WRAPPED_REQUIRED(cluster, weight)), per_filter_configs_(THROW_OR_RETURN_VALUE( PerFilterConfigs::create(cluster.typed_per_filter_config(), factory_context, validator), diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index b27fe572645f..be70d40e61a4 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -461,7 +461,8 @@ class RetryPolicyImpl : public RetryPolicy { RetryPolicyImpl(const envoy::config::route::v3::RetryPolicy& retry_policy, ProtobufMessage::ValidationVisitor& validation_visitor, Upstream::RetryExtensionFactoryContext& factory_context, - Server::Configuration::CommonFactoryContext& common_context); + Server::Configuration::CommonFactoryContext& common_context, + absl::Status& creation_status); std::chrono::milliseconds per_try_timeout_{0}; std::chrono::milliseconds per_try_idle_timeout_{0}; // We set the number of retries to 1 by default (i.e. when no route or vhost level retry policy is @@ -991,10 +992,11 @@ class RouteEntryImplBase : public RouteEntryAndRoute, */ class WeightedClusterEntry : public DynamicRouteEntry { public: - WeightedClusterEntry(const RouteEntryImplBase* parent, const std::string& rutime_key, - Server::Configuration::ServerFactoryContext& factory_context, - ProtobufMessage::ValidationVisitor& validator, - const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster); + static absl::StatusOr> + create(const RouteEntryImplBase* parent, const std::string& rutime_key, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator, + const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster); uint64_t clusterWeight() const { return loader_.snapshot().getInteger(runtime_key_, cluster_weight_); @@ -1063,6 +1065,11 @@ class RouteEntryImplBase : public RouteEntryAndRoute, const Http::LowerCaseString& clusterHeaderName() const { return cluster_header_name_; } private: + WeightedClusterEntry(const RouteEntryImplBase* parent, const std::string& rutime_key, + Server::Configuration::ServerFactoryContext& factory_context, + ProtobufMessage::ValidationVisitor& validator, + const envoy::config::route::v3::WeightedCluster::ClusterWeight& cluster); + const std::string runtime_key_; Runtime::Loader& loader_; const uint64_t cluster_weight_; diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 712ed4721145..b93d0d01f340 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -4957,10 +4957,9 @@ TEST_F(RouteMatcherTest, InvalidRetryBackOff) { )EOF"; factory_context_.cluster_manager_.initializeClusters({"backoff"}, {}); - EXPECT_THROW_WITH_MESSAGE( - TestConfigImpl(parseRouteConfigurationFromYaml(yaml), factory_context_, true, - creation_status_), - EnvoyException, "retry_policy.max_interval must greater than or equal to the base_interval"); + TestConfigImpl(parseRouteConfigurationFromYaml(yaml), factory_context_, true, creation_status_); + EXPECT_EQ(creation_status_.message(), + "retry_policy.max_interval must greater than or equal to the base_interval"); } TEST_F(RouteMatcherTest, RateLimitedRetryBackOff) {