From 80a94c42a1f24a0bf5a94908c04f2f1cb37bdb00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= Date: Fri, 8 Dec 2017 15:17:20 +0000 Subject: [PATCH 1/3] Fix `storeKeysOnMachine` defaulting to True in MachineState. See #803. This defaulted to True even though the nixops changelog for version 1.2 says that it was changed to False for security reasons. --- nixops/backends/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixops/backends/__init__.py b/nixops/backends/__init__.py index b95902498..93e48c1e9 100644 --- a/nixops/backends/__init__.py +++ b/nixops/backends/__init__.py @@ -45,7 +45,7 @@ class MachineState(nixops.resources.ResourceState): ssh_pinged = nixops.util.attr_property("sshPinged", False, bool) ssh_port = nixops.util.attr_property("targetPort", 22, int) public_vpn_key = nixops.util.attr_property("publicVpnKey", None) - store_keys_on_machine = nixops.util.attr_property("storeKeysOnMachine", True, bool) + store_keys_on_machine = nixops.util.attr_property("storeKeysOnMachine", False, bool) keys = nixops.util.attr_property("keys", {}, 'json') owners = nixops.util.attr_property("owners", [], 'json') From d625c714b28529ce796e7cd26e85b3c38822a428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= Date: Fri, 8 Dec 2017 15:09:44 +0000 Subject: [PATCH 2/3] Fix keys not being deployed to containers. Fixes #803. In the same fashion as in commit 985886c, container.py did not call `set_common_state()`, so keys remained set to `{}` (and other attributes to their default values likely as well). `get_ssh_flags()` was incorrectly called twice, and the second time with missing `**kwargs` thus leading to `scp` getting incorrect parameters, such as `-p 22`, which is not a valid flag for scp, which made uploading keys fail. The failure error message was also very bad, I filed an OpenSSH issue for scp at https://bugzilla.mindrot.org/show_bug.cgi?id=2809. Finally, the code that set the home directory in container.py's `run_command()` assumed that the passed command would be a single command that can be prefixed like `HOME=/root ...command...` which is not the case; the command may as well be a `(subshell)`, in which case this failed (nixops uses a subshell for keys permission settings to save roundtrips). Fixed with an `export` and semicolon. --- nixops/backends/container.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nixops/backends/container.py b/nixops/backends/container.py index 3c4e1e6de..f54eaa4f5 100644 --- a/nixops/backends/container.py +++ b/nixops/backends/container.py @@ -62,9 +62,7 @@ def get_ssh_flags(self, *args, **kwargs): # connection to the container via the host. flags = super(ContainerState, self).get_ssh_flags(*args, **kwargs) flags += ["-i", self.get_ssh_private_key_file()] - if self.host == "localhost": - flags.extend(MachineState.get_ssh_flags(self)) - else: + if self.host != "localhost": cmd = "ssh -x -a root@{0} {1} nc {2} {3}".format(self.get_host_ssh(), " ".join(self.get_host_ssh_flags()), self.private_ipv4, self.ssh_port) flags.extend(["-o", "ProxyCommand=" + cmd]) return flags @@ -104,7 +102,7 @@ def wait_for_ssh(self, check=False): def run_command(self, command, **kwargs): command = command.replace("'", r"'\''") return self.host_ssh.run_command( - "nixos-container run {0} -- bash --login -c 'HOME=/root {1}'".format(self.vm_id, command), + "nixos-container run {0} -- bash --login -c 'export HOME=/root; {1}'".format(self.vm_id, command), **kwargs) def get_physical_spec(self): @@ -120,6 +118,8 @@ def create_after(self, resources, defn): def create(self, defn, check, allow_reboot, allow_recreate): assert isinstance(defn, ContainerDefinition) + self.set_common_state(defn) + if not self.client_private_key: (self.client_private_key, self.client_public_key) = nixops.util.create_key_pair() From e041873b2e97ea5fefc0fea1000754edf71f3b45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20Hamb=C3=BCchen?= Date: Fri, 8 Dec 2017 15:20:12 +0000 Subject: [PATCH 3/3] Use a class instead of `object()` to model `undefined`. This has the same effect, but makes it be printed more obviously when debugging. --- nixops/util.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nixops/util.py b/nixops/util.py index 69f5570cd..a61b7ca24 100644 --- a/nixops/util.py +++ b/nixops/util.py @@ -216,7 +216,11 @@ def abs_nix_path(x): return xs[0] + '=' + _maybe_abspath(xs[1]) -undefined = object() +class Undefined: + pass + +undefined = Undefined() + def attr_property(name, default, type=str): """Define a property that corresponds to a value in the NixOps state file."""