[#229,!140] option-def is not accepted in class parser for v6 case.

Per review comment.

diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
index 49b871f2dd396eb75fcba834dd32410a89e96aaa..a9a8c92dd6bb37dbf691d8523ef070ef874e16fc 100644 (file)
--- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
+++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
@@ -228,12 +228,16 @@ ClientClassDefParser::checkParametersSupported(const ConstElementPtr& class_def_
     }
 
     // Common v4 and v6 parameters supported for the client class.
-    static std::set<std::string> supported_params = { "name", "test", "option-def",
-                                                      "option-data", "user-context",
+    static std::set<std::string> supported_params = { "name",
+                                                      "test",
+                                                      "option-data",
+                                                      "user-context",
                                                       "only-if-required" };
 
     // The v4 client class supports additional parmeters.
-    static std::set<std::string> supported_params_v4 = { "next-server", "server-hostname",
+    static std::set<std::string> supported_params_v4 = { "option-def",
+                                                         "next-server",
+                                                         "server-hostname",
                                                          "boot-file-name" };
 
     // Iterate over the specified parameters and check if they are all supported.

diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
index 93c0f3ce6d814d0dc7ddccff84ba6749879cf5e7..0f249311500b9546587458a6c3855f96183c0dd0 100644 (file)
--- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
@@ -126,6 +126,21 @@ protected:
         // Return NULL if for some reason the class doesn't exist.
         return (ClientClassDefPtr());
     }
+
+    /// @brief Test that client class parser throws when unspported parameter
+    /// is specfied.
+    ///
+    /// @param config JSON string containing the client class configuration.
+    /// @param family The address family indicating whether the DHCPv4 or
+    /// DHCPv6 client class is parsed.
+    void testClassParamsUnsupported(const std::string& config,
+                                    const uint16_t family) {
+        ElementPtr config_element = Element::fromJSON(config);
+
+        ClientClassDefParser parser;
+        EXPECT_THROW(parser.checkParametersSupported(config_element, family),
+                     DhcpConfigError);
+    }
 };
 
 /// @brief Test fixture class for @c ClientClassDefListParser.
@@ -299,7 +314,6 @@ TEST_F(ClientClassDefParserTest, checkAllSupported6) {
         "{\n"
         "    \"name\": \"foo\","
         "    \"test\": \"member('ALL')\","
-        "    \"option-def\": [ ],\n"
         "    \"option-data\": [ ],\n"
         "    \"user-context\": { },\n"
         "    \"only-if-required\": false\n"
@@ -315,24 +329,67 @@ TEST_F(ClientClassDefParserTest, checkAllSupported6) {
 // are supported throws if DHCPv4 specific parameters are specified for the
 // DHCPv6 client class.
 TEST_F(ClientClassDefParserTest, checkParams4Unsupported6) {
-    std::string cfg_text =
-        "{\n"
-        "    \"name\": \"foo\","
-        "    \"test\": \"member('ALL')\","
-        "    \"option-def\": [ ],\n"
-        "    \"option-data\": [ ],\n"
-        "    \"user-context\": { },\n"
-        "    \"only-if-required\": false,\n"
-        "    \"next-server\": \"192.0.2.3\",\n"
-        "    \"server-hostname\": \"myhost\",\n"
-        "    \"boot-file-name\": \"efi\""
-        "}\n";
+    std::string cfg_text;
 
-    ElementPtr config_element = Element::fromJSON(cfg_text);
+    {
+        SCOPED_TRACE("option-def");
+        cfg_text =
+            "{\n"
+            "    \"name\": \"foo\","
+            "    \"test\": \"member('ALL')\","
+            "    \"option-def\": [ ],\n"
+            "    \"option-data\": [ ],\n"
+            "    \"user-context\": { },\n"
+            "    \"only-if-required\": false\n"
+            "}\n";
+
+        testClassParamsUnsupported(cfg_text, AF_INET6);
+    }
 
-    ClientClassDefParser parser;
-    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET6),
-                 DhcpConfigError);
+    {
+        SCOPED_TRACE("next-server");
+        cfg_text =
+            "{\n"
+            "    \"name\": \"foo\","
+            "    \"test\": \"member('ALL')\","
+            "    \"option-data\": [ ],\n"
+            "    \"user-context\": { },\n"
+            "    \"only-if-required\": false,\n"
+            "    \"next-server\": \"192.0.2.3\"\n"
+            "}\n";
+
+        testClassParamsUnsupported(cfg_text, AF_INET6);
+    }
+
+    {
+        SCOPED_TRACE("server-hostname");
+        cfg_text =
+            "{\n"
+            "    \"name\": \"foo\","
+            "    \"test\": \"member('ALL')\","
+            "    \"option-data\": [ ],\n"
+            "    \"user-context\": { },\n"
+            "    \"only-if-required\": false,\n"
+            "    \"server-hostname\": \"myhost\"\n"
+            "}\n";
+
+        testClassParamsUnsupported(cfg_text, AF_INET6);
+    }
+
+    {
+        SCOPED_TRACE("boot-file-name");
+        cfg_text =
+            "{\n"
+            "    \"name\": \"foo\","
+            "    \"test\": \"member('ALL')\","
+            "    \"option-data\": [ ],\n"
+            "    \"user-context\": { },\n"
+            "    \"only-if-required\": false,\n"
+            "    \"boot-file-name\": \"efi\""
+            "}\n";
+
+        testClassParamsUnsupported(cfg_text, AF_INET6);
+    }
 }
 
 // Verifies that the function checking if specified DHCPv4 client class
@@ -344,11 +401,7 @@ TEST_F(ClientClassDefParserTest, checkParams4Unsupported) {
         "    \"unsupported\": \"member('ALL')\""
         "}\n";
 
-    ElementPtr config_element = Element::fromJSON(cfg_text);
-
-    ClientClassDefParser parser;
-    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET),
-                 DhcpConfigError);
+    testClassParamsUnsupported(cfg_text, AF_INET);
 }
 
 // Verifies that the function checking if specified DHCPv6 client class
@@ -360,11 +413,7 @@ TEST_F(ClientClassDefParserTest, checkParams6Unsupported) {
         "    \"unsupported\": \"member('ALL')\""
         "}\n";
 
-    ElementPtr config_element = Element::fromJSON(cfg_text);
-
-    ClientClassDefParser parser;
-    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET6),
-                 DhcpConfigError);
+    testClassParamsUnsupported(cfg_text, AF_INET6);
 }
 
 // Verifies you can create a class with only a name