Compare commits

...

5 Commits

Author SHA1 Message Date
overtrue 6de965ae3c test(e2e): register paginator regression in Darwin inventory 2026-10-03 07:07:41 +08:00
overtrue 7c56853067 fix(storage): publish delete rollback backups atomically
Stage rollback metadata outside the rollback directory and publish it only after the full write succeeds. A short write must not leave a backup that quorum rollback can rename over acknowledged version history.

Add an isolated real short-write regression and register the backported ListParts SDK test in the smoke and Linux full inventories.
2026-10-03 06:43:25 +08:00
Hauser 6f61761c7b fix(s3): honor sparse ListParts markers and verify termination
Resume part listings at the first part above the numeric marker, even
when that marker is absent. Use binary search over the sorted part
numbers and retain the existing exact-tail empty-slice path.

Add storage, XML, and real AWS SDK paginator regressions for empty and
terminal pages, sparse markers, and multipart completion integrity.

Co-Authored-By: heihutu <heihutu@gmail.com>
Co-Authored-By: zhi22915 <qiuzgang@gmail.com>
(cherry picked from commit 673031eea1)
2026-10-03 06:32:24 +08:00
Chapman 1bae764530 fix: emit NextPartNumberMarker only when ListParts is truncated
ListPartsInfo.next_part_number_marker was a non-optional usize that
defaulted to 0 and was only assigned when the response was truncated.
The S3 serializer then emitted it unconditionally as Some(0), causing
AWS SDK paginators to loop infinitely on part_number_marker=0 instead
of terminating.

Change the field to Option<usize> (None by default) and set it only
inside the is_truncated branch. The S3 output layer now uses
.and_then() so NextPartNumberMarker is absent when IsTruncated=false,
matching AWS S3 behavior.

Fixes #8208

