-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
Merge pull request #78358 from serokell/yorickvp/alertmanager-secret #78358
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
Merge pull request #78358 from serokell/yorickvp/alertmanager-secret #78358
Conversation
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.
Looking good!
@@ -127,6 +127,18 @@ in { | |||
Extra commandline options when launching the Alertmanager. | |||
''; | |||
}; | |||
|
|||
environmentFile = mkOption { | |||
type = types.nullOr types.path; |
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.
Are there concerns around this being added to the store?
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 checked, it won't be added to the store even when set with environmentFile = ./local/file
due to how systemd.services.*.serviceConfig
is implemented.
@@ -144,9 +156,14 @@ in { | |||
systemd.services.alertmanager = { | |||
wantedBy = [ "multi-user.target" ]; | |||
after = [ "network.target" ]; | |||
preStart = '' | |||
${lib.getBin pkgs.envsubst}/bin/envsubst -o /tmp/alert-manager-substituted.yaml" \ |
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.
We might want to set a umask here, and also below explicitly enable PrivateTmp instead of depending on the implied PrivateTemp.
…cret nixos/alertmanager: add environmentFile, envsubst for secrets (cherry picked from commit 342bc51)
Motivation for this change
Alertmanager has secret inputs, like mail passwords. Let's use
envsubst
to substitute environment variables into the yaml config.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)