Merge branch 'test_ui_tunnel_settings-fails-a-lot-des-1351'

7 files changed, 72 insertions, 47 deletions

diff --git a/desktop/packages/mullvad-vpn/test/e2e/installed/state-dependent/tunnel-state.spec.ts b/desktop/packages/mullvad-vpn/test/e2e/installed/state-dependent/tunnel-state.spec.ts
index 15ce240e57..3f767a8ffa 100644
--- a/desktop/packages/mullvad-vpn/test/e2e/installed/state-dependent/tunnel-state.spec.ts
+++ b/desktop/packages/mullvad-vpn/test/e2e/installed/state-dependent/tunnel-state.spec.ts
@@ -35,7 +35,9 @@ test('App should connect', async () => {
 
   const relay = page.getByTestId('hostname-line');
   const inIp = page.locator(':text("In") + span');
-  const outIp = page.locator(':text("Out") + div > span');
+  // If IPv6 is enabled, there will be two "Out" IPs, one for IPv4 and one for IPv6
+  // Selecting the first resolves to the IPv4 address regardless of the IP setting
+  const outIp = page.locator(':text("Out") + div > span').first();
 
   await expect(relay).toHaveText(process.env.HOSTNAME!);
   await expect(inIp).not.toBeVisible();
diff --git a/mullvad-daemon/src/custom_list.rs b/mullvad-daemon/src/custom_list.rs
index b989a7fe56..ba662a2dac 100644
--- a/mullvad-daemon/src/custom_list.rs
+++ b/mullvad-daemon/src/custom_list.rs
@@ -1,4 +1,5 @@
-use crate::{new_selector_config, Daemon, Error};
+use crate::{Daemon, Error};
+use mullvad_relay_selector::SelectorConfig;
 use mullvad_types::{
     constraints::Constraint,
     custom_list::{CustomList, Id},
@@ -38,7 +39,7 @@ impl Daemon {
 
         if let Ok(true) = settings_changed {
             self.relay_selector
-                .set_config(new_selector_config(&self.settings));
+                .set_config(SelectorConfig::from_settings(&self.settings));
 
             if self.change_should_cause_reconnect(Some(id)) {
                 log::info!("Initiating tunnel restart because a selected custom list was deleted");
@@ -65,7 +66,7 @@ impl Daemon {
 
         if let Ok(true) = settings_changed {
             self.relay_selector
-                .set_config(new_selector_config(&self.settings));
+                .set_config(SelectorConfig::from_settings(&self.settings));
 
             if self.change_should_cause_reconnect(Some(list_id)) {
                 log::info!("Initiating tunnel restart because a selected custom list changed");
@@ -89,7 +90,7 @@ impl Daemon {
 
         if let Ok(true) = settings_changed {
             self.relay_selector
-                .set_config(new_selector_config(&self.settings));
+                .set_config(SelectorConfig::from_settings(&self.settings));
 
             if self.change_should_cause_reconnect(None) {
                 log::info!("Initiating tunnel restart because a selected custom list was deleted");
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index 7fb8193ba6..8299e1d20c 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -38,9 +38,7 @@ use futures::{
 };
 use geoip::GeoIpHandler;
 use management_interface::ManagementInterfaceServer;
-use mullvad_relay_selector::{
-    AdditionalRelayConstraints, AdditionalWireguardConstraints, RelaySelector, SelectorConfig,
-};
+use mullvad_relay_selector::{RelaySelector, SelectorConfig};
 #[cfg(target_os = "android")]
 use mullvad_types::account::{PlayPurchase, PlayPurchasePaymentToken};
 #[cfg(any(windows, target_os = "android", target_os = "macos"))]
@@ -636,7 +634,7 @@ impl Daemon {
             settings_event_listener.notify_settings(settings.to_owned());
         });
 
-        let initial_selector_config = new_selector_config(&settings);
+        let initial_selector_config = SelectorConfig::from_settings(&settings);
         let relay_selector = RelaySelector::new(
             initial_selector_config,
             resource_dir.join(RELAYS_FILENAME),
@@ -648,7 +646,7 @@ impl Daemon {
             // Notify relay selector of changes to the settings/selector config
             settings_relay_selector
                 .clone()
-                .set_config(new_selector_config(settings));
+                .set_config(SelectorConfig::from_settings(settings));
         });
 
         let (access_mode_handler, access_mode_provider) = api::AccessModeSelector::spawn(
@@ -3045,38 +3043,6 @@ impl DaemonShutdownHandle {
     }
 }
 
-fn new_selector_config(settings: &Settings) -> SelectorConfig {
-    let additional_constraints = AdditionalRelayConstraints {
-        wireguard: AdditionalWireguardConstraints {
-            #[cfg(daita)]
-            daita: settings.tunnel_options.wireguard.daita.enabled,
-            #[cfg(daita)]
-            daita_use_multihop_if_necessary: settings
-                .tunnel_options
-                .wireguard
-                .daita
-                .use_multihop_if_necessary,
-
-            #[cfg(not(daita))]
-            daita: false,
-            #[cfg(not(daita))]
-            daita_use_multihop_if_necessary: false,
-
-            quantum_resistant: settings.tunnel_options.wireguard.quantum_resistant,
-        },
-    };
-
-    SelectorConfig {
-        relay_settings: settings.relay_settings.clone(),
-        additional_constraints,
-        bridge_state: settings.bridge_state,
-        bridge_settings: settings.bridge_settings.clone(),
-        obfuscation_settings: settings.obfuscation_settings.clone(),
-        custom_lists: settings.custom_lists.clone(),
-        relay_overrides: settings.relay_overrides.clone(),
-    }
-}
-
 /// Consume a oneshot sender of `T1` and return a sender that takes a different type `T2`.
 /// `forwarder` should map `T1` back to `T2` and send the result back to the original receiver.
 fn oneshot_map<T1: Send + 'static, T2: Send + 'static>(
diff --git a/mullvad-relay-selector/src/relay_selector/matcher.rs b/mullvad-relay-selector/src/relay_selector/matcher.rs
index 21376e9485..f9fa0d7a01 100644
--- a/mullvad-relay-selector/src/relay_selector/matcher.rs
+++ b/mullvad-relay-selector/src/relay_selector/matcher.rs
@@ -246,7 +246,7 @@ impl<'a> ResolvedLocationConstraint<'a> {
                         ResolvedLocationConstraint(custom_list.locations.iter().collect())
                     })
                     .unwrap_or_else(|| {
-                        log::warn!("Resolved non-existent custom list");
+                        log::warn!("Resolved non-existent custom list with id {list_id:?}");
                         ResolvedLocationConstraint(vec![])
                     }),
             }),
diff --git a/mullvad-relay-selector/src/relay_selector/mod.rs b/mullvad-relay-selector/src/relay_selector/mod.rs
index 6a3e20605f..550d1955a0 100644
--- a/mullvad-relay-selector/src/relay_selector/mod.rs
+++ b/mullvad-relay-selector/src/relay_selector/mod.rs
@@ -118,6 +118,40 @@ pub struct SelectorConfig {
     pub bridge_settings: BridgeSettings,
 }
 
+impl SelectorConfig {
+    pub fn from_settings(settings: &Settings) -> Self {
+        let additional_constraints = AdditionalRelayConstraints {
+            wireguard: AdditionalWireguardConstraints {
+                #[cfg(daita)]
+                daita: settings.tunnel_options.wireguard.daita.enabled,
+                #[cfg(daita)]
+                daita_use_multihop_if_necessary: settings
+                    .tunnel_options
+                    .wireguard
+                    .daita
+                    .use_multihop_if_necessary,
+
+                #[cfg(not(daita))]
+                daita: false,
+                #[cfg(not(daita))]
+                daita_use_multihop_if_necessary: false,
+
+                quantum_resistant: settings.tunnel_options.wireguard.quantum_resistant,
+            },
+        };
+
+        Self {
+            relay_settings: settings.relay_settings.clone(),
+            additional_constraints,
+            bridge_state: settings.bridge_state,
+            bridge_settings: settings.bridge_settings.clone(),
+            obfuscation_settings: settings.obfuscation_settings.clone(),
+            custom_lists: settings.custom_lists.clone(),
+            relay_overrides: settings.relay_overrides.clone(),
+        }
+    }
+}
+
 /// Extra relay constraints not specified in `relay_settings`.
 #[derive(Default, Debug, Clone, Eq, PartialEq)]
 pub struct AdditionalRelayConstraints {
diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs
index 10842985ef..0c3f16ccb3 100644
--- a/test/test-manager/src/tests/helpers.rs
+++ b/test/test-manager/src/tests/helpers.rs
@@ -723,9 +723,10 @@ pub async fn constrain_to_relay(
         }
     }
 
+    let settings = mullvad_client.get_settings().await?;
     // Construct a relay selector with up-to-date information from the runnin daemon's relay list
     let relay_list = mullvad_client.get_relay_locations().await?;
-    let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list);
+    let relay_selector = get_daemon_relay_selector(&settings, relay_list);
     // Select an(y) appropriate relay for the given query and constrain the daemon to only connect
     // to that specific relay (when connecting).
     let relay = relay_selector.get_relay_by_query(query.clone())?;
@@ -735,6 +736,17 @@ pub async fn constrain_to_relay(
     Ok(exit)
 }
 
+/// Get a mirror of the relay selector used by the daemon.
+///
+/// This can be used to query the relay selector without triggering a tunnel state change in the
+/// daemon.
+fn get_daemon_relay_selector(
+    settings: &mullvad_types::settings::Settings,
+    relay_list: mullvad_types::relay_list::RelayList,
+) -> RelaySelector {
+    RelaySelector::from_list(SelectorConfig::from_settings(settings), relay_list)
+}
+
 /// Convenience function for constructing a constraint from a given [`Relay`].
 ///
 /// # Panics
@@ -1190,8 +1202,9 @@ pub mod custom_lists {
     // Expose all custom list variants as a shorthand.
     pub use List::*;
 
-    /// Mapping between [List] to daemon custom lists. Since custom list ids are assigned by the daemon at the creation
-    /// of the custom list settings object, we can't map a custom list name to a specific list before runtime.
+    /// Mapping between [List] to daemon custom lists. Since custom list ids are assigned by the
+    /// daemon at the creation of the custom list settings object, we can't map a custom list
+    /// name to a specific list before runtime.
     static IDS: LazyLock<Mutex<HashMap<List, Id>>> = LazyLock::new(|| Mutex::new(HashMap::new()));
 
     /// Pre-defined (well-typed) custom lists which may be useuful in different test scenarios.
@@ -1281,6 +1294,7 @@ pub mod custom_lists {
                 .create_custom_list(custom_list.name())
                 .await?;
             let mut daemon_dito = find_custom_list(mullvad_client, &custom_list.name()).await?;
+            assert_eq!(id, daemon_dito.id);
             for locations in custom_list.locations() {
                 daemon_dito.locations.insert(locations);
             }
diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs
index 53c769a7cc..994557748b 100644
--- a/test/test-manager/src/tests/ui.rs
+++ b/test/test-manager/src/tests/ui.rs
@@ -91,11 +91,19 @@ pub async fn test_ui_tunnel_settings(
     rpc: ServiceClient,
     mut mullvad_client: MullvadProxyClient,
 ) -> anyhow::Result<()> {
+    // NOTE: This test connects multiple times using various settings, some of which may cauase a
+    // significant increase in connection time, e.g. multihop and OpenVPN. For this reason, it is
+    // preferable to only target low latency servers.
+    use helpers::custom_lists::LowLatency;
+
     // tunnel-state.spec precondition: a single WireGuard relay should be selected
     log::info!("Select WireGuard relay");
     let entry = helpers::constrain_to_relay(
         &mut mullvad_client,
-        RelayQueryBuilder::new().wireguard().build(),
+        RelayQueryBuilder::new()
+            .wireguard()
+            .location(LowLatency)
+            .build(),
     )
     .await?;