rework based on new interface

Signed-off-by: Shriram Rajagopalan <shriramr@vmware.com>

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.
  */

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:

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<const envoy::config::filter::http::fault::v2::HTTPFault&>(*proto_config);
+    const PerRouteFaultFilterConfig* per_filter_config =
+        route_entry->perFilterConfigTyped<PerRouteFaultFilterConfig>(name)
+            ?: route_entry->virtualHost().perFilterConfigTyped<PerRouteFaultFilterConfig>(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()) {

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<Router::ConfigUtility::HeaderData> fault_filter_headers_;
+  std::unordered_set<std::string> downstream_nodes_{}; // Inject failures for specific downstream
+};
+
+typedef std::shared_ptr<const PerRouteFaultFilterConfig> 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<Router::ConfigUtility::HeaderData>& 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<std::string>& downstreamNodes() { return downstream_nodes_; }
+  const std::unordered_set<std::string>& 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<Router::ConfigUtility::HeaderData> fault_filter_headers_;
-  std::unordered_set<std::string> 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_;

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<FaultFilter> 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
   {