[v23.3.x] allow property aliases in redpanda.yaml, fix aliases not being applied

src/v/cluster/config_manager.cc

@@ -413,7 +413,7 @@ config_manager::preload(YAML::Node const& legacy_config) {
         // to set something in redpanda.yaml and it's not working.
         if (legacy_config["redpanda"]) {
             const auto nag_properties
-              = config::shard_local_cfg().property_names();
+              = config::shard_local_cfg().property_names_and_aliases();
             for (auto const& node : legacy_config["redpanda"]) {
                 auto name = node.first.as<ss::sstring>();
                 if (nag_properties.contains(name)) {

src/v/config/config_store.h

@@ -70,42 +70,50 @@ class config_store {
 
         for (auto const& node : root_node) {
             auto name = node.first.as<ss::sstring>();
-            auto found = _properties.find(name);
-            if (found == _properties.end()) {
-                found = _aliases.find(name);
-                if (found == _aliases.end()) {
-                    if (!ignore_missing.contains(name)) {
-                        throw std::invalid_argument(
-                          fmt::format("Unknown property {}", name));
-                    }
+            auto* prop = [&]() -> base_property* {
+                auto found = _properties.find(name);
+                if (found != _properties.end()) {
+                    return found->second;
                 }
-            } else {
-                bool ok = false;
-                try {
-                    auto validation_err = found->second->validate(node.second);
-                    if (validation_err.has_value()) {
-                        errors[name] = fmt::format(
-                          "Validation error: {}",
-                          validation_err.value().error_message());
-                    }
-
-                    found->second->set_value(node.second);
-                    ok = true;
-                } catch (YAML::InvalidNode const& e) {
-                    errors[name] = fmt::format("Invalid syntax: {}", e);
-                } catch (YAML::ParserException const& e) {
-                    errors[name] = fmt::format("Invalid syntax: {}", e);
-                } catch (YAML::BadConversion const& e) {
-                    errors[name] = fmt::format("Invalid value: {}", e);
+                found = _aliases.find(name);
+                if (found != _aliases.end()) {
+                    return found->second;
                 }
 
-                // A validation error is fatal if the property was required,
-                // e.g. if someone entered a non-integer node_id, or an invalid
-                // internal RPC address.
-                if (!ok && found->second->is_required()) {
-                    throw std::invalid_argument(fmt::format(
-                      "Property {} is required and has invalid value", name));
+                return nullptr;
+            }();
+
+            if (prop == nullptr) {
+                if (!ignore_missing.contains(name)) {
+                    throw std::invalid_argument(
+                      fmt::format("Unknown property {}", name));
                 }
+                continue;
+            }
+            bool ok = false;
+            try {
+                auto validation_err = prop->validate(node.second);
+                if (validation_err.has_value()) {
+                    errors[name] = fmt::format(
+                      "Validation error: {}",
+                      validation_err.value().error_message());
+                }
+                prop->set_value(node.second);
+                ok = true;
+            } catch (YAML::InvalidNode const& e) {
+                errors[name] = fmt::format("Invalid syntax: {}", e);
+            } catch (YAML::ParserException const& e) {
+                errors[name] = fmt::format("Invalid syntax: {}", e);
+            } catch (YAML::BadConversion const& e) {
+                errors[name] = fmt::format("Invalid value: {}", e);
+            }
+
+            // A validation error is fatal if the property was required,
+            // e.g. if someone entered a non-integer node_id, or an invalid
+            // internal RPC address.
+            if (!ok && prop->is_required()) {
+                throw std::invalid_argument(fmt::format(
+                  "Property {} is required and has invalid value", name));
             }
         }
 
@@ -186,6 +194,21 @@ class config_store {
         return result;
     }
 
+    std::set<std::string_view> property_aliases() const {
+        std::set<std::string_view> result;
+        for (const auto& i : _aliases) {
+            result.insert(i.first);
+        }
+
+        return result;
+    }
+
+    std::set<std::string_view> property_names_and_aliases() const {
+        auto all = property_names();
+        all.merge(property_aliases());
+        return all;
+    }
+
     friend std::ostream&
     operator<<(std::ostream& o, const config::config_store& c) {
         o << "{ ";

src/v/config/configuration.cc

@@ -2867,9 +2867,9 @@ configuration::error_map_t configuration::load(const YAML::Node& root_node) {
         throw std::invalid_argument("'redpanda' root is required");
     }
 
-    const auto& ignore = node().property_names();
+    auto ignore = node().property_names_and_aliases();
 
-    return config_store::read_yaml(root_node["redpanda"], ignore);
+    return config_store::read_yaml(root_node["redpanda"], std::move(ignore));
 }
 
 configuration& shard_local_cfg() {

src/v/config/node_config.cc

@@ -245,9 +245,10 @@ node_config::error_map_t node_config::load(const YAML::Node& root_node) {
         throw std::invalid_argument("'redpanda' root is required");
     }
 
-    const auto& ignore = shard_local_cfg().property_names();
+    auto ignore = shard_local_cfg().property_names_and_aliases();
 
-    auto errors = config_store::read_yaml(root_node["redpanda"], ignore);
+    auto errors = config_store::read_yaml(
+      root_node["redpanda"], std::move(ignore));
     validate_multi_node_property_config(errors);
     return errors;
 }

src/v/config/tests/config_store_test.cc

@@ -136,7 +136,8 @@ YAML::Node valid_configuration() {
                       " - two\n"
                       " - three\n"
                       "nullable_int: 111\n"
-                      "secret_string: actual_secret\n");
+                      "secret_string: actual_secret\n"
+                      "aliased_bool_legacy: false\n");
 }
 
 } // namespace
@@ -214,6 +215,7 @@ SEASTAR_THREAD_TEST_CASE(read_valid_configuration) {
     BOOST_TEST(cfg.strings().at(2) == "three");
     BOOST_TEST(cfg.nullable_int() == std::make_optional(111));
     BOOST_TEST(cfg.secret_string() == "actual_secret");
+    BOOST_TEST(cfg.aliased_bool() == false);
 };
 
 SEASTAR_THREAD_TEST_CASE(update_property_value) {
@@ -525,4 +527,6 @@ SEASTAR_THREAD_TEST_CASE(property_aliasing) {
     auto property_names = cfg.property_names();
     BOOST_TEST(property_names.contains("aliased_bool") == true);
     BOOST_TEST(property_names.contains("aliased_bool_legacy") == false);
+
+    BOOST_TEST(cfg.property_names_and_aliases().contains("aliased_bool"));
 }