From: Marcin Siodelski Date: Wed, 13 May 2015 17:41:48 +0000 (+0200) Subject: [3747] Renamed parameter record-client-id to match-client-id. X-Git-Tag: trac3867_base~2^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d21a9b2bd62f87fd2a20f22591db55ed23651180;p=thirdparty%2Fkea.git [3747] Renamed parameter record-client-id to match-client-id. Also fixed a couple of failing unit tests. --- diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 6dd86dcf12..0d8ec185bb 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -74,7 +74,7 @@ "item_default": true }, - { "item_name": "record-client-id", + { "item_name": "match-client-id", "item_type": "boolean", "item_optional": true, "item_default": true @@ -275,7 +275,7 @@ "item_default": "0.0.0.0" }, - { "item_name": "record-client-id", + { "item_name": "match-client-id", "item_type": "boolean", "item_optional": true, "item_default": true diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 25054d13b0..df45e3ac98 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -108,11 +108,11 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, // Hardware address. context_->hwaddr_ = query->getHWAddr(); - // Set client identifier if the record-client-id flag is enabled (default). + // Set client identifier if the match-client-id flag is enabled (default). // If the subnet wasn't found it doesn't matter because we will not be // able to allocate a lease anyway so this context will not be used. if (subnet) { - if (subnet->getRecordClientId()) { + if (subnet->getMatchClientId()) { OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER); if (opt_clientid) { context_->clientid_.reset(new ClientId(opt_clientid->getData())); @@ -1135,7 +1135,7 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { // we don't know this client. /// @todo The client_id logged here should be the client_id from the message /// rather than effective client id which is null when the - /// record-client-id is set to false. This is addressed by the #3806 + /// match-client-id is set to false. This is addressed by the #3806 /// which is under review. if (!lease || !lease->belongsToClient(hwaddr, client_id)) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, @@ -1578,7 +1578,7 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) { /// sanityCheck(release, MANDATORY); // Try to find client-id. Note that for the DHCPRELEASE we don't check if the - // record-client-id configuration parameter is disabled because this parameter + // match-client-id configuration parameter is disabled because this parameter // is configured for subnets and we don't select subnet for the DHCPRELEASE. // Bogus clients usually generate new client identifiers when they first // connect to the network, so whatever client identifier has been used to diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index c52489112f..a15d9524b8 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -194,7 +194,7 @@ protected: parser = new RelayInfoParser(config_id, relay_info_, Option::V4); } else if (config_id.compare("option-data") == 0) { parser = new OptionDataListParser(config_id, options_, AF_INET); - } else if (config_id.compare("record-client-id") == 0) { + } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, boolean_values_); } else { isc_throw(NotImplemented, "unsupported parameter: " << config_id); @@ -252,26 +252,26 @@ protected: Subnet4Ptr subnet4(new Subnet4(addr, len, t1, t2, valid, subnet_id)); subnet_ = subnet4; - // record-client-id - isc::util::OptionalValue record_client_id; + // match-client-id + isc::util::OptionalValue match_client_id; try { - record_client_id = boolean_values_->getParam("record-client-id"); + match_client_id = boolean_values_->getParam("match-client-id"); } catch (...) { // Ignore because this parameter is optional and it may be specified // in the global scope. } - // If the record-client-id wasn't specified as a subnet specific parameter + // If the match-client-id wasn't specified as a subnet specific parameter // check if there is global value specified. - if (!record_client_id.isSpecified()) { + if (!match_client_id.isSpecified()) { // If not specified, use false. - record_client_id.specify(globalContext()->boolean_values_-> - getOptionalParam("record-client-id", true)); + match_client_id.specify(globalContext()->boolean_values_-> + getOptionalParam("match-client-id", true)); } - // Set the record-client-id value for the subnet. - subnet4->setRecordClientId(record_client_id.get()); + // Set the match-client-id value for the subnet. + subnet4->setMatchClientId(match_client_id.get()); // next-server try { @@ -397,7 +397,7 @@ namespace dhcp { parser = new BooleanParser(config_id, globalContext()->boolean_values_); } else if (config_id.compare("dhcp-ddns") == 0) { parser = new D2ClientConfigParser(config_id); - } else if (config_id.compare("record-client-id") == 0) { + } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, globalContext()->boolean_values_); } else { isc_throw(DhcpConfigError, @@ -433,6 +433,9 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START).arg(config_set->str()); + // Reset global context. + globalContext().reset(new ParserContext(Option::V4)); + // Before starting any subnet operations, let's reset the subnet-id counter, // so newly recreated configuration starts with first subnet-id equal 1. Subnet::resetSubnetID(); diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 96dd7346d3..5cdaf98612 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1102,9 +1102,9 @@ TEST_F(Dhcp4ParserTest, echoClientId) { CfgMgr::instance().echoClientId(true); } -// This test checks that the global record-client-id parameter is optional +// This test checks that the global match-client-id parameter is optional // and that values under the subnet are used. -TEST_F(Dhcp4ParserTest, recordClientIdNoGlobal) { +TEST_F(Dhcp4ParserTest, matchClientIdNoGlobal) { ConstElementPtr status; std::string config = "{ " + genIfaceConfig() + "," + @@ -1112,12 +1112,12 @@ TEST_F(Dhcp4ParserTest, recordClientIdNoGlobal) { "\"renew-timer\": 1000, " "\"subnet4\": [ " "{" - " \"record-client-id\": true," + " \"match-client-id\": true," " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," " \"subnet\": \"192.0.2.0/24\"" "}," "{" - " \"record-client-id\": false," + " \"match-client-id\": false," " \"pools\": [ { \"pool\": \"192.0.3.1 - 192.0.3.100\" } ]," " \"subnet\": \"192.0.3.0/24\"" "} ]," @@ -1130,26 +1130,26 @@ TEST_F(Dhcp4ParserTest, recordClientIdNoGlobal) { CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); Subnet4Ptr subnet1 = cfg->selectSubnet(IOAddress("192.0.2.1")); ASSERT_TRUE(subnet1); - EXPECT_TRUE(subnet1->getRecordClientId()); + EXPECT_TRUE(subnet1->getMatchClientId()); Subnet4Ptr subnet2 = cfg->selectSubnet(IOAddress("192.0.3.1")); ASSERT_TRUE(subnet2); - EXPECT_FALSE(subnet2->getRecordClientId()); + EXPECT_FALSE(subnet2->getMatchClientId()); } -// This test checks that the global record-client-id parameter is used +// This test checks that the global match-client-id parameter is used // when there is no such parameter under subnet and that the parameter // specified for a subnet overrides the global setting. -TEST_F(Dhcp4ParserTest, recordClientIdGlobal) { +TEST_F(Dhcp4ParserTest, matchClientIdGlobal) { ConstElementPtr status; std::string config = "{ " + genIfaceConfig() + "," + "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " - "\"record-client-id\": true," + "\"match-client-id\": true," "\"subnet4\": [ " "{" - " \"record-client-id\": false," + " \"match-client-id\": false," " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," " \"subnet\": \"192.0.2.0/24\"" "}," @@ -1166,11 +1166,11 @@ TEST_F(Dhcp4ParserTest, recordClientIdGlobal) { CfgSubnets4Ptr cfg = CfgMgr::instance().getStagingCfg()->getCfgSubnets4(); Subnet4Ptr subnet1 = cfg->selectSubnet(IOAddress("192.0.2.1")); ASSERT_TRUE(subnet1); - EXPECT_FALSE(subnet1->getRecordClientId()); + EXPECT_FALSE(subnet1->getMatchClientId()); Subnet4Ptr subnet2 = cfg->selectSubnet(IOAddress("192.0.3.1")); ASSERT_TRUE(subnet2); - EXPECT_TRUE(subnet2->getRecordClientId()); + EXPECT_TRUE(subnet2->getMatchClientId()); } // This test checks if it is possible to override global values @@ -2076,6 +2076,7 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) { // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. config = "{ " + genIfaceConfig() + "," + + "\"valid-lifetime\": 3000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" @@ -2656,6 +2657,7 @@ TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) { // they should be included as sub-options in the 'vendor-opts' // option. config = "{ " + genIfaceConfig() + "," + + "\"valid-lifetime\": 3000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index 2835bbcbd0..d8d10a4eaf 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -59,10 +59,10 @@ namespace { /// aa:bb:cc:dd:ee:ff, reserved address 10.0.0.7 /// /// - Configuration 3: -/// - Use for testing record-client-id flag +/// - Use for testing match-client-id flag /// - 1 subnet: 10.0.0.0/24 /// - 1 pool: 10.0.0.10-10.0.0.100 -/// - record-client-id flag is set to false, thus the server +/// - match-client-id flag is set to false, thus the server /// uses HW address for lease lookup, rather than client id const char* DORA_CONFIGS[] = { // Configuration 0 @@ -166,7 +166,7 @@ const char* DORA_CONFIGS[] = { " \"interfaces\": [ \"*\" ]" "}," "\"valid-lifetime\": 600," - "\"record-client-id\": false," + "\"match-client-id\": false," "\"subnet4\": [ { " " \"subnet\": \"10.0.0.0/24\", " " \"id\": 1," @@ -711,7 +711,7 @@ TEST_F(DORATest, reservation) { ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, clientB.config_.lease_.addr_)); } -// This test checks that setting the record-client-id value to false causes +// This test checks that setting the match-client-id value to false causes // the server to ignore changing client identifier when the client is // using consistent HW address. TEST_F(DORATest, ignoreChangingClientId) { @@ -747,7 +747,7 @@ TEST_F(DORATest, ignoreChangingClientId) { EXPECT_EQ(leased_address, client.config_.lease_.addr_); } -// This test checks that the record-client-id parameter doesn't have +// This test checks that the match-client-id parameter doesn't have // effect on the lease lookup using the HW address. TEST_F(DORATest, changingHWAddress) { Dhcp4Client client(Dhcp4Client::SELECTING); @@ -767,7 +767,7 @@ TEST_F(DORATest, changingHWAddress) { IOAddress leased_address = client.config_.lease_.addr_; // Modify HW address but leave client id in place. The value of the - // record-client-id set to false must not have any effect on the + // match-client-id set to false must not have any effect on the // case when the HW address is changing. In such case the server will // allocate the new address for the client. client.setHWAddress("01:01:01:01:01:01"); diff --git a/src/bin/dhcp4/tests/release_unittest.cc b/src/bin/dhcp4/tests/release_unittest.cc index 2083509466..7547929f1e 100644 --- a/src/bin/dhcp4/tests/release_unittest.cc +++ b/src/bin/dhcp4/tests/release_unittest.cc @@ -56,62 +56,6 @@ const char* RELEASE_CONFIGS[] = { " \"space\": \"dhcp4\"" " } ]" " } ]" - "}", - -// Configuration 1 - "{ \"interfaces-config\": {" - " \"interfaces\": [ \"*\" ]" - "}," - - "\"valid-lifetime\": 600," - "\"subnet4\": [ { " - " \"subnet\": \"192.0.2.0/24\", " - " \"option-data\": [ {" - " \"name\": \"routers\"," - " \"code\": 3," - " \"data\": \"192.0.2.200,192.0.2.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" - " }," - " {" - " \"name\": \"domain-name-servers\"," - " \"code\": 6," - " \"data\": \"192.0.2.202,192.0.2.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" - " }," - " {" - " \"name\": \"log-servers\"," - " \"code\": 7," - " \"data\": \"10.0.0.200,10.0.0.201\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" - " }," - " {" - " \"name\": \"cookie-servers\"," - " \"code\": 8," - " \"data\": \"10.0.0.202,10.0.0.203\"," - " \"csv-format\": true," - " \"space\": \"dhcp4\"" - " } ]" - " } ]" - "}", - -// Configuration 2 - "{ \"interfaces-config\": {" - " \"interfaces\": [ \"*\" ]" - "}," - "\"valid-lifetime\": 600," - "\"subnet4\": [ { " - " \"subnet\": \"10.0.0.0/24\", " - " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," - " \"reservations\": [ " - " {" - " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," - " \"ip-address\": \"10.0.0.7\"" - " }" - " ]" - "} ]" "}" }; @@ -190,6 +134,7 @@ ReleaseTest::successfulRelease(const std::string& hw_address_1, const std::string& client_id_1, const std::string& hw_address_2, const std::string& client_id_2) { + CfgMgr::instance().clear(); Dhcp4Client client(Dhcp4Client::SELECTING); // Configure DHCP server. configure(RELEASE_CONFIGS[0], *client.getServer()); diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 1ca364129d..9e0589f367 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -704,6 +704,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START).arg(config_set->str()); + // Reset global context. + globalContext().reset(new ParserContext(Option::V6)); + // Before starting any subnet operations, let's reset the subnet-id counter, // so newly recreated configuration starts with first subnet-id equal 1. Subnet::resetSubnetID(); diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index f843b160ea..bb0a7bbb8e 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -2238,6 +2238,8 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { // configuration manager sends whole configuration for the lists // where at least one element is being modified or added. config = "{ " + genIfaceConfig() + "," + "\"preferred-lifetime\": 3000," + "\"valid-lifetime\": 4000," "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc index aeff33831b..c5b775b302 100644 --- a/src/lib/dhcpsrv/subnet.cc +++ b/src/lib/dhcpsrv/subnet.cc @@ -180,7 +180,7 @@ Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length, const Triplet& valid_lifetime, const SubnetID id) : Subnet(prefix, length, t1, t2, valid_lifetime, RelayInfo(IOAddress("0.0.0.0")), id), - siaddr_(IOAddress("0.0.0.0")), record_client_id_(true) { + siaddr_(IOAddress("0.0.0.0")), match_client_id_(true) { if (!prefix.isV4()) { isc_throw(BadValue, "Non IPv4 prefix " << prefix.toText() << " specified in subnet4"); diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 14723ba063..4d630cc50e 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -543,20 +543,20 @@ public: isc::asiolink::IOAddress getSiaddr() const; /// @brief Sets the flag indicating if the client identifier should be - /// recorded in the lease database. + /// used to identify the client's lease. /// - /// @param ignore If this value is true, the client identifiers are ignored - /// in the messages from the clients for this subnet. - void setRecordClientId(const bool record) { - record_client_id_ = record; + /// @param match If this value is true, the client identifiers are not + /// used for lease lookup. + void setMatchClientId(const bool match) { + match_client_id_ = match; } /// @brief Returns the flag indicating if the client identifiers should - /// be recorded in the lease database. + /// be used to identify the client's lease. /// - /// @return true if client identifiers should be recorded, false otherwise. - bool getRecordClientId() const { - return (record_client_id_); + /// @return true if client identifiers should be used, false otherwise. + bool getMatchClientId() const { + return (match_client_id_); } private: @@ -578,8 +578,9 @@ private: /// @brief siaddr value for this subnet isc::asiolink::IOAddress siaddr_; - /// @brief Should server record client identifiers. - bool record_client_id_; + /// @brief Should server use client identifiers for client lease + /// lookup. + bool match_client_id_; }; /// @brief A pointer to a @c Subnet4 object diff --git a/src/lib/dhcpsrv/tests/subnet_unittest.cc b/src/lib/dhcpsrv/tests/subnet_unittest.cc index a153209c4a..58377ac396 100644 --- a/src/lib/dhcpsrv/tests/subnet_unittest.cc +++ b/src/lib/dhcpsrv/tests/subnet_unittest.cc @@ -116,20 +116,20 @@ TEST(Subnet4Test, siaddr) { BadValue); } -// Checks if the record-client-id flag can be set and retrieved. -TEST(Subnet4Test, recordClientId) { +// Checks if the match-client-id flag can be set and retrieved. +TEST(Subnet4Test, matchClientId) { Subnet4 subnet(IOAddress("192.0.2.1"), 24, 1000, 2000, 3000); // By default the flag should be set to true. - EXPECT_TRUE(subnet.getRecordClientId()); + EXPECT_TRUE(subnet.getMatchClientId()); // Modify it and retrieve. - subnet.setRecordClientId(false); - EXPECT_FALSE(subnet.getRecordClientId()); + subnet.setMatchClientId(false); + EXPECT_FALSE(subnet.getMatchClientId()); // Modify again. - subnet.setRecordClientId(true); - EXPECT_TRUE(subnet.getRecordClientId()); + subnet.setMatchClientId(true); + EXPECT_TRUE(subnet.getMatchClientId()); } TEST(Subnet4Test, Pool4InSubnet4) { diff --git a/src/lib/util/tests/optional_value_unittest.cc b/src/lib/util/tests/optional_value_unittest.cc index 7d93f76298..0c7f474a86 100644 --- a/src/lib/util/tests/optional_value_unittest.cc +++ b/src/lib/util/tests/optional_value_unittest.cc @@ -47,7 +47,7 @@ TEST(OptionalValueTest, set) { ASSERT_FALSE(value.isSpecified()); // Mark value "specified". The value itself should not change. - value.specify(true); + value.specify(OptionalValueState(true)); ASSERT_EQ(100, value.get()); ASSERT_TRUE(value.isSpecified()); @@ -57,7 +57,7 @@ TEST(OptionalValueTest, set) { ASSERT_TRUE(value.isSpecified()); // Mark it "unspecified". The value should remain the same. - value.specify(false); + value.specify(OptionalValueState(false)); ASSERT_EQ(5, value.get()); ASSERT_FALSE(value.isSpecified()); } @@ -110,7 +110,7 @@ TEST(OptionalValueTest, equalityOperators) { EXPECT_FALSE(value == 123); EXPECT_TRUE(value != 123); - value.specify(true); + value.specify(OptionalValueState(true)); EXPECT_TRUE(value == 10); EXPECT_FALSE(value != 10); @@ -123,7 +123,7 @@ TEST(OptionalValueTest, equalityOperators) { EXPECT_FALSE(value == 10); EXPECT_TRUE(value != 10); - value.specify(false); + value.specify(OptionalValueState(false)); EXPECT_FALSE(value == 123); EXPECT_TRUE(value != 123);