redis: mirroring should work when default value is zero, not just gre…

…ater than zero (envoyproxy#8089) Signed-off-by: Nicolas Flacco <[email protected]>
danzh2010 · Sep 24, 2019 · d1b64c4 · d1b64c4
1 parent 7edfcef
commit d1b64c4
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 4 deletions.
diff --git a/source/extensions/filters/network/redis_proxy/router_impl.cc b/source/extensions/filters/network/redis_proxy/router_impl.cc
@@ -10,7 +10,9 @@ MirrorPolicyImpl::MirrorPolicyImpl(const envoy::config::filter::network::redis_p
                                    const ConnPool::InstanceSharedPtr upstream,
                                    Runtime::Loader& runtime)
     : runtime_key_(config.runtime_fraction().runtime_key()),
-      default_value_(config.runtime_fraction().default_value()),
+      default_value_(config.has_runtime_fraction() ? absl::optional<envoy::type::FractionalPercent>(
+                                                         config.runtime_fraction().default_value())
+                                                   : absl::nullopt),
       exclude_read_commands_(config.exclude_read_commands()), upstream_(upstream),
       runtime_(runtime) {}
 
@@ -23,8 +25,8 @@ bool MirrorPolicyImpl::shouldMirror(const std::string& command) const {
     return false;
   }
 
-  if (default_value_.numerator() > 0) {
-    return runtime_.snapshot().featureEnabled(runtime_key_, default_value_);
+  if (default_value_.has_value()) {
+    return runtime_.snapshot().featureEnabled(runtime_key_, default_value_.value());
   }
 
   return true;

diff --git a/source/extensions/filters/network/redis_proxy/router_impl.h b/source/extensions/filters/network/redis_proxy/router_impl.h
@@ -37,7 +37,7 @@ class MirrorPolicyImpl : public MirrorPolicy {
 
 private:
   const std::string runtime_key_;
-  const envoy::type::FractionalPercent default_value_;
+  const absl::optional<envoy::type::FractionalPercent> default_value_;
   const bool exclude_read_commands_;
   ConnPool::InstanceSharedPtr upstream_;
   Runtime::Loader& runtime_;

diff --git a/test/extensions/filters/network/redis_proxy/router_impl_test.cc b/test/extensions/filters/network/redis_proxy/router_impl_test.cc
@@ -225,6 +225,22 @@ TEST(MirrorPolicyImplTest, ExcludeReadCommands) {
   EXPECT_EQ(true, policy.shouldMirror("set"));
 }
 
+TEST(MirrorPolicyImplTest, DefaultValueZero) {
+  envoy::config::filter::network::redis_proxy::v2::RedisProxy::PrefixRoutes::Route::
+      RequestMirrorPolicy config;
+  auto* runtime_fraction = config.mutable_runtime_fraction();
+  auto* percentage = runtime_fraction->mutable_default_value();
+  percentage->set_numerator(0);
+  percentage->set_denominator(envoy::type::FractionalPercent::HUNDRED);
+  auto upstream = std::make_shared<ConnPool::MockInstance>();
+  NiceMock<Runtime::MockLoader> runtime;
+
+  MirrorPolicyImpl policy(config, upstream, runtime);
+
+  EXPECT_EQ(false, policy.shouldMirror("get"));
+  EXPECT_EQ(false, policy.shouldMirror("set"));
+}
+
 TEST(MirrorPolicyImplTest, DeterminedByRuntimeFraction) {
   envoy::config::filter::network::redis_proxy::v2::RedisProxy::PrefixRoutes::Route::
       RequestMirrorPolicy config;