runtime: Add boolean parsing functionality (envoyproxy#8476)

Signed-off-by: Tony Allen <[email protected]>

docs/root/intro/version_history.rst

@@ -65,6 +65,7 @@ Version history
 * router check tool: add deprecated field check.
 * router check tool: add flag for only printing results of failed tests.
 * router check tool: add support for outputting missing tests in the detailed coverage report.
+* runtime: allow for the ability to parse boolean values.
 * runtime: allow for the ability to parse integers as double values and vice-versa.
 * server: added a post initialization lifecycle event, in addition to the existing startup and shutdown events.
 * server: added :ref:`per-handler listener stats <config_listener_stats_per_handler>` and

include/envoy/runtime/runtime.h

@@ -205,6 +205,15 @@ class Snapshot {
    */
   virtual double getDouble(const std::string& key, double default_value) const PURE;
 
+  /**
+   * Fetch a boolean runtime key.
+   * @param key supplies the key to fetch.
+   * @param default_value supplies the value to return if the key does not exist or it does not
+   *        contain a boolean.
+   * @return bool the runtime value or the default value.
+   */
+  virtual bool getBoolean(absl::string_view key, bool default_value) const PURE;
+
   /**
    * Fetch the OverrideLayers that provide values in this snapshot. Layers are ordered from bottom
    * to top; for instance, the second layer's entries override the first layer's entries, and so on.

source/common/runtime/runtime_impl.cc

@@ -172,14 +172,11 @@ std::string RandomGeneratorImpl::uuid() {
 }
 
 bool SnapshotImpl::deprecatedFeatureEnabled(const std::string& key) const {
-  bool allowed = false;
+  const bool default_allowed = !RuntimeFeaturesDefaults::get().disallowedByDefault(key);
+
   // If the value is not explicitly set as a runtime boolean, the default value is based on
   // disallowedByDefault.
-  if (!getBoolean(key, allowed)) {
-    allowed = !RuntimeFeaturesDefaults::get().disallowedByDefault(key);
-  }
-
-  if (!allowed) {
+  if (!getBoolean(key, default_allowed)) {
     // If either disallowed by default or configured off, the feature is not enabled.
     return false;
   }
@@ -195,14 +192,9 @@ bool SnapshotImpl::deprecatedFeatureEnabled(const std::string& key) const {
 }
 
 bool SnapshotImpl::runtimeFeatureEnabled(absl::string_view key) const {
-  bool enabled = false;
   // If the value is not explicitly set as a runtime boolean, the default value is based on
-  // disallowedByDefault.
-  if (!getBoolean(key, enabled)) {
-    enabled = RuntimeFeaturesDefaults::get().enabledByDefault(key);
-  }
-
-  return enabled;
+  // enabledByDefault.
+  return getBoolean(key, RuntimeFeaturesDefaults::get().enabledByDefault(key));
 }
 
 bool SnapshotImpl::featureEnabled(const std::string& key, uint64_t default_value,
@@ -286,13 +278,13 @@ double SnapshotImpl::getDouble(const std::string& key, double default_value) con
   }
 }
 
-bool SnapshotImpl::getBoolean(absl::string_view key, bool& value) const {
+bool SnapshotImpl::getBoolean(absl::string_view key, bool default_value) const {
   auto entry = values_.find(key);
-  if (entry != values_.end() && entry->second.bool_value_.has_value()) {
-    value = entry->second.bool_value_.value();
-    return true;
+  if (entry == values_.end() || !entry->second.bool_value_.has_value()) {
+    return default_value;
+  } else {
+    return entry->second.bool_value_.value();
   }
-  return false;
 }
 
 const std::vector<Snapshot::OverrideLayerConstPtr>& SnapshotImpl::getLayers() const {

source/common/runtime/runtime_impl.h

@@ -93,15 +93,12 @@ class SnapshotImpl : public Snapshot,
   const std::string& get(const std::string& key) const override;
   uint64_t getInteger(const std::string& key, uint64_t default_value) const override;
   double getDouble(const std::string& key, double default_value) const override;
+  bool getBoolean(absl::string_view key, bool value) const override;
   const std::vector<OverrideLayerConstPtr>& getLayers() const override;
 
   static Entry createEntry(const std::string& value);
   static Entry createEntry(const ProtobufWkt::Value& value);
 
-  // Returns true and sets 'value' to the key if found.
-  // Returns false if the key is not a boolean value.
-  bool getBoolean(absl::string_view key, bool& value) const;
-
 private:
   static void resolveEntryType(Entry& entry) {
     if (parseEntryBooleanValue(entry)) {

test/common/runtime/runtime_impl_test.cc

@@ -176,8 +176,7 @@ TEST_F(DiskLoaderImplTest, All) {
   // Valid float string followed by newlines.
   EXPECT_EQ(3.141, loader_->snapshot().getDouble("file_with_double_newlines", 1.1));
 
-  bool value;
-  const SnapshotImpl* snapshot = reinterpret_cast<const SnapshotImpl*>(&loader_->snapshot());
+  const auto snapshot = reinterpret_cast<const SnapshotImpl*>(&loader_->snapshot());
 
   // Validate that the layer name is set properly for static layers.
   EXPECT_EQ("base", snapshot->getLayers()[0]->name());
@@ -186,14 +185,18 @@ TEST_F(DiskLoaderImplTest, All) {
   EXPECT_EQ("admin", snapshot->getLayers()[3]->name());
 
   // Boolean getting.
-  EXPECT_EQ(true, snapshot->getBoolean("file11", value));
-  EXPECT_EQ(true, value);
-  EXPECT_EQ(true, snapshot->getBoolean("file12", value));
-  EXPECT_EQ(false, value);
-  EXPECT_EQ(true, snapshot->getBoolean("file13", value));
-  EXPECT_EQ(true, value);
-  // File1 is not a boolean.
-  EXPECT_EQ(false, snapshot->getBoolean("file1", value));
+  // Lower-case boolean specification.
+  EXPECT_EQ(true, snapshot->getBoolean("file11", false));
+  EXPECT_EQ(true, snapshot->getBoolean("file11", true));
+  // Mixed-case boolean specification.
+  EXPECT_EQ(false, snapshot->getBoolean("file12", true));
+  EXPECT_EQ(false, snapshot->getBoolean("file12", false));
+  // Lower-case boolean specification with leading whitespace.
+  EXPECT_EQ(true, snapshot->getBoolean("file13", true));
+  EXPECT_EQ(true, snapshot->getBoolean("file13", false));
+  // File1 is not a boolean. Should take default.
+  EXPECT_EQ(true, snapshot->getBoolean("file1", true));
+  EXPECT_EQ(false, snapshot->getBoolean("file1", false));
 
   // Feature defaults.
   // test_feature_true is explicitly set true in runtime_features.cc
@@ -603,19 +606,22 @@ TEST_F(StaticLoaderImplTest, ProtoParsing) {
   EXPECT_EQ(23.2, loader_->snapshot().getDouble("file_with_double", 1.1));
 
   // Boolean getting.
-  bool value;
-  const SnapshotImpl* snapshot = reinterpret_cast<const SnapshotImpl*>(&loader_->snapshot());
-
-  EXPECT_EQ(true, snapshot->getBoolean("file11", value));
-  EXPECT_EQ(true, value);
-  EXPECT_EQ(true, snapshot->getBoolean("file12", value));
-  EXPECT_EQ(false, value);
-  EXPECT_EQ(true, snapshot->getBoolean("file13", value));
-  EXPECT_EQ(false, value);
-  // File1 is not a boolean.
-  EXPECT_EQ(false, snapshot->getBoolean("file1", value));
-  // Neither is blah.blah
-  EXPECT_EQ(false, snapshot->getBoolean("blah.blah", value));
+  const auto snapshot = reinterpret_cast<const SnapshotImpl*>(&loader_->snapshot());
+
+  EXPECT_EQ(true, snapshot->getBoolean("file11", true));
+  EXPECT_EQ(true, snapshot->getBoolean("file11", false));
+
+  EXPECT_EQ(false, snapshot->getBoolean("file12", true));
+  EXPECT_EQ(false, snapshot->getBoolean("file12", false));
+
+  EXPECT_EQ(false, snapshot->getBoolean("file13", true));
+  EXPECT_EQ(false, snapshot->getBoolean("file13", false));
+
+  // Not a boolean. Expect the default.
+  EXPECT_EQ(true, snapshot->getBoolean("file1", true));
+  EXPECT_EQ(false, snapshot->getBoolean("file1", false));
+  EXPECT_EQ(true, snapshot->getBoolean("blah.blah", true));
+  EXPECT_EQ(false, snapshot->getBoolean("blah.blah", false));
 
   // Fractional percent feature enablement
   envoy::type::FractionalPercent fractional_percent;

test/mocks/runtime/mocks.h

@@ -55,6 +55,7 @@ class MockSnapshot : public Snapshot {
   MOCK_CONST_METHOD1(get, const std::string&(const std::string& key));
   MOCK_CONST_METHOD2(getInteger, uint64_t(const std::string& key, uint64_t default_value));
   MOCK_CONST_METHOD2(getDouble, double(const std::string& key, double default_value));
+  MOCK_CONST_METHOD2(getBoolean, bool(absl::string_view key, bool default_value));
   MOCK_CONST_METHOD0(getLayers, const std::vector<OverrideLayerConstPtr>&());
 };