Browse Source

[4259] Addressed review comments

    Moved string/enum conversion functions inside D2ClientConfig

    Changed underscores to hypens in replace-client-name values and changed
    all literal values to lower case.

    Fixed minor wording issues in admin guide and new log messages.

    Added commentary and extra error logging to unit tests

    Added test case for invalid value of replace-client-name
Thomas Markwalder 9 years ago
parent
commit
773659ff23

+ 5 - 5
doc/guide/dhcp4-srv.xml

@@ -2078,7 +2078,7 @@ It is merely echoed by the server
       supply a portion or all of that name based upon what it receives from
       the client in the DHCP REQUEST.</para>
       <para>
-       The basic rules for constructing the FQDN that will be used for DNS
+       The default rules for constructing the FQDN that will be used for DNS
        entries are:
       <orderedlist>
       <listitem><para>
@@ -2107,15 +2107,15 @@ It is merely echoed by the server
       </para></listitem>
       <listitem><para>
         <command>always</command> - Replace the name the client sent. If the
-        client sent no name, generate one for them.
+        client sent no name, generate one for the client.
       </para></listitem>
       <listitem><para>
-        <command>when_present</command> - Replace the name the client sent.
+        <command>when-present</command> - Replace the name the client sent.
         If the client sent no name, do not generate one.
       </para></listitem>
       <listitem><para>
-        <command>when_not_present</command> - Use the name the client sent.
-        If the client sent no name, generate one for them.
+        <command>when-not-present</command> - Use the name the client sent.
+        If the client sent no name, generate one for the client.
       </para></listitem>
       </itemizedlist>
     <note>

+ 9 - 9
doc/guide/dhcp6-srv.xml

@@ -1926,14 +1926,14 @@ should include options from the isc option space:
       In general, kea-dhcp6 will generate DDNS update requests when:
       <orderedlist>
       <listitem><para>
-      A new lease is granted in response to a DHCP REQUEST
+      A new lease is granted in response to a REQUEST
       </para></listitem>
       <listitem><para>
       An existing lease is renewed but the FQDN associated with it has
       changed.
       </para></listitem>
       <listitem><para>
-      An existing lease is released in response to a DHCP RELEASE
+      An existing lease is released in response to a RELEASE
       </para></listitem>
       </orderedlist>
       In the second case, lease renewal, two  DDNS requests will be issued: one
@@ -1944,7 +1944,7 @@ should include options from the isc option space:
       discussed next.
       </para>
       <para>
-      kea-dhcp6 will generate a DDNS update request only if the DHCP REQUEST
+      kea-dhcp6 will generate a DDNS update request only if the REQUEST
       contains the FQDN option (code 39). By default kea-dhcp6 will
       respect the FQDN N and S flags specified by the client as shown in the
       following table:
@@ -2044,9 +2044,9 @@ should include options from the isc option space:
       <para>Each NameChangeRequest must of course include the fully qualified
       domain name whose DNS entries are to be affected.  kea-dhcp6 can be
       configured to supply a portion or all of that name based upon what it
-      receives from the client in the DHCP REQUEST.</para>
+      receives from the client.</para>
       <para>
-       The basic rules for constructing the FQDN that will be used for DNS
+       The default rules for constructing the FQDN that will be used for DNS
        entries are:
       <orderedlist>
       <listitem><para>
@@ -2075,15 +2075,15 @@ should include options from the isc option space:
       </para></listitem>
       <listitem><para>
         <command>always</command> - Replace the name the client sent. If the
-        client sent no name, generate one for them.
+        client sent no name, generate one for the client.
       </para></listitem>
       <listitem><para>
-        <command>when_present</command> - Replace the name the client sent.
+        <command>when-present</command> - Replace the name the client sent.
         If the client sent no name, do not generate one.
       </para></listitem>
       <listitem><para>
-        <command>when_not_present</command> - Use the name the client sent.
-        Supply the name if the client did not.
+        <command>when-not-present</command> - Use the name the client sent.
+        If the client sent no name, generate one for the client.
       </para></listitem>
       </itemizedlist>
     <note>

+ 5 - 5
src/bin/dhcp4/dhcp4_messages.mes

@@ -217,6 +217,11 @@ from a client. Server does not process empty Hostname options and therefore
 option is skipped. The argument holds the client and transaction identification
 information.
 
