[#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

diff --git a/src/bin/admin/kea-admin.in b/src/bin/admin/kea-admin.in
index 3a2af183618364e7932aadf77dc96d07a159fcdc..b1172ff8ca6f87334f41a27dd18b11fa0acd0037 100644 (file)
--- 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
index 52e8a2e18d58d3c6681d1d9c7b577193995d588c..f224c22528466cc9fd74bb354ebdd0b283d3ce1d 100644 (file)
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -35,34 +35,35 @@ public:
 
 /// @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
index 5ca6175de76928b27b94e0b01bfc4c8a9b11ae47..6bdd691fefd3f0da5d2e5ec3dc3d0c438bb12861 100644 (file)
--- 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
index c95b70f9001144d72168a21a8c6da44a5abfecd8..4731f6019e1d8fa2ee042c30d98dabcf24f78cb1 100644 (file)
--- 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
index dd2dbfc36fcb723dbc2a6ba98f1cd9fea69b6abc..9f54aec1ad195987b403aff589c14f832df7d51f 100644 (file)
--- 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
index edc754074b83e6d6853b0b81d131a163c2b87cd4..c9b8f8148409e0b24d9d22ff475c76528f85562a 100644 (file)
--- 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
index bdd0839d45e9ebbe9c3f8d8645cbb7035e6a4605..fd438731cb87592253dd6c506d531773a67b6909 100644 (file)
--- 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
index 46e3ad6dc49fe8a6f87cce08df4128066a22d28b..e2a5f848a032e3888c15ceedef888f0e02acebf3 100644 (file)
--- 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
index a67cb9ecffacd328d62f5004854b5f4401235bbe..04749d855ac45f36911cd093a72217aa9599a583 100644 (file)
--- 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;