diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 8c9a157748c5..b28f690b2564 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -64,6 +64,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 ` and diff --git a/include/envoy/runtime/runtime.h b/include/envoy/runtime/runtime.h index e4ca1ed18a7d..46ba9ba57498 100644 --- a/include/envoy/runtime/runtime.h +++ b/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. diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index 22a0508bc56a..060a77b73d4d 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/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& SnapshotImpl::getLayers() const { diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index f5276961886d..352e1b4d0c5a 100644 --- a/source/common/runtime/runtime_impl.h +++ b/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& 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)) { diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 182b92d1ef5d..0a9350e47971 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/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(&loader_->snapshot()); + const auto snapshot = reinterpret_cast(&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(&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(&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; diff --git a/test/mocks/runtime/mocks.h b/test/mocks/runtime/mocks.h index 8e55cc6732d3..19416df01257 100644 --- a/test/mocks/runtime/mocks.h +++ b/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&()); };