Write e2e tests for split tunneling

[hulthe]

Basic E2E tests of split tunneling for Windows and Linux. Also includes some changes to the testing framework that fixes some problems I encountered while implementing and testing this.

The split tunnel test is implemented by spawning a seperate process (connecton-checker) that tries to contact https://am.i.mullvad.net, and send TCP, UDP, and ICMP packets to 1.1.1.1.

This test does not run on MacOS ATM. But if split tunnel handling on MacOS isn't notably different from Linux or Windows, then it could be enabled fairly easily.

This PR also:

• Replaces panics in test_function macro with syn errors, to get nicer compiler errors.
• Enables anyhow errors in e2e tests.
• Makes paths to OVMF vars configurable (the default paths were different on my system).
• Tries to tidy up any code I found hard to read.

^ I could spend some time to split some of these into seperate PRs if you, dear reviewer, thinks its worth it.

---

This change is 

[linear]

DES-276 Write tests for split tunneling

[hulthe]

mullvad-management-interface/src/client.rs line 594 at r1 (raw file):

    }

    #[cfg(target_os = "linux")]

This one is interesting. It's needed because we run the MullvadProxyClient on a different host from the daemon. Do you see any problems with exposing these methods on other platforms?

[dlon]

Reviewed 16 of 23 files at r1, 9 of 9 files at r2, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @hulthe)

---

mullvad-management-interface/src/client.rs line 594 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

This one is interesting. It's needed because we run the MullvadProxyClient on a different host from the daemon. Do you see any problems with exposing these methods on other platforms?

It's nice that we cannot accidentally use these methods when they're unavailable. But since we need them, I think it's fine.

---

test/scripts/build-runner-image.sh line 36 at r2 (raw file):

            -i "${TEST_RUNNER_IMAGE_PATH}" \
            "${SCRIPT_DIR}/../target/$TARGET/release/test-runner.exe" \
            "${SCRIPT_DIR}/../target/$TARGET/release/connection-checker.exe" \

ssh-setup.sh should also be updated to copy this file from the temp dir.

---

test/test-manager/src/tests/mod.rs line 39 at r2 (raw file):

#[derive(thiserror::Error, Debug)]
pub enum Error {

We could probably get rid of this Error. Very unrelated to the PR, though.

---

test/test-manager/src/tests/split_tunnel.rs line 18 at r2 (raw file):

use super::{config::TEST_CONFIG, helpers, TestContext};

const CHECKER_PATH_WINDOWS: &str = "E:\\connection-checker.exe";

The path should be relative to TEST_CONFIG.artifacts_dir.

---

test/test-manager/src/tests/split_tunnel.rs line 27 at r2 (raw file):

/// - Splitting/unsplitting should work regardless if process is running.
#[test_function]
pub async fn test_split_tunnel(

We need to prevent the test from running on macOS.

---

test/test-manager/src/tests/split_tunnel.rs line 32 at r2 (raw file):

    mut mullvad_client: MullvadProxyClient,
) -> anyhow::Result<()> {
    helpers::disconnect_and_wait(&mut mullvad_client).await?;

This can be removed.

---

test/test-manager/src/tests/split_tunnel.rs line 85 at r2 (raw file):

    (handle.assert_secure().await)
        .with_context(|| "Test connected and being unsplit while running")?;
    drop(handle);

Nit: This can be removed.

---

test/test-manager/src/tests/split_tunnel.rs line 110 at r2 (raw file):

}

struct ConnectonStatus {

Nit: Connecton

---

test/test-rpc/src/transport.rs line 259 at r2 (raw file):

    loop {
        futures::select! {

Nice. A lot more readable. 😅

---

test/test-runner/src/main.rs line 449 at r2 (raw file):

            .spawned_procs
            .get_mut(&pid)
            .expect("TODO: unknown pid error");

TODO

---

test/test-runner/src/main.rs line 484 at r2 (raw file):

            .spawned_procs
            .get_mut(&pid)
            .expect("TODO: unknown pid error");

TODO

---

test/test-runner/src/main.rs line 504 at r2 (raw file):

            .spawned_procs
            .get_mut(&pid)
            .expect("TODO: unknown pid error");

TODO

---

test/test-runner/src/util.rs line 2 at r2 (raw file):

/// Drop guard that executes the provided callback function when dropped.
pub struct OnDrop<F = Box<dyn FnOnce() + Send>>

I think we could probably replace AbortOnDrop with this as well.

[hulthe]

Reviewable status: 24 of 26 files reviewed, 9 unresolved discussions (waiting on @dlon)

---

test/test-manager/src/tests/split_tunnel.rs line 18 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

The path should be relative to TEST_CONFIG.artifacts_dir.

Fixed!

---

test/test-manager/src/tests/split_tunnel.rs line 32 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

This can be removed.

👍

---

test/test-manager/src/tests/split_tunnel.rs line 85 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nit: This can be removed.

👍

---

test/test-rpc/src/transport.rs line 259 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nice. A lot more readable. 😅

happy you agree :D

[hulthe]

Reviewable status: 24 of 26 files reviewed, 7 unresolved discussions (waiting on @dlon)

---

test/scripts/build-runner-image.sh line 36 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

ssh-setup.sh should also be updated to copy this file from the temp dir.

fixed

---

test/test-manager/src/tests/split_tunnel.rs line 18 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Fixed!

fixed

[dlon]

Reviewed 2 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @hulthe)

[hulthe]

Reviewable status: 24 of 27 files reviewed, 2 unresolved discussions (waiting on @dlon)

---

test/test-manager/src/tests/mod.rs line 39 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

We could probably get rid of this Error. Not very unrelated to the PR, though.

I could look into doing this in a seperate PR

---

test/test-runner/src/util.rs line 2 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

I think we could probably replace AbortOnDrop with this as well.

Agreed, but they're in different crates so we'd need to move it to a shared one.

[dlon]

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @hulthe)

---

test/test-manager/src/tests/test_metadata.rs line 8 at r5 (raw file):

    pub name: &'static str,
    pub command: &'static str,
    pub targets: &'static [Os],

Nice!

[dlon]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved