From 0855b715a97a44cbcb23492c94ed91fcf7162c4d Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Thu, 9 Oct 2025 02:42:14 +0000 Subject: [PATCH] feat(libstore): add curl-based S3 store implementation Add a new S3BinaryCacheStore implementation that inherits from HttpBinaryCacheStore. The implementation is activated with NIX_WITH_CURL_S3, keeping the existing NIX_WITH_S3_SUPPORT (AWS SDK) implementation unchanged. --- src/libstore-tests/s3-binary-cache-store.cc | 127 ++++++++++++++++++ .../nix/store/s3-binary-cache-store.hh | 75 +++++++++++ src/libstore/s3-binary-cache-store.cc | 46 +++++++ 3 files changed, 248 insertions(+) diff --git a/src/libstore-tests/s3-binary-cache-store.cc b/src/libstore-tests/s3-binary-cache-store.cc index 251e96172..8c58b8408 100644 --- a/src/libstore-tests/s3-binary-cache-store.cc +++ b/src/libstore-tests/s3-binary-cache-store.cc @@ -15,4 +15,131 @@ TEST(S3BinaryCacheStore, constructConfig) } // namespace nix +#elif NIX_WITH_CURL_S3 + +# include "nix/store/http-binary-cache-store.hh" +# include "nix/store/filetransfer.hh" +# include "nix/store/s3-url.hh" + +# include + +namespace nix { + +TEST(S3BinaryCacheStore, constructConfig) +{ + S3BinaryCacheStoreConfig config{"s3", "foobar", {}}; + + // The bucket name is stored as the host part of the authority in cacheUri + EXPECT_EQ( + config.cacheUri, + (ParsedURL{ + .scheme = "s3", + .authority = ParsedURL::Authority{.host = "foobar"}, + })); +} + +TEST(S3BinaryCacheStore, constructConfigWithRegion) +{ + Store::Config::Params params{{"region", "eu-west-1"}}; + S3BinaryCacheStoreConfig config{"s3", "my-bucket", params}; + + EXPECT_EQ( + config.cacheUri, + (ParsedURL{ + .scheme = "s3", + .authority = ParsedURL::Authority{.host = "my-bucket"}, + .query = (StringMap) {{"region", "eu-west-1"}}, + })); + EXPECT_EQ(config.region.get(), "eu-west-1"); +} + +TEST(S3BinaryCacheStore, defaultSettings) +{ + S3BinaryCacheStoreConfig config{"s3", "test-bucket", {}}; + + EXPECT_EQ( + config.cacheUri, + (ParsedURL{ + .scheme = "s3", + .authority = ParsedURL::Authority{.host = "test-bucket"}, + })); + + // Check default values + EXPECT_EQ(config.region.get(), "us-east-1"); + EXPECT_EQ(config.profile.get(), "default"); + EXPECT_EQ(config.scheme.get(), "https"); + EXPECT_EQ(config.endpoint.get(), ""); +} + +/** + * Test that S3BinaryCacheStore properly preserves S3-specific parameters + */ +TEST(S3BinaryCacheStore, s3StoreConfigPreservesParameters) +{ + StringMap params; + params["region"] = "eu-west-1"; + params["endpoint"] = "custom.s3.com"; + + S3BinaryCacheStoreConfig config("s3", "test-bucket", params); + + // The config should preserve S3-specific parameters + EXPECT_EQ( + config.cacheUri, + (ParsedURL{ + .scheme = "s3", + .authority = ParsedURL::Authority{.host = "test-bucket"}, + .query = (StringMap) {{"region", "eu-west-1"}, {"endpoint", "custom.s3.com"}}, + })); +} + +/** + * Test that S3 store scheme is properly registered + */ +TEST(S3BinaryCacheStore, s3SchemeRegistration) +{ + auto schemes = S3BinaryCacheStoreConfig::uriSchemes(); + EXPECT_TRUE(schemes.count("s3") > 0) << "S3 scheme should be supported"; + + // Verify HttpBinaryCacheStoreConfig doesn't directly list S3 + auto httpSchemes = HttpBinaryCacheStoreConfig::uriSchemes(); + EXPECT_FALSE(httpSchemes.count("s3") > 0) << "HTTP store shouldn't directly list S3 scheme"; +} + +/** + * Test that only S3-specific parameters are preserved in cacheUri, + * while non-S3 store parameters are not propagated to the URL + */ +TEST(S3BinaryCacheStore, parameterFiltering) +{ + StringMap params; + params["region"] = "eu-west-1"; + params["endpoint"] = "minio.local"; + params["want-mass-query"] = "true"; // Non-S3 store parameter + params["priority"] = "10"; // Non-S3 store parameter + + S3BinaryCacheStoreConfig config("s3", "test-bucket", params); + + // Only S3-specific params should be in cacheUri.query + EXPECT_EQ( + config.cacheUri, + (ParsedURL{ + .scheme = "s3", + .authority = ParsedURL::Authority{.host = "test-bucket"}, + .query = (StringMap) {{"region", "eu-west-1"}, {"endpoint", "minio.local"}}, + })); + + // But the non-S3 params should still be set on the config + EXPECT_EQ(config.wantMassQuery.get(), true); + EXPECT_EQ(config.priority.get(), 10); + + // And all params (S3 and non-S3) should be returned by getReference() + auto ref = config.getReference(); + EXPECT_EQ(ref.params["region"], "eu-west-1"); + EXPECT_EQ(ref.params["endpoint"], "minio.local"); + EXPECT_EQ(ref.params["want-mass-query"], "true"); + EXPECT_EQ(ref.params["priority"], "10"); +} + +} // namespace nix + #endif diff --git a/src/libstore/include/nix/store/s3-binary-cache-store.hh b/src/libstore/include/nix/store/s3-binary-cache-store.hh index 2fe66b0ad..0f8fff030 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -134,4 +134,79 @@ struct S3BinaryCacheStore : virtual BinaryCacheStore } // namespace nix +#elif NIX_WITH_CURL_S3 + +# include "nix/store/http-binary-cache-store.hh" + +namespace nix { + +struct S3BinaryCacheStoreConfig : HttpBinaryCacheStoreConfig +{ + using HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig; + + S3BinaryCacheStoreConfig(std::string_view uriScheme, std::string_view bucketName, const Params & params); + + const Setting profile{ + this, + "default", + "profile", + R"( + The name of the AWS configuration profile to use. By default + Nix uses the `default` profile. + )"}; + +public: + + const Setting region{ + this, + "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. + )"}; + + const Setting scheme{ + this, + "https", + "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 S3 endpoint to use. When empty (default), uses AWS S3 with + region-specific endpoints (e.g., s3.us-east-1.amazonaws.com). + For S3-compatible services such as MinIO, set this to your service's endpoint. + + > **Note** + > + > Custom endpoints must support HTTPS and use path-based + > addressing instead of virtual host based addressing. + )"}; + + static const std::string name() + { + return "S3 Binary Cache Store"; + } + + static StringSet uriSchemes(); + + static std::string doc(); +}; + +} // namespace nix + #endif diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index b70f04be7..ab0847bb1 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -589,4 +589,50 @@ static RegisterStoreImplementation regS3BinaryCa } // namespace nix +#elif NIX_WITH_CURL_S3 + +# include + +# include "nix/store/s3-binary-cache-store.hh" +# include "nix/store/http-binary-cache-store.hh" +# include "nix/store/store-registration.hh" + +namespace nix { + +StringSet S3BinaryCacheStoreConfig::uriSchemes() +{ + return {"s3"}; +} + +S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( + std::string_view scheme, std::string_view _cacheUri, const Params & params) + : StoreConfig(params) + , HttpBinaryCacheStoreConfig(scheme, _cacheUri, params) +{ + // For S3 stores, preserve S3-specific query parameters as part of the URL + // These are needed for region specification and other S3-specific settings + assert(cacheUri.query.empty()); + + // Only copy S3-specific parameters to the URL query + static const std::set s3Params = {"region", "endpoint", "profile", "scheme"}; + for (const auto & [key, value] : params) { + if (s3Params.contains(key)) { + cacheUri.query[key] = value; + } + } +} + +std::string S3BinaryCacheStoreConfig::doc() +{ + return R"( + **Store URL format**: `s3://bucket-name` + + This store allows reading and writing a binary cache stored in an AWS S3 bucket. + )"; +} + +static RegisterStoreImplementation registerS3BinaryCacheStore; + +} // namespace nix + #endif