-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
nixos/opnborg: init module #351630
base: master
Are you sure you want to change the base?
nixos/opnborg: init module #351630
Conversation
92f8f69
to
c3d26b1
Compare
Result of 1 package blacklisted:
|
Since you are also the upstream developer, you would most likely know best how to properly add a nice test for this module. @paepckehh would mind doing that? It's by no means a hard requirement to get this merged, but would be cool nonetheless! |
Yes, big fan of unit tests - will pick up that point for my other packages as well! In this special case the opnborg orchestrator needs a special (open source) firewall appliance (https://github.com/opnsense/) access (via api-key/secret) Without counterpart it will just terminate directly. |
Love nixpkgs-review, really a great tool! Thank you! |
options.services.opnborg = { | ||
enable = mkEnableOption "opnborg"; | ||
|
||
user = mkOption { |
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.
Do you need a fixed user for this service? Or can a systemd DynamicUser be used?
I think the main reason for needing a "real" user is if a local user needs to interact with the data directly, outside of the service. If the data is only accessed via the service than DynamicUser is the easiest solution.
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.
The backup data from the appliances contain sensitive information.
Having a dedicated user as owner might help develop individual firewall / security concepts if needed.
When this is a blocking point, I can remove the user. Its no hard requirement within the application itself.
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.
It's not about having a separate user. I think that makes sense. It's about creating an explicit user versus letting systemd create one implicitly via DynamicUser.
It's explained in more detail when to use what in https://discourse.nixos.org/t/predefined-uids-gids-when-to-use-them-and-when-not-to-use-them/956/3
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.
I may need some guidance how to solve the following conflict:
-
The application clearly needs a fixed reliable deterministic gid/guid to store/backup/restore the appliances config data (/var/lib/) that contain secrets (keys,...)
-
The appliance (open source firewall / infrastructure / cluster) audience is rather small, so reserve a systemuser deterministic uid/gid below 399 via nixos/modules/misc/ids.nix tight address space might be not justified !?
Maybe my updated commit/proposal (fully verified & end-to-end tested) could be a common ground till rfc0052 makes more official headroom?
c3d26b1
to
f3004ab
Compare
f3004ab
to
7b44b6b
Compare
Selfhostable OPNSense Appliance Configuration Management & Backup Portal
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.