chore(windows-client): proof of concept for installing a system service with WiX #4903

ReactorScram · 2024-05-07T18:31:34Z

Before merging

Give feedback

Make sure the service auto-starts

Make sure the service auto-starts
Options
Successfully updated the issue's project

There was an error updating the issue's project
Make the process idle and report its status to Windows properly using https://github.com/mullvad/windows-service-rs

Make the process idle and report its status to Windows properly using https://github.com/mullvad/windows-service-rs
Options
Successfully updated the issue's project

There was an error updating the issue's project
DRY log dir code

DRY log dir code
Options
Successfully updated the issue's project

There was an error updating the issue's project
Figure out where service logs will go and how the GUI will zip them

Figure out where service logs will go and how the GUI will zip them
Options
Successfully updated the issue's project

There was an error updating the issue's project
Make sure the service gets a shut down signal from Windows (this is hard to catch in the Tauri GUI)

Make sure the service gets a shut down signal from Windows (this is hard to catch in the Tauri GUI)
Options
Successfully updated the issue's project

There was an error updating the issue's project
Make sure the service restarts when Firezone is updated

Make sure the service restarts when Firezone is updated
Options
Successfully updated the issue's project

There was an error updating the issue's project
Make sure the service is stopped and un-installed when Firezone is un-installed

Make sure the service is stopped and un-installed when Firezone is un-installed
Options
Successfully updated the issue's project

There was an error updating the issue's project
Add test to install the MSI and check that the service runs

Add test to install the MSI and check that the service runs
Options
Successfully updated the issue's project

There was an error updating the issue's project
(will move to another PR) ~~Clean up function names~~

(will move to another PR) ~~Clean up function names~~
Options
Successfully updated the issue's project

There was an error updating the issue's project
Make sure the Linux GUI was not broken by refactoring

Make sure the Linux GUI was not broken by refactoring
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

vercel · 2024-05-07T18:31:38Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
firezone	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2024 7:20pm

github-actions · 2024-05-07T18:33:32Z

Terraform Cloud Plan Output

Plan: 15 to add, 15 to change, 15 to destroy.

Terraform Cloud Plan

github-actions · 2024-05-07T18:41:23Z

Performance Test Results

TCP

Test Name	Received/s	Sent/s	Retransmits
direct-tcp-client2server	239.3 MiB (-1%)	241.0 MiB (-1%)	265 (+10%)
direct-tcp-server2client	237.6 MiB (-2%)	239.4 MiB (-2%)	182 (-15%)
relayed-tcp-client2server	222.9 MiB (-0%)	223.9 MiB (+0%)	310 (+26%)
relayed-tcp-server2client	249.3 MiB (+4%)	250.2 MiB (+4%)	340 (-3%)

UDP

Test Name	Total/s	Jitter	Lost
direct-udp-client2server	500.0 MiB (+0%)	0.02ms (-2%)	41.72% (-10%)
direct-udp-server2client	500.0 MiB (-0%)	0.02ms (-57%)	20.51% (-0%)
relayed-udp-client2server	500.0 MiB (+0%)	0.09ms (+103%)	56.44% (+4%)
relayed-udp-server2client	500.0 MiB (-0%)	0.01ms (-65%)	39.14% (-1%)

…gets confused

…-poc

I'm going to want a well-known dir that the Windows IPC service writes logs to, and that the Windows GUI can pick them up from. I don't know how I did this for Linux last week, but it should probably be in here too.

…one into chore/windows-service-poc

ReactorScram · 2024-05-08T19:56:37Z

rust/gui-client/docs/intended_behavior.md

@@ -39,7 +39,7 @@ Best performed on a clean VM
 1. Export the logs
 1. Expect the zip file to start with "firezone_logs_"
 1. Expect `zipinfo` to show a single directory in the root of the zip, to prevent zip bombing
-1. Expect two subdirectories in the zip, "connlib", and "app", each with 3 files, totalling 6 files
+1. Expect two subdirectories in the zip, "connlib", and "app", with 3 and 2 files respectively, totalling 5 files


I'm not sure why this had 6 files before. I have two hypotheses:

I had been clicking on the notification to test the single-instance behavior, that could create a 3rd log file

Maybe the deep link used to write a log file and stopped recently

The 5 files should be correct now though:

The IPC service runs when it's first installed

After you reboot, the IPC service runs again

Then the GUI runs

Then there's another reboot, and the IPC service runs again

Then the GUI runs again

