Skip to content

Commit 50c50a3

Browse files
fxdupontfxdupont
committed
[#2961] Chased last auto-gen ids
1 parent 5b60e5f commit 50c50a3

11 files changed

+33
-132
lines changed

‎ChangeLog

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
2232. [func]* fdupont
2+
Configurations which contain subnets without explicit
3+
subnet identifiers (without "id" entry) are now rejected:
4+
subnet identifiers are no longer auto-generated.
5+
(Gitlab #2961)
6+
17
2231. [func] fdupont
28
The "ip-address" parameter in the "relay" element
39
is no longer supported: it was replaced by

‎src/bin/dhcp4/tests/config_parser_unittest.cc

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,65 +1152,6 @@ TEST_F(Dhcp4ParserTest, subnetGlobalDefaults) {
11521152
EXPECT_EQ(1, subnet->getID());
11531153
}
11541154

1155-
// Goal of this test is to verify that multiple subnets get unique
1156-
// subnet-ids. Also, test checks that it's possible to do reconfiguration
1157-
// multiple times.
1158-
TEST_F(Dhcp4ParserTest, multipleSubnets) {
1159-
ConstElementPtr x;
1160-
// Collection of four subnets for which subnet ids should be
1161-
// autogenerated - ids are unspecified or set to 0.
1162-
string config = "{ " + genIfaceConfig() + "," +
1163-
"\"rebind-timer\": 2000, "
1164-
"\"renew-timer\": 1000, "
1165-
"\"subnet4\": [ { "
1166-
" \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
1167-
" \"subnet\": \"192.0.2.0/24\" "
1168-
" },"
1169-
" {"
1170-
" \"pools\": [ { \"pool\": \"192.0.3.101 - 192.0.3.150\" } ],"
1171-
" \"subnet\": \"192.0.3.0/24\", "
1172-
" \"id\": 0 "
1173-
" },"
1174-
" {"
1175-
" \"pools\": [ { \"pool\": \"192.0.4.101 - 192.0.4.150\" } ],"
1176-
" \"subnet\": \"192.0.4.0/24\" "
1177-
" },"
1178-
" {"
1179-
" \"pools\": [ { \"pool\": \"192.0.5.101 - 192.0.5.150\" } ],"
1180-
" \"subnet\": \"192.0.5.0/24\" "
1181-
" } ],"
1182-
"\"valid-lifetime\": 4000 }";
1183-
1184-
ConstElementPtr json;
1185-
ASSERT_NO_THROW(json = parseDHCP4(config));
1186-
1187-
int cnt = 0; // Number of reconfigurations
1188-
1189-
do {
1190-
EXPECT_NO_THROW(x = Dhcpv4SrvTest::configure(*srv_, json));
1191-
checkResult(x, 0);
1192-
1193-
CfgMgr::instance().commit();
1194-
1195-
const Subnet4Collection* subnets =
1196-
CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
1197-
ASSERT_TRUE(subnets);
1198-
ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
1199-
1200-
// Check subnet-ids of each subnet (it should be monotonously increasing)
1201-
auto subnet = subnets->begin();
1202-
EXPECT_EQ(1, (*subnet)->getID());
1203-
EXPECT_EQ(2, (*++subnet)->getID());
1204-
EXPECT_EQ(3, (*++subnet)->getID());
1205-
EXPECT_EQ(4, (*++subnet)->getID());
1206-
1207-
// Repeat reconfiguration process 10 times and check that the subnet-id
1208-
// is set to the same value. Technically, just two iterations would be
1209-
// sufficient, but it's nice to have a test that exercises reconfiguration
1210-
// a bit.
1211-
} while (++cnt < 10);
1212-
}
1213-
12141155
// This test checks that it is possible to assign arbitrary ids for subnets.
12151156
TEST_F(Dhcp4ParserTest, multipleSubnetsExplicitIDs) {
12161157
ConstElementPtr x;
@@ -7834,26 +7775,31 @@ TEST_F(Dhcp4ParserTest, storeDdnsConflictResolutionMode) {
78347775
"\"renew-timer\": 1000, "
78357776
"\"subnet4\": [ "
78367777
"{"
7778+
" \"id\": 1,"
78377779
" \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ],"
78387780
" \"ddns-conflict-resolution-mode\": \"check-with-dhcid\","
78397781
" \"subnet\": \"192.0.2.0/24\""
78407782
"},"
78417783
"{"
7784+
" \"id\": 2,"
78427785
" \"pools\": [ { \"pool\": \"192.0.3.1 - 192.0.3.100\" } ],"
78437786
" \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\","
78447787
" \"subnet\": \"192.0.3.0/24\""
78457788
"},"
78467789
"{"
7790+
" \"id\": 3,"
78477791
" \"pools\": [ { \"pool\": \"192.0.4.1 - 192.0.4.100\" } ],"
78487792
" \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\","
78497793
" \"subnet\": \"192.0.4.0/24\""
78507794
"},"
78517795
"{"
7796+
" \"id\": 4,"
78527797
" \"pools\": [ { \"pool\": \"192.0.5.1 - 192.0.5.100\" } ],"
78537798
" \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\","
78547799
" \"subnet\": \"192.0.5.0/24\""
78557800
"},"
78567801
"{"
7802+
" \"id\": 5,"
78577803
" \"pools\": [ { \"pool\": \"192.0.6.1 - 192.0.6.100\" } ],"
78587804
" \"subnet\": \"192.0.6.0/24\""
78597805
"} ],"

‎src/bin/dhcp4/tests/get_config_unittest.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2411,6 +2411,7 @@ const char* EXTRACTED_CONFIGS[] = {
24112411
" \"subnet4\": [\n"
24122412
" {\n"
24132413
" \"ddns-conflict-resolution-mode\": \"check-with-dhcid\",\n"
2414+
" \"id\": 1,\n"
24142415
" \"pools\": [\n"
24152416
" {\n"
24162417
" \"pool\": \"192.0.2.1 - 192.0.2.100\"\n"
@@ -2420,6 +2421,7 @@ const char* EXTRACTED_CONFIGS[] = {
24202421
" },\n"
24212422
" {\n"
24222423
" \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\",\n"
2424+
" \"id\": 2,\n"
24232425
" \"pools\": [\n"
24242426
" {\n"
24252427
" \"pool\": \"192.0.3.1 - 192.0.3.100\"\n"
@@ -2429,6 +2431,7 @@ const char* EXTRACTED_CONFIGS[] = {
24292431
" },\n"
24302432
" {\n"
24312433
" \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\",\n"
2434+
" \"id\": 3,\n"
24322435
" \"pools\": [\n"
24332436
" {\n"
24342437
" \"pool\": \"192.0.4.1 - 192.0.4.100\"\n"
@@ -2438,6 +2441,7 @@ const char* EXTRACTED_CONFIGS[] = {
24382441
" },\n"
24392442
" {\n"
24402443
" \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\",\n"
2444+
" \"id\": 4,\n"
24412445
" \"pools\": [\n"
24422446
" {\n"
24432447
" \"pool\": \"192.0.5.1 - 192.0.5.100\"\n"
@@ -2446,6 +2450,7 @@ const char* EXTRACTED_CONFIGS[] = {
24462450
" \"subnet\": \"192.0.5.0/24\"\n"
24472451
" },\n"
24482452
" {\n"
2453+
" \"id\": 5,\n"
24492454
" \"pools\": [\n"
24502455
" {\n"
24512456
" \"pool\": \"192.0.6.1 - 192.0.6.100\"\n"

‎src/bin/dhcp4/tests/simple_parser4_unittest.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (C) 2016-2022 Internet Systems Consortium, Inc. ("ISC")
1+
// Copyright (C) 2016-2024 Internet Systems Consortium, Inc. ("ISC")
22
//
33
// This Source Code Form is subject to the terms of the Mozilla Public
44
// License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -157,8 +157,8 @@ TEST_F(SimpleParser4Test, subnetDefaults4) {
157157
ConstElementPtr subnet = subnets->get(0);
158158
ASSERT_TRUE(subnet);
159159

160-
// we should have "id" parameter with the default value of 0 added for us.
161-
checkIntegerValue(subnet, "id", 0);
160+
// no "id" where added.
161+
ASSERT_FALSE(subnet->get("id"));
162162
}
163163

164164
// This test checks if the parameters in option-data are assigned default values

‎src/bin/dhcp6/tests/config_parser_unittest.cc

Lines changed: 5 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,64 +1417,6 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) {
14171417
EXPECT_EQ(1, subnet->getID());
14181418
}
14191419

1420-
// This test checks that multiple subnets can be defined and handled properly.
1421-
TEST_F(Dhcp6ParserTest, multipleSubnets) {
1422-
ConstElementPtr x;
1423-
// Collection of four subnets for which ids should be autogenerated
1424-
// - ids are unspecified or set to 0.
1425-
string config = "{ " + genIfaceConfig() + ","
1426-
"\"preferred-lifetime\": 3000,"
1427-
"\"rebind-timer\": 2000, "
1428-
"\"renew-timer\": 1000, "
1429-
"\"subnet6\": [ { "
1430-
" \"pools\": [ { \"pool\": \"2001:db8:1::/80\" } ],"
1431-
" \"subnet\": \"2001:db8:1::/64\" "
1432-
" },"
1433-
" {"
1434-
" \"pools\": [ { \"pool\": \"2001:db8:2::/80\" } ],"
1435-
" \"subnet\": \"2001:db8:2::/64\", "
1436-
" \"id\": 0"
1437-
" },"
1438-
" {"
1439-
" \"pools\": [ { \"pool\": \"2001:db8:3::/80\" } ],"
1440-
" \"subnet\": \"2001:db8:3::/64\" "
1441-
" },"
1442-
" {"
1443-
" \"pools\": [ { \"pool\": \"2001:db8:4::/80\" } ],"
1444-
" \"subnet\": \"2001:db8:4::/64\" "
1445-
" } ],"
1446-
"\"valid-lifetime\": 4000 }";
1447-
1448-
int cnt = 0; // Number of reconfigurations
1449-
1450-
ConstElementPtr json;
1451-
ASSERT_NO_THROW(json = parseDHCP6(config));
1452-
1453-
do {
1454-
EXPECT_NO_THROW(x = Dhcpv6SrvTest::configure(srv_, json));
1455-
checkResult(x, 0);
1456-
1457-
CfgMgr::instance().commit();
1458-
1459-
const Subnet6Collection* subnets =
1460-
CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
1461-
ASSERT_TRUE(subnets);
1462-
ASSERT_EQ(4, subnets->size()); // We expect 4 subnets
1463-
1464-
// Check subnet-ids of each subnet (it should be monotonously increasing)
1465-
auto subnet = subnets->begin();
1466-
EXPECT_EQ(1, (*subnet)->getID());
1467-
EXPECT_EQ(2, (*++subnet)->getID());
1468-
EXPECT_EQ(3, (*++subnet)->getID());
1469-
EXPECT_EQ(4, (*++subnet)->getID());
1470-
1471-
// Repeat reconfiguration process 10 times and check that the subnet-id
1472-
// is set to the same value. Technically, just two iterations would be
1473-
// sufficient, but it's nice to have a test that exercises reconfiguration
1474-
// a bit.
1475-
} while (++cnt < 10);
1476-
}
1477-
14781420
// This test checks that it is possible to assign arbitrary ids for subnets.
14791421
TEST_F(Dhcp6ParserTest, multipleSubnetsExplicitIDs) {
14801422
ConstElementPtr x;
@@ -8416,26 +8358,31 @@ TEST_F(Dhcp6ParserTest, storeDdnsConflictResolutionMode) {
84168358
"\"renew-timer\": 1000, "
84178359
"\"subnet6\": [ "
84188360
"{ "
8361+
" \"id\": 1,"
84198362
" \"pools\": [ { \"pool\": \"2001:db8:1::1 - 2001:db8:1::ffff\" } ],"
84208363
" \"ddns-conflict-resolution-mode\": \"check-with-dhcid\","
84218364
" \"subnet\": \"2001:db8:1::/64\""
84228365
"},"
84238366
"{ "
8367+
" \"id\": 2,"
84248368
" \"pools\": [ { \"pool\": \"2001:db8:2::1 - 2001:db8:2::ffff\" } ],"
84258369
" \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\","
84268370
" \"subnet\": \"2001:db8:2::/64\""
84278371
"},"
84288372
"{"
8373+
" \"id\": 3,"
84298374
" \"pools\": [ { \"pool\": \"2001:db8:3::1 - 2001:db8:3::ffff\" } ],"
84308375
" \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\","
84318376
" \"subnet\": \"2001:db8:3::/64\""
84328377
"},"
84338378
"{"
8379+
" \"id\": 4,"
84348380
" \"pools\": [ { \"pool\": \"2001:db8:4::1 - 2001:db8:4::ffff\" } ],"
84358381
" \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\","
84368382
" \"subnet\": \"2001:db8:4::/64\""
84378383
"},"
84388384
"{"
8385+
" \"id\": 5,"
84398386
" \"pools\": [ { \"pool\": \"2001:db8:5::1 - 2001:db8:5::ffff\" } ],"
84408387
" \"subnet\": \"2001:db8:5::/64\""
84418388
"} ],"

‎src/bin/dhcp6/tests/get_config_unittest.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,6 +2118,7 @@ const char* EXTRACTED_CONFIGS[] = {
21182118
" \"subnet6\": [\n"
21192119
" {\n"
21202120
" \"ddns-conflict-resolution-mode\": \"check-with-dhcid\",\n"
2121+
" \"id\": 1,\n"
21212122
" \"pools\": [\n"
21222123
" {\n"
21232124
" \"pool\": \"2001:db8:1::1 - 2001:db8:1::ffff\"\n"
@@ -2127,6 +2128,7 @@ const char* EXTRACTED_CONFIGS[] = {
21272128
" },\n"
21282129
" {\n"
21292130
" \"ddns-conflict-resolution-mode\": \"check-exists-with-dhcid\",\n"
2131+
" \"id\": 2,\n"
21302132
" \"pools\": [\n"
21312133
" {\n"
21322134
" \"pool\": \"2001:db8:2::1 - 2001:db8:2::ffff\"\n"
@@ -2136,6 +2138,7 @@ const char* EXTRACTED_CONFIGS[] = {
21362138
" },\n"
21372139
" {\n"
21382140
" \"ddns-conflict-resolution-mode\": \"no-check-without-dhcid\",\n"
2141+
" \"id\": 3,\n"
21392142
" \"pools\": [\n"
21402143
" {\n"
21412144
" \"pool\": \"2001:db8:3::1 - 2001:db8:3::ffff\"\n"
@@ -2145,6 +2148,7 @@ const char* EXTRACTED_CONFIGS[] = {
21452148
" },\n"
21462149
" {\n"
21472150
" \"ddns-conflict-resolution-mode\": \"no-check-with-dhcid\",\n"
2151+
" \"id\": 4,\n"
21482152
" \"pools\": [\n"
21492153
" {\n"
21502154
" \"pool\": \"2001:db8:4::1 - 2001:db8:4::ffff\"\n"
@@ -2153,6 +2157,7 @@ const char* EXTRACTED_CONFIGS[] = {
21532157
" \"subnet\": \"2001:db8:4::/64\"\n"
21542158
" },\n"
21552159
" {\n"
2160+
" \"id\": 5,\n"
21562161
" \"pools\": [\n"
21572162
" {\n"
21582163
" \"pool\": \"2001:db8:5::1 - 2001:db8:5::ffff\"\n"

‎src/bin/dhcp6/tests/simple_parser6_unittest.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ TEST_F(SimpleParser6Test, subnetDefaults6) {
204204
ConstElementPtr subnet = subnets->get(0);
205205
ASSERT_TRUE(subnet);
206206

207-
// we should have "id" parameter with the default value of 0 added for us.
208-
checkIntegerValue(subnet, "id", 0);
207+
// no "id" where added.
208+
ASSERT_FALSE(subnet->get("id"));
209209
}
210210

211211
// This test checks if the parameters in option-data are assigned default values

‎src/bin/keactrl/tests/keactrl_tests.sh.in

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ dhcp4_config="{
8484
},
8585
\"subnet4\": [
8686
{
87+
\"id\": 1,
8788
\"subnet\": \"10.0.0.0/24\",
8889
\"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]
8990
} ],
@@ -123,6 +124,7 @@ dhcp6_config="{
123124
},
124125
\"subnet6\": [
125126
{
127+
\"id\": 1,
126128
\"subnet\": \"2001:db8:1::/64\",
127129
\"pools\": [ { \"pool\": \"2001:db8:1::10-2001:db8:1::100\" } ]
128130
} ],

‎src/lib/dhcpsrv/dhcpsrv_messages.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS = "DHCPSRV_CFGMGR_US
5454
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR = "DHCPSRV_CFGMGR_USE_ALLOCATOR";
5555
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST = "DHCPSRV_CFGMGR_USE_UNICAST";
5656
extern const isc::log::MessageID DHCPSRV_CLOSE_DB = "DHCPSRV_CLOSE_DB";
57-
extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID = "DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID";
5857
extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL = "DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL";
5958
extern const isc::log::MessageID DHCPSRV_DEPRECATED = "DHCPSRV_DEPRECATED";
6059
extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET = "DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET";
@@ -340,7 +339,6 @@ const char* values[] = {
340339
"DHCPSRV_CFGMGR_USE_ALLOCATOR", "using the %1 allocator for %2 leases in subnet %3",
341340
"DHCPSRV_CFGMGR_USE_UNICAST", "listening on unicast address %1, on interface %2",
342341
"DHCPSRV_CLOSE_DB", "closing currently open %1 database",
343-
"DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID", "a subnet was configured without an id: %1",
344342
"DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL", "ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it",
345343
"DHCPSRV_DEPRECATED", "This configuration is using a deprecated feature: %1",
346344
"DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET", "received bad DHCPv4o6 packet: %1",

‎src/lib/dhcpsrv/dhcpsrv_messages.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ADDRESS;
5555
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_ALLOCATOR;
5656
extern const isc::log::MessageID DHCPSRV_CFGMGR_USE_UNICAST;
5757
extern const isc::log::MessageID DHCPSRV_CLOSE_DB;
58-
extern const isc::log::MessageID DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID;
5958
extern const isc::log::MessageID DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL;
6059
extern const isc::log::MessageID DHCPSRV_DEPRECATED;
6160
extern const isc::log::MessageID DHCPSRV_DHCP4O6_RECEIVED_BAD_PACKET;

‎src/lib/dhcpsrv/dhcpsrv_messages.mes

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,6 @@ the database access parameters are changed: in the latter case, the
281281
server closes the currently open database, and opens a database using
282282
the new parameters.
283283

284-
% DHCPSRV_CONFIGURED_SUBNET_WITHOUT_ID a subnet was configured without an id: %1
285-
A warning message issued when a subnet was configured with a zero or without
286-
an id, causing the server to auto-generate it. Using auto-generated subnet
287-
ids is now deprecated. Each configured subnet should have an explicit subnet id
288-
specified with the "id" entry. The sole argument of this warning message contains
289-
a subnet prefix.
290-
291284
% DHCPSRV_DDNS_TTL_PERCENT_TOO_SMALL ddns-ttl-percent %1 of lease lifetime %2 is too small, ignoring it
292285
A debug message issued when the DDNS TTL value calculated using the
293286
ddns-ttl-percent is zero. Kea will ignore the value and calculate

0 commit comments

Comments
 (0)