From 57399bfc0e438d699700c6a4f2b6b63b157bcd2a Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 17 Jul 2024 23:32:27 -0400 Subject: [PATCH] Refactor unix domain socket store config (#11109) Following what is outlined in #10766 refactor the uds-remote-store such that the member variables (state) don't live in the store itself but in the config object. Additionally, the config object includes a new necessary constructor that takes a scheme & authority. Tests are commented out because of linking errors with the current config system. When there is a new config system we can reenable them. Co-authored-by: John Ericson --- src/libstore/dummy-store.cc | 20 ++-- src/libstore/http-binary-cache-store.cc | 50 +++++----- src/libstore/http-binary-cache-store.hh | 21 +++++ src/libstore/local-binary-cache-store.cc | 36 ++++--- src/libstore/local-binary-cache-store.hh | 21 +++++ src/libstore/local-fs-store.cc | 14 +++ src/libstore/local-fs-store.hh | 9 ++ src/libstore/local-overlay-store.cc | 6 +- src/libstore/local-overlay-store.hh | 21 +++-- src/libstore/local-store.cc | 33 +++---- src/libstore/local-store.hh | 5 + src/libstore/meson.build | 2 + src/libstore/s3-binary-cache-store.cc | 94 +++++-------------- src/libstore/s3-binary-cache-store.hh | 90 ++++++++++++++++++ src/libstore/ssh-store.cc | 55 +++++------ src/libstore/ssh-store.hh | 23 +++++ src/libstore/uds-remote-store.cc | 54 ++++++----- src/libstore/uds-remote-store.hh | 41 ++++++-- .../unit/libstore/http-binary-cache-store.cc | 21 +++++ .../unit/libstore/local-binary-cache-store.cc | 14 +++ tests/unit/libstore/local-overlay-store.cc | 34 +++++++ tests/unit/libstore/local-store.cc | 40 ++++++++ tests/unit/libstore/meson.build | 6 ++ tests/unit/libstore/s3-binary-cache-store.cc | 18 ++++ tests/unit/libstore/ssh-store.cc | 35 ++++++- tests/unit/libstore/uds-remote-store.cc | 23 +++++ 26 files changed, 571 insertions(+), 215 deletions(-) create mode 100644 src/libstore/http-binary-cache-store.hh create mode 100644 src/libstore/local-binary-cache-store.hh create mode 100644 tests/unit/libstore/http-binary-cache-store.cc create mode 100644 tests/unit/libstore/local-binary-cache-store.cc create mode 100644 tests/unit/libstore/local-overlay-store.cc create mode 100644 tests/unit/libstore/local-store.cc create mode 100644 tests/unit/libstore/s3-binary-cache-store.cc create mode 100644 tests/unit/libstore/uds-remote-store.cc diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 17ebaace6..af0dd9092 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -6,6 +6,13 @@ namespace nix { struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; + DummyStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) + : StoreConfig(params) + { + if (!authority.empty()) + throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority); + } + const std::string name() override { return "Dummy Store"; } std::string doc() override @@ -19,16 +26,13 @@ struct DummyStoreConfig : virtual StoreConfig { struct DummyStore : public virtual DummyStoreConfig, public virtual Store { 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); - } + : StoreConfig(params) + , DummyStoreConfig(scheme, authority, params) + , Store(params) + { } DummyStore(const Params & params) - : StoreConfig(params) - , DummyStoreConfig(params) - , Store(params) + : DummyStore("dummy", "", params) { } std::string getUri() override diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 3328caef9..49ec26b9f 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -1,4 +1,4 @@ -#include "binary-cache-store.hh" +#include "http-binary-cache-store.hh" #include "filetransfer.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" @@ -8,26 +8,37 @@ namespace nix { MakeError(UploadToHTTP, Error); -struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig + +HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig( + std::string_view scheme, + std::string_view _cacheUri, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , cacheUri( + std::string { scheme } + + "://" + + (!_cacheUri.empty() + ? _cacheUri + : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + while (!cacheUri.empty() && cacheUri.back() == '/') + cacheUri.pop_back(); +} - const std::string name() override { return "HTTP Binary Cache Store"; } - std::string doc() override - { - return - #include "http-binary-cache-store.md" - ; - } -}; +std::string HttpBinaryCacheStoreConfig::doc() +{ + return + #include "http-binary-cache-store.md" + ; +} + class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public virtual BinaryCacheStore { private: - Path cacheUri; - struct State { bool enabled = true; @@ -40,23 +51,14 @@ public: HttpBinaryCacheStore( std::string_view scheme, - PathView _cacheUri, + PathView cacheUri, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , HttpBinaryCacheStoreConfig(params) + , HttpBinaryCacheStoreConfig(scheme, cacheUri, params) , Store(params) , BinaryCacheStore(params) - , 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(); - diskCache = getNarInfoDiskCache(); } diff --git a/src/libstore/http-binary-cache-store.hh b/src/libstore/http-binary-cache-store.hh new file mode 100644 index 000000000..230e52b33 --- /dev/null +++ b/src/libstore/http-binary-cache-store.hh @@ -0,0 +1,21 @@ +#include "binary-cache-store.hh" + +namespace nix { + +struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + HttpBinaryCacheStoreConfig(std::string_view scheme, std::string_view _cacheUri, const Params & params); + + Path cacheUri; + + const std::string name() override + { + return "HTTP Binary Cache Store"; + } + + std::string doc() override; +}; + +} diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index aa2efdb8f..106663132 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -1,4 +1,4 @@ -#include "binary-cache-store.hh" +#include "local-binary-cache-store.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" #include "signals.hh" @@ -7,28 +7,27 @@ namespace nix { -struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +LocalBinaryCacheStoreConfig::LocalBinaryCacheStoreConfig( + std::string_view scheme, + PathView binaryCacheDir, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , binaryCacheDir(binaryCacheDir) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; +} - const std::string name() override { return "Local Binary Cache Store"; } - std::string doc() override - { - return - #include "local-binary-cache-store.md" - ; - } -}; - -class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore +std::string LocalBinaryCacheStoreConfig::doc() { -private: + return + #include "local-binary-cache-store.md" + ; +} - Path binaryCacheDir; - -public: +struct LocalBinaryCacheStore : virtual LocalBinaryCacheStoreConfig, virtual BinaryCacheStore +{ /** * @param binaryCacheDir `file://` is a short-hand for `file:///` * for now. @@ -39,10 +38,9 @@ public: const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , LocalBinaryCacheStoreConfig(params) + , LocalBinaryCacheStoreConfig(scheme, binaryCacheDir, params) , Store(params) , BinaryCacheStore(params) - , binaryCacheDir(binaryCacheDir) { } diff --git a/src/libstore/local-binary-cache-store.hh b/src/libstore/local-binary-cache-store.hh new file mode 100644 index 000000000..7701eb38b --- /dev/null +++ b/src/libstore/local-binary-cache-store.hh @@ -0,0 +1,21 @@ +#include "binary-cache-store.hh" + +namespace nix { + +struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + LocalBinaryCacheStoreConfig(std::string_view scheme, PathView binaryCacheDir, const Params & params); + + Path binaryCacheDir; + + const std::string name() override + { + return "Local Binary Cache Store"; + } + + std::string doc() override; +}; + +} diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 843c0d288..5449b20eb 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -8,6 +8,20 @@ namespace nix { +LocalFSStoreConfig::LocalFSStoreConfig(PathView rootDir, const Params & params) + : StoreConfig(params) + // Default `?root` from `rootDir` if non set + // FIXME don't duplicate description once we don't have root setting + , rootDir{ + this, + !rootDir.empty() && params.count("root") == 0 + ? (std::optional{rootDir}) + : std::nullopt, + "root", + "Directory prefixed to all other paths."} +{ +} + LocalFSStore::LocalFSStore(const Params & params) : Store(params) { diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index 8fb081200..9bb569f0b 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -11,6 +11,15 @@ struct LocalFSStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; + /** + * Used to override the `root` settings. Can't be done via modifying + * `params` reliably because this parameter is unused except for + * passing to base class constructors. + * + * @todo Make this less error-prone with new store settings system. + */ + LocalFSStoreConfig(PathView path, const Params & params); + const OptionalPathSetting rootDir{this, std::nullopt, "root", "Directory prefixed to all other paths."}; diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index 598415db8..ec2c5f4e9 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -18,11 +18,11 @@ Path LocalOverlayStoreConfig::toUpperPath(const StorePath & path) { return upperLayer + "/" + path.to_string(); } -LocalOverlayStore::LocalOverlayStore(const Params & params) +LocalOverlayStore::LocalOverlayStore(std::string_view scheme, PathView path, const Params & params) : StoreConfig(params) - , LocalFSStoreConfig(params) + , LocalFSStoreConfig(path, params) , LocalStoreConfig(params) - , LocalOverlayStoreConfig(params) + , LocalOverlayStoreConfig(scheme, path, params) , Store(params) , LocalFSStore(params) , LocalStore(params) diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh index 35a301013..a3f15e484 100644 --- a/src/libstore/local-overlay-store.hh +++ b/src/libstore/local-overlay-store.hh @@ -8,11 +8,16 @@ namespace nix { struct LocalOverlayStoreConfig : virtual LocalStoreConfig { LocalOverlayStoreConfig(const StringMap & params) - : StoreConfig(params) - , LocalFSStoreConfig(params) - , LocalStoreConfig(params) + : LocalOverlayStoreConfig("local-overlay", "", params) { } + LocalOverlayStoreConfig(std::string_view scheme, PathView path, const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(path, params) + , LocalStoreConfig(scheme, path, params) + { + } + const Setting lowerStoreUri{(StoreConfig*) this, "", "lower-store", R"( [Store URL](@docroot@/command-ref/new-cli/nix3-help-stores.md#store-url-format) @@ -90,15 +95,13 @@ class LocalOverlayStore : public virtual LocalOverlayStoreConfig, public virtual ref lowerStore; public: - LocalOverlayStore(const Params & params); - - LocalOverlayStore(std::string_view scheme, PathView path, const Params & params) - : LocalOverlayStore(params) + LocalOverlayStore(const Params & params) + : LocalOverlayStore("local-overlay", "", params) { - if (!path.empty()) - throw UsageError("local-overlay:// store url doesn't support path part, only scheme and query params"); } + LocalOverlayStore(std::string_view scheme, PathView path, const Params & params); + static std::set uriSchemes() { return { "local-overlay" }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 82b70ff21..819cee345 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -56,6 +56,15 @@ namespace nix { +LocalStoreConfig::LocalStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(authority, params) +{ +} + std::string LocalStoreConfig::doc() { return @@ -183,10 +192,13 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) } } -LocalStore::LocalStore(const Params & params) +LocalStore::LocalStore( + std::string_view scheme, + PathView path, + const Params & params) : StoreConfig(params) - , LocalFSStoreConfig(params) - , LocalStoreConfig(params) + , LocalFSStoreConfig(path, params) + , LocalStoreConfig(scheme, path, params) , Store(params) , LocalFSStore(params) , dbDir(stateDir + "/db") @@ -465,19 +477,8 @@ LocalStore::LocalStore(const Params & 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; - }()) +LocalStore::LocalStore(const Params & params) + : LocalStore("local", "", params) { } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index b0a0def9a..026729bf2 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -38,6 +38,11 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; + LocalStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params); + Setting requireSigs{this, settings.requireSigs, "require-sigs", diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 297bf7187..29ee95b75 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -243,10 +243,12 @@ headers = [config_h] + files( 'filetransfer.hh', 'gc-store.hh', 'globals.hh', + 'http-binary-cache-store.hh', 'indirect-root-store.hh', 'keys.hh', 'legacy-ssh-store.hh', 'length-prefixed-protocol-helper.hh', + 'local-binary-cache-store.hh', 'local-fs-store.hh', 'local-overlay-store.hh', 'local-store.hh', diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 27013c0f1..d865684d7 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -1,5 +1,7 @@ #if ENABLE_S3 +#include + #include "s3.hh" #include "s3-binary-cache-store.hh" #include "nar-info.hh" @@ -190,76 +192,31 @@ S3BinaryCacheStore::S3BinaryCacheStore(const Params & params) , BinaryCacheStore(params) { } -struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig + +S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( + std::string_view uriScheme, + std::string_view bucketName, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , bucketName(bucketName) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + // Don't want to use use AWS SDK in header, so we check the default + // here. TODO do this better after we overhaul the store settings + // system. + assert(std::string{defaultRegion} == std::string{Aws::Region::US_EAST_1}); - const Setting profile{this, "", "profile", - R"( - The name of the AWS configuration profile to use. By default - Nix will use the `default` profile. - )"}; + if (bucketName.empty()) + throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme); +} - const Setting region{this, Aws::Region::US_EAST_1, "region", - R"( - The region of the S3 bucket. If your bucket is not in - `us–east-1`, you should always explicitly specify the region - parameter. - )"}; +std::string S3BinaryCacheStoreConfig::doc() +{ + return + #include "s3-binary-cache-store.md" + ; +} - const Setting scheme{this, "", "scheme", - R"( - The scheme used for S3 requests, `https` (default) or `http`. This - option allows you to disable HTTPS for binary caches which don't - support it. - - > **Note** - > - > HTTPS should be used if the cache might contain sensitive - > information. - )"}; - - const Setting endpoint{this, "", "endpoint", - R"( - The URL of the endpoint of an S3-compatible service such as MinIO. - 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. - )"}; - - const Setting narinfoCompression{this, "", "narinfo-compression", - "Compression method for `.narinfo` files."}; - - const Setting lsCompression{this, "", "ls-compression", - "Compression method for `.ls` files."}; - - const Setting logCompression{this, "", "log-compression", - R"( - Compression method for `log/*` files. It is recommended to - use a compression method supported by most web browsers - (e.g. `brotli`). - )"}; - - const Setting multipartUpload{ - this, false, "multipart-upload", - "Whether to use multi-part uploads."}; - - const Setting bufferSize{ - this, 5 * 1024 * 1024, "buffer-size", - "Size (in bytes) of each part in multi-part uploads."}; - - const std::string name() override { return "S3 Binary Cache Store"; } - - std::string doc() override - { - return - #include "s3-binary-cache-store.md" - ; - } -}; struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore { @@ -275,15 +232,12 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , S3BinaryCacheStoreConfig(params) + , S3BinaryCacheStoreConfig(uriScheme, bucketName, params) , Store(params) , BinaryCacheStore(params) , S3BinaryCacheStore(params) - , 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/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index c62ea5147..bca5196cd 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -7,6 +7,96 @@ namespace nix { +struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + std::string bucketName; + + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + S3BinaryCacheStoreConfig(std::string_view uriScheme, std::string_view bucketName, const Params & params); + + const Setting profile{ + this, + "", + "profile", + R"( + The name of the AWS configuration profile to use. By default + Nix will use the `default` profile. + )"}; + +protected: + + constexpr static const char * defaultRegion = "us-east-1"; + +public: + + const Setting region{ + this, + defaultRegion, + "region", + R"( + The region of the S3 bucket. If your bucket is not in + `us–east-1`, you should always explicitly specify the region + parameter. + )"}; + + const Setting scheme{ + this, + "", + "scheme", + R"( + The scheme used for S3 requests, `https` (default) or `http`. This + option allows you to disable HTTPS for binary caches which don't + support it. + + > **Note** + > + > HTTPS should be used if the cache might contain sensitive + > information. + )"}; + + const Setting endpoint{ + this, + "", + "endpoint", + R"( + The URL of the endpoint of an S3-compatible service such as MinIO. + 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. + )"}; + + const Setting narinfoCompression{ + this, "", "narinfo-compression", "Compression method for `.narinfo` files."}; + + const Setting lsCompression{this, "", "ls-compression", "Compression method for `.ls` files."}; + + const Setting logCompression{ + this, + "", + "log-compression", + R"( + Compression method for `log/*` files. It is recommended to + use a compression method supported by most web browsers + (e.g. `brotli`). + )"}; + + const Setting multipartUpload{this, false, "multipart-upload", "Whether to use multi-part uploads."}; + + const Setting bufferSize{ + this, 5 * 1024 * 1024, "buffer-size", "Size (in bytes) of each part in multi-part uploads."}; + + const std::string name() override + { + return "S3 Binary Cache Store"; + } + + std::string doc() override; +}; + class S3BinaryCacheStore : public virtual BinaryCacheStore { protected: diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index b21c22d7e..d63995bf3 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -89,43 +89,32 @@ protected: }; }; -struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig + +MountedSSHStoreConfig::MountedSSHStoreConfig(StringMap params) + : StoreConfig(params) + , RemoteStoreConfig(params) + , CommonSSHStoreConfig(params) + , SSHStoreConfig(params) + , LocalFSStoreConfig(params) { - using SSHStoreConfig::SSHStoreConfig; - using LocalFSStoreConfig::LocalFSStoreConfig; +} - MountedSSHStoreConfig(StringMap params) - : StoreConfig(params) - , RemoteStoreConfig(params) - , CommonSSHStoreConfig(params) - , SSHStoreConfig(params) - , LocalFSStoreConfig(params) - { - } +MountedSSHStoreConfig::MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params) + : StoreConfig(params) + , RemoteStoreConfig(params) + , CommonSSHStoreConfig(scheme, host, params) + , SSHStoreConfig(params) + , LocalFSStoreConfig(params) +{ +} - MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params) - : StoreConfig(params) - , RemoteStoreConfig(params) - , CommonSSHStoreConfig(scheme, host, params) - , SSHStoreConfig(params) - , LocalFSStoreConfig(params) - { - } +std::string MountedSSHStoreConfig::doc() +{ + return + #include "mounted-ssh-store.md" + ; +} - const std::string name() override { return "Experimental SSH Store with filesystem mounted"; } - - std::string doc() override - { - return - #include "mounted-ssh-store.md" - ; - } - - std::optional experimentalFeature() const override - { - return ExperimentalFeature::MountedSSHStore; - } -}; /** * The mounted ssh store assumes that filesystems on the remote host are diff --git a/src/libstore/ssh-store.hh b/src/libstore/ssh-store.hh index 6ef2219a2..feb57ccba 100644 --- a/src/libstore/ssh-store.hh +++ b/src/libstore/ssh-store.hh @@ -3,6 +3,7 @@ #include "common-ssh-store-config.hh" #include "store-api.hh" +#include "local-fs-store.hh" #include "remote-store.hh" namespace nix { @@ -25,4 +26,26 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig std::string doc() override; }; +struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig +{ + using LocalFSStoreConfig::LocalFSStoreConfig; + using SSHStoreConfig::SSHStoreConfig; + + MountedSSHStoreConfig(StringMap params); + + MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params); + + const std::string name() override + { + return "Experimental SSH Store with filesystem mounted"; + } + + std::string doc() override; + + std::optional experimentalFeature() const override + { + return ExperimentalFeature::MountedSSHStore; + } +}; + } diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 499f76967..3c445eb13 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -2,10 +2,8 @@ #include "unix-domain-socket.hh" #include "worker-protocol.hh" -#include #include #include -#include #include #include @@ -19,6 +17,21 @@ namespace nix { +UDSRemoteStoreConfig::UDSRemoteStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(params) + , RemoteStoreConfig(params) + , path{authority.empty() ? settings.nixDaemonSocketFile : authority} +{ + if (scheme != UDSRemoteStoreConfig::scheme) { + throw UsageError("Scheme must be 'unix'"); + } +} + + std::string UDSRemoteStoreConfig::doc() { return @@ -27,11 +40,20 @@ std::string UDSRemoteStoreConfig::doc() } +// A bit gross that we now pass empty string but this is knowing that +// empty string will later default to the same nixDaemonSocketFile. Why +// don't we just wire it all through? I believe there are cases where it +// will live reload so we want to continue to account for that. UDSRemoteStore::UDSRemoteStore(const Params & params) + : UDSRemoteStore(scheme, "", params) +{} + + +UDSRemoteStore::UDSRemoteStore(std::string_view scheme, std::string_view authority, const Params & params) : StoreConfig(params) , LocalFSStoreConfig(params) , RemoteStoreConfig(params) - , UDSRemoteStoreConfig(params) + , UDSRemoteStoreConfig(scheme, authority, params) , Store(params) , LocalFSStore(params) , RemoteStore(params) @@ -39,25 +61,15 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) } -UDSRemoteStore::UDSRemoteStore( - std::string_view scheme, - PathView socket_path, - const Params & params) - : UDSRemoteStore(params) -{ - if (!socket_path.empty()) - path.emplace(socket_path); -} - - std::string UDSRemoteStore::getUri() { - if (path) { - return std::string("unix://") + *path; - } else { - // unix:// with no path also works. Change what we return? - return "daemon"; - } + return path == settings.nixDaemonSocketFile + ? // FIXME: Not clear why we return daemon here and not default + // to settings.nixDaemonSocketFile + // + // unix:// with no path also works. Change what we return? + "daemon" + : std::string(scheme) + "://" + path; } @@ -74,7 +86,7 @@ ref UDSRemoteStore::openConnection() /* Connect to a daemon that does the privileged work for us. */ conn->fd = createUnixDomainSocket(); - nix::connect(toSocket(conn->fd.get()), path ? *path : settings.nixDaemonSocketFile); + nix::connect(toSocket(conn->fd.get()), path); conn->from.fd = conn->fd.get(); conn->to.fd = conn->fd.get(); diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh index 6f0494bb6..0e9542eab 100644 --- a/src/libstore/uds-remote-store.hh +++ b/src/libstore/uds-remote-store.hh @@ -9,16 +9,33 @@ namespace nix { struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig { - UDSRemoteStoreConfig(const Params & params) - : StoreConfig(params) - , LocalFSStoreConfig(params) - , RemoteStoreConfig(params) - { - } + // TODO(fzakaria): Delete this constructor once moved over to the factory pattern + // outlined in https://github.com/NixOS/nix/issues/10766 + using LocalFSStoreConfig::LocalFSStoreConfig; + using RemoteStoreConfig::RemoteStoreConfig; + + /** + * @param authority is the socket path. + */ + UDSRemoteStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params); const std::string name() override { return "Local Daemon Store"; } std::string doc() override; + + /** + * The path to the unix domain socket. + * + * The default is `settings.nixDaemonSocketFile`, but we don't write + * that below, instead putting in the constructor. + */ + Path path; + +protected: + static constexpr char const * scheme = "unix"; }; class UDSRemoteStore : public virtual UDSRemoteStoreConfig @@ -27,16 +44,23 @@ class UDSRemoteStore : public virtual UDSRemoteStoreConfig { public: + /** + * @deprecated This is the old API to construct the store. + */ UDSRemoteStore(const Params & params); + + /** + * @param authority is the socket path. + */ UDSRemoteStore( std::string_view scheme, - PathView path, + std::string_view authority, const Params & params); std::string getUri() override; static std::set uriSchemes() - { return {"unix"}; } + { return {scheme}; } ref getFSAccessor(bool requireValidPath = true) override { return LocalFSStore::getFSAccessor(requireValidPath); } @@ -63,7 +87,6 @@ private: }; ref openConnection() override; - std::optional path; }; } diff --git a/tests/unit/libstore/http-binary-cache-store.cc b/tests/unit/libstore/http-binary-cache-store.cc new file mode 100644 index 000000000..1e415f625 --- /dev/null +++ b/tests/unit/libstore/http-binary-cache-store.cc @@ -0,0 +1,21 @@ +#include + +#include "http-binary-cache-store.hh" + +namespace nix { + +TEST(HttpBinaryCacheStore, constructConfig) +{ + HttpBinaryCacheStoreConfig config{"http", "foo.bar.baz", {}}; + + EXPECT_EQ(config.cacheUri, "http://foo.bar.baz"); +} + +TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash) +{ + HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", {}}; + + EXPECT_EQ(config.cacheUri, "https://foo.bar.baz/a/b"); +} + +} // namespace nix diff --git a/tests/unit/libstore/local-binary-cache-store.cc b/tests/unit/libstore/local-binary-cache-store.cc new file mode 100644 index 000000000..2e840228d --- /dev/null +++ b/tests/unit/libstore/local-binary-cache-store.cc @@ -0,0 +1,14 @@ +#include + +#include "local-binary-cache-store.hh" + +namespace nix { + +TEST(LocalBinaryCacheStore, constructConfig) +{ + LocalBinaryCacheStoreConfig config{"local", "/foo/bar/baz", {}}; + + EXPECT_EQ(config.binaryCacheDir, "/foo/bar/baz"); +} + +} // namespace nix diff --git a/tests/unit/libstore/local-overlay-store.cc b/tests/unit/libstore/local-overlay-store.cc new file mode 100644 index 000000000..b34ca9237 --- /dev/null +++ b/tests/unit/libstore/local-overlay-store.cc @@ -0,0 +1,34 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "local-overlay-store.hh" + +namespace nix { + +TEST(LocalOverlayStore, constructConfig_rootQueryParam) +{ + LocalOverlayStoreConfig config{ + "local-overlay", + "", + { + { + "root", + "/foo/bar", + }, + }, + }; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +TEST(LocalOverlayStore, constructConfig_rootPath) +{ + LocalOverlayStoreConfig config{"local-overlay", "/foo/bar", {}}; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +} // namespace nix +#endif diff --git a/tests/unit/libstore/local-store.cc b/tests/unit/libstore/local-store.cc new file mode 100644 index 000000000..abc3ea796 --- /dev/null +++ b/tests/unit/libstore/local-store.cc @@ -0,0 +1,40 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "local-store.hh" + +// Needed for template specialisations. This is not good! When we +// overhaul how store configs work, this should be fixed. +# include "args.hh" +# include "config-impl.hh" +# include "abstract-setting-to-json.hh" + +namespace nix { + +TEST(LocalStore, constructConfig_rootQueryParam) +{ + LocalStoreConfig config{ + "local", + "", + { + { + "root", + "/foo/bar", + }, + }, + }; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +TEST(LocalStore, constructConfig_rootPath) +{ + LocalStoreConfig config{"local", "/foo/bar", {}}; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +} // namespace nix +#endif diff --git a/tests/unit/libstore/meson.build b/tests/unit/libstore/meson.build index 41b2fb0ab..8534ba8c5 100644 --- a/tests/unit/libstore/meson.build +++ b/tests/unit/libstore/meson.build @@ -58,7 +58,11 @@ sources = files( 'derivation.cc', 'derived-path.cc', 'downstream-placeholder.cc', + 'http-binary-cache-store.cc', 'legacy-ssh-store.cc', + 'local-binary-cache-store.cc', + 'local-overlay-store.cc', + 'local-store.cc', 'machines.cc', 'nar-info-disk-cache.cc', 'nar-info.cc', @@ -67,9 +71,11 @@ sources = files( 'path-info.cc', 'path.cc', 'references.cc', + 's3-binary-cache-store.cc', 'serve-protocol.cc', 'ssh-store.cc', 'store-reference.cc', + 'uds-remote-store.cc', 'worker-protocol.cc', ) diff --git a/tests/unit/libstore/s3-binary-cache-store.cc b/tests/unit/libstore/s3-binary-cache-store.cc new file mode 100644 index 000000000..7aa5f2f2c --- /dev/null +++ b/tests/unit/libstore/s3-binary-cache-store.cc @@ -0,0 +1,18 @@ +#if ENABLE_S3 + +# include + +# include "s3-binary-cache-store.hh" + +namespace nix { + +TEST(S3BinaryCacheStore, constructConfig) +{ + S3BinaryCacheStoreConfig config{"s3", "foobar", {}}; + + EXPECT_EQ(config.bucketName, "foobar"); +} + +} // namespace nix + +#endif diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc index 7010ad157..b853a5f1f 100644 --- a/tests/unit/libstore/ssh-store.cc +++ b/tests/unit/libstore/ssh-store.cc @@ -1,6 +1,9 @@ -#include +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include -#include "ssh-store.hh" +# include "ssh-store.hh" namespace nix { @@ -15,7 +18,9 @@ TEST(SSHStore, constructConfig) // TODO #11106, no more split on space "foo bar", }, - }}; + }, + }; + EXPECT_EQ( config.remoteProgram.get(), (Strings{ @@ -23,4 +28,28 @@ TEST(SSHStore, constructConfig) "bar", })); } + +TEST(MountedSSHStore, constructConfig) +{ + MountedSSHStoreConfig config{ + "mounted-ssh", + "localhost", + StoreConfig::Params{ + { + "remote-program", + // TODO #11106, no more split on space + "foo bar", + }, + }, + }; + + EXPECT_EQ( + config.remoteProgram.get(), + (Strings{ + "foo", + "bar", + })); } + +} +#endif diff --git a/tests/unit/libstore/uds-remote-store.cc b/tests/unit/libstore/uds-remote-store.cc new file mode 100644 index 000000000..5ccb20871 --- /dev/null +++ b/tests/unit/libstore/uds-remote-store.cc @@ -0,0 +1,23 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "uds-remote-store.hh" + +namespace nix { + +TEST(UDSRemoteStore, constructConfig) +{ + UDSRemoteStoreConfig config{"unix", "/tmp/socket", {}}; + + EXPECT_EQ(config.path, "/tmp/socket"); +} + +TEST(UDSRemoteStore, constructConfigWrongScheme) +{ + EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError); +} + +} // namespace nix +#endif