nm route rule: Only search desired interface for storing route rule

When finding interface to store the route rule, we should only desired
interfaces (use loopback as fallback), so we do not touch unmentioned
interfaces which might lead to unexpected behaviour.

Integration test case included.
Unit test case updated for this change.

Resolves: https://issues.redhat.com/browse/RHEL-59965

Signed-off-by: Gris Ge <fge@redhat.com>

rust/src/lib/nm/route_rule.rs

@@ -229,8 +229,9 @@ fn append_route_rule(
     Ok(())
 }
 
-// * If rule has `iif`, we use that
-// * If rule has table id, we find a interface configured for that route table
+// * If rule has `iif`, we use that interface.
+// * If rule has table id, we find a interface in desire state configured for
+//   that route table
 // * fallback to first desired interface with ip stack enabled.
 // * fallback to use loop interface.
 fn find_interface_for_rule<'a>(
@@ -293,34 +294,6 @@ fn find_interface_for_rule<'a>(
         }
     }
 
-    let mut cur_iface_names: Vec<&str> = merged_state
-        .interfaces
-        .kernel_ifaces
-        .iter()
-        .filter_map(|(n, i)| {
-            if !i.is_changed() {
-                Some(n.as_str())
-            } else {
-                None
-            }
-        })
-        .collect();
-
-    // we should be persistent on choice, hence sort the iface names.
-    cur_iface_names.sort_unstable();
-
-    // Try interfaces in current state
-    for iface_name in cur_iface_names {
-        if iface_has_route_for_table_id(
-            iface_name,
-            merged_state,
-            rule.is_ipv6(),
-            table_id,
-        ) {
-            return Ok(iface_name);
-        }
-    }
-
     // Fallback to first interface in desire state with IP stack enabled.
     for (iface_name, iface_type) in
         merged_state.interfaces.insert_order.as_slice().iter()

rust/src/lib/unit_tests/nm/route_rule.rs

@@ -200,6 +200,9 @@ route-rules:
     - route-table: 500
       priority: 3200
       ip-from: 192.0.3.0/24
+interfaces:
+  - name: br0
+    type: ovs-interface
 ",
     )
     .unwrap();
@@ -258,15 +261,15 @@ interfaces:
     .unwrap();
 
     let desired: NetworkState = serde_yaml::from_str(
-        r"
----
-route-rules:
-  config:
-    - priority: 3200
-      ip-to: 192.0.3.0/24
-    - priority: 3200
-      ip-from: 192.0.3.0/24
-",
+        r"---
+        route-rules:
+          config:
+            - priority: 3200
+              ip-to: 192.0.3.0/24
+            - priority: 3200
+              ip-from: 192.0.3.0/24
+        interfaces:
+          - name: eth1",
     )
     .unwrap();
 
@@ -304,24 +307,22 @@ route-rules:
 #[test]
 fn test_route_rule_use_loopback() {
     let current: NetworkState = serde_yaml::from_str(
-        r"
----
-interfaces:
-  - name: lo
-    type: loopback
-    state: up
-    mtu: 65536
-    ipv4:
-      enabled: true
-      address:
-      - ip: 127.0.0.1
-        prefix-length: 8
-    ipv6:
-      enabled: true
-      address:
-      - ip: ::1
-        prefix-length: 128
-",
+        r"---
+        interfaces:
+          - name: lo
+            type: loopback
+            state: up
+            mtu: 65536
+            ipv4:
+              enabled: true
+              address:
+              - ip: 127.0.0.1
+                prefix-length: 8
+            ipv6:
+              enabled: true
+              address:
+              - ip: ::1
+                prefix-length: 128",
     )
     .unwrap();
 

tests/integration/nm/route_test.py

@@ -107,3 +107,102 @@ def test_route_delayed_by_nm_fails(eth1_up):
                 Loader=yaml.SafeLoader,
             )
         )
+
+
+@pytest.fixture
+def eth1_with_static_routes_table_id_200(eth1_up):
+    libnmstate.apply(
+        yaml.load(
+            """---
+            routes:
+              config:
+              - destination: 192.168.2.0/24
+                metric: 108
+                next-hop-address: 192.168.1.3
+                next-hop-interface: eth1
+                table-id: 200
+              - destination: 2001:db8:a::/64
+                metric: 108
+                next-hop-address: 2001:db8:1::2
+                next-hop-interface: eth1
+                table-id: 200
+            interfaces:
+              - name: eth1
+                type: ethernet
+                state: up
+                mtu: 1500
+                ipv4:
+                  enabled: true
+                  dhcp: false
+                  address:
+                  - ip: 192.168.1.1
+                    prefix-length: 24
+                ipv6:
+                  enabled: true
+                  dhcp: false
+                  autoconf: false
+                  address:
+                  - ip: 2001:db8:1::1
+                    prefix-length: 64
+            """,
+            Loader=yaml.SafeLoader,
+        )
+    )
+    yield
+    libnmstate.apply(
+        yaml.load(
+            """---
+            route-rules:
+              config:
+                - state: absent
+            routes:
+              config:
+              - state: absent
+                next-hop-interface: eth1
+            interfaces:
+            - name: eth1
+              state: absent
+            """,
+            Loader=yaml.SafeLoader,
+        )
+    )
+
+
+# https://issues.redhat.com/browse/RHEL-59965
+@pytest.mark.tier1
+def test_route_rule_use_loopback_for_no_desired_iface(
+    eth1_with_static_routes_table_id_200,
+):
+    # Even the eth1 has routes on route table 200, since it is not mentioned
+    # in desired state, we should not use eth1 but fallback to loopback
+    # interface.
+    libnmstate.apply(
+        yaml.load(
+            """---
+            route-rules:
+              config:
+                - ip-from: 192.168.3.2/32
+                  route-table: 200
+                  priority: 1000
+                - ip-from: 2001:db8:b::/64
+                  route-table: 200
+                  priority: 1001
+            """,
+            Loader=yaml.SafeLoader,
+        )
+    )
+
+    assert (
+        exec_cmd(
+            "nmcli -g ipv4.routing-rules c show lo".split(),
+            check=True,
+        )[1].strip()
+        == "priority 1000 from 192.168.3.2 table 200"
+    )
+    assert (
+        exec_cmd(
+            "nmcli -g ipv6.routing-rules c show lo".split(),
+            check=True,
+        )[1].strip()
+        == r"priority 1001 from 2001\:db8\:b\:\:/64 table 200"
+    )