[master] Merged trac3590 (migrate remaining items from CfgMgr to SrvConfig

src/bin/dhcp4/dhcp4_srv.cc

@@ -257,7 +257,7 @@ void
 Dhcpv4Exchange::copyDefaultOptions() {
     // Let's copy client-id to response. See RFC6842.
     // It is possible to disable RFC6842 to keep backward compatibility
-    bool echo = CfgMgr::instance().echoClientId();
+    bool echo = CfgMgr::instance().getCurrentCfg()->getEchoClientId();
     OptionPtr client_id = query_->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
     if (client_id && echo) {
         resp_->addOption(client_id);

src/bin/dhcp4/json_config_parser.cc

@@ -331,7 +331,7 @@ public:
         // Set whether v4 server is supposed to echo back client-id
         // (yes = RFC6842 compatible, no = backward compatibility)
         bool echo_client_id = getBoolean(global, "echo-client-id");
-        CfgMgr::instance().echoClientId(echo_client_id);
+        cfg->setEchoClientId(echo_client_id);
 
         // Set the probation period for decline handling.
         uint32_t probation_period =

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -1210,21 +1210,21 @@ TEST_F(Dhcp4ParserTest, echoClientId) {
     ASSERT_NO_THROW(json_true = parseDHCP4(config_true));
 
     // Let's check the default. It should be true
-    ASSERT_TRUE(CfgMgr::instance().echoClientId());
+    ASSERT_TRUE(CfgMgr::instance().getStagingCfg()->getEchoClientId());
 
     // Now check that "false" configuration is really applied.
     ConstElementPtr status;
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json_false));
-    ASSERT_FALSE(CfgMgr::instance().echoClientId());
+    ASSERT_FALSE(CfgMgr::instance().getStagingCfg()->getEchoClientId());
 
     CfgMgr::instance().clear();
 
     // Now check that "true" configuration is really applied.
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json_true));
-    ASSERT_TRUE(CfgMgr::instance().echoClientId());
+    ASSERT_TRUE(CfgMgr::instance().getStagingCfg()->getEchoClientId());
 
     // In any case revert back to the default value (true)
-    CfgMgr::instance().echoClientId(true);
+    CfgMgr::instance().getStagingCfg()->setEchoClientId(true);
 }
 
 // This test checks that the global match-client-id parameter is optional

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -744,7 +744,14 @@ TEST_F(Dhcpv4SrvTest, discoverEchoClientId) {
     checkResponse(offer, DHCPOFFER, 1234);
     checkClientId(offer, clientid);
 
-    CfgMgr::instance().echoClientId(false);
+    ConstSrvConfigPtr cfg = CfgMgr::instance().getCurrentCfg();
+    const Subnet4Collection* subnets = cfg->getCfgSubnets4()->getAll();
+    ASSERT_EQ(1, subnets->size());
+    CfgMgr::instance().clear();
+    CfgMgr::instance().getStagingCfg()->getCfgSubnets4()->add(subnets->at(0));
+    CfgMgr::instance().getStagingCfg()->setEchoClientId(false);
+    CfgMgr::instance().commit();
+    
     offer = srv.processDiscover(dis);
 
     // Check if we get response at all
@@ -812,7 +819,14 @@ TEST_F(Dhcpv4SrvTest, requestEchoClientId) {
     checkResponse(ack, DHCPACK, 1234);
     checkClientId(ack, clientid);
 
-    CfgMgr::instance().echoClientId(false);
+    ConstSrvConfigPtr cfg = CfgMgr::instance().getCurrentCfg();
+    const Subnet4Collection* subnets = cfg->getCfgSubnets4()->getAll();
+    ASSERT_EQ(1, subnets->size());
+    CfgMgr::instance().clear();
+    CfgMgr::instance().getStagingCfg()->getCfgSubnets4()->add(subnets->at(0));
+    CfgMgr::instance().getStagingCfg()->setEchoClientId(false);
+    CfgMgr::instance().commit();
+
     ack = srv.processRequest(dis);
 
     // Check if we get response at all

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -83,7 +83,6 @@ Dhcpv4SrvTest::~Dhcpv4SrvTest() {
 
     // Make sure that we revert to default value
     CfgMgr::instance().clear();
-    CfgMgr::instance().echoClientId(true);
 
     LibDHCP::clearRuntimeOptionDefs();
 
@@ -360,7 +359,8 @@ void Dhcpv4SrvTest::checkServerId(const Pkt4Ptr& rsp, const OptionPtr& expected_
 
 void Dhcpv4SrvTest::checkClientId(const Pkt4Ptr& rsp, const OptionPtr& expected_clientid) {
 
-    bool include_clientid = CfgMgr::instance().echoClientId();
+    bool include_clientid =
+        CfgMgr::instance().getCurrentCfg()->getEchoClientId();
 
     // check that server included our own client-id
     OptionPtr opt = rsp->getOption(DHO_DHCP_CLIENT_IDENTIFIER);

src/lib/dhcpsrv/cfgmgr.cc

@@ -10,14 +10,11 @@
 #include <dhcp/libdhcp++.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
-#include <dhcpsrv/subnet_id.h>
-#include <stats/stats_mgr.h>
 #include <sstream>
 #include <string>
 
 using namespace isc::asiolink;
 using namespace isc::util;
-using namespace isc::stats;
 
 namespace isc {
 namespace dhcp {
@@ -30,35 +27,6 @@ CfgMgr::instance() {
     return (cfg_mgr);
 }
 
-void
-CfgMgr::addOptionSpace4(const OptionSpacePtr& space) {
-    if (!space) {
-        isc_throw(InvalidOptionSpace,
-                  "provided option space object is NULL.");
-    }
-    OptionSpaceCollection::iterator it = spaces4_.find(space->getName());
-    if (it != spaces4_.end()) {
-        isc_throw(InvalidOptionSpace, "option space " << space->getName()
-                  << " already added.");
-    }
-    spaces4_.insert(make_pair(space->getName(), space));
-}
-
-void
-CfgMgr::addOptionSpace6(const OptionSpacePtr& space) {
-    if (!space) {
-        isc_throw(InvalidOptionSpace,
-                  "provided option space object is NULL.");
-    }
-    OptionSpaceCollection::iterator it = spaces6_.find(space->getName());
-    if (it != spaces6_.end()) {
-        isc_throw(InvalidOptionSpace, "option space " << space->getName()
-                  << " already added.");
-    }
-    spaces6_.insert(make_pair(space->getName(), space));
-}
-
-
 std::string CfgMgr::getDataDir() const {
     return (datadir_);
 }
@@ -68,18 +36,6 @@ CfgMgr::setDataDir(const std::string& datadir) {
     datadir_ = datadir;
 }
 
-bool
-CfgMgr::isDuplicate(const Subnet6& subnet) const {
-    for (Subnet6Collection::const_iterator subnet_it = subnets6_.begin();
-         subnet_it != subnets6_.end(); ++subnet_it) {
-        if ((*subnet_it)->getID() == subnet.getID()) {
-            return (true);
-        }
-    }
-    return (false);
-}
-
-
 void
 CfgMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) {
     ensureCurrentAllocated();
@@ -211,8 +167,7 @@ CfgMgr::getStagingCfg() {
 }
 
 CfgMgr::CfgMgr()
-    : datadir_(DHCP_DATA_DIR), echo_v4_client_id_(true),
-      d2_client_mgr_(), verbose_mode_(false) {
+    : datadir_(DHCP_DATA_DIR), d2_client_mgr_(), verbose_mode_(false) {
     // DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am
     // Note: the definition of DHCP_DATA_DIR needs to include quotation marks
     // See AM_CPPFLAGS definition in Makefile.am

src/lib/dhcpsrv/cfgmgr.h

@@ -13,7 +13,6 @@
 #include <dhcp/classify.h>
 #include <dhcpsrv/d2_client_mgr.h>
 #include <dhcpsrv/pool.h>
-#include <dhcpsrv/subnet.h>
 #include <dhcpsrv/srv_config.h>
 #include <util/buffer.h>
 
@@ -41,12 +40,9 @@ public:
 /// @brief Configuration Manager
 ///
 /// This singleton class holds the whole configuration for DHCPv4 and DHCPv6
-/// servers. It currently holds information about zero or more subnets6.
-/// Each subnet may contain zero or more pools. Pool4 and Pool6 is the most
-/// basic "chunk" of configuration. It contains a range of assignable
-/// addresses.
+/// servers.
 ///
-/// Below is a sketch of configuration inheritance (not implemented yet).
+/// Below is a sketch of configuration inheritance.
 /// Let's investigate the following configuration:
 ///
 /// @code
@@ -67,13 +63,9 @@ public:
 /// and a valid lifetime of 1000s. The second subnet has preferred lifetime
 /// of 500s, but valid lifetime of 2000s.
 ///
-/// Parameter inheritance is likely to be implemented in configuration handling
-/// routines, so there is no storage capability in a global scope for
-/// subnet-specific parameters.
-///
-/// @todo: Implement Subnet4 support (ticket #2237)
-/// @todo: Implement option definition support
-/// @todo: Implement parameter inheritance
+/// Parameter inheritance is implemented in dedicated classes. See
+/// @ref isc::dhcp::SimpleParser4::deriveParameters and
+/// @ref isc::dhcp::SimpleParser6::deriveParameters.
 class CfgMgr : public boost::noncopyable {
 public:
 
@@ -88,36 +80,6 @@ public:
     /// accessing it.
     static CfgMgr& instance();
 
-    /// @brief Adds new DHCPv4 option space to the collection.
-    ///
-    /// @param space option space to be added.
-    ///
-    /// @throw isc::dhcp::InvalidOptionSpace invalid option space
-    /// has been specified.
-    void addOptionSpace4(const OptionSpacePtr& space);
-
-    /// @brief Adds new DHCPv6 option space to the collection.
-    ///
-    /// @param space option space to be added.
-    ///
-    /// @throw isc::dhcp::InvalidOptionSpace invalid option space
-    /// has been specified.
-    void addOptionSpace6(const OptionSpacePtr& space);
-
-    /// @brief Return option spaces for DHCPv4.
-    ///
-    /// @return A collection of option spaces.
-    const OptionSpaceCollection& getOptionSpaces4() const {
-        return (spaces4_);
-    }
-
-    /// @brief Return option spaces for DHCPv6.
-    ///
-    /// @return A collection of option spaces.
-    const OptionSpaceCollection& getOptionSpaces6() const {
-        return (spaces6_);
-    }
-
     /// @brief returns path do the data directory
     ///
     /// This method returns a path to writable directory that DHCP servers
@@ -130,22 +92,6 @@ public:
     /// @param datadir New data directory.
     void setDataDir(const std::string& datadir);
 
-    /// @brief Sets whether server should send back client-id in DHCPv4
-    ///
-    /// This is a compatibility flag. The default (true) is compliant with
-    /// RFC6842. False is for backward compatibility.
-    ///
-    /// @param echo should the client-id be sent or not
-    void echoClientId(const bool echo) {
-        echo_v4_client_id_ = echo;
-    }
-
-    /// @brief Returns whether server should send back client-id in DHCPv4.
-    /// @return true if client-id should be returned, false otherwise.
-    bool echoClientId() const {
-        return (echo_v4_client_id_);
-    }
-
     /// @brief Updates the DHCP-DDNS client configuration to the given value.
     ///
     /// Passes the new configuration to the D2ClientMgr instance,
@@ -317,14 +263,6 @@ protected:
     /// @brief virtual destructor
     virtual ~CfgMgr();
 
-    /// @brief a container for IPv6 subnets.
-    ///
-    /// That is a simple vector of pointers. It does not make much sense to
-    /// optimize access time (e.g. using a map), because typical search
-    /// pattern will use calling inRange() method on each subnet until
-    /// a match is found.
-    Subnet6Collection subnets6_;
-
 private:
 
     /// @brief Checks if current configuration is created and creates it if needed.
@@ -334,25 +272,9 @@ private:
     /// default current configuration.
     void ensureCurrentAllocated();
 
-    /// @brief Checks that the IPv6 subnet with the given id already exists.
-    ///
-    /// @param subnet Subnet for which this function will check if the other
-    /// subnet with equal id already exists.
-    /// @return true if the duplicate subnet exists.
-    bool isDuplicate(const Subnet6& subnet) const;
-
-    /// @brief Container for defined DHCPv6 option spaces.
-    OptionSpaceCollection spaces6_;
-
-    /// @brief Container for defined DHCPv4 option spaces.
-    OptionSpaceCollection spaces4_;
-
     /// @brief directory where data files (e.g. server-id) are stored
     std::string datadir_;
 
-    /// Indicates whether v4 server should send back client-id
-    bool echo_v4_client_id_;
-
     /// @brief Manages the DHCP-DDNS client and its configuration.
     D2ClientMgr d2_client_mgr_;
 

src/lib/dhcpsrv/srv_config.cc

@@ -29,7 +29,7 @@ SrvConfig::SrvConfig()
       cfg_host_operations4_(CfgHostOperations::createConfig4()),
       cfg_host_operations6_(CfgHostOperations::createConfig6()),
       class_dictionary_(new ClientClassDictionary()),
-      decline_timer_(0), dhcp4o6_port_(0),
+      decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0),
       d2_client_config_(new D2ClientConfig()) {
 }
 
@@ -43,7 +43,7 @@ SrvConfig::SrvConfig(const uint32_t sequence)
       cfg_host_operations4_(CfgHostOperations::createConfig4()),
       cfg_host_operations6_(CfgHostOperations::createConfig6()),
       class_dictionary_(new ClientClassDictionary()),
-      decline_timer_(0), dhcp4o6_port_(0),
+      decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0),
       d2_client_config_(new D2ClientConfig()) {
 }
 

src/lib/dhcpsrv/srv_config.h

@@ -470,6 +470,22 @@ public:
         return (decline_timer_);
     }
 
+    /// @brief Sets whether server should send back client-id in DHCPv4
+    ///
+    /// This is a compatibility flag. The default (true) is compliant with
+    /// RFC6842. False is for backward compatibility.
+    ///
+    /// @param echo should the client-id be sent or not
+    void setEchoClientId(const bool echo) {
+        echo_v4_client_id_ = echo;
+    }
+
+    /// @brief Returns whether server should send back client-id in DHCPv4.
+    /// @return true if client-id should be returned, false otherwise.
+    bool getEchoClientId() const {
+        return (echo_v4_client_id_);
+    }
+
     /// @brief Sets DHCP4o6 IPC port
     ///
     /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication,
@@ -583,6 +599,9 @@ private:
     /// lease is recovered back to available state. Expressed in seconds.
     uint32_t decline_timer_;
 
+    /// @brief Indicates whether v4 server should send back client-id
+    bool echo_v4_client_id_;
+
     /// @brief DHCP4o6 IPC port
     ///
     /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication,

src/lib/dhcpsrv/tests/cfgmgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -317,89 +317,6 @@ TEST_F(CfgMgrTest, configuration) {
     EXPECT_TRUE(configuration->getLoggingInfo().empty());
 }
 
-// This test verifies that new DHCPv4 option spaces can be added to
-// the configuration manager and that duplicated option space is
-// rejected.
-TEST_F(CfgMgrTest, optionSpace4) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-
-    // Create some option spaces.
-    OptionSpacePtr space1(new OptionSpace("isc", false));
-    OptionSpacePtr space2(new OptionSpace("xyz", true));
-
-    // Add option spaces with different names and expect they
-    // are accepted.
-    ASSERT_NO_THROW(cfg_mgr.addOptionSpace4(space1));
-    ASSERT_NO_THROW(cfg_mgr.addOptionSpace4(space2));
-
-    // Validate that the option spaces have been added correctly.
-    const OptionSpaceCollection& spaces = cfg_mgr.getOptionSpaces4();
-
-    ASSERT_EQ(2, spaces.size());
-    EXPECT_FALSE(spaces.find("isc") == spaces.end());
-    EXPECT_FALSE(spaces.find("xyz") == spaces.end());
-
-    // Create another option space with the name that duplicates
-    // the existing option space.
-    OptionSpacePtr space3(new OptionSpace("isc", true));
-    // Expect that the duplicate option space is rejected.
-    ASSERT_THROW(
-        cfg_mgr.addOptionSpace4(space3), isc::dhcp::InvalidOptionSpace
-    );
-
-    /// @todo decode if a duplicate vendor space is allowed.
-}
-
-// This test verifies that new DHCPv6 option spaces can be added to
-// the configuration manager and that duplicated option space is
-// rejected.
-TEST_F(CfgMgrTest, optionSpace6) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-
-    // Create some option spaces.
-    OptionSpacePtr space1(new OptionSpace("isc", false));
-    OptionSpacePtr space2(new OptionSpace("xyz", true));
-
-    // Add option spaces with different names and expect they
-    // are accepted.
-    ASSERT_NO_THROW(cfg_mgr.addOptionSpace6(space1));
-    ASSERT_NO_THROW(cfg_mgr.addOptionSpace6(space2));
-
-    // Validate that the option spaces have been added correctly.
-    const OptionSpaceCollection& spaces = cfg_mgr.getOptionSpaces6();
-
-    ASSERT_EQ(2, spaces.size());
-    EXPECT_FALSE(spaces.find("isc") == spaces.end());
-    EXPECT_FALSE(spaces.find("xyz") == spaces.end());
-
-    // Create another option space with the name that duplicates
-    // the existing option space.
-    OptionSpacePtr space3(new OptionSpace("isc", true));
-    // Expect that the duplicate option space is rejected.
-    ASSERT_THROW(
-        cfg_mgr.addOptionSpace6(space3), isc::dhcp::InvalidOptionSpace
-    );
-
-    /// @todo decide if a duplicate vendor space is allowed.
-}
-
-// This test verifies that RFC6842 (echo client-id) compatibility may be
-// configured.
-TEST_F(CfgMgrTest, echoClientId) {
-    CfgMgr& cfg_mgr = CfgMgr::instance();
-
-    // Check that the default is true
-    EXPECT_TRUE(cfg_mgr.echoClientId());
-
-    // Check that it can be modified to false
-    cfg_mgr.echoClientId(false);
-    EXPECT_FALSE(cfg_mgr.echoClientId());
-
-    // Check that the default value can be restored
-    cfg_mgr.echoClientId(true);
-    EXPECT_TRUE(cfg_mgr.echoClientId());
-}
-
 // This test checks the D2ClientMgr wrapper methods.
 TEST_F(CfgMgrTest, d2ClientConfig) {
     // After CfgMgr construction, D2ClientMgr member should be initialized

src/lib/dhcpsrv/tests/srv_config_unittest.cc

@@ -261,6 +261,27 @@ TEST_F(SrvConfigTest, classDictionaryBasics) {
     EXPECT_EQ(ref_dictionary_->getClasses()->size(), cd->getClasses()->size());
 }
 
+// This test verifies that RFC6842 (echo client-id) compatibility may be
+// configured.
+TEST_F(SrvConfigTest, echoClientId) {
+    SrvConfig conf;
+
+    // Check that the default is true
+    EXPECT_TRUE(conf.getEchoClientId());
+
+    // Check that it can be modified to false
+    conf.setEchoClientId(false);
+    EXPECT_FALSE(conf.getEchoClientId());
+
+    // Check that the default value can be restored
+    conf.setEchoClientId(true);
+    EXPECT_TRUE(conf.getEchoClientId());
+
+    // Check the other constructor has the same default
+    SrvConfig conf1(1);
+    EXPECT_TRUE(conf1.getEchoClientId());
+}
+
 // This test checks if entire configuration can be copied and that the sequence
 // number is not affected.
 TEST_F(SrvConfigTest, copy) {