diff --git a/configure.ac b/configure.ac index 3d13aa850e..8b51e9a032 100644 --- a/configure.ac +++ b/configure.ac @@ -245,11 +245,13 @@ case "$host" in # it should be avoided to rely on 'osx_version' unless there's no # viable alternative. osx_version=`/usr/bin/sw_vers -productVersion` + if [ test $osx_version = "10.9" \ - -o $osx_version = "10.9.1" \ - -o $osx_version = "10.9.2" ]; then - bind10_undefined_pthread_behavior=yes - fi + -o $osx_version = "10.9.1" \ + -o $osx_version = "10.9.2" \ + -o $osx_version = "10.9.3" ]; then + bind10_undefined_pthread_behavior=yes + fi # libtool doesn't work perfectly with Darwin: libtool embeds the # final install path in dynamic libraries and our loadable python diff --git a/doc/guide/bind10-guide.xml b/doc/guide/bind10-guide.xml index 9bf190395e..812bb998b5 100644 --- a/doc/guide/bind10-guide.xml +++ b/doc/guide/bind10-guide.xml @@ -5263,9 +5263,11 @@ Dhcp6/renew-timer 1000 integer (default) configuration will be available. It will look similar to this: > config show DhcpDdns -DhcpDdns/interface "eth0" string (default) DhcpDdns/ip_address "127.0.0.1" string (default) DhcpDdns/port 53001 integer (default) +DhcpDdns/dns_server_timeout 100 integer (default) +DhcpDdns/ncr_protocol "UDP" string (default) +DhcpDdns/ncr_format "JSON" string (default) DhcpDdns/tsig_keys [] list (default) DhcpDdns/forward_ddns/ddns_domains [] list (default) DhcpDdns/reverse_ddns/ddns_domains [] list (default) @@ -5282,7 +5284,7 @@ DhcpDdns/reverse_ddns/ddns_domains [] list (default) - General Server Parameters — + Global Server Parameters — values which control connectivity and global server behavior @@ -5306,20 +5308,43 @@ DhcpDdns/reverse_ddns/ddns_domains [] list (default)
- General Server Parameters + Global Server Parameters + + + ip_address - IP address on which D2 listens for requests. The default is + the local loopback interface at address 127.0.0.1. You may specify + either an IPv4 or IPv6 address. + + + port - Port on which D2 listens for requests. The default value + is 53001. + + + ncr_format - Socket protocol to use when sending requests to D2. + Currently only UDP is supported. TCP may be available in an upcoming + release. + + + ncr_protocol - Packet format to use when sending requests to D2. + Currently only JSON format is supported. Other formats may be available + in future releases. + + + dns_server_timeout - The maximum amount of time in milliseconds, that + D2 will wait for a response from a DNS server to a single DNS update + message. + + - The DHCP-DDNS server must listen for requests on a known address and - port. By default, it will listen at 127.0.0.1 on port 53001. This is - governed by the parameters, "ip-address" and "port". Either value - may be changed using config set/commit. For example to change the - server to listen at 192.168.1.10 port 900: + D2 must listen for change requests on a known address and port. By + default it listens at 127.0.0.1 on port 53001. The following example + illustrates how to change D2's global parameters so it will listen + at 192.168.1.10 port 900: > config set DhcpDdns/ip_address "192.168.1.10" > config set DhcpDdns/port 900 > config commit - The server may be configured to listen over IPv4 or IPv6, therefore - ip-address may an IPv4 or IPv6 address. @@ -5332,7 +5357,6 @@ DhcpDdns/reverse_ddns/ddns_domains [] list (default) authentication to guard against such attacks. - If the ip_address and port are changed, it will be necessary to change the diff --git a/src/bin/d2/d2_cfg_mgr.cc b/src/bin/d2/d2_cfg_mgr.cc index 4bddb99a6f..536c9cfa24 100644 --- a/src/bin/d2/d2_cfg_mgr.cc +++ b/src/bin/d2/d2_cfg_mgr.cc @@ -30,12 +30,14 @@ typedef std::vector ByteAddress; // *********************** D2CfgContext ************************* D2CfgContext::D2CfgContext() - : forward_mgr_(new DdnsDomainListMgr("forward_mgr")), + : d2_params_(new D2Params()), + forward_mgr_(new DdnsDomainListMgr("forward_mgr")), reverse_mgr_(new DdnsDomainListMgr("reverse_mgr")), keys_(new TSIGKeyInfoMap()) { } D2CfgContext::D2CfgContext(const D2CfgContext& rhs) : DCfgContextBase(rhs) { + d2_params_ = rhs.d2_params_; if (rhs.forward_mgr_) { forward_mgr_.reset(new DdnsDomainListMgr(rhs.forward_mgr_->getName())); forward_mgr_->setDomains(rhs.forward_mgr_->getDomains()); @@ -61,9 +63,6 @@ const char* D2CfgMgr::IPV6_REV_ZONE_SUFFIX = "ip6.arpa."; D2CfgMgr::D2CfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new D2CfgContext())) { // TSIG keys need to parse before the Domains, so we can catch Domains // that specify undefined keys. Create the necessary parsing order now. - addToParseOrder("interface"); - addToParseOrder("ip_address"); - addToParseOrder("port"); addToParseOrder("tsig_keys"); addToParseOrder("forward_ddns"); addToParseOrder("reverse_ddns"); @@ -72,6 +71,11 @@ D2CfgMgr::D2CfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new D2CfgContext())) { D2CfgMgr::~D2CfgMgr() { } +DCfgContextBasePtr +D2CfgMgr::createNewContext() { + return (DCfgContextBasePtr(new D2CfgContext())); +} + bool D2CfgMgr::forwardUpdatesEnabled() { // Forward updates are not enabled if no forward servers are defined. @@ -187,6 +191,47 @@ D2CfgMgr::reverseV6Address(const isc::asiolink::IOAddress& ioaddr) { return(stream.str()); } +const D2ParamsPtr& +D2CfgMgr::getD2Params() { + return (getD2CfgContext()->getD2Params()); +} + +void +D2CfgMgr::buildParams(isc::data::ConstElementPtr params_config) { + // Base class build creates parses and invokes build on each parser. + // This populate the context scalar stores with all of the parameters. + DCfgMgrBase::buildParams(params_config); + + // Fetch the parameters from the context to create the D2Params. + D2CfgContextPtr context = getD2CfgContext(); + isc::dhcp::StringStoragePtr strings = context->getStringStorage(); + asiolink::IOAddress ip_address(strings-> + getOptionalParam("ip_address", + D2Params::DFT_IP_ADDRESS)); + + isc::dhcp::Uint32StoragePtr ints = context->getUint32Storage(); + uint32_t port = ints->getOptionalParam("port", D2Params::DFT_PORT); + + uint32_t dns_server_timeout + = ints->getOptionalParam("dns_server_timeout", + D2Params::DFT_DNS_SERVER_TIMEOUT); + + dhcp_ddns::NameChangeProtocol ncr_protocol + = dhcp_ddns::stringToNcrProtocol(strings-> + getOptionalParam("ncr_protocol", + D2Params:: + DFT_NCR_PROTOCOL)); + dhcp_ddns::NameChangeFormat ncr_format + = dhcp_ddns::stringToNcrFormat(strings-> + getOptionalParam("ncr_format", + D2Params:: + DFT_NCR_FORMAT)); + // Attempt to create the new client config. + D2ParamsPtr params(new D2Params(ip_address, port, dns_server_timeout, + ncr_protocol, ncr_format)); + + context->getD2Params() = params; +} isc::dhcp::ParserPtr D2CfgMgr::createConfigParser(const std::string& config_id) { @@ -194,31 +239,34 @@ D2CfgMgr::createConfigParser(const std::string& config_id) { D2CfgContextPtr context = getD2CfgContext(); // Create parser instance based on element_id. - isc::dhcp::DhcpConfigParser* parser = NULL; - if ((config_id == "interface") || - (config_id == "ip_address")) { - parser = new isc::dhcp::StringParser(config_id, - context->getStringStorage()); - } else if (config_id == "port") { - parser = new isc::dhcp::Uint32Parser(config_id, - context->getUint32Storage()); + isc::dhcp::ParserPtr parser; + if ((config_id.compare("port") == 0) || + (config_id.compare("dns_server_timeout") == 0)) { + parser.reset(new isc::dhcp::Uint32Parser(config_id, + context->getUint32Storage())); + } else if ((config_id.compare("ip_address") == 0) || + (config_id.compare("ncr_protocol") == 0) || + (config_id.compare("ncr_format") == 0)) { + parser.reset(new isc::dhcp::StringParser(config_id, + context->getStringStorage())); } else if (config_id == "forward_ddns") { - parser = new DdnsDomainListMgrParser("forward_mgr", - context->getForwardMgr(), - context->getKeys()); + parser.reset(new DdnsDomainListMgrParser("forward_mgr", + context->getForwardMgr(), + context->getKeys())); } else if (config_id == "reverse_ddns") { - parser = new DdnsDomainListMgrParser("reverse_mgr", - context->getReverseMgr(), - context->getKeys()); + parser.reset(new DdnsDomainListMgrParser("reverse_mgr", + context->getReverseMgr(), + context->getKeys())); } else if (config_id == "tsig_keys") { - parser = new TSIGKeyInfoListParser("tsig_key_list", context->getKeys()); + parser.reset(new TSIGKeyInfoListParser("tsig_key_list", + context->getKeys())); } else { isc_throw(NotImplemented, "parser error: D2CfgMgr parameter not supported: " << config_id); } - return (isc::dhcp::ParserPtr(parser)); + return (parser); } }; // end of isc::dhcp namespace diff --git a/src/bin/d2/d2_cfg_mgr.h b/src/bin/d2/d2_cfg_mgr.h index 0e903043e2..dcc03e1e8f 100644 --- a/src/bin/d2/d2_cfg_mgr.h +++ b/src/bin/d2/d2_cfg_mgr.h @@ -53,6 +53,11 @@ public: return (DCfgContextBasePtr(new D2CfgContext(*this))); } + /// @brief Fetches a reference to the D2Params + D2ParamsPtr& getD2Params() { + return (d2_params_); + } + /// @brief Fetches the forward DNS domain list manager. /// /// @return returns a pointer to the forward manager. @@ -82,6 +87,9 @@ private: /// @brief Private assignment operator to avoid potential for slicing. D2CfgContext& operator=(const D2CfgContext& rhs); + /// @brief Global level parameter storage + D2ParamsPtr d2_params_; + /// @brief Forward domain list manager. DdnsDomainListMgrPtr forward_mgr_; @@ -226,17 +234,33 @@ public: /// @throw D2CfgError if not given an IPv6 address. static std::string reverseV6Address(const isc::asiolink::IOAddress& ioaddr); + /// @brief Convenience method fetches the D2Params from context + /// @return reference to const D2ParamsPtr + const D2ParamsPtr& getD2Params(); + protected: + /// @brief Performs the parsing of the given "params" element. + /// + /// Iterates over the set of parameters, creating a parser based on the + /// parameter's id and then invoking its build method passing in the + /// parameter's configuration value. + /// + /// @param params_config set of scalar configuration elements to parse + virtual void buildParams(isc::data::ConstElementPtr params_config); + /// @brief Given an element_id returns an instance of the appropriate /// parser. /// /// It is responsible for top-level or outermost DHCP-DDNS configuration /// elements (see dhcp-ddns.spec): - /// 1. interface - /// 2. ip_address - /// 3. port - /// 4. forward_ddns - /// 5. reverse_ddns + /// -# ip_address + /// -# port + /// -# dns_server_timeout + /// -# ncr_protocol + /// -# ncr_format + /// -# tsig_keys + /// -# forward_ddns + /// -# reverse_ddns /// /// @param element_id is the string name of the element as it will appear /// in the configuration set. @@ -245,6 +269,17 @@ protected: /// @throw throws DCfgMgrBaseError if an error occurs. virtual isc::dhcp::ParserPtr createConfigParser(const std::string& element_id); + + /// @brief Creates an new, blank D2CfgContext context + /// + /// This method is used at the beginning of configuration process to + /// create a fresh, empty copy of a D2CfgContext. This new context will + /// be populated during the configuration process and will replace the + /// existing context provided the configuration process completes without + /// error. + /// + /// @return Returns a DCfgContextBasePtr to the new context instance. + virtual DCfgContextBasePtr createNewContext(); }; /// @brief Defines a shared pointer to D2CfgMgr. diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index fbfbf763d3..c8e331e30e 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -27,6 +27,102 @@ namespace isc { namespace d2 { +// *********************** D2Params ************************* + +const char *D2Params::DFT_IP_ADDRESS = "127.0.0.1"; +const size_t D2Params::DFT_PORT = 53001; +const size_t D2Params::DFT_DNS_SERVER_TIMEOUT = 100; +const char *D2Params::DFT_NCR_PROTOCOL = "UDP"; +const char *D2Params::DFT_NCR_FORMAT = "JSON"; + +D2Params::D2Params(const isc::asiolink::IOAddress& ip_address, + const size_t port, + const size_t dns_server_timeout, + const dhcp_ddns::NameChangeProtocol& ncr_protocol, + const dhcp_ddns::NameChangeFormat& ncr_format) + : ip_address_(ip_address), + port_(port), + dns_server_timeout_(dns_server_timeout), + ncr_protocol_(ncr_protocol), + ncr_format_(ncr_format) { + validateContents(); +} + +D2Params::D2Params() + : ip_address_(isc::asiolink::IOAddress(DFT_IP_ADDRESS)), + port_(DFT_PORT), + dns_server_timeout_(DFT_DNS_SERVER_TIMEOUT), + ncr_protocol_(dhcp_ddns::NCR_UDP), + ncr_format_(dhcp_ddns::FMT_JSON) { + validateContents(); +} + +D2Params::~D2Params(){}; + +void +D2Params::validateContents() { + if ((ip_address_.toText() == "0.0.0.0") || (ip_address_.toText() == "::")) { + isc_throw(D2CfgError, + "D2Params: IP address cannot be \"" << ip_address_ << "\""); + } + + if (port_ == 0) { + isc_throw(D2CfgError, "D2Params: port cannot be 0"); + } + + if (dns_server_timeout_ < 1) { + isc_throw(D2CfgError, + "D2Params: DNS server timeout must be larger than 0"); + } + + if (ncr_format_ != dhcp_ddns::FMT_JSON) { + isc_throw(D2CfgError, "D2Params: NCR Format:" + << dhcp_ddns::ncrFormatToString(ncr_format_) + << " is not yet supported"); + } + + if (ncr_protocol_ != dhcp_ddns::NCR_UDP) { + isc_throw(D2CfgError, "D2Params: NCR Protocol:" + << dhcp_ddns::ncrProtocolToString(ncr_protocol_) + << " is not yet supported"); + } +} + +bool +D2Params::operator == (const D2Params& other) const { + return ((ip_address_ == other.ip_address_) && + (port_ == other.port_) && + (dns_server_timeout_ == other.dns_server_timeout_) && + (ncr_protocol_ == other.ncr_protocol_) && + (ncr_format_ == other.ncr_format_)); +} + +bool +D2Params::operator != (const D2Params& other) const { + return (!(*this == other)); +} + +std::string +D2Params::toText() const { + std::ostringstream stream; + + stream << ", ip_address: " << ip_address_.toText() + << ", port: " << port_ + << ", dns_server_timeout_: " << dns_server_timeout_ + << ", ncr_protocol: " + << dhcp_ddns::ncrProtocolToString(ncr_protocol_) + << ", ncr_format: " << ncr_format_ + << dhcp_ddns::ncrFormatToString(ncr_format_); + + return (stream.str()); +} + +std::ostream& +operator<<(std::ostream& os, const D2Params& config) { + os << config.toText(); + return (os); +} + // *********************** TSIGKeyInfo ************************* TSIGKeyInfo::TSIGKeyInfo(const std::string& name, const std::string& algorithm, diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index 5af60eee80..168559b192 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -143,6 +143,125 @@ public: isc::Exception(file, line, what) { }; }; +/// @brief Acts as a storage vault for D2 global scalar parameters +class D2Params { +public: + /// @brief Default configuration constants. + //@{ + /// @todo For now these are hard-coded as configuration layer cannot + /// readily provide them (see Trac #3358). + static const char *DFT_IP_ADDRESS; + static const size_t DFT_PORT; + static const size_t DFT_DNS_SERVER_TIMEOUT; + static const char *DFT_NCR_PROTOCOL; + static const char *DFT_NCR_FORMAT; + //@} + + /// @brief Constructor + /// + /// @param ip_address IP address at which D2 should listen for NCRs + /// @param port port on which D2 should listen NCRs + /// @param dns_server_timeout maximum amount of time in milliseconds to + /// wait for a response to a single DNS update request. + /// @param ncr_protocol socket protocol D2 should use to receive NCRS + /// @param ncr_format packet format of the inbound NCRs + /// + /// @throw D2CfgError if: + /// -# ip_address is 0.0.0.0 or :: + /// -# port is 0 + /// -# dns_server_timeout is < 1 + /// -# ncr_protocol is invalid, currently only NCR_UDP is supported + /// -# ncr_format is invalid, currently only FMT_JSON is supported + D2Params(const isc::asiolink::IOAddress& ip_address, + const size_t port, + const size_t dns_server_timeout, + const dhcp_ddns::NameChangeProtocol& ncr_protocol, + const dhcp_ddns::NameChangeFormat& ncr_format); + + /// @brief Default constructor + /// The default constructor creates an instance that has updates disabled. + D2Params(); + + /// @brief Destructor + virtual ~D2Params(); + + /// @brief Return the IP address D2 listens on. + const isc::asiolink::IOAddress& getIpAddress() const { + return(ip_address_); + } + + /// @brief Return the TCP/UPD port D2 listens on. + size_t getPort() const { + return(port_); + } + + /// @brief Return the DNS server timeout value. + size_t getDnsServerTimeout() const { + return(dns_server_timeout_); + } + + /// @brief Return the socket protocol in use. + const dhcp_ddns::NameChangeProtocol& getNcrProtocol() const { + return(ncr_protocol_); + } + + /// @brief Return the expected format of inbound requests (NCRs). + const dhcp_ddns::NameChangeFormat& getNcrFormat() const { + return(ncr_format_); + } + + /// @brief Compares two D2Paramss for equality + bool operator == (const D2Params& other) const; + + /// @brief Compares two D2Paramss for inequality + bool operator != (const D2Params& other) const; + + /// @brief Generates a string representation of the class contents. + std::string toText() const; + +protected: + /// @brief Validates member values. + /// + /// Method is used by the constructor to validate member contents. + /// Currently checks: + /// -# ip_address is not 0.0.0.0 or :: + /// -# port is not 0 + /// -# dns_server_timeout is 0 + /// -# ncr_protocol is UDP + /// -# ncr_format is JSON + /// + /// @throw D2CfgError if contents are invalid + virtual void validateContents(); + +private: + /// @brief IP address D2 listens on. + isc::asiolink::IOAddress ip_address_; + + /// @brief IP port D2 listens on. + size_t port_; + + /// @brief Timeout for a single DNS packet exchange in milliseconds. + size_t dns_server_timeout_; + + /// @brief The socket protocol to use. + /// Currently only UDP is supported. + dhcp_ddns::NameChangeProtocol ncr_protocol_; + + /// @brief Format of the inbound requests (NCRs). + /// Currently only JSON format is supported. + dhcp_ddns::NameChangeFormat ncr_format_; +}; + +/// @brief Dumps the contents of a D2Params as text to an output stream +/// +/// @param os output stream to which text should be sent +/// @param config D2Param instnace to dump +std::ostream& +operator<<(std::ostream& os, const D2Params& config); + +/// @brief Defines a pointer for D2Params instances. +typedef boost::shared_ptr D2ParamsPtr; + /// @brief Represents a TSIG Key. /// /// Currently, this is simple storage class containing the basic attributes of diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index cca71e21d3..acd58bc814 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -207,7 +207,7 @@ D2Process::configure(isc::data::ConstElementPtr config_set) { if (rcode) { // Non-zero means we got an invalid configuration, take no further - // action. In integrated mode, this will send a failed response back + // action. In integrated mode, this will send a failed response back // to BIND10. reconf_queue_flag_ = false; return (answer); @@ -248,7 +248,7 @@ D2Process::checkQueueStatus() { queue_mgr_->stopListening(); } catch (const isc::Exception& ex) { // It is very unlikey that we would experience an error - // here, but theoretically possible. + // here, but theoretically possible. LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_STOP_ERROR) .arg(ex.what()); } @@ -325,37 +325,42 @@ D2Process::reconfigureQueueMgr() { queue_mgr_->removeListener(); // Get the configuration parameters that affect Queue Manager. - // @todo Need to add parameters for listener TYPE, FORMAT, address reuse - std::string ip_address; - uint32_t port; - getCfgMgr()->getContext()->getParam("ip_address", ip_address); + const D2ParamsPtr& d2_params = getD2CfgMgr()->getD2Params(); // Warn the user if the server address is not the loopback. /// @todo Remove this once we provide a secure mechanism. + std::string ip_address = d2_params->getIpAddress().toText(); if (ip_address != "127.0.0.1" && ip_address != "::1") { LOG_WARN(dctl_logger, DHCP_DDNS_NOT_ON_LOOPBACK).arg(ip_address); } - getCfgMgr()->getContext()->getParam("port", port); - isc::asiolink::IOAddress addr(ip_address); - // Instantiate the listener. - queue_mgr_->initUDPListener(addr, port, dhcp_ddns::FMT_JSON, true); + if (d2_params->getNcrProtocol() == dhcp_ddns::NCR_UDP) { + queue_mgr_->initUDPListener(d2_params->getIpAddress(), + d2_params->getPort(), + d2_params->getNcrFormat(), true); + } else { + /// @todo Add TCP/IP once it's supported + // We should never get this far but if we do deal with it. + isc_throw(DProcessBaseError, "Unsupported NCR listener protocol:" + << dhcp_ddns::ncrProtocolToString(d2_params-> + getNcrProtocol())); + } // Now start it. This assumes that starting is a synchronous, // blocking call that executes quickly. @todo Should that change then // we will have to expand the state model to accommodate this. queue_mgr_->startListening(); } catch (const isc::Exception& ex) { - // Queue manager failed to initialize and therefore not listening. - // This is most likely due to an unavailable IP address or port, + // Queue manager failed to initialize and therefore not listening. + // This is most likely due to an unavailable IP address or port, // which is a configuration issue. LOG_ERROR(dctl_logger, DHCP_DDNS_QUEUE_MGR_START_ERROR).arg(ex.what()); } } isc::data::ConstElementPtr -D2Process::command(const std::string& command, +D2Process::command(const std::string& command, isc::data::ConstElementPtr args) { // @todo This is the initial implementation. If and when D2 is extended // to support its own commands, this implementation must change. Otherwise diff --git a/src/bin/d2/d2_update_mgr.cc b/src/bin/d2/d2_update_mgr.cc index 269610b7f7..7b0438dc96 100644 --- a/src/bin/d2/d2_update_mgr.cc +++ b/src/bin/d2/d2_update_mgr.cc @@ -195,10 +195,12 @@ D2UpdateMgr::makeTransaction(dhcp_ddns::NameChangeRequestPtr& next_ncr) { NameChangeTransactionPtr trans; if (next_ncr->getChangeType() == dhcp_ddns::CHG_ADD) { trans.reset(new NameAddTransaction(io_service_, next_ncr, - forward_domain, reverse_domain)); + forward_domain, reverse_domain, + cfg_mgr_)); } else { trans.reset(new NameRemoveTransaction(io_service_, next_ncr, - forward_domain, reverse_domain)); + forward_domain, reverse_domain, + cfg_mgr_)); } // Add the new transaction to the list. diff --git a/src/bin/d2/d_cfg_mgr.cc b/src/bin/d2/d_cfg_mgr.cc index 1e6bb57bfb..151202f360 100644 --- a/src/bin/d2/d_cfg_mgr.cc +++ b/src/bin/d2/d_cfg_mgr.cc @@ -97,15 +97,28 @@ DCfgContextBase::~DCfgContextBase() { // *********************** DCfgMgrBase ************************* DCfgMgrBase::DCfgMgrBase(DCfgContextBasePtr context) - : parse_order_(), context_(context) { - if (!context_) { - isc_throw(DCfgMgrBaseError, "DCfgMgrBase ctor: context cannot be NULL"); - } + : parse_order_() { + setContext(context); } DCfgMgrBase::~DCfgMgrBase() { } +void +DCfgMgrBase::resetContext() { + DCfgContextBasePtr context = createNewContext(); + setContext(context); +} + +void +DCfgMgrBase::setContext(DCfgContextBasePtr& context) { + if (!context) { + isc_throw(DCfgMgrBaseError, "DCfgMgrBase: context cannot be NULL"); + } + + context_ = context; +} + isc::data::ConstElementPtr DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { LOG_DEBUG(dctl_logger, DBGLVL_COMMAND, @@ -122,7 +135,8 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { // inconsistency if the parsing operation fails after the context has been // modified. We need to preserve the original context here // so as we can rollback changes when an error occurs. - DCfgContextBasePtr original_context = context_->clone(); + DCfgContextBasePtr original_context = context_; + resetContext(); // Answer will hold the result returned to the caller. ConstElementPtr answer; @@ -131,15 +145,42 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { std::string element_id; try { - // Grab a map of element_ids and their data values from the new - // configuration set. - const std::map& values_map = - config_set->mapValue(); + // Split the configuration into two maps. The first containing only + // top-level scalar parameters (i.e. globals), the second containing + // non-scalar or object elements (maps, lists, etc...). This allows + // us to parse and validate all of the global values before we do + // objects which may depend on them. + ElementMap params_map; + ElementMap objects_map; + + isc::dhcp::ConfigPair config_pair; + BOOST_FOREACH(config_pair, config_set->mapValue()) { + std::string element_id = config_pair.first; + isc::data::ConstElementPtr element = config_pair.second; + switch (element->getType()) { + case isc::data::Element::integer: + case isc::data::Element::real: + case isc::data::Element::boolean: + case isc::data::Element::string: + params_map[element_id] = element; + break; + default: + objects_map[element_id] = element; + break; + } + } + + // Parse the global, scalar parameters. These are "committed" to + // the context to make them available during object parsing. + boost::shared_ptr params_config(new MapElement()); + params_config->setValue(params_map); + buildParams(params_config); + + // Now parse the configuration objects. // Use a pre-ordered list of element ids to parse the elements in a // specific order if the list (parser_order_) is not empty; otherwise // elements are parsed in the order the value_map presents them. - if (!parse_order_.empty()) { // For each element_id in the parse order list, look for it in the // value map. If the element exists in the map, pass it and it's @@ -151,8 +192,8 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { int parsed_count = 0; std::map::const_iterator it; BOOST_FOREACH(element_id, parse_order_) { - it = values_map.find(element_id); - if (it != values_map.end()) { + it = objects_map.find(element_id); + if (it != objects_map.end()) { ++parsed_count; buildAndCommit(element_id, it->second); } @@ -175,7 +216,7 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { // or we parsed fewer than are in the map; then either the parse i // order is incomplete OR the map has unsupported values. if (!parsed_count || - (parsed_count && ((parsed_count + 1) < values_map.size()))) { + (parsed_count && ((parsed_count + 1) < objects_map.size()))) { LOG_ERROR(dctl_logger, DCTL_ORDER_ERROR); isc_throw(DCfgMgrBaseError, "Configuration contains elements not in parse order"); @@ -184,7 +225,7 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { // Order doesn't matter so iterate over the value map directly. // Pass each element and it's associated data in to be parsed. ConfigPair config_pair; - BOOST_FOREACH(config_pair, values_map) { + BOOST_FOREACH(config_pair, objects_map) { element_id = config_pair.first; buildAndCommit(element_id, config_pair.second); } @@ -207,6 +248,17 @@ DCfgMgrBase::parseConfig(isc::data::ConstElementPtr config_set) { return (answer); } +void +DCfgMgrBase::buildParams(isc::data::ConstElementPtr params_config) { + // Loop through scalars parsing them and committing them to storage. + BOOST_FOREACH(dhcp::ConfigPair param, params_config->mapValue()) { + // Call derivation's method to create the proper parser. + dhcp::ParserPtr parser(createConfigParser(param.first)); + parser->build(param.second); + parser->commit(); + } +} + void DCfgMgrBase::buildAndCommit(std::string& element_id, isc::data::ConstElementPtr value) { // Call derivation's implementation to create the appropriate parser diff --git a/src/bin/d2/d_cfg_mgr.h b/src/bin/d2/d_cfg_mgr.h index a0bd9bb267..d088feb8db 100644 --- a/src/bin/d2/d_cfg_mgr.h +++ b/src/bin/d2/d_cfg_mgr.h @@ -25,6 +25,9 @@ namespace isc { namespace d2 { +/// @brief Defines a map of ConstElementPtrs keyed by name +typedef std::map ElementMap; + /// @brief Exception thrown if the configuration manager encounters an error. class DCfgMgrBaseError : public isc::Exception { public: @@ -181,7 +184,7 @@ private: isc::dhcp::StringStoragePtr string_values_; }; -/// @brief Defines an unsorted, list of string Element IDs. +/// @brief Defines a sequence of Element IDs used to specify a parsing order. typedef std::vector ElementIdList; /// @brief Configuration Manager @@ -198,7 +201,16 @@ typedef std::vector ElementIdList; /// /// @code /// make backup copy of configuration context -/// for each top level element in new configuration +/// Split top-level configuration elements into to sets: +/// 1. Set of scalar elements (strings, booleans, ints, etc..) +/// 2. Set of object elements (maps, lists, etc...) +/// For each entry in the scalar set: +/// get derivation-specific parser for element +/// run parser +/// update context with parsed results +/// break on error +/// +/// For each entry in the object set; /// get derivation-specific parser for element /// run parser /// update context with parsed results @@ -208,9 +220,9 @@ typedef std::vector ElementIdList; /// restore configuration context from backup /// @endcode /// -/// After making a backup of the current context, it iterates over the top-level -/// elements in the new configuration. The order in which the elements are -/// processed is either: +/// The above structuring ensures that global parameters are parsed first +/// making them available during subsequent object element parsing. The order +/// in which the object elements are processed is either: /// /// 1. Natural order presented by the configuration set /// 2. Specific order determined by a list of element ids @@ -256,9 +268,10 @@ public: /// @brief Adds a given element id to the end of the parse order list. /// - /// The order in which elements are retrieved from this is the order in - /// which they are added to the list. Derivations should use this method - /// to populate the parse order as part of their constructor. + /// The order in which object elements are retrieved from this is the + /// order in which they are added to the list. Derivations should use this + /// method to populate the parse order as part of their constructor. + /// Scalar parameters should NOT be included in this list. /// /// @param element_id is the string name of the element as it will appear /// in the configuration set. @@ -281,6 +294,20 @@ public: } protected: + /// @brief Parses a set of scalar configuration elements into global + /// parameters + /// + /// For each scalar element in the set: + /// - create a parser for the element + /// - invoke the parser's build method + /// - invoke the parser's commit method + /// + /// This will commit the values to context storage making them accessible + /// during object parsing. + /// + /// @param params_config set of scalar configuration elements to parse + virtual void buildParams(isc::data::ConstElementPtr params_config); + /// @brief Create a parser instance based on an element id. /// /// Given an element_id returns an instance of the appropriate parser. @@ -295,6 +322,27 @@ protected: virtual isc::dhcp::ParserPtr createConfigParser(const std::string& element_id) = 0; + /// @brief Abstract factory which creates a context instance. + /// + /// This method is used at the beginning of configuration process to + /// create a fresh, empty copy of the derivation-specific context. This + /// new context will be populated during the configuration process + /// and will replace the existing context provided the configuration + /// process completes without error. + /// + /// @return Returns a DCfgContextBasePtr to the new context instance. + virtual DCfgContextBasePtr createNewContext() = 0; + + /// @brief Replaces existing context with a new, emtpy context. + void resetContext(); + + /// @brief Update the current context. + /// + /// Replaces the existing context with the given context. + /// @param context Pointer to the new context. + /// @throw DCfgMgrBaseError if context is NULL. + void setContext(DCfgContextBasePtr& context); + private: /// @brief Parse a configuration element. diff --git a/src/bin/d2/dhcp-ddns.spec b/src/bin/d2/dhcp-ddns.spec index b4240b1420..ac336d7b6d 100644 --- a/src/bin/d2/dhcp-ddns.spec +++ b/src/bin/d2/dhcp-ddns.spec @@ -4,26 +4,36 @@ "module_name": "DhcpDdns", "module_description": "DHPC-DDNS Service", "config_data": [ - { - "item_name": "interface", - "item_type": "string", - "item_optional": true, - "item_default": "eth0" - }, - { "item_name": "ip_address", "item_type": "string", "item_optional": false, "item_default": "127.0.0.1" }, - { "item_name": "port", "item_type": "integer", "item_optional": true, "item_default": 53001 }, + { + "item_name": "dns_server_timeout", + "item_type": "integer", + "item_optional": true, + "item_default": 100 + }, + { + "item_name": "ncr_protocol", + "item_type": "string", + "item_optional": true, + "item_default": "UDP" + }, + { + "item_name": "ncr_format", + "item_type": "string", + "item_optional": true, + "item_default": "JSON" + }, { "item_name": "tsig_keys", "item_type": "list", diff --git a/src/bin/d2/nc_add.cc b/src/bin/d2/nc_add.cc index e350534298..70fff23c99 100644 --- a/src/bin/d2/nc_add.cc +++ b/src/bin/d2/nc_add.cc @@ -38,8 +38,10 @@ NameAddTransaction:: NameAddTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain) - : NameChangeTransaction(io_service, ncr, forward_domain, reverse_domain) { + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) + : NameChangeTransaction(io_service, ncr, forward_domain, reverse_domain, + cfg_mgr) { if (ncr->getChangeType() != isc::dhcp_ddns::CHG_ADD) { isc_throw (NameAddTransactionError, "NameAddTransaction, request type must be CHG_ADD"); diff --git a/src/bin/d2/nc_add.h b/src/bin/d2/nc_add.h index 1fe167b99e..7fcdf1f98d 100644 --- a/src/bin/d2/nc_add.h +++ b/src/bin/d2/nc_add.h @@ -87,13 +87,15 @@ public: /// @param ncr is the NameChangeRequest to fulfill /// @param forward_domain is the domain to use for forward DNS updates /// @param reverse_domain is the domain to use for reverse DNS updates + /// @param cfg_mgr pointer to the configuration manager /// /// @throw NameAddTransaction error if given request is not a CHG_ADD, /// NameChangeTransaction error for base class construction errors. NameAddTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain); + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr); /// @brief Destructor virtual ~NameAddTransaction(); diff --git a/src/bin/d2/nc_remove.cc b/src/bin/d2/nc_remove.cc index 5eb0521133..224974ad2d 100644 --- a/src/bin/d2/nc_remove.cc +++ b/src/bin/d2/nc_remove.cc @@ -35,8 +35,10 @@ NameRemoveTransaction:: NameRemoveTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain) - : NameChangeTransaction(io_service, ncr, forward_domain, reverse_domain) { + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) + : NameChangeTransaction(io_service, ncr, forward_domain, reverse_domain, + cfg_mgr) { if (ncr->getChangeType() != isc::dhcp_ddns::CHG_REMOVE) { isc_throw (NameRemoveTransactionError, "NameRemoveTransaction, request type must be CHG_REMOVE"); diff --git a/src/bin/d2/nc_remove.h b/src/bin/d2/nc_remove.h index f7b0dcc081..fb79eb495d 100644 --- a/src/bin/d2/nc_remove.h +++ b/src/bin/d2/nc_remove.h @@ -83,13 +83,15 @@ public: /// @param ncr is the NameChangeRequest to fulfill /// @param forward_domain is the domain to use for forward DNS updates /// @param reverse_domain is the domain to use for reverse DNS updates + /// @param cfg_mgr pointer to the configuration manager /// /// @throw NameRemoveTransaction error if given request is not a CHG_REMOVE, /// NameChangeTransaction error for base class construction errors. NameRemoveTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain); + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr); /// @brief Destructor virtual ~NameRemoveTransaction(); diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc index f9319f0596..f615a03572 100644 --- a/src/bin/d2/nc_trans.cc +++ b/src/bin/d2/nc_trans.cc @@ -41,22 +41,22 @@ const int NameChangeTransaction::UPDATE_FAILED_EVT; const int NameChangeTransaction::NCT_DERIVED_EVENT_MIN; -const unsigned int NameChangeTransaction::DNS_UPDATE_DEFAULT_TIMEOUT; const unsigned int NameChangeTransaction::MAX_UPDATE_TRIES_PER_SERVER; NameChangeTransaction:: NameChangeTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain) + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) : io_service_(io_service), ncr_(ncr), forward_domain_(forward_domain), reverse_domain_(reverse_domain), dns_client_(), dns_update_request_(), dns_update_status_(DNSClient::OTHER), dns_update_response_(), forward_change_completed_(false), reverse_change_completed_(false), current_server_list_(), current_server_(), next_server_pos_(0), - update_attempts_(0) { - // @todo if io_service is NULL we are multi-threading and should - // instantiate our own + update_attempts_(0), cfg_mgr_(cfg_mgr) { + /// @todo if io_service is NULL we are multi-threading and should + /// instantiate our own if (!io_service_) { isc_throw(NameChangeTransactionError, "IOServicePtr cannot be null"); } @@ -75,6 +75,11 @@ NameChangeTransaction(IOServicePtr& io_service, isc_throw(NameChangeTransactionError, "Reverse change must have a reverse domain"); } + + if (!cfg_mgr_) { + isc_throw(NameChangeTransactionError, + "Configuration manager cannot be null"); + } } NameChangeTransaction::~NameChangeTransaction(){ @@ -171,11 +176,10 @@ NameChangeTransaction::sendUpdate(const std::string& comment, // use_tsig_ is true. We should be able to navigate to the TSIG key // for the current server. If not we would need to add that. - // @todo time out should ultimately be configurable, down to - // server level? + D2ParamsPtr d2_params = cfg_mgr_->getD2Params(); dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(), current_server_->getPort(), *dns_update_request_, - DNS_UPDATE_DEFAULT_TIMEOUT); + d2_params->getDnsServerTimeout()); // Message is on its way, so the next event should be NOP_EVT. postNextEvent(NOP_EVT); diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h index b747839e6e..17f7f46ce9 100644 --- a/src/bin/d2/nc_trans.h +++ b/src/bin/d2/nc_trans.h @@ -19,7 +19,7 @@ #include #include -#include +#include #include #include #include @@ -168,6 +168,7 @@ public: /// @param ncr is the NameChangeRequest to fulfill /// @param forward_domain is the domain to use for forward DNS updates /// @param reverse_domain is the domain to use for reverse DNS updates + /// @param cfg_mgr reference to the current configuration manager /// /// @throw NameChangeTransactionError if given an null request, /// if forward change is enabled but forward domain is null, if @@ -175,7 +176,8 @@ public: NameChangeTransaction(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain); + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr); /// @brief Destructor virtual ~NameChangeTransaction(); @@ -570,6 +572,9 @@ private: /// @brief Number of transmit attempts for the current request. size_t update_attempts_; + + /// @brief Pointer to the configuration manager. + D2CfgMgrPtr cfg_mgr_; }; /// @brief Defines a pointer to a NameChangeTransaction. diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index e1b3313f1d..79d162ba7b 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -39,7 +39,7 @@ class D2CfgMgrTest : public ConfigParseTest { public: /// @brief Constructor - D2CfgMgrTest():cfg_mgr_(new D2CfgMgr) { + D2CfgMgrTest():cfg_mgr_(new D2CfgMgr()), d2_params_() { } /// @brief Destructor @@ -48,6 +48,71 @@ public: /// @brief Configuration manager instance. D2CfgMgrPtr cfg_mgr_; + + /// @brief Build JSON configuration string for a D2Params element + /// + /// Constructs a JSON string for "params" element using replacable + /// parameters. + /// + /// @param ip_address string to insert as ip_address value + /// @param port integer to insert as port value + /// @param dns_server_timeout integer to insert as dns_server_timeout value + /// @param ncr_protocol string to insert as ncr_protocol value + /// @param ncr_format string to insert as ncr_format value + /// + /// @return std::string containing the JSON configuration text + std::string makeParamsConfigString(const std::string& ip_address, + const int port, + const int dns_server_timeout, + const std::string& ncr_protocol, + const std::string& ncr_format) { + std::ostringstream config; + config << + "{" + " \"ip_address\": \"" << ip_address << "\" , " + " \"port\": " << port << " , " + " \"dns_server_timeout\": " << dns_server_timeout << " , " + " \"ncr_protocol\": \"" << ncr_protocol << "\" , " + " \"ncr_format\": \"" << ncr_format << "\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + return (config.str()); + } + + /// @brief Parses a configuration string and tests against a given outcome + /// + /// Convenience method which accepts JSON text and an expected pass or fail + /// outcome. It converts the text into an ElementPtr and passes that to + /// configuration manager's parseConfig method. It then tests the + /// parse result against the expected outcome If they do not match it + /// the method asserts a failure. If they do match, it refreshes the + /// the D2Params pointer with the newly parsed instance. + /// + /// @param config_str the JSON configuration text to parse + /// @param should_fail boolean indicator if the parsing should fail or not. + /// It defaults to false. + void runConfig(std::string config_str, bool should_fail=false) { + // We assume the config string is valid JSON. + ASSERT_TRUE(fromJSON(config_str)); + + // Parse the configuration and verify we got the expected outcome. + answer_ = cfg_mgr_->parseConfig(config_set_); + ASSERT_TRUE(checkAnswer(should_fail)); + + // Verify that the D2 context can be retrieved and is not null. + D2CfgContextPtr context; + ASSERT_NO_THROW(context = cfg_mgr_->getD2CfgContext()); + + // Verify that the global scalars have the proper values. + d2_params_ = context->getD2Params(); + ASSERT_TRUE(d2_params_); + } + + /// @brief Pointer the D2Params most recently parsed. + D2ParamsPtr d2_params_; }; /// @brief Tests that the spec file is valid. @@ -245,6 +310,150 @@ public: isc::dhcp::ParserPtr parser_; }; +/// @brief Tests a basic valid configuration for D2Param. +TEST_F(D2CfgMgrTest, validParamsEntry) { + // Verify that ip_address can be valid v4 address. + std::string config = makeParamsConfigString ("192.0.0.1", 777, 333, + "UDP", "JSON"); + runConfig(config); + + EXPECT_EQ(isc::asiolink::IOAddress("192.0.0.1"), + d2_params_->getIpAddress()); + EXPECT_EQ(777, d2_params_->getPort()); + EXPECT_EQ(333, d2_params_->getDnsServerTimeout()); + EXPECT_EQ(dhcp_ddns::NCR_UDP, d2_params_->getNcrProtocol()); + EXPECT_EQ(dhcp_ddns::FMT_JSON, d2_params_->getNcrFormat()); + + // Verify that ip_address can be valid v6 address. + config = makeParamsConfigString ("3001::5", 777, 333, "UDP", "JSON"); + runConfig(config); + + // Verify that the global scalars have the proper values. + EXPECT_EQ(isc::asiolink::IOAddress("3001::5"), + d2_params_->getIpAddress()); +} + +/// @brief Tests default values for D2Params. +/// It verifies that D2Params is populated with default value for optional +/// parameter if not supplied in the configuration. +/// Currently they are all optional. +TEST_F(D2CfgMgrTest, defaultValues) { + + // Check that omitting ip_address gets you its default + std::string config = + "{" + " \"port\": 777 , " + " \"dns_server_timeout\": 333 , " + " \"ncr_protocol\": \"UDP\" , " + " \"ncr_format\": \"JSON\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + runConfig(config); + EXPECT_EQ(isc::asiolink::IOAddress(D2Params::DFT_IP_ADDRESS), + d2_params_->getIpAddress()); + + // Check that omitting port gets you its default + config = + "{" + " \"ip_address\": \"192.0.0.1\" , " + " \"dns_server_timeout\": 333 , " + " \"ncr_protocol\": \"UDP\" , " + " \"ncr_format\": \"JSON\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + runConfig(config); + EXPECT_EQ(D2Params::DFT_PORT, d2_params_->getPort()); + + // Check that omitting timeout gets you its default + config = + "{" + " \"ip_address\": \"192.0.0.1\" , " + " \"port\": 777 , " + " \"ncr_protocol\": \"UDP\" , " + " \"ncr_format\": \"JSON\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + runConfig(config); + EXPECT_EQ(D2Params::DFT_DNS_SERVER_TIMEOUT, + d2_params_->getDnsServerTimeout()); + + // Check that protocol timeout gets you its default + config = + "{" + " \"ip_address\": \"192.0.0.1\" , " + " \"port\": 777 , " + " \"dns_server_timeout\": 333 , " + " \"ncr_format\": \"JSON\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + runConfig(config); + EXPECT_EQ(dhcp_ddns::stringToNcrProtocol(D2Params::DFT_NCR_PROTOCOL), + d2_params_->getNcrProtocol()); + + // Check that format timeout gets you its default + config = + "{" + " \"ip_address\": \"192.0.0.1\" , " + " \"port\": 777 , " + " \"dns_server_timeout\": 333 , " + " \"ncr_protocol\": \"UDP\", " + "\"tsig_keys\": [], " + "\"forward_ddns\" : {}, " + "\"reverse_ddns\" : {} " + "}"; + + runConfig(config); + EXPECT_EQ(dhcp_ddns::stringToNcrFormat(D2Params::DFT_NCR_FORMAT), + d2_params_->getNcrFormat()); +} + +/// @brief Tests the enforcement of data validation when parsing D2Params. +/// It verifies that: +/// -# ip_address cannot be "0.0.0.0" +/// -# ip_address cannot be "::" +/// -# port cannot be 0 +/// -# dns_server_timeout cannat be 0 +/// -# ncr_protocol must be valid +/// -# ncr_format must be valid +TEST_F(D2CfgMgrTest, invalidEntry) { + // Cannot use IPv4 ANY address + std::string config = makeParamsConfigString ("0.0.0.0", 777, 333, + "UDP", "JSON"); + runConfig(config, 1); + + // Cannot use IPv6 ANY address + config = makeParamsConfigString ("::", 777, 333, "UDP", "JSON"); + runConfig(config, 1); + + // Cannot use port 0 + config = makeParamsConfigString ("127.0.0.1", 0, 333, "UDP", "JSON"); + runConfig(config, 1); + + // Cannot use dns server timeout of 0 + config = makeParamsConfigString ("127.0.0.1", 777, 0, "UDP", "JSON"); + runConfig(config, 1); + + // Invalid protocol + config = makeParamsConfigString ("127.0.0.1", 777, 333, "BOGUS", "JSON"); + runConfig(config, 1); + + // Invalid format + config = makeParamsConfigString ("127.0.0.1", 777, 333, "UDP", "BOGUS"); + runConfig(config, 1); +} + /// @brief Tests the enforcement of data validation when parsing TSIGKeyInfos. /// It verifies that: /// 1. Name cannot be blank. @@ -873,9 +1082,11 @@ TEST_F(D2CfgMgrTest, fullConfig) { // both the forward and reverse ddns managers. Both managers have two // domains with three servers per domain. std::string config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " + " \"dns_server_timeout\": 333 , " + " \"ncr_protocol\": \"UDP\" , " + " \"ncr_format\": \"JSON\", " "\"tsig_keys\": [" "{" " \"name\": \"d2_key.tmark.org\" , " @@ -924,6 +1135,7 @@ TEST_F(D2CfgMgrTest, fullConfig) { " { \"hostname\": \"six.rev\" } " " ] } " "] } }"; + ASSERT_TRUE(fromJSON(config)); // Verify that we can parse the configuration. @@ -934,18 +1146,16 @@ TEST_F(D2CfgMgrTest, fullConfig) { D2CfgContextPtr context; ASSERT_NO_THROW(context = cfg_mgr_->getD2CfgContext()); - // Verify that the application level scalars have the proper values. - std::string interface; - EXPECT_NO_THROW (context->getParam("interface", interface)); - EXPECT_EQ("eth1", interface); + // Verify that the global scalars have the proper values. + D2ParamsPtr& d2_params = context->getD2Params(); + ASSERT_TRUE(d2_params); - std::string ip_address; - EXPECT_NO_THROW (context->getParam("ip_address", ip_address)); - EXPECT_EQ("192.168.1.33", ip_address); - - uint32_t port = 0; - EXPECT_NO_THROW (context->getParam("port", port)); - EXPECT_EQ(88, port); + EXPECT_EQ(isc::asiolink::IOAddress("192.168.1.33"), + d2_params->getIpAddress()); + EXPECT_EQ(88, d2_params->getPort()); + EXPECT_EQ(333, d2_params->getDnsServerTimeout()); + EXPECT_EQ(dhcp_ddns::NCR_UDP, d2_params->getNcrProtocol()); + EXPECT_EQ(dhcp_ddns::FMT_JSON, d2_params->getNcrFormat()); // Verify that the forward manager can be retrieved. DdnsDomainListMgrPtr mgr = context->getForwardMgr(); @@ -1014,7 +1224,6 @@ TEST_F(D2CfgMgrTest, forwardMatch) { // Create configuration with one domain, one sub domain, and the wild // card. std::string config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " "\"tsig_keys\": [] ," @@ -1088,7 +1297,6 @@ TEST_F(D2CfgMgrTest, forwardMatch) { TEST_F(D2CfgMgrTest, matchNoWildcard) { // Create a configuration with one domain, one sub-domain, and NO wild card. std::string config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " "\"tsig_keys\": [] ," @@ -1136,7 +1344,6 @@ TEST_F(D2CfgMgrTest, matchNoWildcard) { /// This test verifies that any FQDN matches the wild card. TEST_F(D2CfgMgrTest, matchAll) { std::string config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " "\"tsig_keys\": [] ," @@ -1183,7 +1390,6 @@ TEST_F(D2CfgMgrTest, matchAll) { /// as a match. TEST_F(D2CfgMgrTest, matchReverse) { std::string config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " "\"tsig_keys\": [] ," diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index bd6f77042a..025d501c7e 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -35,7 +35,6 @@ namespace { /// @brief Valid configuration containing an unavailable IP address. const char* bad_ip_d2_config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"1.1.1.1\" , " "\"port\" : 5031, " "\"tsig_keys\": [" @@ -607,7 +606,6 @@ TEST_F(D2ProcessTest, fatalErrorShutdown) { /// loopback. TEST_F(D2ProcessTest, notLoopbackTest) { const char* config = "{ " - "\"interface\" : \"\" , " "\"ip_address\" : \"0.0.0.0\" , " "\"port\" : 53001, " "\"tsig_keys\": []," @@ -626,7 +624,6 @@ TEST_F(D2ProcessTest, notLoopbackTest) { /// DHCP_DDNS_NOT_ON_LOOPBACK is not issued. TEST_F(D2ProcessTest, v4LoopbackTest) { const char* config = "{ " - "\"interface\" : \"\" , " "\"ip_address\" : \"127.0.0.1\" , " "\"port\" : 53001, " "\"tsig_keys\": []," @@ -640,7 +637,6 @@ TEST_F(D2ProcessTest, v4LoopbackTest) { /// DHCP_DDNS_NOT_ON_LOOPBACK is not issued. TEST_F(D2ProcessTest, v6LoopbackTest) { const char* config = "{ " - "\"interface\" : \"\" , " "\"ip_address\" : \"::1\" , " "\"port\" : 53001, " "\"tsig_keys\": []," diff --git a/src/bin/d2/tests/d2_update_mgr_unittests.cc b/src/bin/d2/tests/d2_update_mgr_unittests.cc index f669aeb970..9718a4ae88 100644 --- a/src/bin/d2/tests/d2_update_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_update_mgr_unittests.cc @@ -120,7 +120,6 @@ public: void makeCannedConfig() { std::string canned_config_ = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"192.168.1.33\" , " "\"port\" : 88 , " "\"tsig_keys\": [] ," @@ -202,29 +201,22 @@ public: /// timer, the number of passes through the loop is also limited to /// a given number. This is a failsafe to guard against an infinite loop /// in the test. - /// - /// @param timeout_millisec Maximum amount of time to allow IOService to - /// run before failing the test. Note, If the intent of a test is to - /// verify proper handling of DNSClient timeouts, the value must be - /// slightly larger than that being used for DNSClient timeout value. - /// @param max_passes Maximum number of times through the loop before - /// failing the test. The default value of twenty is likely large enough - /// for most tests. The number of passes required for a given test can - /// vary. - void processAll(unsigned int timeout_millisec = - NameChangeTransaction::DNS_UPDATE_DEFAULT_TIMEOUT + 100, - size_t max_passes = 100) { + void processAll(size_t max_passes = 100) { // Loop until all the transactions have been dequeued and run through to // completion. size_t passes = 0; size_t handlers = 0; + + // Set the timeout to slightly more than DNSClient timeout to allow + // timeout processing to occur naturally. + size_t timeout = cfg_mgr_->getD2Params()->getDnsServerTimeout() + 100; while (update_mgr_->getQueueCount() || update_mgr_->getTransactionCount()) { ++passes; update_mgr_->sweep(); // If any transactions are waiting on IO, run the service. if (anyoneWaiting()) { - int cnt = runTimedIO(timeout_millisec); + int cnt = runTimedIO(timeout); // If cnt is zero then the service stopped unexpectedly. if (cnt == 0) { diff --git a/src/bin/d2/tests/d_cfg_mgr_unittests.cc b/src/bin/d2/tests/d_cfg_mgr_unittests.cc index 512b896bed..88438d215d 100644 --- a/src/bin/d2/tests/d_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d_cfg_mgr_unittests.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -45,6 +46,11 @@ public: virtual ~DCtorTestCfgMgr() { } + /// @brief Dummy implementation as this method is abstract. + virtual DCfgContextBasePtr createNewContext() { + return (DCfgContextBasePtr()); + } + /// @brief Dummy implementation as this method is abstract. virtual isc::dhcp::ParserPtr createConfigParser(const std::string& /* element_id */) { @@ -112,7 +118,7 @@ TEST(DCfgMgrBase, construction) { /// 4. An unknown element error is handled. TEST_F(DStubCfgMgrTest, basicParseTest) { // Create a simple configuration. - string config = "{ \"test-value\": 1000 } "; + string config = "{ \"test-value\": [] } "; ASSERT_TRUE(fromJSON(config)); // Verify that we can parse a simple configuration. @@ -151,6 +157,9 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) { std::string charlie("charlie"); std::string bravo("bravo"); std::string alpha("alpha"); + std::string string_test("string_test"); + std::string uint32_test("uint32_test"); + std::string bool_test("bool_test"); // Create the test configuration with the elements in "random" order. @@ -158,9 +167,15 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) { // are in lexical order by element_id. This means that iterating over // such an element set, will present the elements in lexical order. Should // this change, this test will need to be modified accordingly. - string config = "{ \"bravo\": 2, " - " \"alpha\": 1, " - " \"charlie\": 3 } "; + string config = "{" + " \"string_test\": \"hoopla\", " + " \"bravo\": [], " + " \"uint32_test\": 55, " + " \"alpha\": {}, " + " \"charlie\": [], " + " \"bool_test\": true " + "} "; + ASSERT_TRUE(fromJSON(config)); // Verify that non-ordered parsing, results in an as-they-come parse order. @@ -169,6 +184,13 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) { // present the elements in lexical order. Should this change, the expected // order list below would need to be changed accordingly). ElementIdList order_expected; + + // scalar params should be first and lexically + order_expected.push_back(bool_test); + order_expected.push_back(string_test); + order_expected.push_back(uint32_test); + + // objects second and lexically order_expected.push_back(alpha); order_expected.push_back(bravo); order_expected.push_back(charlie); @@ -212,8 +234,20 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) { answer_ = cfg_mgr_->parseConfig(config_set_); EXPECT_TRUE(checkAnswer(0)); + // Build expected order + // primitives should be first and lexically + order_expected.clear(); + order_expected.push_back(bool_test); + order_expected.push_back(string_test); + order_expected.push_back(uint32_test); + + // objects second and by the parse order + order_expected.push_back(charlie); + order_expected.push_back(bravo); + order_expected.push_back(alpha); + // Verify that the parsed order is the order we configured. - EXPECT_TRUE(cfg_mgr_->getParseOrder() == cfg_mgr_->parsed_order_); + EXPECT_TRUE(cfg_mgr_->parsed_order_ == order_expected); // Create a parse order list that has too many entries. Verify that // when parsing the test config, it fails. @@ -233,24 +267,24 @@ TEST_F(DStubCfgMgrTest, parseOrderTest) { /// 1. Boolean parameters can be parsed and retrieved. /// 2. Uint32 parameters can be parsed and retrieved. /// 3. String parameters can be parsed and retrieved. -/// 4. Derivation-specific parameters can be parsed and retrieved. -/// 5. Parsing a second configuration, updates the existing context values +/// 4. Map elements can be parsed and retrieved. +/// 5. List elements can be parsed and retrieved. +/// 6. Parsing a second configuration, updates the existing context values /// correctly. TEST_F(DStubCfgMgrTest, simpleTypesTest) { - // Fetch a derivation specific pointer to the context. - DStubContextPtr context = getStubContext(); - ASSERT_TRUE(context); - // Create a configuration with all of the parameters. string config = "{ \"bool_test\": true , " " \"uint32_test\": 77 , " " \"string_test\": \"hmmm chewy\" , " - " \"extra_test\": 430 } "; + " \"map_test\" : {} , " + " \"list_test\": [] }"; ASSERT_TRUE(fromJSON(config)); // Verify that the configuration parses without error. answer_ = cfg_mgr_->parseConfig(config_set_); ASSERT_TRUE(checkAnswer(0)); + DStubContextPtr context = getStubContext(); + ASSERT_TRUE(context); // Verify that the boolean parameter was parsed correctly by retrieving // its value from the context. @@ -270,22 +304,26 @@ TEST_F(DStubCfgMgrTest, simpleTypesTest) { EXPECT_NO_THROW(context->getParam("string_test", actual_string)); EXPECT_EQ("hmmm chewy", actual_string); - // Verify that the "extra" parameter was parsed correctly by retrieving - // its value from the context. - uint32_t actual_extra = 0; - EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra)); - EXPECT_EQ(430, actual_extra); + isc::data::ConstElementPtr object; + EXPECT_NO_THROW(context->getObjectParam("map_test", object)); + EXPECT_TRUE(object); + + EXPECT_NO_THROW(context->getObjectParam("list_test", object)); + EXPECT_TRUE(object); // Create a configuration which "updates" all of the parameter values. string config2 = "{ \"bool_test\": false , " " \"uint32_test\": 88 , " " \"string_test\": \"ewww yuk!\" , " - " \"extra_test\": 11 } "; + " \"map_test2\" : {} , " + " \"list_test2\": [] }"; ASSERT_TRUE(fromJSON(config2)); // Verify that the configuration parses without error. answer_ = cfg_mgr_->parseConfig(config_set_); EXPECT_TRUE(checkAnswer(0)); + context = getStubContext(); + ASSERT_TRUE(context); // Verify that the boolean parameter was updated correctly by retrieving // its value from the context. @@ -305,31 +343,38 @@ TEST_F(DStubCfgMgrTest, simpleTypesTest) { EXPECT_NO_THROW(context->getParam("string_test", actual_string)); EXPECT_EQ("ewww yuk!", actual_string); - // Verify that the "extra" parameter was updated correctly by retrieving - // its value from the context. - actual_extra = 0; - EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra)); - EXPECT_EQ(11, actual_extra); + // Verify previous objects are not there. + EXPECT_THROW(context->getObjectParam("map_test", object), + isc::dhcp::DhcpConfigError); + EXPECT_THROW(context->getObjectParam("list_test", object), + isc::dhcp::DhcpConfigError); + + // Verify new map object is there. + EXPECT_NO_THROW(context->getObjectParam("map_test2", object)); + EXPECT_TRUE(object); + + // Verify new list object is there. + EXPECT_NO_THROW(context->getObjectParam("list_test2", object)); + EXPECT_TRUE(object); } /// @brief Tests that the configuration context is preserved after failure /// during parsing causes a rollback. /// 1. Verifies configuration context rollback. TEST_F(DStubCfgMgrTest, rollBackTest) { - // Fetch a derivation specific pointer to the context. - DStubContextPtr context = getStubContext(); - ASSERT_TRUE(context); - // Create a configuration with all of the parameters. string config = "{ \"bool_test\": true , " " \"uint32_test\": 77 , " " \"string_test\": \"hmmm chewy\" , " - " \"extra_test\": 430 } "; + " \"map_test\" : {} , " + " \"list_test\": [] }"; ASSERT_TRUE(fromJSON(config)); // Verify that the configuration parses without error. answer_ = cfg_mgr_->parseConfig(config_set_); EXPECT_TRUE(checkAnswer(0)); + DStubContextPtr context = getStubContext(); + ASSERT_TRUE(context); // Verify that all of parameters have the expected values. bool actual_bool = false; @@ -344,16 +389,20 @@ TEST_F(DStubCfgMgrTest, rollBackTest) { EXPECT_NO_THROW(context->getParam("string_test", actual_string)); EXPECT_EQ("hmmm chewy", actual_string); - uint32_t actual_extra = 0; - EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra)); - EXPECT_EQ(430, actual_extra); + isc::data::ConstElementPtr object; + EXPECT_NO_THROW(context->getObjectParam("map_test", object)); + EXPECT_TRUE(object); + + EXPECT_NO_THROW(context->getObjectParam("list_test", object)); + EXPECT_TRUE(object); // Create a configuration which "updates" all of the parameter values // plus one unknown at the end. string config2 = "{ \"bool_test\": false , " " \"uint32_test\": 88 , " " \"string_test\": \"ewww yuk!\" , " - " \"extra_test\": 11 , " + " \"map_test2\" : {} , " + " \"list_test2\": [] , " " \"zeta_unknown\": 33 } "; ASSERT_TRUE(fromJSON(config2)); @@ -361,9 +410,8 @@ TEST_F(DStubCfgMgrTest, rollBackTest) { SimFailure::set(SimFailure::ftElementUnknown); answer_ = cfg_mgr_->parseConfig(config_set_); EXPECT_TRUE(checkAnswer(1)); - - // Refresh our local pointer. context = getStubContext(); + ASSERT_TRUE(context); // Verify that all of parameters have the original values. actual_bool = false; @@ -378,9 +426,11 @@ TEST_F(DStubCfgMgrTest, rollBackTest) { EXPECT_NO_THROW(context->getParam("string_test", actual_string)); EXPECT_EQ("hmmm chewy", actual_string); - actual_extra = 0; - EXPECT_NO_THROW(context->getExtraParam("extra_test", actual_extra)); - EXPECT_EQ(430, actual_extra); + EXPECT_NO_THROW(context->getObjectParam("map_test", object)); + EXPECT_TRUE(object); + + EXPECT_NO_THROW(context->getObjectParam("list_test", object)); + EXPECT_TRUE(object); } } // end of anonymous namespace diff --git a/src/bin/d2/tests/d_test_stubs.cc b/src/bin/d2/tests/d_test_stubs.cc index 319dcd74e4..07dc1d4d37 100644 --- a/src/bin/d2/tests/d_test_stubs.cc +++ b/src/bin/d2/tests/d_test_stubs.cc @@ -22,7 +22,6 @@ namespace isc { namespace d2 { const char* valid_d2_config = "{ " - "\"interface\" : \"eth1\" , " "\"ip_address\" : \"127.0.0.1\" , " "\"port\" : 5031, " "\"tsig_keys\": [" @@ -218,16 +217,18 @@ DStubController::~DStubController() { // Initialize controller wrapper's static instance getter member. DControllerTest::InstanceGetter DControllerTest::instanceGetter_ = NULL; -//************************** TestParser ************************* +//************************** ObjectParser ************************* -TestParser::TestParser(const std::string& param_name):param_name_(param_name) { +ObjectParser::ObjectParser(const std::string& param_name, + ObjectStoragePtr& object_values) + : param_name_(param_name), object_values_(object_values) { } -TestParser::~TestParser(){ +ObjectParser::~ObjectParser(){ } void -TestParser::build(isc::data::ConstElementPtr new_config) { +ObjectParser::build(isc::data::ConstElementPtr new_config) { if (SimFailure::shouldFailOn(SimFailure::ftElementBuild)) { // Simulates an error during element data parsing. isc_throw (DCfgMgrBaseError, "Simulated build exception"); @@ -237,29 +238,33 @@ TestParser::build(isc::data::ConstElementPtr new_config) { } void -TestParser::commit() { +ObjectParser::commit() { if (SimFailure::shouldFailOn(SimFailure::ftElementCommit)) { // Simulates an error while committing the parsed element data. throw std::runtime_error("Simulated commit exception"); } + + object_values_->setParam(param_name_, value_, + isc::data::Element::Position()); } //************************** DStubContext ************************* -DStubContext::DStubContext(): extra_values_(new isc::dhcp::Uint32Storage()) { +DStubContext::DStubContext(): object_values_(new ObjectStorage()) { } DStubContext::~DStubContext() { } void -DStubContext::getExtraParam(const std::string& name, uint32_t& value) { - value = extra_values_->getParam(name); +DStubContext::getObjectParam(const std::string& name, + isc::data::ConstElementPtr& value) { + value = object_values_->getParam(name); } -isc::dhcp::Uint32StoragePtr -DStubContext::getExtraStorage() { - return (extra_values_); +ObjectStoragePtr& +DStubContext::getObjectStorage() { + return (object_values_); } DCfgContextBasePtr @@ -268,7 +273,7 @@ DStubContext::clone() { } DStubContext::DStubContext(const DStubContext& rhs): DCfgContextBase(rhs), - extra_values_(new isc::dhcp::Uint32Storage(*(rhs.extra_values_))) { + object_values_(new ObjectStorage(*(rhs.object_values_))) { } //************************** DStubCfgMgr ************************* @@ -280,40 +285,43 @@ DStubCfgMgr::DStubCfgMgr() DStubCfgMgr::~DStubCfgMgr() { } +DCfgContextBasePtr +DStubCfgMgr::createNewContext() { + return (DCfgContextBasePtr (new DStubContext())); +} + isc::dhcp::ParserPtr DStubCfgMgr::createConfigParser(const std::string& element_id) { - isc::dhcp::DhcpConfigParser* parser = NULL; - DStubContextPtr context = - boost::dynamic_pointer_cast(getContext()); + isc::dhcp::ParserPtr parser; + DStubContextPtr context + = boost::dynamic_pointer_cast(getContext()); if (element_id == "bool_test") { - parser = new isc::dhcp::BooleanParser(element_id, - context->getBooleanStorage()); + parser.reset(new isc::dhcp:: + BooleanParser(element_id, + context->getBooleanStorage())); } else if (element_id == "uint32_test") { - parser = new isc::dhcp::Uint32Parser(element_id, - context->getUint32Storage()); + parser.reset(new isc::dhcp::Uint32Parser(element_id, + context->getUint32Storage())); } else if (element_id == "string_test") { - parser = new isc::dhcp::StringParser(element_id, - context->getStringStorage()); - } else if (element_id == "extra_test") { - parser = new isc::dhcp::Uint32Parser(element_id, - context->getExtraStorage()); + parser.reset(new isc::dhcp::StringParser(element_id, + context->getStringStorage())); } else { // Fail only if SimFailure dictates we should. This makes it easier // to test parse ordering, by permitting a wide range of element ids // to "succeed" without specifically supporting them. if (SimFailure::shouldFailOn(SimFailure::ftElementUnknown)) { - isc_throw(DCfgMgrBaseError, "Configuration parameter not supported: " - << element_id); + isc_throw(DCfgMgrBaseError, + "Configuration parameter not supported: " << element_id); } - parsed_order_.push_back(element_id); - parser = new TestParser(element_id); + // Going to assume anything else is an object element. + parser.reset(new ObjectParser(element_id, context->getObjectStorage())); } - return (isc::dhcp::ParserPtr(parser)); + parsed_order_.push_back(element_id); + return (parser); } - }; // namespace isc::d2 }; // namespace isc diff --git a/src/bin/d2/tests/d_test_stubs.h b/src/bin/d2/tests/d_test_stubs.h index 7680a985ca..1e456e3d9b 100644 --- a/src/bin/d2/tests/d_test_stubs.h +++ b/src/bin/d2/tests/d_test_stubs.h @@ -135,9 +135,9 @@ public: /// indicate an abnormal termination. virtual void run(); - /// @brief Implements the process shutdown procedure. + /// @brief Implements the process shutdown procedure. /// - /// This sets the instance shutdown flag monitored by run() and stops + /// This sets the instance shutdown flag monitored by run() and stops /// the IO service. virtual isc::data::ConstElementPtr shutdown(isc::data::ConstElementPtr); @@ -446,9 +446,12 @@ public: } }; -/// @brief Simple parser derivation for testing the basics of configuration -/// parsing. -class TestParser : public isc::dhcp::DhcpConfigParser { +/// @brief a collection of elements that store uint32 values +typedef isc::dhcp::ValueStorage ObjectStorage; +typedef boost::shared_ptr ObjectStoragePtr; + +/// @brief Simple parser derivation for parsing object elements. +class ObjectParser : public isc::dhcp::DhcpConfigParser { public: /// @brief Constructor @@ -456,10 +459,10 @@ public: /// See @ref DhcpConfigParser class for details. /// /// @param param_name name of the parsed parameter - TestParser(const std::string& param_name); + ObjectParser(const std::string& param_name, ObjectStoragePtr& object_values); /// @brief Destructor - virtual ~TestParser(); + virtual ~ObjectParser(); /// @brief Builds parameter value. /// @@ -486,8 +489,12 @@ private: /// pointer to the parsed value of the parameter isc::data::ConstElementPtr value_; + + /// Pointer to the storage where committed value is stored. + ObjectStoragePtr object_values_; }; + /// @brief Test Derivation of the DCfgContextBase class. /// /// This class is used to test basic functionality of configuration context. @@ -505,6 +512,11 @@ public: /// @brief Destructor virtual ~DStubContext(); + /// @brief Creates a clone of a DStubContext. + /// + /// @return returns a pointer to the new clone. + virtual DCfgContextBasePtr clone(); + /// @brief Fetches the value for a given "extra" configuration parameter /// from the context. /// @@ -513,17 +525,10 @@ public: /// value. /// @throw throws DhcpConfigError if the context does not contain the /// parameter. - void getExtraParam(const std::string& name, uint32_t& value); + void getObjectParam(const std::string& name, + isc::data::ConstElementPtr& value); - /// @brief Fetches the extra storage. - /// - /// @return returns a pointer to the extra storage. - isc::dhcp::Uint32StoragePtr getExtraStorage(); - - /// @brief Creates a clone of a DStubContext. - /// - /// @return returns a pointer to the new clone. - virtual DCfgContextBasePtr clone(); + ObjectStoragePtr& getObjectStorage(); protected: /// @brief Copy constructor @@ -533,8 +538,8 @@ private: /// @brief Private assignment operator, not implemented. DStubContext& operator=(const DStubContext& rhs); - /// @brief Extra storage for uint32 parameters. - isc::dhcp::Uint32StoragePtr extra_values_; + /// @brief Stores non-scalar configuration elements + ObjectStoragePtr object_values_; }; /// @brief Defines a pointer to DStubContext. @@ -579,6 +584,9 @@ public: /// @brief A list for remembering the element ids in the order they were /// parsed. ElementIdList parsed_order_; + + /// @todo + virtual DCfgContextBasePtr createNewContext(); }; /// @brief Defines a pointer to DStubCfgMgr. @@ -609,9 +617,9 @@ public: try { config_set_ = isc::data::Element::fromJSON(json_text); } catch (const isc::Exception &ex) { - return (::testing::AssertionFailure(::testing::Message() << - "JSON text failed to parse:" - << ex.what())); + return (::testing::AssertionFailure(::testing::Message() << + "JSON text failed to parse:" + << ex.what())); } return (::testing::AssertionSuccess()); @@ -631,7 +639,7 @@ public: /// @brief Compares the status in the given parse result to a given value. /// /// @param answer Element set containing an integer response and string - /// comment. + /// comment. /// @param should_be is an integer against which to compare the status. /// /// @return returns AssertionSuccess if there were no parsing errors, @@ -645,8 +653,8 @@ public: return (testing::AssertionSuccess()); } - return (::testing::AssertionFailure(::testing::Message() << - "checkAnswer rcode:" << rcode + return (::testing::AssertionFailure(::testing::Message() << + "checkAnswer rcode:" << rcode << " comment: " << *comment)); } diff --git a/src/bin/d2/tests/nc_add_unittests.cc b/src/bin/d2/tests/nc_add_unittests.cc index 8b91f5a595..bb8d841251 100644 --- a/src/bin/d2/tests/nc_add_unittests.cc +++ b/src/bin/d2/tests/nc_add_unittests.cc @@ -35,8 +35,10 @@ public: NameAddStub(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain) - : NameAddTransaction(io_service, ncr, forward_domain, reverse_domain), + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) + : NameAddTransaction(io_service, ncr, forward_domain, reverse_domain, + cfg_mgr), simulate_send_exception_(false), simulate_build_request_exception_(false) { } @@ -212,7 +214,7 @@ public: // Now create the test transaction as would occur in update manager. return (NameAddStubPtr(new NameAddStub(io_service_, ncr_, forward_domain_, - reverse_domain_))); + reverse_domain_, cfg_mgr_))); } /// @brief Creates a transaction which requests an IPv6 DNS update. @@ -230,7 +232,8 @@ public: // Now create the test transaction as would occur in update manager. return (NameAddStubPtr(new NameAddStub(io_service_, ncr_, forward_domain_, - reverse_domain_))); + reverse_domain_, + cfg_mgr_))); } /// @brief Create a test transaction at a known point in the state model. @@ -266,6 +269,7 @@ public: /// 2. Valid construction functions properly TEST(NameAddTransaction, construction) { IOServicePtr io_service(new isc::asiolink::IOService()); + D2CfgMgrPtr cfg_mgr(new D2CfgMgr()); const char* msg_str = "{" @@ -291,13 +295,14 @@ TEST(NameAddTransaction, construction) { // Verify that construction with wrong change type fails. EXPECT_THROW(NameAddTransaction(io_service, ncr, - forward_domain, reverse_domain), + forward_domain, reverse_domain, cfg_mgr), NameAddTransactionError); // Verify that a valid construction attempt works. ncr->setChangeType(isc::dhcp_ddns::CHG_ADD); EXPECT_NO_THROW(NameAddTransaction(io_service, ncr, - forward_domain, reverse_domain)); + forward_domain, reverse_domain, + cfg_mgr)); } /// @brief Tests event and state dictionary construction and verification. diff --git a/src/bin/d2/tests/nc_remove_unittests.cc b/src/bin/d2/tests/nc_remove_unittests.cc index 6ca4ecf87e..8e19e6eeed 100644 --- a/src/bin/d2/tests/nc_remove_unittests.cc +++ b/src/bin/d2/tests/nc_remove_unittests.cc @@ -35,9 +35,10 @@ public: NameRemoveStub(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, DdnsDomainPtr& forward_domain, - DdnsDomainPtr& reverse_domain) + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) : NameRemoveTransaction(io_service, ncr, forward_domain, - reverse_domain), + reverse_domain, cfg_mgr), simulate_send_exception_(false), simulate_build_request_exception_(false) { } @@ -212,7 +213,8 @@ public: // Now create the test transaction as would occur in update manager. return (NameRemoveStubPtr(new NameRemoveStub(io_service_, ncr_, forward_domain_, - reverse_domain_))); + reverse_domain_, + cfg_mgr_))); } /// @brief Creates a transaction which requests an IPv6 DNS update. @@ -230,7 +232,8 @@ public: // Now create the test transaction as would occur in update manager. return (NameRemoveStubPtr(new NameRemoveStub(io_service_, ncr_, forward_domain_, - reverse_domain_))); + reverse_domain_, + cfg_mgr_))); } /// @brief Create a test transaction at a known point in the state model. @@ -268,6 +271,7 @@ public: /// 2. Valid construction functions properly TEST(NameRemoveTransaction, construction) { IOServicePtr io_service(new isc::asiolink::IOService()); + D2CfgMgrPtr cfg_mgr(new D2CfgMgr()); const char* msg_str = "{" @@ -293,13 +297,14 @@ TEST(NameRemoveTransaction, construction) { // Verify that construction with wrong change type fails. EXPECT_THROW(NameRemoveTransaction(io_service, ncr, - forward_domain, reverse_domain), + forward_domain, reverse_domain, cfg_mgr), NameRemoveTransactionError); // Verify that a valid construction attempt works. ncr->setChangeType(isc::dhcp_ddns::CHG_REMOVE); EXPECT_NO_THROW(NameRemoveTransaction(io_service, ncr, - forward_domain, reverse_domain)); + forward_domain, reverse_domain, + cfg_mgr)); } /// @brief Tests event and state dictionary construction and verification. diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/bin/d2/tests/nc_test_utils.cc index 2799b1c3dc..69e766fa0a 100644 --- a/src/bin/d2/tests/nc_test_utils.cc +++ b/src/bin/d2/tests/nc_test_utils.cc @@ -31,6 +31,7 @@ namespace d2 { const char* TEST_DNS_SERVER_IP = "127.0.0.1"; size_t TEST_DNS_SERVER_PORT = 5301; +//const bool HAS_RDATA = true; const bool NO_RDATA = false; //*************************** FauxServer class *********************** @@ -196,7 +197,7 @@ const unsigned int TransactionTest::FORWARD_CHG = 0x01; const unsigned int TransactionTest::REVERSE_CHG = 0x02; const unsigned int TransactionTest::FWD_AND_REV_CHG = REVERSE_CHG | FORWARD_CHG; -TransactionTest::TransactionTest() : ncr_() { +TransactionTest::TransactionTest() : ncr_(), cfg_mgr_(new D2CfgMgr()) { } TransactionTest::~TransactionTest() { diff --git a/src/bin/d2/tests/nc_test_utils.h b/src/bin/d2/tests/nc_test_utils.h index a99752528c..e681edcbfa 100644 --- a/src/bin/d2/tests/nc_test_utils.h +++ b/src/bin/d2/tests/nc_test_utils.h @@ -166,6 +166,7 @@ public: dhcp_ddns::NameChangeRequestPtr ncr_; DdnsDomainPtr forward_domain_; DdnsDomainPtr reverse_domain_; + D2CfgMgrPtr cfg_mgr_; /// #brief constants used to specify change directions for a transaction. static const unsigned int FORWARD_CHG; // Only forward change. diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/bin/d2/tests/nc_trans_unittests.cc index 5784647fd2..60bfb56c46 100644 --- a/src/bin/d2/tests/nc_trans_unittests.cc +++ b/src/bin/d2/tests/nc_trans_unittests.cc @@ -28,11 +28,13 @@ #include #include +#include #include using namespace std; using namespace isc; using namespace isc::d2; +using namespace boost::posix_time; namespace { @@ -59,10 +61,11 @@ public: /// Parameters match those needed by NameChangeTransaction. NameChangeStub(IOServicePtr& io_service, dhcp_ddns::NameChangeRequestPtr& ncr, - DdnsDomainPtr forward_domain, - DdnsDomainPtr reverse_domain) + DdnsDomainPtr& forward_domain, + DdnsDomainPtr& reverse_domain, + D2CfgMgrPtr& cfg_mgr) : NameChangeTransaction(io_service, ncr, forward_domain, - reverse_domain), + reverse_domain, cfg_mgr), use_stub_callback_(false) { } @@ -296,7 +299,7 @@ public: // Now create the test transaction as would occur in update manager. // Instantiate the transaction as would be done by update manager. return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr_, - forward_domain_, reverse_domain_))); + forward_domain_, reverse_domain_, cfg_mgr_))); } /// @brief Builds and then sends an update request @@ -348,6 +351,7 @@ public: /// 4. Valid construction functions properly TEST(NameChangeTransaction, construction) { IOServicePtr io_service(new isc::asiolink::IOService()); + D2CfgMgrPtr cfg_mgr(new D2CfgMgr()); const char* msg_str = "{" @@ -377,43 +381,54 @@ TEST(NameChangeTransaction, construction) { // @todo Subject to change if multi-threading is implemented. IOServicePtr empty; EXPECT_THROW(NameChangeTransaction(empty, ncr, - forward_domain, reverse_domain), + forward_domain, reverse_domain, cfg_mgr), NameChangeTransactionError); // Verify that construction with an empty NameChangeRequest throws. EXPECT_THROW(NameChangeTransaction(io_service, empty_ncr, - forward_domain, reverse_domain), + forward_domain, reverse_domain, cfg_mgr), NameChangeTransactionError); + // Verify that construction with an empty D2CfgMgr throws. + D2CfgMgrPtr empty_cfg; + EXPECT_THROW(NameChangeTransaction(io_service, empty_ncr, + forward_domain, reverse_domain, + empty_cfg), + NameChangeTransactionError); + + // Verify that construction with an empty forward domain when the // NameChangeRequest calls for a forward change throws. EXPECT_THROW(NameChangeTransaction(io_service, ncr, - empty_domain, reverse_domain), + empty_domain, reverse_domain, cfg_mgr), NameChangeTransactionError); // Verify that construction with an empty reverse domain when the // NameChangeRequest calls for a reverse change throws. EXPECT_THROW(NameChangeTransaction(io_service, ncr, - forward_domain, empty_domain), + forward_domain, empty_domain, cfg_mgr), NameChangeTransactionError); // Verify that a valid construction attempt works. EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr, - forward_domain, reverse_domain)); + forward_domain, reverse_domain, + cfg_mgr)); // Verify that an empty forward domain is allowed when the requests does // not include a forward change. ncr->setForwardChange(false); ncr->setReverseChange(true); EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr, - empty_domain, reverse_domain)); + empty_domain, reverse_domain, + cfg_mgr)); // Verify that an empty reverse domain is allowed when the requests does // not include a reverse change. ncr->setForwardChange(true); ncr->setReverseChange(false); EXPECT_NO_THROW(NameChangeTransaction(io_service, ncr, - forward_domain, empty_domain)); + forward_domain, empty_domain, + cfg_mgr)); } /// @brief General testing of member accessors. @@ -944,9 +959,11 @@ TEST_F(NameChangeTransactionTest, sendUpdateTimeout) { // not only it but the invoking test as well. In other words, if the // doOneExchange blows up the rest of test is pointless. I use // ASSERT_NO_FATAL_FAILURE to abort the test immediately. - ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change, - NameChangeTransaction:: - DNS_UPDATE_DEFAULT_TIMEOUT + 100)); + + D2ParamsPtr d2_params = cfg_mgr_->getD2Params(); + size_t timeout = d2_params->getDnsServerTimeout() + 100; + + ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change, timeout)); // Verify that next event is IO_COMPLETED_EVT and DNS status is TIMEOUT. ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT, diff --git a/src/lib/dns/tests/labelsequence_unittest.cc b/src/lib/dns/tests/labelsequence_unittest.cc index 2d0f256763..b593f8ccd0 100644 --- a/src/lib/dns/tests/labelsequence_unittest.cc +++ b/src/lib/dns/tests/labelsequence_unittest.cc @@ -649,6 +649,7 @@ const char* const root_servers[] = { "j.root-servers.net", "k.root-servers.net", "l.root-servers.net", "m.root-servers.net", NULL }; + const char* const jp_servers[] = { "a.dns.jp", "b.dns.jp", "c.dns.jp", "d.dns.jp", "e.dns.jp", "f.dns.jp", "g.dns.jp", NULL