(cherry picked from commit 44de803a38)
2026-10-03 06:32:24 +08:00
overtrue 7150ed2066 ci: remove Docker Hub description sync 2026-10-02 19:59:28 +08:00
10 changed files with 352 additions and 67 deletions
+2 -2
View File
@@ -1,2 +1,2 @@
sha256-darwin=14c8cd76b8cbf643329c371daa70a5edd4357d42259a2fd9c653e548675d4a6d
sha256-linux=80d0fae627007c31f250fb1fb21c8a952861195d53d7f60b6e71636d5ca65b46
sha256-darwin=6b8e35ef69456bd244d6a6b19179e408763763e1e163aaaf90055490ae12da4b
sha256-linux=ff4c40f288ab5e91fd28d533c277538c0e3281fa353328ab52f5a8f88e0dc931
+1 -1
View File
@@ -1 +1 @@
sha256=476cb92aa9e385c9bb509ee5df3e5bd07e8c929730040b60d16791b6c0616d00
sha256=7e3989625e2e6087c2e7db7ee02d30624b566d5a99832122032fb413ff456662
-48
View File
@@ -551,54 +551,6 @@ jobs:
if-no-files-found: error
retention-days: 30
# Docker Hub's overview is a separate `full_description` field that
# `docker push` never touches, so it drifted from README.md and kept
# linking to pages that no longer exist (#7995). Republish it from the
# commit the images were built from, only when images were pushed.
sync-dockerhub-description:
name: Sync Docker Hub Description
needs: [ build-check, build-docker ]
if: needs.build-check.outputs.should_build == 'true' && needs.build-check.outputs.should_push == 'true'
runs-on: sm-standard-2
timeout-minutes: 10
steps:
- name: Checkout repository
uses: actions/checkout@f548e57e544e1ff5a4c46bf1e1b8685f8e4a348a # v7
with:
persist-credentials: false
ref: ${{ needs.build-check.outputs.source_ref }}
sparse-checkout: README.md
sparse-checkout-cone-mode: false
- name: Sync Docker Hub description
uses: peter-evans/dockerhub-description@1b9a80c056b620d92cedb9d9b5a223409c68ddfa # v5.0.0
with:
username: ${{ env.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
repository: ${{ env.REGISTRY_DOCKERHUB }}
readme-filepath: ./README.md
# README.md links to docs/ and CONTRIBUTING.md by relative path;
# rewrite those to github.com URLs so they resolve on Docker Hub.
enable-url-completion: true
# Docker Hub caps `full_description` at 25,000 bytes and the action
# truncates to fit with only a warning, which would publish a README
# cut off mid-sentence. README.md is already near the cap, so read the
# published overview back and fail if it hit the limit.
- name: Check published description was not truncated
env:
REPOSITORY: ${{ env.REGISTRY_DOCKERHUB }}
run: |
limit=25000
published=$(curl -sSf "https://hub.docker.com/v2/repositories/${REPOSITORY}/" \
| jq -j '.full_description' | wc -c | tr -d ' ')
echo "Published overview: ${published} bytes (Docker Hub limit: ${limit})"
# Truncation lands within one UTF-8 character of the cap.
if [ "$published" -ge $((limit - 4)) ]; then
echo "::error file=README.md::Docker Hub overview was truncated at ${limit} bytes; trim README.md before the next release."
exit 1
fi
# Docker build summary
docker-summary:
name: Docker Build Summary
@@ -65,6 +65,130 @@ mod tests {
Ok(())
}
#[tokio::test]
async fn list_parts_sdk_paginator_terminates_and_preserves_completion() -> Result<(), Box<dyn std::error::Error + Send + Sync>>
{
init_logging();
let mut env = RustFSTestEnvironment::new().await?;
env.start_rustfs_server(Vec::new()).await?;
let client = env.create_s3_client();
let bucket = "multipart-pagination";
let key = "sparse-parts.bin";
env.create_test_bucket(bucket).await?;
let upload = client.create_multipart_upload().bucket(bucket).key(key).send().await?;
let upload_id = upload.upload_id().ok_or("CreateMultipartUpload returned no upload ID")?;
let mut empty_pages = client
.list_parts()
.bucket(bucket)
.key(key)
.upload_id(upload_id)
.into_paginator()
.stop_on_duplicate_token(false)
.send();
let empty = tokio::time::timeout(std::time::Duration::from_secs(30), empty_pages.next())
.await?
.ok_or("empty upload must return one page")??;
assert!(empty.parts().is_empty());
assert_eq!(empty.is_truncated(), Some(false));
assert_eq!(empty.next_part_number_marker(), None);
assert!(
tokio::time::timeout(std::time::Duration::from_secs(30), empty_pages.next())
.await?
.is_none()
);
let mut expected_body = Vec::new();
let mut completed_parts = Vec::new();
for (part_number, body) in [(1, vec![b'a'; PART_SIZE]), (3, vec![b'b'; PART_SIZE]), (10, b"tail".to_vec())] {
expected_body.extend_from_slice(&body);
let uploaded = client
.upload_part()
.bucket(bucket)
.key(key)
.upload_id(upload_id)
.part_number(part_number)
.body(ByteStream::from(body))
.send()
.await?;
completed_parts.push(
CompletedPart::builder()
.part_number(part_number)
.set_e_tag(uploaded.e_tag().map(str::to_owned))
.build(),
);
}
for (page_size, expected_pages) in [(1, 3), (2, 2), (3, 1), (1000, 1)] {
let mut pages = client
.list_parts()
.bucket(bucket)
.key(key)
.upload_id(upload_id)
.into_paginator()
.page_size(page_size)
// The server must terminate pagination even without the SDK's duplicate-token safeguard.
.stop_on_duplicate_token(false)
.send();
let mut page_count = 0;
let mut listed_parts = Vec::new();
while let Some(page) = tokio::time::timeout(std::time::Duration::from_secs(30), pages.next()).await? {
assert!(page_count < expected_pages, "paginator must terminate after the final page");
let page = page?;
page_count += 1;
let truncated = page_count < expected_pages;
assert_eq!(page.is_truncated(), Some(truncated));
let last_part = page.parts().last().and_then(|part| part.part_number());
let expected_marker = truncated.then(|| last_part.expect("truncated page must contain parts").to_string());
assert_eq!(page.next_part_number_marker(), expected_marker.as_deref());
listed_parts.extend(
page.parts()
.iter()
.map(|part| part.part_number().expect("part number must be present")),
);
}
assert_eq!(page_count, expected_pages);
assert_eq!(listed_parts, [1, 3, 10]);
}
let mut resumed_pages = client
.list_parts()
.bucket(bucket)
.key(key)
.upload_id(upload_id)
.part_number_marker("2")
.into_paginator()
.page_size(1)
.stop_on_duplicate_token(false)
.send();
for (part_number, next_marker) in [(3, Some("3")), (10, None)] {
let page = tokio::time::timeout(std::time::Duration::from_secs(30), resumed_pages.next())
.await?
.ok_or("parts above a missing marker must still be listed")??;
assert_eq!(page.parts().len(), 1);
assert_eq!(page.parts()[0].part_number(), Some(part_number));
assert_eq!(page.is_truncated(), Some(next_marker.is_some()));
assert_eq!(page.next_part_number_marker(), next_marker);
}
assert!(
tokio::time::timeout(std::time::Duration::from_secs(30), resumed_pages.next())
.await?
.is_none()
);
client
.complete_multipart_upload()
.bucket(bucket)
.key(key)
.upload_id(upload_id)
.multipart_upload(CompletedMultipartUpload::builder().set_parts(Some(completed_parts)).build())
.send()
.await?;
assert_completed_object(&client, bucket, key, "STANDARD", &expected_body).await?;
env.stop_server();
Ok(())
}
#[tokio::test]
async fn multipart_upload_preserves_standard_and_rrs_across_retry_and_resume()
-> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+100 -6
View File
@@ -280,18 +280,19 @@ async fn write_delete_rollback_file(
) -> Result<()> {
let backup_dir = object_dir.join(rollback_dir.to_string());
let path = backup_dir.join(name);
if namespace_owner.is_none() {
fs::create_dir_all(&backup_dir).await.map_err(to_file_error)?;
fs::write(path, data).await.map_err(to_file_error)?;
return Ok(());
}
let lease = os::acquire_namespace_mutation_lease_with_owner(&path, namespace_owner).await;
let data = data.to_vec();
let object_dir = object_dir.to_path_buf();
os::run_blocking_namespace_operation(lease, move || {
std::fs::create_dir_all(&backup_dir)?;
#[cfg(test)]
run_owned_file_write_before_open(&path);
std::fs::write(path, data)
// Rollback must never consume a partially written backup. Stage outside
// the rollback directory so interrupted writes cannot become staged data.
let mut staged = tempfile::Builder::new().prefix(".delete-rollback-").tempfile_in(object_dir)?;
std::io::Write::write_all(&mut staged, &data)?;
staged.persist(path).map_err(|err| err.error)?;
Ok(())
})
.await
.map_err(to_file_error)?;
@@ -16850,6 +16851,99 @@ mod test {
);
}
#[cfg(unix)]
#[tokio::test]
async fn delete_rollback_partial_backup_write_preserves_version_history() {
const CHILD: &str = "RUSTFS_TEST_DELETE_BACKUP_PARTIAL_WRITE_CHILD";
let bucket = "partial-backup-bucket";
let object = "versioned-object";
if let Some(root) = std::env::var_os(CHILD) {
let root = PathBuf::from(root);
let endpoint = Endpoint::try_from(root.to_str().expect("UTF-8 disk path")).expect("disk endpoint");
let disk = LocalDisk::new(&endpoint, false).await.expect("local disk");
let xl_path = root.join(bucket).join(object).join(STORAGE_FORMAT_FILE);
let original = fs::read(&xl_path).await.expect("committed history");
let version_id = Uuid::parse_str(&std::env::var("RUSTFS_TEST_DELETE_BACKUP_VERSION").expect("version ID"))
.expect("valid version ID");
let first = test_file_info(object, version_id, None, None);
let rollback_dir = Uuid::new_v4();
let result = disk
.delete_version(
bucket,
object,
first.clone(),
false,
DeleteOptions {
old_data_dir: Some(rollback_dir),
..Default::default()
},
)
.await;
assert!(result.is_err(), "the backup must fail under the file-size limit");
disk.undo_write_with_namespace_owner(
bucket,
object,
first,
DeleteOptions {
undo_write: true,
undo_delete: true,
old_data_dir: Some(rollback_dir),
..Default::default()
},
None,
)
.await
.expect("failed delete rollback");
assert_eq!(fs::read(&xl_path).await.expect("metadata after rollback"), original);
return;
}
let dir = tempfile::tempdir().expect("temporary disk");
let endpoint = Endpoint::try_from(dir.path().to_str().expect("UTF-8 disk path")).expect("disk endpoint");
let disk = LocalDisk::new(&endpoint, false).await.expect("local disk");
ensure_test_volume(&disk, bucket).await;
ensure_test_volume(&disk, RUSTFS_META_TMP_BUCKET).await;
let object_dir = dir.path().join(bucket).join(object);
fs::create_dir_all(&object_dir).await.expect("object directory");
let xl_path = object_dir.join(STORAGE_FORMAT_FILE);
let version_id = Uuid::new_v4();
let mut meta = FileMeta::new();
meta.add_version(test_file_info(object, version_id, None, None))
.expect("first version");
for _ in 1..100 {
meta.add_version(test_file_info(object, Uuid::new_v4(), None, None))
.expect("history version");
}
let original = meta.marshal_msg().expect("version history");
assert!(original.len() > 4096, "backup must exceed the injected write limit");
fs::write(&xl_path, &original).await.expect("committed metadata");
// Isolate the process-wide limit and ignored signal in a child. A real
// EFBIG short write exercises the failed-backup path without privileges.
let output = std::process::Command::new("sh")
.args(["-c", "trap '' XFSZ; ulimit -f 4; exec \"$@\"", "partial-backup-test"])
.arg(std::env::current_exe().expect("test executable"))
.args([
"--exact",
"disk::local::test::delete_rollback_partial_backup_write_preserves_version_history",
"--nocapture",
])
.env(CHILD, dir.path())
.env("RUSTFS_TEST_DELETE_BACKUP_VERSION", version_id.to_string())
.output()
.expect("isolated partial-write test should run");
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(output.status.success(), "partial-write child failed: {stdout} {stderr}");
assert!(stdout.contains("1 passed"), "child must execute the selected regression: {stdout}");
assert_eq!(fs::read(&xl_path).await.expect("metadata after rollback"), original);
disk.write_metadata("", bucket, object, test_file_info(object, Uuid::new_v4(), None, None))
.await
.expect("write after failed delete");
let after = fs::read(&xl_path).await.expect("metadata after subsequent write");
assert_eq!(FileMeta::load(&after).expect("readable history").versions.len(), 101);
}
// The undo_write restore consumes `<rollback>/xl.meta.bkp` by rename; a
// synthetic rollback dir is then empty and must be reclaimed so the object
// dir can empty out (BucketNotEmpty leak). A real data dir still holds its
+22 -4
View File
@@ -7182,10 +7182,10 @@ fn parts_after_marker(part_numbers: &[usize], part_number_marker: usize) -> Opti
return Some(part_numbers);
}
part_numbers
.iter()
.position(|&part_number| part_number != 0 && part_number == part_number_marker)
.map(|index| &part_numbers[index + 1..])
// reduce_quorum_part_numbers returns sorted numbers; the marker need not exist.
part_numbers.last().filter(|&&last| part_number_marker <= last)?;
let index = part_numbers.partition_point(|&part_number| part_number <= part_number_marker);
Some(&part_numbers[index..])
}
pub fn canonicalize_etag(etag: &str) -> String {
@@ -13983,6 +13983,24 @@ mod tests {
assert!(parts_after_marker(&part_numbers, 4).is_none());
}
#[test]
fn parts_after_marker_uses_exclusive_numeric_boundary_for_sparse_parts() {
let part_numbers = [1, 3, 10];
for (marker, expected) in [
(0, Some(&part_numbers[..])),
(1, Some(&part_numbers[1..])),
(2, Some(&part_numbers[1..])),
(3, Some(&part_numbers[2..])),
(9, Some(&part_numbers[2..])),
(10, Some(&part_numbers[3..])),
(11, None),
(usize::MAX, None),
] {
assert_eq!(parts_after_marker(&part_numbers, marker), expected, "marker {marker}");
}
assert_eq!(parts_after_marker(&[], 1), None);
}
#[test]
fn delete_file_info_version_id_maps_explicit_null_version_to_stored_null() {
assert_eq!(delete_file_info_version_id(Some(Uuid::nil())), None);
+49 -1
View File
@@ -2125,7 +2125,7 @@ impl crate::storage_api_contracts::multipart::MultipartOperations for SetDisks {
if object_parts.len() > ret.parts.len() {
ret.is_truncated = true;
ret.next_part_number_marker = ret.parts.last().map(|v| v.part_num).unwrap_or_default();
ret.next_part_number_marker = ret.parts.last().map(|v| v.part_num);
}
ensure_multipart_bucket_lifecycle_lock_held(bucket, object, opts)?;
@@ -4505,6 +4505,54 @@ mod tests {
}
}
#[tokio::test]
async fn list_object_parts_pagination_preserves_sparse_markers_and_terminates() {
let (_temp_dirs, disks, set_disks) = hermetic_set_disks(4).await;
let bucket = "multipart-pagination";
let object = "object";
make_bucket_on_all(&disks, bucket).await;
let opts = ObjectOptions::default();
let upload = set_disks
.new_multipart_upload(bucket, object, &opts)
.await
.expect("multipart upload should be created");
let empty = set_disks
.list_object_parts(bucket, object, &upload.upload_id, None, 2, &opts)
.await
.expect("empty upload should be listable");
assert!(empty.parts.is_empty());
assert!(!empty.is_truncated);
assert_eq!(empty.next_part_number_marker, None);
for part_number in [10, 1, 3] {
put_test_part(&set_disks, bucket, object, &upload.upload_id, part_number, b"part", 4).await;
}
for (marker, max_parts, expected_parts, next_marker) in [
(None, 0, vec![], None),
(None, 1, vec![1], Some(1)),
(None, 2, vec![1, 3], Some(3)),
(None, 3, vec![1, 3, 10], None),
(None, MAX_PARTS_COUNT + 1, vec![1, 3, 10], None),
(Some(1), 2, vec![3, 10], None),
(Some(2), 1, vec![3], Some(3)),
(Some(3), 2, vec![10], None),
(Some(10), 2, vec![], None),
(Some(11), 2, vec![], None),
] {
let page = set_disks
.list_object_parts(bucket, object, &upload.upload_id, marker, max_parts, &opts)
.await
.expect("uploaded parts should be listable");
assert_eq!(page.part_number_marker, marker.unwrap_or_default());
assert_eq!(page.max_parts, max_parts.min(MAX_PARTS_COUNT));
assert_eq!(page.parts.iter().map(|part| part.part_num).collect::<Vec<_>>(), expected_parts);
assert_eq!(page.is_truncated, next_marker.is_some());
assert_eq!(page.next_part_number_marker, next_marker);
}
}
async fn object_transaction_epochs(disks: &[DiskStore], bucket: &str, object: &str) -> Vec<Option<Uuid>> {
let mut epochs = Vec::with_capacity(disks.len());
let deadline = tokio::time::Instant::now() + Duration::from_secs(30);
+1 -1
View File
@@ -61,7 +61,7 @@ pub struct ListPartsInfo {
pub upload_id: String,
pub storage_class: String,
pub part_number_marker: usize,
pub next_part_number_marker: usize,
pub next_part_number_marker: Option<usize>,
pub max_parts: usize,
pub is_truncated: bool,
pub parts: Vec<PartInfo>,
+1 -1
View File
@@ -118,7 +118,7 @@ Post-merge and tag-driven; not a substitute for a PR gate.
|---|---|---|
| Push to `main`, weekly schedule, dispatch | `build.yml` `build-rustfs` (a development build on a main push restricts the matrix to the Linux targets) | build artifacts; no release publication |
| Valid release or preview tag | `build.yml` `build-rustfs`, `create-release`, `upload-release-assets`, `publish-release` | draft release, checksummed assets, publish |
| Successful non-preview release-tag build (`workflow_run`) | `docker.yml` `build-docker`, `scan-docker-image`, `sync-dockerhub-description` | multi-architecture images, vulnerability report, and the Docker Hub overview republished from `README.md` |
| Successful non-preview release-tag build (`workflow_run`) | `docker.yml` `build-docker`, `scan-docker-image` | multi-architecture images and vulnerability report |
| Successful release-tag build (`workflow_run`) | `package.yml` `package` | DEB/RPM packages and checksums uploaded to the release |
| Successful non-preview release-tag build (`workflow_run`) | `helm-package.yml` `build-helm-package`, `publish-helm-package` | versioned chart and repository index |
| Final tag's release published | `build.yml` `cleanup-preview-releases` | deletes every `<target>-preview.<N>` Release for that target; the tags are kept |
+52 -3
View File
@@ -71,7 +71,7 @@ pub(crate) fn build_list_parts_output(res: ListPartsInfo) -> ListPartsOutput {
owner: Some(owner),
initiator: Some(initiator),
is_truncated: Some(res.is_truncated),
next_part_number_marker: res.next_part_number_marker.try_into().ok(),
next_part_number_marker: res.next_part_number_marker.and_then(|v| v.try_into().ok()),
max_parts: res.max_parts.try_into().ok(),
part_number_marker: res.part_number_marker.try_into().ok(),
storage_class: Some(
@@ -228,7 +228,7 @@ mod tests {
upload_id: "upload-a".to_string(),
storage_class: "STANDARD".to_string(),
part_number_marker: 1,
next_part_number_marker: 2,
next_part_number_marker: Some(2),
max_parts: 1000,
is_truncated: true,
parts: vec![PartInfo {
@@ -259,6 +259,55 @@ mod tests {
assert_eq!(output.initiator, Some(rustfs_initiator()));
}
#[test]
fn test_list_parts_output_xml_omits_terminal_marker() {
for (marker, part_numbers) in [(0, vec![]), (3, vec![10]), (0, vec![1, 3])] {
let output = build_list_parts_output(ListPartsInfo {
part_number_marker: marker,
max_parts: 2,
parts: part_numbers
.into_iter()
.map(|part_num| PartInfo {
part_num,
..Default::default()
})
.collect(),
..Default::default()
});
assert_eq!(output.is_truncated, Some(false));
assert_eq!(output.next_part_number_marker, None);
let xml = String::from_utf8(crate::storage::storage_api::serialize(&output).expect("output should serialize"))
.expect("XML should be UTF-8");
assert!(xml.contains("<IsTruncated>false</IsTruncated>"));
assert!(xml.contains(&format!("<PartNumberMarker>{marker}</PartNumberMarker>")));
assert!(!xml.contains("NextPartNumberMarker"), "terminal page must omit the token: {xml}");
}
}
#[test]
fn test_list_parts_output_xml_preserves_truncated_marker() {
let output = build_list_parts_output(ListPartsInfo {
is_truncated: true,
next_part_number_marker: Some(3),
max_parts: 2,
parts: [1, 3]
.into_iter()
.map(|part_num| PartInfo {
part_num,
..Default::default()
})
.collect(),
..Default::default()
});
let xml = String::from_utf8(crate::storage::storage_api::serialize(&output).expect("output should serialize"))
.expect("XML should be UTF-8");
assert!(xml.contains("<IsTruncated>true</IsTruncated>"));
assert!(xml.contains("<NextPartNumberMarker>3</NextPartNumberMarker>"));
assert_eq!(output.parts.as_ref().expect("parts should be present").len(), 2);
}
#[test]
fn test_list_parts_output_reports_logical_size_for_compressed_parts() {
let input = ListPartsInfo {
@@ -374,7 +423,7 @@ mod tests {
let input = ListPartsInfo {
storage_class: "STANDARD_IA".to_string(),
part_number_marker: usize::MAX,
next_part_number_marker: usize::MAX,
next_part_number_marker: Some(usize::MAX),
max_parts: usize::MAX,
parts: vec![PartInfo {
part_num: usize::MAX,