From 61fbef42a6eeae7553f148f1759c5a770a2f65aa Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sat, 18 Oct 2025 18:47:27 +0300 Subject: [PATCH 1/2] libstore: Simplify check for S3-specific URI query parameters Instead of hardcoding strings we should instead use the setting objects to determine the query names that should be preserved. --- .../include/nix/store/s3-binary-cache-store.hh | 8 ++++++-- src/libstore/s3-binary-cache-store.cc | 15 +++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) 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 c8cb967c1..e5fcbeda3 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -21,8 +21,6 @@ struct S3BinaryCacheStoreConfig : HttpBinaryCacheStoreConfig Nix uses the `default` profile. )"}; -public: - const Setting region{ this, "us-east-1", @@ -63,6 +61,12 @@ public: > addressing instead of virtual host based addressing. )"}; + /** + * Set of settings that are part of the S3 URI itself. + * These are needed for region specification and other S3-specific settings. + */ + const std::set s3UriSettings = {&profile, ®ion, &scheme, &endpoint}; + static const std::string name() { return "S3 Binary Cache Store"; diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index a84ea5fcb..ac08a4982 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -1,10 +1,10 @@ #include "nix/store/s3-binary-cache-store.hh" - -#include - #include "nix/store/http-binary-cache-store.hh" #include "nix/store/store-registration.hh" +#include +#include + namespace nix { StringSet S3BinaryCacheStoreConfig::uriSchemes() @@ -17,14 +17,13 @@ S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( : 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()); + assert(cacheUri.scheme == "s3"); - // 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)) { + auto s3Params = + std::views::transform(s3UriSettings, [](const AbstractSetting * setting) { return setting->name; }); + if (std::ranges::contains(s3Params, key)) { cacheUri.query[key] = value; } } From 3d147c04a5f9d03e1696fb25b495a077885d2cf7 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sat, 18 Oct 2025 19:11:39 +0300 Subject: [PATCH 2/2] libstore: Implement getHumanReadableURI for S3BinaryCacheStoreConfig This slightly improves the logs situation by including the region/profile/endpoint in the logs when S3 store references get printed. Instead of: copying path '/nix/store/lxnp9cs4cfh2g9r2bs4z7gwwz9kdj2r9-test-package-c' to 's3://bucketname'... This now includes: copying path '/nix/store/lxnp9cs4cfh2g9r2bs4z7gwwz9kdj2r9-test-package-c' to 's3://bucketname?endpoint=http://server:9000®ion=eu-west-1'... --- .../include/nix/store/s3-binary-cache-store.hh | 2 ++ src/libstore/s3-binary-cache-store.cc | 13 +++++++++++++ 2 files changed, 15 insertions(+) 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 e5fcbeda3..288ca41a0 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -75,6 +75,8 @@ struct S3BinaryCacheStoreConfig : HttpBinaryCacheStoreConfig static StringSet uriSchemes(); static std::string doc(); + + std::string getHumanReadableURI() const override; }; } // namespace nix diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index ac08a4982..0b37ac5d7 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -29,6 +29,19 @@ S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( } } +std::string S3BinaryCacheStoreConfig::getHumanReadableURI() const +{ + auto reference = getReference(); + reference.params = [&]() { + Params relevantParams; + for (auto & setting : s3UriSettings) + if (setting->overridden) + relevantParams.insert({setting->name, reference.params.at(setting->name)}); + return relevantParams; + }(); + return reference.render(); +} + std::string S3BinaryCacheStoreConfig::doc() { return R"(