From ac7fc9821133e115453f035dff2d5579f5e185f4 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Wed, 6 Nov 2024 08:57:40 +0100 Subject: [PATCH 1/8] Generalize `entry` and `location` functions of relay query builder API --- mullvad-relay-selector/src/relay_selector/query.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mullvad-relay-selector/src/relay_selector/query.rs b/mullvad-relay-selector/src/relay_selector/query.rs index 162865368acc..1f0e393ac13d 100644 --- a/mullvad-relay-selector/src/relay_selector/query.rs +++ b/mullvad-relay-selector/src/relay_selector/query.rs @@ -539,8 +539,8 @@ pub mod builder { // [`WireguardRelayQuery`] & [`OpenVpnRelayQuery`] impl RelayQueryBuilder { /// Configure the [`LocationConstraint`] to use. - pub fn location(mut self, location: GeographicLocationConstraint) -> Self { - self.query.location = Constraint::Only(LocationConstraint::from(location)); + pub fn location(mut self, location: impl Into) -> Self { + self.query.location = Constraint::Only(location.into()); self } @@ -732,9 +732,8 @@ pub mod builder { { /// Set the entry location in a multihop configuration. This requires /// multihop to be enabled. - pub fn entry(mut self, location: GeographicLocationConstraint) -> Self { - self.query.wireguard_constraints.entry_location = - Constraint::Only(LocationConstraint::from(location)); + pub fn entry(mut self, location: impl Into) -> Self { + self.query.wireguard_constraints.entry_location = Constraint::Only(location.into()); self } } From bd9cebbf6fe704e5c074cbe8f14b8e9c70395fb9 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 8 Nov 2024 09:14:14 +0100 Subject: [PATCH 2/8] Add helper module for working with custom lists in end-to-end tests --- test/test-manager/src/tests/helpers.rs | 124 +++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/test/test-manager/src/tests/helpers.rs b/test/test-manager/src/tests/helpers.rs index 2ebc5eb7ada0..10842985efdd 100644 --- a/test/test-manager/src/tests/helpers.rs +++ b/test/test-manager/src/tests/helpers.rs @@ -1181,3 +1181,127 @@ fn parse_am_i_mullvad(result: String) -> anyhow::Result { bail!("Unexpected output from connection-checker: {result:?}") }) } + +pub mod custom_lists { + use super::*; + use mullvad_types::custom_list::{CustomList, Id}; + use std::sync::{LazyLock, Mutex}; + + // 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. + static IDS: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); + + /// Pre-defined (well-typed) custom lists which may be useuful in different test scenarios. + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] + pub enum List { + /// A selection of Nordic servers + Nordic, + /// A selection of European servers + Europe, + /// This custom list contains relays which are close geographically to the computer running + /// the test scenarios, which hopefully means there will be little latency between the test + /// machine and these relays + LowLatency, + /// Antithesis of [List::LowLatency], these relays are located far away from the test + /// server. Use this custom list if you want to simulate scenarios where the probability + /// of experiencing high latencies is desirable. + HighLatency, + } + + impl List { + pub fn name(self) -> String { + use List::*; + match self { + Nordic => "Nordic".to_string(), + Europe => "Europe".to_string(), + LowLatency => "Low Latency".to_string(), + HighLatency => "High Latency".to_string(), + } + } + + /// Iterator over all custom lists. + pub fn all() -> impl Iterator { + use List::*; + [Nordic, Europe, LowLatency, HighLatency].into_iter() + } + + pub fn locations(self) -> impl Iterator { + use List::*; + let country = GeographicLocationConstraint::country; + let city = GeographicLocationConstraint::city; + match self { + Nordic => { + vec![country("no"), country("se"), country("fi"), country("dk")].into_iter() + } + Europe => vec![ + // North + country("se"), + // West + country("fr"), + // East + country("ro"), + // South + country("it"), + ] + .into_iter(), + LowLatency => { + // Assumption: Test server is located in Gothenburg, Sweden. + vec![city("se", "got")].into_iter() + } + HighLatency => { + // Assumption: Test server is located in Gothenburg, Sweden. + vec![country("au"), country("ca"), country("za")].into_iter() + } + } + } + + pub fn to_constraint(self) -> Option { + let ids = IDS.lock().unwrap(); + let id = ids.get(&self)?; + Some(LocationConstraint::CustomList { list_id: *id }) + } + } + + impl From for LocationConstraint { + fn from(custom_list: List) -> Self { + // TODO: Is this _too_ unsound ?? + custom_list.to_constraint().unwrap() + } + } + + /// Add a set of custom lists which can be used in different test scenarios. + /// + /// See [`List`] for available custom lists. + pub async fn add_default_lists(mullvad_client: &mut MullvadProxyClient) -> anyhow::Result<()> { + for custom_list in List::all() { + let id = mullvad_client + .create_custom_list(custom_list.name()) + .await?; + let mut daemon_dito = find_custom_list(mullvad_client, &custom_list.name()).await?; + for locations in custom_list.locations() { + daemon_dito.locations.insert(locations); + } + mullvad_client.update_custom_list(daemon_dito).await?; + // Associate this custom list variant with a specific, runtime custom list id. + IDS.lock().unwrap().insert(custom_list, id); + } + Ok(()) + } + + /// Dig out a custom list from the daemon settings based on the custom list's name. + /// There should be an rpc for this. + async fn find_custom_list( + rpc: &mut MullvadProxyClient, + name: &str, + ) -> anyhow::Result { + rpc.get_settings() + .await? + .custom_lists + .into_iter() + .find(|list| list.name == name) + .ok_or(anyhow!("List '{name}' not found")) + } +} From dcbcc5aa9ecbbade58515a3639f1887f856a15fe Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 8 Nov 2024 11:52:08 +0100 Subject: [PATCH 3/8] Add default custom lists before running a test --- test/test-manager/src/tests/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index bc17a7f3f64a..14c2a1e3a5de 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -121,6 +121,7 @@ pub async fn prepare_daemon( .await .context("Failed to disconnect daemon after test")?; helpers::ensure_logged_in(&mut mullvad_client).await?; + helpers::custom_lists::add_default_lists(&mut mullvad_client).await?; Ok(()) } From b434952a292e3193be18eb782f8beabe623cdbfc Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 4 Nov 2024 07:53:01 +0100 Subject: [PATCH 4/8] Mitigate `test_quantum_resistant_multihop_shadowsocks_tunnel` flakiness Limit relay selection in `test_quantum_resistant_multihop_shadowsocks_tunnel` to reduce flakiness. Hopefully this should be able to (at least partially) mitigate timeout-related issues. --- test/test-manager/src/tests/tunnel.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index 6fb57adc1498..1d4a54f88bf5 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -559,6 +559,10 @@ pub async fn test_quantum_resistant_multihop_shadowsocks_tunnel( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { + // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. + // This is an attempt to try to reduce this type of flakiness. + use helpers::custom_lists::LowLatency; + mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::On) .await @@ -568,6 +572,8 @@ pub async fn test_quantum_resistant_multihop_shadowsocks_tunnel( .wireguard() .multihop() .shadowsocks() + .entry(LowLatency) + .location(LowLatency) .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; From 26e4255c5cca90cdc9e868c982ee3c3af828dd69 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 4 Nov 2024 08:12:29 +0100 Subject: [PATCH 5/8] Mitigate `test_quantum_resistant_multihop_udp2tcp_tunnel` flakiness Limit relay selection in `test_quantum_resistant_multihop_udp2tcp_tunnel` to reduce flakiness. Hopefully this should be able to (at least partially) mitigate timeout-related issues. --- test/test-manager/src/tests/tunnel.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index 1d4a54f88bf5..cdb890164bfa 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -524,6 +524,10 @@ pub async fn test_quantum_resistant_multihop_udp2tcp_tunnel( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> Result<(), Error> { + // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. + // This is an attempt to try to reduce this type of flakiness. + use helpers::custom_lists::LowLatency; + mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::On) .await @@ -533,6 +537,8 @@ pub async fn test_quantum_resistant_multihop_udp2tcp_tunnel( .wireguard() .multihop() .udp2tcp() + .entry(LowLatency) + .location(LowLatency) .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; From c2977e14534dd0b0bcacf00cdb04e19a010543e7 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 8 Nov 2024 12:56:52 +0100 Subject: [PATCH 6/8] Mitigate `test_quantum_resistant_tunnel` flakiness Limit relay selection in `test_quantum_resistant_tunnel` to reduce flakiness. Hopefully this should be able to (at least partially) mitigate timeout-related issues. --- test/test-manager/src/tests/tunnel.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index cdb890164bfa..e7056a629d4e 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -445,7 +445,11 @@ pub async fn test_quantum_resistant_tunnel( _: TestContext, rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, -) -> Result<(), Error> { +) -> anyhow::Result<()> { + // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. + // This is an attempt to try to reduce this type of flakiness. + use helpers::custom_lists::LowLatency; + mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::Off) .await @@ -459,13 +463,12 @@ pub async fn test_quantum_resistant_tunnel( log::info!("Setting tunnel protocol to WireGuard"); - let relay_settings = RelaySettings::Normal(RelayConstraints { - tunnel_protocol: Constraint::Only(TunnelType::Wireguard), - ..Default::default() - }); - set_relay_settings(&mut mullvad_client, relay_settings) - .await - .expect("Failed to update relay settings"); + let query = RelayQueryBuilder::new() + .wireguard() + .location(LowLatency) + .build(); + + apply_settings_from_relay_query(&mut mullvad_client, query).await?; mullvad_client .set_quantum_resistant_tunnel(wireguard::QuantumResistantState::On) From 0aface3813466fca789e1c984f427848c09ced6e Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 8 Nov 2024 12:58:27 +0100 Subject: [PATCH 7/8] Mitigate `test_multihop` flakiness Limit relay selection in `test_multihop` to reduce flakiness. Hopefully this should be able to (at least partially) mitigate timeout-related issues. --- test/test-manager/src/tests/tunnel.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index e7056a629d4e..beaeb63ca54a 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -294,7 +294,16 @@ pub async fn test_multihop( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> Result<(), Error> { - let query = RelayQueryBuilder::new().wireguard().multihop().build(); + // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. + // This is an attempt to try to reduce this type of flakiness. + use helpers::custom_lists::LowLatency; + + let query = RelayQueryBuilder::new() + .wireguard() + .multihop() + .location(LowLatency) + .entry(LowLatency) + .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?; From be778b2932c365e442643658098137576b1b5b51 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 8 Nov 2024 13:00:28 +0100 Subject: [PATCH 8/8] Mitigate `test_wireguard_over_shadowsocks` flakiness Limit relay selection in `test_wireguard_over_shadowsocks` to reduce flakiness. Hopefully this should be able to (at least partially) mitigate timeout-related issues. --- test/test-manager/src/tests/tunnel.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/test-manager/src/tests/tunnel.rs b/test/test-manager/src/tests/tunnel.rs index beaeb63ca54a..fde8c7304954 100644 --- a/test/test-manager/src/tests/tunnel.rs +++ b/test/test-manager/src/tests/tunnel.rs @@ -195,7 +195,15 @@ pub async fn test_wireguard_over_shadowsocks( rpc: ServiceClient, mut mullvad_client: MullvadProxyClient, ) -> anyhow::Result<()> { - let query = RelayQueryBuilder::new().wireguard().shadowsocks().build(); + // NOTE: We have experienced flakiness due to timeout issues if distant relays are selected. + // This is an attempt to try to reduce this type of flakiness. + use helpers::custom_lists::LowLatency; + + let query = RelayQueryBuilder::new() + .wireguard() + .shadowsocks() + .location(LowLatency) + .build(); apply_settings_from_relay_query(&mut mullvad_client, query).await?;