diff --git a/src/v/cluster/config_manager.cc b/src/v/cluster/config_manager.cc index d0b23804b0f0d..e83d380431b1a 100644 --- a/src/v/cluster/config_manager.cc +++ b/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(); if (nag_properties.contains(name)) { diff --git a/src/v/config/config_store.h b/src/v/config/config_store.h index 2d6d522cda8e1..254e23e696c8a 100644 --- a/src/v/config/config_store.h +++ b/src/v/config/config_store.h @@ -70,42 +70,50 @@ class config_store { for (auto const& node : root_node) { auto name = node.first.as(); - 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 property_aliases() const { + std::set result; + for (const auto& i : _aliases) { + result.insert(i.first); + } + + return result; + } + + std::set 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 << "{ "; diff --git a/src/v/config/configuration.cc b/src/v/config/configuration.cc index 04fe1e8a1b580..ed1d25a43ccde 100644 --- a/src/v/config/configuration.cc +++ b/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() { diff --git a/src/v/config/node_config.cc b/src/v/config/node_config.cc index 68918c3889e84..8aac0e3dac669 100644 --- a/src/v/config/node_config.cc +++ b/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; } diff --git a/src/v/config/tests/config_store_test.cc b/src/v/config/tests/config_store_test.cc index d8d3a858109ab..9a91d5867f4f7 100644 --- a/src/v/config/tests/config_store_test.cc +++ b/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")); }