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.

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

nixos-rebuild: support sudo + --target-host #71143

Merged
merged 1 commit into from Oct 22, 2019

Conversation

@bjornfor
Copy link
Contributor

bjornfor commented Oct 14, 2019

Motivation for this change

This adds support for deploying to remote hosts without being root:

sudo nixos-rebuild --target-host non-root@host

Without this change, only root@host is able to deploy.

The idea is that if the local command is run with sudo, so should the
remote one, thus there is no need for adding any CLI options.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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): nix-build ./nixos/tests/installer.nix passes
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-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.
Notify maintainers

cc @jtojnar @matthewbauer @LnL7 @Mic92 (last few people to touch nixos-rebuild)

This adds support for deploying to remote hosts without being root:

  sudo nixos-rebuild --target-host non-root@host

Without this change, only root@host is able to deploy.

The idea is that if the local command is run with sudo, so should the
remote one, thus there is no need for adding any CLI options.
@@ -111,17 +115,17 @@ buildHostCmd() {
if [ -z "$buildHost" ]; then
"$@"
elif [ -n "$remoteNix" ]; then
ssh $SSHOPTS "$buildHost" env PATH="$remoteNix:$PATH" "$@"
ssh $SSHOPTS "$buildHost" env PATH="$remoteNix:$PATH" "$maybeSudo$@"

This comment has been minimized.

Copy link
@jtojnar

jtojnar Oct 14, 2019

Contributor

Security policies might prevent sudo from passing PATH, so maybe something like @edolstra suggested might be needed after all.

This comment has been minimized.

Copy link
@bjornfor

bjornfor Oct 15, 2019

Author Contributor

Good point. At least this works well with NixOS, I tested with my setup:

  security.sudo = {
    enable = true;
    wheelNeedsPassword = false;
  };

(I don't want to do change how PATH is handled here, as I feel it's orthogonal to this PR.)

@bjornfor

This comment has been minimized.

Copy link
Contributor Author

bjornfor commented Oct 19, 2019

I plan to merge this in 2 days (monday evening), if no other comments.

@bjornfor bjornfor merged commit 263a81e into NixOS:master Oct 22, 2019
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@bjornfor bjornfor deleted the bjornfor:nixos-rebuild-retain-sudo branch Oct 22, 2019
@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Oct 23, 2019

I just find the assumption a bit weird, why would I want to use sudo remotely because it was used locally and the other way around?
Somebody might use sudo locally because the configuration is not owned by the current user. Somebody might not want to type in a sudo password locally while it might be needed when deploying remotely.

@bjornfor

This comment has been minimized.

Copy link
Contributor Author

bjornfor commented Oct 23, 2019

I did it because of simplicity. But I agree there are use cases for local sudo != remote sudo.

I initially wanted to have a flag like --nixos-rebuild-path="sudo nixos-rebuild" (this matches rsync and borg), but nixos-rebuild doesn't call itself on the remote side, it calls nix-build, nix-env etc.

How about --use-sudo=0/1 that is only accepted when --target-host is set?

@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Oct 23, 2019

@bjornfor that sounds like a plan.

@Mic92 Mic92 mentioned this pull request Oct 23, 2019
0 of 10 tasks complete
@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Oct 23, 2019

I opened a revert PR: #71788 so we don't forget to change that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.