From: Francis Dupont Date: Sat, 11 Feb 2017 09:29:40 +0000 (+0100) Subject: [3590] Removed unused isDuplicate and move echo client-id from CfgMgr to SrvConfig X-Git-Tag: trac5137_base~13^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=363690d6df42fa12979665c317a00bbfdeef6097;p=thirdparty%2Fkea.git [3590] Removed unused isDuplicate and move echo client-id from CfgMgr to SrvConfig --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index f13de734aa..adf1fcc6e7 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/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); diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 7e49264c03..865f86a179 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/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 = diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 243936bdd9..cbb7c9bbab 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/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 diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 0c86a115ea..6c5c25265c 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/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 diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index 0d4175a1a6..7924125962 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/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); diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 6cf5801f7f..0aaf14da14 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -68,18 +67,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 +198,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 diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index aff077d2c6..d76015b0b1 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -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 @@ -71,9 +67,6 @@ public: /// 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 class CfgMgr : public boost::noncopyable { public: @@ -130,22 +123,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 +294,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,13 +303,6 @@ 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_; @@ -350,9 +312,6 @@ private: /// @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_; diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index cb69f7d30f..15bfd03d31 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/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()) { } diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index dcf9104984..4bfa4ccae3 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/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, diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index a6de68d7dd..59082dc2cb 100644 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/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 @@ -383,23 +383,6 @@ TEST_F(CfgMgrTest, optionSpace6) { /// @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 diff --git a/src/lib/dhcpsrv/tests/srv_config_unittest.cc b/src/lib/dhcpsrv/tests/srv_config_unittest.cc index 86f48b4357..3a87083675 100644 --- a/src/lib/dhcpsrv/tests/srv_config_unittest.cc +++ b/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) {