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

ssh: put custom options before generated options #53767

Merged
merged 1 commit into from Feb 19, 2019

Conversation

Projects
None yet
4 participants
@kwohlfahrt
Copy link
Contributor

kwohlfahrt commented Jan 10, 2019

Otherwise, the standard options (e.g. AddressFamily) cannot be overriden
in extraConfig, as the option is applied on the first (not most
specific) match. Closes #52267

Motivation for this change

See bug #52267

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)
  • 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

subtest "configuration", sub {
$client->succeed("ssh -G other_server | grep -i 'addressfamily any'");
$client->succeed("ssh -G example_server | grep -i 'addressfamily inet'");

This comment has been minimized.

@Mic92

Mic92 Jan 10, 2019

Contributor

Not sure if we need an extra test for that.

This comment has been minimized.

@kwohlfahrt

kwohlfahrt Jan 10, 2019

Author Contributor

Fair enough, will drop that commit.

@@ -203,6 +203,9 @@ in
# generation in the sshd service.
environment.etc."ssh/ssh_config".text =
''
${cfg.extraConfig}

This comment has been minimized.

@Mic92

Mic92 Jan 10, 2019

Contributor
Suggested change Beta
${cfg.extraConfig}
# To allow users to override existing configuration, we prepend `extraConfig`.
${cfg.extraConfig}

This comment has been minimized.

@kwohlfahrt

kwohlfahrt Jan 10, 2019

Author Contributor

Is this comment aimed at people inspecting /etc/ssh/ssh_config, or people reading ssh.nix?

This comment has been minimized.

@Mic92

Mic92 Jan 11, 2019

Contributor

Ah right. Your version makes more sense.

@kwohlfahrt kwohlfahrt force-pushed the kwohlfahrt:ssh branch from 1dda710 to 545a6cd Jan 10, 2019

@kwohlfahrt

This comment has been minimized.

Copy link
Contributor Author

kwohlfahrt commented Jan 10, 2019

@Mic92 I think I have addressed your comments

@kwohlfahrt kwohlfahrt force-pushed the kwohlfahrt:ssh branch from 2408f78 to 67e2f6a Jan 11, 2019

@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Jan 11, 2019

@GrahamcOfBorg build nixosTests.openssh

@kwohlfahrt

This comment has been minimized.

Copy link
Contributor Author

kwohlfahrt commented Feb 14, 2019

@Mic92 - any update on this?

@@ -203,6 +203,9 @@ in
# generation in the sshd service.
environment.etc."ssh/ssh_config".text =
''
${cfg.extraConfig}
Host *

This comment has been minimized.

@Infinisil

Infinisil Feb 14, 2019

Contributor

What's this for?

This comment has been minimized.

@kwohlfahrt

kwohlfahrt Feb 18, 2019

Author Contributor

The Host * is so the following config options apply to all hosts if they haven't been set before. For example if cfg.extraConfig is:

Host example.com
    Port 10022

then the options below would only apply to example.com without the Host * - ssh options are not indentation sensitive, that is just for readability.

@Infinisil

This comment has been minimized.

Copy link
Contributor

Infinisil commented Feb 18, 2019

Can you squash the commits together? And then the commit message should have the "nixos/ssh:" prefix instead.

nixos/ssh: apply options after extraConfig
Otherwise, the standard options (e.g. AddressFamily) cannot be overriden
in extraConfig, as the option is applied on the first (not most
specific) match. Closes #52267

@kwohlfahrt kwohlfahrt force-pushed the kwohlfahrt:ssh branch from 67e2f6a to de7abf6 Feb 18, 2019

@kwohlfahrt

This comment has been minimized.

Copy link
Contributor Author

kwohlfahrt commented Feb 18, 2019

@Infinisil rebased onto master and squashed commits

@Infinisil Infinisil merged commit 266315c into NixOS:master Feb 19, 2019

10 checks passed

grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment