make dhcp-ddns qualifying-suffix mandatory in enabled configs

ChangeLog

@@ -1,3 +1,10 @@
+893.	[func,bug]	fdupont
+	Changed the qualifying-suffix parameter in the dhcp-ddns
+	configuration element to be mandatory with no default value when
+	updates are enabled (i.e., the enable-updates mandatory parameter
+	is true).
+	(Trac #3632, git )
+
 892.	[func]		sar
 	A class, LeaseFileStats, has been added to provide simple
 	statistics for use with lease files.  Also added logging

doc/guide/dhcp4-srv.xml

@@ -318,7 +318,7 @@ url="http://jsonviewer.stack.hu/"/>.
   database host name must also be specified (although it should be noted that this
   configuration may have a severe impact on server performance):
 <screen>
-"Dhcp4": { "lease-database": { <userinput>"host": <replaceable>remote-host-name</replaceable>"</userinput>, ... }, ... }
+"Dhcp4": { "lease-database": { <userinput>"host": <replaceable>remote-host-name</replaceable></userinput>, ... }, ... }
 </screen>
   The usual state of affairs will be to have the database on the same machine as
   the DHCPv4 server.  In this case, set the value to the empty string:
@@ -1270,7 +1270,7 @@ temporarily override a list of interface names and listen on all interfaces.
     "subnet4": [
         {
             "subnet": "192.0.2.0/24"
-            "option-data": [ {"
+            "option-data": [ {
                 "name": "domain-name-servers",
                 "code": 6,
                 "data": "192.0.2.200,192.0.2.201",
@@ -1416,27 +1416,68 @@ temporarily override a list of interface names and listen on all interfaces.
       </orderedlist>
       The parameters for controlling the generation of NCRs for submission to D2
       are contained in the <command>dhcp-ddns</command> section of the kea-dhcp4 server
-      configuration. The default values for this section are as follows:
+      configuration. The mandatory parameters for the DHCP DDNS configuration
+      are <command>enable-updates</command> which is unconditionally
+      required, and <command>qualifying-suffix</command> which has no
+      default value and is required when <command>enable-updates</command>
+      is set to <command>true</command>.
+
+      The two (disabled and enabled) minimal DHCP DDNS configurations are:
+<screen>
+"Dhcp4": {
+    "dhcp-ddns": {
+        <userinput>"enable-updates": false</userinput>
+    },
+    ...
+}
+</screen>
+      and for example:
 <screen>
 "Dhcp4": {
     "dhcp-ddns": {
         <userinput>"enable-updates": true,
-        "server-ip": "127.0.0.1",
-        "server-port": 53001,
-        "sender-ip": "",
-        "sender-port": 0,
-        "max-queue-size": 1024,
-        "ncr-protocol": "UDP",
-        "ncr-format": "JSON",
-        "override-no-update": false,
-        "override-client-update": false,
-        "replace-client-name": false,
-        "generated-prefix": "myhost",
-        "qualifying-suffix": "example.com"</userinput>
+        "qualifying-suffix": "example."</userinput>
     },
     ...
 }
 </screen>
+
+      The default values for the "dhcp-ddns" section are as follows:
+      <itemizedlist>
+      <listitem><simpara>
+      <command>"server-ip": "127.0.0.1"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"server-port": 53001</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"sender-ip": ""</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"sender-port": 0</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"max-queue-size": 1024</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"ncr-protocol": "UDP"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"ncr-format": "JSON"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"override-no-update": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"override-client-update": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"replace-client-name": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"generated-prefix": "myhost"</command>
+      </simpara></listitem>
+      </itemizedlist>
       </para>
 
       <section id="dhcpv4-d2-io-config">
@@ -1712,9 +1753,12 @@ temporarily override a list of interface names and listen on all interfaces.
 }
 </screen>
       <para>
-      The suffix used when generating a FQDN or when qualifying a partial name
-      is specified by the <command>qualifying-suffix</command> parameter. There
-      is no default value. To set its value simply set it to the desired string:
+      The suffix used when generating a FQDN or when qualifying a
+      partial name is specified by
+      the <command>qualifying-suffix</command> parameter. This
+      parameter has no default value, thus it is mandatory when
+      DDNS updates are enabled.
+      To set its value simply set it to the desired string:
       </para>
 <screen>
 "Dhcp4": {
@@ -1734,10 +1778,9 @@ temporarily override a list of interface names and listen on all interfaces.
       </para>
       <para>
       where address-text is simply the lease IP address converted to a
-      hyphenated string.  For example, if the lease address is 172.16.1.10 and
-      default values are used for
-      <command>generated-prefix</command> and <command>qualifying-suffix</command>, the
-      generated FQDN would be:
+      hyphenated string.  For example, if the lease address is 172.16.1.10,
+      the qualifying suffix "example.com", and the default value is used for
+      <command>generated-prefix</command>, the generated FQDN would be:
       </para>
       <para>
         myhost-172-16-1-10.example.com.

doc/guide/dhcp6-srv.xml

@@ -322,7 +322,7 @@ JSON validator is available at <ulink url="http://jsonviewer.stack.hu/"/>.
   database host name must also be specified (although it should be noted that this
   configuration may have a severe impact on server performance):
 <screen>
-"Dhcp6": { "lease-database": { <userinput>"host": <replaceable>remote-host-name</replaceable>"</userinput>, ... }, ... }
+"Dhcp6": { "lease-database": { <userinput>"host": <replaceable>remote-host-name</replaceable></userinput>, ... }, ... }
 </screen>
   The usual state of affairs will be to have the database on the same machine as
   the DHCPv6 server.  In this case, set the value to the empty string:
@@ -1029,10 +1029,10 @@ temporarily override a list of interface names and listen on all interfaces.
             "type": "ipv6-address".
             "record-types": "",
             "array": false,
-            "encapsulate ""
+            "encapsulate": ""</userinput>
         },
         {
-            "name": "subopt2",
+            <userinput>"name": "subopt2",
             "code": 2,
             "space": "isc",
             "type": "string",
@@ -1402,27 +1402,68 @@ should include options from the isc option space:
       </orderedlist>
       The parameters controlling the generation of NCRs for submission to D2
       are contained in the "dhcp-ddns" section of kea-dhcp6
-      configuration. The default values for this section appears as follows:
+      configuration. The mandatory parameters for the DHCP DDNS configuration
+      are <command>enable-updates</command> which is unconditionally
+      required, and <command>qualifying-suffix</command> which has no
+      default value and is required when <command>enable-updates</command>
+      is set to <command>true</command>.
+
+      The two (disabled and enabled) minimal DHCP DDNS configurations are:
+<screen>
+"Dhcp6": {
+    "dhcp-ddns": {
+        <userinput>"enable-updates": false</userinput>
+    },
+    ...
+}
+</screen>
+      and for example:
 <screen>
 "Dhcp6": {
     "dhcp-ddns": {
         <userinput>"enable-updates": true,
-        "server-ip": "127.0.0.1",
-        "server-port": 53001,
-        "sender-ip": "",
-        "sender-port": 0,
-        "max-queue-size": 1024,
-        "ncr-protocol": "UDP",
-        "ncr-format": "JSON",
-        "override-no-update": false,
-        "override-client-update": false,
-        "replace-client-name": false,
-        "generated-prefix": "myhost",
-        "qualifying-suffix": "example.com"</userinput>
+        "qualifying-suffix": "example."</userinput>
     },
     ...
 }
 </screen>
+
+      The default values for the "dhcp-ddns" section are as follows:
+      <itemizedlist>
+      <listitem><simpara>
+      <command>"server-ip": "127.0.0.1"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"server-port": 53001</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"sender-ip": ""</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"sender-port": 0</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"max-queue-size": 1024</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"ncr-protocol": "UDP"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"ncr-format": "JSON"</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"override-no-update": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"override-client-update": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"replace-client-name": false</command>
+      </simpara></listitem>
+      <listitem><simpara>
+      <command>"generated-prefix": "myhost"</command>
+      </simpara></listitem>
+      </itemizedlist>
       </para>
 
 
@@ -1694,9 +1735,12 @@ should include options from the isc option space:
 }
 </screen>
       <para>
-      The suffix used when generating a FQDN or when qualifying a partial name
-      is specified by the <command>qualifying-suffix</command> parameter. There
-      is no default value. To set its value simply set it to the desired string:
+      The suffix used when generating a FQDN or when qualifying a
+      partial name is specified by
+      the <command>qualifying-suffix</command> parameter. This
+      parameter has no default value, thus it is mandatory when
+      DDNS updates are enabled.
+      To set its value simply set it to the desired string:
       </para>
 <screen>
 "Dhcp6": {
@@ -1717,8 +1761,8 @@ should include options from the isc option space:
       </para>
       <para>
       where candidate-name is the partial name supplied in the REQUEST.
-      For example, if FQDN domain name value was "some-computer" and assuming
-      the default value for qualifying-suffix, the generated FQDN would be:
+      For example, if FQDN domain name value was "some-computer" and
+      qualifying-suffix "example.com", the generated FQDN would be:
       </para>
       <para>
         some-computer.example.com.
@@ -1732,10 +1776,9 @@ should include options from the isc option space:
       </para>
       <para>
       where address-text is simply the lease IP address converted to a
-      hyphenated string.  For example, if lease address is 3001:1::70E and
-      default values are used for 
-      <command>generated-prefix</command> and <command>qualifying-suffix</command>, the
-      generated FQDN would be:
+      hyphenated string.  For example, if lease address is 3001:1::70E,
+      the qualifying suffix "example.com", and the default value is used for
+      <command>generated-prefix</command>, the generated FQDN would be:
       </para>
       <para>
         myhost-3001-1--70E.example.com.

src/bin/dhcp4/dhcp4.spec

@@ -488,7 +488,7 @@
                 "item_name": "qualifying-suffix",
                 "item_type": "string",
                 "item_optional": true,
-                "item_default": "example.com",
+                "item_default": "",
                 "item_description": "Fully qualified domain-name suffix if partial name provided by client"
             },
         ]

src/bin/dhcp6/dhcp6.spec

@@ -554,7 +554,7 @@
                 "item_name": "qualifying-suffix",
                 "item_type": "string",
                 "item_optional": true,
-                "item_default": "example.com",
+                "item_default": "",
                 "item_description": "Fully qualified domain-name suffix if partial name provided by client"
             },
         ]

src/lib/dhcpsrv/d2_client_cfg.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -36,7 +36,6 @@ const bool D2ClientConfig::DFT_OVERRIDE_NO_UPDATE = false;
 const bool D2ClientConfig::DFT_OVERRIDE_CLIENT_UPDATE = false;
 const bool D2ClientConfig::DFT_REPLACE_CLIENT_NAME = false;
 const char *D2ClientConfig::DFT_GENERATED_PREFIX = "myhost";
-const char *D2ClientConfig::DFT_QUALIFYING_SUFFIX = "example.com";
 
 D2ClientConfig::D2ClientConfig(const  bool enable_updates,
                                const isc::asiolink::IOAddress& server_ip,
@@ -85,7 +84,7 @@ D2ClientConfig::D2ClientConfig()
       override_client_update_(DFT_OVERRIDE_CLIENT_UPDATE),
       replace_client_name_(DFT_REPLACE_CLIENT_NAME),
       generated_prefix_(DFT_GENERATED_PREFIX),
-      qualifying_suffix_(DFT_QUALIFYING_SUFFIX) {
+      qualifying_suffix_("") {
     validateContents();
 }
 

src/lib/dhcpsrv/d2_client_cfg.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -73,7 +73,6 @@ public:
     static const bool DFT_OVERRIDE_CLIENT_UPDATE;
     static const bool DFT_REPLACE_CLIENT_NAME;
     static const char *DFT_GENERATED_PREFIX;
-    static const char *DFT_QUALIFYING_SUFFIX;
 
     /// @brief Constructor
     ///
@@ -96,7 +95,10 @@ public:
     /// @param replace_client_name enables replacement of the domain-name
     /// supplied by the client with a generated name.
     /// @param generated_prefix Prefix to use when generating domain-names.
-    /// @param  qualifying_suffix Suffix to use to qualify partial domain-names.
+    /// @param qualifying_suffix Suffix to use to qualify partial domain-names.
+    ///
+    /// @c enable_updates is mandatory, @c qualifying_suffix is mandatory
+    /// when updates are enabled, other parameters are optional.
     ///
     /// @throw D2ClientError if given an invalid protocol or format.
     D2ClientConfig(const bool enable_updates,

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -1238,6 +1238,11 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) {
         }
 
         // Get all parameters that are needed to create the D2ClientConfig.
+
+        // The qualifying suffix is mandatory when updates are enabled
+        std::string qualifying_suffix =
+            string_values_->getParam("qualifying-suffix");
+
         IOAddress server_ip =
             IOAddress(string_values_->getOptionalParam("server-ip",
                                                        D2ClientConfig::
@@ -1278,11 +1283,6 @@ D2ClientConfigParser::build(isc::data::ConstElementPtr client_config) {
             string_values_->getOptionalParam("generated-prefix",
                                              D2ClientConfig::
                                              DFT_GENERATED_PREFIX);
-        std::string qualifying_suffix =
-            string_values_->getOptionalParam("qualifying-suffix",
-                                             D2ClientConfig::
-                                             DFT_QUALIFYING_SUFFIX);
-
         bool always_include_fqdn =
             boolean_values_->getOptionalParam("always-include-fqdn",
                                                 D2ClientConfig::

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -1120,6 +1120,7 @@ public:
     /// The elements currently supported are (see isc::dhcp::D2ClientConfig
     /// for details on each):
     /// -# enable-updates
+    /// -# qualifying-suffix
     /// -# server-ip
     /// -# server-port
     /// -# ncr-protocol
@@ -1131,7 +1132,6 @@ public:
     /// -# override-client-update
     /// -# replace-client-name
     /// -# generated-prefix
-    /// -# qualifying-suffix
     ///
     /// @param config_id is the "item_name" for a specific member element of
     /// the "dns_server" specification.

src/lib/dhcpsrv/tests/d2_client_unittest.cc

@@ -689,8 +689,8 @@ TEST(D2ClientMgr, generateFqdn) {
     ASSERT_NO_THROW(mgr.setD2ClientConfig(cfg));
 
     // Verify names generate properly with a disabled configuration.
-    EXPECT_EQ("myhost-192-0-2-75.example.com.", mgr.generateFqdn(v4address,true));
-    EXPECT_EQ("myhost-2001-db8--2.example.com.", mgr.generateFqdn(v6address,true));
+    EXPECT_EQ("myhost-192-0-2-75.", mgr.generateFqdn(v4address,true));
+    EXPECT_EQ("myhost-2001-db8--2.", mgr.generateFqdn(v6address,true));
 }
 
 /// @brief Tests adjustDomainName template method with Option4ClientFqdn

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -1036,7 +1036,7 @@ TEST_F(ParseConfigTest, validD2Config) {
 /// false only.
 TEST_F(ParseConfigTest, validDisabledD2Config) {
 
-    // Configuration string.  This contains a set of valid libraries.
+    // Configuration string.  This defines a disabled D2 client config.
     std::string config_str =
         "{ \"dhcp-ddns\" :"
         "    {"
@@ -1062,11 +1062,14 @@ TEST_F(ParseConfigTest, validDisabledD2Config) {
 /// default values
 TEST_F(ParseConfigTest, parserDefaultsD2Config) {
 
-    // Configuration string.  This contains a set of valid libraries.
+    // Configuration string.  This defines an enabled D2 client config
+    // with the mandatory parameter in such a case, all other parameters
+    // are optional and their default values will be used.
     std::string config_str =
         "{ \"dhcp-ddns\" :"
         "    {"
-        "     \"enable-updates\" : true"
+        "     \"enable-updates\" : true, "
+        "     \"qualifying-suffix\" : \"test.suffix.\" "
         "    }"
         "}";
 
@@ -1100,7 +1103,7 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) {
               d2_client_config->getReplaceClientName());
     EXPECT_EQ(D2ClientConfig::DFT_GENERATED_PREFIX,
               d2_client_config->getGeneratedPrefix());
-    EXPECT_EQ(D2ClientConfig::DFT_QUALIFYING_SUFFIX,
+    EXPECT_EQ("test.suffix.",
               d2_client_config->getQualifyingSuffix());
 }
 
@@ -1108,11 +1111,17 @@ TEST_F(ParseConfigTest, parserDefaultsD2Config) {
 /// @brief Check various invalid D2 client configurations.
 TEST_F(ParseConfigTest, invalidD2Config) {
     std::string invalid_configs[] = {
-        // Must supply at lease enable-updates
+        // Must supply at least enable-updates
         "{ \"dhcp-ddns\" :"
         "    {"
         "    }"
         "}",
+        // Must supply qualifying-suffix when updates are enabled
+        "{ \"dhcp-ddns\" :"
+        "    {"
+        "     \"enable-updates\" : true"
+        "    }"
+        "}",
         // Invalid server ip value
         "{ \"dhcp-ddns\" :"
         "    {"