From: Francis Dupont Date: Fri, 13 Jan 2017 14:28:52 +0000 (+0100) Subject: [5033] Fixed defaults and enable-updates=false shortcut problems X-Git-Tag: trac5112_base~6^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=568aa8f5f6d63b4b10a1040fa6918769b2d46a4a;p=thirdparty%2Fkea.git [5033] Fixed defaults and enable-updates=false shortcut problems --- diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index c8dc4f5c2a..37399cb4f2 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -637,6 +637,10 @@ configureDhcp4Server(Dhcpv4Srv&, 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); + } D2ClientConfigParser parser; D2ClientConfigPtr cfg = parser.parse(config_pair.second); CfgMgr::instance().getStagingCfg()->setD2ClientConfig(cfg); diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc index 9a581a6bfb..b1c36bfbe0 100644 --- a/src/bin/dhcp4/simple_parser4.cc +++ b/src/bin/dhcp4/simple_parser4.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -71,27 +71,6 @@ const ParamsList SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4 = { "rebind-timer", "valid-lifetime" }; - -/// @brief This table defines default values for D2 client configuration -/// -const SimpleDefaults SimpleParser4::D2_CLIENT_CONFIG_DEFAULTS = { - { "server-ip", Element::string, "127.0.0.1" }, - { "server-port", Element::integer, "53001" }, - // default sender-ip depends on server-ip family, so we leave default blank - // parser knows to use the appropriate ZERO address based on server-ip - { "sender-ip", Element::string, "" }, - { "sender-port", Element::integer, "0" }, - { "max-queue-size", Element::integer, "1024" }, - { "ncr-protocol", Element::string, "UDP" }, - { "ncr-format", Element::string, "JSON" }, - { "always-include-fqdn", Element::boolean, "false" }, - { "override-no-update", Element::boolean, "false" }, - { "override-client-update", Element::boolean, "false" }, - { "replace-client-name", Element::string, "NEVER" }, - { "generated-prefix", Element::string, "myhost" }, - { "qualifying-suffix", Element::string, "" } -}; - /// @} /// --------------------------------------------------------------------------- @@ -120,14 +99,6 @@ size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) { } } - ConstElementPtr d2_client = global->get("dhcp-ddns"); - /// @todo - what if it's not in global? should we add it? - if (d2_client) { - // Get the mutable form of d2 client config - ElementPtr mutable_d2 = boost::const_pointer_cast(d2_client); - cnt += SimpleParser::setDefaults(mutable_d2, D2_CLIENT_CONFIG_DEFAULTS); - } - return (cnt); } diff --git a/src/bin/dhcp4/simple_parser4.h b/src/bin/dhcp4/simple_parser4.h index abc842b108..18524f9190 100644 --- a/src/bin/dhcp4/simple_parser4.h +++ b/src/bin/dhcp4/simple_parser4.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -33,7 +33,6 @@ public: static const isc::data::SimpleDefaults OPTION4_DEFAULTS; static const isc::data::SimpleDefaults GLOBAL4_DEFAULTS; static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET4; - static const isc::data::SimpleDefaults D2_CLIENT_CONFIG_DEFAULTS; }; }; diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 9512eba44e..097be389ab 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -911,6 +911,10 @@ 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); + } D2ClientConfigParser parser; D2ClientConfigPtr cfg = parser.parse(config_pair.second); CfgMgr::instance().getStagingCfg()->setD2ClientConfig(cfg); diff --git a/src/bin/dhcp6/simple_parser6.cc b/src/bin/dhcp6/simple_parser6.cc index 1840993fdc..aa2844d939 100644 --- a/src/bin/dhcp6/simple_parser6.cc +++ b/src/bin/dhcp6/simple_parser6.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -73,27 +73,6 @@ const ParamsList SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6 = { "preferred-lifetime", "valid-lifetime" }; - -/// @brief This table defines default values for D2 client configuration -/// -const SimpleDefaults SimpleParser6::D2_CLIENT_CONFIG_DEFAULTS = { - { "server-ip", Element::string, "127.0.0.1" }, - { "server-port", Element::integer, "53001" }, - // default sender-ip depends on server-ip family, so we leave default blank - // parser knows to use the appropriate ZERO address based on server-ip - { "sender-ip", Element::string, "" }, - { "sender-port", Element::integer, "0" }, - { "max-queue-size", Element::integer, "1024" }, - { "ncr-protocol", Element::string, "UDP" }, - { "ncr-format", Element::string, "JSON" }, - { "always-include-fqdn", Element::boolean, "false" }, - { "override-no-update", Element::boolean, "false" }, - { "override-client-update", Element::boolean, "false" }, - { "replace-client-name", Element::string, "NEVER" }, - { "generated-prefix", Element::string, "myhost" }, - { "qualifying-suffix", Element::string, "" } -}; - /// @} /// --------------------------------------------------------------------------- @@ -122,14 +101,6 @@ size_t SimpleParser6::setAllDefaults(isc::data::ElementPtr global) { } } - ConstElementPtr d2_client = global->get("dhcp-ddns"); - /// @todo - what if it's not in global? should we add it? - if (d2_client) { - // Get the mutable form of d2 client config - ElementPtr mutable_d2 = boost::const_pointer_cast(d2_client); - cnt += SimpleParser::setDefaults(mutable_d2, D2_CLIENT_CONFIG_DEFAULTS); - } - return (cnt); } diff --git a/src/bin/dhcp6/simple_parser6.h b/src/bin/dhcp6/simple_parser6.h index 931079f025..6093afc28e 100644 --- a/src/bin/dhcp6/simple_parser6.h +++ b/src/bin/dhcp6/simple_parser6.h @@ -1,4 +1,4 @@ -// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -34,7 +34,6 @@ public: static const isc::data::SimpleDefaults OPTION6_DEFAULTS; static const isc::data::SimpleDefaults GLOBAL6_DEFAULTS; static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET6; - static const isc::data::SimpleDefaults D2_CLIENT_CONFIG_DEFAULTS; }; }; diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 93b5af4746..0b241b50b3 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -1257,6 +1257,16 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { std::string qualifying_suffix; std::string current_param; + + 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); + } + // Get all parameters that are needed to create the D2ClientConfig. // We fetch all the parameters inside their own try clause so we // spit out an error with an accurate text position. Use the @@ -1388,5 +1398,47 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { return(new_config); } +bool +D2ClientConfigParser::isShortCutDisabled(isc::data::ConstElementPtr d2_config) { + if (!d2_config->contains("enable-updates")) { + isc_throw(DhcpConfigError, + "Mandatory parameter 'enable-updates' missing (" + << d2_config->getPosition() << ")"); + } + ConstElementPtr enable = d2_config->get("enable-updates"); + if (enable->getType() != Element::boolean) { + isc_throw(DhcpConfigError, + "invalid value type specified for parameter" + " 'enable-updates' (" << enable->getPosition() << ")"); + } + return (!enable->boolValue() && (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 + { "server-ip", Element::string, "127.0.0.1" }, + { "server-port", Element::integer, "53001" }, + // default sender-ip depends on server-ip family, so we leave default blank + // parser knows to use the appropriate ZERO address based on server-ip + { "sender-ip", Element::string, "" }, + { "sender-port", Element::integer, "0" }, + { "max-queue-size", Element::integer, "1024" }, + { "ncr-protocol", Element::string, "UDP" }, + { "ncr-format", Element::string, "JSON" }, + { "always-include-fqdn", Element::boolean, "false" }, + { "override-no-update", Element::boolean, "false" }, + { "override-client-update", Element::boolean, "false" }, + { "replace-client-name", Element::string, "never" }, + { "generated-prefix", Element::string, "myhost" } + // qualifying-suffix has no default +}; + +size_t +D2ClientConfigParser::setAllDefaults(isc::data::ConstElementPtr d2_config) { + ElementPtr mutable_d2 = boost::const_pointer_cast(d2_config); + return (SimpleParser::setDefaults(mutable_d2, D2_CLIENT_CONFIG_DEFAULTS)); +} + }; // namespace dhcp }; // namespace isc diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index e283fd840a..c25998c4c8 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -1053,6 +1053,30 @@ 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; + + /// @brief Sets all defaults for D2 client configuration. + /// + /// This method sets defaults value. It must not be called + /// before the short cut disabled updates condition was checked. + /// + /// @param d2_config d2 client configuration (will be const cast + // to ElementPtr) + /// @return number of parameters inserted + static size_t setAllDefaults(isc::data::ConstElementPtr d2_config); }; // Pointers to various parser objects. diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 9ebfa37192..7bd69ddde0 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -302,9 +302,6 @@ public: return (answer); } - // option parsing must be done last, so save it if we hit if first - ParserPtr option_parser; - ConfigPair config_pair; try { // Iterate over the config elements. @@ -334,14 +331,8 @@ public: // Create the parser based on element name. ParserPtr parser(createConfigParser(config_pair.first)); - // Options must be parsed last - if (config_pair.first == "option-data") { - option_parser = parser; - } else { - // Anything else we can call build straight away. - parser->build(config_pair.second); - parser->commit(); - } + parser->build(config_pair.second); + parser->commit(); } int family = parser_context_->universe_ == Option::V4 @@ -434,8 +425,7 @@ public: size_t setAllDefaults(isc::data::ElementPtr global, const SimpleDefaults& global_defaults, const SimpleDefaults& option_defaults, - const SimpleDefaults& option_def_defaults, - const SimpleDefaults& d2_client_defaults) { + const SimpleDefaults& option_def_defaults) { size_t cnt = 0; // Set global defaults first. cnt = SimpleParser::setDefaults(global, global_defaults); @@ -455,16 +445,6 @@ public: } } - ConstElementPtr d2_client = global->get("dhcp-ddns"); - /// @todo - what if it's not in global? should we add it? - if (d2_client) { - // Get the mutable form of d2 client config - ElementPtr mutable_d2 = - boost::const_pointer_cast(d2_client); - cnt += SimpleParser::setDefaults(mutable_d2, d2_client_defaults); - } - - return (cnt); } @@ -521,31 +501,19 @@ public: { "valid-lifetime", Element::integer, "7200" } }; - /// @brief This table defines default values for D2 client configuration - const SimpleDefaults D2_CLIENT_CFG_DEFAULTS = { - { "server-ip", Element::string, "127.0.0.1" }, - { "server-port", Element::integer, "53001" }, - // default sender-ip depends on server-ip family, so we leave default blank - // parser knows to use the appropriate ZERO address based on server-ip - { "sender-ip", Element::string, "" }, - { "sender-port", Element::integer, "0" }, - { "max-queue-size", Element::integer, "1024" }, - { "ncr-protocol", Element::string, "UDP" }, - { "ncr-format", Element::string, "JSON" }, - { "always-include-fqdn", Element::boolean, "false" }, - { "override-no-update", Element::boolean, "false" }, - { "override-client-update", Element::boolean, "false" }, - { "replace-client-name", Element::string, "NEVER" }, - { "generated-prefix", Element::string, "myhost" }, - { "qualifying-suffix", Element::string, "" } - }; - if (v6) { setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION6_DEFAULTS, - OPTION6_DEF_DEFAULTS, D2_CLIENT_CFG_DEFAULTS); + OPTION6_DEF_DEFAULTS); } else { setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION4_DEFAULTS, - OPTION4_DEF_DEFAULTS, D2_CLIENT_CFG_DEFAULTS); + OPTION4_DEF_DEFAULTS); + } + + /// D2 client configuration code is in this library + ConstElementPtr d2_client = config->get("dhcp-ddns"); + if (d2_client && + !D2ClientConfigParser::isShortCutDisabled(d2_client)) { + D2ClientConfigParser::setAllDefaults(d2_client); } } @@ -1868,7 +1836,38 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) { /// @brief Check various invalid D2 client configurations. TEST_F(ParseConfigTest, invalidD2Config) { + std::string invalid_shortcuts[] = { + // Must supply at least enable-updates + "{ \"dhcp-ddns\" :" + " {" + " }" + "}", + // Enable-updates must be a boolean + "{ \"dhcp-ddns\" :" + " {" + " \"enable-updates\" : 0" + " }" + "}", + // stop + "" + }; + int i = 0; + while (!invalid_shortcuts[i].empty()) { + // Verify that the configuration string parsing throws + EXPECT_THROW(parseConfiguration(invalid_shortcuts[i]), + DhcpConfigError); + i++; + } + std::string invalid_configs[] = { +#if simple_parser_get_position_got_fixed + // Must supply qualifying-suffix when updates are enabled + "{ \"dhcp-ddns\" :" + " {" + " \"enable-updates\" : true" + " }" + "}", +#endif // Invalid server ip value "{ \"dhcp-ddns\" :" " {" @@ -2017,7 +2016,7 @@ TEST_F(ParseConfigTest, invalidD2Config) { // Iterate through the invalid configuration strings, attempting to // parse each one. They should fail to parse, but fail gracefully. D2ClientConfigPtr current_config; - int i = 0; + i = 0; while (!invalid_configs[i].empty()) { // Verify that the configuration string parses without throwing. int rcode = parseConfiguration(invalid_configs[i]);