[#4011] flex-id identifier-expression now optional

new file:   changelog_unreleased/4011-kea-dhcp6-client-ip-reservation-inconsistently-assigned
modified:   doc/sphinx/arm/hooks-flex-id.rst
modified:   src/hooks/dhcp/flex_id/flex_id_messages.cc
modified:   src/hooks/dhcp/flex_id/flex_id_messages.h
modified:   src/hooks/dhcp/flex_id/flex_id_messages.mes
modified:   src/hooks/dhcp/flex_id/libloadtests/load_unload_unittests.cc
modified:   src/hooks/dhcp/flex_id/load_unload.cc

diff --git a/changelog_unreleased/4011-kea-dhcp6-client-ip-reservation-inconsistently-assigned b/changelog_unreleased/4011-kea-dhcp6-client-ip-reservation-inconsistently-assigned
new file mode 100644 (file)
index 0000000..90c4346
--- /dev/null
+++ b/changelog_unreleased/4011-kea-dhcp6-client-ip-reservation-inconsistently-assigned
@@ -0,0 +1,4 @@
+[func]         tmark
+       The flex-id hook library parameter, ``identifier-expression``,
+       is now optional. Prior to this, it was manadatory.
+       (Gitlab #4011)

diff --git a/doc/sphinx/arm/hooks-flex-id.rst b/doc/sphinx/arm/hooks-flex-id.rst
index bb75575c4195fb494ece7d56d5312506bfe51462..5a9c2c9b84d3a96752d19cb108b6aead8ad15e2e 100644 (file)
--- a/doc/sphinx/arm/hooks-flex-id.rst
+++ b/doc/sphinx/arm/hooks-flex-id.rst
@@ -40,8 +40,8 @@ be host reservations that are tied to specific values of the flexible
 identifier.
 
 The library can be loaded similarly to other hook libraries. It
-takes a mandatory parameter ``identifier-expression`` and some optional boolean
-parameters like ``replace-client-id`` and ``ignore-iaid``:
+supports the following parameters: ``identifier-expression``, ``replace-client-id``,
+and ``ignore-iaid``:
 
 ::
 
@@ -283,3 +283,12 @@ part of the expression.
 
    This functionality breaks RFC compliance and should be enabled only if
    required. When enabled, a warning message is issued at configure time.
+
+.. note::
+
+    The ``ignore-iaid`` parameter operates independently of the flexible identifier
+    feature and may be used wihtout specifying a value for ``identifier-expression``.
+    When ``identifier-expression`` is omitted or specified as an empty string, `""`,
+    the flexible identifier feature is disabled. Kea versions prior to 3.1.2 require
+    a value for ``indentifier-expression`` but accept the empty string value. As of
+    Kea 3.1.2, the parameter is optional.

diff --git a/src/hooks/dhcp/flex_id/flex_id_messages.cc b/src/hooks/dhcp/flex_id/flex_id_messages.cc
index b10344e1b8bb1389658c01fd2b0eafbbc6154c6e..feab1a3a5177f1232daaabe42de1037e489abb2e 100644 (file)
--- a/src/hooks/dhcp/flex_id/flex_id_messages.cc
+++ b/src/hooks/dhcp/flex_id/flex_id_messages.cc
@@ -4,12 +4,10 @@
 #include <log/message_types.h>
 #include <log/message_initializer.h>
 
-extern const isc::log::MessageID FLEX_ID_EXPRESSION_EMPTY = "FLEX_ID_EXPRESSION_EMPTY";
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_EVALUATED = "FLEX_ID_EXPRESSION_EVALUATED";
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_EVALUATED_NP = "FLEX_ID_EXPRESSION_EVALUATED_NP";
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_HEX = "FLEX_ID_EXPRESSION_HEX";
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_INVALID_JSON_TYPE = "FLEX_ID_EXPRESSION_INVALID_JSON_TYPE";
-extern const isc::log::MessageID FLEX_ID_EXPRESSION_NOT_DEFINED = "FLEX_ID_EXPRESSION_NOT_DEFINED";
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_PARSE_FAILED = "FLEX_ID_EXPRESSION_PARSE_FAILED";
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_APPLIED_ON_NA = "FLEX_ID_IGNORE_IAID_APPLIED_ON_NA";
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_APPLIED_ON_PD = "FLEX_ID_IGNORE_IAID_APPLIED_ON_PD";
@@ -18,6 +16,7 @@ extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_JSON_TYPE = "FLEX_ID_IGNORE
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_NA = "FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_NA";
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_PD = "FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_PD";
 extern const isc::log::MessageID FLEX_ID_LOAD_ERROR = "FLEX_ID_LOAD_ERROR";
+extern const isc::log::MessageID FLEX_ID_NO_IDENTIFIER_EXPRESSION = "FLEX_ID_NO_IDENTIFIER_EXPRESSION";
 extern const isc::log::MessageID FLEX_ID_REPLACE_CLIENT_ID_JSON_TYPE = "FLEX_ID_REPLACE_CLIENT_ID_JSON_TYPE";
 extern const isc::log::MessageID FLEX_ID_RESTORE_CLIENT_ID = "FLEX_ID_RESTORE_CLIENT_ID";
 extern const isc::log::MessageID FLEX_ID_RESTORE_DUID = "FLEX_ID_RESTORE_DUID";
@@ -28,12 +27,10 @@ extern const isc::log::MessageID FLEX_ID_USED_AS_DUID = "FLEX_ID_USED_AS_DUID";
 namespace {
 
 const char* values[] = {
-    "FLEX_ID_EXPRESSION_EMPTY", "Specified identifier-expression is empty",
     "FLEX_ID_EXPRESSION_EVALUATED", "Expression evaluated for packet to \"%1\" (size: %2)",
     "FLEX_ID_EXPRESSION_EVALUATED_NP", "Expression evaluated for packet to 0x%1 (size: %2)",
     "FLEX_ID_EXPRESSION_HEX", "evaluated expression in hexadecimal form \"%1\"",
     "FLEX_ID_EXPRESSION_INVALID_JSON_TYPE", "The identifier-expression is %1, but expected JSON string",
-    "FLEX_ID_EXPRESSION_NOT_DEFINED", "Expression (identifier-expression) is not defined.",
     "FLEX_ID_EXPRESSION_PARSE_FAILED", "The identifier-expression is [%1], but fails to parse with error: %2",
     "FLEX_ID_IGNORE_IAID_APPLIED_ON_NA", "the ignore-iaid has changed IAID (%1) to 0 for the IA_NA option.",
     "FLEX_ID_IGNORE_IAID_APPLIED_ON_PD", "the ignore-iaid has changed IAID (%1) to 0 for the IA_PD option.",
@@ -42,6 +39,7 @@ const char* values[] = {
     "FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_NA", "the ignore-iaid was not applied on the packet because it contains more than one IA_NA.",
     "FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_PD", "the ignore-iaid was not applied on the packet because it contains more than one IA_PD.",
     "FLEX_ID_LOAD_ERROR", "An error occurred loading the library %1",
+    "FLEX_ID_NO_IDENTIFIER_EXPRESSION", "identifier-expression is either not specified or empty",
     "FLEX_ID_REPLACE_CLIENT_ID_JSON_TYPE", "the replace-client-id is %1 but expected boolean value",
     "FLEX_ID_RESTORE_CLIENT_ID", "restoring original client identifier '%1' in the response",
     "FLEX_ID_RESTORE_DUID", "restoring original DUID \"%1\" in the response",

diff --git a/src/hooks/dhcp/flex_id/flex_id_messages.h b/src/hooks/dhcp/flex_id/flex_id_messages.h
index 1fb008951bd7836e00c8c4eb165c75b6c488247b..6c8ad58db871bbb9f13e7887ed44cdb989bf5e1e 100644 (file)
--- a/src/hooks/dhcp/flex_id/flex_id_messages.h
+++ b/src/hooks/dhcp/flex_id/flex_id_messages.h
@@ -5,12 +5,10 @@
 
 #include <log/message_types.h>
 
-extern const isc::log::MessageID FLEX_ID_EXPRESSION_EMPTY;
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_EVALUATED;
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_EVALUATED_NP;
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_HEX;
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_INVALID_JSON_TYPE;
-extern const isc::log::MessageID FLEX_ID_EXPRESSION_NOT_DEFINED;
 extern const isc::log::MessageID FLEX_ID_EXPRESSION_PARSE_FAILED;
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_APPLIED_ON_NA;
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_APPLIED_ON_PD;
@@ -19,6 +17,7 @@ extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_JSON_TYPE;
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_NA;
 extern const isc::log::MessageID FLEX_ID_IGNORE_IAID_NOT_APPLIED_ON_PD;
 extern const isc::log::MessageID FLEX_ID_LOAD_ERROR;
+extern const isc::log::MessageID FLEX_ID_NO_IDENTIFIER_EXPRESSION;
 extern const isc::log::MessageID FLEX_ID_REPLACE_CLIENT_ID_JSON_TYPE;
 extern const isc::log::MessageID FLEX_ID_RESTORE_CLIENT_ID;
 extern const isc::log::MessageID FLEX_ID_RESTORE_DUID;

diff --git a/src/hooks/dhcp/flex_id/flex_id_messages.mes b/src/hooks/dhcp/flex_id/flex_id_messages.mes
index 6e6cb1e4fd897fe055722f79dd8837a0abd75f76..424e6a361b2f161fd7f3a5ea096fd9b03d616f7d 100644 (file)
--- a/src/hooks/dhcp/flex_id/flex_id_messages.mes
+++ b/src/hooks/dhcp/flex_id/flex_id_messages.mes
@@ -4,11 +4,12 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-% FLEX_ID_EXPRESSION_EMPTY Specified identifier-expression is empty
-This warning message is printed when the flex-id library is being loaded,
-but the expression used to generate the identifier is empty. The library
-will load, but will not generate any identifiers. Please make sure that
-the identifier-expression parameter is specified.
+% FLEX_ID_NO_IDENTIFIER_EXPRESSION identifier-expression is either not specified or empty
+This debug message is printed when the flex-id library is being loaded,
+but the expression used to generate the identifier was either omitted or
+is empty. The library will load, but will neither generate nor replace
+client identifiers. Typically this occurs when users are only interested
+in using ignore-iaid, which operates independently of the expression.
 
 % FLEX_ID_EXPRESSION_EVALUATED Expression evaluated for packet to "%1" (size: %2)
 Logged at debug log level 40.
@@ -33,12 +34,6 @@ This error message is printed when the flex-id library is being loaded,
 but the expression used to generate the identifier is malformed. It has
 a different JSON type (e.g. is a map) rather than expected string.
 
-% FLEX_ID_EXPRESSION_NOT_DEFINED Expression (identifier-expression) is not defined.
-This warning message is printed when the flex-id library is loaded, but the
-expression used to generate the identifier is not specified. The library
-will load, but will not generate any identifiers. Please make sure that
-the identifier-expression parameter is specified for your library.
-
 % FLEX_ID_EXPRESSION_PARSE_FAILED The identifier-expression is [%1], but fails to parse with error: %2
 This error message is printed when the flex-id library is being loaded,
 but the expression used to generate the identifier is malformed. It failed

diff --git a/src/hooks/dhcp/flex_id/libloadtests/load_unload_unittests.cc b/src/hooks/dhcp/flex_id/libloadtests/load_unload_unittests.cc
index ba34c0ce21675758b43b5317319685449b466d19..489f1e9507c08a4b26199fddbc7c9a75b299a512 100644 (file)
--- a/src/hooks/dhcp/flex_id/libloadtests/load_unload_unittests.cc
+++ b/src/hooks/dhcp/flex_id/libloadtests/load_unload_unittests.cc
@@ -67,17 +67,25 @@ TEST_F(FlexIdLibLoadTest, invalidDaemonLoad) {
     invalidDaemonTest("bogus", AF_INET, valid_params_);
 }
 
-// Simple test that checks the library fails to load on bad expressions.
-TEST_F(FlexIdLibLoadTest, badExpression) {
+// Simple test that checks the library still loads when identifier expression is
+// either omitted or empty.
+TEST_F(FlexIdLibLoadTest, emptyOrOmittedExpressionValid) {
     // Prepare parameters for the callout parameters library.
     ElementPtr params = Element::createMap();
     params->set("replace-client-id", Element::create(true));
-    invalidDaemonTest("kea-dhcp4", AF_INET, params);
+    params->set("ignore-iaid", Element::create(true));
+    validDaemonTest("kea-dhcp4", AF_INET, params);
 
     // Empty expression.
     params->set("identifier-expression", Element::create(string("")));
-    // Warning (vs error)
     validDaemonTest("kea-dhcp4", AF_INET, params);
+}
+
+// Simple test that checks the library fails to load on bad expressions.
+TEST_F(FlexIdLibLoadTest, badExpression) {
+    // Prepare parameters for the callout parameters library.
+    ElementPtr params = Element::createMap();
+    params->set("replace-client-id", Element::create(true));
 
     // Bad expression.
     params->set("identifier-expression", Element::create("foobar"));

diff --git a/src/hooks/dhcp/flex_id/load_unload.cc b/src/hooks/dhcp/flex_id/load_unload.cc
index af842d8420000c3959c92d0852e29dd563189761..5bc5f3a30f4ea3b5f3de43085bd87f4ff9616f2f 100644 (file)
--- a/src/hooks/dhcp/flex_id/load_unload.cc
+++ b/src/hooks/dhcp/flex_id/load_unload.cc
@@ -24,6 +24,7 @@ using namespace flex_id;
 using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::process;
+using namespace isc::log;
 
 namespace { // anonymous namespace.
 
@@ -72,31 +73,29 @@ int load(LibraryHandle& handle) {
                       << ", expected " << expected_proc_name);
         }
 
-        // identifier-expression is mandatory
+        // identifier-expression is optional.
         data::ConstElementPtr param = handle.getParameter("identifier-expression");
-        if (!param) {
-            LOG_ERROR(flex_id_logger, FLEX_ID_EXPRESSION_NOT_DEFINED);
-            return (1);
-        }
+        std::string expr;
+        if (param) {
+            // It must be a string...
+            if (param->getType() != Element::string) {
+                LOG_ERROR(flex_id_logger, FLEX_ID_EXPRESSION_INVALID_JSON_TYPE)
+                          .arg(Element::typeToName(param->getType()));
+                return (1);
+            }
 
-        // It must be a string...
-        if (param->getType() != Element::string) {
-            LOG_ERROR(flex_id_logger, FLEX_ID_EXPRESSION_INVALID_JSON_TYPE)
-                .arg(Element::typeToName(param->getType()));
-            return (1);
+            std::string expr = param->stringValue();
+            if (!expr.empty() && !checkExpression(v6, expr)) {
+                // The error was logged.
+                return (1);
+            }
         }
 
-        // ... and shouldn't be empty.
-        std::string expr = param->stringValue();
         if (expr.empty()) {
-            // Ok, we can continue without the expression, but it's just useless
-            // to have this lib loaded. Nevertheless, there may be cases when
-            // user temporarily changes the expression to empty string to
-            // troubleshoot something.
-            LOG_WARN(flex_id_logger, FLEX_ID_EXPRESSION_EMPTY);
-        } else if (!checkExpression(v6, expr)) {
-            // The error was logged.
-            return (1);
+            // Ok, we can continue without the expression. This is likely the
+            // case when users are only interested in ignore-iaid. 
+            LOG_DEBUG(flex_id_logger, DBGLVL_TRACE_BASIC,
+                      FLEX_ID_NO_IDENTIFIER_EXPRESSION);
         }
 
         // replace-client-id indicates if flexible identifier should be used to
@@ -127,6 +126,9 @@ int load(LibraryHandle& handle) {
                 }
 
                 ignore_iaid = param_ignore->boolValue();
+                if (ignore_iaid) {
+                    LOG_WARN(flex_id_logger, FLEX_ID_IGNORE_IAID_ENABLED);
+                }
             }
         }
 
@@ -135,10 +137,6 @@ int load(LibraryHandle& handle) {
 
         // Store specified expression.
         storeConfiguration(v6, expr, replace_client_id, ignore_iaid);
-
-        if (ignore_iaid) {
-            LOG_WARN(flex_id_logger, FLEX_ID_IGNORE_IAID_ENABLED);
-        }
     } catch (const std::exception& ex) {
         // Log the error and return failure.
         LOG_ERROR(flex_id_logger, FLEX_ID_LOAD_ERROR)