Skip to content

GitLab

Commit d21a9b2b authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[3747] Renamed parameter record-client-id to match-client-id. 

Also fixed a couple of failing unit tests.
parent fa00f7c6
Hide whitespace changes
Inline Side-by-side
Showing with 70 additions and 114 deletions
src/bin/dhcp4/dhcp4.spec
View file @ d21a9b2b
......@@ -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
......
src/bin/dhcp4/dhcp4_srv.cc
View file @ d21a9b2b
......@@ -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
......
src/bin/dhcp4/json_config_parser.cc
View file @ d21a9b2b
......@@ -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<bool> record_client_id;
// match-client-id
isc::util::OptionalValue<bool> 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();
......
src/bin/dhcp4/tests/config_parser_unittest.cc
View file @ d21a9b2b
......@@ -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\": [ {"
......
src/bin/dhcp4/tests/dora_unittest.cc
View file @ d21a9b2b
......@@ -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");
......
src/bin/dhcp4/tests/release_unittest.cc
View file @ d21a9b2b
......@@ -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());
......
src/bin/dhcp6/json_config_parser.cc
View file @ d21a9b2b
......@@ -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();
......
src/bin/dhcp6/tests/config_parser_unittest.cc
View file @ d21a9b2b
......@@ -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\": [ {"
......
src/lib/dhcpsrv/subnet.cc
View file @ d21a9b2b
......@@ -180,7 +180,7 @@ Subnet4::Subnet4(const isc::asiolink::IOAddress& prefix, uint8_t length,
const Triplet<uint32_t>& 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");
......
src/lib/dhcpsrv/subnet.h
View file @ d21a9b2b
......@@ -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
......
src/lib/dhcpsrv/tests/subnet_unittest.cc
View file @ d21a9b2b
......@@ -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) {
......
src/lib/util/tests/optional_value_unittest.cc
View file @ d21a9b2b
......@@ -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);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment