From a7494d54131984151d10b6d9c481120a625a98b2 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Thu, 21 Sep 2023 16:45:56 +0200
Subject: [PATCH 01/14] feat: added store_policy function

Added __store_interface helper
Added __store_member helper
---
 src/nethsec/mwan/__init__.py | 158 ++++++++++++++++++++++++++++++++
 tests/test_mwan.py           | 173 +++++++++++++++++++++++++++++++++++
 2 files changed, 331 insertions(+)
 create mode 100644 src/nethsec/mwan/__init__.py
 create mode 100644 tests/test_mwan.py

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
new file mode 100644
index 00000000..df861acb
--- /dev/null
+++ b/src/nethsec/mwan/__init__.py
@@ -0,0 +1,158 @@
+#!/usr/bin/python3
+
+#
+# Copyright (C) 2023 Nethesis S.r.l.
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+from euci import EUci
+
+from nethsec import utils
+
+
+def __generate_metric(e_uci: EUci, interface_metrics: list[int] = None, metric: int = 1) -> int:
+    """
+    Generates a metric for an interface.
+    Args:
+        e_uci: EUci instance
+        interface_metrics: list of metrics already used, will be generated if not provided
+        metric: metric to start from
+
+    Returns:
+        first metric that is not present in interface_metrics
+    """
+    if interface_metrics is None:
+        interface_metrics = list[int]()
+        for interface in utils.get_all_by_type(e_uci, 'network', 'interface').values():
+            if 'metric' in interface:
+                interface_metrics.append(int(interface['metric']))
+
+    if metric not in interface_metrics:
+        return metric
+    else:
+        return __generate_metric(e_uci, interface_metrics, metric + 1)
+
+
+def __store_interface(e_uci: EUci, name: str) -> tuple[bool, bool]:
+    """
+    Stores interface configuration for mwan3 and network, not suited to be used outside store_policy.
+    Args:
+        e_uci: EUci instance
+        name: name of interface
+
+    Returns:
+        tuple of booleans, first one indicates if mwan interface was created, second one indicates if metric was added
+        to network interface
+
+    Raises:
+        ValueError: if interface name is not defined in /etc/config/network
+    """
+    # checking if interface is configured
+    available_interfaces = utils.get_all_by_type(e_uci, 'network', 'interface')
+    if name not in available_interfaces.keys():
+        raise ValueError(name, 'invalid')
+
+    created_interface = False
+    # if no interface with name exists, create one with defaults
+    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'interface').keys():
+        created_interface = True
+        # fetch default configuration and set interface
+        default_interface_config = utils.get_all_by_type(e_uci, 'ns-api', 'defaults_mwan').get('defaults_mwan')
+        e_uci.set('mwan3', name, 'interface')
+        e_uci.set('mwan3', name, 'enabled', '1')
+        e_uci.set('mwan3', name, 'initial_state', default_interface_config['initial_state'])
+        e_uci.set('mwan3', name, 'family', default_interface_config['protocol'])
+        e_uci.set('mwan3', name, 'track_ip', default_interface_config['track_ip'])
+        e_uci.set('mwan3', name, 'track_method', default_interface_config['tracking_method'])
+        e_uci.set('mwan3', name, 'reliability', default_interface_config['tracking_reliability'])
+        e_uci.set('mwan3', name, 'count', default_interface_config['ping_count'])
+        e_uci.set('mwan3', name, 'size', default_interface_config['ping_size'])
+        e_uci.set('mwan3', name, 'max_ttl', default_interface_config['ping_max_ttl'])
+        e_uci.set('mwan3', name, 'timeout', default_interface_config['ping_timeout'])
+        e_uci.set('mwan3', name, 'interval', default_interface_config['ping_interval'])
+        e_uci.set('mwan3', name, 'failure_interval', default_interface_config['ping_failure_interval'])
+        e_uci.set('mwan3', name, 'recovery_interval', default_interface_config['ping_recovery_interval'])
+        e_uci.set('mwan3', name, 'down', default_interface_config['interface_down_threshold'])
+        e_uci.set('mwan3', name, 'up', default_interface_config['interface_up_threshold'])
+
+    added_metric = False
+    # avoid adding metric if already present
+    if 'metric' not in available_interfaces[name]:
+        added_metric = True
+        # generate metric
+        metric = __generate_metric(e_uci)
+        # configure metric for interface
+        e_uci.set('network', name, 'metric', metric)
+    return created_interface, added_metric
+
+
+def __store_member(e_uci: EUci, interface_name: str, metric: int, weight: int) -> tuple[str, bool]:
+    """
+    Stores member configuration for mwan3, not suited to be used outside store_policy.
+    Args:
+        e_uci: EUci instance
+        interface_name: name of interface to link the member to
+        metric: metric of the member
+        weight: weight of the member
+
+    Returns:
+        tuple of string and boolean, first one is the generated name of the member, second one indicates if the member
+        was created
+    """
+    member_config_name = utils.get_id(f'{interface_name}_M{metric}_W{weight}')
+    changed = False
+    if member_config_name not in utils.get_all_by_type(e_uci, 'mwan3', 'member').keys():
+        changed = True
+        e_uci.set('mwan3', member_config_name, 'member')
+        e_uci.set('mwan3', member_config_name, 'interface', interface_name)
+        e_uci.set('mwan3', member_config_name, 'metric', metric)
+        e_uci.set('mwan3', member_config_name, 'weight', weight)
+    return f'mwan3.{member_config_name}', changed
+
+
+def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
+    """
+    Stores a policy for mwan3, takes care of creating interfaces and members.
+    Args:
+        e_uci: EUci instance
+        name: name of policy
+        interfaces: list of interfaces to add to policy, must have a name, metric and weight fields
+
+    Returns:
+        list of changed configuration
+
+    Raises:
+        ValueError: if name is not unique
+    """
+    changed_config = []
+    # generate policy name
+    policy_config_name = utils.get_id(name)
+    # make sure name is not something that already exists
+    if policy_config_name in e_uci.get('mwan3').keys():
+        raise ValueError(name, 'unique')
+    # generate policy config with corresponding name
+    e_uci.set('mwan3', policy_config_name, 'policy')
+    e_uci.set('mwan3', policy_config_name, 'name', name)
+    changed_config.append(f'mwan3.{policy_config_name}')
+
+    member_names: list[str] = []
+    for interface in interfaces:
+        added_mwan_interface, updated_interface = __store_interface(e_uci, interface['name'])
+        if added_mwan_interface:
+            changed_config.append(f'mwan3.{interface["name"]}')
+        if updated_interface:
+            changed_config.append(f'network.{interface["name"]}')
+
+        member_config_name, member_created = __store_member(e_uci,
+                                                            interface['name'],
+                                                            interface['metric'],
+                                                            interface['weight'])
+        member_names.append(member_config_name)
+        if member_created:
+            changed_config.append(member_config_name)
+
+    e_uci.set('mwan3', policy_config_name, 'use_member', member_names)
+
+    e_uci.save('mwan3')
+    e_uci.save('network')
+    return changed_config
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
new file mode 100644
index 00000000..293e1a6f
--- /dev/null
+++ b/tests/test_mwan.py
@@ -0,0 +1,173 @@
+import pathlib
+
+import pytest
+from euci import EUci
+
+from nethsec import mwan
+
+network_db = """
+config interface 'loopback'
+        option device 'lo'
+        option proto 'static'
+        option ipaddr '127.0.0.1'
+        option netmask '255.0.0.0'
+
+config interface 'GREEN_1'
+        option proto 'static'
+        option device 'eth0'
+        option ipaddr '192.168.200.2'
+        option netmask '255.255.255.0'
+
+config interface 'RED_1'
+        option proto 'static'
+        option device 'eth1'
+        option ipaddr '10.0.0.2'
+        option netmask '255.255.255.0'
+        option gateway '10.0.0.1'
+
+config interface 'RED_2'
+        option proto 'static'
+        option device 'eth2'
+        option ipaddr '10.0.1.2'
+        option netmask '255.255.255.0'
+        option gateway '10.0.1.1'
+
+config interface 'RED_3'
+        option proto 'static'
+        option device 'eth3'
+        option ipaddr '10.0.2.2'
+        option netmask '255.255.255.0'
+        option gateway '10.0.2.1'
+
+config device
+        option name 'eth0'
+
+config device
+        option name 'eth1'
+
+config device
+        option name 'eth2'
+
+config device
+        option name 'eth3'
+"""
+
+ns_api_db = """
+config defaults_mwan 'defaults_mwan'
+        option initial_state 'online'
+        option protocol 'ipv4'
+        list track_ip '8.8.8.8'
+        list track_ip '208.67.222.222'
+        option tracking_method 'ping'
+        option tracking_reliability '1'
+        option ping_count '1'
+        option ping_size '56'
+        option ping_max_ttl '60'
+        option ping_timeout '4'
+        option ping_interval '10'
+        option ping_failure_interval '5'
+        option ping_recovery_interval '5'
+        option interface_down_threshold '5'
+        option interface_up_threshold '5'
+        option link_quality '0'
+        option quality_failure_latency '1000'
+        option quality_failure_packet_loss '40'
+        option quality_recovery_latency '500'
+        option quality_recovery_packet_loss '10'
+"""
+
+
+@pytest.fixture
+def e_uci(tmp_path: pathlib.Path) -> EUci:
+    conf_dir = tmp_path.joinpath('conf')
+    conf_dir.mkdir()
+    save_dir = tmp_path.joinpath('save')
+    save_dir.mkdir()
+    with conf_dir.joinpath('network').open('w') as fp:
+        fp.write(network_db)
+    with conf_dir.joinpath('ns-api').open('w') as fp:
+        fp.write(ns_api_db)
+    with conf_dir.joinpath('mwan3').open('w') as fp:
+        fp.write('')
+    return EUci(confdir=conf_dir.as_posix(), savedir=save_dir.as_posix())
+
+
+def test_create_interface(e_uci):
+    assert mwan.__store_interface(e_uci, 'RED_1') == (True, True)
+    assert mwan.__store_interface(e_uci, 'RED_2') == (True, True)
+    # assert every interface has defaults
+    assert e_uci.get('mwan3', 'RED_1') == 'interface'
+    assert e_uci.get('mwan3', 'RED_1', 'enabled') == '1'
+    assert e_uci.get('mwan3', 'RED_1', 'initial_state') == 'online'
+    assert e_uci.get('mwan3', 'RED_1', 'family') == 'ipv4'
+    assert e_uci.get('mwan3', 'RED_1', 'track_ip', list=True) == ('8.8.8.8', '208.67.222.222')
+    assert e_uci.get('mwan3', 'RED_1', 'track_method') == 'ping'
+    assert e_uci.get('mwan3', 'RED_1', 'reliability') == '1'
+    assert e_uci.get('mwan3', 'RED_1', 'count') == '1'
+    assert e_uci.get('mwan3', 'RED_1', 'size') == '56'
+    assert e_uci.get('mwan3', 'RED_1', 'max_ttl') == '60'
+    assert e_uci.get('mwan3', 'RED_1', 'timeout') == '4'
+    assert e_uci.get('mwan3', 'RED_1', 'interval') == '10'
+    assert e_uci.get('mwan3', 'RED_1', 'failure_interval') == '5'
+    assert e_uci.get('mwan3', 'RED_1', 'recovery_interval') == '5'
+    assert e_uci.get('mwan3', 'RED_1', 'down') == '5'
+    assert e_uci.get('mwan3', 'RED_1', 'up') == '5'
+    # assert interface has metric
+    assert e_uci.get('network', 'RED_1', 'metric') == '1'
+    assert e_uci.get('network', 'RED_2', 'metric') == '2'
+    assert mwan.__store_interface(e_uci, 'RED_1') == (False, False)
+
+
+def test_fail_create_invalid_interface(e_uci):
+    with pytest.raises(ValueError):
+        mwan.__store_interface(e_uci, 'RED_4')
+
+
+def test_interface_avoid_edit_of_metric(e_uci):
+    e_uci.set('network', 'RED_1', 'metric', '10')
+    assert mwan.__store_interface(e_uci, 'RED_1') == (True, False)
+
+
+def test_create_member(e_uci):
+    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('mwan3.ns_RED_1_M10_W100', True)
+    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('mwan3.ns_RED_1_M10_W100', False)
+    assert mwan.__store_member(e_uci, 'RED_1', 1, 100) == ('mwan3.ns_RED_1_M1_W100', True)
+
+
+def test_create_default_mwan(e_uci):
+    assert mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '200',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '20',
+            'weight': '100',
+        }
+    ]) == ['mwan3.ns_default',
+           'mwan3.RED_1',
+           'network.RED_1',
+           'mwan3.ns_RED_1_M10_W200',
+           'mwan3.RED_2',
+           'network.RED_2',
+           'mwan3.ns_RED_2_M20_W100']
+
+    assert e_uci.get('mwan3', 'ns_default') == 'policy'
+    assert e_uci.get('mwan3', 'ns_default', 'name') == 'default'
+    assert e_uci.get('mwan3', 'ns_default', 'use_member', list=True) == (
+        'mwan3.ns_RED_1_M10_W100', 'mwan3.ns_RED_2_M10_W100')
+
+
+def test_create_unique_mwan(e_uci):
+    mwan.store_policy(e_uci, 'this', [])
+    with pytest.raises(ValueError):
+        mwan.store_policy(e_uci, 'this', [])
+
+
+def test_metric_generation(e_uci):
+    assert mwan.__generate_metric(e_uci) == 1
+    assert mwan.__generate_metric(e_uci, [1, 4]) == 2
+    assert mwan.__generate_metric(e_uci, [1, 2, 4]) == 3
+    assert mwan.__generate_metric(e_uci, [4, 3, 1]) == 2

From 97691ca175fda0e0ea6a7e85c9a7da2fe1838dd0 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Thu, 21 Sep 2023 17:03:46 +0200
Subject: [PATCH 02/14] fix: typo in tests

---
 tests/test_mwan.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 293e1a6f..f5adcbe7 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -157,7 +157,7 @@ def test_create_default_mwan(e_uci):
     assert e_uci.get('mwan3', 'ns_default') == 'policy'
     assert e_uci.get('mwan3', 'ns_default', 'name') == 'default'
     assert e_uci.get('mwan3', 'ns_default', 'use_member', list=True) == (
-        'mwan3.ns_RED_1_M10_W100', 'mwan3.ns_RED_2_M10_W100')
+        'mwan3.ns_RED_1_M10_W200', 'mwan3.ns_RED_2_M20_W100')
 
 
 def test_create_unique_mwan(e_uci):

From 4b8a500d2321d77f0e8856457187df2966d594d3 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Fri, 22 Sep 2023 09:54:11 +0200
Subject: [PATCH 03/14] fix: fixed configuration insertion for members

---
 src/nethsec/mwan/__init__.py | 4 ++--
 tests/test_mwan.py           | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index df861acb..6ffbbbdb 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -107,7 +107,7 @@ def __store_member(e_uci: EUci, interface_name: str, metric: int, weight: int) -
         e_uci.set('mwan3', member_config_name, 'interface', interface_name)
         e_uci.set('mwan3', member_config_name, 'metric', metric)
         e_uci.set('mwan3', member_config_name, 'weight', weight)
-    return f'mwan3.{member_config_name}', changed
+    return member_config_name, changed
 
 
 def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
@@ -149,7 +149,7 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
                                                             interface['weight'])
         member_names.append(member_config_name)
         if member_created:
-            changed_config.append(member_config_name)
+            changed_config.append(f'mwan3.{member_config_name}')
 
     e_uci.set('mwan3', policy_config_name, 'use_member', member_names)
 
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index f5adcbe7..0bcc2f48 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -129,9 +129,9 @@ def test_interface_avoid_edit_of_metric(e_uci):
 
 
 def test_create_member(e_uci):
-    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('mwan3.ns_RED_1_M10_W100', True)
-    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('mwan3.ns_RED_1_M10_W100', False)
-    assert mwan.__store_member(e_uci, 'RED_1', 1, 100) == ('mwan3.ns_RED_1_M1_W100', True)
+    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('ns_RED_1_M10_W100', True)
+    assert mwan.__store_member(e_uci, 'RED_1', 10, 100) == ('ns_RED_1_M10_W100', False)
+    assert mwan.__store_member(e_uci, 'RED_1', 1, 100) == ('ns_RED_1_M1_W100', True)
 
 
 def test_create_default_mwan(e_uci):
@@ -157,7 +157,7 @@ def test_create_default_mwan(e_uci):
     assert e_uci.get('mwan3', 'ns_default') == 'policy'
     assert e_uci.get('mwan3', 'ns_default', 'name') == 'default'
     assert e_uci.get('mwan3', 'ns_default', 'use_member', list=True) == (
-        'mwan3.ns_RED_1_M10_W200', 'mwan3.ns_RED_2_M20_W100')
+        'ns_RED_1_M10_W200', 'ns_RED_2_M20_W100')
 
 
 def test_create_unique_mwan(e_uci):

From d1dfc306309b0910cc715737c5a63625ec01e003 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Mon, 25 Sep 2023 16:00:46 +0200
Subject: [PATCH 04/14] feat: added functions

