Merge pull request #14274 from lovesegfault/nix-s3-versioned

feat(libstore): support S3 object versioning via versionId parameter

Author: xokdvium

doc/manual/rl-next/s3-object-versioning.md

@@ -0,0 +1,14 @@
+---
+synopsis: "S3 URLs now support object versioning via versionId parameter"
+prs: [14274]
+issues: [13955]
+---
+
+S3 URLs now support a `versionId` query parameter to fetch specific versions
+of objects from S3 buckets with versioning enabled. This allows pinning to
+exact object versions for reproducibility and protection against unexpected
+changes:
+
+```
+s3://bucket/key?region=us-east-1&versionId=abc123def456
+```

src/libstore-tests/s3-url.cc

@@ -70,6 +70,25 @@ INSTANTIATE_TEST_SUITE_P(
             },
             "with_profile_and_region",
         },
+        ParsedS3URLTestCase{
+            "s3://my-bucket/my-key.txt?versionId=abc123xyz",
+            {
+                .bucket = "my-bucket",
+                .key = {"my-key.txt"},
+                .versionId = "abc123xyz",
+            },
+            "with_versionId",
+        },
+        ParsedS3URLTestCase{
+            "s3://bucket/path/to/object?region=eu-west-1&versionId=version456",
+            {
+                .bucket = "bucket",
+                .key = {"path", "to", "object"},
+                .region = "eu-west-1",
+                .versionId = "version456",
+            },
+            "with_region_and_versionId",
+        },
         ParsedS3URLTestCase{
             "s3://bucket/key?endpoint=https://minio.local&scheme=http",
             {
@@ -222,6 +241,37 @@ INSTANTIATE_TEST_SUITE_P(
             },
             "https://s3.ap-southeast-2.amazonaws.com/bucket/path/to/file.txt",
             "complex_path_and_region",
+        },
+        S3ToHttpsConversionTestCase{
+            ParsedS3URL{
+                .bucket = "my-bucket",
+                .key = {"my-key.txt"},
+                .versionId = "abc123xyz",
+            },
+            ParsedURL{
+                .scheme = "https",
+                .authority = ParsedURL::Authority{.host = "s3.us-east-1.amazonaws.com"},
+                .path = {"", "my-bucket", "my-key.txt"},
+                .query = {{"versionId", "abc123xyz"}},
+            },
+            "https://s3.us-east-1.amazonaws.com/my-bucket/my-key.txt?versionId=abc123xyz",
+            "with_versionId",
+        },
+        S3ToHttpsConversionTestCase{
+            ParsedS3URL{
+                .bucket = "versioned-bucket",
+                .key = {"path", "to", "object"},
+                .region = "eu-west-1",
+                .versionId = "version456",
+            },
+            ParsedURL{
+                .scheme = "https",
+                .authority = ParsedURL::Authority{.host = "s3.eu-west-1.amazonaws.com"},
+                .path = {"", "versioned-bucket", "path", "to", "object"},
+                .query = {{"versionId", "version456"}},
+            },
+            "https://s3.eu-west-1.amazonaws.com/versioned-bucket/path/to/object?versionId=version456",
+            "with_region_and_versionId",
         }),
     [](const ::testing::TestParamInfo<S3ToHttpsConversionTestCase> & info) { return info.param.description; });
 

src/libstore/include/nix/store/s3-url.hh

@@ -26,6 +26,7 @@ struct ParsedS3URL
     std::optional<std::string> profile;
     std::optional<std::string> region;
     std::optional<std::string> scheme;
