[#3476] Addressed review comments

modified: src/bin/admin/kea-admin.in src/lib/dhcp/option_data_types.h src/lib/dhcp/tests/option_definition_unittest.cc src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc src/share/database/scripts/mysql/dhcpdb_create.mysql src/share/database/scripts/mysql/upgrade_023_to_024.sh.in src/share/database/scripts/pgsql/dhcpdb_create.pgsql src/share/database/scripts/pgsql/upgrade_023_to_024.sh.in
isc-projects · Jul 18, 2024 · 48196f7 · 48196f7
1 parent 7d87bf4
commit 48196f7
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 35 deletions.
diff --git a/src/bin/admin/kea-admin.in b/src/bin/admin/kea-admin.in
@@ -371,12 +371,13 @@ mysql_upgrade() {
     printf "Verifying upgrade permissions for %s\n" "$db_user"
     mysql_can_create
 
-    for script in "${upgrade_scripts_dir}"/upgrade*.sh
+    upgrade_scripts=$(find "${upgrade_scripts_dir}" -type f -name 'upgrade_*.sh' | sort -V)
+    for script in ${upgrade_scripts}
     do
         echo "Processing $script file..."
         "${script}" --host="${db_host}" --user="${db_user}" \
         --password="${db_password}" "${db_name}" ${extra_arguments}
-    done | sort -V
+    done
 
     version=$(checked_mysql_version)
     printf 'Schema version reported after upgrade: %s\n' "${version}"
@@ -419,11 +420,12 @@ pgsql_upgrade() {
     export PGPASSWORD=$db_password
 
     upgrade_scripts=$(find "${upgrade_scripts_dir}" -type f -name 'upgrade_*.sh' | sort -V)
-    for script in ${upgrade_scripts}; do
+    for script in ${upgrade_scripts}
+    do
         echo "Processing $script file..."
         "${script}" -U "${db_user}" -h "${db_host}" \
         -d "${db_name}" ${extra_arguments}
-    done | sort -V
+    done
 
     version=$(checked_pgsql_version)
     printf 'Schema version reported after upgrade: %s\n' "${version}"

diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
@@ -35,34 +35,35 @@ class BadDataTypeCast : public Exception {
 
 /// @brief Data types of DHCP option fields.
 ///
-/// @warning The order of data types matters: OPT_UNKNOWN_TYPE
-/// must always be the last position. Also, OPT_RECORD_TYPE
-/// must be at last but one position. This is because some
-/// functions perform sanity checks on data type values using
-/// '>' operators, assuming that all values beyond the
-/// OPT_RECORD_TYPE are invalid.
-enum OptionDataType {
-    OPT_EMPTY_TYPE,
-    OPT_BINARY_TYPE,
-    OPT_BOOLEAN_TYPE,
-    OPT_INT8_TYPE,
-    OPT_INT16_TYPE,
-    OPT_INT32_TYPE,
-    OPT_UINT8_TYPE,
-    OPT_UINT16_TYPE,
-    OPT_UINT32_TYPE,
-    OPT_ANY_ADDRESS_TYPE,
-    OPT_IPV4_ADDRESS_TYPE,
-    OPT_IPV6_ADDRESS_TYPE,
-    OPT_IPV6_PREFIX_TYPE,
-    OPT_PSID_TYPE,
-    OPT_STRING_TYPE,
-    OPT_TUPLE_TYPE,
-    OPT_FQDN_TYPE,
+/// @warning Do NOT alter existing values to add (or remove) new types.
+/// These values are stored by config backend.  Altering any existing
+/// values will produce code that is incompatiable with pre-existing data.
+/// Futhermore, the order of data types matters: OPT_UNKNOWN_TYPE
+/// must always be and OPT_RECORD_TYPE must be at second to last.
+/// This is because some functions perform sanity checks on data type
+/// values using '>' operators, assuming that all values beyond the
+enum OptionDataType : int {
+    OPT_EMPTY_TYPE          = 0,
+    OPT_BINARY_TYPE         = 1,
+    OPT_BOOLEAN_TYPE        = 2,
+    OPT_INT8_TYPE           = 3,
+    OPT_INT16_TYPE          = 4,
+    OPT_INT32_TYPE          = 5,
+    OPT_UINT8_TYPE          = 6,
+    OPT_UINT16_TYPE         = 7,
+    OPT_UINT32_TYPE         = 8,
+    OPT_ANY_ADDRESS_TYPE    = 9,
+    OPT_IPV4_ADDRESS_TYPE   = 10,
+    OPT_IPV6_ADDRESS_TYPE   = 11,
+    OPT_IPV6_PREFIX_TYPE    = 12,
+    OPT_PSID_TYPE           = 13,
+    OPT_STRING_TYPE         = 14,
+    OPT_TUPLE_TYPE          = 15,
+    OPT_FQDN_TYPE           = 16,
     // Type to be used only internally. Allows convenient notation of the option config.
-    OPT_INTERNAL_TYPE,
-    OPT_RECORD_TYPE = 254,  // Do not alter this value.
-    OPT_UNKNOWN_TYPE = 255  // Do not alter this value.
+    OPT_INTERNAL_TYPE       = 17,
+    OPT_RECORD_TYPE         = 254,
+    OPT_UNKNOWN_TYPE        = 255
 };
 
 /// @brief Parameters being used to make up an option definition.

diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -2173,7 +2173,7 @@ TEST(OptionDataTypeUtil, typeToString) {
     EXPECT_EQ(OPT_STRING_TYPE, OptionDataTypeUtil::getDataType("string"));
     EXPECT_EQ(OPT_TUPLE_TYPE, OptionDataTypeUtil::getDataType("tuple"));
     EXPECT_EQ(OPT_FQDN_TYPE, OptionDataTypeUtil::getDataType("fqdn"));
-    // EXPECT_EQ(OPT_INTERNAL_TYPE, OptionDataTypeUtil::getDataType("internal"));
+    EXPECT_EQ(OPT_INTERNAL_TYPE, OptionDataTypeUtil::getDataType("internal"));
     EXPECT_EQ(OPT_RECORD_TYPE, OptionDataTypeUtil::getDataType("record"));
     EXPECT_EQ(OPT_UNKNOWN_TYPE, OptionDataTypeUtil::getDataType("bogus"));
 }
@@ -2196,7 +2196,7 @@ TEST(OptionDataTypeUtil, stringToType) {
     EXPECT_EQ("string", OptionDataTypeUtil::getDataTypeName(OPT_STRING_TYPE));
     EXPECT_EQ("tuple", OptionDataTypeUtil::getDataTypeName(OPT_TUPLE_TYPE));
     EXPECT_EQ("fqdn", OptionDataTypeUtil::getDataTypeName(OPT_FQDN_TYPE));
-    // EXPECT_EQ("internal", OptionDataTypeUtil::getDataTypeName(OPT_INTERNAL_TYPE));
+    EXPECT_EQ("internal", OptionDataTypeUtil::getDataTypeName(OPT_INTERNAL_TYPE));
     EXPECT_EQ("record", OptionDataTypeUtil::getDataTypeName(OPT_RECORD_TYPE));
     EXPECT_EQ("unknown", OptionDataTypeUtil::getDataTypeName(OPT_UNKNOWN_TYPE));
 }

diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc
@@ -3170,7 +3170,6 @@ GenericConfigBackendDHCPv4Test::allOptionDefDataTypes4Test() {
 
         ASSERT_TRUE(found_def) << "no option found for " << test_def->getName();
         ASSERT_EQ(*found_def, *test_def);
-        std::cout << "option ok for " << found_def->getName() << std::endl;
     }
 }
 

diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc
@@ -3196,7 +3196,6 @@ GenericConfigBackendDHCPv6Test::allOptionDefDataTypes6Test() {
 
         ASSERT_TRUE(found_def) << "no option found for " << test_def->getName();
         ASSERT_EQ(*found_def, *test_def);
-        std::cout << "option ok for " << found_def->getName() << std::endl;
     }
 }
 

diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql
@@ -5984,6 +5984,7 @@ BEGIN
 
         ALTER TABLE dhcp6_option_def
           ADD CONSTRAINT fk_option_def_data_type6 FOREIGN KEY (type) REFERENCES option_def_data_type(id);
+        SET @disable_audit = 0;
     END IF;
 END $$
 DELIMITER ;

diff --git a/src/share/database/scripts/mysql/upgrade_023_to_024.sh.in b/src/share/database/scripts/mysql/upgrade_023_to_024.sh.in
@@ -111,6 +111,7 @@ BEGIN
 
         ALTER TABLE dhcp6_option_def
           ADD CONSTRAINT fk_option_def_data_type6 FOREIGN KEY (type) REFERENCES option_def_data_type(id);
+        SET @disable_audit = 0;
     END IF;
 END $$
 DELIMITER ;

diff --git a/src/share/database/scripts/pgsql/dhcpdb_create.pgsql b/src/share/database/scripts/pgsql/dhcpdb_create.pgsql
@@ -6445,6 +6445,7 @@ BEGIN
     ALTER TABLE dhcp6_option_def
         ADD CONSTRAINT fk_option_def_data_type6 FOREIGN KEY (type) REFERENCES option_def_data_type(id);
 
+    PERFORM set_config('kea.disable_audit', 'false', false);
     RETURN 'UPDATED';
 END;
 $$ LANGUAGE plpgsql;

diff --git a/src/share/database/scripts/pgsql/upgrade_023_to_024.sh.in b/src/share/database/scripts/pgsql/upgrade_023_to_024.sh.in
@@ -93,6 +93,7 @@ BEGIN
     ALTER TABLE dhcp6_option_def
         ADD CONSTRAINT fk_option_def_data_type6 FOREIGN KEY (type) REFERENCES option_def_data_type(id);
 
+    PERFORM set_config('kea.disable_audit', 'false', false);
     RETURN 'UPDATED';
 END;
 \$\$ LANGUAGE plpgsql;