T5160: firewall refactor: move <set firewall ipv6 ipv6-name ...> to <set firewall ipv6 name ...> . Also fix some unexpected behaviour with geoip.

9 files changed, 63 insertions, 64 deletions

diff --git a/data/templates/firewall/nftables.j2 b/data/templates/firewall/nftables.j2
index 1c70a6b77..10cbc68cb 100644
--- a/data/templates/firewall/nftables.j2
+++ b/data/templates/firewall/nftables.j2
@@ -183,8 +183,8 @@ table ip6 vyos_filter {
         exthdr frag exists meta mark set 0xffff1 return
     }
 
-{%     if ipv6.ipv6_name is vyos_defined %}
-{%         for name_text, conf in ipv6.ipv6_name.items() %}
+{%     if ipv6.name is vyos_defined %}
+{%         for name_text, conf in ipv6.name.items() %}
     chain NAME6_{{ name_text }} {
 {%             if conf.rule is vyos_defined %}
 {%                 for rule_id, rule_conf in conf.rule.items() if rule_conf.disable is not vyos_defined %}
diff --git a/interface-definitions/include/firewall/ipv6-custom-name.xml.i b/interface-definitions/include/firewall/ipv6-custom-name.xml.i
index 6275036c1..4b6777293 100644
--- a/interface-definitions/include/firewall/ipv6-custom-name.xml.i
+++ b/interface-definitions/include/firewall/ipv6-custom-name.xml.i
@@ -1,5 +1,5 @@
 <!-- include start from firewall/ipv6-custom-name.xml.i -->
-<tagNode name="ipv6-name">
+<tagNode name="name">
   <properties>
     <help>IPv6 custom firewall</help>
     <constraint>
@@ -14,7 +14,7 @@
       <properties>
         <help>Set jump target. Action jump must be defined in default-action to use this setting</help>
         <completionHelp>
-          <path>firewall ipv6 ipv6-name</path>
+          <path>firewall ipv6 name</path>
         </completionHelp>
       </properties>
     </leafNode>
@@ -38,7 +38,7 @@
           <properties>
             <help>Set jump target. Action jump must be defined to use this setting</help>
             <completionHelp>
-              <path>firewall ipv6 ipv6-name</path>
+              <path>firewall ipv6 name</path>
             </completionHelp>
           </properties>
         </leafNode>
diff --git a/interface-definitions/include/firewall/ipv6-hook-forward.xml.i b/interface-definitions/include/firewall/ipv6-hook-forward.xml.i
index 042bd9931..25e1bd288 100644
--- a/interface-definitions/include/firewall/ipv6-hook-forward.xml.i
+++ b/interface-definitions/include/firewall/ipv6-hook-forward.xml.i
@@ -31,7 +31,7 @@
               <properties>
                 <help>Set jump target. Action jump must be defined to use this setting</help>
                 <completionHelp>
-                  <path>firewall ipv6 ipv6-name</path>
+                  <path>firewall ipv6 name</path>
                 </completionHelp>
               </properties>
             </leafNode>
diff --git a/interface-definitions/include/firewall/ipv6-hook-input.xml.i b/interface-definitions/include/firewall/ipv6-hook-input.xml.i
index 8c41e0aca..f9a4d71b4 100644
--- a/interface-definitions/include/firewall/ipv6-hook-input.xml.i
+++ b/interface-definitions/include/firewall/ipv6-hook-input.xml.i
@@ -30,7 +30,7 @@
               <properties>
                 <help>Set jump target. Action jump must be defined to use this setting</help>
                 <completionHelp>
-                  <path>firewall ipv6 ipv6-name</path>
+                  <path>firewall ipv6 name</path>
                 </completionHelp>
               </properties>
             </leafNode>
diff --git a/interface-definitions/include/firewall/ipv6-hook-output.xml.i b/interface-definitions/include/firewall/ipv6-hook-output.xml.i
index 9b756d870..9bf73a778 100644
--- a/interface-definitions/include/firewall/ipv6-hook-output.xml.i
+++ b/interface-definitions/include/firewall/ipv6-hook-output.xml.i
@@ -30,7 +30,7 @@
               <properties>
                 <help>Set jump target. Action jump must be defined to use this setting</help>
                 <completionHelp>
-                  <path>firewall ipv6 ipv6-name</path>
+                  <path>firewall ipv6 name</path>
                 </completionHelp>
               </properties>
             </leafNode>
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index b028f0af0..4aa509fe2 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -51,8 +51,8 @@ def fqdn_config_parse(firewall):
             
         if (path[0] == 'ipv4') and (path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'name'):
             firewall['ip_fqdn'][set_name] = domain
-        elif (path[0] == 'ipv6') and (path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'ipv6_name'):
-            if path[1] == 'ipv6_name':
+        elif (path[0] == 'ipv6') and (path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'name'):
+            if path[1] == 'name':
                 set_name = f'name6_{priority}_{rule}_{suffix}'
             firewall['ip6_fqdn'][set_name] = domain
 
@@ -160,8 +160,8 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
                 if hook == 'OUT':
                     hook_name = 'output'
                 if hook == 'NAM':
-                    hook_name = f'name{def_suffix}'
-                output.append(f'{ip_name} {prefix}addr {operator} @GEOIP_CC_{hook_name}_{fw_name}_{rule_id}')
+                    hook_name = f'name'
+                output.append(f'{ip_name} {prefix}addr {operator} @GEOIP_CC{def_suffix}_{hook_name}_{fw_name}_{rule_id}')
 
             if 'mac_address' in side_conf:
                 suffix = side_conf["mac_address"]
@@ -519,12 +519,11 @@ def geoip_update(firewall, force=False):
         # Map country codes to set names
         for codes, path in dict_search_recursive(firewall, 'country_code'):
             set_name = f'GEOIP_CC_{path[1]}_{path[2]}_{path[4]}'
-            if path[1] == 'ipv6_name':
-                set_name = f'GEOIP_CC_name6_{path[2]}_{path[4]}'
-            if ( path[0] == 'ipv4' ) and ( path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'name' ):
+            if ( path[0] == 'ipv4'):
                 for code in codes:
                     ipv4_codes.setdefault(code, []).append(set_name)
-            elif ( path[0] == 'ipv6' ) and ( path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'ipv6_name' ):
+            elif ( path[0] == 'ipv6' ):
+                set_name = f'GEOIP_CC6_{path[1]}_{path[2]}_{path[4]}'
                 for code in codes:
                     ipv6_codes.setdefault(code, []).append(set_name)
 
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index bd7666313..9412ce984 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -362,14 +362,14 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         name = 'v6-smoketest'
         interface = 'eth0'
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'default-action', 'drop'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'enable-default-log'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'default-action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'enable-default-log'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'source', 'address', '2002::1'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'destination', 'address', '2002::1:1'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'log', 'enable'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'log-options', 'level', 'crit'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'source', 'address', '2002::1'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'destination', 'address', '2002::1:1'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'log', 'enable'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'log-options', 'level', 'crit'])
 
         self.cli_set(['firewall', 'ipv6', 'forward', 'filter', 'default-action', 'accept'])
         self.cli_set(['firewall', 'ipv6', 'forward', 'filter', 'rule', '2', 'action', 'reject'])
@@ -411,15 +411,15 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         name2 = 'v6-smoketest-adv2'
         interface = 'eth0'
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'default-action', 'drop'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'enable-default-log'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'default-action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'enable-default-log'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'packet-length', '65'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'packet-length', '513'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'packet-length', '1025'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'dscp', '18'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'dscp', '53'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'packet-length', '65'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'packet-length', '513'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'packet-length', '1025'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'dscp', '18'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'dscp', '53'])
 
         self.cli_set(['firewall', 'ipv6', 'forward', 'filter', 'rule', '4', 'action', 'accept'])
         self.cli_set(['firewall', 'ipv6', 'forward', 'filter', 'rule', '4', 'packet-length', '1-1999'])
@@ -454,20 +454,20 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         self.cli_set(['firewall', 'group', 'ipv6-address-group', 'mask_group', 'address', '::beef'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'default-action', 'drop'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'enable-default-log'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'default-action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'enable-default-log'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'action', 'drop'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'destination', 'address', '::1111:2222:3333:4444'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '1', 'destination', 'address-mask', '::ffff:ffff:ffff:ffff'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'destination', 'address', '::1111:2222:3333:4444'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'destination', 'address-mask', '::ffff:ffff:ffff:ffff'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '2', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '2', 'source', 'address', '!::aaaa:bbbb:cccc:dddd'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '2', 'source', 'address-mask', '::ffff:ffff:ffff:ffff'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '2', 'action', 'accept'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '2', 'source', 'address', '!::aaaa:bbbb:cccc:dddd'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '2', 'source', 'address-mask', '::ffff:ffff:ffff:ffff'])
 
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'action', 'drop'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'source', 'group', 'address-group', 'mask_group'])
-        self.cli_set(['firewall', 'ipv6', 'ipv6-name', name, 'rule', '3', 'source', 'address-mask', '::ffff:ffff:ffff:ffff'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'action', 'drop'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'source', 'group', 'address-group', 'mask_group'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '3', 'source', 'address-mask', '::ffff:ffff:ffff:ffff'])
 
         self.cli_commit()
 
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index a50ae2ec6..c8b1e27db 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -98,21 +98,21 @@ def geoip_updated(conf, firewall):
 
     for key, path in dict_search_recursive(firewall, 'geoip'):
         set_name = f'GEOIP_CC_{path[1]}_{path[2]}_{path[4]}'
-        if path[1] == 'ipv6_name':
-            set_name = f'GEOIP_CC_name6_{path[2]}_{path[4]}'
-
-        if (path[0] == 'ipv4') and ( path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'name' ):
+        if (path[0] == 'ipv4'):
             out['name'].append(set_name)
-        elif (path[0] == 'ipv6') and ( path[1] == 'forward' or path[1] == 'input' or path[1] == 'output' or path[1] == 'ipv6_name' ):
+        elif (path[0] == 'ipv6'):
+            set_name = f'GEOIP_CC6_{path[1]}_{path[2]}_{path[4]}'
             out['ipv6_name'].append(set_name)
+            
         updated = True
 
     if 'delete' in node_diff:
         for key, path in dict_search_recursive(node_diff['delete'], 'geoip'):
-            set_name = f'GEOIP_CC_{path[2]}_{path[4]}'
-            if path[1] == 'name':
+            set_name = f'GEOIP_CC_{path[1]}_{path[2]}_{path[4]}'
+            if (path[0] == 'ipv4'):
                 out['deleted_name'].append(set_name)
-            elif path[1] == 'ipv6-name':
+            elif (path[0] == 'ipv6'):
+                set_name = f'GEOIP_CC_{path[1]}_{path[2]}_{path[4]}'
                 out['deleted_ipv6_name'].append(set_name)
             updated = True
 
@@ -140,7 +140,7 @@ def get_config(config=None):
     default_values = defaults(base)
 
     for family in ['ipv4', 'ipv6']:
-        for tmp in ['name', 'ipv6_name', 'forward', 'input', 'output', 'prerouting']:
+        for tmp in ['name', 'forward', 'input', 'output', 'prerouting']:
             if tmp in default_values[family]:
                 del default_values[family][tmp]
 
@@ -162,11 +162,11 @@ def get_config(config=None):
                                                         firewall['ipv4'][hook][priority])
 
     # Merge in defaults for IPv6 ruleset
