nixos/packetbeat: Add basic module for packetbeat #94862
Conversation
| mkdir -p ${cfg.stateDir}/data | ||
| mkdir -p ${cfg.stateDir}/logs | ||
| ''; | ||
| serviceConfig = { |
There was a problem hiding this comment.
Please do not run as root if not required. Maybe DynamicUser would be appropriate.
There was a problem hiding this comment.
This is completely copied from the official unit, packetbeat needs to run as root because it captures packets from interfaces. I dunno if CAP_NET_ADMIN might be enough instead of having root, but this is how the official systemd unit does it.
| ''; | ||
| }; | ||
|
|
||
| configFlows = mkOption { |
There was a problem hiding this comment.
If this is just yaml does it even need is open option? RFC42 has been promoting a settings option which I think would benefit this module greatly.
There was a problem hiding this comment.
It is just yaml, this was a quick way of getting the module usable. My plan was to first get something usable, then refactor it to be smart too.
| ''; | ||
| }; | ||
|
|
||
| configProtocols = mkOption { |
There was a problem hiding this comment.
Would this benefit from a structured type? Maybe type = with types; attrsOf (either (bool (listOf port)));.
There was a problem hiding this comment.
Not impossible, its purely yaml it needs to generate, so a structured type would probably fit. As said in other comments, this was a quick way to getting it usable.
| let | ||
| cfg = config.services.packetbeat; | ||
|
|
||
| packetbeatYml = pkgs.writeText "packetbeat.yml" '' |
There was a problem hiding this comment.
See comment below about a settings option which could replace this.
There was a problem hiding this comment.
I will look into that RFC, I've added the configFile option, that short-circuits the config options in the module, and lets the user handle the configuration file as they please.
lejonet
force-pushed
the
packetbeat-module
branch
from
83033ce
to
1d33bd3
Compare
|
A bit of a rough sketch but what about something like this? |
| wantedBy = [ "multi-user.target" ]; | ||
| preStart = '' | ||
| mkdir -p ${cfg.stateDir}/data | ||
| mkdir -p ${cfg.stateDir}/logs |
There was a problem hiding this comment.
Is it possible to log to stdout instead so journald can pick this up?
There was a problem hiding this comment.
There is an option to log to stderr (-e) that could be used in ExecStart
There was a problem hiding this comment.
There is an option to log to stderr that should make it possible for journald to pick it up (-e).
Well, I've merged your suggestion and fixed it up a bit, however I cannot get it to evaluate. It would seem like there is something wrong with the lib.formats.yaml.generate call, I get the following error message: |
|
@lejonet I'll try to get back to this in the next few days... |
|
I'm sorry this took me so long to circle back to. I've pushed another copy which actually builds now... here. Maybe @infinisil wants to take a quick look over this module and offer some helpful advice in general, and specifically for the 2 problematic lines: |
|
I had to correct the type from |
|
Those freeform module options look great! Thanks for taking a look at this module @infinisil. |
|
Sweet, I'll hopefully have some time this weekend to look at this and merge your changes, so that I can do the testruns of the module. Just as a curious question, what is the actual difference between either and oneOf? Like, semantically, it would seem like they're the same, because both of them choose exactly one of a list of options, but in this case, I guess the implementations might differ in such a way that oneOf works with the freeform module suggestion and either doesn't? |
|
|
Ah that makes sense, I honestly thought that either was that shorthand, then I know |
|
I'm getting a weird issue that I hope that you guys can help with, my nix-fu is way too rusty and basic to find the cause of this. With the module in the current state and the following config: The error I get when I try to deploy the config is: The traceback isn't all to useful for me, but maybe it is for you, I think its whining about something with the ExecStart value, but that is supposed to be a string, isnt' it? |
It has not been used by KDE for many years and depends on umaintained libraries we want to drop (Qt4 and Gamin).
They have been broken and disabled for ages and now the dependencies are being removed.
It is insecure and broken. Follow NixOS#93398
it is insecure and broken
fixes at least 17 CVEs also adds test to package
previously it was using the legacy one
ncspot: 0.2.1 -> 0.2.2
plasma5: 5.17.5 -> 5.18.5
doppler: 3.10.1 -> 3.10.3
lejonet
requested review from
adisbladis,
alyssais,
andir,
cdepillabout,
edolstra,
edwtjo,
Ericson2314,
FRidh,
infinisil,
joachifm,
jonringer,
kalbasit,
LnL7,
matthewbauer,
Mic92,
mmahut,
nbp,
peti,
Profpatsch,
tfc,
thoughtpolice,
ttuegel and
zowoq
as code owners
Motivation for this change
Basic module to enable configuring and enabling packetbeat to send
packet data to elasticsearch. Modeled after the journalbeat service.
We have the packetbeat package in nixpkgs for some time, but no module, so I decided to make a basic one. I decided to make the configuration for flows and protocols their separate config values, so that if all you want to modify is general settings and outputs, you don't have to copy the entire default config and maintain it yourself.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)