headscale: make module conform with rfc42 and upstream (0.17.1) config. #203938

kradalby · 2022-12-01T15:30:45Z

Description of changes

This PR changes the headscale module to conform with RFC42 as it had already drifted from upstream configuration and updates headscale to version 0.17.1.

There are currently multiple commits, and might be some more after comments, I'll squash it up before merge.

This PR contains the changes from #203694.
This PR supersede and closes #167513.

Things done

Formatted the code with nixpkgs-fmt
Moved all settings to settings key, mirroring the naming from upstream
Marked all renamed module settings

Signed-off-by: Kristoffer Dalby [email protected]

kradalby · 2022-12-01T15:51:07Z

Doc build failure seem to be related to the redmine module and not headscale.

06kellyjac

generally looks good. Added some comments

06kellyjac · 2022-12-01T15:51:19Z

nixos/modules/services/networking/headscale.nix

    };
  };

-  meta.maintainers = with maintainers; [ kradalby ];
+  meta.maintainers = with maintainers; [kradalby];


spaces should remain. Same in other places

nixpkgs-fmt

06kellyjac · 2022-12-01T15:53:51Z

nixos/doc/manual/release-notes/rl-2305.section.md

@@ -38,3 +38,9 @@ In addition to numerous new and upgraded packages, this release has the followin
 - The module for the application firewall `opensnitch` got the ability to configure rules. Available as [services.opensnitch.rules](#opt-services.opensnitch.rules)

 - A new `virtualisation.rosetta` module was added to allow running `x86_64` binaries through [Rosetta](https://developer.apple.com/documentation/apple-silicon/about-the-rosetta-translation-environment) inside virtualised NixOS guests on Apple silicon. This feature works by default with the [UTM](https://docs.getutm.app/) virtualisation [package](https://search.nixos.org/packages?channel=unstable&show=utm&from=0&size=1&sort=relevance&type=packages&query=utm).
+
+- The module `services.headscale` was refactored to be compliant with [RFC 0042](https://github.com/NixOS/rfcs/blob/master/rfcs/0042-config-option.md). To be precise, this means that the following things have changed:
+  - Most settings has been migrated under [](#opt-services.grafana.settings) which is an attribute-set that


[] probably needs to be filled & the link updated to not be grafana

This list is pretty short so it's bound to happen right now but the advice is to put the item in at a random point in this list to avoid merge conflicts

Misterio77

Overall, looks great!

Just pulled it and tested on my server, here's a few small errors I found:

Misterio77 · 2022-12-01T16:22:54Z

nixos/modules/services/networking/headscale.nix

+        ${optionalString (cfg.setting.db_password_file != null) ''
+          export HEADSCALE_DB_PASS="$(head -n1 ${escapeShellArg cfg.setting.db_password_file})"


Suggested change

${optionalString (cfg.setting.db_password_file != null) ''

export HEADSCALE_DB_PASS="$(head -n1 ${escapeShellArg cfg.setting.db_password_file})"

${optionalString (cfg.settings.db_password_file != null) ''

export HEADSCALE_DB_PASS="$(head -n1 ${escapeShellArg cfg.settings.db_password_file})"

Misterio77 · 2022-12-01T16:23:39Z

nixos/modules/services/networking/headscale.nix

+              description = lib.mdDoc "Database user.";
+            };
+
+            db_passwordFile = mkOption {


Suggested change

db_passwordFile = mkOption {

db_password_file = mkOption {

RaitoBezarius · 2022-12-01T16:35:01Z

Blocked on #203952.

kradalby · 2022-12-01T16:50:54Z

@Misterio77 How do you feel about me cherry picking your two relevant commits (f69b63e and f76bebb) onto this one?

Misterio77 · 2022-12-01T17:05:41Z

Feel free! :)

Misterio77 · 2022-12-01T17:10:45Z

Also, I'd love to help maintain the package and/or module. Feel free to add me as a maintainer if you folks could use some help.

nixos/modules/services/networking/headscale.nix

kradalby · 2022-12-01T21:03:15Z

I picked @Misterio77's commits from #203694 onto this PR so we have a consistent change with since the new module and the new version depend on each other.

I also added @Misterio77 as a headscale maintainer.

I believe this is good for review now, and I can squash it all up when someone has approved it.

Misterio77

LGTM!

nixos/modules/services/networking/headscale.nix

Misterio77 · 2022-12-03T00:26:52Z

Hey!

I was wondering if we should include all options like that. We're using freeform options following RFC42 but, according to it, we should probably only explicitly include options that are either important, required for the program to run, or are secrets (so that the user can use a file instead).

ghuntley · 2022-12-03T07:10:03Z

explicitly include options that are either important

override_local_dns is a key feature so my vote is to include?
we have log.level ... undecided if to include log.format

A downside of RFC42 is it reduces discovery of options. I often head to https://search.nixos.org -> options and search for config options. Having items listed there highlights that the option is configurable.

kradalby · 2022-12-05T07:31:15Z

@ghuntley Thanks!, I will get in the changes a bit later today.

On backport: I am happy to have it backported, "open question", this will break people who has already upgraded and use headscale, what is NixOS's policy for that?

On adding all options: I am not against adding all options per sei, as I appreciate search.nixos.org a lot (Really I would love Nix to be the way to configure headscale and the defacto standard). I like the way its documented and makes it discoverable. But I am worried about drift if I forget, or an option goes unnoticed by other maintainers.

In any case, I do not have a strong opinion, and I'm happy to try to keep it up to date, and with the new format, we allow passthrough of options so it should at least not block updates.

@Misterio77 @ghuntley @06kellyjac

pkgs/servers/headscale/default.nix

kradalby · 2022-12-07T13:11:08Z

I've updated this to 0.17.1 which has been released and added a missing build flag. PTAL

viq · 2022-12-20T10:46:10Z

Does anything else need to happen for this to be merged?

viq · 2022-12-23T11:44:54Z

@kradalby @Misterio77 @ghuntley @NKJe @06kellyjac Does anything else need to happen for this to be merged?

kradalby · 2022-12-23T14:16:54Z

@viq should not be, or I seem to have to resolve some new conflicts, but other than that its been ready a few weeks.

K900 · 2022-12-23T14:19:51Z

I'm interested in getting this in, so ping me when you resolve conflicts and I can merge.

This commit upgrades headscale to the newest version, 0.17.0 and updates the module with the current breaking config changes. In addition, the module is rewritten to conform with RFC0042 to try to prevent some drift between the module and the upstream. A new maintainer, Misterio77, is added as maintainer. Signed-off-by: Kristoffer Dalby <[email protected]> Co-authored-by: Gabriel Fontes <[email protected]> Co-authored-by: Geoffrey Huntley <[email protected]>

kradalby · 2022-12-23T14:48:40Z

@K900 @viq Resolved.

github-actions · 2022-12-23T15:14:48Z

Backport failed for release-22.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-22.11
git worktree add -d .worktree/backport-203938-to-release-22.11 origin/release-22.11
cd .worktree/backport-203938-to-release-22.11
git checkout -b backport-203938-to-release-22.11
ancref=$(git merge-base ef77bcb369d0f5c7840992277b8621abe803f042 571780384a4327b5af15c411b23e318be5cfc9f0)
git cherry-pick -x $ancref..571780384a4327b5af15c411b23e318be5cfc9f0

K900 · 2023-01-10T12:59:58Z

nixos/modules/services/networking/headscale.nix

+                '';
+              };
+
+              override_local_dns = mkOption {


Any particular reason we're setting this to false when upstream defaults to true? This seems to break MagicDNS by default (which may or may not be an upstream bug).

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 Dec 1, 2022

kradalby mentioned this pull request Dec 1, 2022

headscale: 0.16.4 -> 0.17.0 #203694

Closed

13 tasks

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 1, 2022

github-actions bot added 8.has: changelog 8.has: documentation labels Dec 1, 2022

06kellyjac reviewed Dec 1, 2022

View reviewed changes

kradalby requested a review from Misterio77 December 1, 2022 15:57

kradalby force-pushed the headscale-rfc0042 branch from 7715148 to def65e4 Compare December 1, 2022 15:59

Misterio77 suggested changes Dec 1, 2022

View reviewed changes

kradalby force-pushed the headscale-rfc0042 branch 2 times, most recently from e256801 to 9bba67b Compare December 1, 2022 16:38

kradalby requested a review from 06kellyjac December 1, 2022 16:48

kradalby marked this pull request as ready for review December 1, 2022 16:48

kradalby requested a review from Misterio77 December 1, 2022 16:48

Misterio77 suggested changes Dec 1, 2022

View reviewed changes

nixos/modules/services/networking/headscale.nix Outdated Show resolved Hide resolved

nixos/modules/services/networking/headscale.nix Outdated Show resolved Hide resolved

ofborg bot requested a review from NKJe December 1, 2022 21:13

ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Dec 1, 2022

ofborg bot requested a review from Misterio77 December 1, 2022 21:14

Misterio77 approved these changes Dec 1, 2022

View reviewed changes

ghuntley reviewed Dec 2, 2022

View reviewed changes

nixos/modules/services/networking/headscale.nix Show resolved Hide resolved

ghuntley reviewed Dec 2, 2022

View reviewed changes

nixos/modules/services/networking/headscale.nix Show resolved Hide resolved

mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 3, 2022

kradalby force-pushed the headscale-rfc0042 branch 2 times, most recently from 918db65 to 2586008 Compare December 5, 2022 10:06

kradalby requested a review from ghuntley December 5, 2022 10:06

kradalby added the backport release-22.11 label Dec 5, 2022

ofborg bot requested a review from Misterio77 December 5, 2022 10:16

kradalby commented Dec 5, 2022

View reviewed changes

pkgs/servers/headscale/default.nix Outdated Show resolved Hide resolved

kradalby force-pushed the headscale-rfc0042 branch from 00ddd96 to 34176a5 Compare December 7, 2022 13:10

kradalby changed the title ~~headscale: make module conform with rfc42 and upstream (0.17.0) config.~~ headscale: make module conform with rfc42 and upstream (0.17.1) config. Dec 7, 2022

kradalby force-pushed the headscale-rfc0042 branch from 34176a5 to 80ab363 Compare December 9, 2022 12:16

Misterio77 approved these changes Dec 9, 2022

View reviewed changes

kradalby force-pushed the headscale-rfc0042 branch from 80ab363 to 5717803 Compare December 23, 2022 14:48

ofborg bot requested a review from Misterio77 December 23, 2022 14:59

K900 merged commit e0a2c59 into NixOS:master Dec 23, 2022

LogicalOverflow mentioned this pull request Dec 28, 2022

headscale: 0.16.4 -> 0.17.0 #203446

Closed

2 tasks

K900 reviewed Jan 10, 2023

View reviewed changes

dali99 mentioned this pull request Feb 9, 2023

[22.11] nixos/headscale: Update openidconnect options to be in sync with package #215438

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

headscale: make module conform with rfc42 and upstream (0.17.1) config. #203938

headscale: make module conform with rfc42 and upstream (0.17.1) config. #203938

kradalby commented Dec 1, 2022 •

edited

Loading

kradalby commented Dec 1, 2022 •

edited

Loading

06kellyjac left a comment

06kellyjac Dec 1, 2022

kradalby Dec 1, 2022

06kellyjac Dec 1, 2022

06kellyjac Dec 1, 2022

kradalby Dec 1, 2022

Misterio77 left a comment

Misterio77 Dec 1, 2022

kradalby Dec 1, 2022

Misterio77 Dec 1, 2022

kradalby Dec 1, 2022

RaitoBezarius commented Dec 1, 2022

kradalby commented Dec 1, 2022

Misterio77 commented Dec 1, 2022

Misterio77 commented Dec 1, 2022

kradalby commented Dec 1, 2022

Misterio77 left a comment

Misterio77 commented Dec 3, 2022 •

edited

Loading

ghuntley commented Dec 3, 2022

kradalby commented Dec 5, 2022

kradalby commented Dec 7, 2022

viq commented Dec 20, 2022

viq commented Dec 23, 2022

kradalby commented Dec 23, 2022

K900 commented Dec 23, 2022

kradalby commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

K900 Jan 10, 2023

		${optionalString (cfg.setting.db_password_file != null) ''
		export HEADSCALE_DB_PASS="$(head -n1 ${escapeShellArg cfg.setting.db_password_file})"

headscale: make module conform with rfc42 and upstream (0.17.1) config. #203938

headscale: make module conform with rfc42 and upstream (0.17.1) config. #203938

Conversation

kradalby commented Dec 1, 2022 • edited Loading

Description of changes

Things done

kradalby commented Dec 1, 2022 • edited Loading

06kellyjac 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

Misterio77 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

RaitoBezarius commented Dec 1, 2022

kradalby commented Dec 1, 2022

Misterio77 commented Dec 1, 2022

Misterio77 commented Dec 1, 2022

kradalby commented Dec 1, 2022

Misterio77 left a comment

Choose a reason for hiding this comment

Misterio77 commented Dec 3, 2022 • edited Loading

ghuntley commented Dec 3, 2022

kradalby commented Dec 5, 2022

kradalby commented Dec 7, 2022

viq commented Dec 20, 2022

viq commented Dec 23, 2022

kradalby commented Dec 23, 2022

K900 commented Dec 23, 2022

kradalby commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

Choose a reason for hiding this comment

kradalby commented Dec 1, 2022 •

edited

Loading

kradalby commented Dec 1, 2022 •

edited

Loading

Misterio77 commented Dec 3, 2022 •

edited

Loading