+% DHCP4_GENERATE_FQDN %1: client did not send a FQDN or hostname; FQDN will be be generated for the client
+This debug message is issued when the server did not receive a Hostname option
+from the client and hostname generation is enabled.  This provides a means to
+create DNS entries for unsophisticated clients.
+
 % DHCP4_HANDLE_SIGNAL_EXCEPTION An exception was thrown while handing signal: %1
 This error message is printed when an ISC or standard exception was raised during signal
 processing. This likely indicates a coding error and should be reported to ISC.
@@ -672,8 +677,3 @@ will drop its message if the received message was DHCPDISCOVER,
 and will send DHCPNAK if the received message was DHCPREQUEST.
 The argument includes the client and the transaction identification
 information.
-
-% DHCP4_SUPPLY_HOSTNAME %1: client did not send hostname option, will supply onefor them
-This debug message is issued when the server did not receive a Hostname option
-from the client and hostname generation is enabled.  This provides a means to
-create DNS entries for unsophisticated clients.

+ 1 - 1
src/bin/dhcp4/dhcp4_srv.cc

@@ -1170,7 +1170,7 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
         if (replace_name_mode == D2ClientConfig::RCM_ALWAYS ||
             replace_name_mode == D2ClientConfig::RCM_WHEN_NOT_PRESENT) {
             LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA,
-                      DHCP4_SUPPLY_HOSTNAME)
+                      DHCP4_GENERATE_FQDN)
                 .arg(ex.getQuery()->getLabel());
             OptionStringPtr opt_hostname_resp(new OptionString(Option::V4,
                                                                DHO_HOST_NAME,

+ 2 - 2
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -3176,7 +3176,7 @@ TEST_F(Dhcp4ParserTest, d2ClientConfig) {
         "     \"allow-client-update\" : true, "
         "     \"override-no-update\" : true, "
         "     \"override-client-update\" : true, "
-        "     \"replace-client-name\" : \"WHEN_PRESENT\", "
+        "     \"replace-client-name\" : \"when-present\", "
         "     \"generated-prefix\" : \"test.prefix\", "
         "     \"qualifying-suffix\" : \"test.suffix.\" },"
         "\"valid-lifetime\": 4000 }";
@@ -3238,7 +3238,7 @@ TEST_F(Dhcp4ParserTest, invalidD2ClientConfig) {
         "     \"allow-client-update\" : true, "
         "     \"override-no-update\" : true, "
         "     \"override-client-update\" : true, "
-        "     \"replace-client-name\" : \"WHEN_PRESENT\", "
+        "     \"replace-client-name\" : \"when-present\", "
         "     \"generated-prefix\" : \"test.prefix\", "
         "     \"qualifying-suffix\" : \"test.suffix.\" },"
         "\"valid-lifetime\": 4000 }";

+ 53 - 22
src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -404,7 +404,7 @@ public:
         // Create the configuration and configure the server
         char config_buf[1024];
         sprintf(config_buf, config_template, mode);
-        configure(config_buf, srv_);
+        ASSERT_NO_THROW(configure(config_buf, srv_)) << "configuration failed";
 
         // Build our client packet
         Pkt4Ptr query;
@@ -422,18 +422,22 @@ public:
         ASSERT_NO_THROW(
             hostname = processHostname(query,
                                        client_name_flag == CLIENT_NAME_PRESENT)
-        );
+        ) << "processHostname throw an exception";
 
         // Verify the contents (or lack thereof) of the hostname
         if (exp_replacement_flag == NAME_REPLACED) {
-            ASSERT_TRUE(hostname);
+            ASSERT_TRUE(hostname)
+                << "No host name, it should have the replacement name \".\"";
             EXPECT_EQ(".", hostname->getValue());
         } else {
             if (client_name_flag == CLIENT_NAME_PRESENT) {
-                ASSERT_TRUE(hostname);
+                ASSERT_TRUE(hostname)
+                    << "No host name, expected original from client";
                 EXPECT_EQ("my.example.com.", hostname->getValue());
             } else {
-                ASSERT_FALSE(hostname);
+                ASSERT_FALSE(hostname)
+                    << "Host name is: " << hostname
+                    << ", it should have been null";
             }
         }
     }
@@ -458,11 +462,19 @@ public:
     }
 
 
-    // Processes the Hostname option in the client's message and returns
-    // the hostname option which would be sent to the client. It will
-    // throw NULL pointer if the hostname option is not to be included
-    // in the response.
-    OptionStringPtr processHostname(const Pkt4Ptr& query, bool must_have_host = true) {
+    /// @brief  Invokes Dhcpsrv4::processHostname on the given packet
+    ///
+    /// Processes the Hostname option in the client's message and returns
+    /// the hostname option which would be sent to the client. It will
+    /// return empty if the hostname option is not to be included
+    /// server's response.
+    /// @param query - client packet to process
+    /// @param must_have_host - flag indicating whether or not the client
+    /// packet must contain the hostname option
+    ///
+    /// @return a pointer to the hostname option constructed by the server
+    OptionStringPtr processHostname(const Pkt4Ptr& query,
+                                    bool must_have_host = true) {
         if (!getHostnameOption(query) && must_have_host) {
             ADD_FAILURE() << "Hostname option not carried in the query";
         }
@@ -483,7 +495,26 @@ public:
 
     }
 
-    // Verify that NameChangeRequest holds valid values.
+    ///@brief Verify that NameChangeRequest holds valid values.
+    ///
+    /// Pulls the NCR from the top of the send queue and checks it's content
+    ///  against a number of expected parameters.
+    ///
+    /// @param type - expected NCR change type, CHG_ADD or CHG_REMOVE
+    /// @param reverse - flag indicating whether or not the NCR specifies
+    /// reverse change
+    /// @param forward - flag indication whether or not the NCR specifies
+    /// forward change
+    /// @param addr  - expected lease address in the NCR
+    /// @param fqdn  - expected FQDN in the NCR
+    /// @param dhcid - expected DHCID in the NCR (comparison is performed only
+    /// if the value supplied is not empty):w
+    /// @param cltt - cltt value from the lease the NCR for which the NCR
+    /// was generated expected value for
+    /// @param len - expected lease length in the NCR
+    /// @param not_strict_expire_check - when true the comparison of the NCR
+    /// lease expiration time is conducted as greater than or equal to rather
+    /// equal to CLTT plus lease lenght.
     void verifyNameChangeRequest(const isc::dhcp_ddns::NameChangeType type,
                                  const bool reverse, const bool forward,
                                  const std::string& addr,
@@ -508,9 +539,9 @@ public:
         }
         // In some cases, the test doesn't have access to the last transmission
         // time for the particular client. In such cases, the test can use the
-        // current time as cltt but the it may not check the lease expiration time
-        // for equality but rather check that the lease expiration time is not
-        // greater than the current time + lease lifetime.
+        // current time as cltt but the it may not check the lease expiration
+        // time for equality but rather check that the lease expiration time
+        // is not greater than the current time + lease lifetime.
         if (not_strict_expire_check) {
             EXPECT_GE(cltt + len, ncr->getLeaseExpiresOn());
         } else {
@@ -1387,24 +1418,24 @@ TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) {
 
     // We pass mode labels in with enclosing quotes so we can also test
     // unquoted boolean literals true/false
-    testReplaceClientNameMode("\"NEVER\"",
+    testReplaceClientNameMode("\"never\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
-    testReplaceClientNameMode("\"NEVER\"",
+    testReplaceClientNameMode("\"never\"",
                               CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
 
-    testReplaceClientNameMode("\"ALWAYS\"",
+    testReplaceClientNameMode("\"always\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
-    testReplaceClientNameMode("\"ALWAYS\"",
+    testReplaceClientNameMode("\"always\"",
                               CLIENT_NAME_PRESENT, NAME_REPLACED);
 
-    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+    testReplaceClientNameMode("\"when-present\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
-    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+    testReplaceClientNameMode("\"when-present\"",
                               CLIENT_NAME_PRESENT, NAME_REPLACED);
 
-    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+    testReplaceClientNameMode("\"when-not-present\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
-    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+    testReplaceClientNameMode("\"when-not-present\"",
                               CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
 
     // Verify that boolean false produces the same result as RCM_NEVER

+ 6 - 5
src/bin/dhcp6/dhcp6_messages.mes

@@ -145,6 +145,12 @@ to the server. The first argument includes the client and
 transaction identification information. The second argument includes
 the generated FQDN.
 
+% DHCP6_DDNS_GENERATE_FQDN %1: client did not send a FQDN option; FQDN will be
+generated for the client.
+This debug message is issued when the server did not receive a FQDN option
+from the client and client name replacement is enabled.  This provides a means
+to create DNS entries for unsophisticated clients.
+
 % DHCP6_DDNS_GENERATED_FQDN_UPDATE_FAIL %1: failed to update the lease using address %2, after generating FQDN for a client, reason: %3
 This message indicates the failure when trying to update the lease and/or
 options in the server's response with the hostname generated by the server
@@ -183,11 +189,6 @@ the lease is acquired for the client.
 This debug message is logged when server includes an DHCPv6 Client FQDN Option
 in its response to the client.
 
-% DHCP6_DDNS_SUPPLY_FQDN %1: client did not send a FQDN option, will supply one for them
-This debug message is issued when the server did not receive a FQDN option
-from the client and client name replacement is enabled.  This provides a means
-to create DNS entries for unsophisticated clients.
-
 % DHCP6_DEACTIVATE_INTERFACE deactivate interface %1
 This message is printed when DHCPv6 server disables an interface from being
 used to receive DHCPv6 traffic. Sockets on this interface will not be opened

+ 3 - 2
src/bin/dhcp6/dhcp6_srv.cc

@@ -1081,10 +1081,11 @@ Dhcpv6Srv::processClientFqdn(const Pkt6Ptr& question, const Pkt6Ptr& answer,
             // be supplied later on.
             fqdn.reset(new Option6ClientFqdn(Option6ClientFqdn::FLAG_S, "",
                                              Option6ClientFqdn::PARTIAL));
-            LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL, DHCP6_DDNS_SUPPLY_FQDN)
+            LOG_DEBUG(ddns6_logger, DBG_DHCP6_DETAIL, DHCP6_DDNS_GENERATE_FQDN)
                 .arg(question->getLabel());
         } else {
-            // No FQDN so lease hostname comes from host reservation if one
+            // No FQDN so get the lease hostname from the host reservation if
+            // there is one.
             if (ctx.host_) {
                 ctx.hostname_ = ctx.host_->getHostname();
             }

+ 2 - 2
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -3410,7 +3410,7 @@ TEST_F(Dhcp6ParserTest, d2ClientConfig) {
         "     \"allow-client-update\" : true, "
         "     \"override-no-update\" : true, "
         "     \"override-client-update\" : true, "
-        "     \"replace-client-name\" : \"WHEN_PRESENT\", "
+        "     \"replace-client-name\" : \"when-present\", "
         "     \"generated-prefix\" : \"test.prefix\", "
         "     \"qualifying-suffix\" : \"test.suffix.\" },"
         "\"valid-lifetime\": 4000 }";
@@ -3472,7 +3472,7 @@ TEST_F(Dhcp6ParserTest, invalidD2ClientConfig) {
         "     \"allow-client-update\" : true, "
         "     \"override-no-update\" : true, "
         "     \"override-client-update\" : true, "
-        "     \"replace-client-name\" : \"WHEN_PRESENT\", "
+        "     \"replace-client-name\" : \"when-present\", "
         "     \"generated-prefix\" : \"test.prefix\", "
         "     \"qualifying-suffix\" : \"test.suffix.\" },"
         "\"valid-lifetime\": 4000 }";

+ 8 - 8
src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -1334,24 +1334,24 @@ TEST_F(FqdnDhcpv6SrvTest, replaceClientNameModeTest) {
 
     // We pass mode labels in with enclosing quotes so we can also test
     // unquoted boolean literals true/false
-    testReplaceClientNameMode("\"NEVER\"",
+    testReplaceClientNameMode("\"never\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
-    testReplaceClientNameMode("\"NEVER\"",
+    testReplaceClientNameMode("\"never\"",
                               CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
 
-    testReplaceClientNameMode("\"ALWAYS\"",
+    testReplaceClientNameMode("\"always\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
-    testReplaceClientNameMode("\"ALWAYS\"",
+    testReplaceClientNameMode("\"always\"",
                               CLIENT_NAME_PRESENT, NAME_REPLACED);
 
-    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+    testReplaceClientNameMode("\"when-present\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
-    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+    testReplaceClientNameMode("\"when-present\"",
                               CLIENT_NAME_PRESENT, NAME_REPLACED);
 
-    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+    testReplaceClientNameMode("\"when-not-present\"",
                               CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
-    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+    testReplaceClientNameMode("\"when-not-present\"",
                               CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
 
     // Verify that boolean false produces the same result as RCM_NEVER

+ 13 - 11
src/lib/dhcpsrv/d2_client_cfg.cc

@@ -34,20 +34,21 @@ const char* D2ClientConfig::DFT_REPLACE_CLIENT_NAME_MODE = "NEVER";
 const char* D2ClientConfig::DFT_GENERATED_PREFIX = "myhost";
 
 
-D2ClientConfig::ReplaceClientNameMode stringToReplaceClientNameMode(const std::string& mode_str) {
-    if (boost::iequals(mode_str, "NEVER")) {
+D2ClientConfig::ReplaceClientNameMode
+D2ClientConfig::stringToReplaceClientNameMode(const std::string& mode_str) {
+    if (boost::iequals(mode_str, "never")) {
         return (D2ClientConfig::RCM_NEVER);
     }
 
-    if (boost::iequals(mode_str, "ALWAYS")) {
+    if (boost::iequals(mode_str, "always")) {
         return (D2ClientConfig::RCM_ALWAYS);
     }
 
-    if (boost::iequals(mode_str, "WHEN_PRESENT")) {
+    if (boost::iequals(mode_str, "when-present")) {
         return (D2ClientConfig::RCM_WHEN_PRESENT);
     }
 
-    if (boost::iequals(mode_str, "WHEN_NOT_PRESENT")) {
+    if (boost::iequals(mode_str, "when-not-present")) {
         return (D2ClientConfig::RCM_WHEN_NOT_PRESENT);
     }
 
@@ -55,22 +56,23 @@ D2ClientConfig::ReplaceClientNameMode stringToReplaceClientNameMode(const std::s
               "Invalid ReplaceClientNameMode: " << mode_str);
 }
 
-std::string replaceClientNameModeToString(D2ClientConfig::ReplaceClientNameMode mode) {
+std::string
+D2ClientConfig::replaceClientNameModeToString(const ReplaceClientNameMode& mode) {
     switch (mode) {
     case D2ClientConfig::RCM_NEVER:
-        return ("NEVER");
+        return ("never");
     case D2ClientConfig::RCM_ALWAYS:
-        return ("ALWAYS");
+        return ("always");
     case D2ClientConfig::RCM_WHEN_PRESENT:
-        return ("WHEN_PRESENT");
+        return ("when-present");
     case D2ClientConfig::RCM_WHEN_NOT_PRESENT:
-        return ("WHEN_NOT_PRESENT");
+        return ("when-not-present");
     default:
         break;
     }
 
     std::ostringstream stream;
-    stream  << "UNKNOWN(" << mode << ")";
+    stream  << "unknown(" << mode << ")";
     return (stream.str());
 }
 

+ 20 - 20
src/lib/dhcpsrv/d2_client_cfg.h

@@ -210,6 +210,26 @@ public:
     /// @param enable boolean value to assign to the enable-updates flag
     void enableUpdates(bool enable);
 
+    /// @brief Converts labels to  ReplaceClientNameMode enum values.
+    ///
+    /// @param mode_str text to convert to an enum.
+    /// Valid string values: "never", "always", "when-present",
+    /// "when-not-present" (case insensistive)
+    ///
+    /// @return NameChangeFormat value which maps to the given string.
+    ///
+    /// @throw isc::BadValue if given a string value which does not map to an
+    /// enum value.
+    static ReplaceClientNameMode stringToReplaceClientNameMode(const std::string& mode_str);
+
+    /// @brief Converts NameChangeFormat enums to text labels.
+    ///
+    /// @param mode enum value to convert to label
+    ///
+    /// @return std:string containing the text label if the value is valid, or
+    /// "unknown" if not.
+    static std::string replaceClientNameModeToString(const ReplaceClientNameMode& mode);
+
 protected:
     /// @brief Validates member values.
     ///
@@ -268,26 +288,6 @@ private:
 std::ostream&
 operator<<(std::ostream& os, const D2ClientConfig& config);
 
-/// @brief Function which converts labels to  ReplaceClientNameMode enum values.
-///
-/// @param mode_str text to convert to an enum.
-/// Valid string values: "NEVER", "ALWAYS", "WHEN_PRESENT", "WHEN_NOT_PRESENT"
-///
-/// @return NameChangeFormat value which maps to the given string.
-///
-/// @throw isc::BadValue if given a string value which does not map to an
-/// enum value.
-extern D2ClientConfig::ReplaceClientNameMode stringToReplaceClientNameMode(const std::string& mode_str);
-
-/// @brief Function which converts NameChangeFormat enums to text labels.
-///
-/// @param mode enum value to convert to label
-///
-/// @return std:string containing the text label if the value is valid, or
-/// "UNKNOWN" if not.
-extern std::string replaceClientNameModeToString(D2ClientConfig::ReplaceClientNameMode mode);
-
-
 /// @brief Defines a pointer for D2ClientConfig instances.
 typedef boost::shared_ptr<D2ClientConfig> D2ClientConfigPtr;
 

+ 6 - 4
src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1409,9 +1409,10 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) {
         // Formerly, replace-client-name was boolean, so for now we'll support boolean
         // values by mapping them to the appropriate mode
         D2ClientConfig::ReplaceClientNameMode replace_client_name_mode;
-        std::string mode_str = string_values_->getOptionalParam("replace-client-name",
-                                                                D2ClientConfig::
-                                                                DFT_REPLACE_CLIENT_NAME_MODE);
+        std::string mode_str;
+        mode_str  = string_values_->getOptionalParam("replace-client-name",
+                                                     D2ClientConfig::
+                                                     DFT_REPLACE_CLIENT_NAME_MODE);
         if (boost::iequals(mode_str, "FALSE")) {
             // @todo add a debug log
             replace_client_name_mode = D2ClientConfig::RCM_NEVER;
@@ -1420,7 +1421,8 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) {
             // @todo add a debug log
             replace_client_name_mode = D2ClientConfig::RCM_WHEN_PRESENT;
         } else {
-            replace_client_name_mode = stringToReplaceClientNameMode(mode_str);
+            replace_client_name_mode = D2ClientConfig::
+                                       stringToReplaceClientNameMode(mode_str);
         }
 
         // Attempt to create the new client config.

+ 22 - 10
src/lib/dhcpsrv/tests/d2_client_unittest.cc

@@ -22,18 +22,30 @@ namespace {
 
 /// @brief Tests conversion of NameChangeFormat between enum and strings.
 TEST(ReplaceClientNameModeTest, formatEnumConversion){
-    ASSERT_EQ(stringToReplaceClientNameMode("NEVER"), D2ClientConfig::RCM_NEVER);
-    ASSERT_EQ(stringToReplaceClientNameMode("ALWAYS"), D2ClientConfig::RCM_ALWAYS);
-    ASSERT_EQ(stringToReplaceClientNameMode("WHEN_PRESENT"), D2ClientConfig::RCM_WHEN_PRESENT);
-    ASSERT_EQ(stringToReplaceClientNameMode("WHEN_NOT_PRESENT"),
+    ASSERT_EQ(D2ClientConfig::stringToReplaceClientNameMode("never"),
+              D2ClientConfig::RCM_NEVER);
+    ASSERT_EQ(D2ClientConfig::stringToReplaceClientNameMode("always"),
+              D2ClientConfig::RCM_ALWAYS);
+    ASSERT_EQ(D2ClientConfig::stringToReplaceClientNameMode("when-present"),
+              D2ClientConfig::RCM_WHEN_PRESENT);
+    ASSERT_EQ(D2ClientConfig::stringToReplaceClientNameMode("when-not-present"),
               D2ClientConfig::RCM_WHEN_NOT_PRESENT);
-    ASSERT_THROW(stringToReplaceClientNameMode("BOGUS"), isc::BadValue);
+    ASSERT_THROW(D2ClientConfig::stringToReplaceClientNameMode("BOGUS"),
+                 isc::BadValue);
 
-    ASSERT_EQ(replaceClientNameModeToString(D2ClientConfig::RCM_NEVER), "NEVER");
-    ASSERT_EQ(replaceClientNameModeToString(D2ClientConfig::RCM_ALWAYS), "ALWAYS");
-    ASSERT_EQ(replaceClientNameModeToString(D2ClientConfig::RCM_WHEN_PRESENT), "WHEN_PRESENT");
-    ASSERT_EQ(replaceClientNameModeToString(D2ClientConfig::RCM_WHEN_NOT_PRESENT),
-              "WHEN_NOT_PRESENT");
+    ASSERT_EQ(D2ClientConfig::
+              replaceClientNameModeToString(D2ClientConfig::RCM_NEVER),
+              "never");
+    ASSERT_EQ(D2ClientConfig::
+              replaceClientNameModeToString(D2ClientConfig::RCM_ALWAYS),
+              "always");
+    ASSERT_EQ(D2ClientConfig::
+              replaceClientNameModeToString(D2ClientConfig::RCM_WHEN_PRESENT),
+              "when-present");
+    ASSERT_EQ(D2ClientConfig::
+              replaceClientNameModeToString(D2ClientConfig::
+                                            RCM_WHEN_NOT_PRESENT),
+              "when-not-present");
 }
 
 /// @brief Checks constructors and accessors of D2ClientConfig.

+ 24 - 3
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -1391,7 +1391,7 @@ TEST_F(ParseConfigTest, validD2Config) {
         "     \"always-include-fqdn\" : true, "
         "     \"override-no-update\" : true, "
         "     \"override-client-update\" : true, "
-        "     \"replace-client-name\" : \"WHEN_PRESENT\", "
+        "     \"replace-client-name\" : \"when-present\", "
         "     \"generated-prefix\" : \"test.prefix\", "
         "     \"qualifying-suffix\" : \"test.suffix.\" "
         "    }"
@@ -1437,7 +1437,7 @@ TEST_F(ParseConfigTest, validD2Config) {
         "     \"always-include-fqdn\" : false, "
         "     \"override-no-update\" : false, "
         "     \"override-client-update\" : false, "
-        "     \"replace-client-name\" : \"NEVER\", "
+        "     \"replace-client-name\" : \"never\", "
         "     \"generated-prefix\" : \"\", "
         "     \"qualifying-suffix\" : \"\" "
         "    }"
@@ -1533,7 +1533,9 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) {
               d2_client_config->getOverrideNoUpdate());
     EXPECT_EQ(D2ClientConfig::DFT_OVERRIDE_CLIENT_UPDATE,
               d2_client_config->getOverrideClientUpdate());
-    EXPECT_EQ(stringToReplaceClientNameMode(D2ClientConfig::DFT_REPLACE_CLIENT_NAME_MODE),
+    EXPECT_EQ(D2ClientConfig::
+              stringToReplaceClientNameMode(D2ClientConfig::
+                                            DFT_REPLACE_CLIENT_NAME_MODE),
               d2_client_config->getReplaceClientNameMode());
     EXPECT_EQ(D2ClientConfig::DFT_GENERATED_PREFIX,
               d2_client_config->getGeneratedPrefix());
@@ -1674,6 +1676,25 @@ TEST_F(ParseConfigTest, invalidD2Config) {
         "     \"qualifying-suffix\" : \"test.suffix.\" "
         "    }"
         "}",
+        // Invalid replace-client-name value
+        "{ \"dhcp-ddns\" :"
+        "    {"
+        "     \"enable-updates\" : true, "
+        "     \"server-ip\" : \"3001::5\", "
+        "     \"server-port\" : 3433, "
+        "     \"sender-ip\" : \"3001::5\", "
+        "     \"sender-port\" : 3434, "
+        "     \"max-queue-size\" : 2048, "
+        "     \"ncr-protocol\" : \"UDP\", "
+        "     \"ncr-format\" : \"JSON\", "
+        "     \"always-include-fqdn\" : true, "
+        "     \"override-no-update\" : true, "
+        "     \"override-client-update\" : true, "
+        "     \"replace-client-name\" : \"BOGUS\", "
+        "     \"generated-prefix\" : \"test.prefix\", "
+        "     \"qualifying-suffix\" : \"test.suffix.\" "
+        "    }"
+        "}",
         // stop
         ""
     };