From e38128b90d35d31a9ea69802f62219e27f82c12a Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Costa Date: Thu, 16 Oct 2025 18:38:42 +0000 Subject: [PATCH] feat(libstore): support S3 object versioning via versionId parameter S3 buckets support object versioning to prevent unexpected changes, but Nix previously lacked the ability to fetch specific versions of S3 objects. This adds support for a `versionId` query parameter in S3 URLs, enabling users to pin to specific object versions: ``` s3://bucket/key?region=us-east-1&versionId=abc123 ``` --- doc/manual/rl-next/s3-object-versioning.md | 14 ++++++ src/libstore-tests/s3-url.cc | 50 ++++++++++++++++++++ src/libstore/include/nix/store/s3-url.hh | 1 + src/libstore/s3-url.cc | 10 ++++ tests/nixos/s3-binary-cache-store.nix | 53 ++++++++++++++++++++-- 5 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 doc/manual/rl-next/s3-object-versioning.md diff --git a/doc/manual/rl-next/s3-object-versioning.md b/doc/manual/rl-next/s3-object-versioning.md new file mode 100644 index 000000000..3b85e0926 --- /dev/null +++ b/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 +``` diff --git a/src/libstore-tests/s3-url.cc b/src/libstore-tests/s3-url.cc index 2c384c255..9fa625fd6 100644 --- a/src/libstore-tests/s3-url.cc +++ b/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 & info) { return info.param.description; }); diff --git a/src/libstore/include/nix/store/s3-url.hh b/src/libstore/include/nix/store/s3-url.hh index 4ee0c87f9..cf59dbea8 100644 --- a/src/libstore/include/nix/store/s3-url.hh +++ b/src/libstore/include/nix/store/s3-url.hh @@ -26,6 +26,7 @@ struct ParsedS3URL std::optional profile; std::optional region; std::optional scheme; + std::optional 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). diff --git a/src/libstore/s3-url.cc b/src/libstore/s3-url.cc index e8fbba8f7..503c0cd91 100644 --- a/src/libstore/s3-url.cc +++ b/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), }; }, }, diff --git a/tests/nixos/s3-binary-cache-store.nix b/tests/nixos/s3-binary-cache-store.nix index 981fab868..a2ede4572 100644 --- a/tests/nixos/s3-binary-cache-store.nix +++ b/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!")