Merge pull request #10768 from obsidiansystems/legacy-ssh-expose-ssh-master-for-hydra

Create `CommonSSHStoreConfig::createSSHMaster`

src/libstore/legacy-ssh-store.cc

@@ -32,32 +32,19 @@ LegacySSHStore::LegacySSHStore(
     std::string_view scheme,
     std::string_view host,
     const Params & params)
-    : LegacySSHStore{scheme, LegacySSHStoreConfig::extractConnStr(scheme, host), params}
-{
-}
-
-LegacySSHStore::LegacySSHStore(
-    std::string_view scheme,
-    std::string host,
-    const Params & params)
     : StoreConfig(params)
-    , CommonSSHStoreConfig(params)
+    , CommonSSHStoreConfig(scheme, host, params)
     , LegacySSHStoreConfig(params)
     , Store(params)
-    , host(host)
     , connections(make_ref<Pool<Connection>>(
         std::max(1, (int) maxConnections),
         [this]() { return openConnection(); },
         [](const ref<Connection> & r) { return r->good; }
         ))
-    , master(
+    , master(createSSHMaster(
-        host,
-        sshKey.get(),
-        sshPublicHostKey.get(),
         // Use SSH master only if using more than 1 connection.
         connections->capacity() > 1,
-        compress,
+        logFD))
-        logFD)
 {
 }
 

src/libstore/legacy-ssh-store.hh

@@ -33,8 +33,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
 
     struct Connection;
 
-    std::string host;
-
     ref<Pool<Connection>> connections;
 
     SSHMaster master;
@@ -46,13 +44,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         std::string_view host,
         const Params & params);
 
-private:
-    LegacySSHStore(
-        std::string_view scheme,
-        std::string host,
-        const Params & params);
-public:
-
     ref<Connection> openConnection();
 
     std::string getUri() override;

src/libstore/ssh-store-config.cc

@@ -1,10 +1,11 @@
 #include <regex>
 
 #include "ssh-store-config.hh"
+#include "ssh.hh"
 
 namespace nix {
 
-std::string CommonSSHStoreConfig::extractConnStr(std::string_view scheme, std::string_view _connStr)
+static std::string extractConnStr(std::string_view scheme, std::string_view _connStr)
 {
     if (_connStr.empty())
         throw UsageError("`%s` store requires a valid SSH host as the authority part in Store URI", scheme);
@@ -21,4 +22,22 @@ std::string CommonSSHStoreConfig::extractConnStr(std::string_view scheme, std::s
     return connStr;
 }
 
+CommonSSHStoreConfig::CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params)
+    : StoreConfig(params)
+    , host(extractConnStr(scheme, host))
+{
+}
+
+SSHMaster CommonSSHStoreConfig::createSSHMaster(bool useMaster, Descriptor logFD)
+{
+    return {
+        host,
+        sshKey.get(),
+        sshPublicHostKey.get(),
+        useMaster,
+        compress,
+        logFD,
+    };
+}
+
 }

src/libstore/ssh-store-config.hh

@@ -5,10 +5,14 @@
 
 namespace nix {
 
+class SSHMaster;
+
 struct CommonSSHStoreConfig : virtual StoreConfig
 {
     using StoreConfig::StoreConfig;
 
+    CommonSSHStoreConfig(std::string_view scheme, std::string_view host, const Params & params);
+
     const Setting<Path> sshKey{this, "", "ssh-key",
         "Path to the SSH private key used to authenticate to the remote machine."};
 
@@ -30,9 +34,7 @@ struct CommonSSHStoreConfig : virtual StoreConfig
      * RFC2732, but also pure addresses. The latter one is needed here to
      * connect to a remote store via SSH (it's possible to do e.g. `ssh root@::1`).
      *
-     * This function now ensures that a usable connection string is available:
+     * When initialized, the following adjustments are made:
-     *
-     * - If the store to be opened is not an SSH store, nothing will be done.
      *
      * - If the URL looks like `root@[::1]` (which is allowed by the URL parser and probably
      *   needed to pass further flags), it
@@ -44,9 +46,17 @@ struct CommonSSHStoreConfig : virtual StoreConfig
      *
      * Will throw an error if `connStr` is empty too.
      */
-    static std::string extractConnStr(
+    std::string host;
-        std::string_view scheme,
+
-        std::string_view connStr);
+    /**
+     * Small wrapper around `SSHMaster::SSHMaster` that gets most
+     * arguments from this configuration.
+     *
+     * See that constructor for details on the remaining two arguments.
+     */
+    SSHMaster createSSHMaster(
+        bool useMaster,
+        Descriptor logFD = INVALID_DESCRIPTOR);
 };
 
 }

src/libstore/ssh-store.cc

@@ -32,34 +32,21 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
 
 class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
 {
-    SSHStore(
-        std::string_view scheme,
-        std::string host,
-        const Params & params)
-        : StoreConfig(params)
-        , RemoteStoreConfig(params)
-        , CommonSSHStoreConfig(params)
-        , SSHStoreConfig(params)
-        , Store(params)
-        , RemoteStore(params)
-        , host(host)
-        , master(
-            host,
-            sshKey.get(),
-            sshPublicHostKey.get(),
-            // Use SSH master only if using more than 1 connection.
-            connections->capacity() > 1,
-            compress)
-    {
-    }
-
 public:
 
     SSHStore(
         std::string_view scheme,
         std::string_view host,
         const Params & params)
-        : SSHStore{scheme, SSHStoreConfig::extractConnStr(scheme, host), params}
+        : StoreConfig(params)
+        , RemoteStoreConfig(params)
+        , CommonSSHStoreConfig(scheme, host, params)
+        , SSHStoreConfig(params)
+        , Store(params)
+        , RemoteStore(params)
+        , master(createSSHMaster(
+            // Use SSH master only if using more than 1 connection.
+            connections->capacity() > 1))
     {
     }
 
@@ -119,6 +106,15 @@ struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfi
     {
     }
 
+    MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params)
+        : StoreConfig(params)
+        , RemoteStoreConfig(params)
+        , CommonSSHStoreConfig(scheme, host, params)
+        , SSHStoreConfig(params)
+        , LocalFSStoreConfig(params)
+    {
+    }
+
     const std::string name() override { return "Experimental SSH Store with filesystem mounted"; }
 
     std::string doc() override
@@ -158,7 +154,7 @@ public:
         const Params & params)
         : StoreConfig(params)
         , RemoteStoreConfig(params)
-        , CommonSSHStoreConfig(params)
+        , CommonSSHStoreConfig(scheme, host, params)
         , SSHStoreConfig(params)
         , LocalFSStoreConfig(params)
         , MountedSSHStoreConfig(params)

src/libstore/ssh.cc

@@ -10,7 +10,7 @@ SSHMaster::SSHMaster(
     std::string_view host,
     std::string_view keyFile,
     std::string_view sshPublicHostKey,
-    bool useMaster, bool compress, int logFD)
+    bool useMaster, bool compress, Descriptor logFD)
     : host(host)
     , fakeSSH(host == "localhost")
     , keyFile(keyFile)

src/libstore/ssh.hh

@@ -17,7 +17,7 @@ private:
     const std::string sshPublicHostKey;
     const bool useMaster;
     const bool compress;
-    const int logFD;
+    const Descriptor logFD;
 
     struct State
     {
@@ -43,7 +43,7 @@ public:
         std::string_view host,
         std::string_view keyFile,
         std::string_view sshPublicHostKey,
-        bool useMaster, bool compress, int logFD = -1);
+        bool useMaster, bool compress, Descriptor logFD = INVALID_DESCRIPTOR);
 
     struct Connection
     {