A 6th file would have to come from somewhere odd.

…-poc

conectado

Left a few comments, nothing major but is there an issue corresponding to this PR? I'm not familiar with what WiX is

conectado · 2024-05-09T21:39:58Z

rust/headless-client/src/imp_linux.rs

    let (layer, _handle) = cli.log_dir.as_deref().map(file_logger::layer).unzip();
    setup_global_subscriber(layer);
    tracing::info!(git_version = crate::GIT_VERSION);

    if !nix::unistd::getuid().is_root() {
        anyhow::bail!("This is the IPC service binary, it's not meant to run interactively.");
    }
-    run_ipc_service(cli)
+    let rt = tokio::runtime::Runtime::new()?;
+    let (_shutdown_tx, shutdown_rx) = mpsc::channel(1);


this channel doesn't seem to be used anywhere

Yeah, it's only there to match the signature of the Windows implementation. On Windows it signals from the main thread (which handles requests from the Windows service controller) to the service function (which Windows forces to run in a worker thread) telling our service function to exit.

There is probably a way to move the caller into the platform-specific files, or replace the shutdown channel with a poll function, but I'm not sure how to do it from here.

rust/headless-client/src/known_dirs.rs

rust/headless-client/src/imp_windows.rs

conectado · 2024-05-09T22:56:33Z

rust/headless-client/src/imp_windows.rs

+            ServiceControl::SessionChange(_) => ServiceControlHandlerResult::NotImplemented,
+            ServiceControl::TimeChange => ServiceControlHandlerResult::NotImplemented,
+            ServiceControl::TriggerEvent => ServiceControlHandlerResult::NotImplemented,
+            _ => todo!(),


Can we just match all the above here with NotImplemented?

Yep. The enum is non-exhaustive so I had to keep the _ and stick the others together with |. Otherwise it would run afoul of the linter.

but we could just remove all the previous NotImplemented matches and just match _ => NotImplemented

conectado · 2024-05-09T22:59:31Z

rust/headless-client/src/imp_windows.rs

+    shutdown_rx: mpsc::Receiver<()>,
+) -> Result<()> {
+    tracing::info!("run_ipc_service");
+    rt.block_on(async { ipc_listen(cli, shutdown_rx).await })


can we move shutdown_rx.recv().await here?

Maybe. I did it this way to match the Linux impl, but actually in both cases I could inline the async fn into an async block. All that this does is wrap block_on and save one layer of indentation.

ReactorScram · 2024-05-09T23:15:32Z

@conectado Yes, the related issue is #4899. I couldn't find a way to make Windows automatically delete the DNS rules when we exit, and this was causing problems when the system reboots. By running this background service, we can just clear our own rules on boot. And we'll need the service soon anyway for #3712

ReactorScram · 2024-05-09T23:19:32Z

And WiX is a toolkit from Microsoft that uses an XML schema to generate MSI installers. Tauri uses it internally, so I took it apart a little bit to add our background service. https://wixtoolset.org/docs/v3/

conectado · 2024-05-09T23:48:22Z

@conectado Yes, the related issue is #4899. I couldn't find a way to make Windows automatically delete the DNS rules when we exit, and this was causing problems when the system reboots. By running this background service, we can just clear our own rules on boot. And we'll need the service soon anyway for #3712

oh so this service would only delete our NRPT rules?

ReactorScram · 2024-05-10T14:07:07Z

@conectado Yeah #4918 builds on this to delete the NRPT rules, and that'll close #4899.
In this PR I just added a do-nothing placeholder for the service. #4918 could be squashed into this one, it's only a few lines.

conectado

LGTM

proof of concept for installing a system service with WiX

e742f80

ReactorScram added the area/windows_client Issues related to the Windows client label May 7, 2024

ReactorScram self-assigned this May 7, 2024

github-actions bot added the kind/chore Issues related to repository cleanup or maintenance label May 7, 2024

ReactorScram added 2 commits May 7, 2024 15:06

this one gets the service to run, but it doesn't respond, so Windows …

c83174f

…gets confused

Merge remote-tracking branch 'origin/main' into chore/windows-service…

6d36d31

…-poc

ReactorScram mentioned this pull request May 7, 2024

Run Windows GUI and tunnel in separate processes #3712

Closed

ReactorScram added 4 commits May 7, 2024 15:09

fmt

abc9953

this one starts and stops properly

e615f93

fix Linux build

b8a8670

Merge remote-tracking branch 'origin/main' into chore/windows-service…

4011ca3

…-poc

ReactorScram mentioned this pull request May 7, 2024

ux(headless-client): remove auto-detected IPC service mode #4907

Closed

ReactorScram added 2 commits May 7, 2024 18:15

refactor(gui-client): move known_dirs module into headless-client

618c941

I'm going to want a well-known dir that the Windows IPC service writes logs to, and that the Windows GUI can pick them up from. I don't know how I did this for Linux last week, but it should probably be in here too.

fix Linux build

442e01c

ReactorScram changed the base branch from main to refactor/move-known-dirs May 8, 2024 13:48

ReactorScram added 2 commits May 8, 2024 08:48

Merge branch 'refactor/move-known-dirs' into chore/windows-service-poc

7e17b56

Merge branch 'chore/windows-service-poc' of github.com:firezone/firez…

8585e2d

…one into chore/windows-service-poc

vercel bot deployed to Preview May 8, 2024 13:50 View deployment

ReactorScram added 9 commits May 8, 2024 09:21

DRY log dirs

679a9da

typo

606469f

fix logging

12edbc1

fix stopping, remove debug file

0c77e34

opt in to receive shutdown signals

9bc5d5c

do a test install of the MSI

6083819

/passive wasn't working in CI

0a35424

try to fix CI

d5f58a6

you gotta escape those deadly forward slashes

900737b

fix test checklist

0f9de54

ReactorScram commented May 8, 2024

View reviewed changes

ReactorScram marked this pull request as ready for review May 8, 2024 19:57

ReactorScram requested a review from conectado May 8, 2024 19:57

Base automatically changed from refactor/move-known-dirs to main May 8, 2024 20:42

Merge remote-tracking branch 'origin/main' into chore/windows-service…

470eb1c

…-poc

vercel bot deployed to Preview May 9, 2024 14:14 View deployment

ReactorScram mentioned this pull request May 9, 2024

refactor(headless-client): remove "linux-client" alias #4933

Merged

Merge branch 'main' into chore/windows-service-poc

91b2c10

vercel bot deployed to Preview May 9, 2024 22:12 View deployment

conectado reviewed May 9, 2024

View reviewed changes

ReactorScram added 2 commits May 10, 2024 09:21

remove unimplemented fn

0a7e678

feedback from review

9be70e2

ReactorScram requested a review from conectado May 10, 2024 15:12

Merge branch 'main' into chore/windows-service-poc

733afa0

vercel bot deployed to Preview May 10, 2024 19:20 View deployment

conectado approved these changes May 10, 2024

View reviewed changes

ReactorScram added this pull request to the merge queue May 13, 2024

Merged via the queue into main with commit 5814efc May 13, 2024
135 checks passed

ReactorScram deleted the chore/windows-service-poc branch May 13, 2024 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(windows-client): proof of concept for installing a system service with WiX #4903

chore(windows-client): proof of concept for installing a system service with WiX #4903

ReactorScram commented May 7, 2024 •

edited

Before merging

vercel bot commented May 7, 2024 •

edited

github-actions bot commented May 7, 2024 •

edited

github-actions bot commented May 7, 2024 •

edited

ReactorScram May 8, 2024

conectado left a comment

conectado May 9, 2024

ReactorScram May 10, 2024

conectado May 9, 2024

ReactorScram May 10, 2024

conectado May 10, 2024

conectado May 9, 2024

ReactorScram May 10, 2024

ReactorScram commented May 9, 2024

ReactorScram commented May 9, 2024

conectado commented May 9, 2024

ReactorScram commented May 10, 2024

conectado left a comment

chore(windows-client): proof of concept for installing a system service with WiX #4903

chore(windows-client): proof of concept for installing a system service with WiX #4903

Conversation

ReactorScram commented May 7, 2024 • edited

Before merging

vercel bot commented May 7, 2024 • edited

github-actions bot commented May 7, 2024 • edited

Terraform Cloud Plan Output

github-actions bot commented May 7, 2024 • edited

Performance Test Results

TCP

UDP

Choose a reason for hiding this comment

conectado left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ReactorScram commented May 9, 2024

ReactorScram commented May 9, 2024

conectado commented May 9, 2024

ReactorScram commented May 10, 2024

conectado left a comment

Choose a reason for hiding this comment

ReactorScram commented May 7, 2024 •

edited

vercel bot commented May 7, 2024 •

edited

github-actions bot commented May 7, 2024 •

edited

github-actions bot commented May 7, 2024 •

edited