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

netbird: rework server and include new component #354032

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PatrickDaG
Copy link
Contributor

This is a pretty hefty rework of the nixos netbird modules.

First of all I split the package into three because currently you cannot have the client installed without the server components coming with it, now it's three packages, a client, a client with gui and a server.
You still have the option to build a package containing everything but I don't think most people need that.

Secondly I wrote a basic test for the server, now we at least know if it starts, which it currently doesn't cause upstream introduced clashing ports for all server, that cannot be disabled.
I would love further testing but I think that would need actually logging in into the kanidm instance inside the testing framework, which is something for another day.
The test also currently depend on #353681.

Netbird is currently switching away from coturn in favour of their own relay implementation, which this pull adds.
Their communication towards whether coturn will be needed going forward is a bit confusing, but I'm pretty sure right now you need both their relay and coturn, maybe in a few updates we can remove coturn.

Lastly I reworked the nginx setup, realizing you don't necesarrily need it, apart from serving the dashboard.
I removed it from all services and the default setup should now work without it, but you have to forward and open all relevant ports, for the management, signal, coturn, dashboard and relay.

To make it easier for people using nginx as a reverse proxy I've added the proxy, module which is written and maintained completely by myself and has no affiliation to upstream netbird. I do plan on using this and think it's valuable to have even just as a documentation of nginx options to use with netbird, but I am scared that people have problems with this module and complain to upstream netbird. Don't really know what to do whether to include or not, feedback appreciated.

In general this isn't extensively tested yet, but I would be very happy if people help me test it.
It has to wait for branch off anyway because it contains a bunch of breaking changes.

Also should probably write more documentation especially regarding the proxy module.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 6, 2024
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Nov 6, 2024
@ofborg ofborg bot requested review from vrifox and Saturn745 November 6, 2024 18:39
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@h7x4 h7x4 added 8.has: module (new) This PR adds a module in `nixos/` 8.has: tests This PR has tests labels Nov 11, 2024
@PatrickDaG PatrickDaG force-pushed the fix-netbird branch 2 times, most recently from d6f32bf to 67b0145 Compare November 12, 2024 18:33
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 13, 2024
@ofborg ofborg bot requested review from Saturn745 and vrifox November 13, 2024 06:30
@Saturn745
Copy link
Member

Result of nixpkgs-review pr 354032 run on x86_64-linux 1

2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test
3 packages built:
  • netbird
  • netbird-server
  • netbird-ui

@vrifox
Copy link
Member

vrifox commented Nov 14, 2024

Result of nixpkgs-review pr 354032 run on x86_64-linux 1
2 packages blacklisted:
nixos-install-tools tests.nixos-functions.nixos-test

3 packages built:
netbird netbird-server netbird-ui

Same here, also on x86_64-linux. :)

@TheRealGramdalf
Copy link
Contributor

TheRealGramdalf commented Nov 17, 2024

Fellow user of Kanidm/Netbird here. Can confirm that the server is non functional at the moment, again due to clashing ports. For now I'm going to have to override the signal/mgmt metrics port manually (by editing the systemd service), since there's currently no extraArgs setting or the like for the netbird components.
Edit: Turns out there's an extraOptions for the management server, but not the signal server. I fixed the port clash by adding

services.netbird.server.management.extraOptions = [ "--metrics-port=9091" ];

to my configuration.

...Is it worth creating a separate PR to fix that the port clash in the meantime?

Also please let me know if I can help with testing in any way.

@PatrickDaG
Copy link
Contributor Author

Might be a good idea to put the extraOption in separate PR so they get merged sooner. I feel like this one still has quite the journey ahead.
You wanna do it? Else I can make one in a few days.

More eyes and people testing are always welcome. I'm just gonna run the new module while I work on this PR for a few weeks and see if anything comes up. Seems like right now the blocker for both of us is kanidm kanidm/kanidm#3217

@TheRealGramdalf
Copy link
Contributor

Might be a good idea to put the extraOption in separate PR so they get merged sooner. I feel like this one still has quite the journey ahead.
You wanna do it? Else I can make one in a few days.

Yeah, can do - I was thinking add an extraOption to signal, and potentially even adding my snippet above as the default for one of them so it works out of the box again?
Or would adding a dedicated metrics option be better?

@PatrickDaG
Copy link
Contributor Author

I would say you could just add your snippet as the default and we can always switch to a dedicated option later, but thinking about it I'm not sure the newest update will work without the relay server packaged and setup so I'm not sure a new PR is worth it if it won't work anyway.

@TheRealGramdalf
Copy link
Contributor

...but thinking about it I'm not sure the newest update will work without the relay server packaged and setup so I'm not sure a new PR is worth it if it won't work anyway.

I'm on nixpkgs dc460ec76cbff0e66e269457d7b728432263166c, netbird v0.31.0, and my peers using setup keys (i.e. not depending on OIDC) are functioning just fine. #356512 was just merged though so I'm not sure if that changes things, the release notes don't seem to indicate anything however. I'll open a PR to add the snippet and hold off on a dedicated option for now, and I'll throw in an extraOptions to signal to make things easier in case things change down the line.

@PatrickDaG
Copy link
Contributor Author

I'm on nixpkgs dc460ec76cbff0e66e269457d7b728432263166c, netbird v0.31.0, and my peers using setup keys (i.e. not depending on OIDC) are functioning just fine.

Oh nice. Wasn't sure how much they already depend on the relay.

I'll open a PR to add the snippet and hold off on a dedicated option for now, and I'll throw in an extraOptions to signal to make things easier in case things change down the line.

Sounds great.

I'll try and finish this one soon as well, now that the branchoff happened.

@TheRealGramdalf
Copy link
Contributor

PR opened, input would be appreciated @PatrickDaG

@PatrickDaG PatrickDaG force-pushed the fix-netbird branch 5 times, most recently from 040588f to 0ec9479 Compare November 22, 2024 20:41
nixos/netbird: introduce proxy for unified nginx setup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 8.has: tests This PR has tests 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants