From: Francis Dupont Date: Sat, 11 May 2024 22:15:12 +0000 (+0200) Subject: [#2961] Chased last auto-gen ids X-Git-Tag: Kea-2.6.0~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=50c50a313282f6a9b45331830beec04332ca99e3;p=thirdparty%2Fkea.git [#2961] Chased last auto-gen ids --- diff --git a/ChangeLog b/ChangeLog index 197b7c51c1..2e76a628c7 100644 --- 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 473a32e6c0..797305d5a9 100644 --- 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 2f483876b8..13fe908335 100644 --- 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 62d12c4e0e..3a661ba79a 100644 --- 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 c4ed0cfe31..1a7fa160ad 100644 --- 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 2c0cace701..343434b9b6 100644 --- 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 a7ebb65eb9..a625211296 100644 --- 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 fe7644265f..6c7c17b47b 100644 --- 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 855a80af70..4d4a57f2e9 100644 --- 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 dd467ef99e..40296633dc 100644 --- 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 13400c778d..2398daff36 100644 --- 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