Skip to content

Commit

Permalink
router: cleaning up some exceptions
Browse files Browse the repository at this point in the history 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
@alyssawilk
alyssawilk committed Jun 4, 2024
1 parent 3f8ec81 commit 452c5de
Show file tree
Hide file tree
Showing 10 changed files with 723 additions and 426 deletions.
209 changes: 144 additions & 65 deletions source/common/router/config_impl.cc
View file

Large diffs are not rendered by default.

76 changes: 50 additions & 26 deletions source/common/router/config_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,16 @@ class Matchable {

class PerFilterConfigs : public Logger::Loggable<Logger::Id::http> {
public:
static absl::StatusOr<std::unique_ptr<PerFilterConfigs>>
create(const Protobuf::Map<std::string, ProtobufWkt::Any>& typed_configs,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator);

struct FilterConfig {
RouteSpecificFilterConfigConstSharedPtr config_;
bool disabled_{};
};

PerFilterConfigs(const Protobuf::Map<std::string, ProtobufWkt::Any>& typed_configs,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator);

const RouteSpecificFilterConfig* get(const std::string& name) const;

/**
Expand All @@ -101,6 +102,10 @@ class PerFilterConfigs : public Logger::Loggable<Logger::Id::http> {
absl::optional<bool> disabled(absl::string_view name) const;

private:
PerFilterConfigs(const Protobuf::Map<std::string, ProtobufWkt::Any>& typed_configs,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator, absl::Status& creation_status);

absl::StatusOr<RouteSpecificFilterConfigConstSharedPtr>
createRouteSpecificFilterConfig(const std::string& name, const ProtobufWkt::Any& typed_config,
bool is_optional,
Expand Down Expand Up @@ -244,11 +249,11 @@ using CommonConfigSharedPtr = std::shared_ptr<CommonConfigImpl>;
*/
class CommonVirtualHostImpl : public VirtualHost, Logger::Loggable<Logger::Id::router> {
public:
CommonVirtualHostImpl(const envoy::config::route::v3::VirtualHost& virtual_host,
const CommonConfigSharedPtr& global_route_config,
Server::Configuration::ServerFactoryContext& factory_context,
Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validator,
absl::Status& creation_status);
static absl::StatusOr<std::shared_ptr<CommonVirtualHostImpl>>
create(const envoy::config::route::v3::VirtualHost& virtual_host,
const CommonConfigSharedPtr& global_route_config,
Server::Configuration::ServerFactoryContext& factory_context, Stats::Scope& scope,
ProtobufMessage::ValidationVisitor& validator);

const VirtualCluster* virtualClusterFromEntries(const Http::HeaderMap& headers) const;
const CommonConfigImpl& globalRouteConfig() const { return *global_route_config_; }
Expand Down Expand Up @@ -302,6 +307,11 @@ class CommonVirtualHostImpl : public VirtualHost, Logger::Loggable<Logger::Id::r
const Envoy::Config::TypedMetadata& typedMetadata() const override;

private:
CommonVirtualHostImpl(const envoy::config::route::v3::VirtualHost& virtual_host,
const CommonConfigSharedPtr& global_route_config,
Server::Configuration::ServerFactoryContext& factory_context,
Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validator,
absl::Status& creation_status);
struct StatNameProvider {
StatNameProvider(absl::string_view name, Stats::SymbolTable& symbol_table)
: stat_name_storage_(name, symbol_table) {}
Expand Down Expand Up @@ -353,7 +363,7 @@ class CommonVirtualHostImpl : public VirtualHost, Logger::Loggable<Logger::Id::r
const CommonConfigSharedPtr global_route_config_;
HeaderParserPtr request_headers_parser_;
HeaderParserPtr response_headers_parser_;
PerFilterConfigs per_filter_configs_;
std::unique_ptr<PerFilterConfigs> per_filter_configs_;
std::unique_ptr<envoy::config::route::v3::RetryPolicy> retry_policy_;
std::unique_ptr<envoy::config::route::v3::HedgePolicy> hedge_policy_;
std::unique_ptr<const CatchAllVirtualCluster> virtual_cluster_catch_all_;
Expand Down Expand Up @@ -485,7 +495,8 @@ using DefaultRetryPolicy = ConstSingleton<RetryPolicyImpl>;
class ShadowPolicyImpl : public ShadowPolicy {
public:
using RequestMirrorPolicy = envoy::config::route::v3::RouteAction::RequestMirrorPolicy;
explicit ShadowPolicyImpl(const RequestMirrorPolicy& config);
static absl::StatusOr<std::shared_ptr<ShadowPolicyImpl>>
create(const RequestMirrorPolicy& config);

// Router::ShadowPolicy
const std::string& cluster() const override { return cluster_; }
Expand All @@ -496,6 +507,8 @@ class ShadowPolicyImpl : public ShadowPolicy {
bool disableShadowHostSuffixAppend() const override { return disable_shadow_host_suffix_append_; }

private:
explicit ShadowPolicyImpl(const RequestMirrorPolicy& config, absl::Status& creation_status);

const std::string cluster_;
const Http::LowerCaseString cluster_header_;
std::string runtime_key_;
Expand Down Expand Up @@ -808,7 +821,7 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
absl::optional<bool> filterDisabled(absl::string_view config_name) const override;
const RouteSpecificFilterConfig*
mostSpecificPerFilterConfig(const std::string& name) const override {
auto* config = per_filter_configs_.get(name);
auto* config = per_filter_configs_->get(name);
return config ? config : vhost_->mostSpecificPerFilterConfig(name);
}
void traversePerFilterConfig(
Expand Down Expand Up @@ -1027,15 +1040,15 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
bool do_formatting = true) const override;

absl::optional<bool> filterDisabled(absl::string_view config_name) const override {
absl::optional<bool> result = per_filter_configs_.disabled(config_name);
absl::optional<bool> result = per_filter_configs_->disabled(config_name);
if (result.has_value()) {
return result.value();
}
return DynamicRouteEntry::filterDisabled(config_name);
}
const RouteSpecificFilterConfig*
mostSpecificPerFilterConfig(const std::string& name) const override {
auto* config = per_filter_configs_.get(name);
auto* config = per_filter_configs_->get(name);
return config ? config : DynamicRouteEntry::mostSpecificPerFilterConfig(name);
}

Expand All @@ -1052,7 +1065,7 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
MetadataMatchCriteriaConstPtr cluster_metadata_match_criteria_;
HeaderParserPtr request_headers_parser_;
HeaderParserPtr response_headers_parser_;
PerFilterConfigs per_filter_configs_;
std::unique_ptr<PerFilterConfigs> per_filter_configs_;
const std::string host_rewrite_;
const Http::LowerCaseString cluster_header_name_;
};
Expand Down Expand Up @@ -1148,7 +1161,7 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
buildHedgePolicy(HedgePolicyConstOptRef vhost_hedge_policy,
const envoy::config::route::v3::RouteAction& route_config) const;

std::unique_ptr<RetryPolicyImpl>
absl::StatusOr<std::unique_ptr<RetryPolicyImpl>>
buildRetryPolicy(RetryPolicyConstOptRef vhost_retry_policy,
const envoy::config::route::v3::RouteAction& route_config,
ProtobufMessage::ValidationVisitor& validation_visitor,
Expand Down Expand Up @@ -1228,7 +1241,7 @@ class RouteEntryImplBase : public RouteEntryAndRoute,
const DecoratorConstPtr decorator_;
const RouteTracingConstPtr route_tracing_;
std::string direct_response_body_;
PerFilterConfigs per_filter_configs_;
std::unique_ptr<PerFilterConfigs> per_filter_configs_;
const std::string route_name_;
TimeSource& time_source_;
EarlyDataPolicyPtr early_data_policy_;
Expand Down Expand Up @@ -1563,9 +1576,10 @@ class RouteMatcher {
*/
class CommonConfigImpl : public CommonConfig {
public:
CommonConfigImpl(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator);
static absl::StatusOr<std::shared_ptr<CommonConfigImpl>>
create(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator);

const HeaderParser& requestHeaderParser() const {
if (request_headers_parser_ != nullptr) {
Expand All @@ -1581,10 +1595,10 @@ class CommonConfigImpl : public CommonConfig {
}

const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const {
return per_filter_configs_.get(name);
return per_filter_configs_->get(name);
}
absl::optional<bool> filterDisabled(absl::string_view config_name) const {
return per_filter_configs_.disabled(config_name);
return per_filter_configs_->disabled(config_name);
}

// Router::CommonConfig
Expand All @@ -1609,6 +1623,9 @@ class CommonConfigImpl : public CommonConfig {
const Envoy::Config::TypedMetadata& typedMetadata() const override;

private:
CommonConfigImpl(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator, absl::Status& creation_status);
std::list<Http::LowerCaseString> internal_only_headers_;
HeaderParserPtr request_headers_parser_;
HeaderParserPtr response_headers_parser_;
Expand All @@ -1617,7 +1634,7 @@ class CommonConfigImpl : public CommonConfig {
std::vector<ShadowPolicyPtr> shadow_policies_;
// Cluster specifier plugins/providers.
absl::flat_hash_map<std::string, ClusterSpecifierPluginSharedPtr> cluster_specifier_plugins_;
PerFilterConfigs per_filter_configs_;
std::unique_ptr<PerFilterConfigs> per_filter_configs_;
RouteMetadataPackPtr metadata_;
// Keep small members (bools and enums) at the end of class, to reduce alignment overhead.
const uint32_t max_direct_response_body_size_bytes_;
Expand All @@ -1631,9 +1648,10 @@ class CommonConfigImpl : public CommonConfig {
*/
class ConfigImpl : public Config {
public:
ConfigImpl(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default);
static absl::StatusOr<std::shared_ptr<ConfigImpl>>
create(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default);

bool virtualHostExists(const Http::RequestHeaderMap& headers) const {
return route_matcher_->findVirtualHost(headers) != nullptr;
Expand Down Expand Up @@ -1672,6 +1690,12 @@ class ConfigImpl : public Config {
return shared_config_->typedMetadata();
}

protected:
ConfigImpl(const envoy::config::route::v3::RouteConfiguration& config,
Server::Configuration::ServerFactoryContext& factory_context,
ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default,
absl::Status& creation_status);

private:
CommonConfigSharedPtr shared_config_;
std::unique_ptr<RouteMatcher> route_matcher_;
Expand Down
7 changes: 4 additions & 3 deletions source/common/router/route_config_update_receiver_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ ConfigTraitsImpl::createConfig(const Protobuf::Message& rc,
Server::Configuration::ServerFactoryContext& factory_context,
bool validate_clusters_default) const {
ASSERT(dynamic_cast<const envoy::config::route::v3::RouteConfiguration*>(&rc));
return std::make_shared<ConfigImpl>(
static_cast<const envoy::config::route::v3::RouteConfiguration&>(rc), factory_context,
validator_, validate_clusters_default);
return THROW_OR_RETURN_VALUE(
ConfigImpl::create(static_cast<const envoy::config::route::v3::RouteConfiguration&>(rc),
factory_context, validator_, validate_clusters_default),
std::shared_ptr<ConfigImpl>);
}

bool RouteConfigUpdateReceiverImpl::onRdsUpdate(const Protobuf::Message& rc,
Expand Down
6 changes: 3 additions & 3 deletions test/common/router/config_impl_headermap_benchmark_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ static void manyCountryRoutesLongHeaders(benchmark::State& state) {
Api::ApiPtr api(Api::createApiForTest());
NiceMock<Server::Configuration::MockServerFactoryContext> factory_context;
ON_CALL(factory_context, api()).WillByDefault(ReturnRef(*api));
ConfigImpl config(proto_config, factory_context, ProtobufMessage::getNullValidationVisitor(),
true);
std::shared_ptr<ConfigImpl> config = *ConfigImpl::create(
proto_config, factory_context, ProtobufMessage::getNullValidationVisitor(), true);

const auto stream_info = NiceMock<Envoy::StreamInfo::MockStreamInfo>();
auto req_headers = Http::TestRequestHeaderMapImpl{
Expand All @@ -68,7 +68,7 @@ static void manyCountryRoutesLongHeaders(benchmark::State& state) {
}
req_headers.addReferenceKey(country_header_name, absl::StrCat("country", countries_num));
for (auto _ : state) { // NOLINT
auto& result = config.route(req_headers, stream_info, 0)->routeEntry()->clusterName();
auto& result = config->route(req_headers, stream_info, 0)->routeEntry()->clusterName();
benchmark::DoNotOptimize(result);
}
}
Expand Down
7 changes: 4 additions & 3 deletions test/common/router/config_impl_speed_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,15 @@ static void bmRouteTableSize(benchmark::State& state, RouteMatch::PathSpecifierC
ON_CALL(factory_context, api()).WillByDefault(ReturnRef(*api));

// Create router config.
ConfigImpl config(genRouteConfig(state, match_type), factory_context,
ProtobufMessage::getNullValidationVisitor(), true);
std::shared_ptr<ConfigImpl> config =
*ConfigImpl::create(genRouteConfig(state, match_type), factory_context,
ProtobufMessage::getNullValidationVisitor(), true);

for (auto _ : state) { // NOLINT
// Do the actual timing here.
// Single request that will match the last route in the config.
int last_route_num = state.range(0) - 1;
config.route(genRequestHeaders(last_route_num), stream_info, 0);
config->route(genRequestHeaders(last_route_num), stream_info, 0);
}
}

Expand Down
Loading

0 comments on commit 452c5de

Please sign in to comment.