From 470c0501eb994c8a31f03561d4d38114236ab8b3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 25 Jan 2024 10:31:52 -0500 Subject: [PATCH] Ensure all store types support "real" URIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In particular `local://` 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 https://github.com/NixOS/nix/pull/9839 requested during review to desugar those earlier. Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com> --- src/libstore/dummy-store.cc | 7 ++-- src/libstore/http-binary-cache-store.cc | 11 +++++-- src/libstore/legacy-ssh-store.cc | 16 +++++++-- src/libstore/legacy-ssh-store.hh | 12 ++++++- src/libstore/local-binary-cache-store.cc | 8 +++-- src/libstore/local-store.cc | 16 +++++++-- src/libstore/local-store.hh | 7 ++-- src/libstore/s3-binary-cache-store.cc | 10 +++--- src/libstore/ssh-store-config.cc | 24 ++++++++++++++ src/libstore/ssh-store-config.hh | 23 +++++++++++++ src/libstore/ssh-store.cc | 26 +++++++++++---- src/libstore/ssh.cc | 6 +++- src/libstore/ssh.hh | 6 +++- src/libstore/store-api.cc | 42 ++---------------------- src/libstore/store-api.hh | 11 +++++-- src/libstore/uds-remote-store.cc | 8 +++-- src/libstore/uds-remote-store.hh | 5 ++- src/libstore/unix/local-overlay-store.hh | 2 +- tests/functional/read-only-store.sh | 5 ++- 19 files changed, 170 insertions(+), 75 deletions(-) create mode 100644 src/libstore/ssh-store-config.cc diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 30f23cff9..0d5d03091 100644 --- 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::string_view authority, const Params & params) : DummyStore(params) - { } + { + if (!authority.empty()) + 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 index 5da87e935..3328caef9 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -39,15 +39,20 @@ private: public: HttpBinaryCacheStore( - const std::string & scheme, - const Path & _cacheUri, + std::string_view scheme, + PathView _cacheUri, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) , HttpBinaryCacheStoreConfig(params) , Store(params) , BinaryCacheStore(params) - , cacheUri(scheme + "://" + _cacheUri) + , cacheUri( + std::string { scheme } + + "://" + + (!_cacheUri.empty() + ? _cacheUri + : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))) { while (!cacheUri.empty() && cacheUri.back() == '/') cacheUri.pop_back(); diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 9bc986bfa..c75d50ade 100644 --- 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::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 index 343823693..81872996a 100644 --- 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 uriSchemes() { return {"ssh"}; } - LegacySSHStore(const std::string & scheme, const std::string & host, const Params & params); + LegacySSHStore( + std::string_view scheme, + std::string_view host, + const Params & params); + +private: + LegacySSHStore( + std::string_view scheme, + std::string host, + const Params & params); +public: ref openConnection(); diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 87a6026f1..3e25ab8a4 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -28,9 +28,13 @@ private: public: + /** + * @param binaryCacheDir `file://` is a short-hand for `file:///` + * for now. + */ LocalBinaryCacheStore( - const std::string scheme, - const Path & binaryCacheDir, + std::string_view scheme, + PathView binaryCacheDir, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index dd06e5b65..c39060ba7 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -463,10 +463,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, + PathView path, + const Params & _params) + : LocalStore([&]{ + // Default `?root` from `path` if non set + if (!path.empty() && _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 index b3d7bd6d0..b0a0def9a 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -137,12 +137,15 @@ public: * necessary. */ LocalStore(const Params & params); - LocalStore(std::string scheme, std::string path, const Params & params); + LocalStore( + std::string_view scheme, + PathView path, + const Params & params); ~LocalStore(); static std::set 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 index 1a62d92d4..e9850dce6 100644 --- 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,8 +269,8 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3Helper s3Helper; S3BinaryCacheStoreImpl( - const std::string & uriScheme, - const std::string & bucketName, + std::string_view uriScheme, + std::string_view bucketName, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) @@ -281,6 +281,8 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual , bucketName(bucketName) , s3Helper(profile, region, scheme, endpoint) { + if (bucketName.empty()) + throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme); diskCache = getNarInfoDiskCache(); } diff --git a/src/libstore/ssh-store-config.cc b/src/libstore/ssh-store-config.cc new file mode 100644 index 000000000..742354cce --- /dev/null +++ b/src/libstore/ssh-store-config.cc @@ -0,0 +1,24 @@ +#include + +#include "ssh-store-config.hh" + +namespace nix { + +std::string CommonSSHStoreConfig::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); + + std::string connStr{_connStr}; + + 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 index 4ce4ffc4c..09f9f23a7 100644 --- 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::string_view connStr); }; } diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 220d5d31b..246abaac2 100644 --- 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 @@ public: , 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::string_view host, + const Params & params) + : SSHStore{scheme, SSHStoreConfig::extractConnStr(scheme, host), params} + { + } + static std::set 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::string_view host, + const Params & params) : StoreConfig(params) , RemoteStoreConfig(params) , CommonSSHStoreConfig(params) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 7e730299a..5eb63fa67 100644 --- 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 index 3b1a0827a..be0a32287 100644 --- a/src/libstore/ssh.hh +++ b/src/libstore/ssh.hh @@ -39,7 +39,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 { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 419c55e92..6eba3a77d 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -21,7 +21,6 @@ #include "users.hh" #include -#include using json = nlohmann::json; @@ -1321,9 +1320,7 @@ std::shared_ptr openFromNonUri(const std::string & uri, const Store::Para warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); } else debug("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); - Store::Params params2; - params2["root"] = chrootStore; - return std::make_shared(params2); + return std::make_shared("local", chrootStore, params); } #endif else @@ -1333,42 +1330,12 @@ std::shared_ptr openFromNonUri(const std::string & uri, const Store::Para } else if (uri == "local") { return std::make_shared(params); } else if (isNonUriPath(uri)) { - Store::Params params2 = params; - params2["root"] = absPath(uri); - return std::make_shared(params2); + return std::make_shared("local", absPath(uri), params); } else { return nullptr; } } -// 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. -static std::string extractConnStr(const std::string &proto, const std::string &connStr) -{ - if (proto.rfind("ssh") != std::string::npos) { - std::smatch result; - std::regex v6AddrRegex("^((.*)@)?\\[(.*)\\]$"); - - if (std::regex_match(connStr, result, v6AddrRegex)) { - if (result[1].matched) { - return result.str(1) + result.str(3); - } - return result.str(3); - } - } - - return connStr; -} - ref openStore(const std::string & uri_, const Store::Params & extraParams) { @@ -1377,10 +1344,7 @@ ref openStore(const std::string & uri_, auto parsedUri = parseURL(uri_); params.insert(parsedUri.query.begin(), parsedUri.query.end()); - auto baseURI = extractConnStr( - parsedUri.scheme, - parsedUri.authority.value_or("") + parsedUri.path - ); + auto baseURI = parsedUri.authority.value_or("") + parsedUri.path; for (auto implem : *Implementations::registered) { if (implem.uriSchemes.count(parsedUri.scheme)) { diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index ae8c22437..a508e5a00 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -901,7 +901,14 @@ std::list> getDefaultSubstituters(); struct StoreFactory { std::set uriSchemes; - std::function (const std::string & scheme, const std::string & uri, const Store::Params & params)> create; + /** + * The `authorityPath` parameter is `/`, or really + * whatever comes after `://` and before `?`. + */ + std::function ( + std::string_view scheme, + std::string_view authorityPath, + const Store::Params & params)> create; std::function ()> getConfig; }; @@ -916,7 +923,7 @@ struct Implementations StoreFactory factory{ .uriSchemes = T::uriSchemes(), .create = - ([](const std::string & scheme, const std::string & uri, const Store::Params & params) + ([](auto scheme, auto uri, auto & params) -> std::shared_ptr { return std::make_shared(scheme, uri, params); }), .getConfig = diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 649644146..499f76967 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -40,12 +40,13 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) UDSRemoteStore::UDSRemoteStore( - const std::string scheme, - std::string socket_path, + std::string_view scheme, + PathView socket_path, const Params & params) : UDSRemoteStore(params) { - path.emplace(socket_path); + if (!socket_path.empty()) + path.emplace(socket_path); } @@ -54,6 +55,7 @@ std::string UDSRemoteStore::getUri() if (path) { return std::string("unix://") + *path; } else { + // unix:// with no path also works. Change what we return? return "daemon"; } } diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh index 8bce8994a..6f0494bb6 100644 --- a/src/libstore/uds-remote-store.hh +++ b/src/libstore/uds-remote-store.hh @@ -28,7 +28,10 @@ class UDSRemoteStore : public virtual UDSRemoteStoreConfig public: UDSRemoteStore(const Params & params); - UDSRemoteStore(const std::string scheme, std::string path, const Params & params); + UDSRemoteStore( + std::string_view scheme, + PathView path, + const Params & params); std::string getUri() override; diff --git a/src/libstore/unix/local-overlay-store.hh b/src/libstore/unix/local-overlay-store.hh index 2c24285dd..35a301013 100644 --- a/src/libstore/unix/local-overlay-store.hh +++ b/src/libstore/unix/local-overlay-store.hh @@ -92,7 +92,7 @@ class LocalOverlayStore : public virtual LocalOverlayStoreConfig, public virtual public: LocalOverlayStore(const Params & params); - LocalOverlayStore(std::string scheme, std::string path, const Params & params) + LocalOverlayStore(std::string_view scheme, PathView path, const Params & params) : LocalOverlayStore(params) { if (!path.empty()) diff --git a/tests/functional/read-only-store.sh b/tests/functional/read-only-store.sh index d63920c19..834ac1b51 100644 --- a/tests/functional/read-only-store.sh +++ b/tests/functional/read-only-store.sh @@ -9,7 +9,10 @@ clearStore happy () { # We can do a read-only query just fine with a read-only store nix --store local?read-only=true path-info $dummyPath - + + # `local://` also works. + nix --store local://?read-only=true path-info $dummyPath + # We can "write" an already-present store-path a read-only store, because no IO is actually required nix-store --store local?read-only=true --add dummy }