Skip to content

Commit e071417

Browse files
soulxusoulxu
authored
http: return per route config when direct response is set (#17449)
Signed-off-by: He Jie Xu <[email protected]>
1 parent be63e34 commit e071417

File tree

26 files changed

+230
-448
lines changed

26 files changed

+230
-448
lines changed

‎envoy/router/router.h‎

Lines changed: 11 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -547,21 +547,6 @@ class VirtualHost {
547547
*/
548548
virtual const Config& routeConfig() const PURE;
549549

550-
/**
551-
* @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
552-
* the given filter name. If there is not per-filter config, or the filter factory returns
553-
* nullptr, nullptr is returned.
554-
*/
555-
virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
556-
557-
/**
558-
* This is a helper on top of perFilterConfig() that casts the return object to the specified
559-
* type.
560-
*/
561-
template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
562-
return dynamic_cast<const Derived*>(perFilterConfig(name));
563-
}
564-
565550
/**
566551
* @return bool whether to include the request count header in upstream requests.
567552
*/
@@ -902,31 +887,6 @@ class RouteEntry : public ResponseEntry {
902887
*/
903888
virtual const PathMatchCriterion& pathMatchCriterion() const PURE;
904889

905-
/**
906-
* @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
907-
* the given filter name. If there is not per-filter config, or the filter factory returns
908-
* nullptr, nullptr is returned.
909-
*/
910-
virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
911-
912-
/**
913-
* This is a helper on top of perFilterConfig() that casts the return object to the specified
914-
* type.
915-
*/
916-
template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
917-
return dynamic_cast<const Derived*>(perFilterConfig(name));
918-
};
919-
920-
/**
921-
* This is a helper to get the route's per-filter config if it exists, otherwise the virtual
922-
* host's. Or nullptr if none of them exist.
923-
*/
924-
template <class Derived>
925-
const Derived* mostSpecificPerFilterConfigTyped(const std::string& name) const {
926-
const Derived* config = perFilterConfigTyped<Derived>(name);
927-
return config ? config : virtualHost().perFilterConfigTyped<Derived>(name);
928-
}
929-
930890
/**
931891
* True if the virtual host this RouteEntry belongs to is configured to include the attempt
932892
* count header.
@@ -1050,19 +1010,21 @@ class Route {
10501010
virtual const RouteTracing* tracingConfig() const PURE;
10511011

10521012
/**
1053-
* @return const RouteSpecificFilterConfig* the per-filter config pre-processed object for
1054-
* the given filter name. If there is not per-filter config, or the filter factory returns
1055-
* nullptr, nullptr is returned.
1013+
* This is a helper to get the route's per-filter config if it exists, otherwise the virtual
1014+
* host's. Or nullptr if none of them exist.
10561015
*/
1057-
virtual const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const PURE;
1016+
virtual const RouteSpecificFilterConfig*
1017+
mostSpecificPerFilterConfig(const std::string& name) const PURE;
10581018

10591019
/**
1060-
* This is a helper on top of perFilterConfig() that casts the return object to the specified
1061-
* type.
1020+
* Fold all the available per route filter configs, invoking the callback with each config (if
1021+
* it is present). Iteration of the configs is in order of specificity. That means that the
1022+
* callback will be called first for a config on a Virtual host, then a route, and finally a route
1023+
* entry (weighted cluster). If a config is not present, the callback will not be invoked.
10621024
*/
1063-
template <class Derived> const Derived* perFilterConfigTyped(const std::string& name) const {
1064-
return dynamic_cast<const Derived*>(perFilterConfig(name));
1065-
}
1025+
virtual void traversePerFilterConfig(
1026+
const std::string& filter_name,
1027+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const PURE;
10661028

10671029
/**
10681030
* @return const envoy::config::core::v3::Metadata& return the metadata provided in the config for

‎source/common/http/async_client_impl.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,6 @@ class AsyncStreamImpl : public AsyncClient::Stream,
192192
const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
193193
const Router::CorsPolicy* corsPolicy() const override { return nullptr; }
194194
const Router::Config& routeConfig() const override { return route_configuration_; }
195-
const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
196-
return nullptr;
197-
}
198195
bool includeAttemptCountInRequest() const override { return false; }
199196
bool includeAttemptCountInResponse() const override { return false; }
200197
uint32_t retryShadowBufferLimit() const override {
@@ -303,9 +300,6 @@ class AsyncStreamImpl : public AsyncClient::Stream,
303300
return path_match_criterion_;
304301
}
305302

306-
const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
307-
return nullptr;
308-
}
309303
const absl::optional<ConnectConfig>& connectConfig() const override {
310304
return connect_config_nullopt_;
311305
}
@@ -345,9 +339,13 @@ class AsyncStreamImpl : public AsyncClient::Stream,
345339
const Router::RouteEntry* routeEntry() const override { return &route_entry_; }
346340
const Router::Decorator* decorator() const override { return nullptr; }
347341
const Router::RouteTracing* tracingConfig() const override { return nullptr; }
348-
const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
342+
const Router::RouteSpecificFilterConfig*
343+
mostSpecificPerFilterConfig(const std::string&) const override {
349344
return nullptr;
350345
}
346+
void traversePerFilterConfig(
347+
const std::string&,
348+
std::function<void(const Router::RouteSpecificFilterConfig&)>) const override {}
351349
const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
352350
const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
353351

‎source/common/http/utility.cc‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -921,35 +921,6 @@ void Utility::transformUpgradeResponseFromH2toH1(ResponseHeaderMap& headers,
921921
}
922922
}
923923

924-
void Utility::traversePerFilterConfigGeneric(
925-
const std::string& filter_name, const Router::RouteConstSharedPtr& route,
926-
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) {
927-
if (!route) {
928-
return;
929-
}
930-
931-
const Router::RouteEntry* routeEntry = route->routeEntry();
932-
933-
if (routeEntry != nullptr) {
934-
auto maybe_vhost_config = routeEntry->virtualHost().perFilterConfig(filter_name);
935-
if (maybe_vhost_config != nullptr) {
936-
cb(*maybe_vhost_config);
937-
}
938-
}
939-
940-
auto maybe_route_config = route->perFilterConfig(filter_name);
941-
if (maybe_route_config != nullptr) {
942-
cb(*maybe_route_config);
943-
}
944-
945-
if (routeEntry != nullptr) {
946-
auto maybe_weighted_cluster_config = routeEntry->perFilterConfig(filter_name);
947-
if (maybe_weighted_cluster_config != nullptr) {
948-
cb(*maybe_weighted_cluster_config);
949-
}
950-
}
951-
}
952-
953924
std::string Utility::PercentEncoding::encode(absl::string_view value,
954925
absl::string_view reserved_chars) {
955926
absl::flat_hash_set<char> reserved_char_set{reserved_chars.begin(), reserved_chars.end()};

‎source/common/http/utility.h‎

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -512,40 +512,10 @@ const ConfigType* resolveMostSpecificPerFilterConfig(const std::string& filter_n
512512
const Router::RouteConstSharedPtr& route) {
513513
static_assert(std::is_base_of<Router::RouteSpecificFilterConfig, ConfigType>::value,
514514
"ConfigType must be a subclass of Router::RouteSpecificFilterConfig");
515-
if (!route || !route->routeEntry()) {
515+
if (!route) {
516516
return nullptr;
517517
}
518-
return route->routeEntry()->mostSpecificPerFilterConfigTyped<ConfigType>(filter_name);
519-
}
520-
521-
/**
522-
* The non template implementation of traversePerFilterConfig. see
523-
* traversePerFilterConfig for docs.
524-
*/
525-
void traversePerFilterConfigGeneric(
526-
const std::string& filter_name, const Router::RouteConstSharedPtr& route,
527-
std::function<void(const Router::RouteSpecificFilterConfig&)> cb);
528-
529-
/**
530-
* Fold all the available per route filter configs, invoking the callback with each config (if
531-
* it is present). Iteration of the configs is in order of specificity. That means that the callback
532-
* will be called first for a config on a Virtual host, then a route, and finally a route entry
533-
* (weighted cluster). If a config is not present, the callback will not be invoked.
534-
*/
535-
template <class ConfigType>
536-
void traversePerFilterConfig(const std::string& filter_name,
537-
const Router::RouteConstSharedPtr& route,
538-
std::function<void(const ConfigType&)> cb) {
539-
static_assert(std::is_base_of<Router::RouteSpecificFilterConfig, ConfigType>::value,
540-
"ConfigType must be a subclass of Router::RouteSpecificFilterConfig");
541-
542-
traversePerFilterConfigGeneric(
543-
filter_name, route, [&cb](const Router::RouteSpecificFilterConfig& cfg) {
544-
const ConfigType* typed_cfg = dynamic_cast<const ConfigType*>(&cfg);
545-
if (typed_cfg != nullptr) {
546-
cb(*typed_cfg);
547-
}
548-
});
518+
return dynamic_cast<const ConfigType*>(route->mostSpecificPerFilterConfig(filter_name));
549519
}
550520

551521
/**
@@ -567,14 +537,17 @@ getMergedPerFilterConfig(const std::string& filter_name, const Router::RouteCons
567537

568538
absl::optional<ConfigType> merged;
569539

570-
traversePerFilterConfig<ConfigType>(filter_name, route,
571-
[&reduce, &merged](const ConfigType& cfg) {
572-
if (!merged) {
573-
merged.emplace(cfg);
574-
} else {
575-
reduce(merged.value(), cfg);
576-
}
577-
});
540+
if (route) {
541+
route->traversePerFilterConfig(
542+
filter_name, [&reduce, &merged](const Router::RouteSpecificFilterConfig& cfg) {
543+
const ConfigType* typed_cfg = dynamic_cast<const ConfigType*>(&cfg);
544+
if (!merged) {
545+
merged.emplace(*typed_cfg);
546+
} else {
547+
reduce(merged.value(), *typed_cfg);
548+
}
549+
});
550+
}
578551

579552
return merged;
580553
}

‎source/common/router/config_impl.cc‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,9 +1005,24 @@ void RouteEntryImplBase::validateClusters(
10051005
}
10061006
}
10071007

1008-
const RouteSpecificFilterConfig*
1009-
RouteEntryImplBase::perFilterConfig(const std::string& name) const {
1010-
return per_filter_configs_.get(name);
1008+
void RouteEntryImplBase::traversePerFilterConfig(
1009+
const std::string& filter_name,
1010+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const {
1011+
const Router::RouteEntry* route_entry = routeEntry();
1012+
1013+
// TODO(soulxu): This has similar bug with https://github.com/envoyproxy/envoy/issues/17377
1014+
// it should be fixed.
1015+
if (route_entry != nullptr) {
1016+
auto maybe_vhost_config = vhost_.perFilterConfig(filter_name);
1017+
if (maybe_vhost_config != nullptr) {
1018+
cb(*maybe_vhost_config);
1019+
}
1020+
}
1021+
1022+
auto maybe_route_config = per_filter_configs_.get(filter_name);
1023+
if (maybe_route_config != nullptr) {
1024+
cb(*maybe_route_config);
1025+
}
10111026
}
10121027

10131028
RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry(
@@ -1050,10 +1065,15 @@ Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::responseHeaderT
10501065
return transforms;
10511066
}
10521067

1053-
const RouteSpecificFilterConfig*
1054-
RouteEntryImplBase::WeightedClusterEntry::perFilterConfig(const std::string& name) const {
1055-
const auto cfg = per_filter_configs_.get(name);
1056-
return cfg != nullptr ? cfg : DynamicRouteEntry::perFilterConfig(name);
1068+
void RouteEntryImplBase::WeightedClusterEntry::traversePerFilterConfig(
1069+
const std::string& filter_name,
1070+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const {
1071+
DynamicRouteEntry::traversePerFilterConfig(filter_name, cb);
1072+
1073+
const auto* cfg = per_filter_configs_.get(filter_name);
1074+
if (cfg) {
1075+
cb(*cfg);
1076+
}
10571077
}
10581078

10591079
PrefixRouteEntryImpl::PrefixRouteEntryImpl(

‎source/common/router/config_impl.h‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,12 @@ class SslRedirectRoute : public Route {
127127
const RouteEntry* routeEntry() const override { return nullptr; }
128128
const Decorator* decorator() const override { return nullptr; }
129129
const RouteTracing* tracingConfig() const override { return nullptr; }
130-
const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override {
130+
const RouteSpecificFilterConfig* mostSpecificPerFilterConfig(const std::string&) const override {
131131
return nullptr;
132132
}
133+
void traversePerFilterConfig(
134+
const std::string&,
135+
std::function<void(const Router::RouteSpecificFilterConfig&)>) const override {}
133136
const envoy::config::core::v3::Metadata& metadata() const override { return metadata_; }
134137
const Envoy::Config::TypedMetadata& typedMetadata() const override { return typed_metadata_; }
135138

@@ -212,7 +215,7 @@ class VirtualHostImpl : public VirtualHost {
212215
Stats::StatName statName() const override { return stat_name_storage_.statName(); }
213216
const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
214217
const Config& routeConfig() const override;
215-
const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
218+
const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const;
216219
bool includeAttemptCountInRequest() const override { return include_attempt_count_in_request_; }
217220
bool includeAttemptCountInResponse() const override { return include_attempt_count_in_response_; }
218221
const absl::optional<envoy::config::route::v3::RetryPolicy>& retryPolicy() const {
@@ -591,7 +594,14 @@ class RouteEntryImplBase : public RouteEntry,
591594
const RouteEntry* routeEntry() const override;
592595
const Decorator* decorator() const override { return decorator_.get(); }
593596
const RouteTracing* tracingConfig() const override { return route_tracing_.get(); }
594-
const RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
597+
const RouteSpecificFilterConfig*
598+
mostSpecificPerFilterConfig(const std::string& name) const override {
599+
auto* config = per_filter_configs_.get(name);
600+
return config ? config : vhost_.perFilterConfig(name);
601+
}
602+
void traversePerFilterConfig(
603+
const std::string& filter_name,
604+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const override;
595605

596606
protected:
597607
const bool case_sensitive_;
@@ -737,9 +747,14 @@ class RouteEntryImplBase : public RouteEntry,
737747
const RouteEntry* routeEntry() const override { return this; }
738748
const Decorator* decorator() const override { return parent_->decorator(); }
739749
const RouteTracing* tracingConfig() const override { return parent_->tracingConfig(); }
740-
741-
const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override {
742-
return parent_->perFilterConfig(name);
750+
const RouteSpecificFilterConfig*
751+
mostSpecificPerFilterConfig(const std::string& name) const override {
752+
return parent_->mostSpecificPerFilterConfig(name);
753+
}
754+
void traversePerFilterConfig(
755+
const std::string& filter_name,
756+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const override {
757+
parent_->traversePerFilterConfig(filter_name, cb);
743758
};
744759

745760
private:
@@ -789,7 +804,15 @@ class RouteEntryImplBase : public RouteEntry,
789804
Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
790805
bool do_formatting = true) const override;
791806

792-
const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override;
807+
const RouteSpecificFilterConfig*
808+
mostSpecificPerFilterConfig(const std::string& name) const override {
809+
auto* config = per_filter_configs_.get(name);
810+
return config ? config : DynamicRouteEntry::mostSpecificPerFilterConfig(name);
811+
}
812+
813+
void traversePerFilterConfig(
814+
const std::string& filter_name,
815+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const override;
793816

794817
private:
795818
const std::string runtime_key_;

‎source/common/router/delegating_route_impl.cc‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@ const Decorator* DelegatingRoute::decorator() const { return base_route_->decora
1414

1515
const RouteTracing* DelegatingRoute::tracingConfig() const { return base_route_->tracingConfig(); }
1616

17-
const RouteSpecificFilterConfig* DelegatingRoute::perFilterConfig(const std::string& name) const {
18-
return base_route_->perFilterConfig(name);
19-
}
20-
2117
// Router:DelegatingRouteEntry
2218
void DelegatingRouteEntry::finalizeResponseHeaders(
2319
Http::ResponseHeaderMap& headers, const StreamInfo::StreamInfo& stream_info) const {
@@ -150,11 +146,6 @@ const PathMatchCriterion& DelegatingRouteEntry::pathMatchCriterion() const {
150146
return base_route_->routeEntry()->pathMatchCriterion();
151147
}
152148

153-
const RouteSpecificFilterConfig*
154-
DelegatingRouteEntry::perFilterConfig(const std::string& name) const {
155-
return base_route_->routeEntry()->perFilterConfig(name);
156-
}
157-
158149
bool DelegatingRouteEntry::includeAttemptCountInRequest() const {
159150
return base_route_->routeEntry()->includeAttemptCountInRequest();
160151
}

‎source/common/router/delegating_route_impl.h‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,17 @@ class DelegatingRoute : public Router::Route {
2626
const Router::RouteEntry* routeEntry() const override;
2727
const Router::Decorator* decorator() const override;
2828
const Router::RouteTracing* tracingConfig() const override;
29-
const Router::RouteSpecificFilterConfig* perFilterConfig(const std::string&) const override;
29+
30+
const RouteSpecificFilterConfig*
31+
mostSpecificPerFilterConfig(const std::string& name) const override {
32+
return base_route_->mostSpecificPerFilterConfig(name);
33+
}
34+
void traversePerFilterConfig(
35+
const std::string& filter_name,
36+
std::function<void(const Router::RouteSpecificFilterConfig&)> cb) const override {
37+
base_route_->traversePerFilterConfig(filter_name, cb);
38+
}
39+
3040
const envoy::config::core::v3::Metadata& metadata() const override {
3141
return base_route_->metadata();
3242
}
@@ -90,7 +100,6 @@ class DelegatingRouteEntry : public Router::RouteEntry {
90100
bool includeVirtualHostRateLimits() const override;
91101
const TlsContextMatchCriteria* tlsContextMatchCriteria() const override;
92102
const PathMatchCriterion& pathMatchCriterion() const override;
93-
const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override;
94103
bool includeAttemptCountInRequest() const override;
95104
bool includeAttemptCountInResponse() const override;
96105
const UpgradeMap& upgradeMap() const override;

0 commit comments

Comments
 (0)