fix(nixos): address Nix review findings #183

Closed
alexion wants to merge 1 commits from fix/nix-review-findings into main
13 changed files with 215 additions and 94 deletions
Showing only changes of commit 66efae302f - Show all commits

View File

@@ -12,43 +12,22 @@ in
interior = { interior = {
modules.services.reverse-proxy.enable = true; modules.services.reverse-proxy.enable = true;
services.nginx.virtualHosts = { services.nginx.virtualHosts =
"games.alexion.dev" = { lib.genAttrs
forceSSL = true; [
enableACME = true; "games.alexion.dev"
locations."/".proxyPass = "http://10.23.20.32"; "git.alexion.dev"
}; "photos.alexion.dev"
"git.alexion.dev" = { "plex.alexion.dev"
forceSSL = true; "servers.alexion.dev"
enableACME = true; "todo.alexion.dev"
locations."/".proxyPass = "http://10.23.20.32"; "wings.alexion.dev"
}; ]
"photos.alexion.dev" = { (_domain: {
forceSSL = true; forceSSL = true;
enableACME = true; enableACME = true;
locations."/".proxyPass = "http://10.23.20.32"; locations."/".proxyPass = "http://10.23.20.32";
}; });
"plex.alexion.dev" = {
forceSSL = true;
enableACME = true;
locations."/".proxyPass = "http://10.23.20.32";
};
"servers.alexion.dev" = {
forceSSL = true;
enableACME = true;
locations."/".proxyPass = "http://10.23.20.32";
};
"todo.alexion.dev" = {
forceSSL = true;
enableACME = true;
locations."/".proxyPass = "http://10.23.20.32";
};
"wings.alexion.dev" = {
forceSSL = true;
enableACME = true;
locations."/".proxyPass = "http://10.23.20.32";
};
};
networking.firewall.allowedTCPPorts = [ 2022 ]; networking.firewall.allowedTCPPorts = [ 2022 ];

View File

@@ -60,7 +60,6 @@
serverName = "Tracion"; serverName = "Tracion";
autoStart = false; autoStart = false;
adminPasswordSecret = "services/game-servers/admin/password"; adminPasswordSecret = "services/game-servers/admin/password";
publicIP = config.guests.palworld.address;
reverseProxy = { reverseProxy = {
enable = true; enable = true;
route = { route = {

View File

@@ -1,6 +1,5 @@
{ {
config, config,
lib,
... ...
}: }:
# zeus — Ryzen desktop with AMD graphics. # zeus — Ryzen desktop with AMD graphics.
@@ -8,9 +7,9 @@
# `fileSystems` are derived from it, none declared here. # `fileSystems` are derived from it, none declared here.
{ {
imports = [ imports = [
./hardware-configuration.nix
./disk.nix ./disk.nix
] ];
++ lib.optional (builtins.pathExists ./hardware-configuration.nix) ./hardware-configuration.nix;
nixpkgs.hostPlatform = "x86_64-linux"; nixpkgs.hostPlatform = "x86_64-linux";

View File

@@ -62,18 +62,6 @@
description = "Start the Palworld server after the guest reaches network readiness."; description = "Start the Palworld server after the guest reaches network readiness.";
}; };
rcon = {
enable = lib.mkEnableOption "Palworld RCON" // {
default = true;
};
port = lib.mkOption {
type = lib.types.port;
default = 25575;
description = "TCP port for Palworld RCON.";
};
};
passwordFile = lib.mkOption { passwordFile = lib.mkOption {
type = lib.types.path; type = lib.types.path;
description = "Runtime file containing the Palworld server password."; description = "Runtime file containing the Palworld server password.";
@@ -89,65 +77,120 @@
let let
service = config.modules.services.games.palworld; service = config.modules.services.games.palworld;
savedDir = "${service.installDir}/Pal/Saved"; savedDir = "${service.installDir}/Pal/Saved";
defaultSettingsFile = "${service.installDir}/DefaultPalWorldSettings.ini";
settingsFile = "${savedDir}/Config/LinuxServer/PalWorldSettings.ini";
renderSettingsScript = pkgs.writeText "render-palworld-settings.py" ''
import json
import os
from pathlib import Path
import re
import sys
import tempfile
default_path, settings_path, server_name, password_path, admin_password_path = sys.argv[1:]
default_path = Path(default_path)
settings_path = Path(settings_path)
source_path = settings_path if settings_path.exists() else default_path
text = source_path.read_text(encoding="utf-8")
def read_secret(path):
return Path(path).read_text(encoding="utf-8").rstrip("\r\n")
def set_string_setting(text, name, value):
assignment = f"{name}={json.dumps(value, ensure_ascii=False)}"
pattern = re.compile(rf'(?<![A-Za-z0-9_]){re.escape(name)}="(?:[^"\\]|\\.)*"')
text, replacements = pattern.subn(lambda _match: assignment, text, count=1)
if replacements:
return text
marker = "OptionSettings=("
if marker not in text:
raise ValueError(f"{source_path} has no OptionSettings declaration")
return text.replace(marker, f"{marker}{assignment},", 1)
settings = {
"ServerName": server_name,
"ServerPassword": read_secret(password_path),
"AdminPassword": read_secret(admin_password_path),
}
for name, value in settings.items():
text = set_string_setting(text, name, value)
settings_path.parent.mkdir(parents=True, exist_ok=True)
descriptor, temporary_name = tempfile.mkstemp(
prefix=f".{settings_path.name}.",
dir=settings_path.parent,
)
temporary_path = Path(temporary_name)
try:
os.fchmod(descriptor, 0o600)
with os.fdopen(descriptor, "w", encoding="utf-8", newline="\n") as settings_file:
settings_file.write(text)
os.replace(temporary_path, settings_path)
finally:
temporary_path.unlink(missing_ok=True)
'';
startScript = pkgs.writeShellScript "palworld-start" '' startScript = pkgs.writeShellScript "palworld-start" ''
set -eu set -eu
${pkgs.coreutils}/bin/install -d \ ${lib.getExe' pkgs.coreutils "install"} -d \
-m 0770 \ -m 0770 \
-- \
${lib.escapeShellArg service.stateDir} \ ${lib.escapeShellArg service.stateDir} \
${lib.escapeShellArg (builtins.dirOf savedDir)} ${lib.escapeShellArg (builtins.dirOf savedDir)}
if [ -L ${lib.escapeShellArg savedDir} ]; then if [ -L ${lib.escapeShellArg savedDir} ]; then
current_target="$(${pkgs.coreutils}/bin/readlink ${lib.escapeShellArg savedDir})" current_target="$(${lib.getExe' pkgs.coreutils "readlink"} -- ${lib.escapeShellArg savedDir})"
if [ "$current_target" != ${lib.escapeShellArg service.stateDir} ]; then if [ "$current_target" != ${lib.escapeShellArg service.stateDir} ]; then
${pkgs.coreutils}/bin/rm ${lib.escapeShellArg savedDir} ${lib.getExe' pkgs.coreutils "rm"} -- ${lib.escapeShellArg savedDir}
fi fi
fi fi
if [ -d ${lib.escapeShellArg savedDir} ] && [ ! -L ${lib.escapeShellArg savedDir} ]; then if [ -d ${lib.escapeShellArg savedDir} ] && [ ! -L ${lib.escapeShellArg savedDir} ]; then
if ${pkgs.findutils}/bin/find ${lib.escapeShellArg service.stateDir} -mindepth 1 -maxdepth 1 | ${pkgs.gnugrep}/bin/grep -q .; then if ${lib.getExe' pkgs.findutils "find"} -- ${lib.escapeShellArg service.stateDir} -mindepth 1 -maxdepth 1 | ${lib.getExe pkgs.gnugrep} -q .; then
${pkgs.coreutils}/bin/rm -rf ${lib.escapeShellArg savedDir} ${lib.getExe' pkgs.coreutils "rm"} -rf -- ${lib.escapeShellArg savedDir}
else else
${pkgs.coreutils}/bin/cp -a ${lib.escapeShellArg savedDir}/. ${lib.escapeShellArg service.stateDir}/ ${lib.getExe' pkgs.coreutils "cp"} -a -- ${lib.escapeShellArg savedDir}/. ${lib.escapeShellArg service.stateDir}/
${pkgs.coreutils}/bin/rm -rf ${lib.escapeShellArg savedDir} ${lib.getExe' pkgs.coreutils "rm"} -rf -- ${lib.escapeShellArg savedDir}
fi fi
fi fi
if [ ! -e ${lib.escapeShellArg savedDir} ]; then if [ ! -e ${lib.escapeShellArg savedDir} ]; then
${pkgs.coreutils}/bin/ln -s ${lib.escapeShellArg service.stateDir} ${lib.escapeShellArg savedDir} ${lib.getExe' pkgs.coreutils "ln"} -s -- ${lib.escapeShellArg service.stateDir} ${lib.escapeShellArg savedDir}
fi fi
password="$(${pkgs.coreutils}/bin/tr -d '\n' < ${lib.escapeShellArg service.passwordFile})" ${lib.getExe pkgs.python3} \
admin_password="$(${pkgs.coreutils}/bin/tr -d '\n' < ${lib.escapeShellArg service.adminPasswordFile})" ${renderSettingsScript} \
${lib.escapeShellArg defaultSettingsFile} \
${lib.escapeShellArg settingsFile} \
${lib.escapeShellArg service.serverName} \
${lib.escapeShellArg service.passwordFile} \
${lib.escapeShellArg service.adminPasswordFile}
exec ./PalServer.sh \ exec ./PalServer.sh "$@"
"$@" \
"-serverpassword=$password" \
"-adminpassword=$admin_password"
''; '';
in in
{ {
modules.services.steamcmd = { modules.services.steamcmd = {
enable = true; enable = true;
inherit (service) steamDir installDir stateDir; inherit (service) steamDir installDir stateDir;
serviceName = "palworld-${lib.toLower service.serverName}"; serviceName = "palworld";
appId = 2394010; appId = 2394010;
inherit (service) autoStart; inherit (service) autoStart;
executable = "${startScript}"; executable = "${startScript}";
arguments = [ arguments = [
"-useperfthreads"
"-NoAsyncLoadingThread"
"-UseMultithreadForDS"
"-port=${toString service.port}" "-port=${toString service.port}"
"-publicport=${toString service.port}"
"-servername=${service.serverName}"
"-players=${toString service.maxPlayers}" "-players=${toString service.maxPlayers}"
] ]
++ lib.optional service.publicLobby "-publiclobby" ++ lib.optionals service.publicLobby (
++ lib.optionals (service.publicIP != null) [ "-publicip=${service.publicIP}" ] [
++ lib.optional service.rcon.enable "-rcon"; "-publiclobby"
"-publicport=${toString service.port}"
]
++ lib.optionals (service.publicIP != null) [ "-publicip=${service.publicIP}" ]
);
udpPorts = [ service.port ]; udpPorts = [ service.port ];
tcpPorts = [ service.port ];
}; };
} }
); );

View File

@@ -51,7 +51,7 @@
startScript = pkgs.writeShellScript "valheim-start" '' startScript = pkgs.writeShellScript "valheim-start" ''
set -eu set -eu
password="$(${pkgs.coreutils}/bin/tr -d '\n' < ${lib.escapeShellArg service.passwordFile})" password="$(${lib.getExe' pkgs.coreutils "tr"} -d '\n' < ${lib.escapeShellArg service.passwordFile})"
exec ./valheim_server.x86_64 "$@" -password "$password" exec ./valheim_server.x86_64 "$@" -password "$password"
''; '';
in in

View File

@@ -15,7 +15,7 @@
}; };
serviceName = lib.mkOption { serviceName = lib.mkOption {
type = lib.types.str; type = lib.types.strMatching "[A-Za-z0-9_.@-]+";
example = "valheim"; example = "valheim";
description = "Name of the systemd service that runs the game server."; description = "Name of the systemd service that runs the game server.";
}; };
@@ -87,10 +87,11 @@
prepareScript = pkgs.writeShellScript "steamcmd-${service.serviceName}-prepare" '' prepareScript = pkgs.writeShellScript "steamcmd-${service.serviceName}-prepare" ''
set -eu set -eu
${pkgs.coreutils}/bin/install -d \ ${lib.getExe' pkgs.coreutils "install"} -d \
-o ${user} \ -o ${user} \
-g ${group} \ -g ${group} \
-m 0770 \ -m 0770 \
-- \
${lib.escapeShellArg service.steamDir} \ ${lib.escapeShellArg service.steamDir} \
${lib.escapeShellArg service.installDir} \ ${lib.escapeShellArg service.installDir} \
${lib.escapeShellArg service.stateDir} ${lib.escapeShellArg service.stateDir}
@@ -110,7 +111,7 @@
startScript = pkgs.writeShellScript "steamcmd-${service.serviceName}-start" '' startScript = pkgs.writeShellScript "steamcmd-${service.serviceName}-start" ''
set -eu set -eu
cd ${lib.escapeShellArg service.installDir} cd -- ${lib.escapeShellArg service.installDir}
export LD_LIBRARY_PATH=linux64:''${LD_LIBRARY_PATH-} export LD_LIBRARY_PATH=linux64:''${LD_LIBRARY_PATH-}
exec ${lib.getExe pkgs.steam-run} ${lib.escapeShellArg service.executable} ${lib.escapeShellArgs service.arguments} exec ${lib.getExe pkgs.steam-run} ${lib.escapeShellArg service.executable} ${lib.escapeShellArgs service.arguments}
''; '';
@@ -155,7 +156,7 @@
wantedBy = lib.mkIf service.autoStart [ "multi-user.target" ]; wantedBy = lib.mkIf service.autoStart [ "multi-user.target" ];
serviceConfig = { serviceConfig = {
Type = "oneshot"; Type = "oneshot";
ExecStart = "${pkgs.systemd}/bin/systemctl --no-block start ${service.serviceName}.service"; ExecStart = "${lib.getExe' pkgs.systemd "systemctl"} --no-block -- start ${service.serviceName}.service";
}; };
}; };

View File

@@ -86,7 +86,7 @@ in
backup = { backup = {
description = "Copy <file> to <file>.bak"; description = "Copy <file> to <file>.bak";
argumentNames = "filename"; argumentNames = "filename";
body = "cp $filename $filename.bak"; body = "cp -- $filename $filename.bak";
}; };
copy = { copy = {

View File

@@ -22,21 +22,21 @@ let
root = root + "/registry"; root = root + "/registry";
}; };
# TODO: Refactor guest-module composition.
guestModules = import ./guests.nix { guestModules = import ./guests.nix {
inherit inherit
inputs inputs
registry registry
repositoryModules
specialArgs specialArgs
util util
; ;
repositoryModules = modules;
root = root + "/guests"; root = root + "/guests";
}; };
modules.imports = [ repositoryModules.imports = [
{ {
nixpkgs.overlays = [ self.overlays.custom ]; nixpkgs.overlays = [ self.overlays.custom ];
} }
registry.module
] ]
++ util.collectNixFiles (root + "/modules"); ++ util.collectNixFiles (root + "/modules");
@@ -47,8 +47,7 @@ import ./hosts.nix {
lib lib
specialArgs specialArgs
guestModules guestModules
repositoryModules
; ;
registryModule = registry.module;
repositoryModules = modules;
root = root + "/hosts"; root = root + "/hosts";
} }

View File

@@ -10,7 +10,12 @@ let
files = util.collectNixFiles root; files = util.collectNixFiles root;
mkGuest = mkGuest =
name: name:
{ config, lib, ... }: {
config,
lib,
pkgs,
...
}:
let let
cfg = config.guests.${name}; cfg = config.guests.${name};
@@ -18,10 +23,11 @@ let
directoryScript = lib.concatStringsSep "\n" ( directoryScript = lib.concatStringsSep "\n" (
lib.mapAttrsToList (_: mount: '' lib.mapAttrsToList (_: mount: ''
${config.system.path}/bin/install -d \ ${lib.getExe' pkgs.coreutils "install"} -d \
${lib.optionalString (mount.owner != null) "-o ${lib.escapeShellArg (toString mount.owner)}"} \ ${lib.optionalString (mount.owner != null) "-o ${lib.escapeShellArg (toString mount.owner)}"} \
${lib.optionalString (mount.group != null) "-g ${lib.escapeShellArg mount.group}"} \ ${lib.optionalString (mount.group != null) "-g ${lib.escapeShellArg mount.group}"} \
${lib.optionalString (mount.mode != null) "-m ${lib.escapeShellArg mount.mode}"} \ ${lib.optionalString (mount.mode != null) "-m ${lib.escapeShellArg mount.mode}"} \
-- \
${lib.escapeShellArg mount.directory} ${lib.escapeShellArg mount.directory}
'') directoryMounts '') directoryMounts
); );
@@ -254,7 +260,6 @@ let
inputs.stylix.nixosModules.stylix inputs.stylix.nixosModules.stylix
repositoryModules repositoryModules
registry.module
cfg.interior cfg.interior
{ {

View File

@@ -6,7 +6,6 @@
repositoryModules, repositoryModules,
guestModules, guestModules,
registryModule,
}: }:
let let
names = lib.attrNames directories; names = lib.attrNames directories;
@@ -27,8 +26,6 @@ let
repositoryModules repositoryModules
guestModules guestModules
# TODO: refactor
registryModule
(root + "/${name}") (root + "/${name}")
{ {

View File

@@ -10,6 +10,7 @@ in
{ {
fix = { fix = {
type = "app"; type = "app";
meta.description = "Formats and cleans repository files.";
program = lib.getExe ( program = lib.getExe (
pkgs.writeShellApplication { pkgs.writeShellApplication {
name = "fix"; name = "fix";

View File

@@ -11,3 +11,13 @@ Use `flake.nix` to pass inputs into that implementation.
### Rationale ### Rationale
This keeps dependency declarations separate from output logic. This keeps dependency declarations separate from output logic.
## Describe flake apps
### Rule
Set `meta.description` on every flake app.
### Rationale
Flake tools can display each app's purpose without emitting a missing-metadata warning.

View File

@@ -50,3 +50,91 @@ Do not read `config.home-manager.users.<user>.programs.<name>.package` for comma
Home Manager uses the NixOS package set in this repository. Home Manager uses the NixOS package set in this repository.
`pkgs.<name>` avoids dependency paths through the user tree under Home Manager. `pkgs.<name>` avoids dependency paths through the user tree under Home Manager.
Configured NixOS package options preserve package selection when a module owns the tool. Configured NixOS package options preserve package selection when a module owns the tool.
## Configure services through documented interfaces
### Rule
Use current upstream documentation when mapping Nix options to service arguments or configuration files.
Do not expose undocumented service arguments as module options.
### Rationale
Documented interfaces preserve configuration behavior across service updates.
## Keep module options effective
### Rule
Every custom module option must change the generated configuration or behavior.
Remove unused options instead of exposing speculative configuration.
### Rationale
An inert option promises behavior that the module does not provide.
## Separate public and private addresses
### Rule
Do not assign a private guest address to an option that advertises a public service address.
Leave optional public-address settings unset when the service can detect them.
### Rationale
Private addresses are not routable by public clients.
## Separate display names from identifiers
### Rule
Do not derive systemd unit names or other machine identifiers from unrestricted display names.
Give identifiers constrained types or fixed values.
### Rationale
Display names can contain characters that machine interfaces reject or reinterpret.
## Terminate command options before variable paths
### Rule
In generated shell text, pass `--` before variable-derived path operands when the command supports it.
### Rationale
Values that start with a hyphen remain operands instead of command options.
## Compose repository modules once
### Rule
Add shared NixOS modules to `repositoryModules` in `src/dotfiles/default.nix`.
Import `repositoryModules` into each host and guest.
Do not pass a shared module through a separate parallel parameter.
### Rationale
A single module collection prevents host and guest configurations from diverging.
## Generate identical named attributes
### Rule
Use `lib.genAttrs` when a fixed list of attribute names shares one value.
Do not repeat identical attribute bodies.
### Rationale
One definition prevents copied settings from diverging.
## Import tracked host hardware directly
### Rule
Import each tracked `hardware-configuration.nix` directly from its host module.
Do not guard the import with `builtins.pathExists`.
### Rationale
A missing hardware description must fail evaluation instead of silently changing the host configuration.