From 739e572812192148e859869ebe38d5a29d620ba9 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Thu, 19 Apr 2018 21:02:24 -0400 Subject: [PATCH] rework based on new interface Signed-off-by: Shriram Rajagopalan --- .../extensions/filters/http/fault/config.cc | 9 ++++ source/extensions/filters/http/fault/config.h | 3 ++ .../filters/http/fault/fault_filter.cc | 47 +++++++++++------- .../filters/http/fault/fault_filter.h | 48 +++++++++++++------ .../filters/http/fault/fault_filter_test.cc | 17 ++++--- 5 files changed, 84 insertions(+), 40 deletions(-) diff --git a/source/extensions/filters/http/fault/config.cc b/source/extensions/filters/http/fault/config.cc index 1ee045228db1..db976cf45b9b 100644 --- a/source/extensions/filters/http/fault/config.cc +++ b/source/extensions/filters/http/fault/config.cc @@ -42,6 +42,15 @@ FaultFilterFactory::createFilterFactoryFromProto(const Protobuf::Message& proto_ stats_prefix, context); } +Router::RouteSpecificFilterConfigConstSharedPtr +FaultFilterFactory::createRouteSpecificFilterConfig(const ProtobufWkt::Struct& struct_config) { + envoy::config::filter::http::fault::v2::HTTPFault proto_config; + MessageUtil::jsonConvert(struct_config, proto_config); + Router::RouteSpecificFilterConfigConstSharedPtr rconfig; + rconfig.reset(new Fault::PerRouteFaultFilterConfig(proto_config)); + return rconfig; +} + /** * Static registration for the fault filter. @see RegisterFactory. */ diff --git a/source/extensions/filters/http/fault/config.h b/source/extensions/filters/http/fault/config.h index 29253bacae19..bd5a93fdf618 100644 --- a/source/extensions/filters/http/fault/config.h +++ b/source/extensions/filters/http/fault/config.h @@ -27,6 +27,9 @@ class FaultFilterFactory : public Server::Configuration::NamedHttpFilterConfigFa return ProtobufTypes::MessagePtr{new envoy::config::filter::http::fault::v2::HTTPFault()}; } + Router::RouteSpecificFilterConfigConstSharedPtr + createRouteSpecificFilterConfig(const ProtobufWkt::Struct&) override; + std::string name() override { return HttpFilterNames::get().FAULT; } private: diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index fe10634bde83..b3a5d9ebe8ea 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -32,11 +32,12 @@ const std::string FaultFilter::ABORT_PERCENT_KEY = "fault.http.abort.abort_perce const std::string FaultFilter::DELAY_DURATION_KEY = "fault.http.delay.fixed_duration_ms"; const std::string FaultFilter::ABORT_HTTP_STATUS_KEY = "fault.http.abort.http_status"; -FaultFilterConfig::FaultFilterConfig(const envoy::config::filter::http::fault::v2::HTTPFault& fault, - Runtime::Loader& runtime, const std::string& stats_prefix, - Stats::Scope& scope) - : runtime_(runtime), stats_(generateStats(stats_prefix, scope)), stats_prefix_(stats_prefix), - scope_(scope) { +PerRouteFaultFilterConfig::PerRouteFaultFilterConfig( + const envoy::config::filter::http::fault::v2::HTTPFault& fault) { + + if (!fault.has_abort() && !fault.has_delay()) { + throw EnvoyException("fault filter must have at least abort or delay specified in the config."); + } if (fault.has_abort()) { abort_percent_ = fault.abort().percent(); @@ -60,6 +61,23 @@ FaultFilterConfig::FaultFilterConfig(const envoy::config::filter::http::fault::v } } +FaultFilterConfig::FaultFilterConfig(const envoy::config::filter::http::fault::v2::HTTPFault& fault, + Runtime::Loader& runtime, const std::string& stats_prefix, + Stats::Scope& scope) + : runtime_(runtime), stats_(generateStats(stats_prefix, scope)), stats_prefix_(stats_prefix), + scope_(scope) { + + // A small optimization to eliminate unnecessary processing in the + // fault filter when an empty config is provided + if (!fault.has_delay() && !fault.has_abort()) { + return; + } + + local_config_.reset(new PerRouteFaultFilterConfig(fault)); + // This feels a bit ugly, but we don't want to allocate anything here. + filter_config_ = local_config_.get(); +} + FaultFilter::FaultFilter(FaultFilterConfigSharedPtr config) : config_(config) {} FaultFilter::~FaultFilter() { ASSERT(!delay_timer_); } @@ -77,18 +95,15 @@ Http::FilterHeadersStatus FaultFilter::decodeHeaders(Http::HeaderMap& headers, b const std::string name = Extensions::HttpFilters::HttpFilterNames::get().FAULT; const auto* route_entry = callbacks_->route()->routeEntry(); - const auto* proto_config = - route_entry->perFilterConfig(name) ?: route_entry->virtualHost().perFilterConfig(name); - - if (proto_config) { - const envoy::config::filter::http::fault::v2::HTTPFault per_filter_config = - dynamic_cast(*proto_config); + const PerRouteFaultFilterConfig* per_filter_config = + route_entry->perFilterConfigTyped(name) + ?: route_entry->virtualHost().perFilterConfigTyped(name); + config_->updateFilterConfig(per_filter_config); + } - // TODO (qiwzhang): Optimize this such that its updated only by the - // route update callback - config_.reset(new FaultFilterConfig(per_filter_config, config_->runtime(), - config_->statsPrefix(), config_->scope())); - } + // ignore if filter config is empty + if (config_->emptyFilterConfig()) { + return Http::FilterHeadersStatus::Continue; } if (!matchesTargetUpstreamCluster()) { diff --git a/source/extensions/filters/http/fault/fault_filter.h b/source/extensions/filters/http/fault/fault_filter.h index cd64aff71b42..9f54d55838fe 100644 --- a/source/extensions/filters/http/fault/fault_filter.h +++ b/source/extensions/filters/http/fault/fault_filter.h @@ -35,6 +35,23 @@ struct FaultFilterStats { ALL_FAULT_FILTER_STATS(GENERATE_COUNTER_STRUCT) }; +/** + * Configuration for the fault filter that can be specified per route + */ +struct PerRouteFaultFilterConfig : public Router::RouteSpecificFilterConfig { + PerRouteFaultFilterConfig(const envoy::config::filter::http::fault::v2::HTTPFault& fault); + + uint64_t abort_percent_{}; // 0-100 + uint64_t http_status_{}; // HTTP or gRPC return codes + uint64_t fixed_delay_percent_{}; // 0-100 + uint64_t fixed_duration_ms_{}; // in milliseconds + std::string upstream_cluster_; // restrict faults to specific upstream cluster + std::vector fault_filter_headers_; + std::unordered_set downstream_nodes_{}; // Inject failures for specific downstream +}; + +typedef std::shared_ptr PerRouteFaultFilterConfigConstSharedPtr; + /** * Configuration for the fault filter. */ @@ -44,30 +61,31 @@ class FaultFilterConfig { Runtime::Loader& runtime, const std::string& stats_prefix, Stats::Scope& scope); const std::vector& filterHeaders() { - return fault_filter_headers_; + return filter_config_->fault_filter_headers_; } - uint64_t abortPercent() { return abort_percent_; } - uint64_t delayPercent() { return fixed_delay_percent_; } - uint64_t delayDuration() { return fixed_duration_ms_; } - uint64_t abortCode() { return http_status_; } - const std::string& upstreamCluster() { return upstream_cluster_; } + uint64_t abortPercent() { return filter_config_->abort_percent_; } + uint64_t delayPercent() { return filter_config_->fixed_delay_percent_; } + uint64_t delayDuration() { return filter_config_->fixed_duration_ms_; } + uint64_t abortCode() { return filter_config_->http_status_; } + const std::string& upstreamCluster() { return filter_config_->upstream_cluster_; } Runtime::Loader& runtime() { return runtime_; } FaultFilterStats& stats() { return stats_; } - const std::unordered_set& downstreamNodes() { return downstream_nodes_; } + const std::unordered_set& downstreamNodes() { + return filter_config_->downstream_nodes_; + } const std::string& statsPrefix() { return stats_prefix_; } Stats::Scope& scope() { return scope_; } + bool emptyFilterConfig() { return !!filter_config_; } + + void updateFilterConfig(const PerRouteFaultFilterConfig* config) { + filter_config_ = config != nullptr ? config : filter_config_; + } private: static FaultFilterStats generateStats(const std::string& prefix, Stats::Scope& scope); - uint64_t abort_percent_{}; // 0-100 - uint64_t http_status_{}; // HTTP or gRPC return codes - uint64_t fixed_delay_percent_{}; // 0-100 - uint64_t fixed_duration_ms_{}; // in milliseconds - std::string upstream_cluster_; // restrict faults to specific upstream cluster - std::vector fault_filter_headers_; - std::unordered_set downstream_nodes_{}; // Inject failures for specific downstream - // nodes. If not set then inject for all. + const PerRouteFaultFilterConfig* filter_config_; + PerRouteFaultFilterConfigConstSharedPtr local_config_; Runtime::Loader& runtime_; FaultFilterStats stats_; const std::string stats_prefix_; diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc index 40f3378df6a1..150a4c568c18 100644 --- a/test/extensions/filters/http/fault/fault_filter_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_test.cc @@ -24,13 +24,13 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::_; using testing::DoAll; using testing::Invoke; using testing::NiceMock; using testing::Return; using testing::ReturnRef; using testing::WithArgs; -using testing::_; namespace Envoy { namespace Extensions { @@ -142,8 +142,8 @@ class FaultFilterTest : public testing::Test { EXPECT_CALL(*timer_, disableTimer()); } - void TestPerFilterConfigFault(envoy::config::filter::http::fault::v2::HTTPFault* route_fault, - envoy::config::filter::http::fault::v2::HTTPFault* vhost_fault); + void TestPerFilterConfigFault(const Router::RouteSpecificFilterConfig* route_fault, + const Router::RouteSpecificFilterConfig* vhost_fault); FaultFilterConfigSharedPtr config_; std::unique_ptr filter_; @@ -752,8 +752,8 @@ TEST_F(FaultFilterTest, FaultWithTargetClusterNullRoute) { } void FaultFilterTest::TestPerFilterConfigFault( - envoy::config::filter::http::fault::v2::HTTPFault* route_fault, - envoy::config::filter::http::fault::v2::HTTPFault* vhost_fault) { + const Router::RouteSpecificFilterConfig* route_fault, + const Router::RouteSpecificFilterConfig* vhost_fault) { ON_CALL(filter_callbacks_.route_->route_entry_, perFilterConfig(Extensions::HttpFilters::HttpFilterNames::get().FAULT)) @@ -798,10 +798,9 @@ void FaultFilterTest::TestPerFilterConfigFault( TEST_F(FaultFilterTest, RouteFaultOverridesListenerFault) { - envoy::config::filter::http::fault::v2::HTTPFault abort_fault = - convertJsonStrToProtoConfig(abort_only_json); - envoy::config::filter::http::fault::v2::HTTPFault delay_fault = - convertJsonStrToProtoConfig(delay_with_upstream_cluster_json); + Fault::PerRouteFaultFilterConfig abort_fault(convertJsonStrToProtoConfig(abort_only_json)); + Fault::PerRouteFaultFilterConfig delay_fault( + convertJsonStrToProtoConfig(delay_with_upstream_cluster_json)); // route-level fault overrides listener-level fault {