From: Thomas Markwalder Date: Wed, 11 Jan 2017 18:47:39 +0000 (-0500) Subject: [5033] - migrated D2ClientConfigParser to SimpleParser, kea-dhcp4 uses it X-Git-Tag: trac5112_base~6^2~10 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=85640c193b72e7ebfc27d453c38038863f31dc80;p=thirdparty%2Fkea.git [5033] - migrated D2ClientConfigParser to SimpleParser, kea-dhcp4 uses it src/bin/dhcp4/json_config_parser.cc createGlobalDhcp4ConfigParser() - added clause to invoke new D2ClientConfigParser to set staging config - added clause to apply staged D2ClientConfig (formerly done by parser commit src/bin/dhcp4/parser_context.h src/bin/dhcp4/parser_context.cc aded PARSER_DHCP_DDNS context src/bin/dhcp4/simple_parser4.h src/bin/dhcp4/simple_parser4.cc defined SimpleParser4::D2_CLIENT_CONFIG_DEFAULTS SimpleParser4::setAllDefaults() - now sets defaults for D2ClientConfig src/bin/dhcp4/tests/d2_unittest.cc src/bin/dhcp4/tests/fqdn_unittest.cc Updated replace-name-mode values (true/false no longer supported) src/lib/dhcpsrv/parsers/dhcp_parsers.h src/lib/dhcpsrv/parsers/dhcp_parsers.cc D2ClientConfig now derives from SimpleParser src/lib/dhcpsrv/srv_config.h src/lib/dhcpsrv/srv_config.cc Added a D2ClientConfigPtr member to SrvConfig. src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc Updated tests to set D2 client config defaults doc/guide/dhcp4-srv.xml Updated, replace-client-name no longer accepts booleans --- diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index c2423b1266..1fa320a5f3 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -2174,9 +2174,10 @@ It is merely echoed by the server Note that formerly, this parameter was a boolean and permitted only values of true and false. Boolean values - will still be accepted but may eventually be deprecated. A value of - true equates to when-present, - false equates to never. + have been deprecated and are no longer accepted. If you are currently using + booleans, you must replace them with the desired mode name. A value of + true maps to "when-present", while + false maps to "never". For example, To instruct kea-dhcp4 to always generate the FQDN for a diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index d291e1444a..dc8ec9e709 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -436,8 +436,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, parser = new HooksLibrariesParser(config_id); } else if (config_id.compare("echo-client-id") == 0) { parser = new BooleanParser(config_id, globalContext()->boolean_values_); - } else if (config_id.compare("dhcp-ddns") == 0) { - parser = new D2ClientConfigParser(config_id); + // dhcp-ddns has been converted to SimpleParser. } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, globalContext()->boolean_values_); } else if (config_id.compare("control-socket") == 0) { @@ -637,6 +636,13 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { continue; } + if (config_pair.first == "dhcp-ddns") { + D2ClientConfigParser parser; + D2ClientConfigPtr cfg = parser.parse(config_pair.second); + CfgMgr::instance().getStagingCfg()->setD2ClientConfig(cfg); + continue; + } + ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED) @@ -735,6 +741,13 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { if (hooks_parser) { hooks_parser->commit(); } + + { + // Used to be done by parser commit + D2ClientConfigPtr cfg; + cfg = CfgMgr::instance().getStagingCfg()->getD2ClientConfig(); + CfgMgr::instance().setD2ClientConfig(cfg); + } } catch (const isc::Exception& ex) { LOG_ERROR(dhcp4_logger, DHCP4_PARSER_COMMIT_FAIL).arg(ex.what()); diff --git a/src/bin/dhcp4/parser_context.cc b/src/bin/dhcp4/parser_context.cc index 12e4831293..5cd16c380b 100644 --- a/src/bin/dhcp4/parser_context.cc +++ b/src/bin/dhcp4/parser_context.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2017 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 @@ -159,6 +159,8 @@ Parser4Context::contextName() return ("loggers"); case OUTPUT_OPTIONS: return ("output-options"); + case DHCP_DDNS: + return ("dhcp-ddns"); default: return ("__unknown__"); } diff --git a/src/bin/dhcp4/parser_context.h b/src/bin/dhcp4/parser_context.h index 5f37dda521..6f65360c0e 100644 --- a/src/bin/dhcp4/parser_context.h +++ b/src/bin/dhcp4/parser_context.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2017 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 @@ -80,7 +80,10 @@ public: PARSER_OPTION_DATA, /// This will parse the input as hooks-library. - PARSER_HOOKS_LIBRARY + PARSER_HOOKS_LIBRARY, + + /// This will parse the input as dhcp-ddns. + PARSER_DHCP_DDNS } ParserType; /// @brief Default constructor. @@ -184,9 +187,6 @@ public: ///< Used while parsing content of Dhcp4. DHCP4, - // not yet DHCP6, - // not yet DHCP_DDNS, - ///< Used while parsing content of Logging LOGGING, @@ -241,7 +241,10 @@ public: LOGGERS, /// Used while parsing Logging/loggers/output_options structures. - OUTPUT_OPTIONS + OUTPUT_OPTIONS, + + /// Used while parsing Dhcp4/dhcp-ddns + DHCP_DDNS } ParserContext; /// @brief File name diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc index b1c36bfbe0..29317d03d7 100644 --- a/src/bin/dhcp4/simple_parser4.cc +++ b/src/bin/dhcp4/simple_parser4.cc @@ -71,6 +71,27 @@ 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, "" } +}; + /// @} /// --------------------------------------------------------------------------- @@ -99,6 +120,22 @@ 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) { + // Because "dhcp-ddns" is a MapElement and global->get() + // returns a ConstElementPtr, then we get a map we can't + // change. So go thru gyrations to create a non-const + // map, update it with default values and then replace + // the one in global with the new one. Ick. + std::map d2_map; + d2_client->getValue(d2_map); + ElementPtr new_map(new MapElement()); + new_map->setValue(d2_map); + cnt += SimpleParser::setDefaults(new_map, D2_CLIENT_CONFIG_DEFAULTS); + global->set("dhcp-ddns", new_map); + } + return (cnt); } diff --git a/src/bin/dhcp4/simple_parser4.h b/src/bin/dhcp4/simple_parser4.h index 18524f9190..3feea2f667 100644 --- a/src/bin/dhcp4/simple_parser4.h +++ b/src/bin/dhcp4/simple_parser4.h @@ -33,6 +33,7 @@ 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/dhcp4/tests/d2_unittest.cc b/src/bin/dhcp4/tests/d2_unittest.cc index c7084bb95c..840e79403a 100644 --- a/src/bin/dhcp4/tests/d2_unittest.cc +++ b/src/bin/dhcp4/tests/d2_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2017 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 @@ -112,7 +112,7 @@ Dhcp4SrvD2Test::configureD2(bool enable_d2, const bool exp_result, " \"allow-client-update\" : true, " " \"override-no-update\" : true, " " \"override-client-update\" : true, " - " \"replace-client-name\" : true, " + " \"replace-client-name\" : \"when-present\", " " \"generated-prefix\" : \"test.prefix\", " " \"qualifying-suffix\" : \"test.suffix.\" }," "\"valid-lifetime\": 4000 }"; diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index afb469cd46..41ef4ba788 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/src/bin/dhcp4/tests/fqdn_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2017 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 @@ -1551,8 +1551,6 @@ TEST_F(NameDhcpv4SrvTest, emptyFqdn) { // the supported modes. TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) { - // We pass mode labels in with enclosing quotes so we can also test - // unquoted boolean literals true/false testReplaceClientNameMode("\"never\"", CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED); testReplaceClientNameMode("\"never\"", @@ -1572,18 +1570,6 @@ TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) { CLIENT_NAME_NOT_PRESENT, NAME_REPLACED); testReplaceClientNameMode("\"when-not-present\"", CLIENT_NAME_PRESENT, NAME_NOT_REPLACED); - - // Verify that boolean false produces the same result as RCM_NEVER - testReplaceClientNameMode("false", - CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED); - testReplaceClientNameMode("false", - CLIENT_NAME_PRESENT, NAME_NOT_REPLACED); - - // Verify that boolean true produces the same result as RCM_WHEN_PRESENT - testReplaceClientNameMode("true", - CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED); - testReplaceClientNameMode("true", - CLIENT_NAME_PRESENT, NAME_REPLACED); } } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 6f37ffafde..6cf5801f7f 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2017 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 @@ -82,7 +82,17 @@ CfgMgr::isDuplicate(const Subnet6& subnet) const { void CfgMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) { + ensureCurrentAllocated(); + // Note that D2ClientMgr::setD2Config() actually attempts to apply the + // configuration by stopping its sender and opening a new one and so + // forth per the new configuration. d2_client_mgr_.setD2ClientConfig(new_config); + + // Manager will throw if the set fails, if it succeeds + // we'll update our SrvConfig, configuration_, with the D2ClientConfig + // used. This is largely bookkeeping in case we ever want to compare + // configuration_ to another SrvConfig. + configuration_->setD2ClientConfig(new_config); } bool diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index 9a95b25d98..403ccd0c7d 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2017 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 @@ -148,6 +148,11 @@ public: /// @brief Updates the DHCP-DDNS client configuration to the given value. /// + /// Passes the new configuration to the D2ClientMgr instance, + /// d2_client_mgr_, which will attempt to apply the new configuration + /// by shutting down its sender and opening a new connection per the new + /// configuration (see @c D2ClientMgr::setD2ClientConfig()). + /// /// @param new_config pointer to the new client configuration. /// /// @throw Underlying method(s) will throw D2ClientError if given an empty diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index f3d58d5b52..dd8579446e 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2017 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 @@ -1237,119 +1237,85 @@ SubnetConfigParser::getOptionalParam(const std::string& name) { } //**************************** D2ClientConfigParser ********************** -D2ClientConfigParser::D2ClientConfigParser(const std::string& entry_name) - : entry_name_(entry_name), boolean_values_(new BooleanStorage()), - uint32_values_(new Uint32Storage()), string_values_(new StringStorage()), - local_client_config_() { -} - -D2ClientConfigParser::~D2ClientConfigParser() { -} - -void -D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) { - BOOST_FOREACH(ConfigPair param, client_config->mapValue()) { - ParserPtr parser; - try { - parser = createConfigParser(param.first); - } catch (std::exception& ex) { - // Catch exception in case the configuration contains the - // unsupported parameter. In this case, we will need to - // append the position of this element. - isc_throw(DhcpConfigError, ex.what() << " (" - << param.second->getPosition() << ")"); - } - - parser->build(param.second); - parser->commit(); - } - /// @todo Create configuration from the configuration parameters. Because - /// the validation of the D2 configuration is atomic, there is no way to - /// tell which parameter is invalid. Therefore, we catch all exceptions - /// and append the line number of the parent element. In the future we - /// may should extend D2ClientConfig code so as it returns the name of - /// the invalid parameter. +D2ClientConfigPtr +D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { + D2ClientConfigPtr new_config; + bool enable_updates; + IOAddress server_ip(0); + uint32_t server_port; + IOAddress sender_ip(0); + uint32_t sender_port; + uint32_t max_queue_size; + dhcp_ddns::NameChangeProtocol ncr_protocol; + dhcp_ddns::NameChangeFormat ncr_format; + bool always_include_fqdn; + bool override_no_update; + bool override_client_update; + D2ClientConfig::ReplaceClientNameMode replace_client_name_mode; + std::string generated_prefix; + std::string qualifying_suffix; + std::string current_param; + + // 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 + // local, current_param, to keep track of the parameter involved. try { - bool enable_updates = boolean_values_->getParam("enable-updates"); + current_param = "enable-updates"; + enable_updates = getBoolean(client_config, current_param); if (!enable_updates && (client_config->mapValue().size() == 1)) { // 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. - local_client_config_.reset(new D2ClientConfig()); + new_config.reset(new D2ClientConfig()); + return (new_config); + } - return; + current_param = "server-ip"; + server_ip = IOAddress(getString(client_config, (current_param))); + + current_param = "server-port"; + server_port = getInteger(client_config, current_param); + + current_param = "sender-ip"; + std::string sender_ip_str(getString(client_config, current_param)); + if (sender_ip_str.empty()) { + // The default sender IP depends on the server IP family + sender_ip = (server_ip.isV4() ? IOAddress::IPV4_ZERO_ADDRESS() : + IOAddress::IPV6_ZERO_ADDRESS()); + } + else { + sender_ip = IOAddress(sender_ip_str); } - // Get all parameters that are needed to create the D2ClientConfig. - - // The qualifying suffix is mandatory when updates are enabled - std::string qualifying_suffix = - string_values_->getParam("qualifying-suffix"); - - IOAddress server_ip = - IOAddress(string_values_->getOptionalParam("server-ip", - D2ClientConfig:: - DFT_SERVER_IP)); - - uint32_t server_port = - uint32_values_->getOptionalParam("server-port", - D2ClientConfig::DFT_SERVER_PORT); - - // The default sender IP depends on the server IP family - asiolink::IOAddress - sender_ip(string_values_-> - getOptionalParam("sender-ip", - (server_ip.isV4() ? - D2ClientConfig::DFT_V4_SENDER_IP : - D2ClientConfig::DFT_V6_SENDER_IP))); - - uint32_t sender_port = - uint32_values_->getOptionalParam("sender-port", - D2ClientConfig:: - DFT_SENDER_PORT); - uint32_t max_queue_size - = uint32_values_->getOptionalParam("max-queue-size", - D2ClientConfig:: - DFT_MAX_QUEUE_SIZE); - - dhcp_ddns::NameChangeProtocol ncr_protocol = - dhcp_ddns::stringToNcrProtocol(string_values_-> - getOptionalParam("ncr-protocol", - D2ClientConfig:: - DFT_NCR_PROTOCOL)); - dhcp_ddns::NameChangeFormat ncr_format - = dhcp_ddns::stringToNcrFormat(string_values_-> - getOptionalParam("ncr-format", - D2ClientConfig:: - DFT_NCR_FORMAT)); - std::string generated_prefix = - string_values_->getOptionalParam("generated-prefix", - D2ClientConfig:: - DFT_GENERATED_PREFIX); - bool always_include_fqdn = - boolean_values_->getOptionalParam("always-include-fqdn", - D2ClientConfig:: - DFT_ALWAYS_INCLUDE_FQDN); - - bool override_no_update = - boolean_values_->getOptionalParam("override-no-update", - D2ClientConfig:: - DFT_OVERRIDE_NO_UPDATE); - - bool override_client_update = - boolean_values_->getOptionalParam("override-client-update", - D2ClientConfig:: - DFT_OVERRIDE_CLIENT_UPDATE); - - // Formerly, replace-client-name was boolean, so for now we'll support boolean - // values by mapping them to the appropriate mode - D2ClientConfig::ReplaceClientNameMode replace_client_name_mode; - std::string mode_str; - mode_str = string_values_->getOptionalParam("replace-client-name", - D2ClientConfig:: - DFT_REPLACE_CLIENT_NAME_MODE); + current_param = "sender-port"; + sender_port = getInteger(client_config, current_param); + + current_param = "max-queue-size"; + max_queue_size = getInteger(client_config, current_param); + + current_param = "ncr-protocol"; + ncr_protocol = dhcp_ddns::stringToNcrProtocol(getString(client_config, + current_param)); + current_param = "ncr-format"; + ncr_format = dhcp_ddns::stringToNcrFormat(getString(client_config, + current_param)); + + current_param = "always-include-fqdn"; + always_include_fqdn = getBoolean(client_config, current_param); + + current_param = "override-no-update"; + override_no_update = getBoolean(client_config, current_param); + + current_param = "override-client-update"; + override_client_update = getBoolean(client_config, current_param); + + // Formerly, replace-client-name was boolean, so for now we'll support + // boolean values by mapping them to the appropriate mode + current_param = "replace-client-name"; + std::string mode_str = getString(client_config, current_param); if (boost::iequals(mode_str, "false")) { // @todo add a debug log replace_client_name_mode = D2ClientConfig::RCM_NEVER; @@ -1362,8 +1328,52 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) { stringToReplaceClientNameMode(mode_str); } + current_param = "generated-prefix"; + generated_prefix = getString(client_config, current_param); + + current_param = "qualifying-suffix"; + qualifying_suffix = getString(client_config, current_param); + } catch (const std::exception& ex) { + isc_throw(D2ClientError, "D2ClientConfig error: " << ex.what() + << " " << getPosition(current_param, client_config)); + } + + // 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. + if (ncr_format != dhcp_ddns::FMT_JSON) { + isc_throw(D2ClientError, "D2ClientConfig error: NCR Format: " + << dhcp_ddns::ncrFormatToString(ncr_format) + << " is not supported." + << " " << getPosition("ncr-format", client_config)); + } + + if (ncr_protocol != dhcp_ddns::NCR_UDP) { + isc_throw(D2ClientError, "D2ClientConfig error: NCR Protocol: " + << dhcp_ddns::ncrProtocolToString(ncr_protocol) + << " is not supported." + << " " << getPosition("ncr-protocol", client_config)); + } + + if (sender_ip.getFamily() != server_ip.getFamily()) { + isc_throw(D2ClientError, "D2ClientConfig error: address family mismatch: " + << "server-ip: " << server_ip.toText() + << " is: " << (server_ip.isV4() ? "IPv4" : "IPv6") + << " while sender-ip: " << sender_ip.toText() + << " is: " << (sender_ip.isV4() ? "IPv4" : "IPv6") + << " " << getPosition("sender-ip", client_config)); + } + + if (server_ip == sender_ip && server_port == sender_port) { + isc_throw(D2ClientError, "D2ClientConfig error: server and sender cannot" + " share the exact same IP address/port: " + << server_ip.toText() << "/" << server_port + << " " << getPosition("sender-ip", client_config)); + } + + try { // Attempt to create the new client config. - local_client_config_.reset(new D2ClientConfig(enable_updates, + new_config.reset(new D2ClientConfig(enable_updates, server_ip, server_port, sender_ip, @@ -1382,49 +1392,8 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) { isc_throw(DhcpConfigError, ex.what() << " (" << client_config->getPosition() << ")"); } -} - -isc::dhcp::ParserPtr -D2ClientConfigParser::createConfigParser(const std::string& config_id) { - DhcpConfigParser* parser = NULL; - if ((config_id.compare("server-port") == 0) || - (config_id.compare("sender-port") == 0) || - (config_id.compare("max-queue-size") == 0)) { - parser = new Uint32Parser(config_id, uint32_values_); - } else if ((config_id.compare("server-ip") == 0) || - (config_id.compare("ncr-protocol") == 0) || - (config_id.compare("ncr-format") == 0) || - (config_id.compare("generated-prefix") == 0) || - (config_id.compare("sender-ip") == 0) || - (config_id.compare("qualifying-suffix") == 0) || - (config_id.compare("replace-client-name") == 0)) { - parser = new StringParser(config_id, string_values_); - } else if ((config_id.compare("enable-updates") == 0) || - (config_id.compare("always-include-fqdn") == 0) || - (config_id.compare("allow-client-update") == 0) || - (config_id.compare("override-no-update") == 0) || - (config_id.compare("override-client-update") == 0)) { - parser = new BooleanParser(config_id, boolean_values_); - } else { - isc_throw(NotImplemented, - "parser error: D2ClientConfig parameter not supported: " - << config_id); - } - - return (isc::dhcp::ParserPtr(parser)); -} -void -D2ClientConfigParser::commit() { - // @todo if local_client_config_ is empty then shutdown the listener... - // @todo Should this also attempt to start a listener? - // In keeping with Interface, Subnet, and Hooks parsers, then this - // should initialize the listener. Failure to init it, should cause - // rollback. This gets sticky, because who owns the listener instance? - // Does CfgMgr maintain it or does the server class? If the latter - // how do we get that value here? - // I'm thinkikng D2ClientConfig could contain the listener instance - CfgMgr::instance().setD2ClientConfig(local_client_config_); + return(new_config); } }; // namespace dhcp diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index a030580c2b..5f7e2d4125 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2017 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 @@ -1026,29 +1026,18 @@ protected: /// This class parses the configuration element "dhcp-ddns" common to the /// spec files for both dhcp4 and dhcp6. It creates an instance of a /// D2ClientConfig. -class D2ClientConfigParser : public isc::dhcp::DhcpConfigParser { +class D2ClientConfigParser : public isc::data::SimpleParser { public: /// @brief Constructor /// - /// @param entry_name is an arbitrary label assigned to this configuration - /// definition. - D2ClientConfigParser(const std::string& entry_name); + D2ClientConfigParser(){}; /// @brief Destructor - virtual ~D2ClientConfigParser(); + virtual ~D2ClientConfigParser(){}; - /// @brief Performs the parsing of the given dhcp-ddns element. - /// - /// The results of the parsing are retained internally for use during - /// commit. - /// @todo This parser supplies hard-coded default values for all - /// optional parameters. This should be changed once a new plan - /// for configuration is determined. + /// @brief Parses a given dhcp-ddns element into D2ClientConfig. /// /// @param client_config is the "dhcp-ddns" configuration to parse - virtual void build(isc::data::ConstElementPtr client_config); - - /// @brief Creates a parser for the given "dhcp-ddns" member element id. /// /// The elements currently supported are (see isc::dhcp::D2ClientConfig /// for details on each): @@ -1066,33 +1055,8 @@ public: /// -# replace-client-name /// -# generated-prefix /// - /// @param config_id is the "item_name" for a specific member element of - /// the "dns_server" specification. - /// - /// @return returns a pointer to newly created parser. - virtual isc::dhcp::ParserPtr createConfigParser(const std::string& - config_id); - - /// @brief Instantiates a D2ClientConfig from internal data values - /// passes to CfgMgr singleton. - virtual void commit(); - -private: - /// @brief Arbitrary label assigned to this parser instance. - /// Primarily used for diagnostics. - std::string entry_name_; - - /// Storage for subnet-specific boolean values. - BooleanStoragePtr boolean_values_; - - /// Storage for subnet-specific integer values. - Uint32StoragePtr uint32_values_; - - /// Storage for subnet-specific string values. - StringStoragePtr string_values_; - - /// @brief Pointer to temporary local instance created during build. - D2ClientConfigPtr local_client_config_ ; + /// @return returns a pointer to newly created D2ClientConfig. + D2ClientConfigPtr parse(isc::data::ConstElementPtr d2_client_cfg); }; // Pointers to various parser objects. diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 7c7830966f..cb69f7d30f 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2017 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 @@ -29,7 +29,8 @@ SrvConfig::SrvConfig() cfg_host_operations4_(CfgHostOperations::createConfig4()), cfg_host_operations6_(CfgHostOperations::createConfig6()), class_dictionary_(new ClientClassDictionary()), - decline_timer_(0), dhcp4o6_port_(0) { + decline_timer_(0), dhcp4o6_port_(0), + d2_client_config_(new D2ClientConfig()) { } SrvConfig::SrvConfig(const uint32_t sequence) @@ -42,7 +43,8 @@ SrvConfig::SrvConfig(const uint32_t sequence) cfg_host_operations4_(CfgHostOperations::createConfig4()), cfg_host_operations6_(CfgHostOperations::createConfig6()), class_dictionary_(new ClientClassDictionary()), - decline_timer_(0), dhcp4o6_port_(0) { + decline_timer_(0), dhcp4o6_port_(0), + d2_client_config_(new D2ClientConfig()) { } std::string @@ -70,7 +72,7 @@ SrvConfig::getConfigSummary(const uint32_t selection) const { } if ((selection & CFGSEL_DDNS) == CFGSEL_DDNS) { - bool ddns_enabled = CfgMgr::instance().ddnsEnabled(); + bool ddns_enabled = getD2ClientConfig()->getEnableUpdates(); s << "DDNS: " << (ddns_enabled ? "enabled" : "disabled") << "; "; } @@ -106,7 +108,8 @@ SrvConfig::copy(SrvConfig& new_config) const { cfg_option_->copyTo(*new_config.cfg_option_); // Replace the client class dictionary new_config.class_dictionary_.reset(new ClientClassDictionary(*class_dictionary_)); - + // Replace the D2 client configuration + new_config.setD2ClientConfig(getD2ClientConfig()); } void @@ -151,7 +154,8 @@ SrvConfig::equals(const SrvConfig& other) const { return ((*cfg_iface_ == *other.cfg_iface_) && (*cfg_option_def_ == *other.cfg_option_def_) && (*cfg_option_ == *other.cfg_option_) && - (*class_dictionary_ == *other.class_dictionary_)); + (*class_dictionary_ == *other.class_dictionary_) && + (*d2_client_config_ == *other.d2_client_config_)); } void diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index 5ab02d1b15..63ecf39831 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2017 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 @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -481,12 +482,28 @@ public: /// @brief Returns DHCP4o6 IPC port /// - /// See @ref setDhcp4o6Port or brief discussion. + /// See @ref setDhcp4o6Port or brief discussion. /// @return value of DHCP4o6 IPC port uint32_t getDhcp4o6Port() { return (dhcp4o6_port_); } + /// @brief Returns pointer to the D2 client configuration + D2ClientConfigPtr getD2ClientConfig() { + return (d2_client_config_); + } + + /// @brief Returns pointer to const D2 client configuration + const D2ClientConfigPtr getD2ClientConfig() const { + return (d2_client_config_); + } + + /// @brief Sets the D2 client configuration + /// @param dictionary pointer to the new D2 client configuration + void setD2ClientConfig(const D2ClientConfigPtr& d2_client_config) { + d2_client_config_ = d2_client_config; + } + private: /// @brief Sequence number identifying the configuration. @@ -570,6 +587,8 @@ private: /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication, /// this socket is bound and connected to this port and port + 1 uint32_t dhcp4o6_port_; + + D2ClientConfigPtr d2_client_config_; }; /// @name Pointers to the @c SrvConfig object. diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 566be20b64..7681be1505 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -314,17 +314,18 @@ public: // These are the simple parsers. No need to go through // the ParserPtr hooplas with them. - if ( (config_pair.first == "option-data") || - (config_pair.first == "option-def")) { + if ((config_pair.first == "option-data") || + (config_pair.first == "option-def") || + (config_pair.first == "dhcp-ddns")) { continue; } // We also don't care about the default values that may be been // inserted - if ( (config_pair.first == "preferred-lifetime") || - (config_pair.first == "valid-lifetime") || - (config_pair.first == "renew-timer") || - (config_pair.first == "rebind-timer")) { + if ((config_pair.first == "preferred-lifetime") || + (config_pair.first == "valid-lifetime") || + (config_pair.first == "renew-timer") || + (config_pair.first == "rebind-timer")) { continue; } @@ -366,6 +367,16 @@ public: option_list_parser.parse(cfg_option, option_config->second); } + // The dhcp-ddns parser is the next one to be run. + std::map::const_iterator + d2_client_config = values_map.find("dhcp-ddns"); + if (d2_client_config != values_map.end()) { + // Used to be done by parser commit + D2ClientConfigParser parser; + D2ClientConfigPtr cfg = parser.parse(d2_client_config->second); + CfgMgr::instance().setD2ClientConfig(cfg); + } + // Everything was fine. Configuration is successful. answer = isc::config::createAnswer(0, "Configuration committed."); } catch (const isc::Exception& ex) { @@ -401,8 +412,6 @@ public: parser.reset(new HooksLibrariesParser(config_id)); hooks_libraries_parser_ = boost::dynamic_pointer_cast(parser); - } else if (config_id.compare("dhcp-ddns") == 0) { - parser.reset(new D2ClientConfigParser(config_id)); } else { isc_throw(NotImplemented, "Parser error: configuration parameter not supported: " @@ -425,7 +434,8 @@ public: size_t setAllDefaults(isc::data::ElementPtr global, const SimpleDefaults& global_defaults, const SimpleDefaults& option_defaults, - const SimpleDefaults& option_def_defaults) { + const SimpleDefaults& option_def_defaults, + const SimpleDefaults& d2_client_defaults) { size_t cnt = 0; // Set global defaults first. cnt = SimpleParser::setDefaults(global, global_defaults); @@ -445,6 +455,23 @@ public: } } + ConstElementPtr d2_client = global->get("dhcp-ddns"); + /// @todo - what if it's not in global? should we add it? + if (d2_client) { + // Because "dhcp-ddns" is a MapElement and global->get() + // returns a ConstElementPtr, then we get a map we can't + // change. So go thru gyrations to create a non-const + // map, update it with default values and then replace + // the one in global with the new one. Ick. + std::map d2_map; + d2_client->getValue(d2_map); + ElementPtr new_map(new MapElement()); + new_map->setValue(d2_map); + cnt += SimpleParser::setDefaults(new_map, d2_client_defaults); + global->set("dhcp-ddns", new_map); + } + + return (cnt); } @@ -456,6 +483,10 @@ public: /// src/bin/dhcp{4,6}, the most straightforward way is to simply copy the /// minimum code here. Hence this method. /// + /// @todo - TKM, I think this is fairly hideous and we should figure out a + /// a way to not have to replicate in this fashion. It may be minimum code + /// now, but it won't be fairly soon. + /// /// @param config configuration structure to be filled with default values /// @param v6 true = DHCPv6, false = DHCPv4 void setAllDefaults(ElementPtr config, bool v6) { @@ -497,12 +528,31 @@ 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); + OPTION6_DEF_DEFAULTS, D2_CLIENT_CFG_DEFAULTS); } else { setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION4_DEFAULTS, - OPTION4_DEF_DEFAULTS); + OPTION4_DEF_DEFAULTS, D2_CLIENT_CFG_DEFAULTS); } } @@ -529,6 +579,7 @@ public: // If error was reported, the error string should contain // position of the data element which caused failure. if (rcode_ != 0) { + std::cout << "Error text:" << error_text_ << std::endl; EXPECT_TRUE(errorContainsPosition(status, "")); } } @@ -801,7 +852,6 @@ TEST_F(ParseConfigTest, escapedOptionDataTest) { " \"data\": \"\\\\SMSBoot\\\\x64\\\\wdsnbp.com\"" " } ]" "}"; - std::cout << config << std::endl; // Verify that the configuration string parses. int rcode = parseConfiguration(config); @@ -1826,17 +1876,6 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) { /// @brief Check various invalid D2 client configurations. TEST_F(ParseConfigTest, invalidD2Config) { std::string invalid_configs[] = { - // Must supply at least enable-updates - "{ \"dhcp-ddns\" :" - " {" - " }" - "}", - // Must supply qualifying-suffix when updates are enabled - "{ \"dhcp-ddns\" :" - " {" - " \"enable-updates\" : true" - " }" - "}", // Invalid server ip value "{ \"dhcp-ddns\" :" " {" diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc index 1890157f0a..85533d97f6 100644 --- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc +++ b/src/lib/dhcpsrv/tests/srv_config_unittest.cc @@ -141,8 +141,7 @@ SrvConfigTest::addSubnet6(const unsigned int index) { void SrvConfigTest::enableDDNS(const bool enable) { - // D2 configuration should always be non-NULL. - CfgMgr::instance().getD2ClientConfig()->enableUpdates(enable); + conf_.getD2ClientConfig()->enableUpdates(enable); } // Check that by default there are no logging entries diff --git a/src/lib/dhcpsrv/testutils/config_result_check.cc b/src/lib/dhcpsrv/testutils/config_result_check.cc index e36aac7cce..36a4c83292 100644 --- a/src/lib/dhcpsrv/testutils/config_result_check.cc +++ b/src/lib/dhcpsrv/testutils/config_result_check.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2017 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 @@ -67,12 +67,6 @@ bool errorContainsPosition(ConstElementPtr error_element, ++i; } - // Make sure that there has been at least one digit and that the - // position is followed by the paren. - if ((i == 0) || (split_pos[2][i] != ')')) { - return (false); - } - // All checks passed. return (true); }