[#3588] Addressed comments

diff --git a/changelog_unreleased/3588-required-no-test b/changelog_unreleased/3588-required-no-test
index 0f2b9e0f1964919485a3ca04b1e717bc91888908..f978c8fd29ee5606d04ebd53bd45622e7d610ea2 100644 (file)
--- a/changelog_unreleased/3588-required-no-test
+++ b/changelog_unreleased/3588-required-no-test
@@ -1,6 +1,6 @@
 [func]*                fdupont
-       Modified the behavior of required client classes
-       configured without a test expression: they are now
-       unconditionally added as they always evaluate to
-       true (vs false previously).
+       Classes included in 'require-client-classes' that do
+       not have test expressions will now be unconditionally
+       added to the client's list of matching classes.
+       Previously they were ignored.
        (Gitlab #3388)

diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc
index 81d191c9254af1e0d9ce4bf4376e0171c9311dd1..6a77966f171f118fb258103bdc1ab9c67e784510 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.cc
+++ b/src/bin/dhcp4/dhcp4_messages.cc
@@ -148,8 +148,8 @@ extern const isc::log::MessageID DHCP4_RELEASE_FAIL_WRONG_CLIENT = "DHCP4_RELEAS
 extern const isc::log::MessageID DHCP4_REQUEST = "DHCP4_REQUEST";
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_EVAL_ERROR = "DHCP4_REQUIRED_CLASS_EVAL_ERROR";
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_EVAL_RESULT = "DHCP4_REQUIRED_CLASS_EVAL_RESULT";
+extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_NO_TEST = "DHCP4_REQUIRED_CLASS_NO_TEST";
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_UNDEFINED = "DHCP4_REQUIRED_CLASS_UNDEFINED";
-extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_UNTESTABLE = "DHCP4_REQUIRED_CLASS_UNTESTABLE";
 extern const isc::log::MessageID DHCP4_RESERVATIONS_LOOKUP_FIRST_ENABLED = "DHCP4_RESERVATIONS_LOOKUP_FIRST_ENABLED";
 extern const isc::log::MessageID DHCP4_RESERVED_HOSTNAME_ASSIGNED = "DHCP4_RESERVED_HOSTNAME_ASSIGNED";
 extern const isc::log::MessageID DHCP4_RESPONSE_DATA = "DHCP4_RESPONSE_DATA";
@@ -326,8 +326,8 @@ const char* values[] = {
     "DHCP4_REQUEST", "%1: server is processing DHCPREQUEST with hint=%2",
     "DHCP4_REQUIRED_CLASS_EVAL_ERROR", "%1: Expression '%2' evaluated to %3",
     "DHCP4_REQUIRED_CLASS_EVAL_RESULT", "%1: Expression '%2' evaluated to %3",
+    "DHCP4_REQUIRED_CLASS_NO_TEST", "required class %1 has no test expression",
     "DHCP4_REQUIRED_CLASS_UNDEFINED", "required class %1 has no definition",
-    "DHCP4_REQUIRED_CLASS_UNTESTABLE", "required class %1 has no test expression",
     "DHCP4_RESERVATIONS_LOOKUP_FIRST_ENABLED", "Multi-threading is enabled and host reservations lookup is always performed first.",
     "DHCP4_RESERVED_HOSTNAME_ASSIGNED", "%1: server assigned reserved hostname %2",
     "DHCP4_RESPONSE_DATA", "%1: responding with packet %2 (type %3), packet details: %4",

diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h
index 00e60df071236635614974b2926e5daef2af8108..56795aec23f362c0a2991ec9515304a7f23fbef8 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.h
+++ b/src/bin/dhcp4/dhcp4_messages.h
@@ -149,8 +149,8 @@ extern const isc::log::MessageID DHCP4_RELEASE_FAIL_WRONG_CLIENT;
 extern const isc::log::MessageID DHCP4_REQUEST;
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_EVAL_ERROR;
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_EVAL_RESULT;
+extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_NO_TEST;
 extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_UNDEFINED;
-extern const isc::log::MessageID DHCP4_REQUIRED_CLASS_UNTESTABLE;
 extern const isc::log::MessageID DHCP4_RESERVATIONS_LOOKUP_FIRST_ENABLED;
 extern const isc::log::MessageID DHCP4_RESERVED_HOSTNAME_ASSIGNED;
 extern const isc::log::MessageID DHCP4_RESPONSE_DATA;

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 41cb5652a4fe74e76222ee51be53a19cbe9ccfe3..abcd017ccb24709cc8fc4fecda0dfb9eca4a799f 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -979,17 +979,17 @@ This debug message indicates that the expression of a required client class has
 been successfully evaluated. The client class name and the result value of the
 evaluation are printed.
 
-% DHCP4_REQUIRED_CLASS_UNDEFINED required class %1 has no definition
-Logged at debug log level 40.
-This debug message informs that a class is listed for required evaluation but
-has no definition. The class is ignored.
-
-% DHCP4_REQUIRED_CLASS_UNTESTABLE required class %1 has no test expression
+% DHCP4_REQUIRED_CLASS_NO_TEST required class %1 has no test expression
 Logged at debug log level 40.
 This debug message informs that a class was listed for required evaluation but
 its definition does not include a test expression to evaluate. The class is
 unconditionally added to the query.
 
+% DHCP4_REQUIRED_CLASS_UNDEFINED required class %1 has no definition
+Logged at debug log level 40.
+This debug message informs that a class is listed for required evaluation but
+has no definition. The class is ignored.
+
 % DHCP4_RESERVATIONS_LOOKUP_FIRST_ENABLED Multi-threading is enabled and host reservations lookup is always performed first.
 This is a message informing that host reservations lookup is performed before
 lease lookup when multi-threading is enabled overwriting configured value.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 7dc53314abc01c791205d41a8725cee126e3fa36..edba3b68b1affb00eec2e42e4fcd63b266b819c4 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -4876,7 +4876,7 @@ void Dhcpv4Srv::requiredClassify(Dhcpv4Exchange& ex) {
         // Add a class without an expression to evaluate
         if (!expr_ptr) {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC,
-                      DHCP4_REQUIRED_CLASS_UNTESTABLE)
+                      DHCP4_REQUIRED_CLASS_NO_TEST)
                 .arg(cclass);
             query->addClass(cclass);
             continue;

diff --git a/src/bin/dhcp4/tests/classify_unittest.cc b/src/bin/dhcp4/tests/classify_unittest.cc
index a0fe8bff1d8fd933c6287dad9611a980cd4b1d2a..d88d99d0268891e68ca06bc5ce261b5a63b599f8 100644 (file)
--- a/src/bin/dhcp4/tests/classify_unittest.cc
+++ b/src/bin/dhcp4/tests/classify_unittest.cc
@@ -1294,7 +1294,9 @@ TEST_F(ClassifyTest, requiredNoTest) {
 }
 
 // This test checks a required class which is not defined is ignored.
-TEST_F(ClassifyTest, requiredNoDefined) {
+// Please set KEA_LOGGER_DESTINATION to stderr or stdout and check
+// that DHCP4_REQUIRED_CLASS_UNDEFINED is logged,
+TEST_F(ClassifyTest, requiredNotDefined) {
     std::string config =
         "{"
         "\"interfaces-config\": {"

diff --git a/src/bin/dhcp6/dhcp6_messages.cc b/src/bin/dhcp6/dhcp6_messages.cc
index af7d80707d585ba5d1aba2e7f140e5bd0e26f018..1b4c5184f89807834458383b6ca0dff661e300f6 100644 (file)
--- a/src/bin/dhcp6/dhcp6_messages.cc
+++ b/src/bin/dhcp6/dhcp6_messages.cc
@@ -147,8 +147,8 @@ extern const isc::log::MessageID DHCP6_RELEASE_PD_FAIL_WRONG_DUID = "DHCP6_RELEA
 extern const isc::log::MessageID DHCP6_RELEASE_PD_FAIL_WRONG_IAID = "DHCP6_RELEASE_PD_FAIL_WRONG_IAID";
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_EVAL_ERROR = "DHCP6_REQUIRED_CLASS_EVAL_ERROR";
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_EVAL_RESULT = "DHCP6_REQUIRED_CLASS_EVAL_RESULT";
+extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_NO_TEST = "DHCP6_REQUIRED_CLASS_NO_TEST";
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_UNDEFINED = "DHCP6_REQUIRED_CLASS_UNDEFINED";
-extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_UNTESTABLE = "DHCP6_REQUIRED_CLASS_UNTESTABLE";
 extern const isc::log::MessageID DHCP6_REQUIRED_OPTIONS_CHECK_FAIL = "DHCP6_REQUIRED_OPTIONS_CHECK_FAIL";
 extern const isc::log::MessageID DHCP6_RESERVATIONS_LOOKUP_FIRST_ENABLED = "DHCP6_RESERVATIONS_LOOKUP_FIRST_ENABLED";
 extern const isc::log::MessageID DHCP6_RESPONSE_DATA = "DHCP6_RESPONSE_DATA";
@@ -314,8 +314,8 @@ const char* values[] = {
     "DHCP6_RELEASE_PD_FAIL_WRONG_IAID", "%1: client tried to release prefix %2/%3, but it used wrong IAID (expected %4, but got %5)",
     "DHCP6_REQUIRED_CLASS_EVAL_ERROR", "%1: Expression '%2' evaluated to %3",
     "DHCP6_REQUIRED_CLASS_EVAL_RESULT", "%1: Expression '%2' evaluated to %3",
+    "DHCP6_REQUIRED_CLASS_NO_TEST", "required class %1 has no test expression",
     "DHCP6_REQUIRED_CLASS_UNDEFINED", "required class %1 has no definition",
-    "DHCP6_REQUIRED_CLASS_UNTESTABLE", "required class %1 has no test expression",
     "DHCP6_REQUIRED_OPTIONS_CHECK_FAIL", "%1: %2 message received from %3 failed the following check: %4",
     "DHCP6_RESERVATIONS_LOOKUP_FIRST_ENABLED", "Multi-threading is enabled and host reservations lookup is always performed first.",
     "DHCP6_RESPONSE_DATA", "%1: responding with packet %2 (type %3), packet details: %4",

diff --git a/src/bin/dhcp6/dhcp6_messages.h b/src/bin/dhcp6/dhcp6_messages.h
index 2faa9cee776c113114885b7eb496544d7f6cea68..3afab08b5a3a4c47b6ca7f7addc97888b0ba3d17 100644 (file)
--- a/src/bin/dhcp6/dhcp6_messages.h
+++ b/src/bin/dhcp6/dhcp6_messages.h
@@ -148,8 +148,8 @@ extern const isc::log::MessageID DHCP6_RELEASE_PD_FAIL_WRONG_DUID;
 extern const isc::log::MessageID DHCP6_RELEASE_PD_FAIL_WRONG_IAID;
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_EVAL_ERROR;
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_EVAL_RESULT;
+extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_NO_TEST;
 extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_UNDEFINED;
-extern const isc::log::MessageID DHCP6_REQUIRED_CLASS_UNTESTABLE;
 extern const isc::log::MessageID DHCP6_REQUIRED_OPTIONS_CHECK_FAIL;
 extern const isc::log::MessageID DHCP6_RESERVATIONS_LOOKUP_FIRST_ENABLED;
 extern const isc::log::MessageID DHCP6_RESPONSE_DATA;

diff --git a/src/bin/dhcp6/dhcp6_messages.mes b/src/bin/dhcp6/dhcp6_messages.mes
index 6fb9072a7dfc8707180a62db57cbbe18e3d76a2a..5e9f023937b72834dfe399def25d0b212ff7a2dd 100644 (file)
--- a/src/bin/dhcp6/dhcp6_messages.mes
+++ b/src/bin/dhcp6/dhcp6_messages.mes
@@ -984,17 +984,17 @@ This debug message indicates that the expression of a required client class has
 been successfully evaluated. The client class name and the result value of the
 evaluation are printed.
 
-% DHCP6_REQUIRED_CLASS_UNDEFINED required class %1 has no definition
-Logged at debug log level 40.
-This debug message informs that a class is listed for required evaluation but
-has no definition. The class is ignored.
-
-% DHCP6_REQUIRED_CLASS_UNTESTABLE required class %1 has no test expression
+% DHCP6_REQUIRED_CLASS_NO_TEST required class %1 has no test expression
 Logged at debug log level 40.
 This debug message informs that a class was listed for required evaluation but
 its definition does not include a test expression to evaluate. The class is
 unconditionally added to the query.
 
+% DHCP6_REQUIRED_CLASS_UNDEFINED required class %1 has no definition
+Logged at debug log level 40.
+This debug message informs that a class is listed for required evaluation but
+has no definition. The class is ignored.
+
 % DHCP6_REQUIRED_OPTIONS_CHECK_FAIL %1: %2 message received from %3 failed the following check: %4
 Logged at debug log level 40.
 This message indicates that received DHCPv6 packet is invalid.  This may be due

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index ee4841e16e02e862a0f610f40a6f3951f5f9633d..913701795b852cdc1a8e70b725a02a5f62386020 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -4505,7 +4505,7 @@ Dhcpv6Srv::requiredClassify(const Pkt6Ptr& pkt, AllocEngine::ClientContext6& ctx
         // Add a class without an expression to evaluate
         if (!expr_ptr) {
             LOG_DEBUG(dhcp6_logger, DBG_DHCP6_BASIC,
-                      DHCP6_REQUIRED_CLASS_UNTESTABLE)
+                      DHCP6_REQUIRED_CLASS_NO_TEST)
                 .arg(cclass);
             pkt->addClass(cclass);
             continue;

diff --git a/src/bin/dhcp6/tests/classify_unittest.cc b/src/bin/dhcp6/tests/classify_unittest.cc
index a090f84406df395a3d5f92df712b7211be9d220a..5825086750324afdc6d173a744b7996ae0b55b55 100644 (file)
--- a/src/bin/dhcp6/tests/classify_unittest.cc
+++ b/src/bin/dhcp6/tests/classify_unittest.cc
@@ -2327,7 +2327,9 @@ TEST_F(ClassifyTest, requiredNoTest) {
 }
 
 // This test checks a required class which is not defined is ignored.
-TEST_F(ClassifyTest, requiredNoDefined) {
+// Please set KEA_LOGGER_DESTINATION to stderr or stdout and check
+// that DHCP6_REQUIRED_CLASS_UNDEFINED is logged,
+TEST_F(ClassifyTest, requiredNotDefined) {
     std::string config =
         "{"
         "\"interfaces-config\": {"