Commit 7edd611c authored by Tomek Mrugalski's avatar Tomek Mrugalski 🛰
Browse files

[5425a] Changes after review:

 - it is now possible to define multiple classes for a pool
 - small doc corrections
parent eb9efffc
......@@ -109,7 +109,7 @@
// This one is for VoIP devices only.
{
"pool": "192.0.4.1 - 192.0.4.200",
"client-class": "VoIP"
"client-classes": [ "VoIP" ]
},
// This one doesn't have any client-class specified,
......
......@@ -82,7 +82,7 @@
"pools": [
{
"pool": "2001:db8:3::/80",
"client-class": "cable-modems"
"client-classes": [ "cable-modems" ]
} ],
"subnet": "2001:db8:4::/64",
"interface": "ethY"
......
......@@ -805,8 +805,12 @@ concatenation of the strings</entry></row>
<title>Configuring Pools With Class Information</title>
<para>
Similar to the subnets, it is possible to restrict access to the certain address
or prefix pools to the clients belonging to a specific class, using
the "client-class" parameter when defining the pool.
or prefix pools to the clients belonging to a specific class or
classes, using the "client-classes" parameter when defining the
pool. As opposed to subnets, the pools scope allows to define
more than one class that is allowed. The incoming packet has to
belong to only one of the classes listed to be able to get an
address from that pool.
</para>
<para>
......@@ -841,7 +845,7 @@ concatenation of the strings</entry></row>
"pools": [
{
"pool": "192.0.2.10 - 192.0.2.20",
"client-class": "Client_foo"
"client-classes": [ "Client_foo" ]
}
]</userinput>
},
......@@ -880,7 +884,7 @@ concatenation of the strings</entry></row>
"pools": [
{
"pool": "2001:db8:1::-2001:db8:1::ffff",
"client-class": "Client_foo"
"client-classes": [ "Client_foo" ]
}
]</userinput>
},
......
......@@ -1961,7 +1961,7 @@ should include options from the isc option space:
<para>
Client classification can also be used to restrict access to specific
pools within a subnet. This is useful when to segregate clients belonging
to the same subnet into different address ranges.
to the same subnet into different address or prefix ranges.
</para>
<para>
......
......@@ -813,6 +813,7 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence}
\"client-classes\" {
switch(driver.ctx_) {
case isc::dhcp::Parser4Context::DHCP4:
case isc::dhcp::Parser4Context::POOLS:
case isc::dhcp::Parser4Context::RESERVATIONS:
return isc::dhcp::Dhcp4Parser::make_CLIENT_CLASSES(driver.loc_);
default:
......@@ -823,7 +824,6 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence}
\"client-class\" {
switch(driver.ctx_) {
case isc::dhcp::Parser4Context::SUBNET4:
case isc::dhcp::Parser4Context::POOLS:
case isc::dhcp::Parser4Context::SHARED_NETWORK:
case isc::dhcp::Parser4Context::CLIENT_CLASSES:
return isc::dhcp::Dhcp4Parser::make_CLIENT_CLASS(driver.loc_);
......
......@@ -1344,7 +1344,7 @@ pool_params: pool_param
pool_param: pool_entry
| option_data_list
| client_class
| client_class_names_list
| user_context
| comment
| unknown_map_entry
......@@ -1459,7 +1459,7 @@ not_empty_reservation_params: reservation_param
/// @todo probably need to add mac-address as well here
reservation_param: duid
| reservation_client_classes
| client_class_names_list
| client_id_value
| circuit_id_value
| flex_id_value
......@@ -1555,7 +1555,7 @@ hostname: HOSTNAME {
ctx.leave();
};
reservation_client_classes: CLIENT_CLASSES {
client_class_names_list: CLIENT_CLASSES {
ElementPtr c(new ListElement(ctx.loc2pos(@1)));
ctx.stack_.back()->set("client-classes", c);
ctx.stack_.push_back(c);
......
......@@ -4168,15 +4168,15 @@ TEST_F(Dhcp4ParserTest, classifyPools) {
"\"subnet4\": [ { "
" \"pools\": [ { "
" \"pool\": \"192.0.2.1 - 192.0.2.100\", "
" \"client-class\": \"alpha\" "
" \"client-classes\": [ \"alpha\" ] "
" },"
" {"
" \"pool\": \"192.0.3.101 - 192.0.3.150\", "
" \"client-class\": \"beta\" "
" \"client-classes\": [ \"beta\" ] "
" },"
" {"
" \"pool\": \"192.0.4.101 - 192.0.4.150\", "
" \"client-class\": \"gamma\" "
" \"client-classes\": [ \"gamma\", \"alpha\" ] "
" },"
" {"
" \"pool\": \"192.0.5.101 - 192.0.5.150\" "
......@@ -4200,13 +4200,13 @@ TEST_F(Dhcp4ParserTest, classifyPools) {
ASSERT_EQ(4, pools.size()); // We expect 4 pools
// Let's check if client belonging to alpha class is supported in pool[0]
// and not supported in any other pool (except pool[3], which allows
// everyone).
// and pool[2] (which allows 2 classes) and not supported in any other
// pool (except pool[3], which allows everyone).
ClientClasses classes;
classes.insert("alpha");
EXPECT_TRUE(pools.at(0)->clientSupported(classes));
EXPECT_FALSE(pools.at(1)->clientSupported(classes));
EXPECT_FALSE(pools.at(2)->clientSupported(classes));
EXPECT_TRUE(pools.at(2)->clientSupported(classes));
EXPECT_TRUE(pools.at(3)->clientSupported(classes));
// Let's check if client belonging to beta class is supported in pool[1]
......
......@@ -2337,9 +2337,9 @@ TEST_F(Dhcpv4SrvTest, clientPoolClassify) {
"\"subnet4\": [ "
"{ \"pools\": [ { "
" \"pool\": \"192.0.2.1 - 192.0.2.100\", "
" \"client-class\": \"foo\" }, "
" \"client-classes\": [ \"foo\" ] }, "
" { \"pool\": \"192.0.3.1 - 192.0.3.100\", "
" \"client-class\": \"xyzzy\" } ], "
" \"client-classes\": [ \"xyzzy\" ] } ], "
" \"subnet\": \"192.0.0.0/16\" } "
"],"
"\"valid-lifetime\": 4000 }";
......
......@@ -1245,15 +1245,15 @@ const char* EXTRACTED_CONFIGS[] = {
" {\n"
" \"pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"pool\": \"192.0.2.1 - 192.0.2.100\"\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"pool\": \"192.0.3.101 - 192.0.3.150\"\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"gamma\", \"alpha\" ],\n"
" \"pool\": \"192.0.4.101 - 192.0.4.150\"\n"
" },\n"
" {\n"
......@@ -5267,17 +5267,17 @@ const char* UNPARSED_CONFIGS[] = {
" \"option-data\": [ ],\n"
" \"pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"192.0.2.1-192.0.2.100\"\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"192.0.3.101-192.0.3.150\"\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"alpha\", \"gamma\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"192.0.4.101-192.0.4.150\"\n"
" },\n"
......
......@@ -891,7 +891,7 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"192.0.2.1 - 192.0.2.63\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"192.0.2.100 - 192.0.2.100\""
......@@ -931,11 +931,11 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"192.0.2.1 - 192.0.2.63\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"192.0.2.100 - 192.0.2.100\","
" \"client-class\": \"b-devices\""
" \"client-classes\": [ \"b-devices\" ]"
" }"
" ]"
" }"
......@@ -969,7 +969,7 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"192.0.2.1 - 192.0.2.63\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"192.0.2.100 - 192.0.2.100\""
......@@ -1004,11 +1004,11 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"192.0.2.1 - 192.0.2.63\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"192.0.2.100 - 192.0.2.100\","
" \"client-class\": \"b-devices\""
" \"client-classes\": [ \"b-devices\" ]"
" }"
" ]"
" }"
......
......@@ -1068,6 +1068,8 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence}
\"client-classes\" {
switch(driver.ctx_) {
case isc::dhcp::Parser6Context::DHCP6:
case isc::dhcp::Parser6Context::POOLS:
case isc::dhcp::Parser6Context::PD_POOLS:
case isc::dhcp::Parser6Context::RESERVATIONS:
return isc::dhcp::Dhcp6Parser::make_CLIENT_CLASSES(driver.loc_);
default:
......@@ -1078,8 +1080,6 @@ ControlCharacterFill [^"\\]|\\{JSONEscapeSequence}
\"client-class\" {
switch(driver.ctx_) {
case isc::dhcp::Parser6Context::SUBNET6:
case isc::dhcp::Parser6Context::POOLS:
case isc::dhcp::Parser6Context::PD_POOLS:
case isc::dhcp::Parser6Context::CLIENT_CLASSES:
case isc::dhcp::Parser6Context::SHARED_NETWORK:
return isc::dhcp::Dhcp6Parser::make_CLIENT_CLASS(driver.loc_);
......
......@@ -1307,7 +1307,7 @@ pool_params: pool_param
pool_param: pool_entry
| option_data_list
| client_class
| client_class_names_list
| user_context
| comment
| unknown_map_entry
......@@ -1428,7 +1428,7 @@ pd_pool_param: pd_prefix
| pd_prefix_len
| pd_delegated_len
| option_data_list
| client_class
| client_class_names_list
| excluded_prefix
| excluded_prefix_len
| user_context
......@@ -1516,7 +1516,7 @@ not_empty_reservation_params: reservation_param
/// @todo probably need to add mac-address as well here
reservation_param: duid
| reservation_client_classes
| client_class_names_list
| ip_addresses
| prefixes
| hw_address
......@@ -1580,7 +1580,7 @@ flex_id_value: FLEX_ID {
ctx.leave();
};
reservation_client_classes: CLIENT_CLASSES {
client_class_names_list: CLIENT_CLASSES {
ElementPtr c(new ListElement(ctx.loc2pos(@1)));
ctx.stack_.back()->set("client-classes", c);
ctx.stack_.push_back(c);
......@@ -1664,6 +1664,7 @@ client_class_test: TEST {
ctx.leave();
};
// --- end of client classes ---------------------------------
// --- server-id ---------------------------------------------
......
......@@ -645,11 +645,11 @@ TEST_F(ClassifyTest, clientClassifyPool) {
" { \"pools\": [ "
" { "
" \"pool\": \"2001:db8:1::/64\", "
" \"client-class\": \"foo\" "
" \"client-classes\": [ \"foo\" ] "
" }, "
" { "
" \"pool\": \"2001:db8:2::/64\", "
" \"client-class\": \"xyzzy\" "
" \"client-classes\": [ \"xyzzy\" ] "
" } "
" ], "
" \"subnet\": \"2001:db8:2::/40\" "
......
......@@ -4222,15 +4222,15 @@ TEST_F(Dhcp6ParserTest, classifyPools) {
"\"subnet6\": [ { "
" \"pools\": [ { "
" \"pool\": \"2001:db8:1::/80\", "
" \"client-class\": \"alpha\" "
" \"client-classes\": [ \"alpha\" ] "
" },"
" {"
" \"pool\": \"2001:db8:2::/80\", "
" \"client-class\": \"beta\" "
" \"client-classes\": [ \"beta\" ] "
" },"
" {"
" \"pool\": \"2001:db8:3::/80\", "
" \"client-class\": \"gamma\" "
" \"client-classes\": [ \"gamma\" ] "
" },"
" {"
" \"pool\": \"2001:db8:4::/80\" "
......@@ -4313,20 +4313,20 @@ TEST_F(Dhcp6ParserTest, classifyPdPools) {
" \"pd-pools\": [ { "
" \"prefix-len\": 48, "
" \"delegated-len\": 64, "
" \"prefix\": \"2001:db8:1::\", "
" \"client-class\": \"alpha\" "
" \"prefix\": \"2001:db8:1::\", \n"
" \"client-classes\": [ \"alpha\" ]\n "
" },"
" {"
" \"prefix-len\": 48, "
" \"delegated-len\": 64, "
" \"prefix\": \"2001:db8:2::\", "
" \"client-class\": \"beta\" "
" \"client-classes\": [ \"beta\" ]\n "
" },"
" {"
" \"prefix-len\": 48, "
" \"delegated-len\": 64, "
" \"prefix\": \"2001:db8:3::\", "
" \"client-class\": \"gamma\" "
" \"client-classes\": [ \"gamma\" ]\n "
" },"
" {"
" \"prefix-len\": 48, "
......
......@@ -1028,15 +1028,15 @@ const char* EXTRACTED_CONFIGS[] = {
" {\n"
" \"pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"pool\": \"2001:db8:1::/80\"\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"pool\": \"2001:db8:2::/80\"\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"gamma\" ],\n"
" \"pool\": \"2001:db8:3::/80\"\n"
" },\n"
" {\n"
......@@ -1061,19 +1061,19 @@ const char* EXTRACTED_CONFIGS[] = {
" {\n"
" \"pd-pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"delegated-len\": 64,\n"
" \"prefix\": \"2001:db8:1::\",\n"
" \"prefix-len\": 48\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"delegated-len\": 64,\n"
" \"prefix\": \"2001:db8:2::\",\n"
" \"prefix-len\": 48\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"gamma\" ],\n"
" \"delegated-len\": 64,\n"
" \"prefix\": \"2001:db8:3::\",\n"
" \"prefix-len\": 48\n"
......@@ -4595,17 +4595,17 @@ const char* UNPARSED_CONFIGS[] = {
" \"pd-pools\": [ ],\n"
" \"pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"2001:db8:1::/80\"\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"2001:db8:2::/80\"\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"gamma\" ],\n"
" \"option-data\": [ ],\n"
" \"pool\": \"2001:db8:3::/80\"\n"
" },\n"
......@@ -4684,21 +4684,21 @@ const char* UNPARSED_CONFIGS[] = {
" \"option-data\": [ ],\n"
" \"pd-pools\": [\n"
" {\n"
" \"client-class\": \"alpha\",\n"
" \"client-classes\": [ \"alpha\" ],\n"
" \"delegated-len\": 64,\n"
" \"option-data\": [ ],\n"
" \"prefix\": \"2001:db8:1::\",\n"
" \"prefix-len\": 48\n"
" },\n"
" {\n"
" \"client-class\": \"beta\",\n"
" \"client-classes\": [ \"beta\" ],\n"
" \"delegated-len\": 64,\n"
" \"option-data\": [ ],\n"
" \"prefix\": \"2001:db8:2::\",\n"
" \"prefix-len\": 48\n"
" },\n"
" {\n"
" \"client-class\": \"gamma\",\n"
" \"client-classes\": [ \"gamma\" ],\n"
" \"delegated-len\": 64,\n"
" \"option-data\": [ ],\n"
" \"prefix\": \"2001:db8:3::\",\n"
......
......@@ -984,7 +984,7 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"2001:db8:1::20 - 2001:db8:1::20\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"2001:db8:1::50 - 2001:db8:1::50\""
......@@ -1021,11 +1021,11 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"2001:db8:1::20 - 2001:db8:1::20\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"2001:db8:1::50 - 2001:db8:1::50\","
" \"client-class\": \"b-devices\""
" \"client-classes\": [ \"b-devices\" ]"
" }"
" ]"
" }"
......@@ -1055,7 +1055,7 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"2001:db8:1::20 - 2001:db8:1::20\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"2001:db8:1::50 - 2001:db8:1::50\""
......@@ -1089,11 +1089,11 @@ const char* NETWORKS_CONFIG[] = {
" \"pools\": ["
" {"
" \"pool\": \"2001:db8:1::20 - 2001:db8:1::20\","
" \"client-class\": \"a-devices\""
" \"client-classes\": [ \"a-devices\" ]"
" },"
" {"
" \"pool\": \"2001:db8:1::50 - 2001:db8:1::50\","
" \"client-class\": \"b-devices\""
" \"client-classes\": [ \"b-devices\" ]"
" }"
" ]"
" }"
......
......@@ -83,7 +83,6 @@ protected:
///
/// @param subnet next address will be returned from pool of that subnet
/// @param client_classes list of classes client belongs to
/// @param duid Client's DUID
/// @param hint client's hint
///
......
......@@ -379,11 +379,13 @@ PoolParser::parse(PoolStoragePtr pools,
}
// Client-class.
ConstElementPtr client_class = pool_structure->get("client-class");
if (client_class) {
string cclass = client_class->stringValue();
if (!cclass.empty()) {
pool->allowClientClass(cclass);
ConstElementPtr class_list = pool_structure->get("client-classes");
if (class_list) {
BOOST_FOREACH(ConstElementPtr c, class_list->listValue()) {
string cclass = c->stringValue();
if (!cclass.empty()) {
pool->allowClientClass(cclass);
}
}
}
}
......@@ -855,7 +857,7 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) {
user_context_ = user_context;
}
ConstElementPtr client_class = pd_pool_->get("client-class");
ConstElementPtr client_class = pd_pool_->get("client-classes");
if (client_class) {
client_class_ = client_class;
}
......@@ -884,10 +886,13 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_) {
}
// If present, this is a list of class names
if (client_class_) {
string cclass = client_class_->stringValue();
if (!cclass.empty()) {
pool_->allowClientClass(cclass);
BOOST_FOREACH(ConstElementPtr c, client_class_->listValue()) {
string cclass = c->stringValue();
if (!cclass.empty()) {
pool_->allowClientClass(cclass);
}
}
}
......
......@@ -651,8 +651,14 @@ private:
/// A storage for pool specific option values.
CfgOptionPtr options_;
/// @brief User context (optional, may be null)
///
/// User context is arbitrary user data, to be used by hooks.
isc::data::ConstElementPtr user_context_;
/// @brief List of client classes a client has to be belong to to use this pd-pool
///
/// If empty, everyone is allowed. This is a white-list
isc::data::ConstElementPtr client_class_;
};
......
......@@ -111,11 +111,12 @@ Pool::toElement() const {
// Set client-class
const ClientClasses& cclasses = getClientClasses();
if (cclasses.size() > 1) {
isc_throw(ToElementError, "client-class has too many items: "
<< cclasses.size());
} else if (!cclasses.empty()) {
map->set("client-class", Element::create(*cclasses.cbegin()));
if (!cclasses.empty()) {
ElementPtr list = Element::createList();
for (auto c : cclasses) {
list->add(Element::create(c));
}
map->set("client-classes", list);
}
return (map);
......@@ -340,6 +341,7 @@ Pool6::toElement() const {
isc_throw(ToElementError, "invalid prefix range "
<< prefix.toText() << "-" << last.toText());
}
map->set("prefix-len", Element::create(prefix_len));
// Set delegated-len
uint8_t len = getLength();
......@@ -354,10 +356,13 @@ Pool6::toElement() const {
uint8_t xlen = xopt->getExcludedPrefixLength();
map->set("excluded-prefix-len",
Element::create(static_cast<int>(xlen)));
} else {
map->set("excluded-prefix", Element::create(std::string("::")));
map->set("excluded-prefix-len", Element::create(0));
}
// Let's not insert empty