Merge branch 'fix-macos-tests' into main

mullvad · Nov 22, 2023 · f7eb43c · f7eb43c
2 parents 6b0b627 + 56970ba
commit f7eb43c
Show file tree

Hide file tree

Showing 21 changed files with 450 additions and 433 deletions.
diff --git a/.github/workflows/desktop-e2e.yml b/.github/workflows/desktop-e2e.yml
@@ -70,11 +70,10 @@ jobs:
           path: ~/.cache/mullvad-test/packages
       - name: Checkout repository
         uses: actions/checkout@v4
-        with:
-          fetch-tags: true
       - name: Run end-to-end tests
         shell: bash -ieo pipefail {0}
         run: |
+          git fetch --tags --force
           ./test/ci-runtests.sh ${{ matrix.os }}
       - uses: actions/upload-artifact@v3
         if: '!cancelled()'
@@ -140,11 +139,10 @@ jobs:
           path: ~/.cache/mullvad-test/packages
       - name: Checkout repository
         uses: actions/checkout@v4
-        with:
-          fetch-tags: true
       - name: Run end-to-end tests
         shell: bash -ieo pipefail {0}
         run: |
+          git fetch --tags --force
           ./test/ci-runtests.sh ${{ matrix.os }}
       - uses: actions/upload-artifact@v3
         if: '!cancelled()'
