Ensure all store types support "real" URIs

In particular `local://<path>` and `unix://` (without any path) now work, and mean the same things as `local` and `daemon`, respectively. We thus now have the opportunity to desguar `local` and `daemon` early. This will allow me to make a change to #9839 requested during review to desugar those earlier. Co-authored-by: Théophane Hufschmitt <[email protected]>
NixOS · Jan 30, 2024 · 17b0a12 · 17b0a12
1 parent d661a89
commit 17b0a12
Show file tree

Hide file tree

Showing 18 changed files with 178 additions and 76 deletions.
diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc
@@ -18,9 +18,12 @@ struct DummyStoreConfig : virtual StoreConfig {
 
 struct DummyStore : public virtual DummyStoreConfig, public virtual Store
 {
-    DummyStore(const std::string scheme, const std::string uri, const Params & params)
+    DummyStore(std::string_view scheme, std::optional<std::string_view> authority, const Params & params)
         : DummyStore(params)
-    { }
+    {
+        if (authority)
+            throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, *authority);
+    }
 
     DummyStore(const Params & params)
         : StoreConfig(params)

diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc
@@ -39,15 +39,20 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v
 public:
 
     HttpBinaryCacheStore(
-        const std::string & scheme,
-        const Path & _cacheUri,
+        std::string_view scheme,
+        std::optional<PathView> _cacheUri,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
         , HttpBinaryCacheStoreConfig(params)
         , Store(params)
         , BinaryCacheStore(params)
-        , cacheUri(scheme + "://" + _cacheUri)
+        , cacheUri(
+            std::string { scheme }
+            + "://"
+            + (_cacheUri
+                ? *_cacheUri
+                : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme)))
     {
         if (cacheUri.back() == '/')
             cacheUri.pop_back();

diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
@@ -28,8 +28,18 @@ struct LegacySSHStore::Connection : public ServeProto::BasicClientConnection
     bool good = true;
 };
 
+LegacySSHStore::LegacySSHStore(
+    std::string_view scheme,
+    std::optional<std::string_view> host,
+    const Params & params)
+    : LegacySSHStore{scheme, LegacySSHStoreConfig::extractConnStr(scheme, host), params}
+{
+}
 
-LegacySSHStore::LegacySSHStore(const std::string & scheme, const std::string & host, const Params & params)
+LegacySSHStore::LegacySSHStore(
+    std::string_view scheme,
+    std::string host,
+    const Params & params)
     : StoreConfig(params)
     , CommonSSHStoreConfig(params)
     , LegacySSHStoreConfig(params)
@@ -42,8 +52,8 @@ LegacySSHStore::LegacySSHStore(const std::string & scheme, const std::string & h
         ))
     , master(
         host,
-        sshKey,
-        sshPublicHostKey,
+        sshKey.get(),
+        sshPublicHostKey.get(),
         // Use SSH master only if using more than 1 connection.
         connections->capacity() > 1,
         compress,

diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh
@@ -41,7 +41,17 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
 
     static std::set<std::string> uriSchemes() { return {"ssh"}; }
 
-    LegacySSHStore(const std::string & scheme, const std::string & host, const Params & params);
+    LegacySSHStore(
+        std::string_view scheme,
+        std::optional<std::string_view> host,
+        const Params & params);
+
+private:
+    LegacySSHStore(
+        std::string_view scheme,
+        std::string host,
+        const Params & params);
+public:
 
     ref<Connection> openConnection();
 

diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc
@@ -28,16 +28,20 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
 
 public:
 
+    /**
+     * @param binaryCacheDir `file://` is a short-hand for `file:///`
+     * for now.
+     */
     LocalBinaryCacheStore(
-        const std::string scheme,
-        const Path & binaryCacheDir,
+        std::string_view scheme,
+        std::optional<PathView> binaryCacheDir,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
         , LocalBinaryCacheStoreConfig(params)
         , Store(params)
         , BinaryCacheStore(params)
-        , binaryCacheDir(binaryCacheDir)
+        , binaryCacheDir(binaryCacheDir.value_or(""))
     {
     }
 

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -446,10 +446,20 @@ LocalStore::LocalStore(const Params & params)
 }
 
 
-LocalStore::LocalStore(std::string scheme, std::string path, const Params & params)
-    : LocalStore(params)
+LocalStore::LocalStore(
+    std::string_view scheme,
+    std::optional<PathView> path,
+    const Params & _params)
+    : LocalStore([&]{
+        // Default `?root` from `path` if non set
+        if (path && _params.count("root") == 0) {
+            auto params = _params;
+            params.insert_or_assign("root", std::string { *path });
+            return params;
+        }
+        return _params;
+    }())
 {
-    throw UnimplementedError("LocalStore");
 }
 
 

diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
@@ -138,12 +138,15 @@ public:
      * necessary.
      */
     LocalStore(const Params & params);
-    LocalStore(std::string scheme, std::string path, const Params & params);
+    LocalStore(
+        std::string_view scheme,
+        std::optional<PathView> path,
+        const Params & params);
 
     ~LocalStore();
 
     static std::set<std::string> uriSchemes()
-    { return {}; }
+    { return {"local"}; }
 
     /**
      * Implementations of abstract store API methods.

diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc
@@ -213,7 +213,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           support it.
 
           > **Note**
-          > 
+          >
           > HTTPS should be used if the cache might contain sensitive
           > information.
         )"};
@@ -224,7 +224,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
           Do not specify this setting if you're using Amazon S3.
 
           > **Note**
-          > 
+          >
           > This endpoint must support HTTPS and will use path-based
           > addressing instead of virtual host based addressing.
         )"};
@@ -269,16 +269,18 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual
     S3Helper s3Helper;
 
     S3BinaryCacheStoreImpl(
-        const std::string & uriScheme,
-        const std::string & bucketName,
+        std::string_view uriScheme,
+        std::optional<std::string_view> bucketName,
         const Params & params)
         : StoreConfig(params)
         , BinaryCacheStoreConfig(params)
         , S3BinaryCacheStoreConfig(params)
         , Store(params)
         , BinaryCacheStore(params)
         , S3BinaryCacheStore(params)
-        , bucketName(bucketName)
+        , bucketName(bucketName
+            ? *bucketName
+            : throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme))
         , s3Helper(profile, region, scheme, endpoint)
     {
         diskCache = getNarInfoDiskCache();

diff --git a/src/libstore/ssh-store-config.cc b/src/libstore/ssh-store-config.cc
@@ -0,0 +1,27 @@
+#include <regex>
+
+#include "ssh-store-config.hh"
+
+namespace nix {
+
+std::string CommonSSHStoreConfig::extractConnStr(
+    std::string_view scheme,
+    std::optional<std::string_view> _connStr)
+{
+    std::string connStr = _connStr
+        ? (std::string { *_connStr })
+        : throw UsageError("`%s` store requires a valid SSH host as the authority part in Store URI", scheme);
+
+    std::smatch result;
+    static std::regex v6AddrRegex("^((.*)@)?\\[(.*)\\]$");
+
+    if (std::regex_match(connStr, result, v6AddrRegex)) {
+        connStr = result[1].matched
+            ? result.str(1) + result.str(3)
+            : result.str(3);
+    }
+
+    return connStr;
+}
+
+}
diff --git a/src/libstore/ssh-store-config.hh b/src/libstore/ssh-store-config.hh
@@ -24,6 +24,29 @@ struct CommonSSHStoreConfig : virtual StoreConfig
           to be used on the remote machine. The default is `auto`
           (i.e. use the Nix daemon or `/nix/store` directly).
         )"};
+
+    /**
+     * The `parseURL` function supports both IPv6 URIs as defined in
+     * 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:
+     *
+     * - 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
+     *   will be transformed into `root@::1` for SSH (same for `[::1]` -> `::1`).
+     *
+     * - If the URL looks like `root@::1` it will be left as-is.
+     *
+     * - In any other case, the string will be left as-is.
+     *
+     * Will throw an error if `connStr` is empty too.
+     */
+    static std::string extractConnStr(
+        std::string_view scheme,
+        std::optional<std::string_view> connStr);
 };
 
 }
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
@@ -32,9 +32,10 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig
 
 class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
 {
-public:
-
-    SSHStore(const std::string & scheme, const std::string & host, const Params & params)
+    SSHStore(
+        std::string_view scheme,
+        std::string host,
+        const Params & params)
         : StoreConfig(params)
         , RemoteStoreConfig(params)
         , CommonSSHStoreConfig(params)
@@ -44,14 +45,24 @@ class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore
         , host(host)
         , master(
             host,
-            sshKey,
-            sshPublicHostKey,
+            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::optional<std::string_view> host,
+        const Params & params)
+        : SSHStore{scheme, SSHStoreConfig::extractConnStr(scheme, host), params}
+    {
+    }
+
     static std::set<std::string> uriSchemes() { return {"ssh-ng"}; }
 
     std::string getUri() override
@@ -141,7 +152,10 @@ class MountedSSHStore : public virtual MountedSSHStoreConfig, public virtual SSH
 {
 public:
 
-    MountedSSHStore(const std::string & scheme, const std::string & host, const Params & params)
+    MountedSSHStore(
+        std::string_view scheme,
+        std::optional<std::string_view> host,
+        const Params & params)
         : StoreConfig(params)
         , RemoteStoreConfig(params)
         , CommonSSHStoreConfig(params)

diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc
@@ -6,7 +6,11 @@
 
 namespace nix {
 
-SSHMaster::SSHMaster(const std::string & host, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD)
+SSHMaster::SSHMaster(
+    std::string_view host,
+    std::string_view keyFile,
+    std::string_view sshPublicHostKey,
+    bool useMaster, bool compress, int logFD)
     : host(host)
     , fakeSSH(host == "localhost")
     , keyFile(keyFile)

diff --git a/src/libstore/ssh.hh b/src/libstore/ssh.hh
@@ -33,7 +33,11 @@ private:
 
 public:
 
-    SSHMaster(const std::string & host, const std::string & keyFile, const std::string & sshPublicHostKey, bool useMaster, bool compress, int logFD = -1);
+    SSHMaster(
+        std::string_view host,
+        std::string_view keyFile,
+        std::string_view sshPublicHostKey,
+        bool useMaster, bool compress, int logFD = -1);
 
     struct Connection
     {