From: Thomas Markwalder Date: Wed, 7 May 2014 20:18:54 +0000 (-0400) Subject: [3268] Modified D2CfgMgr and spec file X-Git-Tag: trac3434_base~26^2~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=67f5da73b63a0bdb5adfd9b25090ab459f64bee5;p=thirdparty%2Fkea.git [3268] Modified D2CfgMgr and spec file Added D2CfgMgr::buildParams() method to support the new parameters first processing. Removed support D2's global "interface" parameter, and added three more: - dns_server_timeout - ncr_protocol - ncr_format --- diff --git a/src/bin/d2/d2_cfg_mgr.cc b/src/bin/d2/d2_cfg_mgr.cc index 4bddb99a6f..28796f54dc 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..e12557be64 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 + /// paramter'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,8 @@ protected: /// @throw throws DCfgMgrBaseError if an error occurs. virtual isc::dhcp::ParserPtr createConfigParser(const std::string& element_id); + + virtual DCfgContextBasePtr createNewContext(); }; /// @brief Defines a shared pointer to D2CfgMgr. 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/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index e1b3313f1d..a9c476b9e5 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,58 @@ 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()); + } + + 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_); + } + + D2ParamsPtr d2_params_; }; /// @brief Tests that the spec file is valid. @@ -245,6 +297,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. @@ -864,6 +1060,9 @@ TEST(D2CfgMgr, construction) { EXPECT_NO_THROW(delete cfg_mgr); } +TEST_F(D2CfgMgrTest, paramsConfig) { +} + /// @brief Tests the parsing of a complete, valid DHCP-DDNS configuration. /// This tests passes the configuration into an instance of D2CfgMgr just /// as it would be done by d2_process in response to a configuration update @@ -873,9 +1072,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 +1125,7 @@ TEST_F(D2CfgMgrTest, fullConfig) { " { \"hostname\": \"six.rev\" } " " ] } " "] } }"; + ASSERT_TRUE(fromJSON(config)); // Verify that we can parse the configuration. @@ -934,18 +1136,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); - - std::string ip_address; - EXPECT_NO_THROW (context->getParam("ip_address", ip_address)); - EXPECT_EQ("192.168.1.33", ip_address); + // Verify that the global scalars have the proper values. + D2ParamsPtr& d2_params = context->getD2Params(); + ASSERT_TRUE(d2_params); - 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 +1214,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 +1287,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 +1334,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 +1380,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\": [] ,"