Merge pull request #2327 from vyos/mergify/bp/sagitta/pr-2325

T5165: Migrate policy local-route rule x destination to address (backport #2325)

7 files changed, 170 insertions, 98 deletions

diff --git a/interface-definitions/include/policy/local-route_rule_ipv4_address.xml.i b/interface-definitions/include/policy/local-route_rule_ipv4_address.xml.i
new file mode 100644
index 000000000..ffe73ee32
--- /dev/null
+++ b/interface-definitions/include/policy/local-route_rule_ipv4_address.xml.i
@@ -0,0 +1,20 @@
+<!-- include start from policy/local-route_rule_ipv4_address.xml.i -->
+<leafNode name="address">
+  <properties>
+    <help>IPv4 address or prefix</help>
+    <valueHelp>
+      <format>ipv4</format>
+      <description>Address to match against</description>
+    </valueHelp>
+    <valueHelp>
+      <format>ipv4net</format>
+      <description>Prefix to match against</description>
+    </valueHelp>
+    <constraint>
+      <validator name="ipv4-address"/>
+      <validator name="ip-prefix"/>
+    </constraint>
+    <multi/>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/include/policy/local-route_rule_ipv6_address.xml.i b/interface-definitions/include/policy/local-route_rule_ipv6_address.xml.i
new file mode 100644
index 000000000..d8fb6c074
--- /dev/null
+++ b/interface-definitions/include/policy/local-route_rule_ipv6_address.xml.i
@@ -0,0 +1,20 @@
+<!-- include start from policy/local-route_rule_ipv6_address.xml.i -->
+<leafNode name="address">
+  <properties>
+    <help>IPv6 address or prefix</help>
+    <valueHelp>
+      <format>ipv6</format>
+      <description>Address to match against</description>
+    </valueHelp>
+    <valueHelp>
+      <format>ipv6net</format>
+      <description>Prefix to match against</description>
+    </valueHelp>
+    <constraint>
+      <validator name="ipv6-address"/>
+      <validator name="ipv6-prefix"/>
+    </constraint>
+    <multi/>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/include/version/policy-version.xml.i b/interface-definitions/include/version/policy-version.xml.i
index f1494eaa3..2c96e0f15 100644
--- a/interface-definitions/include/version/policy-version.xml.i
+++ b/interface-definitions/include/version/policy-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/policy-version.xml.i -->
-<syntaxVersion component='policy' version='5'></syntaxVersion>
+<syntaxVersion component='policy' version='6'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/policy-local-route.xml.in b/interface-definitions/policy-local-route.xml.in
index 0a5b81dfa..6827bd64e 100644
--- a/interface-definitions/policy-local-route.xml.in
+++ b/interface-definitions/policy-local-route.xml.in
@@ -54,42 +54,22 @@
                 </properties>
               </leafNode>
               #include <include/policy/local-route_rule_protocol.xml.i>
-              <leafNode name="source">
+              <node name="source">
                 <properties>
-                  <help>Source address or prefix</help>
-                  <valueHelp>
-                    <format>ipv4</format>
-                    <description>Address to match against</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>ipv4net</format>
-                    <description>Prefix to match against</description>
-                  </valueHelp>
-                  <constraint>
-                    <validator name="ipv4-address"/>
-                    <validator name="ip-prefix"/>
-                  </constraint>
-                  <multi/>
+                  <help>Source parameters</help>
                 </properties>
-              </leafNode>
-              <leafNode name="destination">
+                <children>
+                  #include <include/policy/local-route_rule_ipv4_address.xml.i>
+                </children>
+              </node>
+              <node name="destination">
                 <properties>
-                  <help>Destination address or prefix</help>
-                  <valueHelp>
-                    <format>ipv4</format>
-                    <description>Address to match against</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>ipv4net</format>
-                    <description>Prefix to match against</description>
-                  </valueHelp>
-                  <constraint>
-                    <validator name="ipv4-address"/>
-                    <validator name="ip-prefix"/>
-                  </constraint>
-                  <multi/>
+                  <help>Destination parameters</help>
                 </properties>
-              </leafNode>
+                <children>
+                  #include <include/policy/local-route_rule_ipv4_address.xml.i>
+                </children>
+              </node>
               #include <include/interface/inbound-interface.xml.i>
             </children>
           </tagNode>
@@ -145,42 +125,22 @@
                   </constraint>
                 </properties>
               </leafNode>
-              <leafNode name="source">
+              <node name="source">
                 <properties>
-                  <help>Source address or prefix</help>
-                  <valueHelp>
-                    <format>ipv6</format>
-                    <description>Address to match against</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>ipv6net</format>
-                    <description>Prefix to match against</description>
-                  </valueHelp>
-                  <constraint>
-                    <validator name="ipv6-address"/>
-                    <validator name="ipv6-prefix"/>
-                  </constraint>
-                  <multi/>
+                  <help>Source parameters</help>
                 </properties>
-              </leafNode>
-              <leafNode name="destination">
+                <children>
+                  #include <include/policy/local-route_rule_ipv6_address.xml.i>
+                </children>
+              </node>
+              <node name="destination">
                 <properties>
-                  <help>Destination address or prefix</help>
-                  <valueHelp>
-                    <format>ipv6</format>
-                    <description>Address to match against</description>
-                  </valueHelp>
-                  <valueHelp>
-                    <format>ipv6net</format>
-                    <description>Prefix to match against</description>
-                  </valueHelp>
-                  <constraint>
-                    <validator name="ipv6-address"/>
-                    <validator name="ipv6-prefix"/>
-                  </constraint>
-                  <multi/>
+                  <help>Destination parameters</help>
                 </properties>
-              </leafNode>
+                <children>
+                  #include <include/policy/local-route_rule_ipv6_address.xml.i>
+                </children>
+              </node>
               #include <include/interface/inbound-interface.xml.i>
             </children>
           </tagNode>
diff --git a/smoketest/scripts/cli/test_policy.py b/smoketest/scripts/cli/test_policy.py
index e868895ce..4ac422d5f 100755
--- a/smoketest/scripts/cli/test_policy.py
+++ b/smoketest/scripts/cli/test_policy.py
@@ -1467,7 +1467,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '23'
         for src in sources:
             self.cli_set(path + ['rule', rule, 'set', 'table', table])
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
 
         self.cli_commit()
 
@@ -1508,7 +1508,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '154'
 
         self.cli_set(path + ['rule', rule, 'set', 'table', table])
-        self.cli_set(path + ['rule', rule, 'destination', dst])
+        self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
 
         self.cli_commit()
 
@@ -1529,7 +1529,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         proto = 'tcp'
 
         self.cli_set(path + ['rule', rule, 'set', 'table', table])
-        self.cli_set(path + ['rule', rule, 'destination', dst])
+        self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
         self.cli_set(path + ['rule', rule, 'protocol', proto])
 
         self.cli_commit()
@@ -1551,7 +1551,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '150'
         for src in sources:
             self.cli_set(path + ['rule', rule, 'set', 'table', table])
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
             self.cli_set(path + ['rule', rule, 'fwmark', fwmk])
 
         self.cli_commit()
@@ -1576,7 +1576,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         self.cli_set(path + ['rule', rule, 'set', 'table', table])
         self.cli_set(path + ['rule', rule, 'inbound-interface', iif])
         for src in sources:
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
 
         self.cli_commit()
 
@@ -1602,8 +1602,8 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         for src in sources:
             for dst in destinations:
                 self.cli_set(path + ['rule', rule, 'set', 'table', table])
-                self.cli_set(path + ['rule', rule, 'source', src])
-                self.cli_set(path + ['rule', rule, 'destination', dst])
+                self.cli_set(path + ['rule', rule, 'source', 'address', src])
+                self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
                 self.cli_set(path + ['rule', rule, 'fwmark', fwmk])
 
         self.cli_commit()
@@ -1627,7 +1627,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '23'
         for src in sources:
             self.cli_set(path + ['rule', rule, 'set', 'table', table])
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
 
         self.cli_commit()
 
@@ -1668,7 +1668,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '154'
 
         self.cli_set(path + ['rule', rule, 'set', 'table', table])
-        self.cli_set(path + ['rule', rule, 'destination', dst])
+        self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
 
         self.cli_commit()
 
@@ -1689,7 +1689,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '150'
         for src in sources:
             self.cli_set(path + ['rule', rule, 'set', 'table', table])
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
             self.cli_set(path + ['rule', rule, 'fwmark', fwmk])
 
         self.cli_commit()
@@ -1712,7 +1712,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '150'
         for src in sources:
             self.cli_set(path + ['rule', rule, 'set', 'table', table])
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
             self.cli_set(path + ['rule', rule, 'inbound-interface', iif])
 
         self.cli_commit()
@@ -1739,8 +1739,8 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         for src in sources:
             for dst in destinations:
                 self.cli_set(path + ['rule', rule, 'set', 'table', table])
-                self.cli_set(path + ['rule', rule, 'source', src])
-                self.cli_set(path + ['rule', rule, 'destination', dst])
+                self.cli_set(path + ['rule', rule, 'source', 'address', src])
+                self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
                 self.cli_set(path + ['rule', rule, 'fwmark', fwmk])
 
         self.cli_commit()
@@ -1770,15 +1770,15 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         for src in sources:
             for dst in destinations:
                 self.cli_set(path + ['rule', rule, 'set', 'table', table])
-                self.cli_set(path + ['rule', rule, 'source', src])
-                self.cli_set(path + ['rule', rule, 'destination', dst])
+                self.cli_set(path + ['rule', rule, 'source', 'address', src])
+                self.cli_set(path + ['rule', rule, 'destination', 'address', dst])
                 self.cli_set(path + ['rule', rule, 'fwmark', fwmk])
 
         for src in sources_v6:
             for dst in destinations_v6:
                 self.cli_set(path_v6 + ['rule', rule, 'set', 'table', table])
-                self.cli_set(path_v6 + ['rule', rule, 'source', src])
-                self.cli_set(path_v6 + ['rule', rule, 'destination', dst])
+                self.cli_set(path_v6 + ['rule', rule, 'source', 'address', src])
+                self.cli_set(path_v6 + ['rule', rule, 'destination', 'address', dst])
                 self.cli_set(path_v6 + ['rule', rule, 'fwmark', fwmk])
 
         self.cli_commit()
@@ -1821,7 +1821,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         table = '151'
         self.cli_set(path + ['rule', rule, 'set', 'table', table])
         for src in sources:
-            self.cli_set(path + ['rule', rule, 'source', src])
+            self.cli_set(path + ['rule', rule, 'source', 'address', src])
 
         self.cli_commit()
 
@@ -1834,7 +1834,7 @@ class TestPolicy(VyOSUnitTestSHIM.TestCase):
         self.assertEqual(sort_ip(tmp), sort_ip(original_first))
 
         # Create second commit with added destination
-        self.cli_set(path + ['rule', rule, 'destination', destination])
+        self.cli_set(path + ['rule', rule, 'destination', 'address', destination])
         self.cli_commit()
 
         original_second = """
diff --git a/src/conf_mode/policy-local-route.py b/src/conf_mode/policy-local-route.py
index d3c307cdc..2e8aabb80 100755
--- a/src/conf_mode/policy-local-route.py
+++ b/src/conf_mode/policy-local-route.py
@@ -51,20 +51,20 @@ def get_config(config=None):
         tmp = node_changed(conf, base_rule, key_mangling=('-', '_'))
         if tmp:
             for rule in (tmp or []):
-                src = leaf_node_changed(conf, base_rule + [rule, 'source'])
+                src = leaf_node_changed(conf, base_rule + [rule, 'source', 'address'])
                 fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark'])
                 iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface'])
-                dst = leaf_node_changed(conf, base_rule + [rule, 'destination'])
+                dst = leaf_node_changed(conf, base_rule + [rule, 'destination', 'address'])
                 proto = leaf_node_changed(conf, base_rule + [rule, 'protocol'])
                 rule_def = {}
                 if src:
-                    rule_def = dict_merge({'source' : src}, rule_def)
+                    rule_def = dict_merge({'source': {'address': src}}, rule_def)
                 if fwmk:
                     rule_def = dict_merge({'fwmark' : fwmk}, rule_def)
                 if iif:
                     rule_def = dict_merge({'inbound_interface' : iif}, rule_def)
                 if dst:
-                    rule_def = dict_merge({'destination' : dst}, rule_def)
+                    rule_def = dict_merge({'destination': {'address': dst}}, rule_def)
                 if proto:
                     rule_def = dict_merge({'protocol' : proto}, rule_def)
                 dict = dict_merge({dict_id : {rule : rule_def}}, dict)
@@ -78,10 +78,10 @@ def get_config(config=None):
         # delete policy local-route rule x destination x.x.x.x
         if 'rule' in pbr[route]:
             for rule, rule_config in pbr[route]['rule'].items():
-                src = leaf_node_changed(conf, base_rule + [rule, 'source'])
+                src = leaf_node_changed(conf, base_rule + [rule, 'source', 'address'])
                 fwmk = leaf_node_changed(conf, base_rule + [rule, 'fwmark'])
                 iif = leaf_node_changed(conf, base_rule + [rule, 'inbound-interface'])
-                dst = leaf_node_changed(conf, base_rule + [rule, 'destination'])
+                dst = leaf_node_changed(conf, base_rule + [rule, 'destination', 'address'])
                 proto = leaf_node_changed(conf, base_rule + [rule, 'protocol'])
                 # keep track of changes in configuration
                 # otherwise we might remove an existing node although nothing else has changed
@@ -94,7 +94,8 @@ def get_config(config=None):
                     # if a new selector is added, we have to remove all previous rules without this selector
                     # to make sure we remove all previous rules with this source(s), it will be included
                     if 'source' in rule_config:
-                        rule_def = dict_merge({'source': rule_config['source']}, rule_def)
+                        if 'address' in rule_config['source']:
+                            rule_def = dict_merge({'source': {'address': rule_config['source']['address']}}, rule_def)
                 else:
                     # if src is not None, it's previous content will be returned
                     # this can be an empty array if it's just being set, or the previous value
@@ -102,7 +103,8 @@ def get_config(config=None):
                     changed = True
                     # set the old value for removal if it's not empty
                     if len(src) > 0:
-                        rule_def = dict_merge({'source' : src}, rule_def)
+                        rule_def = dict_merge({'source': {'address': src}}, rule_def)
+
                 if fwmk is None:
                     if 'fwmark' in rule_config:
                         rule_def = dict_merge({'fwmark': rule_config['fwmark']}, rule_def)
@@ -110,6 +112,7 @@ def get_config(config=None):
                     changed = True
                     if len(fwmk) > 0:
                         rule_def = dict_merge({'fwmark' : fwmk}, rule_def)
+
                 if iif is None:
                     if 'inbound_interface' in rule_config:
                         rule_def = dict_merge({'inbound_interface': rule_config['inbound_interface']}, rule_def)
@@ -117,13 +120,16 @@ def get_config(config=None):
                     changed = True
                     if len(iif) > 0:
                         rule_def = dict_merge({'inbound_interface' : iif}, rule_def)
+
                 if dst is None:
                     if 'destination' in rule_config:
-                        rule_def = dict_merge({'destination': rule_config['destination']}, rule_def)
+                        if 'address' in rule_config['destination']:
+                            rule_def = dict_merge({'destination': {'address': rule_config['destination']['address']}}, rule_def)
                 else:
                     changed = True
                     if len(dst) > 0:
-                        rule_def = dict_merge({'destination' : dst}, rule_def)
+                        rule_def = dict_merge({'destination': {'address': dst}}, rule_def)
+
                 if proto is None:
                     if 'protocol' in rule_config:
                         rule_def = dict_merge({'protocol': rule_config['protocol']}, rule_def)
@@ -131,6 +137,7 @@ def get_config(config=None):
                     changed = True
                     if len(proto) > 0:
                         rule_def = dict_merge({'protocol' : proto}, rule_def)
+
                 if changed:
                     dict = dict_merge({dict_id : {rule : rule_def}}, dict)
                     pbr.update(dict)
@@ -184,8 +191,8 @@ def apply(pbr):
             v6 = " -6" if rule_rm == 'rule6_remove' else ""
 
             for rule, rule_config in pbr[rule_rm].items():
-                source = rule_config.get('source', [''])
-                destination = rule_config.get('destination', [''])
+                source = rule_config.get('source', {}).get('address', [''])
+                destination = rule_config.get('destination', {}).get('address', [''])
                 fwmark = rule_config.get('fwmark', [''])
                 inbound_interface = rule_config.get('inbound_interface', [''])
                 protocol = rule_config.get('protocol', [''])
@@ -210,8 +217,8 @@ def apply(pbr):
         if 'rule' in pbr_route:
             for rule, rule_config in pbr_route['rule'].items():
                 table = rule_config['set'].get('table', '')
-                source = rule_config.get('source', ['all'])
-                destination = rule_config.get('destination', ['all'])
+                source = rule_config.get('source', {}).get('address', ['all'])
+                destination = rule_config.get('destination', {}).get('address', ['all'])
                 fwmark = rule_config.get('fwmark', '')
                 inbound_interface = rule_config.get('inbound_interface', '')
                 protocol = rule_config.get('protocol', '')
diff --git a/src/migration-scripts/policy/5-to-6 b/src/migration-scripts/policy/5-to-6
new file mode 100755
index 000000000..f1545cddb
--- /dev/null
+++ b/src/migration-scripts/policy/5-to-6
@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2023 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# T5165: Migrate policy local-route rule <tag> destination|source
+
+import re
+
+from sys import argv
+from sys import exit
+
+from vyos.configtree import ConfigTree
+from vyos.ifconfig import Section
+
+if len(argv) < 2:
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+base4 = ['policy', 'local-route']
+base6 = ['policy', 'local-route6']
+config = ConfigTree(config_file)
+
+if not config.exists(base4) and not config.exists(base6):
+    # Nothing to do
+    exit(0)
+
+# replace 'policy local-route{v6} rule <tag> destination|source <x.x.x.x>'
+#      => 'policy local-route{v6} rule <tag> destination|source address <x.x.x.x>'
+for base in [base4, base6]:
+    if config.exists(base + ['rule']):
+        for rule in config.list_nodes(base + ['rule']):
+            dst_path = base + ['rule', rule, 'destination']
+            src_path = base + ['rule', rule, 'source']
+            # Destination
+            if config.exists(dst_path):
+                for dst_addr in config.return_values(dst_path):
+                    config.set(dst_path + ['address'], value=dst_addr, replace=False)
+            # Source
+            if config.exists(src_path):
+                for src_addr in config.return_values(src_path):
+                    config.set(src_path + ['address'], value=src_addr, replace=False)
+
+try:
+    with open(file_name, 'w') as f:
+        f.write(config.to_string())
+except OSError as e:
+    print("Failed to save the modified config: {}".format(e))
+    exit(1)