Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wsl-vpnkit: init at 0.4.1 #238702

Merged
merged 1 commit into from
Jul 23, 2023
Merged

wsl-vpnkit: init at 0.4.1 #238702

merged 1 commit into from
Jul 23, 2023

Conversation

terlar
Copy link
Contributor

@terlar terlar commented Jun 20, 2023

Description of changes

Add the package wsl-vpnkit which can be used to revive network connectivity within WSL when connecting to certain VPN:s that would otherwise break the network within WSL. The main caveat with the packaging was that I could not re-use the nixpkgs gvproxy since it needed the Windows version, hence there is an override to cross-compile it and then refer to the produced exe in the wrapper. Script also needs to be run as root.

Cross linking previous issue from NixOS-WSL

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jun 20, 2023
pkgs/tools/networking/wsl-vpnkit/default.nix Outdated Show resolved Hide resolved
let
version = "0.4.1";
gvproxyCross = gvproxy.overrideAttrs (_: {
buildPhase = "make cross qemu-wrapper vm";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a common usecase? If so maybe an option or a top-level attr would be a good idea?

Copy link
Contributor Author

@terlar terlar Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, perhaps, I just checked what was publishing those binaries on their releases and it seems to be that command. Because wsl-vpnkit was using it from there.

Actually we only need the Windows binary, the vm can be used from the standard gvproxy.

For example this works as well (note this package is from my local packages, hence the strange diff):

diff --git a/packages/wsl-vpnkit/default.nix b/packages/wsl-vpnkit/default.nix
index 807295c..ace87ad 100644
--- a/packages/wsl-vpnkit/default.nix
+++ b/packages/wsl-vpnkit/default.nix
@@ -12,8 +12,10 @@
   iputils,
   wget,
 }: let
-  gvproxyCross = gvproxy.overrideAttrs (_: {
-    buildPhase = "make cross qemu-wrapper vm";
+  gvproxyWin = gvproxy.overrideAttrs (_: {
+    buildPhase = ''
+      GOARCH=amd64 GOOS=windows go build -ldflags '-s -w' -o bin/gvproxy-windows.exe ./cmd/gvproxy
+    '';
   });
 in
   stdenv.mkDerivation rec {
@@ -31,8 +33,8 @@ in

     postPatch = ''
       substituteInPlace wsl-vpnkit \
-        --replace "/app/wsl-vm" "${gvproxyCross}/bin/vm" \
-        --replace "/app/wsl-gvproxy.exe" "${gvproxyCross}/bin/gvproxy-windows.exe"
+        --replace "/app/wsl-vm" "${gvproxy}/bin/vm" \
+        --replace "/app/wsl-gvproxy.exe" "${gvproxyWin}/bin/gvproxy-windows.exe"
     '';

     installPhase = ''

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to only build the gvproxy command for windows inside the buildPhase. And then I pick the gvforwarder (vm) from the gvproxy package directly.

Do you think it still makes sense to try to merge that into the main package?

@terlar terlar force-pushed the add-wsl-vpnkit branch 2 times, most recently from fef0575 to 33660c4 Compare July 5, 2023 22:57
@terlar
Copy link
Contributor Author

terlar commented Jul 12, 2023

@SuperSandro2000 This is ready for another pass, in the meantime I fixed the following things:

  • The gvproxy changed the name of the "vm" binary in the new version tracked by nixpkgs, so updated that
  • Fix ping/iptables binary resolution, found out that they were not sufficient when running as a systemd unit

I guess the only open question is regarding the gvproxy windows executable should be an option or go to the top-level. I am not sure who knows could decide such a thing. Personally I know no other use case for it at this point.

@SuperSandro2000 SuperSandro2000 merged commit 9275ed1 into NixOS:master Jul 23, 2023
@SuperSandro2000
Copy link
Member

Fine for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants