-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
kbfs, nixos/keybase, nixos/kbfs: fix KBFS, add enableRedirector option #75922
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.
LGTM
environment.KEYBASE_SERVICE_TYPE = "systemd"; | ||
|
||
script = '' | ||
${pkgs.keybase}/bin/keybase --use-default-log-file --debug service |
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.
--debug
seems unnecessary, but I guess since upstream does that it should be fine
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.
✔️
} | ||
|
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.
Remove newline
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.
✔️
e207451
to
bf426f6
Compare
bf426f6
to
b4bacff
Compare
Addressed feedback. I was also able to remove the |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/104 |
LGTM, Thank you |
kbfs, nixos/keybase, nixos/kbfs: fix KBFS, add enableRedirector option
Motivation for this change
KBFS wasn't working on my system using the NixOS module, so I updated the systemd services to mirror the upstream service files. Comments from upstream are included, and the main changes are:
notify
service type which is now supported upstream.KEYBASE_SERVICE_TYPE=systemd
in the environment forkeybase.service
; this is apparently what fixed the issues between the keybase and kbfs on my system.$XDG_CONFIG_DIR/keybase/keybase{.autogen}.env
, which is populated by thekeybase ctl init
command as specified in the documentation. This is entirely optional and my setup works without doing this.I also wanted to run
keybase-redirector
on a multi-user machine, so I addedconfig.services.enableRedirector
. This requires the following shenanigans:kbfs
package./keybase
needs to exist; usesystemd.tmpfiles.rules
to add it and clean it up ifenableRedirector
is disabled. If there is a standard way to do this for user services, I'd be happy to change this.setuid
wrapper for${pkgs.kbfs}/bin/redirector
, namedkeybase-redirector
for consistency with the documentation and other distributions.setpcap
wrapper instead; however, the redirector callssetreuid
and thus would requireCAP_SYS_ADMIN
anyway.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @rvolosatovs @bennofs @np