+    std::optional<std::string> versionId;
     /**
      * The endpoint can be either missing, be an absolute URI (with a scheme like `http:`)
      * or an authority (so an IP address or a registered name).

src/libstore/s3-url.cc

@@ -48,6 +48,7 @@ try {
         .profile = getOptionalParam("profile"),
         .region = getOptionalParam("region"),
         .scheme = getOptionalParam("scheme"),
+        .versionId = getOptionalParam("versionId"),
         .endpoint = [&]() -> decltype(ParsedS3URL::endpoint) {
             if (!endpoint)
                 return std::monostate();
@@ -73,6 +74,12 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
     auto regionStr = region.transform(toView).value_or("us-east-1");
     auto schemeStr = scheme.transform(toView).value_or("https");
 
+    // Build query parameters (e.g., versionId if present)
+    StringMap queryParams;
+    if (versionId) {
+        queryParams["versionId"] = *versionId;
+    }
+
     // Handle endpoint configuration using std::visit
     return std::visit(
         overloaded{
@@ -85,6 +92,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
                     .scheme = std::string{schemeStr},
                     .authority = ParsedURL::Authority{.host = "s3." + regionStr + ".amazonaws.com"},
                     .path = std::move(path),
+                    .query = std::move(queryParams),
                 };
             },
             [&](const ParsedURL::Authority & auth) {
@@ -96,6 +104,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
                     .scheme = std::string{schemeStr},
                     .authority = auth,
                     .path = std::move(path),
+                    .query = std::move(queryParams),
                 };
             },
             [&](const ParsedURL & endpointUrl) {
@@ -107,6 +116,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
                     .scheme = endpointUrl.scheme,
                     .authority = endpointUrl.authority,
                     .path = std::move(path),
+                    .query = std::move(queryParams),
                 };
             },
         },

tests/nixos/s3-binary-cache-store.nix

@@ -1,7 +1,5 @@
 {
-  lib,
   config,
-  nixpkgs,
   ...
 }:
 
@@ -147,7 +145,7 @@ in
               else:
                   machine.fail(f"nix path-info {pkg}")
 
-      def setup_s3(populate_bucket=[], public=False):
+      def setup_s3(populate_bucket=[], public=False, versioned=False):
           """
           Decorator that creates/destroys a unique bucket for each test.
           Optionally pre-populates bucket with specified packages.
@@ -156,14 +154,17 @@ in
           Args:
               populate_bucket: List of packages to upload before test runs
               public: If True, make the bucket publicly accessible
+              versioned: If True, enable versioning on the bucket before populating
           """
           def decorator(test_func):
               def wrapper():
                   bucket = str(uuid.uuid4())
                   server.succeed(f"mc mb minio/{bucket}")
-                  if public:
-                      server.succeed(f"mc anonymous set download minio/{bucket}")
                   try:
+                      if public:
+                          server.succeed(f"mc anonymous set download minio/{bucket}")
+                      if versioned:
+                          server.succeed(f"mc version enable minio/{bucket}")
                       if populate_bucket:
                           store_url = make_s3_url(bucket)
                           for pkg in populate_bucket:
@@ -597,6 +598,47 @@ in
 
           print("  ✓ File content verified correct (hash matches)")
 
+      @setup_s3(populate_bucket=[PKGS['A']], versioned=True)
+      def test_versioned_urls(bucket):
+          """Test that versionId parameter is accepted in S3 URLs"""
+          print("\n=== Testing Versioned URLs ===")
+
+          # Get the nix-cache-info file
+          cache_info_url = make_s3_url(bucket, path="/nix-cache-info")
+
+          # Fetch without versionId should work
+          client.succeed(
+              f"{ENV_WITH_CREDS} nix eval --impure --expr "
+              f"'builtins.fetchurl {{ name = \"cache-info\"; url = \"{cache_info_url}\"; }}'"
+          )
+          print("  ✓ Fetch without versionId works")
+
+          # List versions to get a version ID
+          # MinIO output format: [timestamp] size tier versionId versionNumber method filename
+          versions_output = server.succeed(f"mc ls --versions minio/{bucket}/nix-cache-info")
+
+          # Extract version ID from output (4th field after STANDARD)
+          import re
+          version_match = re.search(r'STANDARD\s+(\S+)\s+v\d+', versions_output)
+          if not version_match:
+              print(f"Debug: versions output: {versions_output}")
+              raise Exception("Could not extract version ID from MinIO output")
+
+          version_id = version_match.group(1)
+          print(f"  ✓ Found version ID: {version_id}")
+
+          # Version ID should not be "null" since versioning was enabled before upload
+          if version_id == "null":
+              raise Exception("Version ID is 'null' - versioning may not be working correctly")
+
+          # Fetch with versionId parameter
+          versioned_url = f"{cache_info_url}&versionId={version_id}"
+          client.succeed(
+              f"{ENV_WITH_CREDS} nix eval --impure --expr "
+              f"'builtins.fetchurl {{ name = \"cache-info-versioned\"; url = \"{versioned_url}\"; }}'"
+          )
+          print("  ✓ Fetch with versionId parameter works")
+
       # ============================================================================
       # Main Test Execution
       # ============================================================================
@@ -626,6 +668,7 @@ in
       test_compression_mixed()
       test_compression_disabled()
       test_nix_prefetch_url()
+      test_versioned_urls()
 
       print("\n" + "="*80)
       print("✓ All S3 Binary Cache Store Tests Passed!")