feat: Validate unexpected config values (#2457)

docs/configure-clio.md

@@ -88,13 +88,15 @@ Exactly equal password gains admin rights for the request or a websocket connect
 Clio can cache requests to ETL sources to reduce the load on the ETL source.
 Only following commands are cached: `server_info`, `server_state`, `server_definitions`, `fee`, `ledger_closed`.
 By default the forwarding cache is off.
-To enable the caching for a source, `forwarding_cache_timeout` value should be added to the configuration file, e.g.:
+To enable the caching for a source, `forwarding.cache_timeout` value should be added to the configuration file, e.g.:
 
 ```json
-"forwarding_cache_timeout": 0.250,
+"forwarding": {
+    "cache_timeout": 0.250,
+}
 ```
 
-`forwarding_cache_timeout` defines for how long (in seconds) a cache entry will be valid after being placed into the cache.
+`forwarding.cache_timeout` defines for how long (in seconds) a cache entry will be valid after being placed into the cache.
 Zero value turns off the cache feature.
 
 ## Graceful shutdown (not fully implemented yet)

src/util/config/ConfigDefinition.cpp

@@ -237,6 +237,12 @@ ClioConfigDefinition::parse(ConfigFileInterface const& config)
         });
     }
 
+    for (auto const& key : config.getAllKeys()) {
+        if (!map_.contains(key) && !arrayPrefixesToKeysMap.contains(key)) {
+            listOfErrors.emplace_back("Unknown key: " + key);
+        }
+    }
+
     if (!listOfErrors.empty())
         return listOfErrors;
 

src/util/config/ConfigDefinition.hpp

@@ -19,7 +19,6 @@
 
 #pragma once
 
-#include "rpc/common/APIVersion.hpp"
 #include "util/Assert.hpp"
 #include "util/config/Array.hpp"
 #include "util/config/ConfigConstraints.hpp"
@@ -27,18 +26,15 @@
 #include "util/config/ConfigValue.hpp"
 #include "util/config/Error.hpp"
 #include "util/config/ObjectView.hpp"
-#include "util/config/Types.hpp"
 #include "util/config/ValueView.hpp"
 
 #include <algorithm>
 #include <chrono>
 #include <cstddef>
-#include <cstdint>
 #include <initializer_list>
 #include <optional>
 #include <string>
 #include <string_view>
-#include <thread>
 #include <unordered_map>
 #include <utility>
 #include <variant>

src/util/config/ConfigFileInterface.hpp

@@ -22,6 +22,7 @@
 #include "util/config/Types.hpp"
 
 #include <optional>
+#include <string>
 #include <string_view>
 #include <vector>
 
@@ -63,6 +64,14 @@ public:
      */
     virtual bool
     containsKey(std::string_view key) const = 0;
+
+    /**
+     * @brief Retrieves all keys in the configuration file.
+     *
+     * @return A vector of all keys in the configuration file.
+     */
+    virtual std::vector<std::string>
+    getAllKeys() const = 0;
 };
 
 }  // namespace util::config

src/util/config/ConfigFileJson.cpp

@@ -142,6 +142,16 @@ ConfigFileJson::containsKey(std::string_view key) const
     return jsonObject_.contains(key);
 }
 
+std::vector<std::string>
+ConfigFileJson::getAllKeys() const
+{
+    std::vector<std::string> keys;
+    for (auto const& [key, value] : jsonObject_) {
+        keys.push_back(key);
+    }
+    return keys;
+}
+
 boost::json::object const&
 ConfigFileJson::inner() const
 {

src/util/config/ConfigFileJson.hpp

@@ -28,6 +28,7 @@
 #include <expected>
 #include <filesystem>
 #include <optional>
+#include <string>
 #include <string_view>
 #include <vector>
 
@@ -73,6 +74,14 @@ public:
     [[nodiscard]] bool
     containsKey(std::string_view key) const override;
 
+    /**
+     * @brief Retrieves all keys in the configuration file.
+     *
+     * @return A vector of all keys in the configuration file.
+     */
+    [[nodiscard]] std::vector<std::string>
+    getAllKeys() const override;
+
     /**
      * @brief Creates a new ConfigFileJson by parsing the provided JSON file and
      * stores the values in the object.

src/util/config/ConfigFileYaml.hpp

@@ -1,49 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of clio: https://github.com/XRPLF/clio
-    Copyright (c) 2024, the clio developers.
-
-    Permission to use, copy, modify, and distribute this software for any
-    purpose with or without fee is hereby granted, provided that the above
-    copyright notice and this permission notice appear in all copies.
-
-    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
-    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
-    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
-    ANY  SPECIAL,  DIRECT,  INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
-    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
-    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
-    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
-*/
-//==============================================================================
-
-#pragma once
-
-#include "util/config/ConfigFileInterface.hpp"
-#include "util/config/Types.hpp"
-
-#include <boost/filesystem/path.hpp>
-
-#include <string_view>
-#include <vector>
-
-// TODO: implement when we support yaml
-
-namespace util::config {
-
-/** @brief Yaml representation of config */
-class ConfigFileYaml final : public ConfigFileInterface {
-public:
-    ConfigFileYaml() = default;
-
-    Value
-    getValue(std::string_view key) const override;
-
-    std::vector<Value>
-    getArray(std::string_view key) const override;
-
-    bool
-    containsKey(std::string_view key) const override;
-};
-
-}  // namespace util::config

tests/unit/data/cassandra/SettingsProviderTests.cpp

@@ -81,7 +81,8 @@ class SettingsProviderTest : public NoLoggerFixture {};
 
 TEST_F(SettingsProviderTest, Defaults)
 {
-    auto const cfg = getParseSettingsConfig(json::parse(R"JSON({"contact_points": "127.0.0.1"})JSON"));
+    auto const cfg =
+        getParseSettingsConfig(json::parse(R"JSON({"database.cassandra.contact_points": "127.0.0.1"})JSON"));
     SettingsProvider const provider{cfg.getObject("database.cassandra")};
 
     auto const settings = provider.getSettings();

tests/unit/etl/LoadBalancerTests.cpp

@@ -160,7 +160,7 @@ TEST_F(LoadBalancerConstructorTests, construct)
 TEST_F(LoadBalancerConstructorTests, forwardingTimeoutPassedToSourceFactory)
 {
     auto const forwardingTimeout = 10;
-    configJson_.as_object()["forwarding"] = boost::json::object{{"timeout", float{forwardingTimeout}}};
+    configJson_.as_object()["forwarding"] = boost::json::object{{"cache_timeout", float{forwardingTimeout}}};
     EXPECT_CALL(
         sourceFactory_,
         makeSource(

tests/unit/etlng/LoadBalancerTests.cpp

@@ -182,7 +182,7 @@ TEST_F(LoadBalancerConstructorNgTests, construct)
 TEST_F(LoadBalancerConstructorNgTests, forwardingTimeoutPassedToSourceFactory)
 {
     auto const forwardingTimeout = 10;
-    configJson_.as_object()["forwarding"] = boost::json::object{{"timeout", float{forwardingTimeout}}};
+    configJson_.as_object()["forwarding"] = boost::json::object{{"cache_timeout", float{forwardingTimeout}}};
     EXPECT_CALL(
         sourceFactory_,
         makeSource(

tests/unit/util/config/ClioConfigDefinitionTests.cpp

@@ -465,3 +465,35 @@ TEST_F(ClioConfigDefinitionParseArrayTest, missingAllRequiredFields)
     EXPECT_EQ(result->size(), 1);
     EXPECT_THAT(result->at(0).error, testing::StartsWith("array.[].int"));
 }
+
+TEST(ClioConfigDefinitionParse, unexpectedFields)
+{
+    ClioConfigDefinition config{
+        {"expected", ConfigValue{ConfigType::String}.optional()},
+    };
+
+    auto const configJson = boost::json::parse(R"JSON({
+        "expected": "present",
+        "unexpected_string": "",
+        "unexpected_non_empty_array": [
+            {"string": ""},
+            {"string": ""}
+        ],
+        "unexpected_empty_array": [],
+        "unexpected_object": {
+            "string": ""
+        }
+    })JSON")
+                                .as_object();
+
+    auto const configFile = ConfigFileJson{configJson};
+    auto result = config.parse(configFile);
+    std::ranges::sort(*result, [](auto const& lhs, auto const& rhs) { return lhs.error < rhs.error; });
+    ASSERT_TRUE(result.has_value());
+    ASSERT_EQ(result->size(), 4);
+
+    EXPECT_EQ(result->at(0).error, "Unknown key: unexpected_empty_array.[]");
+    EXPECT_EQ(result->at(1).error, "Unknown key: unexpected_non_empty_array.[].string");
+    EXPECT_EQ(result->at(2).error, "Unknown key: unexpected_object.string");
+    EXPECT_EQ(result->at(3).error, "Unknown key: unexpected_string");
+}

tests/unit/util/config/ConfigFileJsonTests.cpp

@@ -31,6 +31,7 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <ranges>
 #include <string>
 #include <unordered_map>
 #include <variant>
@@ -488,6 +489,31 @@ TEST_F(ConfigFileJsonTest, containsKey)
     EXPECT_FALSE(jsonFileObj.containsKey("array_of_objects.[].object"));
 }
 
+TEST_F(ConfigFileJsonTest, getAllKeys)
+{
+    auto const jsonStr = R"JSON({
+        "int": 42,
+        "object": { "string": "some string", "array": [1, 2, 3] },
+        "array2": [1, 2, 3],
+        "array_of_objects": [ {"int": 42}, {"string": "some string"} ]
+    })JSON";
+    auto const jsonFileObj = ConfigFileJson{boost::json::parse(jsonStr).as_object()};
+
+    auto allKeys = jsonFileObj.getAllKeys();
+    std::ranges::sort(allKeys);
+    EXPECT_EQ(allKeys.size(), 6);
+
+    std::vector<std::string> const expectedKeys{
+        {"array2.[]",
+         "array_of_objects.[].int",
+         "array_of_objects.[].string",
+         "int",
+         "object.array.[]",
+         "object.string"}
+    };
+    EXPECT_EQ(allKeys, expectedKeys);
+}
+
 struct ConfigFileJsonMakeTest : ConfigFileJsonTest {};
 
 TEST_F(ConfigFileJsonMakeTest, invalidFile)