firewall: T8282: Normalize invalid network-group names during migration

Replace unsupported characters (e.g. `,`, `+`) with `_` when migrating
from 1.3.x and handle name collisions safely to prevent commit failures.

1 files changed, 33 insertions, 14 deletions

diff --git a/src/migration-scripts/firewall/6-to-7 b/src/migration-scripts/firewall/6-to-7
index c914a71ee..db83497cd 100644
--- a/src/migration-scripts/firewall/6-to-7
+++ b/src/migration-scripts/firewall/6-to-7
@@ -92,6 +92,24 @@ icmpv6_translations = {
 v4_groups = ["address-group", "network-group", "port-group"]
 v6_groups = ["ipv6-address-group", "ipv6-network-group", "port-group"]
 
+valid_group_name_pattern = re.compile(r'^[A-Za-z0-9_.-]+$')
+invalid_group_name_pattern = re.compile(r'[^A-Za-z0-9_.-]')
+
+def is_valid_group_name(name: str) -> bool:
+    return bool(valid_group_name_pattern.match(name))
+
+def sanitize_group_name(name: str) -> str:
+    """Function to sanitize group name by replacing invalid characters"""
+    return invalid_group_name_pattern.sub('_', name)
+
+def get_unique_group_name(config: ConfigTree, path: list[str], name: str) -> str:
+    """Function to generate a unique group name if the proposed name already exists"""
+
+    new_name = sanitize_group_name(name)
+    while config.exists(path + [new_name]):
+        new_name = f'{new_name}_'
+    return new_name
+
 def migrate(config: ConfigTree) -> None:
     if not config.exists(base):
         # Nothing to do
@@ -108,16 +126,13 @@ def migrate(config: ConfigTree) -> None:
                 if config.exists(name_description):
                     tmp = config.return_value(name_description)
                     config.set(name_description, value=tmp[:max_len_description])
-                if '+' in group_name:
-                    replacement_string = "_"
+
+                if not is_valid_group_name(group_name):
                     if group_type in v4_groups and not v4_found:
                         v4_found = True
                     if group_type in v6_groups and not v6_found:
                         v6_found = True
-                    new_group_name = group_name.replace('+', replacement_string)
-                    while config.exists(base + ['group', group_type, new_group_name]):
-                        replacement_string = replacement_string + "_"
-                        new_group_name = group_name.replace('+', replacement_string)
+                    new_group_name = get_unique_group_name(config, base + ['group', group_type], group_name)
                     translated_dict[group_name] = new_group_name
                     config.copy(base + ['group', group_type, group_name], base + ['group', group_type, new_group_name])
                     config.delete(base + ['group', group_type, group_name])
@@ -185,11 +200,13 @@ def migrate(config: ConfigTree) -> None:
                         if config.exists(base + ['name', name, 'rule', rule, direction, 'group']) and v4_found:
                             for group_type in config.list_nodes(base + ['name', name, 'rule', rule, direction, 'group']):
                                 group_name = config.return_value(base + ['name', name, 'rule', rule, direction, 'group', group_type])
-                                if '+' in group_name:
-                                    if group_name[0] == "!":
-                                        new_group_name = "!" + translated_dict[group_name[1:]]
+
+                                translated_group_name = group_name[1:] if group_name.startswith('!') else group_name
+                                if translated_group_name in translated_dict:
+                                    if group_name.startswith('!'):
+                                        new_group_name = '!' + translated_dict[translated_group_name]
                                     else:
-                                        new_group_name = translated_dict[group_name]
+                                        new_group_name = translated_dict[translated_group_name]
                                     config.set(base + ['name', name, 'rule', rule, direction, 'group', group_type], value=new_group_name)
 
                         pg_base = base + ['name', name, 'rule', rule, direction, 'group', 'port-group']
@@ -288,11 +305,13 @@ def migrate(config: ConfigTree) -> None:
                         if config.exists(base + ['ipv6-name', name, 'rule', rule, direction, 'group']) and v6_found:
                             for group_type in config.list_nodes(base + ['ipv6-name', name, 'rule', rule, direction, 'group']):
                                 group_name = config.return_value(base + ['ipv6-name', name, 'rule', rule, direction, 'group', group_type])
-                                if '+' in group_name:
-                                    if group_name[0] == "!":
-                                        new_group_name = "!" + translated_dict[group_name[1:]]
+
+                                translated_group_name = group_name[1:] if group_name.startswith('!') else group_name
+                                if translated_group_name in translated_dict:
+                                    if group_name.startswith('!'):
+                                        new_group_name = '!' + translated_dict[translated_group_name]
                                     else:
-                                        new_group_name = translated_dict[group_name]
+                                        new_group_name = translated_dict[translated_group_name]
                                     config.set(base + ['ipv6-name', name, 'rule', rule, direction, 'group', group_type], value=new_group_name)
 
                         pg_base = base + ['ipv6-name', name, 'rule', rule, direction, 'group', 'port-group']