[295-min-max-lease-time-configuration-options] Improved lifetime bound checks

diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 240d1e6b92227aa4ae88d317541b23b418832d8f..d03355c450d8f63ac8b5b276059ed803fffbd45e 100644 (file)
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -873,7 +873,7 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) {
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
     string expected = "subnet configuration failed: "
         "the value of min-valid-lifetime (2000) is not "
-        "less than max-valid-lifetime (1000)";
+        "less than (default) valid-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -885,6 +885,9 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) {
 
     ASSERT_NO_THROW(json = parseDHCP4(too_large));
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of (default) valid-lifetime (2000) is not "
+        "less than max-valid-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -916,6 +919,21 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) {
         "the value of (default) valid-lifetime (5000) is not "
         "between min-valid-lifetime (1000) and max-valid-lifetime (4000)";
     checkResult(status, 1, expected);
+    resetConfiguration();
+
+    string crossed =  "{ " + genIfaceConfig() + "," +
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
+        "    \"subnet\": \"192.0.2.0/24\" } ],"
+        "\"valid-lifetime\": 1500, \"min-valid-lifetime\": 2000, "
+        "\"max-valid-lifetime\": 1000 }";
+
+    ASSERT_NO_THROW(json = parseDHCP4(crossed));
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of min-valid-lifetime (2000) is not "
+        "less than max-valid-lifetime (1000)";
+    checkResult(status, 1, expected);
 }
 
 /// Check that the renew-timer doesn't have to be specified, in which case

diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index e458c23c63b2512296bf425e58d5c354b39b6f35..a35bdb1939b97688d4d88202c800a6fd7588e988 100644 (file)
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -1025,7 +1025,7 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) {
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
     string expected = "subnet configuration failed: "
         "the value of min-valid-lifetime (2000) is not "
-        "less than max-valid-lifetime (1000)";
+        "less than (default) valid-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -1037,6 +1037,9 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) {
 
     ASSERT_NO_THROW(json = parseDHCP6(too_large));
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of (default) valid-lifetime (2000) is not "
+        "less than max-valid-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -1068,6 +1071,20 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) {
         "the value of (default) valid-lifetime (5000) is not "
         "between min-valid-lifetime (1000) and max-valid-lifetime (4000)";
     checkResult(status, 1, expected);
+    resetConfiguration();
+
+    string crossed =  "{ " + genIfaceConfig() + "," +
+        "\"subnet6\": [ { "
+        "    \"pools\": [ { \"pool\": \"2001:db8::/64\" } ],"
+        "    \"subnet\": \"2001:db8::/32\" } ],"
+        "\"valid-lifetime\": 1500, \"min-valid-lifetime\": 2000, "
+        "\"max-valid-lifetime\": 1000 }";
+    ASSERT_NO_THROW(json = parseDHCP6(crossed));
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of min-valid-lifetime (2000) is not "
+        "less than max-valid-lifetime (1000)";
+    checkResult(status, 1, expected);
 }
 
 /// Check that preferred-lifetime must be between min-preferred-lifetime and
@@ -1088,7 +1105,7 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) {
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
     string expected = "subnet configuration failed: "
         "the value of min-preferred-lifetime (2000) is not "
-        "less than max-preferred-lifetime (1000)";
+        "less than (default) preferred-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -1100,6 +1117,9 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) {
 
     ASSERT_NO_THROW(json = parseDHCP6(too_large));
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of (default) preferred-lifetime (2000) is not "
+        "less than max-preferred-lifetime (1000)";
     checkResult(status, 1, expected);
     resetConfiguration();
 
@@ -1131,6 +1151,20 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) {
         "the value of (default) preferred-lifetime (5000) is not between "
         "min-preferred-lifetime (1000) and max-preferred-lifetime (4000)";
     checkResult(status, 1, expected);
+    resetConfiguration();
+
+    string crossed =  "{ " + genIfaceConfig() + "," +
+        "\"subnet6\": [ { "
+        "    \"pools\": [ { \"pool\": \"2001:db8::/64\" } ],"
+        "    \"subnet\": \"2001:db8::/32\" } ],"
+        "\"preferred-lifetime\": 1500, \"min-preferred-lifetime\": 2000, "
+        "\"max-preferred-lifetime\": 1000 }";
+    ASSERT_NO_THROW(json = parseDHCP6(crossed));
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    expected = "subnet configuration failed: "
+        "the value of min-preferred-lifetime (2000) is not "
+        "less than max-preferred-lifetime (1000)";
+    checkResult(status, 1, expected);
 }
 
 /// The goal of this test is to verify if configuration without any

diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc
index 6614a604865a83a81e4d682722154c2d45ae7b1b..a83614defef7819e2c1038e7343d18f7891f48c1 100644 (file)
--- a/src/lib/dhcpsrv/parsers/base_network_parser.cc
+++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc
@@ -69,9 +69,21 @@ BaseNetworkParser::parseLifetime(const ConstElementPtr& scope,
     }
     // Check that min <= max.
     if (min_value > max_value) {
-        isc_throw(DhcpConfigError, "the value of min-" << name << " ("
-                  << min_value << ") is not less than max-" << name << " ("
-                  << max_value << ")");
+        if (has_min && has_max) {
+            isc_throw(DhcpConfigError, "the value of min-" << name << " ("
+                      << min_value << ") is not less than max-" << name << " ("
+                      << max_value << ")");
+        } else if (has_min) {
+            // Only min and default so min > default.
+            isc_throw(DhcpConfigError, "the value of min-" << name << " ("
+                      << min_value << ") is not less than (default) " << name
+                      << " (" << value << ")");
+        } else {
+            // Only default and max so default > max.
+            isc_throw(DhcpConfigError, "the value of (default) " << name
+                      << " (" << value << ") is not less than max-" << name
+                      << " (" << max_value << ")");
+        }
     }
     // Check that value is between min and max.
     if ((value < min_value) || (value > max_value)) {