@@ -154,36 +152,30 @@ jobs:
 
   build-macos:
     if: ${{ !startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/main' }}
-    runs-on: macos-latest
+    runs-on: [self-hosted, desktop-test, macOS] # app-test-macos-arm
     steps:
       - name: Checkout repository
         uses: actions/checkout@v2
       - name: Checkout submodules
         run: git submodule update --init --depth=1
+      - name: Install Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: 1.18.5
       - name: Install Protoc
-        uses: arduino/setup-protoc@v1
+        uses: arduino/setup-protoc@v2
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - uses: actions/setup-node@v3
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           node-version: 18
-      - name: Install Rust
-        uses: actions-rs/[email protected]
-        with:
-          toolchain: stable
-          target: aarch64-apple-darwin
-          default: true
-      - name: Install Go
-        uses: actions/setup-go@v3
-        with:
-          go-version: 1.18.5
+          cache: 'npm'
+          cache-dependency-path: gui/package-lock.json
       - name: Build app
-        run: ./build.sh --universal
+        run: ./build.sh
       - name: Build test executable
-        run: ./gui/scripts/build-test-executable.sh aarch64-apple-darwin
-        # FIXME: This fails for some reason, but the artifact is built
-        continue-on-error: true
+        run: ./gui/scripts/build-test-executable.sh
       - uses: actions/upload-artifact@v3
         if: '!cancelled()'
         with:
@@ -210,11 +202,10 @@ jobs:
           path: ~/Library/Caches/mullvad-test/packages
       - name: Checkout repository
         uses: actions/checkout@v4
-        with:
-          fetch-tags: true
       - name: Run end-to-end tests
         shell: bash -ieo pipefail {0}
         run: |
+          git fetch --tags --force
           ./test/ci-runtests.sh ${{ matrix.os }}
       - uses: actions/upload-artifact@v3
         if: '!cancelled()'

diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
@@ -1899,9 +1899,15 @@ where
             .await
         {
             Ok(settings_changed) => {
-                Self::oneshot_send(tx, Ok(()), "set_allow_lan response");
                 if settings_changed {
-                    self.send_tunnel_command(TunnelCommand::AllowLan(allow_lan));
+                    self.send_tunnel_command(TunnelCommand::AllowLan(
+                        allow_lan,
+                        oneshot_map(tx, |tx, ()| {
+                            Self::oneshot_send(tx, Ok(()), "set_allow_lan response");
+                        }),
+                    ));
+                } else {
+                    Self::oneshot_send(tx, Ok(()), "set_allow_lan response");
                 }
             }
             Err(e) => {
@@ -1946,11 +1952,15 @@ where
             .await
         {
             Ok(settings_changed) => {
-                Self::oneshot_send(tx, Ok(()), "set_block_when_disconnected response");
                 if settings_changed {
                     self.send_tunnel_command(TunnelCommand::BlockWhenDisconnected(
                         block_when_disconnected,
+                        oneshot_map(tx, |tx, ()| {
+                            Self::oneshot_send(tx, Ok(()), "set_block_when_disconnected response");
+                        }),
                     ));
+                } else {
+                    Self::oneshot_send(tx, Ok(()), "set_block_when_disconnected response");
                 }
             }
             Err(e) => {
@@ -2148,12 +2158,18 @@ where
             .await
         {
             Ok(settings_changed) => {
-                Self::oneshot_send(tx, Ok(()), "set_dns_options response");
                 if settings_changed {
                     let settings = self.settings.to_settings();
                     let resolvers =
                         dns::addresses_from_options(&settings.tunnel_options.dns_options);
-                    self.send_tunnel_command(TunnelCommand::Dns(resolvers));
+                    self.send_tunnel_command(TunnelCommand::Dns(
+                        resolvers,
+                        oneshot_map(tx, |tx, ()| {
+                            Self::oneshot_send(tx, Ok(()), "set_dns_options response");
+                        }),
+                    ));
+                } else {
+                    Self::oneshot_send(tx, Ok(()), "set_dns_options response");
                 }
             }
             Err(e) => {
@@ -2396,7 +2412,8 @@ where
             && (*self.target_state == TargetState::Secured || self.settings.auto_connect)
         {
             log::debug!("Blocking firewall during shutdown since system is going down");
-            self.send_tunnel_command(TunnelCommand::BlockWhenDisconnected(true));
+            let (tx, _rx) = oneshot::channel();
+            self.send_tunnel_command(TunnelCommand::BlockWhenDisconnected(true, tx));
         }
 
         self.state.shutdown(&self.tunnel_state);
@@ -2408,7 +2425,8 @@ where
         //       without causing the service to be restarted.
 
         if *self.target_state == TargetState::Secured {
-            self.send_tunnel_command(TunnelCommand::BlockWhenDisconnected(true));
+            let (tx, _rx) = oneshot::channel();
+            self.send_tunnel_command(TunnelCommand::BlockWhenDisconnected(true, tx));
         }
         self.target_state.lock();
     }
@@ -2569,3 +2587,19 @@ fn new_selector_config(settings: &Settings) -> SelectorConfig {
         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>(
+    tx: oneshot::Sender<T1>,
+    forwarder: impl Fn(oneshot::Sender<T1>, T2) + Send + 'static,
+) -> oneshot::Sender<T2> {
+    let (new_tx, new_rx) = oneshot::channel();
+    tokio::spawn(async move {
+        match new_rx.await {
+            Ok(result) => forwarder(tx, result),
+            Err(oneshot::Canceled) => (),
+        }
+    });
+    new_tx
+}
diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs
@@ -73,6 +73,10 @@ impl MullvadProxyClient {
         super::new_rpc_client().await.map(Self)
     }
 
+    pub fn from_rpc_client(client: crate::ManagementServiceClient) -> Self {
+        Self(client)
+    }
+
     pub async fn connect_tunnel(&mut self) -> Result<bool> {
         Ok(self
             .0

diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -213,8 +213,8 @@ impl ConnectedState {
         use self::EventConsequence::*;
 
         match command {
-            Some(TunnelCommand::AllowLan(allow_lan)) => {
-                if let Err(error_cause) = shared_values.set_allow_lan(allow_lan) {
+            Some(TunnelCommand::AllowLan(allow_lan, complete_tx)) => {
+                let consequence = if let Err(error_cause) = shared_values.set_allow_lan(allow_lan) {
                     self.disconnect(shared_values, AfterDisconnect::Block(error_cause))
                 } else {
                     match self.set_firewall_policy(shared_values) {
@@ -230,43 +230,55 @@ impl ConnectedState {
                             AfterDisconnect::Block(ErrorStateCause::SetFirewallPolicyError(error)),
                         ),
                     }
-                }
+                };
+                let _ = complete_tx.send(());
+                consequence
             }
             Some(TunnelCommand::AllowEndpoint(endpoint, tx)) => {
                 shared_values.allowed_endpoint = endpoint;
                 let _ = tx.send(());
                 SameState(self)
             }
-            Some(TunnelCommand::Dns(servers)) => match shared_values.set_dns_servers(servers) {
-                Ok(true) => {
-                    if let Err(error) = self.set_firewall_policy(shared_values) {
-                        return self.disconnect(
-                            shared_values,
-                            AfterDisconnect::Block(ErrorStateCause::SetFirewallPolicyError(error)),
-                        );
-                    }
-
-                    match self.set_dns(shared_values) {
-                        #[cfg(target_os = "android")]
-                        Ok(()) => self.disconnect(shared_values, AfterDisconnect::Reconnect(0)),
-                        #[cfg(not(target_os = "android"))]
-                        Ok(()) => SameState(self),
-                        Err(error) => {
-                            log::error!("{}", error.display_chain_with_msg("Failed to set DNS"));
-                            self.disconnect(
+            Some(TunnelCommand::Dns(servers, complete_tx)) => {
+                let consequence = match shared_values.set_dns_servers(servers) {
+                    Ok(true) => {
+                        if let Err(error) = self.set_firewall_policy(shared_values) {
+                            return self.disconnect(
                                 shared_values,
-                                AfterDisconnect::Block(ErrorStateCause::SetDnsError),
-                            )
+                                AfterDisconnect::Block(ErrorStateCause::SetFirewallPolicyError(
+                                    error,
+                                )),
+                            );
+                        }
+
+                        match self.set_dns(shared_values) {
+                            #[cfg(target_os = "android")]
+                            Ok(()) => self.disconnect(shared_values, AfterDisconnect::Reconnect(0)),
+                            #[cfg(not(target_os = "android"))]
+                            Ok(()) => SameState(self),
+                            Err(error) => {
+                                log::error!(
+                                    "{}",
+                                    error.display_chain_with_msg("Failed to set DNS")
+                                );
+                                self.disconnect(
+                                    shared_values,
+                                    AfterDisconnect::Block(ErrorStateCause::SetDnsError),
+                                )
+                            }
                         }
                     }
-                }
-                Ok(false) => SameState(self),
-                Err(error_cause) => {
-                    self.disconnect(shared_values, AfterDisconnect::Block(error_cause))
-                }
-            },
-            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected)) => {
+                    Ok(false) => SameState(self),
+                    Err(error_cause) => {
+                        self.disconnect(shared_values, AfterDisconnect::Block(error_cause))
+                    }
+                };
+                let _ = complete_tx.send(());
+                consequence
+            }
+            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected, complete_tx)) => {
                 shared_values.block_when_disconnected = block_when_disconnected;
+                let _ = complete_tx.send(());
                 SameState(self)
             }
             Some(TunnelCommand::IsOffline(is_offline)) => {

diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -392,12 +392,14 @@ impl ConnectingState {
         use self::EventConsequence::*;
 
         match command {
-            Some(TunnelCommand::AllowLan(allow_lan)) => {
-                if let Err(error_cause) = shared_values.set_allow_lan(allow_lan) {
+            Some(TunnelCommand::AllowLan(allow_lan, complete_tx)) => {
+                let consequence = if let Err(error_cause) = shared_values.set_allow_lan(allow_lan) {
                     self.disconnect(shared_values, AfterDisconnect::Block(error_cause))
                 } else {
                     self.reset_firewall(shared_values)
-                }
+                };
+                let _ = complete_tx.send(());
+                consequence
             }
             Some(TunnelCommand::AllowEndpoint(endpoint, tx)) => {
                 if shared_values.allowed_endpoint != endpoint {
@@ -418,14 +420,19 @@ impl ConnectingState {
                 let _ = tx.send(());
                 SameState(self)
             }
-            Some(TunnelCommand::Dns(servers)) => match shared_values.set_dns_servers(servers) {
-                #[cfg(target_os = "android")]
-                Ok(true) => self.disconnect(shared_values, AfterDisconnect::Reconnect(0)),
-                Ok(_) => SameState(self),
-                Err(cause) => self.disconnect(shared_values, AfterDisconnect::Block(cause)),
-            },
-            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected)) => {
+            Some(TunnelCommand::Dns(servers, complete_tx)) => {
+                let consequence = match shared_values.set_dns_servers(servers) {
+                    #[cfg(target_os = "android")]
+                    Ok(true) => self.disconnect(shared_values, AfterDisconnect::Reconnect(0)),
+                    Ok(_) => SameState(self),
+                    Err(cause) => self.disconnect(shared_values, AfterDisconnect::Block(cause)),
+                };
+                let _ = complete_tx.send(());
+                consequence
+            }
+            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected, complete_tx)) => {
                 shared_values.block_when_disconnected = block_when_disconnected;
+                let _ = complete_tx.send(());
                 SameState(self)
             }
             Some(TunnelCommand::IsOffline(is_offline)) => {

diff --git a/talpid-core/src/tunnel_state_machine/disconnected_state.rs b/talpid-core/src/tunnel_state_machine/disconnected_state.rs
@@ -128,7 +128,7 @@ impl TunnelState for DisconnectedState {
         use self::EventConsequence::*;
 
         match runtime.block_on(commands.next()) {
-            Some(TunnelCommand::AllowLan(allow_lan)) => {
+            Some(TunnelCommand::AllowLan(allow_lan, complete_tx)) => {
                 if shared_values.allow_lan != allow_lan {
                     // The only platform that can fail is Android, but Android doesn't support the
                     // "block when disconnected" option, so the following call never fails.
@@ -138,6 +138,7 @@ impl TunnelState for DisconnectedState {
 
                     Self::set_firewall_policy(shared_values, false);
                 }
+                let _ = complete_tx.send(());
                 SameState(self)
             }
             Some(TunnelCommand::AllowEndpoint(endpoint, tx)) => {
@@ -148,15 +149,15 @@ impl TunnelState for DisconnectedState {
                 let _ = tx.send(());
                 SameState(self)
             }
-            Some(TunnelCommand::Dns(servers)) => {
+            Some(TunnelCommand::Dns(servers, complete_tx)) => {
                 // Same situation as allow LAN above.
                 shared_values
                     .set_dns_servers(servers)
                     .expect("Failed to reconnect after changing custom DNS servers");
-
+                let _ = complete_tx.send(());
                 SameState(self)
             }
-            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected)) => {
+            Some(TunnelCommand::BlockWhenDisconnected(block_when_disconnected, complete_tx)) => {
                 if shared_values.block_when_disconnected != block_when_disconnected {
                     shared_values.block_when_disconnected = block_when_disconnected;
                     Self::set_firewall_policy(shared_values, true);
@@ -178,6 +179,7 @@ impl TunnelState for DisconnectedState {
                         Self::reset_dns(shared_values);
                     }
                 }
+                let _ = complete_tx.send(());
                 SameState(self)
             }
             Some(TunnelCommand::IsOffline(is_offline)) => {