[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

src/bin/d2/d2_cfg_mgr.cc

@@ -30,12 +30,14 @@ typedef std::vector<uint8_t> 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

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.

src/bin/d2/dhcp-ddns.spec

@@ -5,19 +5,11 @@
     "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",
@@ -25,6 +17,24 @@
         "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",
         "item_optional": true, 

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\": [] ,"