Skip to content

longview service: don't write passwords to nix store#24366

Merged
7c6f434c merged 1 commit intoNixOS:masterfrom
rvl:longview-password-file
May 1, 2017
Merged

longview service: don't write passwords to nix store#24366
7c6f434c merged 1 commit intoNixOS:masterfrom
rvl:longview-password-file

Conversation

@rvl
Copy link
Copy Markdown
Contributor

@rvl rvl commented Mar 26, 2017

Adds services.longview.{apiKeyFile,mysqlPasswordFile} options as alternatives to apiKey and mysqlPassword, which still work, but are deprecated with a warning message.

Related to #24288.

Motivation for this change

Security.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Testing note: This PR is rebased to master branch but I actually did the testing on nixos-unstable.

Adds services.longview.{apiKeyFile,mysqlPasswordFile} options as
alternatives to apiKey and mysqlPassword, which still work, but are
deprecated with a warning message.

Related to NixOS#24288.
@mention-bot
Copy link
Copy Markdown

@rvl, thanks for your PR! By analyzing the history of the files in this pull request, we identified @globin, @dezgeg and @zimbatm to be potential reviewers.

@rvl
Copy link
Copy Markdown
Contributor Author

rvl commented Mar 26, 2017

/cc @basvandijk

apiKeyFile = mkOption {
type = types.nullOr types.path;
default = null;
example = "/run/keys/longview-api-key";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the example be something with /var/keys? /run/keys would need to be refilled on every reboot…

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use /run/keys in all the examples of #24288 because that's what nixops is using. See: https://nixos.org/nixops/manual/#idm140737318306400

@7c6f434c 7c6f434c merged commit 7e19fcd into NixOS:master May 1, 2017
@rvl
Copy link
Copy Markdown
Contributor Author

rvl commented May 1, 2017

Thanks @basvandijk, @7c6f434c 🌲

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants