lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

ssh-ng: better way to keep SSH errors visible

Browse source 
A better fix than in 104448e75d, hence a
revert + the fix.

It turns out that this commit has the side-effect that when having e.g.
`StrictHostKeyChecking=accept-new` for a remote builder, the warnings à la

    Warning: Permanently added 'builder' (ED25519) to the list of known hosts.

actually end up in the derivation's log whereas hostkey verification
errors don't, but only in the stderr of the `nix-build` invocation
(which was the motivation for the patch).

This change writes the stderr from the build-hook to

* the daemon's stderr, so that the SSH errors appear in the journal
  (which was the case before 104448e75d)
* the client's stderr, as a log message
* NOT to the drv log (this is handled via `handleJSONLogMessage`)

I tried to fix the issue for legacy-ssh as well, but failed and
ultimately decided to not bother.

I know that we'll sooner or later replace the entire component, however
this is the part of the patch I have working for a while, so I figured I
might still submit it for the time being.

Change-Id: I21ca1aa0d8ae281d2eacddf26e0aa825272707e5
This commit is contained in:
Maximilian Bosch 2024-10-12 23:42:59 +02:00
parent 326cbecb61
commit 4682e40183
4 changed files with 5 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/libstore/build/derivation-goal.cc
View file

@ -1211,6 +1211,7 @@ HookReply DerivationGoal::tryBuildHook()
else {
s += "\n";
writeLogsToStderr(s);
logger->log(lvlInfo, s);
}
}

2
src/libstore/machines.cc
View file

@ -68,11 +68,11 @@ ref<Store> Machine::openStore() const
{
Store::Params storeParams;
if (storeUri.starts_with("ssh://")) {
storeParams["log-fd"] = "4";
storeParams["max-connections"] = "1";
}
if (storeUri.starts_with("ssh://") || storeUri.starts_with("ssh-ng://")) {
storeParams["log-fd"] = "4";
if (sshKey != "")
storeParams["ssh-key"] = sshKey;
if (sshPublicHostKey != "")

8
src/libstore/ssh-store.cc
View file

@ -30,11 +30,6 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
{
public:
// Hack for getting remote build log output.
// Intentionally not in `SSHStoreConfig` so that it doesn't appear in
// the documentation
const Setting<int> logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"};
SSHStore(const std::string & scheme, const std::string & host, const Params & params)
: StoreConfig(params)
, RemoteStoreConfig(params)
@ -49,8 +44,7 @@ public:
sshPublicHostKey,
// Use SSH master only if using more than 1 connection.
connections->capacity() > 1,
compress,
logFD)
compress)
{
}

3
tests/nixos/remote-builds-ssh-ng.nix
View file

@ -97,7 +97,8 @@ in
builder.wait_for_unit("sshd.service")
out = client.fail("nix-build ${expr nodes.client 1} 2>&1")
assert "error: failed to start SSH connection to 'root@builder': Host key verification failed" in out, f"No host verification error in {out}"
assert "Host key verification failed." in out, f"No host verification error:\n{out}"
assert "warning: SSH to 'root@builder' failed, stdout first line: '''" in out, f"No details about which host:\n{out}"
client.succeed(f"ssh -o StrictHostKeyChecking=no {builder.name} 'echo hello world' >&2")