Skip to content

easytier.services.default: init #476861

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.

Sign up for GitHub

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

Jump to bottom
Open
Moraxyc wants to merge 3 commits into
base: master
Choose a base branch
from
Open

easytier.services.default: init #476861

Moraxyc wants to merge 3 commits into from
+379 −5

Conversation

@Moraxyc
Copy link
Member

@Moraxyc Moraxyc commented Jan 4, 2026 •
edited
Loading

  • easytier: switch to finalAttrs
  • easytier.services.default: init
  • nixosTests.easytier-modular: init

An attempt to create a modular service. Please let me know if there’s anything that needs fixing.

It seems there is no way to implement sysctl at the moment. Should we use preStart instead?

nixpkgs/nixos/modules/services/networking/easytier.nix

Lines 283 to 286 in 8f8e97e

boot.kernel.sysctl = mkIf cfg.allowSystemForward {
"net.ipv4.conf.all.forwarding" = mkOverride 97 true;
"net.ipv6.conf.all.forwarding" = mkOverride 97 true;
};

Tracking: #428084

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Jan 4, 2026
@Moraxyc
Copy link
Member Author

Moraxyc commented Jan 4, 2026

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 476861 -p nixosTests.easytier-modular
Commit: 3be0efae74ba10a6b9a8928476cd6159a91e55b7 (subsequent changes)
Merge: 5e46dcf0f629ec5904ef680f1b7e0e29be0f7139

Logs: https://github.com/Moraxyc/nixpkgs-review-gha/actions/runs/20694169445

x86_64-linux

✅ 1 test built:
  • nixosTests.easytier-modular

@Moraxyc Moraxyc marked this pull request as ready for review January 4, 2026 14:17
@nixpkgs-ci nixpkgs-ci bot requested a review from L-Trump January 4, 2026 14:28
@L-Trump
Copy link
Contributor

L-Trump commented Jan 5, 2026 •
edited
Loading

I don't think it's a good idea to directly use sysctl in preStart, since it disrupts the existing module system. Perhaps we just let users who need kernel forwarding manually configure the sysctl options, as is done in other distributions.

  • Has a NixOS VM test
  • Has a meta.maintainers attribute
  • Systemd-specific definitions are behind optionalAttrs (options ? systemd) to promote portability.
  • _class = "service"
  • Modular services provided through passthru.services must override the default of the package option using finalAttrs.finalPackage
  • Is the modular services infrastructure sufficient for this service? If one or more features are not covered, comment in Modular services tracking #428084

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jan 5, 2026
@Moraxyc Moraxyc force-pushed the add-modular-easytier branch from 3be0efa to a557a2f Compare January 5, 2026 05:04
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 5, 2026
SuperSandro2000
SuperSandro2000 reviewed Jan 7, 2026
View reviewed changes
pkgs/by-name/ea/easytier/service.nix

options = {
easytier = {
package = lib.mkOption {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 7, 2026

Choose a reason for hiding this comment

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

Please use lib.mkPackageOption.

Copy link
Member Author

@Moraxyc Moraxyc Jan 7, 2026

Choose a reason for hiding this comment

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

We should not use mkPackageOption here because package is defined in the place where importApply it.

See package.nix

passthru.services.default = {
    imports = [
      (lib.modules.importApply ./service.nix { inherit formats bash iproute2; })
    ];
    easytier.package = finalAttrs.finalPackage;
  };

pkgs/by-name/ea/easytier/service.nix
meta.maintainers = with lib.maintainers; [ moraxyc ];

options = {
easytier = {
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 7, 2026

Choose a reason for hiding this comment

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

An enable option is missing

Copy link
Member Author

@Moraxyc Moraxyc Jan 7, 2026

Choose a reason for hiding this comment

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

Since a modular service is designed to be importable, an additional enable option is no need.

pkgs/by-name/ea/easytier/service.nix
Comment on lines +72 to +85
network_secret = lib.mkOption {
type = with lib.types; nullOr str;
default = null;
description = ''
EasyTier network credential used for verification and encryption.
It is highly recommended to use {option}`easytier.environmentFiles` to
avoid leaking the secret into the world-readable Nix store.
'';
};
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 7, 2026

Choose a reason for hiding this comment

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

Just an idea: should we then offer the option at all if it is insecure?

Copy link
Member Author

@Moraxyc Moraxyc Jan 7, 2026

Choose a reason for hiding this comment

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

I prefer keeping this to provide a clear doc for users to not use it.

pkgs/by-name/ea/easytier/service.nix
Comment on lines +159 to +168
path = [
cfg.package
iproute2
bash
];
Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 7, 2026

Choose a reason for hiding this comment

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

Should we not patch that in the package instead?

Copy link
Member Author

@Moraxyc Moraxyc Jan 7, 2026

Choose a reason for hiding this comment

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

Not sure, I'm not familiar with packaging easytier. @L-Trump Could you take a look at this?

@Moraxyc Moraxyc force-pushed the add-modular-easytier branch from a557a2f to b938c69 Compare January 7, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

+1 more reviewer

@L-Trump L-Trump L-Trump approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Moraxyc @L-Trump @SuperSandro2000