Skip to content

Commit db66c1d

Browse files
committed
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 ```
1 parent d87a06a commit db66c1d

File tree

5 files changed

+108
-2
lines changed

5 files changed

+108
-2
lines changed

src/libstore-tests/s3-url.cc

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,25 @@ INSTANTIATE_TEST_SUITE_P(
7070
},
7171
"with_profile_and_region",
7272
},
73+
ParsedS3URLTestCase{
74+
"s3://my-bucket/my-key.txt?versionId=abc123xyz",
75+
{
76+
.bucket = "my-bucket",
77+
.key = {"my-key.txt"},
78+
.versionId = "abc123xyz",
79+
},
80+
"with_versionId",
81+
},
82+
ParsedS3URLTestCase{
83+
"s3://bucket/path/to/object?region=eu-west-1&versionId=version456",
84+
{
85+
.bucket = "bucket",
86+
.key = {"path", "to", "object"},
87+
.region = "eu-west-1",
88+
.versionId = "version456",
89+
},
90+
"with_region_and_versionId",
91+
},
7392
ParsedS3URLTestCase{
7493
"s3://bucket/key?endpoint=https://minio.local&scheme=http",
7594
{
@@ -222,6 +241,37 @@ INSTANTIATE_TEST_SUITE_P(
222241
},
223242
"https://s3.ap-southeast-2.amazonaws.com/bucket/path/to/file.txt",
224243
"complex_path_and_region",
244+
},
245+
S3ToHttpsConversionTestCase{
246+
ParsedS3URL{
247+
.bucket = "my-bucket",
248+
.key = {"my-key.txt"},
249+
.versionId = "abc123xyz",
250+
},
251+
ParsedURL{
252+
.scheme = "https",
253+
.authority = ParsedURL::Authority{.host = "s3.us-east-1.amazonaws.com"},
254+
.path = {"", "my-bucket", "my-key.txt"},
255+
.query = {{"versionId", "abc123xyz"}},
256+
},
257+
"https://s3.us-east-1.amazonaws.com/my-bucket/my-key.txt?versionId=abc123xyz",
258+
"with_versionId",
259+
},
260+
S3ToHttpsConversionTestCase{
261+
ParsedS3URL{
262+
.bucket = "versioned-bucket",
263+
.key = {"path", "to", "object"},
264+
.region = "eu-west-1",
265+
.versionId = "version456",
266+
},
267+
ParsedURL{
268+
.scheme = "https",
269+
.authority = ParsedURL::Authority{.host = "s3.eu-west-1.amazonaws.com"},
270+
.path = {"", "versioned-bucket", "path", "to", "object"},
271+
.query = {{"versionId", "version456"}},
272+
},
273+
"https://s3.eu-west-1.amazonaws.com/versioned-bucket/path/to/object?versionId=version456",
274+
"with_region_and_versionId",
225275
}),
226276
[](const ::testing::TestParamInfo<S3ToHttpsConversionTestCase> & info) { return info.param.description; });
227277

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct ParsedS3URL
2626
std::optional<std::string> profile;
2727
std::optional<std::string> region;
2828
std::optional<std::string> scheme;
29+
std::optional<std::string> versionId;
2930
/**
3031
* The endpoint can be either missing, be an absolute URI (with a scheme like `http:`)
3132
* or an authority (so an IP address or a registered name).

src/libstore/s3-binary-cache-store.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig(
2222
assert(cacheUri.query.empty());
2323

2424
// Only copy S3-specific parameters to the URL query
25-
static const std::set<std::string> s3Params = {"region", "endpoint", "profile", "scheme"};
25+
static const std::set<std::string> s3Params = {"region", "endpoint", "profile", "scheme", "versionId"};
2626
for (const auto & [key, value] : params) {
2727
if (s3Params.contains(key)) {
2828
cacheUri.query[key] = value;

src/libstore/s3-url.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ try {
4848
.profile = getOptionalParam("profile"),
4949
.region = getOptionalParam("region"),
5050
.scheme = getOptionalParam("scheme"),
51+
.versionId = getOptionalParam("versionId"),
5152
.endpoint = [&]() -> decltype(ParsedS3URL::endpoint) {
5253
if (!endpoint)
5354
return std::monostate();
@@ -73,6 +74,12 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
7374
auto regionStr = region.transform(toView).value_or("us-east-1");
7475
auto schemeStr = scheme.transform(toView).value_or("https");
7576

77+
// Build query parameters (e.g., versionId if present)
78+
StringMap queryParams;
79+
if (versionId) {
80+
queryParams["versionId"] = *versionId;
81+
}
82+
7683
// Handle endpoint configuration using std::visit
7784
return std::visit(
7885
overloaded{
@@ -85,6 +92,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
8592
.scheme = std::string{schemeStr},
8693
.authority = ParsedURL::Authority{.host = "s3." + regionStr + ".amazonaws.com"},
8794
.path = std::move(path),
95+
.query = queryParams,
8896
};
8997
},
9098
[&](const ParsedURL::Authority & auth) {
@@ -96,6 +104,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
96104
.scheme = std::string{schemeStr},
97105
.authority = auth,
98106
.path = std::move(path),
107+
.query = queryParams,
99108
};
100109
},
101110
[&](const ParsedURL & endpointUrl) {
@@ -107,6 +116,7 @@ ParsedURL ParsedS3URL::toHttpsUrl() const
107116
.scheme = endpointUrl.scheme,
108117
.authority = endpointUrl.authority,
109118
.path = std::move(path),
119+
.query = queryParams,
110120
};
111121
},
112122
},

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,19 +133,22 @@ in
133133
print(output)
134134
raise Exception(f"{error_msg}: expected {expected}, got {actual}")
135135
136-
def with_test_bucket(populate_with=[]):
136+
def with_test_bucket(populate_with=[], versioned=False):
137137
"""
138138
Decorator that creates/destroys a unique bucket for each test.
139139
Optionally pre-populates bucket with specified packages.
140140
141141
Args:
142142
populate_with: List of packages to upload before test runs
143+
versioned: If True, enable versioning on the bucket before populating
143144
"""
144145
def decorator(test_func):
145146
def wrapper():
146147
bucket = str(uuid.uuid4())
147148
server.succeed(f"mc mb minio/{bucket}")
148149
try:
150+
if versioned:
151+
server.succeed(f"mc version enable minio/{bucket}")
149152
if populate_with:
150153
store_url = make_s3_url(bucket)
151154
for pkg in populate_with:
@@ -472,6 +475,47 @@ in
472475
473476
print(" ✓ No compression applied by default")
474477
478+
@with_test_bucket(populate_with=[PKG_A], versioned=True)
479+
def test_versioned_urls(bucket):
480+
"""Test that versionId parameter is accepted in S3 URLs"""
481+
print("\n=== Testing Versioned URLs ===")
482+
483+
# Get the nix-cache-info file
484+
cache_info_url = make_s3_url(bucket, path="/nix-cache-info")
485+
486+
# Fetch without versionId should work
487+
client.succeed(
488+
f"{ENV_WITH_CREDS} nix eval --impure --expr "
489+
f"'builtins.fetchurl {{ name = \"cache-info\"; url = \"{cache_info_url}\"; }}'"
490+
)
491+
print(" ✓ Fetch without versionId works")
492+
493+
# List versions to get a version ID
494+
# MinIO output format: [timestamp] size tier versionId versionNumber method filename
495+
versions_output = server.succeed(f"mc ls --versions minio/{bucket}/nix-cache-info")
496+
497+
# Extract version ID from output (4th field after STANDARD)
498+
import re
499+
version_match = re.search(r'STANDARD\s+(\S+)\s+v\d+', versions_output)
500+
if not version_match:
501+
print(f"Debug: versions output: {versions_output}")
502+
raise Exception("Could not extract version ID from MinIO output")
503+
504+
version_id = version_match.group(1)
505+
print(f" ✓ Found version ID: {version_id}")
506+
507+
# Version ID should not be "null" since versioning was enabled before upload
508+
if version_id == "null":
509+
raise Exception("Version ID is 'null' - versioning may not be working correctly")
510+
511+
# Fetch with versionId parameter
512+
versioned_url = f"{cache_info_url}&versionId={version_id}"
513+
client.succeed(
514+
f"{ENV_WITH_CREDS} nix eval --impure --expr "
515+
f"'builtins.fetchurl {{ name = \"cache-info-versioned\"; url = \"{versioned_url}\"; }}'"
516+
)
517+
print(" ✓ Fetch with versionId parameter works")
518+
475519
# ============================================================================
476520
# Main Test Execution
477521
# ============================================================================
@@ -499,6 +543,7 @@ in
499543
test_compression_narinfo_gzip()
500544
test_compression_mixed()
501545
test_compression_disabled()
546+
test_versioned_urls()
502547
503548
print("\n" + "="*80)
504549
print("✓ All S3 Binary Cache Store Tests Passed!")

0 commit comments

Comments
 (0)