Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[20.03-backport] knot: keyFiles, no dynamicUser #80172

Merged
merged 3 commits into from Feb 17, 2020
Merged

[20.03-backport] knot: keyFiles, no dynamicUser #80172

merged 3 commits into from Feb 17, 2020

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Feb 15, 2020

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mic92
knot: put runtime paths outside the nix store
e989a19 
Otherwise knot tries to write to non-writable directories.
This for example breaks dnssec signing.
While it's possible to overwrite these path in the configuration,
having a sane defaults is nicer.

(cherry picked from commit 6adc09e)
@Mic92
knot: drop dynamic user
0af3b75 
This makes it hard to include secret files.
Also using tools like keymgr becomes harder.

(cherry picked from commit 88029bc)
@Mic92
knot: add keyFiles option
8325e2b 
This useful to include tsig keys using nixops without adding those
world-readable to the nix store.

(cherry picked from commit e2ef8b4)
@Mic92 Mic92 requested a review from mweinelt February 15, 2020 11:19
@ofborg ofborg bot requested a review from vcunat February 15, 2020 12:10
mweinelt
mweinelt reviewed Feb 15, 2020
View reviewed changes
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

The module changes lgtm. I'm not too familiar with the package. I aim to testdrive this tonight.

@mweinelt
Copy link
Member

LGTM.

@Mic92 Mic92 merged commit 47dfd37 into NixOS:release-20.03 Feb 17, 2020
@Mic92 Mic92 deleted the knot-backport branch February 17, 2020 17:24
@vcunat
Copy link
Member

vcunat commented Feb 18, 2020

I expect this doesn't need a line in NixOS release notes, as migrating from DynamicUser=yes should be compatible, right? I think even switching back and forth should work OK thanks to systemd adjusting ownership automatically.

@Mic92
Copy link
Member Author

Mic92 commented Feb 25, 2020

Yes. The other way around it might cause some problems but switching away from DynamicUser should not break anything.

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

@mweinelt mweinelt mweinelt left review comments

@vcunat vcunat Awaiting requested review from vcunat

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mic92 @mweinelt @vcunat