nixos/netbird: fix port conflict on metrics endpoint

[TheRealGramdalf]

Added changes discussed in #354032 (comment) to fix a port clash between the netbird management and signal services, as well as an extraOption to facilitate ad-hoc changes in the future should more breaking changes be released by upstream.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

[TheRealGramdalf]

I based this on the netbird 0.32.0 release before realizing that PR was merged to master after the branch off - it would be helpful to know from @Saturn745 or @vrifox whether I should rebase this PR against the 0.31.1 release currently in the branch off (which still has the issues fixed) or if the 0.32.0 release PR should be backported as well.

[PatrickDaG]

24.11 has alredy been branched off so if you want to add a release note it should be 25.05. But since its an anti breaking change I wouldn't say a release note is necessary.

[oddlama]

Not sure how backporting affects release notes, but I would presume that keeping the notes will require a manual backport

[TheRealGramdalf]

I asked on the matrix channel and they said this should definitely be backported, so I'm guessing the release note should stay for 24.11

[TheRealGramdalf]

Not sure how backporting affects release notes, but I would presume that keeping the notes will require a manual backport

I believe a manual backport would be necessary regardless, unless one of the package maintainers were to initiate it once this gets merged

[oddlama]

I based this on the netbird 0.32.0 release before realizing that PR was merged to master after the branch off - it would be helpful to know from @Saturn745 or @vrifox whether I should rebase this PR against the 0.31.1 release currently in the branch off (which still has the issues fixed) or if the 0.32.0 release PR should be backported as well.

No need, backporting will only consider changes in this PR so it doesn't matter what you base this against

which still has the issues fixed

What do you mean? Assuming you mean the issue is not present in 0.31.1 then I guess we won't have to backport at all

[PatrickDaG]

I think if we're backporting this we should do it right and add a dedicated metricsPort option, you can just copy mine from the other PR. Sounds like a bad idea to rely on a default extraOption, those should only be used in special cases.

[TheRealGramdalf]

What do you mean? Assuming you mean the issue is not present in 0.31.1 then I guess we won't have to backport at all

Both 0.31.1 and 0.32.0 are affected by the port clash. I realized after I created the PR that the 0.32.0 update PR didn't make it into the branch off.
This only really matters because of the release notes, since theres a link in there pointing to the 0.32.0 release notes. My question was whether or not I should change the link to point at 0.31.1, or if we should backport the 0.32.0 PR as well.

[TheRealGramdalf]

I think if we're backporting this we should do it right and add a dedicated metricsPort option, you can just copy mine from the other PR. Sounds like a bad idea to rely on a default extraOption, those should only be used in special cases.

Will do. I'll still add the extraOptions to signal to make it consistent and to aid in case of further breakages from upstream.

[TheRealGramdalf]

Both 0.31.1 and 0.32.0 are affected by the port clash. I realized after I created the PR that the 0.32.0 update PR didn't make it into the branch off. This only really matters because of the release notes, since theres a link in there pointing to the 0.32.0 release notes. My question was whether or not I should change the link to point at 0.31.1, or if we should backport the 0.32.0 PR as well.

I'll just go ahead and change this to 0.31.1. If #356512 gets backported before this is merged I can switch it back, but for now I'll assume the version of netbird in the 24.11 release is going to be 0.31.1.

[TheRealGramdalf]

Ready for merge. If I need to create another PR manually for the backport just let me know.

[oddlama]

Just FYI: In case this will not be merged today you'll probably have to change the release notes to 25.05 after branch-off

[TheRealGramdalf]

Yeah, I'll wait until it's clear it won't be getting in, no need for extra ofborg evals. I was hoping to get it in for 24.11 not only for the port fix, but to make people aware of the additional ports in case of other conflicts/if updates to firewall rules are required. Ideally I'd have a note specifically mentioning that (upstream communication isn't the greatest; it looks like they test almost exclusively in docker where clashing ports aren't an issue), but this'll have to do for now.

[JohnRTitor]

Add an assert here, which assumes port != metricsPort

[TheRealGramdalf]

So would this work?

assertions = [
      {
        assertion = cfg.port != cfg.metricsPort;
        message = "The primary listen port cannot be the same as the listen port for the metrics endpoint";
      }
    ];

I verified that it does syntactically (i.e. it correctly identifies and throws an error when I set them to the same value), I'm just not sure if you meant to use assertions or assert.

[JohnRTitor]

Yes, just like this

nixpkgs/nixos/modules/services/scheduling/scx.nix

Lines 101 to 106 in 69fd9db

	assertions = [
	{
	assertion = lib.versionAtLeast config.boot.kernelPackages.kernel.version "6.12";
	message = "SCX is only supported on kernel version >= 6.12.";
	}
	];

I take it you have tested this with your configuration

[TheRealGramdalf]

Yes, I tested both the normal situation (two different ports) and triggering the assertion (metricsPort is the same as port). It behaved as one would expect in both cases.

[TheRealGramdalf]

Once again ready for merge. I did some manual checks and everything functions fine, not sure if we should wait for ofborg or just send it due to the branch window - I'm relatively new to PRs so I'm not too sure if it's required or just recommended.

[JohnRTitor]

Well we mainly wait for Ofborg so it detects any underlying issue during eval. But considering how slower and costlier it has become, there have been calls to replace it with GitHub actions, which should be faster.

New eval workflow PR has been merged yesterday, and it looks like the checks passed. Let's still wait for Ofborg to check other things, I am happy to merge it after that.

[TheRealGramdalf]

Allright, sounds good! I'm mainly just trying to get it in before the 24.11 release so the change is in the release notes. I've been advised over on matrix that this should ideally get backported, but again I'm new to PRs and I'm not entirely sure what the process is like - there were mentions of changes to the release notes requiring a manual backport, but again I'm new to that specific area.

[TheRealGramdalf]

Checks complete, ready for merge - let me know if there's anything to do for the backport PR.

@PatrickDaG one thing to note for your PR - cfg currently points to config.services.netbird.server.<component>, which you changed to omit the <component> . You'll need to edit the metricsPort option to reflect that again when you rebase.

[Scrumplex]

Changes LGTM

[github-actions]

Successfully created backport PR for release-24.11:

• [Backport release-24.11] nixos/netbird: fix port conflict on metrics endpoint #358231