[#2961] Chased last auto-gen ids

diff --git a/ChangeLog b/ChangeLog
index 197b7c51c11e8357604cad90cafe0a457743c24f..2e76a628c726e7dd585c443ab71db0c564901a35 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2232.  [func]*         fdupont
+       Configurations which contain subnets without explicit
+       subnet identifiers (without "id" entry) are now rejected:
+       subnet identifiers are no longer auto-generated.
+       (Gitlab #2961)
+
 2231.  [func]          fdupont
        The "ip-address" parameter in the "relay" element
        is no longer supported: it was replaced by

diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 473a32e6c00ac071a415722a40ed7dd17643482c..797305d5a9475f7e4b4922bb03fc615a46187ed1 100644 (file)
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -1152,65 +1152,6 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
     EXPECT_EQ(1, subnet->getID());
 }
 
-// Goal of this test is to verify that multiple subnets get unique
-// subnet-ids. Also, test checks that it's possible to do reconfiguration
-// multiple times.
-TEST_F(Dhcp4ParserTest, multipleSubnets) {
-    ConstElementPtr x;
-    // Collection of four subnets for which subnet ids should be
-    // autogenerated - ids are unspecified or set to 0.
-    string config = "{ " + genIfaceConfig() + "," +
-        "\"rebind-timer\": 2000, "
-        "\"renew-timer\": 1000, "
-        "\"subnet4\": [ { "
-        "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
-        "    \"subnet\": \"192.0.2.0/24\" "
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"192.0.3.101 - 192.0.3.150\" } ],"
-        "    \"subnet\": \"192.0.3.0/24\", "
-        "    \"id\": 0 "
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"192.0.4.101 - 192.0.4.150\" } ],"
-        "    \"subnet\": \"192.0.4.0/24\" "
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"192.0.5.101 - 192.0.5.150\" } ],"
-        "    \"subnet\": \"192.0.5.0/24\" "
-        " } ],"
-        "\"valid-lifetime\": 4000 }";
-
-    ConstElementPtr json;
-    ASSERT_NO_THROW(json = parseDHCP4(config));
-
-    int cnt = 0; // Number of reconfigurations
-
-    do {
-        EXPECT_NO_THROW(x = Dhcpv4SrvTest::configure(*srv_, json));
-        checkResult(x, 0);
-
-        CfgMgr::instance().commit();
-
-        const Subnet4Collection* subnets =
-            CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
-        ASSERT_TRUE(subnets);
-        ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
-
-        // Check subnet-ids of each subnet (it should be monotonously increasing)
-        auto subnet = subnets->begin();
-        EXPECT_EQ(1, (*subnet)->getID());
-        EXPECT_EQ(2, (*++subnet)->getID());
-        EXPECT_EQ(3, (*++subnet)->getID());
-        EXPECT_EQ(4, (*++subnet)->getID());
-
-        // Repeat reconfiguration process 10 times and check that the subnet-id
-        // is set to the same value. Technically, just two iterations would be
-        // sufficient, but it's nice to have a test that exercises reconfiguration
-        // a bit.
-    } while (++cnt < 10);
-}
-
 // This test checks that it is possible to assign arbitrary ids for subnets.
 TEST_F(Dhcp4ParserTest, multipleSubnetsExplicitIDs) {
     ConstElementPtr x;
@@ -7834,26 +7775,31 @@ TEST_F(Dhcp4ParserTest, storeDdnsConflictResolutionMode) {
         "\"renew-timer\": 1000, "
         "\"subnet4\": [ "
         "{"
+        "    \"id\": 1,"
         "    \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"check-with-dhcid\","
         "    \"subnet\": \"192.0.2.0/24\""
         "},"
         "{"
+        "    \"id\": 2,"
         "    \"pools\": [ { \"pool\": \"192.0.3.1 - 192.0.3.100\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\","
         "    \"subnet\": \"192.0.3.0/24\""
         "},"
         "{"
+        "    \"id\": 3,"
         "    \"pools\": [ { \"pool\": \"192.0.4.1 - 192.0.4.100\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\","
         "    \"subnet\": \"192.0.4.0/24\""
         "},"
         "{"
+        "    \"id\": 4,"
         "    \"pools\": [ { \"pool\": \"192.0.5.1 - 192.0.5.100\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\","
         "    \"subnet\": \"192.0.5.0/24\""
         "},"
         "{"
+        "    \"id\": 5,"
         "    \"pools\": [ { \"pool\": \"192.0.6.1 - 192.0.6.100\" } ],"
         "    \"subnet\": \"192.0.6.0/24\""
         "} ],"

diff --git a/src/bin/dhcp4/tests/get_config_unittest.cc b/src/bin/dhcp4/tests/get_config_unittest.cc
index 2f483876b88e6c799f2ce70cadb4502b4a13ca74..13fe908335bb707fca954a553a3097b01bd93862 100644 (file)
--- a/src/bin/dhcp4/tests/get_config_unittest.cc
+++ b/src/bin/dhcp4/tests/get_config_unittest.cc
@@ -2411,6 +2411,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "        \"subnet4\": [\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"check-with-dhcid\",\n"
+"                \"id\": 1,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"192.0.2.1 - 192.0.2.100\"\n"
@@ -2420,6 +2421,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\",\n"
+"                \"id\": 2,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"192.0.3.1 - 192.0.3.100\"\n"
@@ -2429,6 +2431,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\",\n"
+"                \"id\": 3,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"192.0.4.1 - 192.0.4.100\"\n"
@@ -2438,6 +2441,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\",\n"
+"                \"id\": 4,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"192.0.5.1 - 192.0.5.100\"\n"
@@ -2446,6 +2450,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "                \"subnet\": \"192.0.5.0/24\"\n"
 "            },\n"
 "            {\n"
+"                \"id\": 5,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"192.0.6.1 - 192.0.6.100\"\n"

diff --git a/src/bin/dhcp4/tests/simple_parser4_unittest.cc b/src/bin/dhcp4/tests/simple_parser4_unittest.cc
index 62d12c4e0e8966b3d444b31e88d9c0819e864444..3a661ba79ab4f92e58949c280ecaad0463502708 100644 (file)
--- a/src/bin/dhcp4/tests/simple_parser4_unittest.cc
+++ b/src/bin/dhcp4/tests/simple_parser4_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2022 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2024 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -157,8 +157,8 @@ TEST_F(SimpleParser4Test, subnetDefaults4) {
     ConstElementPtr subnet = subnets->get(0);
     ASSERT_TRUE(subnet);
 
-    // we should have "id" parameter with the default value of 0 added for us.
-    checkIntegerValue(subnet, "id", 0);
+    // no "id" where added.
+    ASSERT_FALSE(subnet->get("id"));
 }
 
 // This test checks if the parameters in option-data are assigned default values

diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index c4ed0cfe31105d75c2dd440aa46e038314dc6c6d..1a7fa160ad62dafbac0d11b684ebe61b535901fd 100644 (file)
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -1417,64 +1417,6 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) {
     EXPECT_EQ(1, subnet->getID());
 }
 
-// This test checks that multiple subnets can be defined and handled properly.
-TEST_F(Dhcp6ParserTest, multipleSubnets) {
-    ConstElementPtr x;
-    // Collection of four subnets for which ids should be autogenerated
-    // - ids are unspecified or set to 0.
-    string config = "{ " + genIfaceConfig() + ","
-        "\"preferred-lifetime\": 3000,"
-        "\"rebind-timer\": 2000, "
-        "\"renew-timer\": 1000, "
-        "\"subnet6\": [ { "
-        "    \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ],"
-        "    \"subnet\": \"2001:db8:1::/64\" "
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"2001:db8:2::/80\" } ],"
-        "    \"subnet\": \"2001:db8:2::/64\", "
-        "    \"id\": 0"
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"2001:db8:3::/80\" } ],"
-        "    \"subnet\": \"2001:db8:3::/64\" "
-        " },"
-        " {"
-        "    \"pools\": [ { \"pool\": \"2001:db8:4::/80\" } ],"
-        "    \"subnet\": \"2001:db8:4::/64\" "
-        " } ],"
-        "\"valid-lifetime\": 4000 }";
-
-    int cnt = 0; // Number of reconfigurations
-
-    ConstElementPtr json;
-    ASSERT_NO_THROW(json = parseDHCP6(config));
-
-    do {
-        EXPECT_NO_THROW(x = Dhcpv6SrvTest::configure(srv_, json));
-        checkResult(x, 0);
-
-        CfgMgr::instance().commit();
-
-        const Subnet6Collection* subnets =
-            CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
-        ASSERT_TRUE(subnets);
-        ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
-
-        // Check subnet-ids of each subnet (it should be monotonously increasing)
-        auto subnet = subnets->begin();
-        EXPECT_EQ(1, (*subnet)->getID());
-        EXPECT_EQ(2, (*++subnet)->getID());
-        EXPECT_EQ(3, (*++subnet)->getID());
-        EXPECT_EQ(4, (*++subnet)->getID());
-
-        // Repeat reconfiguration process 10 times and check that the subnet-id
-        // is set to the same value. Technically, just two iterations would be
-        // sufficient, but it's nice to have a test that exercises reconfiguration
-        // a bit.
-    } while (++cnt < 10);
-}
-
 // This test checks that it is possible to assign arbitrary ids for subnets.
 TEST_F(Dhcp6ParserTest, multipleSubnetsExplicitIDs) {
     ConstElementPtr x;
@@ -8416,26 +8358,31 @@ TEST_F(Dhcp6ParserTest, storeDdnsConflictResolutionMode) {
         "\"renew-timer\": 1000, "
         "\"subnet6\": [ "
         "{ "
+        "    \"id\": 1,"
         "    \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::ffff\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"check-with-dhcid\","
         "    \"subnet\": \"2001:db8:1::/64\""
         "},"
         "{ "
+        "    \"id\": 2,"
         "    \"pools\": [ { \"pool\": \"2001:db8:2::1 - 2001:db8:2::ffff\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\","
         "    \"subnet\": \"2001:db8:2::/64\""
         "},"
         "{"
+        "    \"id\": 3,"
         "    \"pools\": [ { \"pool\": \"2001:db8:3::1 - 2001:db8:3::ffff\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\","
         "    \"subnet\": \"2001:db8:3::/64\""
         "},"
         "{"
+        "    \"id\": 4,"
         "    \"pools\": [ { \"pool\": \"2001:db8:4::1 - 2001:db8:4::ffff\" } ],"
         "    \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\","
         "    \"subnet\": \"2001:db8:4::/64\""
         "},"
         "{"
+        "    \"id\": 5,"
         "    \"pools\": [ { \"pool\": \"2001:db8:5::1 - 2001:db8:5::ffff\" } ],"
         "    \"subnet\": \"2001:db8:5::/64\""
         "} ],"

diff --git a/src/bin/dhcp6/tests/get_config_unittest.cc b/src/bin/dhcp6/tests/get_config_unittest.cc
index 2c0cace701217968bb2a1dea659c9a395aad5884..343434b9b6977f1f520ca77df603bdd52fb2d65b 100644 (file)
--- a/src/bin/dhcp6/tests/get_config_unittest.cc
+++ b/src/bin/dhcp6/tests/get_config_unittest.cc
@@ -2118,6 +2118,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "        \"subnet6\": [\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"check-with-dhcid\",\n"
+"                \"id\": 1,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"2001:db8:1::1 - 2001:db8:1::ffff\"\n"
@@ -2127,6 +2128,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\",\n"
+"                \"id\": 2,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"2001:db8:2::1 - 2001:db8:2::ffff\"\n"
@@ -2136,6 +2138,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\",\n"
+"                \"id\": 3,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"2001:db8:3::1 - 2001:db8:3::ffff\"\n"
@@ -2145,6 +2148,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "            },\n"
 "            {\n"
 "                \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\",\n"
+"                \"id\": 4,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"2001:db8:4::1 - 2001:db8:4::ffff\"\n"
@@ -2153,6 +2157,7 @@ const char* EXTRACTED_CONFIGS[] = {
 "                \"subnet\": \"2001:db8:4::/64\"\n"
 "            },\n"
 "            {\n"
+"                \"id\": 5,\n"
 "                \"pools\": [\n"
 "                    {\n"
 "                        \"pool\": \"2001:db8:5::1 - 2001:db8:5::ffff\"\n"

diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc b/src/bin/dhcp6/tests/simple_parser6_unittest.cc
index a7ebb65eb997c04d353b29e7d9bccb264ea80135..a6252112966105fd5a6ed6163c3e008054fee14e 100644 (file)
--- a/src/bin/dhcp6/tests/simple_parser6_unittest.cc
+++ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc
@@ -204,8 +204,8 @@ TEST_F(SimpleParser6Test, subnetDefaults6) {
     ConstElementPtr subnet = subnets->get(0);
     ASSERT_TRUE(subnet);
 
-    // we should have "id" parameter with the default value of 0 added for us.
-    checkIntegerValue(subnet, "id", 0);
+    // no "id" where added.
+    ASSERT_FALSE(subnet->get("id"));
 }
 
 // This test checks if the parameters in option-data are assigned default values

diff --git a/src/bin/keactrl/tests/keactrl_tests.sh.in b/src/bin/keactrl/tests/keactrl_tests.sh.in
index fe7644265f325969a4a3c4866806c01870797d58..6c7c17b47b429ab5de572b3747311d25f2611673 100644 (file)
--- a/src/bin/keactrl/tests/keactrl_tests.sh.in
+++ b/src/bin/keactrl/tests/keactrl_tests.sh.in
@@ -84,6 +84,7 @@ dhcp4_config="{
         },
         \"subnet4\": [
         {
+            \"id\": 1,
             \"subnet\": \"10.0.0.0/24\",
             \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]
         } ],
@@ -123,6 +124,7 @@ dhcp6_config="{
         },
         \"subnet6\": [
         {
+            \"id\": 1,
             \"subnet\": \"2001:db8:1::/64\",
             \"pools\": [ { \"pool\": \"2001:db8:1::10-2001:db8:1::100\" } ]
         } ],

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.cc b/src/lib/dhcpsrv/dhcpsrv_messages.cc
index 855a80af703e5d05e9b95a72ba104ff6b6a79fe3..4d4a57f2e91b4c37c924c24c5ca79f6683cd7c03 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.cc
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.cc
@@ -54,7 +54,6 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS = "DHCPSRV_CFGMGR_US
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR = "DHCPSRV_CFGMGR_USE_ALLOCATOR";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST = "DHCPSRV_CFGMGR_USE_UNICAST";
 extern const isc::log::MessageID DHCPSRV_CLOSE_DB = "DHCPSRV_CLOSE_DB";
-extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID = "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID";
 extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL = "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL";
 extern const isc::log::MessageID DHCPSRV_DEPRECATED = "DHCPSRV_DEPRECATED";
 extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET = "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET";
@@ -340,7 +339,6 @@ const char* values[] = {
     "DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3",
     "DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2",
     "DHCPSRV_CLOSE_DB", "closing currently open %1 database",
-    "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID", "a subnet was configured without an id: %1",
     "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL", "ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it",
     "DHCPSRV_DEPRECATED", "This configuration is using a deprecated feature: %1",
     "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET", "received bad DHCPv4o6 packet: %1",

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.h b/src/lib/dhcpsrv/dhcpsrv_messages.h
index dd467ef99e8b11adef4cc0551902a02e8ab63bb9..40296633dcc7286b2d3acb3269d6588a94f7f611 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.h
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.h
@@ -55,7 +55,6 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST;
 extern const isc::log::MessageID DHCPSRV_CLOSE_DB;
-extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID;
 extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL;
 extern const isc::log::MessageID DHCPSRV_DEPRECATED;
 extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET;

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index 13400c778da4866e02600a7ef34f6be2109c0e31..2398daff3611c39bbd09d0825ef3326f3b38159c 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -281,13 +281,6 @@ the database access parameters are changed: in the latter case, the
 server closes the currently open database, and opens a database using
 the new parameters.
 
-% DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID a subnet was configured without an id: %1
-A warning message issued when a subnet was configured with a zero or without
-an id, causing the server to auto-generate it. Using auto-generated subnet
-ids is now deprecated. Each configured subnet should have an explicit subnet id
-specified with the "id" entry. The sole argument of this warning message contains
-a subnet prefix.
-
 % DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it
 A debug message issued when the DDNS TTL value calculated using the
 ddns-ttl-percent is zero.  Kea will ignore the value and calculate