nixos/beszel: allow S.M.A.R.T monitoring#460730
Conversation
|
Still not tested as the release 0.15.5 is not out for now. |
|
|
e5d4611 to
00771ee
Compare
|
|
I need to wait for go 1.25.3 to be merged in nixos-unstable to test on my server |
|
May I need to fix the tests on darwin with something like : checkFlags = lib.optionals.stdenv.isDarwin [
# Disable tests that require a running server on darwin
"-run=^(Test(?!StartServer))"
];Or is this normal ? |
|
Just tested it in linux x86_64 and aarch64 and it works. |
| # Capabilities needed for SMART disk monitoring | ||
| AmbientCapabilities = [ | ||
| "CAP_SYS_RAWIO" | ||
| "CAP_SYS_ADMIN" | ||
| ]; | ||
| CapabilityBoundingSet = [ | ||
| "CAP_SYS_RAWIO" | ||
| "CAP_SYS_ADMIN" | ||
| ]; | ||
|
|
||
| # Device access for SMART monitoring | ||
| DevicePolicy = "closed"; | ||
| DeviceAllow = [ | ||
| "block-blkext rw" | ||
| "block-sd rw" | ||
| "char-nvme rw" | ||
| ]; |
There was a problem hiding this comment.
Maybe these should be optional, including the smartmontools package.
For those who are not interested in SMART stats these settings are not needed and the security aspect we had before can be kept.
There was a problem hiding this comment.
Agreed, adding an option for such a purpose would be ideal or somehow manage it via env variables. I will look into this later.
There was a problem hiding this comment.
Having an option for this is definitely a good idea, but I’m not entirely sure whether it should default to true or false.
Pros of setting it to true by default:
- Improves discoverability of Beszel’s features.
- Prevents potential warnings if Beszel tries to access a disk without the proper permissions.
- Could alert users to disk issues they might not have been aware of.
Pros of setting it to false by default:
- Avoids interfering with users’ drive sleep schedules, which can be important for some setups.
|
Here are a few points:
Points 1. and 3. both need Regarding 2., what would be the main point of this PR (IMHO): |
|
Ok I will split this PR into 3 PR (keeping update to 0.16.0 to be faster than r-ryantm) |
You are right, but r-ryantm can be merged by a package maintainer, your PR has to be merged by someone with a commit bit. But we will see what will happen faster. I think there is a way to enforce a r-ryantm PR, but I will have to look into that :) |
|
Any specific reason for |
|
I stole it at : But yes, I agree that just using mkDefault should be enough. |
|
No hint on why this is forced in the PR from Prometheus (#147056), so let's stay with mkDefault. |
|
Note to commiters: this PR is waiting for relevant package update. |
This is not true anymore; S.M.A.R.T. support is included in Beszel since v0.15.0, and the current Beszel in Nixpkgs (unstable) is 0.15.4. |
BonusPlay
left a comment
There was a problem hiding this comment.
Oh right, I meant the systemd is waiting for 0.16. We can merge this one.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/6130 |
8d58269 to
1ada542
Compare
|
@arunoruto is this PR good for you, or are there still things to fix? |
4cd0042 to
c0b6939
Compare
arunoruto
left a comment
There was a problem hiding this comment.
I am not sure about the systemd settings, someone else should take a look at it if its fine. But I have been using the PR for a time now and it has been working. This doesn't mean the security can't be tighten further ;)
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/6301 |
|
Successfully created backport PR for |
Update to beszel 0.15.5 and allow systemd monitoring following this pr merged : henrygd/beszel#1153
following this comment in the module init : #380731 (comment)
Also allow smart monitoring highly copied from nixos/modules/services/monitoring/prometheus/exporters/smartctl.nix
@Bot-wxt1221
@arunoruto
@BonusPlay
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.