-
-
Notifications
You must be signed in to change notification settings - Fork 364
Add nix flake #1466
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
Add nix flake #1466
Conversation
📝 WalkthroughWalkthroughAdds a flake-based NixOS module at Changes
Sequence Diagram(s)sequenceDiagram
participant Flake as "NixOS flake (module)"
participant System as "System (users, tmpfiles)"
participant OCI as "OCI runtime"
participant Registry as "Image registry (ghcr.io)"
Flake->>System: Emit user and group definitions
Flake->>System: Emit tmpfiles rules for /var/lib/netalertx
Flake->>OCI: Declare OCI container (image, mounts, env, capabilities, autoStart)
OCI->>Registry: Pull image ghcr.io/jokob-sk/netalertx:${imageTag}
OCI->>System: Mount volumes (/var/lib/netalertx -> container)
OCI->>OCI: Start container with env (PUID, PGID, LISTEN_ADDR, PORT, GRAPHQL_PORT, APP_CONF_OVERRIDE, ALWAYS_FRESH_INSTALL, NETALERTX_DEBUG)
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@install/nix/flake.nix`:
- Around line 37-41: The default for the mkOption named backendApiUrl lacks an
HTTP scheme; update its default from "localhost:${toString cfg.graphqlPort}" to
include the protocol (e.g. "http://localhost:${toString cfg.graphqlPort}") so
anywhere backendApiUrl is consumed it produces a valid URL; locate the
backendApiUrl option in the flake (the mkOption block referencing
cfg.graphqlPort) and change the default string to include the appropriate scheme
(http:// or https://) as required by the environment.
- Around line 22-30: The container runtime is not using the configured host
user/group so add the user flag to the container extra options: update the
extraOptions list (where the container is configured) to include --user
"${cfg.user}:${cfg.group}" so the container runs as cfg.user:cfg.group; also
confirm the image supports running as that UID/GID (or map via numeric UID/GID)
and ensure tmpfiles/user/group definitions (user/group mkOption and the system
user creation) remain consistent with the container setting.
🧹 Nitpick comments (5)
install/nix/flake.nix (5)
6-6: Consider avoidingwith lib;for better code clarity.Using
with lib;is generally discouraged in modern Nix style as it can cause variable shadowing and makes it harder to trace where functions originate. Consider using explicitlib.prefixes (e.g.,lib.mkEnableOption,lib.mkOption,lib.types,lib.mkIf).
32-36: DefaultimageTag = "latest"reduces reproducibility.Using
"latest"as the default image tag is not reproducible and can lead to unexpected behavior when the upstream image changes. Consider defaulting to a specific version tag that matches a known-good release.Suggested improvement
imageTag = mkOption { type = types.str; - default = "latest"; + default = "26.1.17"; # Pin to a specific version description = "Image tag to run"; };
44-51: Hardcoded UID/GID may conflict with existing system users.The hardcoded
uid = 20211andgid = 20211could conflict with existing users on some systems. Consider either:
- Making these configurable via options
- Removing the hardcoded values to allow NixOS to assign them dynamically
- Adding a check to only create the user/group when using the default user name
Option 1: Make UID/GID configurable
Add options for uid/gid:
uid = mkOption { type = types.nullOr types.int; default = null; description = "UID for the netalertx user (null for dynamic allocation)"; }; gid = mkOption { type = types.nullOr types.int; default = null; description = "GID for the netalertx group (null for dynamic allocation)"; };Then use conditionally:
users.users."${cfg.user}" = { isSystemUser = true; group = cfg.group; - uid = 20211; -}; +} // lib.optionalAttrs (cfg.uid != null) { uid = cfg.uid; };
52-56: Consider more restrictive permissions for sensitive directories.The
dbdirectory stores application data and might contain sensitive information. The current0755permission allows all users to read directory contents. Consider using more restrictive permissions like0750or0700.Suggested improvement
systemd.tmpfiles.rules = [ "d /var/lib/netalertx 0755 ${cfg.user} ${cfg.group} -" - "d /var/lib/netalertx/db 0755 ${cfg.user} ${cfg.group} -" + "d /var/lib/netalertx/db 0750 ${cfg.user} ${cfg.group} -" "d /var/lib/netalertx/config 0755 ${cfg.user} ${cfg.group} -" ];
78-85:LISTEN_ADDRis hardcoded; consider making it configurable.The
LISTEN_ADDR = "0.0.0.0"combined with--network=hostexposes the service on all network interfaces. Consider adding an option to allow users to restrict the listen address (e.g.,127.0.0.1for local-only access).Suggested improvement
Add a new option:
listenAddr = mkOption { type = types.str; default = "0.0.0.0"; description = "Address to listen on for incoming connections"; };Then use it in the environment:
environment = { - LISTEN_ADDR = "0.0.0.0"; + LISTEN_ADDR = cfg.listenAddr; PORT = "${toString cfg.port}";
install/nix/flake.nix
Outdated
| user = mkOption { | ||
| type = types.str; | ||
| default = "netalertx"; | ||
| description = "User to run the app"; | ||
| }; | ||
| group = mkOption { | ||
| type = types.str; | ||
| default = "netalertx"; | ||
| description = "Group to run the app"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the structure of the nix directory
fd . install/nix --max-depth 2 -type fRepository: netalertx/NetAlertX
Length of output: 233
🏁 Script executed:
# Now let's read the flake.nix file to understand the full context
cat -n install/nix/flake.nix | head -100Repository: netalertx/NetAlertX
Length of output: 3782
🏁 Script executed:
# Search for how user and group options are used throughout the config
rg -i "user|group" install/nix/flake.nix -A 2 -B 2Repository: netalertx/NetAlertX
Length of output: 1063
The user and group options are used for host configuration but not for the container runtime.
The user and group options define a system user and group (lines 44–51) and configure tmpfiles ownership (lines 52–56), but the OCI container lacks a --user flag in extraOptions and will run with the image's default user (typically root). If the intent is to run the container as the specified user, add --user "${cfg.user}:${cfg.group}" to extraOptions and verify the container image supports this UID/GID (20211).
Also applies to: 44-51
🤖 Prompt for AI Agents
In `@install/nix/flake.nix` around lines 22 - 30, The container runtime is not
using the configured host user/group so add the user flag to the container extra
options: update the extraOptions list (where the container is configured) to
include --user "${cfg.user}:${cfg.group}" so the container runs as
cfg.user:cfg.group; also confirm the image supports running as that UID/GID (or
map via numeric UID/GID) and ensure tmpfiles/user/group definitions (user/group
mkOption and the system user creation) remain consistent with the container
setting.
|
PR looks great! Will be a great addition, but it does need that UID thing fixed. If you try to use a username it will fail. It needs a PUID/PGID. The bootup sequence is as follows:
If you start as an arbitrary user, you won't be able to change the permissions. This means if you try 20211 at first then some other value, it will never switch because root can't reprovision the /data permissions. Also since it's RO, the username will not be used in the /etc/users and /etc/group as one might expect with a RW container. I made 2 suggestions to change from a username to a UID, and to use PUID/PGID instead of the default container user. |
|
Thanks for the clarification. That makes sense. However, I do not see suggestions that you mentioned. |
The suggestions are in the review. Go to the files tab in the GitHub issue. There are 3 suggestions. |
|
I only see those by coderabbitai[bot]. Are you sure they are not |
|
Thank you for the detailed comments. I have added the suggested changes. I also renamed |
adamoutler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I'm not able to test. Please confirm operation.
|
Yup, just redeployed my config from here and it looks good! |
|
Thanks a lot @2m for the PR and @adamoutler for the review 🙏 |
|
I've added a link to the nix flake from the INSTALLATIONS.md file, LMK if you want to create a full INSTALLATION_NIX.md guide (if needed): https://github.com/netalertx/NetAlertX/blob/main/docs/INSTALLATION.md - thanks again for the contribution! |
This flake definition can be added to another flake as an input like so:
Then add new module as
nixosConfigurationsmodules:And finally enable
netalertxservice:Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.