-    if 'ipv6_name' in firewall['ipv6']:
-        default_values = defaults(base + ['ipv6'] + ['ipv6-name'])
-        for ipv6_name in firewall['ipv6']['ipv6_name']:
-            firewall['ipv6']['ipv6_name'][ipv6_name] = dict_merge(default_values,
-                                                          firewall['ipv6']['ipv6_name'][ipv6_name])
+    if 'name' in firewall['ipv6']:
+        default_values = defaults(base + ['ipv6'] + ['name'])
+        for ipv6_name in firewall['ipv6']['name']:
+            firewall['ipv6']['name'][ipv6_name] = dict_merge(default_values,
+                                                          firewall['ipv6']['name'][ipv6_name])
     for hook in ['forward', 'input', 'output', 'prerouting']:
         if hook in firewall['ipv6']:
             for priority in ['filter', 'mangle', 'raw']:
@@ -209,8 +209,8 @@ def verify_rule(firewall, rule_conf, ipv6):
             if target not in dict_search_args(firewall, 'ipv4', 'name'):
                 raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
         else:
-            if target not in dict_search_args(firewall, 'ipv6', 'ipv6_name'):
-                raise ConfigError(f'Invalid jump-target. Firewall ipv6-name {target} does not exist on the system')
+            if target not in dict_search_args(firewall, 'ipv6', 'name'):
+                raise ConfigError(f'Invalid jump-target. Firewall ipv6 name {target} does not exist on the system')
 
     if 'queue_options' in rule_conf:
         if 'queue' not in rule_conf['action']:
@@ -359,7 +359,7 @@ def verify(firewall):
                             verify_rule(firewall, rule_conf, False)
 
     if 'ipv6' in firewall:
-        for name in ['ipv6_name','forward','input','output']:
+        for name in ['name','forward','input','output']:
             if name in firewall['ipv6']:
                 for name_id, name_conf in firewall['ipv6'][name].items():
                     if 'jump' in name_conf['default_action'] and 'default_jump_target' not in name_conf:
@@ -371,7 +371,7 @@ def verify(firewall):
                         if name_conf['default_jump_target'] == name_id:
                             raise ConfigError(f'Loop detected on default-jump-target.')
                         ## Now need to check that default-jump-target exists (other firewall chain/name)
-                        if target not in dict_search_args(firewall['ipv6'], 'ipv6_name'):
+                        if target not in dict_search_args(firewall['ipv6'], 'name'):
                             raise ConfigError(f'Invalid jump-target. Firewall name {target} does not exist on the system')
 
                     if 'rule' in name_conf:
diff --git a/src/migration-scripts/firewall/10-to-11 b/src/migration-scripts/firewall/10-to-11
index 8cd2a4df8..8afcb64fd 100755
--- a/src/migration-scripts/firewall/10-to-11
+++ b/src/migration-scripts/firewall/10-to-11
@@ -21,7 +21,7 @@
 #  set firewall ipv6-name <name> ...
 #  To
 #  set firewall ipv4 name <name> 
-#  set firewall ipv6 ipv6-name <name> 
+#  set firewall ipv6 name <name> 
 
 ## Also from 'firewall interface' removed.
 ## in and out:
@@ -97,11 +97,11 @@ if config.exists(base + ['name']):
     config.delete(base + ['name'])
 
 if config.exists(base + ['ipv6-name']):
-    config.set(['firewall', 'ipv6', 'ipv6-name'])
-    config.set_tag(['firewall', 'ipv6', 'ipv6-name'])
+    config.set(['firewall', 'ipv6', 'name'])
+    config.set_tag(['firewall', 'ipv6', 'name'])
 
     for ipv6name in config.list_nodes(base + ['ipv6-name']):
-        config.copy(base + ['ipv6-name', ipv6name], base + ['ipv6', 'ipv6-name', ipv6name])
+        config.copy(base + ['ipv6-name', ipv6name], base + ['ipv6', 'name', ipv6name])
     config.delete(base + ['ipv6-name'])
 
 ### Migration of firewall interface