From 3f2ee8d6da632251df98e2c092faa18cfcd3392f Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 4 Mar 2017 16:49:24 +0100 Subject: [PATCH] [5145b] get rid of D2ClientConfig::isShortCutDisabled --- src/bin/dhcp4/json_config_parser.cc | 6 +- src/bin/dhcp6/json_config_parser.cc | 6 +- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 74 ++++++++----------- src/lib/dhcpsrv/parsers/dhcp_parsers.h | 11 --- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 3 +- 5 files changed, 34 insertions(+), 66 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 5b6102f370..f809d742b6 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -520,10 +520,8 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set, // Legacy DhcpConfigParser stuff below if (config_pair.first == "dhcp-ddns") { - // Apply defaults if not in short cut - if (!D2ClientConfigParser::isShortCutDisabled(config_pair.second)) { - D2ClientConfigParser::setAllDefaults(config_pair.second); - } + // Apply defaults + D2ClientConfigParser::setAllDefaults(config_pair.second); D2ClientConfigParser parser; D2ClientConfigPtr cfg = parser.parse(config_pair.second); srv_cfg->setD2ClientConfig(cfg); diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index c029895149..59b021d65f 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -742,10 +742,8 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set, } if (config_pair.first == "dhcp-ddns") { - // Apply defaults if not in short cut - if (!D2ClientConfigParser::isShortCutDisabled(config_pair.second)) { - D2ClientConfigParser::setAllDefaults(config_pair.second); - } + // Apply defaults + D2ClientConfigParser::setAllDefaults(config_pair.second); D2ClientConfigParser parser; D2ClientConfigPtr cfg = parser.parse(config_pair.second); srv_config->setD2ClientConfig(cfg); diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index f139696a56..0073a35d17 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -989,25 +989,8 @@ D2ClientConfigPtr D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { D2ClientConfigPtr new_config; - if (isShortCutDisabled(client_config)) { - // If enable-updates is the only parameter and it is false then - // we're done. This allows for an abbreviated configuration entry - // that only contains that flag. Use the default D2ClientConfig - // constructor to a create a disabled instance. - new_config.reset(new D2ClientConfig()); - return (new_config); - } - - // As isShortCutDisabled() was called this cannot fail - bool enable_updates = client_config->get("enable-updates")->boolValue(); - // Get all parameters that are needed to create the D2ClientConfig. - std::string qualifying_suffix; - bool found_qualifying_suffix = false; - if (client_config->contains("qualifying-suffix")) { - qualifying_suffix = getString(client_config, "qualifying-suffix"); - found_qualifying_suffix = true; - } + bool enable_updates = getBoolean(client_config, "enable-updates"); IOAddress server_ip = getIOAddress(client_config, "server-ip"); @@ -1040,14 +1023,13 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { std::string generated_prefix = getString(client_config, "generated-prefix"); - - // Qualifying-suffix is required when updates are enabled - if (enable_updates && !found_qualifying_suffix) { - isc_throw(DhcpConfigError, - "parameter 'qualifying-suffix' is required when " - "updates are enabled (" - << client_config->getPosition() << ")"); - } + // qualifying-suffix is the only parameter which has no default + std::string qualifying_suffix = ""; + bool found_qualifying_suffix = false; + if (client_config->contains("qualifying-suffix")) { + qualifying_suffix = getString(client_config, "qualifying-suffix"); + found_qualifying_suffix = true; + } IOAddress sender_ip(0); if (sender_ip_str.empty()) { @@ -1064,6 +1046,14 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { } } + // Qualifying-suffix is required when updates are enabled + if (enable_updates && !found_qualifying_suffix) { + isc_throw(DhcpConfigError, + "parameter 'qualifying-suffix' is required when " + "updates are enabled (" + << client_config->getPosition() << ")"); + } + // Now we check for logical errors. This repeats what is done in // D2ClientConfig::validate(), but doing it here permits us to // emit meaningful parameter position info in the error. @@ -1102,19 +1092,19 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { try { // Attempt to create the new client config. new_config.reset(new D2ClientConfig(enable_updates, - server_ip, - server_port, - sender_ip, - sender_port, - max_queue_size, - ncr_protocol, - ncr_format, - always_include_fqdn, - override_no_update, - override_client_update, - replace_client_name_mode, - generated_prefix, - qualifying_suffix)); + server_ip, + server_port, + sender_ip, + sender_port, + max_queue_size, + ncr_protocol, + ncr_format, + always_include_fqdn, + override_no_update, + override_client_update, + replace_client_name_mode, + generated_prefix, + qualifying_suffix)); } catch (const std::exception& ex) { isc_throw(DhcpConfigError, ex.what() << " (" @@ -1124,12 +1114,6 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { return(new_config); } -bool -D2ClientConfigParser::isShortCutDisabled(isc::data::ConstElementPtr d2_config) { - bool value = getBoolean(d2_config, "enable-updates"); - return (!value && (d2_config->mapValue().size() == 1)); -} - /// @brief This table defines default values for D2 client configuration const SimpleDefaults D2ClientConfigParser::D2_CLIENT_CONFIG_DEFAULTS = { // enable-updates is unconditionally required diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 6f43a8f013..d84d976ec7 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -761,17 +761,6 @@ public: /// @return returns a pointer to newly created D2ClientConfig. D2ClientConfigPtr parse(isc::data::ConstElementPtr d2_client_cfg); - /// @brief Check the short cut disabled updates condition - /// - /// The condition is that the d2 client configuration is - /// reduced to "enable-updates": false - /// - /// @param d2_config d2 client configuration - /// @return true if and only if the condition matches. - /// @throw DhcpConfigError if enable-updates is not present or - /// is not a boolean - static bool isShortCutDisabled(isc::data::ConstElementPtr d2_config); - /// @brief Defaults for the D2 client configuration. static const isc::data::SimpleDefaults D2_CLIENT_CONFIG_DEFAULTS; diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 810f3dec0a..e8c48e709d 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -530,8 +530,7 @@ public: /// D2 client configuration code is in this library ConstElementPtr d2_client = config->get("dhcp-ddns"); - if (d2_client && - !D2ClientConfigParser::isShortCutDisabled(d2_client)) { + if (d2_client) { D2ClientConfigParser::setAllDefaults(d2_client); } }