Added index for policies
Added store for policies and rules
Added tests
Added ValidationError Exception
---
 src/nethsec/mwan/__init__.py  | 157 +++++++++++++++++++++++++++++++---
 src/nethsec/utils/__init__.py |   7 ++
 tests/test_mwan.py            | 122 +++++++++++++++++++++++++-
 3 files changed, 272 insertions(+), 14 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index 6ffbbbdb..04aaee0d 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -1,14 +1,18 @@
 #!/usr/bin/python3
+import json
+import subprocess
+
+from euci import EUci
+
+from nethsec import utils
+from nethsec.utils import ValidationError
+
 
 #
 # Copyright (C) 2023 Nethesis S.r.l.
 # SPDX-License-Identifier: GPL-2.0-only
 #
 
-from euci import EUci
-
-from nethsec import utils
-
 
 def __generate_metric(e_uci: EUci, interface_metrics: list[int] = None, metric: int = 1) -> int:
     """
@@ -45,12 +49,12 @@ def __store_interface(e_uci: EUci, name: str) -> tuple[bool, bool]:
         to network interface
 
     Raises:
-        ValueError: if interface name is not defined in /etc/config/network
+        ValidationError: if interface name is not defined in /etc/config/network
     """
     # checking if interface is configured
     available_interfaces = utils.get_all_by_type(e_uci, 'network', 'interface')
     if name not in available_interfaces.keys():
-        raise ValueError(name, 'invalid')
+        raise ValidationError('name', 'invalid', name)
 
     created_interface = False
     # if no interface with name exists, create one with defaults
@@ -110,34 +114,81 @@ def __store_member(e_uci: EUci, interface_name: str, metric: int, weight: int) -
     return member_config_name, changed
 
 
+def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
+               source_addresses: str = None, source_ports: str = None,
+               destination_addresses: str = None, destination_ports: str = None) -> str:
+    """
+    Stores a rule for mwan3
+    Args:
+        e_uci: EUci instance
+        name: name of the rule, must be unique
+        policy: policy to use for the rule, must be already set
+        protocol: must be one of tcp, udp, icmp or all, defaults to 'all'
+        source_addresses: source addresses to match
+        source_ports: source ports to match or range
+        destination_addresses: destination addresses to match
+        destination_ports: destination ports to match or range
+
+    Returns:
+        name of the rule created
+
+    Raises:
+        ValidationError if name is not unique or policy is not valid
+    """
+    rule_config_name = utils.get_id(name, 15)
+    if rule_config_name in e_uci.get('mwan3').keys():
+        raise ValidationError('name', 'unique', name)
+    if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+        raise ValidationError('policy', 'invalid', policy)
+    e_uci.set('mwan3', rule_config_name, 'rule')
+    e_uci.set('mwan3', rule_config_name, 'label', name)
+    e_uci.set('mwan3', rule_config_name, 'use_policy', policy)
+    if protocol is not None:
+        e_uci.set('mwan3', rule_config_name, 'proto', protocol)
+    if source_addresses is not None:
+        e_uci.set('mwan3', rule_config_name, 'src_ip', source_addresses)
+    if source_ports is not None:
+        e_uci.set('mwan3', rule_config_name, 'src_port', source_ports)
+    if destination_addresses is not None:
+        e_uci.set('mwan3', rule_config_name, 'dest_ip', destination_addresses)
+    if destination_ports is not None:
+        e_uci.set('mwan3', rule_config_name, 'dest_port', destination_ports)
+
+    e_uci.save('mwan3')
+    return f'mwan3.{rule_config_name}'
+
+
 def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
     """
     Stores a policy for mwan3, takes care of creating interfaces and members.
     Args:
         e_uci: EUci instance
         name: name of policy
-        interfaces: list of interfaces to add to policy, must have a name, metric and weight fields
+        interfaces: list of interfaces to add to policy, must have name, metric and weight fields
 
     Returns:
         list of changed configuration
 
     Raises:
-        ValueError: if name is not unique
+        ValidationError: if name is not unique
     """
     changed_config = []
     # generate policy name
     policy_config_name = utils.get_id(name)
     # make sure name is not something that already exists
     if policy_config_name in e_uci.get('mwan3').keys():
-        raise ValueError(name, 'unique')
+        raise ValidationError('name', 'unique', name)
     # generate policy config with corresponding name
     e_uci.set('mwan3', policy_config_name, 'policy')
-    e_uci.set('mwan3', policy_config_name, 'name', name)
+    e_uci.set('mwan3', policy_config_name, 'label', name)
     changed_config.append(f'mwan3.{policy_config_name}')
 
     member_names: list[str] = []
     for interface in interfaces:
-        added_mwan_interface, updated_interface = __store_interface(e_uci, interface['name'])
+        try:
+            added_mwan_interface, updated_interface = __store_interface(e_uci, interface['name'])
+        except ValidationError:
+            raise ValidationError('interfaces', 'invalid', interface['name'])
         if added_mwan_interface:
             changed_config.append(f'mwan3.{interface["name"]}')
         if updated_interface:
@@ -153,6 +204,90 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
 
     e_uci.set('mwan3', policy_config_name, 'use_member', member_names)
 
+    if len(utils.get_all_by_type(e_uci, 'mwan3', 'rule')) == 0:
+        changed_config.append(store_rule(e_uci, 'Default Rule', policy_config_name))
+
     e_uci.save('mwan3')
     e_uci.save('network')
     return changed_config
+
+
+def __fetch_interface_status(interface_name: str) -> str:
+    try:
+        output = subprocess.check_output([
+            'ubus',
+            'call',
+            'mwan3',
+            'status',
+            '{"section": "interfaces"}'
+        ]).decode('utf-8')
+        decoded_output = json.JSONDecoder().decode(output)
+        return decoded_output['interfaces'][interface_name]['status']
+    except:
+        return 'unknown'
+
+
+def __parse_member(e_uci: EUci, member_name: str) -> dict:
+    """
+    Parses a member configuration and returns formatted data.
+    Args:
+        e_uci: EUci instance
+        member_name: member name
+
+    Returns:
+        dict with member data
+    """
+    member_data = e_uci.get_all('mwan3', member_name)
+    return {
+        'name': member_name,
+        'interface': member_data['interface'],
+        'metric': member_data['metric'],
+        'weight': member_data['weight'],
+        'status': __fetch_interface_status(member_data['interface'])
+    }
+
+
+def index_policies(e_uci: EUci) -> list[dict]:
+    """
+    Returns a list of policies with their members, interfaces and metrics/weights.
+    Args:
+        e_uci: EUci instance
+
+    Returns:
+        list of dicts with policy data
+    """
+    data = []
+    policies = utils.get_all_by_type(e_uci, 'mwan3', 'policy')
+    # iterate over policies
+    for policy_name in policies.keys():
+        policy = policies[policy_name]
+        policy_data = {
+            'name': policy_name,
+        }
+        # add label only if present
+        if 'label' in policy:
+            policy_data['label'] = policy['label']
+
+        # add members
+        members = []
+        if 'use_member' in policy:
+            policy_data['members'] = {}
+            for member in policy['use_member']:
+                members.append(__parse_member(e_uci, member))
+
+        # infer policy type by metrics
+        metrics = [int(member['metric']) for member in members]
+        if all(metric == metrics[0] for metric in metrics):
+            policy_data['type'] = 'balance'
+        elif all(metrics.index(metric) == key for key, metric in enumerate(metrics)):
+            policy_data['type'] = 'backup'
+        else:
+            policy_data['type'] = 'custom'
+
+        unique_metrics = list(set(metrics))
+        for unique_metric in unique_metrics:
+            policy_data['members'][unique_metric] = list(filter(lambda x: x['metric'] == str(unique_metric), members))
+
+        # append policy to data
+        data.append(policy_data)
+    return data
diff --git a/src/nethsec/utils/__init__.py b/src/nethsec/utils/__init__.py
index 7db0b107..a2a805cf 100644
--- a/src/nethsec/utils/__init__.py
+++ b/src/nethsec/utils/__init__.py
@@ -370,3 +370,10 @@ def generic_error(error):
       - A validation error object
     '''
     return {"error": error.strip().replace(" ", "_").lower()}
+
+
+class ValidationError(ValueError):
+    def __init__(self, parameter, message="", value=""):
+        self.parameter = parameter
+        self.message = message
+        self.value = value
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 0bcc2f48..ec8be300 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -119,8 +119,9 @@ def test_create_interface(e_uci):
 
 
 def test_fail_create_invalid_interface(e_uci):
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError) as err:
         mwan.__store_interface(e_uci, 'RED_4')
+        assert err.value.args[0] == ('RED_4', 'invalid')
 
 
 def test_interface_avoid_edit_of_metric(e_uci):
@@ -152,10 +153,11 @@ def test_create_default_mwan(e_uci):
            'mwan3.ns_RED_1_M10_W200',
            'mwan3.RED_2',
            'network.RED_2',
-           'mwan3.ns_RED_2_M20_W100']
+           'mwan3.ns_RED_2_M20_W100',
+           'mwan3.ns_Default_Rule']
 
     assert e_uci.get('mwan3', 'ns_default') == 'policy'
-    assert e_uci.get('mwan3', 'ns_default', 'name') == 'default'
+    assert e_uci.get('mwan3', 'ns_default', 'label') == 'default'
     assert e_uci.get('mwan3', 'ns_default', 'use_member', list=True) == (
         'ns_RED_1_M10_W200', 'ns_RED_2_M20_W100')
 
@@ -171,3 +173,117 @@ def test_metric_generation(e_uci):
     assert mwan.__generate_metric(e_uci, [1, 4]) == 2
     assert mwan.__generate_metric(e_uci, [1, 2, 4]) == 3
     assert mwan.__generate_metric(e_uci, [4, 3, 1]) == 2
+
+
+def test_list_policies(e_uci):
+    mwan.store_policy(e_uci, 'backup', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '200',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '20',
+            'weight': '100',
+        }
+    ])
+    mwan.store_policy(e_uci, 'balance', [
+        {
+            'name': 'RED_3',
+            'metric': '10',
+            'weight': '200',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    mwan.store_policy(e_uci, 'custom', [
+        {
+            'name': 'RED_3',
+            'metric': '10',
+            'weight': '200',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_1',
+            'metric': '20',
+            'weight': '100',
+        }
+    ])
+    index = mwan.index_policies(e_uci)
+    # check backup policy
+    assert index[0]['name'] == 'ns_backup'
+    assert index[0]['label'] == 'backup'
+    assert index[0]['type'] == 'backup'
+    assert index[0]['members'][10][0]['name'] == 'ns_RED_1_M10_W200'
+    assert index[0]['members'][10][0]['interface'] == 'RED_1'
+    assert index[0]['members'][10][0]['metric'] == '10'
+    assert index[0]['members'][10][0]['weight'] == '200'
+    assert index[0]['members'][20][0]['name'] == 'ns_RED_2_M20_W100'
+    assert index[0]['members'][20][0]['interface'] == 'RED_2'
+    assert index[0]['members'][20][0]['metric'] == '20'
+    assert index[0]['members'][20][0]['weight'] == '100'
+    # check balance policy
+    assert index[1]['name'] == 'ns_balance'
+    assert index[1]['label'] == 'balance'
+    assert index[1]['type'] == 'balance'
+    assert index[1]['members'][10][0]['name'] == 'ns_RED_3_M10_W200'
+    assert index[1]['members'][10][0]['interface'] == 'RED_3'
+    assert index[1]['members'][10][0]['metric'] == '10'
+    assert index[1]['members'][10][0]['weight'] == '200'
+    assert index[1]['members'][10][1]['name'] == 'ns_RED_2_M10_W100'
+    assert index[1]['members'][10][1]['interface'] == 'RED_2'
+    assert index[1]['members'][10][1]['metric'] == '10'
+    assert index[1]['members'][10][1]['weight'] == '100'
+    # check custom policy
+    assert index[2]['name'] == 'ns_custom'
+    assert index[2]['label'] == 'custom'
+    assert index[2]['type'] == 'custom'
+    assert index[2]['members'][10][0]['name'] == 'ns_RED_3_M10_W200'
+    assert index[2]['members'][10][0]['interface'] == 'RED_3'
+    assert index[2]['members'][10][0]['metric'] == '10'
+    assert index[2]['members'][10][0]['weight'] == '200'
+    assert index[2]['members'][10][1]['name'] == 'ns_RED_2_M10_W100'
+    assert index[2]['members'][10][1]['interface'] == 'RED_2'
+    assert index[2]['members'][10][1]['metric'] == '10'
+    assert index[2]['members'][10][1]['weight'] == '100'
+    assert index[2]['members'][20][0]['name'] == 'ns_RED_1_M20_W100'
+    assert index[2]['members'][20][0]['interface'] == 'RED_1'
+    assert index[2]['members'][20][0]['metric'] == '20'
+    assert index[2]['members'][20][0]['weight'] == '100'
+
+
+def test_store_rule(e_uci):
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '20',
+            'weight': '100',
+        }
+    ])
+    assert mwan.store_rule(e_uci, 'additional rule', 'ns_default') == 'mwan3.ns_additional_r'
+    assert e_uci.get('mwan3', 'ns_additional_r') == 'rule'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'label') == 'additional rule'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'use_policy') == 'ns_default'
+
+
+def test_unique_rule(e_uci):
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '20',
+            'weight': '100',
+        }
+    ])
+    with pytest.raises(ValueError) as e:
+        mwan.store_rule(e_uci, 'additional rule', 'ns_default')
+        mwan.store_rule(e_uci, 'additional rule', 'ns_default')
+        assert e.value.args[0] == 'name'
+        assert e.value.args[1] == 'invalid'

From 8d37815f2ce3203086636a274d0408c378a4ca2a Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Mon, 25 Sep 2023 16:36:49 +0200
Subject: [PATCH 05/14] fix: using lowercase identifiers

---
 src/nethsec/mwan/__init__.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index 04aaee0d..c5f1c1c0 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -135,13 +135,14 @@ def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
     Raises:
         ValidationError if name is not unique or policy is not valid
     """
-    rule_config_name = utils.get_id(name, 15)
+    rule_config_name = utils.get_id(name.lower(), 15)
     if rule_config_name in e_uci.get('mwan3').keys():
         raise ValidationError('name', 'unique', name)
     if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
         raise ValidationError('policy', 'invalid', policy)
     e_uci.set('mwan3', rule_config_name, 'rule')
     e_uci.set('mwan3', rule_config_name, 'label', name)
+    e_uci.set('mwan3', rule_config_name, 'label', name)
     e_uci.set('mwan3', rule_config_name, 'use_policy', policy)
     if protocol is not None:
         e_uci.set('mwan3', rule_config_name, 'proto', protocol)
@@ -174,7 +175,7 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
     """
     changed_config = []
     # generate policy name
-    policy_config_name = utils.get_id(name)
+    policy_config_name = utils.get_id(name.lower())
     # make sure name is not something that already exists
     if policy_config_name in e_uci.get('mwan3').keys():
         raise ValidationError('name', 'unique', name)

From bda3273202c43ad804a3599d94d64512172c7955 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Mon, 25 Sep 2023 16:56:44 +0200
Subject: [PATCH 06/14] fix: typo in tests after config name changes

---
 tests/test_mwan.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index ec8be300..1def2d6b 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -154,7 +154,7 @@ def test_create_default_mwan(e_uci):
            'mwan3.RED_2',
            'network.RED_2',
            'mwan3.ns_RED_2_M20_W100',
-           'mwan3.ns_Default_Rule']
+           'mwan3.ns_default_rule']
 
     assert e_uci.get('mwan3', 'ns_default') == 'policy'
     assert e_uci.get('mwan3', 'ns_default', 'label') == 'default'

From ba3fd660be55d8d72e97f75fe0260ded5782d7ce Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Mon, 25 Sep 2023 16:57:37 +0200
Subject: [PATCH 07/14] feat: added delete api

---
 src/nethsec/mwan/__init__.py |  8 ++++++++
 tests/test_mwan.py           | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index c5f1c1c0..0c65020c 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -292,3 +292,11 @@ def index_policies(e_uci: EUci) -> list[dict]:
         # append policy to data
         data.append(policy_data)
     return data
+
+
+def delete_policy(e_uci: EUci, name: str) -> list[str]:
+    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+        raise ValidationError('name', 'invalid', name)
+    e_uci.delete('mwan3', name)
+    e_uci.save('mwan3')
+    return [f'mwan3.{name}']
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 1def2d6b..1bcad9a1 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -4,6 +4,7 @@
 from euci import EUci
 
 from nethsec import mwan
+from nethsec.utils import ValidationError
 
 network_db = """
 config interface 'loopback'
@@ -287,3 +288,23 @@ def test_unique_rule(e_uci):
         mwan.store_rule(e_uci, 'additional rule', 'ns_default')
         assert e.value.args[0] == 'name'
         assert e.value.args[1] == 'invalid'
+
+
+def test_delete_non_existent_policy(e_uci):
+    with pytest.raises(ValidationError) as e:
+        mwan.delete_policy(e_uci, 'ns_default')
+        assert e.value.args[0] == 'name'
+        assert e.value.args[1] == 'invalid'
+        assert e.value.args[2] == 'ns_default'
+
+
+def test_delete_policy(e_uci):
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '20',
+            'weight': '100',
+        }
+    ])
+    assert mwan.delete_policy(e_uci, 'ns_default') == ['mwan3.ns_default']
+    assert e_uci.get('mwan3', 'ns_default', default=None) is None

From 76b2cc954b49746b02081c4f44b441d98b0f97ce Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Wed, 27 Sep 2023 10:46:55 +0200
Subject: [PATCH 08/14] feat: added edit_policy

refactored adding interface
---
 src/nethsec/mwan/__init__.py | 73 ++++++++++++++++++++++++------------
 tests/test_mwan.py           | 38 +++++++++++++++++++
 2 files changed, 87 insertions(+), 24 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index 0c65020c..c73d07c1 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -1,4 +1,10 @@
 #!/usr/bin/python3
+
+#
+# Copyright (C) 2023 Nethesis S.r.l.
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
 import json
 import subprocess
 
@@ -8,12 +14,6 @@
 from nethsec.utils import ValidationError
 
 
-#
-# Copyright (C) 2023 Nethesis S.r.l.
-# SPDX-License-Identifier: GPL-2.0-only
-#
-
-
 def __generate_metric(e_uci: EUci, interface_metrics: list[int] = None, metric: int = 1) -> int:
     """
     Generates a metric for an interface.
@@ -184,24 +184,7 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
     e_uci.set('mwan3', policy_config_name, 'label', name)
     changed_config.append(f'mwan3.{policy_config_name}')
 
-    member_names: list[str] = []
-    for interface in interfaces:
-        try:
-            added_mwan_interface, updated_interface = __store_interface(e_uci, interface['name'])
-        except ValidationError:
-            raise ValidationError('interfaces', 'invalid', interface['name'])
-        if added_mwan_interface:
-            changed_config.append(f'mwan3.{interface["name"]}')
-        if updated_interface:
-            changed_config.append(f'network.{interface["name"]}')
-
-        member_config_name, member_created = __store_member(e_uci,
-                                                            interface['name'],
-                                                            interface['metric'],
-                                                            interface['weight'])
-        member_names.append(member_config_name)
-        if member_created:
-            changed_config.append(f'mwan3.{member_config_name}')
+    member_names = __add_interfaces(e_uci, interfaces, changed_config)
 
     e_uci.set('mwan3', policy_config_name, 'use_member', member_names)
 
@@ -294,6 +277,48 @@ def index_policies(e_uci: EUci) -> list[dict]:
     return data
 
 
+def __add_interfaces(e_uci: EUci, interfaces: list[dict], changed_config: list[str] = None) -> list[str]:
+    if changed_config is None:
+        changed_config = list()
+    member_names: list[str] = []
+    for interface in interfaces:
+        try:
+            added_mwan_interface, updated_interface = __store_interface(e_uci, interface['name'])
+        except ValidationError:
+            raise ValidationError('interfaces', 'invalid', interface['name'])
+        if added_mwan_interface:
+            changed_config.append(f'mwan3.{interface["name"]}')
+        if updated_interface:
+            changed_config.append(f'network.{interface["name"]}')
+
+        member_config_name, member_created = __store_member(e_uci,
+                                                            interface['name'],
+                                                            interface['metric'],
+                                                            interface['weight'])
+        member_names.append(member_config_name)
+        if member_created:
+            changed_config.append(f'mwan3.{member_config_name}')
+
+    return member_names
+
+
+def edit_policy(e_uci: EUci, name: str, label: str, interfaces: list[dict]) -> list[str]:
+    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+        raise ValidationError('name', 'invalid', name)
+    changed_config = []
+    if label != e_uci.get_all('mwan3', name)['label']:
+        e_uci.set('mwan3', name, 'label', label)
+        changed_config.append(f'mwan3.{name}')
+
+    member_names = __add_interfaces(e_uci, interfaces, changed_config)
+
+    e_uci.set('mwan3', name, 'use_member', member_names)
+
+    e_uci.save('mwan3')
+    e_uci.save('network')
+    return changed_config
+
+
 def delete_policy(e_uci: EUci, name: str) -> list[str]:
     if name not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
         raise ValidationError('name', 'invalid', name)
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 1bcad9a1..c6d88453 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -308,3 +308,41 @@ def test_delete_policy(e_uci):
     ])
     assert mwan.delete_policy(e_uci, 'ns_default') == ['mwan3.ns_default']
     assert e_uci.get('mwan3', 'ns_default', default=None) is None
+
+
+def test_edit_policy(e_uci):
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    assert mwan.index_policies(e_uci)[0]['type'] == 'balance'
+    assert mwan.edit_policy(e_uci, 'ns_default', 'new label', [
+        {
+            'name': 'RED_1',
+            'metric': '20',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_3',
+            'metric': '10',
+            'weight': '100',
+        }
+    ]) == ['mwan3.ns_default', 'mwan3.ns_RED_1_M20_W100', 'mwan3.RED_3', 'network.RED_3', 'mwan3.ns_RED_3_M10_W100']
+    assert e_uci.get('mwan3', 'ns_default', 'label') == 'new label'
+    assert mwan.index_policies(e_uci)[0]['type'] == 'backup'
+
+
+def test_missing_policy(e_uci):
+    with pytest.raises(ValidationError) as e:
+        mwan.edit_policy(e_uci, 'dummy', '', [])
+        assert e.value[0] == 'name'
+        assert e.value[1] == 'invalid'
+        assert e.value[2] == 'dummy'

From 936c052d1afff48e7cc34ccefd4985410a490d28 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Wed, 27 Sep 2023 15:19:01 +0200
Subject: [PATCH 09/14] feat: added index_rules

---
 src/nethsec/mwan/__init__.py | 24 ++++++++++++++++
 tests/test_mwan.py           | 53 ++++++++++++++++++++++++++++++------
 2 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index c73d07c1..f0f36864 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -325,3 +325,27 @@ def delete_policy(e_uci: EUci, name: str) -> list[str]:
     e_uci.delete('mwan3', name)
     e_uci.save('mwan3')
     return [f'mwan3.{name}']
+
+
+def index_rules(e_uci: EUci) -> list[dict]:
+    data = []
+    rules = utils.get_all_by_type(e_uci, 'mwan3', 'rule')
+    for rule_key in rules.keys():
+        rule_data = {}
+        rule_value = rules[rule_key]
+        rule_data['name'] = rule_key
+        rule_data['policy'] = {}
+        rule_data['policy']['name'] = rule_value['use_policy']
+        if rule_value['use_policy'] in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+            rule_data['policy']['label'] = utils.get_all_by_type(e_uci, 'mwan3', 'policy')[rule_value['use_policy']]['label']
+        if 'label' in rule_value:
+            rule_data['label'] = rule_value['label']
+        if 'proto' in rule_value:
+            rule_data['protocol'] = rule_value['proto']
+        if 'src_ip' in rule_value:
+            rule_data['source_addresses'] = rule_value['src_ip']
+        if 'dest_ip' in rule_value:
+            rule_data['destination_addresses'] = rule_value['dest_ip']
+
+        data.append(rule_data)
+    return data
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index c6d88453..617cc9d7 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -122,7 +122,9 @@ def test_create_interface(e_uci):
 def test_fail_create_invalid_interface(e_uci):
     with pytest.raises(ValueError) as err:
         mwan.__store_interface(e_uci, 'RED_4')
-        assert err.value.args[0] == ('RED_4', 'invalid')
+    assert err.value.args[0] == 'name'
+    assert err.value.args[1] == 'invalid'
+    assert err.value.args[2] == 'RED_4'
 
 
 def test_interface_avoid_edit_of_metric(e_uci):
@@ -286,16 +288,16 @@ def test_unique_rule(e_uci):
     with pytest.raises(ValueError) as e:
         mwan.store_rule(e_uci, 'additional rule', 'ns_default')
         mwan.store_rule(e_uci, 'additional rule', 'ns_default')
-        assert e.value.args[0] == 'name'
-        assert e.value.args[1] == 'invalid'
+    assert e.value.args[0] == 'name'
+    assert e.value.args[1] == 'unique'
 
 
 def test_delete_non_existent_policy(e_uci):
     with pytest.raises(ValidationError) as e:
         mwan.delete_policy(e_uci, 'ns_default')
-        assert e.value.args[0] == 'name'
-        assert e.value.args[1] == 'invalid'
-        assert e.value.args[2] == 'ns_default'
+    assert e.value.args[0] == 'name'
+    assert e.value.args[1] == 'invalid'
+    assert e.value.args[2] == 'ns_default'
 
 
 def test_delete_policy(e_uci):
@@ -343,6 +345,39 @@ def test_edit_policy(e_uci):
 def test_missing_policy(e_uci):
     with pytest.raises(ValidationError) as e:
         mwan.edit_policy(e_uci, 'dummy', '', [])
-        assert e.value[0] == 'name'
-        assert e.value[1] == 'invalid'
-        assert e.value[2] == 'dummy'
+    assert e.value.args[0] == 'name'
+    assert e.value.args[1] == 'invalid'
+    assert e.value.args[2] == 'dummy'
+
+
+def test_index_rules(e_uci):
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    mwan.store_rule(e_uci, 'additional rule', 'ns_default')
+    index = mwan.index_rules(e_uci)
+    assert index[0] == {
+        'name': 'ns_default_rule',
+        'label': 'Default Rule',
+        'policy': {
+            'name': 'ns_default',
+            'label': 'default',
+        }
+    }
+    assert index[1] == {
+        'name': 'ns_additional_r',
+        'label': 'additional rule',
+        'policy': {
+            'name': 'ns_default',
+            'label': 'default',
+        }
+    }

From 9e57093a1260b0f6af0292862cd109f4a902e495 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Wed, 27 Sep 2023 17:42:43 +0200
Subject: [PATCH 10/14] feat: added order_rules

---
 src/nethsec/mwan/__init__.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index f0f36864..dad77e56 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -349,3 +349,31 @@ def index_rules(e_uci: EUci) -> list[dict]:
 
         data.append(rule_data)
     return data
+
+
+def order_rules(e_uci: EUci, rules: list[str]) -> list[str]:
+    for rule in utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys():
+        if rule not in rules:
+            raise ValidationError('rules', 'missing', rule)
+
+    order: list[str] = []
+
+    for key in e_uci.get_all('mwan3').keys():
+        if key not in rules:
+            order.append(key)
+
+    order.extend(rules)
+
+    subprocess.check_output([
+        'ubus',
+        'call',
+        'uci',
+        'order',
+        json.dumps({
+            'config': 'mwan3',
+            'sections': order,
+        })
+    ])
+
+    e_uci.save('mwan3')
+    return order

From fbcaa08b9e6ae58b63ab9a1e7b321dd420cf15a6 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Thu, 28 Sep 2023 12:45:12 +0200
Subject: [PATCH 11/14] feat: updated store_role

---
 src/nethsec/mwan/__init__.py | 45 ++++++++++++++++++--------------
 tests/test_mwan.py           | 50 +++++++++++++++++++++++++++---------
 2 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index dad77e56..b2c7f2f5 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -115,8 +115,8 @@ def __store_member(e_uci: EUci, interface_name: str, metric: int, weight: int) -
 
 
 def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
-               source_addresses: str = None, source_ports: str = None,
-               destination_addresses: str = None, destination_ports: str = None) -> str:
+               source_address: str = None, source_port: str = None,
+               destination_address: str = None, destination_port: str = None) -> str:
     """
     Stores a rule for mwan3
     Args:
@@ -124,10 +124,10 @@ def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
         name: name of the rule, must be unique
         policy: policy to use for the rule, must be already set
         protocol: must be one of tcp, udp, icmp or all, defaults to 'all'
-        source_addresses: source addresses to match
-        source_ports: source ports to match or range
-        destination_addresses: destination addresses to match
-        destination_ports: destination ports to match or range
+        source_address: source addresses to match
+        source_port: source ports to match or range
+        destination_address: destination addresses to match
+        destination_port: destination ports to match or range
 
     Returns:
         name of the rule created
@@ -136,6 +136,7 @@ def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
         ValidationError if name is not unique or policy is not valid
     """
     rule_config_name = utils.get_id(name.lower(), 15)
+    rules = utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys()
     if rule_config_name in e_uci.get('mwan3').keys():
         raise ValidationError('name', 'unique', name)
     if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
@@ -146,16 +147,17 @@ def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
     e_uci.set('mwan3', rule_config_name, 'use_policy', policy)
     if protocol is not None:
         e_uci.set('mwan3', rule_config_name, 'proto', protocol)
-    if source_addresses is not None:
-        e_uci.set('mwan3', rule_config_name, 'src_ip', source_addresses)
-    if source_ports is not None:
-        e_uci.set('mwan3', rule_config_name, 'src_port', source_ports)
-    if destination_addresses is not None:
-        e_uci.set('mwan3', rule_config_name, 'dest_ip', destination_addresses)
-    if destination_ports is not None:
-        e_uci.set('mwan3', rule_config_name, 'dest_port', destination_ports)
+    if source_address is not None:
+        e_uci.set('mwan3', rule_config_name, 'src_ip', source_address)
+    if source_port is not None:
+        e_uci.set('mwan3', rule_config_name, 'src_port', source_port)
+    if destination_address is not None:
+        e_uci.set('mwan3', rule_config_name, 'dest_ip', destination_address)
+    if destination_port is not None:
+        e_uci.set('mwan3', rule_config_name, 'dest_port', destination_port)
 
     e_uci.save('mwan3')
+    order_rules(e_uci, [rule_config_name] + list(rules))
     return f'mwan3.{rule_config_name}'
 
 
@@ -198,13 +200,14 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
 
 def __fetch_interface_status(interface_name: str) -> str:
     try:
-        output = subprocess.check_output([
+        output = (subprocess.run([
             'ubus',
             'call',
             'mwan3',
             'status',
             '{"section": "interfaces"}'
-        ]).decode('utf-8')
+        ], capture_output=True)
+                  .stdout.decode('utf-8'))
         decoded_output = json.JSONDecoder().decode(output)
         return decoded_output['interfaces'][interface_name]['status']
     except:
@@ -343,9 +346,13 @@ def index_rules(e_uci: EUci) -> list[dict]:
         if 'proto' in rule_value:
             rule_data['protocol'] = rule_value['proto']
         if 'src_ip' in rule_value:
-            rule_data['source_addresses'] = rule_value['src_ip']
+            rule_data['source_address'] = rule_value['src_ip']
+        if 'src_port' in rule_value:
+            rule_data['source_port'] = rule_value['src_port']
         if 'dest_ip' in rule_value:
-            rule_data['destination_addresses'] = rule_value['dest_ip']
+            rule_data['destination_address'] = rule_value['dest_ip']
+        if 'dest_port' in rule_value:
+            rule_data['destination_port'] = rule_value['dest_port']
 
         data.append(rule_data)
     return data
@@ -364,7 +371,7 @@ def order_rules(e_uci: EUci, rules: list[str]) -> list[str]:
 
     order.extend(rules)
 
-    subprocess.check_output([
+    subprocess.run([
         'ubus',
         'call',
         'uci',
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 617cc9d7..4cd27b0e 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -138,7 +138,8 @@ def test_create_member(e_uci):
     assert mwan.__store_member(e_uci, 'RED_1', 1, 100) == ('ns_RED_1_M1_W100', True)
 
 
-def test_create_default_mwan(e_uci):
+def test_create_default_mwan(e_uci, mocker):
+    mocker.patch('subprocess.run')
     assert mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',
@@ -165,7 +166,8 @@ def test_create_default_mwan(e_uci):
         'ns_RED_1_M10_W200', 'ns_RED_2_M20_W100')
 
 
-def test_create_unique_mwan(e_uci):
+def test_create_unique_mwan(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'this', [])
     with pytest.raises(ValueError):
         mwan.store_policy(e_uci, 'this', [])
@@ -178,7 +180,8 @@ def test_metric_generation(e_uci):
     assert mwan.__generate_metric(e_uci, [4, 3, 1]) == 2
 
 
-def test_list_policies(e_uci):
+def test_list_policies(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'backup', [
         {
             'name': 'RED_1',
@@ -263,7 +266,8 @@ def test_list_policies(e_uci):
     assert index[2]['members'][20][0]['weight'] == '100'
 
 
-def test_store_rule(e_uci):
+def test_store_rule(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',
@@ -271,13 +275,20 @@ def test_store_rule(e_uci):
             'weight': '100',
         }
     ])
-    assert mwan.store_rule(e_uci, 'additional rule', 'ns_default') == 'mwan3.ns_additional_r'
+    assert mwan.store_rule(e_uci, 'additional rule', 'ns_default', 'udp', '192.168.1.1/24', '1:1024', '10.0.0.2/12',
+                           '22,443') == 'mwan3.ns_additional_r'
     assert e_uci.get('mwan3', 'ns_additional_r') == 'rule'
     assert e_uci.get('mwan3', 'ns_additional_r', 'label') == 'additional rule'
     assert e_uci.get('mwan3', 'ns_additional_r', 'use_policy') == 'ns_default'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'proto') == 'udp'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'src_ip') == '192.168.1.1/24'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'src_port') == '1:1024'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'dest_ip') == '10.0.0.2/12'
+    assert e_uci.get('mwan3', 'ns_additional_r', 'dest_port') == '22,443'
 
 
-def test_unique_rule(e_uci):
+def test_unique_rule(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',
@@ -285,14 +296,25 @@ def test_unique_rule(e_uci):
             'weight': '100',
         }
     ])
-    with pytest.raises(ValueError) as e:
+    with pytest.raises(ValidationError) as e:
         mwan.store_rule(e_uci, 'additional rule', 'ns_default')
         mwan.store_rule(e_uci, 'additional rule', 'ns_default')
+
     assert e.value.args[0] == 'name'
     assert e.value.args[1] == 'unique'
 
 
-def test_delete_non_existent_policy(e_uci):
+def test_missing_policy_rule(e_uci):
+    with pytest.raises(ValidationError) as e:
+        mwan.store_rule(e_uci, 'cool rule', 'ns_default')
+
+    assert e.value.args[0] == 'policy'
+    assert e.value.args[1] == 'invalid'
+    assert e.value.args[2] == 'ns_default'
+
+
+def test_delete_non_existent_policy(e_uci, mocker):
+    mocker.patch('subprocess.run')
     with pytest.raises(ValidationError) as e:
         mwan.delete_policy(e_uci, 'ns_default')
     assert e.value.args[0] == 'name'
@@ -300,7 +322,8 @@ def test_delete_non_existent_policy(e_uci):
     assert e.value.args[2] == 'ns_default'
 
 
-def test_delete_policy(e_uci):
+def test_delete_policy(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',
@@ -312,7 +335,8 @@ def test_delete_policy(e_uci):
     assert e_uci.get('mwan3', 'ns_default', default=None) is None
 
 
-def test_edit_policy(e_uci):
+def test_edit_policy(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',
@@ -342,7 +366,8 @@ def test_edit_policy(e_uci):
     assert mwan.index_policies(e_uci)[0]['type'] == 'backup'
 
 
-def test_missing_policy(e_uci):
+def test_missing_policy(e_uci, mocker):
+    mocker.patch('subprocess.run')
     with pytest.raises(ValidationError) as e:
         mwan.edit_policy(e_uci, 'dummy', '', [])
     assert e.value.args[0] == 'name'
@@ -350,7 +375,8 @@ def test_missing_policy(e_uci):
     assert e.value.args[2] == 'dummy'
 
 
-def test_index_rules(e_uci):
+def test_index_rules(e_uci, mocker):
+    mocker.patch('subprocess.run')
     mwan.store_policy(e_uci, 'default', [
         {
             'name': 'RED_1',

From 8fd4a45282a1f533b70a2e6e10a941df88f9bbf1 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Thu, 28 Sep 2023 14:55:32 +0200
Subject: [PATCH 12/14] feat: added rule deletion

---
 src/nethsec/mwan/__init__.py |  8 ++++++++
 tests/test_mwan.py           | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index b2c7f2f5..282bf3a3 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -384,3 +384,11 @@ def order_rules(e_uci: EUci, rules: list[str]) -> list[str]:
 
     e_uci.save('mwan3')
     return order
+
+
+def delete_rule(e_uci: EUci, name: str):
+    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys():
+        raise ValidationError('name', 'invalid', name)
+
+    e_uci.delete('mwan3', name)
+    e_uci.save('mwan3')
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index 4cd27b0e..d593c747 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -407,3 +407,22 @@ def test_index_rules(e_uci, mocker):
             'label': 'default',
         }
     }
+
+
+def test_delete_rule(e_uci, mocker):
+    mocker.patch('subprocess.run')
+    mwan.store_policy(e_uci, 'default', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    mwan.store_rule(e_uci, 'additional rule', 'ns_default')
+    mwan.delete_rule(e_uci, 'ns_additional_r')
+    assert 'ns_additional_r' not in e_uci.get_all('mwan3').keys()
\ No newline at end of file

From d8b3987c3ea25a7777c9908ccd057e09bf517417 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Thu, 28 Sep 2023 17:43:40 +0200
Subject: [PATCH 13/14] feat: added edit_rule

---
 src/nethsec/mwan/__init__.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index 282bf3a3..db93117b 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -392,3 +392,33 @@ def delete_rule(e_uci: EUci, name: str):
 
     e_uci.delete('mwan3', name)
     e_uci.save('mwan3')
+    return f'mwan3.{name}'
+
+
+def edit_rule(e_uci: EUci, name: str, policy: str, label: str, protocol: str = None,
+              source_address: str = None, source_port: str = None,
+              destination_address: str = None, destination_port: str = None):
+    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys():
+        raise ValidationError('name', 'invalid', name)
+
+    if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+        raise ValidationError('policy', 'invalid', policy)
+    e_uci.set('mwan3', name, 'use_policy', policy)
+    e_uci.set('mwan3', name, 'label', label)
+    if protocol is not None:
+        e_uci.set('mwan3', name, 'proto', protocol)
+        if protocol != 'tcp' and protocol != 'udp':
+            e_uci.delete('mwan3', name, 'src_port')
+            e_uci.delete('mwan3', name, 'dest_port')
+        else:
+            if destination_port is not None:
+                e_uci.set('mwan3', name, 'dest_port', destination_port)
+            if source_port is not None:
+                e_uci.set('mwan3', name, 'src_port', source_port)
+    if source_address is not None:
+        e_uci.set('mwan3', name, 'src_ip', source_address)
+    if destination_address is not None:
+        e_uci.set('mwan3', name, 'dest_ip', destination_address)
+
+    e_uci.save('mwan3')
+    return f'mwan3.{name}'

From 6befa0a8950a7d037c03dfd5691362edf3fc7b27 Mon Sep 17 00:00:00 2001
From: Tommaso Bailetti <tommaso.bailetti@nethesis.it>
Date: Fri, 29 Sep 2023 11:40:30 +0200
Subject: [PATCH 14/14] fix: review changes

---
 src/nethsec/mwan/__init__.py | 54 ++++++++++++--------------
 tests/test_mwan.py           | 75 +++++++++++++++++++++++++++++++++---
 2 files changed, 94 insertions(+), 35 deletions(-)

diff --git a/src/nethsec/mwan/__init__.py b/src/nethsec/mwan/__init__.py
index db93117b..ccc0f987 100644
--- a/src/nethsec/mwan/__init__.py
+++ b/src/nethsec/mwan/__init__.py
@@ -8,33 +8,27 @@
 import json
 import subprocess
 
+import uci
 from euci import EUci
 
 from nethsec import utils
 from nethsec.utils import ValidationError
 
 
-def __generate_metric(e_uci: EUci, interface_metrics: list[int] = None, metric: int = 1) -> int:
+def __generate_metric(e_uci: EUci) -> int:
     """
     Generates a metric for an interface.
     Args:
         e_uci: EUci instance
-        interface_metrics: list of metrics already used, will be generated if not provided
-        metric: metric to start from
 
     Returns:
         first metric that is not present in interface_metrics
     """
-    if interface_metrics is None:
-        interface_metrics = list[int]()
-        for interface in utils.get_all_by_type(e_uci, 'network', 'interface').values():
-            if 'metric' in interface:
-                interface_metrics.append(int(interface['metric']))
-
-    if metric not in interface_metrics:
-        return metric
-    else:
-        return __generate_metric(e_uci, interface_metrics, metric + 1)
+    next_metric = 0
+    for interface in utils.get_all_by_type(e_uci, 'network', 'interface').values():
+        if 'metric' in interface:
+            next_metric = max(next_metric, int(interface['metric']))
+    return next_metric + 1
 
 
 def __store_interface(e_uci: EUci, name: str) -> tuple[bool, bool]:
@@ -52,16 +46,17 @@ def __store_interface(e_uci: EUci, name: str) -> tuple[bool, bool]:
         ValidationError: if interface name is not defined in /etc/config/network
     """
     # checking if interface is configured
-    available_interfaces = utils.get_all_by_type(e_uci, 'network', 'interface')
-    if name not in available_interfaces.keys():
+    try:
+        e_uci.get('network', name)
+    except uci.UciExceptionNotFound:
         raise ValidationError('name', 'invalid', name)
 
     created_interface = False
     # if no interface with name exists, create one with defaults
-    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'interface').keys():
+    if e_uci.get('mwan3', name, default=None) is None:
         created_interface = True
         # fetch default configuration and set interface
-        default_interface_config = utils.get_all_by_type(e_uci, 'ns-api', 'defaults_mwan').get('defaults_mwan')
+        default_interface_config = e_uci.get_all('ns-api', 'defaults_mwan')
         e_uci.set('mwan3', name, 'interface')
         e_uci.set('mwan3', name, 'enabled', '1')
         e_uci.set('mwan3', name, 'initial_state', default_interface_config['initial_state'])
@@ -81,7 +76,7 @@ def __store_interface(e_uci: EUci, name: str) -> tuple[bool, bool]:
 
     added_metric = False
     # avoid adding metric if already present
-    if 'metric' not in available_interfaces[name]:
+    if e_uci.get('network', name, 'metric', default=None) is None:
         added_metric = True
         # generate metric
         metric = __generate_metric(e_uci)
@@ -105,7 +100,7 @@ def __store_member(e_uci: EUci, interface_name: str, metric: int, weight: int) -
     """
     member_config_name = utils.get_id(f'{interface_name}_M{metric}_W{weight}')
     changed = False
-    if member_config_name not in utils.get_all_by_type(e_uci, 'mwan3', 'member').keys():
+    if e_uci.get('mwan3', member_config_name, default=None) is None:
         changed = True
         e_uci.set('mwan3', member_config_name, 'member')
         e_uci.set('mwan3', member_config_name, 'interface', interface_name)
@@ -137,13 +132,12 @@ def store_rule(e_uci: EUci, name: str, policy: str, protocol: str = None,
     """
     rule_config_name = utils.get_id(name.lower(), 15)
     rules = utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys()
-    if rule_config_name in e_uci.get('mwan3').keys():
+    if e_uci.get('mwan3', rule_config_name, default=None) is not None:
         raise ValidationError('name', 'unique', name)
-    if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+    if e_uci.get('mwan3', policy, default=None) is None:
         raise ValidationError('policy', 'invalid', policy)
     e_uci.set('mwan3', rule_config_name, 'rule')
     e_uci.set('mwan3', rule_config_name, 'label', name)
-    e_uci.set('mwan3', rule_config_name, 'label', name)
     e_uci.set('mwan3', rule_config_name, 'use_policy', policy)
     if protocol is not None:
         e_uci.set('mwan3', rule_config_name, 'proto', protocol)
@@ -179,7 +173,7 @@ def store_policy(e_uci: EUci, name: str, interfaces: list[dict]) -> list[str]:
     # generate policy name
     policy_config_name = utils.get_id(name.lower())
     # make sure name is not something that already exists
-    if policy_config_name in e_uci.get('mwan3').keys():
+    if e_uci.get('mwan3', policy_config_name, default=None) is not None:
         raise ValidationError('name', 'unique', name)
     # generate policy config with corresponding name
     e_uci.set('mwan3', policy_config_name, 'policy')
@@ -206,7 +200,7 @@ def __fetch_interface_status(interface_name: str) -> str:
             'mwan3',
             'status',
             '{"section": "interfaces"}'
-        ], capture_output=True)
+        ], capture_output=True, check=True)
                   .stdout.decode('utf-8'))
         decoded_output = json.JSONDecoder().decode(output)
         return decoded_output['interfaces'][interface_name]['status']
@@ -306,7 +300,7 @@ def __add_interfaces(e_uci: EUci, interfaces: list[dict], changed_config: list[s
 
 
 def edit_policy(e_uci: EUci, name: str, label: str, interfaces: list[dict]) -> list[str]:
-    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+    if e_uci.get('mwan3', name, default=None) is None:
         raise ValidationError('name', 'invalid', name)
     changed_config = []
     if label != e_uci.get_all('mwan3', name)['label']:
@@ -323,7 +317,7 @@ def edit_policy(e_uci: EUci, name: str, label: str, interfaces: list[dict]) -> l
 
 
 def delete_policy(e_uci: EUci, name: str) -> list[str]:
-    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+    if e_uci.get('mwan3', name, default=None) is None:
         raise ValidationError('name', 'invalid', name)
     e_uci.delete('mwan3', name)
     e_uci.save('mwan3')
@@ -339,7 +333,7 @@ def index_rules(e_uci: EUci) -> list[dict]:
         rule_data['name'] = rule_key
         rule_data['policy'] = {}
         rule_data['policy']['name'] = rule_value['use_policy']
-        if rule_value['use_policy'] in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+        if e_uci.get('mwan3', rule_value['use_policy'], default=None) is not None:
             rule_data['policy']['label'] = utils.get_all_by_type(e_uci, 'mwan3', 'policy')[rule_value['use_policy']]['label']
         if 'label' in rule_value:
             rule_data['label'] = rule_value['label']
@@ -387,7 +381,7 @@ def order_rules(e_uci: EUci, rules: list[str]) -> list[str]:
 
 
 def delete_rule(e_uci: EUci, name: str):
-    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys():
+    if e_uci.get('mwan3', name, default=None) is None:
         raise ValidationError('name', 'invalid', name)
 
     e_uci.delete('mwan3', name)
@@ -398,10 +392,10 @@ def delete_rule(e_uci: EUci, name: str):
 def edit_rule(e_uci: EUci, name: str, policy: str, label: str, protocol: str = None,
               source_address: str = None, source_port: str = None,
               destination_address: str = None, destination_port: str = None):
-    if name not in utils.get_all_by_type(e_uci, 'mwan3', 'rule').keys():
+    if e_uci.get('mwan3', name, default=None) is None:
         raise ValidationError('name', 'invalid', name)
 
-    if policy not in utils.get_all_by_type(e_uci, 'mwan3', 'policy').keys():
+    if e_uci.get('mwan3', policy, default=None) is None:
         raise ValidationError('policy', 'invalid', policy)
     e_uci.set('mwan3', name, 'use_policy', policy)
     e_uci.set('mwan3', name, 'label', label)
diff --git a/tests/test_mwan.py b/tests/test_mwan.py
index d593c747..283d36fc 100644
--- a/tests/test_mwan.py
+++ b/tests/test_mwan.py
@@ -175,9 +175,11 @@ def test_create_unique_mwan(e_uci, mocker):
 
 def test_metric_generation(e_uci):
     assert mwan.__generate_metric(e_uci) == 1
-    assert mwan.__generate_metric(e_uci, [1, 4]) == 2
-    assert mwan.__generate_metric(e_uci, [1, 2, 4]) == 3
-    assert mwan.__generate_metric(e_uci, [4, 3, 1]) == 2
+    assert mwan.__store_interface(e_uci, 'RED_1') == (True, True)
+    assert mwan.__generate_metric(e_uci) == 2
+    assert mwan.__generate_metric(e_uci) == 2
+    assert mwan.__store_interface(e_uci, 'RED_2') == (True, True)
+    assert mwan.__generate_metric(e_uci) == 3
 
 
 def test_list_policies(e_uci, mocker):
@@ -424,5 +426,68 @@ def test_delete_rule(e_uci, mocker):
         }
     ])
     mwan.store_rule(e_uci, 'additional rule', 'ns_default')
-    mwan.delete_rule(e_uci, 'ns_additional_r')
-    assert 'ns_additional_r' not in e_uci.get_all('mwan3').keys()
\ No newline at end of file
+    assert mwan.delete_rule(e_uci, 'ns_additional_r') == 'mwan3.ns_additional_r'
+    assert 'ns_additional_r' not in e_uci.get_all('mwan3').keys()
+
+
+def test_edit_rule(e_uci, mocker):
+    mocker.patch('subprocess.run')
+    mwan.store_policy(e_uci, 'hello world', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    mwan.store_policy(e_uci, 'cool policy', [
+        {
+            'name': 'RED_3',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    assert mwan.edit_rule(e_uci, 'ns_default_rule', 'ns_cool_policy', 'new label!', 'udp', '192.168.10.1/12', '80,443',
+                          '0.0.0.0/0', '4040-8080') == 'mwan3.ns_default_rule'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'label') == 'new label!'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'use_policy') == 'ns_cool_policy'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'proto') == 'udp'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'src_ip') == '192.168.10.1/12'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'src_port') == '80,443'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'dest_ip') == '0.0.0.0/0'
+    assert e_uci.get('mwan3', 'ns_default_rule', 'dest_port') == '4040-8080'
+
+
+def test_cant_edit_invalid_rule(e_uci, mocker):
+    mocker.patch('subprocess.run')
+    with pytest.raises(ValidationError) as e:
+        mwan.edit_rule(e_uci, 'ns_default_rule', 'ns_cool_policy', 'new label!')
+    assert e.value.args[0] == 'name'
+    assert e.value.args[1] == 'invalid'
+    assert e.value.args[2] == 'ns_default_rule'
+    mwan.store_policy(e_uci, 'hello world', [
+        {
+            'name': 'RED_1',
+            'metric': '10',
+            'weight': '100',
+        },
+        {
+            'name': 'RED_2',
+            'metric': '10',
+            'weight': '100',
+        }
+    ])
+    with pytest.raises(ValidationError) as e:
+        mwan.edit_rule(e_uci, 'ns_default_rule', 'ns_cool_policy', 'new label!')
+    assert e.value.args[0] == 'policy'
+    assert e.value.args[1] == 'invalid'
+    assert e.value.args[2] == 'ns_cool_policy'