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

…ater than zero (envoyproxy#8089) Signed-off-by: Nicolas Flacco <nflacco@lyft.com>
danzh2010 · Sep 24, 2019 · 2869f2d · 2869f2d
1 parent 96c8000
commit 2869f2d
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;