mir/kea
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-30 13:37:55 +00:00
Code Issues Releases Wiki Activity

[#2736] Warn on additional and lifetime params

Browse Source 
Updated the ARM:
/doc/sphinx/arm/dhcp4-srv.rst
/doc/sphinx/arm/dhcp6-srv.rst

Added ChangeLog

/src/lib/dhcpsrv/dhcpsrv_messages.*
    DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES - new message

/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
    ClientClassDefParser::parse() - now emits WARN log

/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
    TEST_F(ClientClassDefParserTest, addtionalWithLifetimes4)
    TEST_F(ClientClassDefParserTest, addtionalWithLifetimes6)
    - new tests
This commit is contained in:
Thomas Markwalder
2024-11-12 12:06:56 -05:00
parent 9af00a7533
commit 34e88d7312
8 changed files with 193 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
ChangeLog
View File

@@ -1,3 +1,11 @@
2304. [func] tmark
Both kea-dhcp4 and kea-dhcp6 will now emit a warning
log message when classes are configured with both
``only-in-additional-list`` true and parameter(s)
that normally impact lease lifetimes (e.g. 'valid-
lifetime', 'preferred-lifetime`).
(Gitlab #2736)
2303. [bug] tmark
Modified both kea-dhcp4 and kea-dhcp6 to avoid
generating DDNS update requests when leases are

6
doc/sphinx/arm/dhcp4-srv.rst
View File

@@ -3483,6 +3483,12 @@ Since Kea version 2.7.4 additional classes configured without
a test expression are unconditionally added, i.e. they are considered
to always be evaluated to ``true``.
.. note::
Because additional evaluation occurs after lease assignment, parameters
that would otherwise impact lease life times (e.g. ``valid-lifetime``,
``offer-lifetime``) will have no effect when specified in a class that
also sets ``only-in-additional-list`` true.
.. note::
As of Kea version 2.7.4, ``only-if-required`` and ``require-client-classes``

6
doc/sphinx/arm/dhcp6-srv.rst
View File

@@ -3219,6 +3219,12 @@ Since Kea version 2.7.4 additional classes configured without
a test expression are unconditionally added, i.e. they are considered
to always be evaluated to ``true``.
.. note::
Because additional evaluation occurs after lease assignment, parameters
that would otherwise impact lease life times (e.g. ``valid-lifetime``,
``preferred-lifetime``) will have no effect when specified in a class that
also sets ``only-in-additional-list`` true.
.. note::
As of Kea version 2.7.4, ``only-if-required`` and ``require-client-classes``

2
src/lib/dhcpsrv/dhcpsrv_messages.cc
View File

@@ -46,6 +46,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_UPDATE_SUBNET6 = "DHCPSRV_CFGMGR
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS = "DHCPSRV_CFGMGR_USE_ADDRESS";
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR = "DHCPSRV_CFGMGR_USE_ALLOCATOR";
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST = "DHCPSRV_CFGMGR_USE_UNICAST";
extern const isc::log::MessageID DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES = "DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES";
extern const isc::log::MessageID DHCPSRV_CLOSE_DB = "DHCPSRV_CLOSE_DB";
extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL = "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL";
extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET = "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET";
@@ -215,6 +216,7 @@ const char* values[] = {
"DHCPSRV_CFGMGR_USE_ADDRESS", "listening on address %1, on interface %2",
"DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3",
"DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2",
"DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES", "class: %1 has 'only-in-additional-list' true while specifying one or more lease life time values. Life time values will be ignored.",
"DHCPSRV_CLOSE_DB", "closing currently open %1 database",
"DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL", "ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it",
"DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET", "received bad DHCPv4o6 packet: %1",

1
src/lib/dhcpsrv/dhcpsrv_messages.h
View File

@@ -47,6 +47,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_UPDATE_SUBNET6;
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS;
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR;
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST;
extern const isc::log::MessageID DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES;
extern const isc::log::MessageID DHCPSRV_CLOSE_DB;
extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL;
extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET;

9
src/lib/dhcpsrv/dhcpsrv_messages.mes
View File

@@ -957,3 +957,12 @@ included in the message.
% DHCPSRV_UNKNOWN_DB unknown database type: %1
The database access string specified a database type (given in the
message) that is unknown to the software. This is a configuration error.
% DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES class: %1 has 'only-in-additional-list' true while specifying one or more lease life time values. Life time values will be ignored.
This warning is emitted whenever a class is configured with
'only-in-addition-list' true as well as specifying one or
more lease life time parameters (e.g. 'valid-lifetime',
'preferred-lifetime', or 'offer-lifetime'). Additional list classes
are evaluated after lease assignment, thus parameters that would otherwise
impact lease life times will have no affect.

8
src/lib/dhcpsrv/parsers/client_class_def_parser.cc
View File

@@ -292,6 +292,14 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
// depend_on_known is now allowed
}
if (additional &&
(!valid_lft.unspecified() ||
!preferred_lft.unspecified() ||
!offer_lft.unspecified())) {
LOG_WARN(dhcpsrv_logger, DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES)
.arg(name);
}
// Add the client class definition
try {
class_dictionary->addClass(name, match_expr, test, additional,

153
src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
View File

@@ -16,6 +16,7 @@
#include <asiolink/io_address.h>
#include <eval/evaluate.h>
#include <testutils/gtest_utils.h>
#include <testutils/log_utils.h>
#include <gtest/gtest.h>
#include <sstream>
#include <stdint.h>
@@ -28,6 +29,7 @@ using namespace isc::data;
using namespace isc::dhcp;
using namespace isc::asiolink;
using namespace isc::util;
using namespace isc::dhcp::test;
namespace {
@@ -101,7 +103,8 @@ protected:
};
/// @brief Test fixture class for @c ClientClassDefParser.
class ClientClassDefParserTest : public ::testing::Test {
//class ClientClassDefParserTest : public ::testing::Test {
class ClientClassDefParserTest : public LogContentTest {
protected:
/// @brief Convenience method for parsing a configuration
@@ -2192,4 +2195,152 @@ TEST_F(ClientClassDefParserTest, deprecatedOnlyIfRequired) {
"'only-in-additional-list'. Use only the latter.");
}
// Verifies that the parser detects use of life time parameters
// in classes that also set `only-in-additinal-list' true.
TEST_F(ClientClassDefParserTest, addtionalWithLifetimes4) {
CfgMgr::instance().setFamily(AF_INET);
boost::scoped_ptr<ClientClassDef> cclass;
struct Scenario {
size_t line_no_;
std::string cfg_;
bool should_log_;
};
std::list<Scenario> scenarios = {
{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true
})",
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true,
"valid-lifetime": 100
})",
true
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true,
"offer-lifetime": 100
})",
true
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": false,
"valid-lifetime": 100
})",
false
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": false,
"offer-lifetime": 100
})",
false
}
};
size_t exp_log_count = 0;
for (auto& scenario : scenarios) {
std::stringstream oss;
oss << "scenario at: " << scenario.line_no_;
SCOPED_TRACE(oss.str());
// Parse the class definition.
auto class_def = parseClientClassDef(scenario.cfg_, AF_INET);
ASSERT_TRUE(class_def);
// If we expect the warning log to be emitted the occurrences
// in the log file should bump by 1.
if (scenario.should_log_) {
// Veriy we emitted another instance of the log message.
++exp_log_count;
ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"),
exp_log_count);
}
}
}
// Verifies that the parser detects use of life time parameters
// in classes that also set `only-in-additinal-list' true.
TEST_F(ClientClassDefParserTest, addtionalWithLifetimes6) {
CfgMgr::instance().setFamily(AF_INET6);
boost::scoped_ptr<ClientClassDef> cclass;
struct Scenario {
size_t line_no_;
std::string cfg_;
bool should_log_;
};
std::list<Scenario> scenarios = {
{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true
})",
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true,
"valid-lifetime": 100
})",
true
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": true,
"preferred-lifetime": 100
})",
true
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": false,
"valid-lifetime": 100
})",
false
},{
__LINE__,
R"({
"name": "boo",
"only-in-additional-list": false,
"preferred-lifetime": 100
})",
false
}
};
size_t exp_log_count = 0;
for (auto& scenario : scenarios) {
std::stringstream oss;
oss << "scenario at: " << scenario.line_no_;
SCOPED_TRACE(oss.str());
// Parse the class definition.
auto class_def = parseClientClassDef(scenario.cfg_, AF_INET6);
ASSERT_TRUE(class_def);
// If we expect the warning log to be emitted the occurrences
// in the log file should bump by 1.
if (scenario.should_log_) {
// Veriy we emitted another instance of the log message.
++exp_log_count;
ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"),
exp_log_count);
}
}
}
} // end of anonymous namespace