[202311][dhcp_server] Add support for customize string option include comma (#18811)

* [dhcp_server] Add support for customize comma-sperated string option

Author: yaqiangz

src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_cfggen.py

@@ -94,7 +94,7 @@ def _parse_customized_options(self, customized_options_ipv4):
             always_send = config["always_send"] if "always_send" in config else "true"
             customized_options[option_name] = {
                 "id": config["id"],
-                "value": config["value"],
+                "value": config["value"].replace(",", "\\\\,") if option_type == "string" else config["value"],
                 "type": option_type,
                 "always_send": always_send
             }

src/sonic-dhcp-utilities/tests/test_data/test_kea_config.conf

@@ -0,0 +1,3 @@
+{
+    "key": "dummy_value\\,dummy_value"
+}

src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py

@@ -209,61 +209,53 @@
         }
     }
 }
-tested_options_data = [
-    {
-        "data": {
-            "option223": {
-                "id": "223",
-                "type": "string",
-                "value": "dummy_value"
-            }
+tested_options_data = {
+    "data": {
+        "option223": {
+            "id": "223",
+            "type": "string",
+            "value": "dummy_value"
         },
-        "res": True
-    },
-    {
-        "data": {
-            "option60": {
-                "id": "60",
-                "type": "string",
-                "value": "dummy_value"
-            }
+        "option60": {
+            "id": "60",
+            "type": "string",
+            "value": "dummy_value"
         },
-        "res": False
-    },
-    {
-        "data": {
-            "option222": {
-                "id": "222",
-                "type": "text",
-                "value": "dummy_value"
-            }
+        "option222": {
+            "id": "222",
+            "type": "text",
+            "value": "dummy_value"
         },
-        "res": False
-    },
-    {
-        "data": {
-            "option219": {
-                "id": "219",
-                "type": "uint8",
-                "value": "259"
-            }
+        "option219": {
+            "id": "219",
+            "type": "uint8",
+            "value": "259"
         },
-        "res": False
-    },
-    {
-        "data": {
-            "option223": {
-                "id": "223",
-                "type": "string",
-                "value": "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
-                         "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
-                         "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
-                         "long_valuelong_valuelong_valuelong_valuelong_value"
-            }
+        "option218": {
+            "id": "218",
+            "type": "string",
+            "value": "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
+                        "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
+                        "long_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_valuelong_value" +
+                        "long_valuelong_valuelong_valuelong_valuelong_value"
+        },
+        "option217": {
+            "id": "217",
+            "type": "string",
+            "value": "dummy_value,dummy_value"
         },
-        "res": False
+        "option216": {
+            "id": "216",
+            "type": "uint8",
+            "value": "8"
+        }
+    },
+    "res": {
+        "option223": "dummy_value",
+        "option217": "dummy_value\\\\,dummy_value",
+        "option216": "8"
     }
-]
+}
 
 
 def test_parse_port_alias(mock_swsscommon_dbconnector_init, mock_get_render_template):
@@ -397,21 +389,19 @@ def test_render_config(mock_swsscommon_dbconnector_init, mock_parse_port_map_ali
     assert json.loads(config) == expected_config if with_port_config else expected_config
 
 
-@pytest.mark.parametrize("tested_options_data", tested_options_data)
 def test_parse_customized_options(mock_swsscommon_dbconnector_init, mock_get_render_template,
-                                  mock_parse_port_map_alias, tested_options_data):
+                                  mock_parse_port_map_alias):
     dhcp_db_connector = DhcpDbConnector()
     dhcp_cfg_generator = DhcpServCfgGenerator(dhcp_db_connector)
     customized_options_ipv4 = tested_options_data["data"]
     customized_options = dhcp_cfg_generator._parse_customized_options(customized_options_ipv4)
-    if tested_options_data["res"]:
-        assert customized_options == {
-            "option223": {
-                "id": "223",
-                "value": "dummy_value",
-                "type": "string",
-                "always_send": "true"
-            }
+    expected_res = {}
+    for key, value in tested_options_data["res"].items():
+        expected_res[key] = {
+            "id": customized_options_ipv4[key]["id"],
+            "value": value,
+            "type": customized_options_ipv4[key]["type"],
+            "always_send": "true"
         }
     else:
-        assert customized_options == {}
+        assert customized_options == expected_res

src/sonic-dhcp-utilities/tests/test_dhcpservd.py

@@ -1,4 +1,5 @@
 import pytest
+import json
 import psutil
 import signal
 import sys
@@ -18,11 +19,18 @@
                      "VlanMemberTableEventChecker"]
 
 
+tested_config = """
+{
+    "key": "dummy_value\\\\,dummy_value"
+}
+"""
+
+
 @pytest.mark.parametrize("enabled_checker", [None, set(PORT_MODE_CHECKER)])
 def test_dump_dhcp4_config(mock_swsscommon_dbconnector_init, enabled_checker):
     new_enabled_checker = set(["VlanTableEventChecker"])
     with patch("dhcp_utilities.dhcpservd.dhcp_cfggen.DhcpServCfgGenerator.generate",
-               return_value=("dummy_config", set(), set(), set(), new_enabled_checker)) as mock_generate, \
+               return_value=(tested_config, set(), set(), set(), new_enabled_checker)) as mock_generate, \
          patch("dhcp_utilities.dhcpservd.dhcpservd.DhcpServd._notify_kea_dhcp4_proc",
                MagicMock()) as mock_notify_kea_dhcp4_proc, \
          patch.object(DhcpServd, "dhcp_servd_monitor", return_value=DhcpServdDbMonitor,
@@ -39,6 +47,11 @@ def test_dump_dhcp4_config(mock_swsscommon_dbconnector_init, enabled_checker):
         dhcpservd.dump_dhcp4_config()
         # Verfiy whether generate() func of dhcp_cfggen is called
         mock_generate.assert_called_once_with()
+        with open("tests/test_data/test_kea_config.conf", "r") as file, \
+             open("/tmp/kea-dhcp4.conf", "r") as output:
+            expected_content = file.read()
+            actual_content = output.read()
+            assert json.loads(expected_content) == json.loads(actual_content)
         # Verify whether notify func of dhcpservd is called, which is expected to call after new config generated
         mock_notify_kea_dhcp4_proc.assert_called_once_with()
         if enabled_checker is None: