From c5a8c64d4cb974d6ebd732d30c10f3c3ea38de2b Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Wed, 16 Sep 2026 11:50:33 +0200 Subject: [PATCH 1/3] CLDSRV-961 Fix lifecycle listings stuck on PHD master keys Pin arsenal to 8.4.26 (8.5.17 on 9.4), which fixes lifecycle listings (DelimiterNonCurrent, DelimiterOrphanDeleteMarker) getting stuck forever on a run of dangling PHD master keys longer than the scan cap. bucketd runs the actual lifecycle listing, not cloudserver, so also repin the MetaData image to the released ghcr.io/scality/metadata:9.17.0-standalone tag (MD-1359): the s3c legs need the fix there too, not only in the pinned arsenal. Add a functional suite proving the real stack -- an actual metadata backend that writes PHD masters and races a repair timer, through the real backbeat route -- makes forward progress across such a desert. The listing algorithm itself is exhaustively unit-tested in Arsenal against both v0 and v1 (scality/Arsenal#2685); this suite intentionally does not re-derive those cases and stays scoped to end-to-end wiring. (cherry picked from commit d14b3265e53b26411504133edc6383bd0d48660e) --- .github/docker/docker-compose.yaml | 2 +- package.json | 2 +- .../backbeat/listLifecyclePHDKeys.js | 426 ++++++++++++++++++ yarn.lock | 6 +- 4 files changed, 431 insertions(+), 5 deletions(-) create mode 100644 tests/functional/backbeat/listLifecyclePHDKeys.js diff --git a/.github/docker/docker-compose.yaml b/.github/docker/docker-compose.yaml index 370ced54f6..628d7abec7 100644 --- a/.github/docker/docker-compose.yaml +++ b/.github/docker/docker-compose.yaml @@ -125,7 +125,7 @@ services: depends_on: - redis metadata-standalone: - image: ghcr.io/scality/metadata:8.11.0-standalone + image: ghcr.io/scality/metadata:9.17.0-standalone profiles: ['metadata-standalone'] network_mode: 'host' volumes: diff --git a/package.json b/package.json index cb07ebb836..0ced078fed 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "@azure/storage-blob": "^12.28.0", "@hapi/joi": "^17.1.1", "@smithy/node-http-handler": "^3.0.0", - "arsenal": "git+https://github.com/scality/Arsenal#8.4.24", + "arsenal": "git+https://github.com/scality/Arsenal#8.4.26", "async": "2.6.4", "bucketclient": "scality/bucketclient#8.2.7", "bufferutil": "^4.0.8", diff --git a/tests/functional/backbeat/listLifecyclePHDKeys.js b/tests/functional/backbeat/listLifecyclePHDKeys.js new file mode 100644 index 0000000000..fb85b81c1e --- /dev/null +++ b/tests/functional/backbeat/listLifecyclePHDKeys.js @@ -0,0 +1,426 @@ +const assert = require('assert'); +const async = require('async'); +const crypto = require('crypto'); +const { + CreateBucketCommand, + DeleteBucketCommand, + DeleteObjectCommand, + HeadObjectCommand, + PutBucketVersioningCommand, + PutObjectCommand, +} = require('@aws-sdk/client-s3'); +const BucketUtility = require('../aws-node-sdk/lib/utility/bucket-util'); +const { removeAllVersions } = require('../aws-node-sdk/lib/utility/versioning-util'); +const { makeBackbeatRequest } = require('./utils'); +const { promisify } = require('util'); + +/** + * Deleting an object's current version writes a PHD master. + * If no other version survives, it stays dangling. A run of dangling + * PHDs longer than max-scanned-lifecycle-listing-entries is a desert + * ISSUE: it used to truncate the orphan and noncurrent listings with no resume marker, so + * backbeat requeued the same listing forever. + * + * Metadata repairs a deleted key's PHD master after 15s (arsenal + * VersioningRequestProcessor.processVersionSpecificDelete). assertDesertWasScanned() + * checks the resume marker landed inside the desert, proving the scan cap hit + * before repair could shrink it. Seeding takes ~200ms, well inside that window. + * + * v1: skipped for bucketd/file -- its v1 format never generates a PHD master + * (arsenal delimiterVersions.js: "S3C does not use PHDs in V1 format"). + * NOTE: Mongo v1 does use PHDs but mongo is already skipped below for the dangling case. + * mongo: a zero-survivor PHD master is deleted synchronously, in the same DELETE request + * (MongoClientInterface.deleteOrRepairPHD) so no window for it to persist, so + * this suite can not seed a desert there. + * bucketd/file repair it on a best-effort 15s timer instead, which is the window that lets it go dangling. + */ +const isV1 = process.env.DEFAULT_BUCKET_KEY_FORMAT === 'v1'; +const isMongo = process.env.S3METADATA === 'mongodb'; +const describePHD = isV1 || isMongo ? describe.skip : describe; + +const bucketUtil = new BucketUtility('default', {}); +const s3 = bucketUtil.s3; + +const removeAllVersionsPromise = promisify(removeAllVersions); + +const DESERT_SIZE = 12; +const DESERT_PREFIX = 'phd-'; +const SEED_CONCURRENCY = 8; +const SCAN_CAP = '5'; +// Hard page guard. A markerless listing that keeps restarting fails fast, and does not hang. +const MAX_PAGES = 20; +// Metadata repair delay, per key. See "Timing contract" above. +const PHD_REPAIR_WINDOW_MS = 15000; + +let credentials = null; + +async function getCredentials() { + const creds = await s3.config.credentials(); + return { + accessKey: creds.accessKeyId, + secretKey: creds.secretAccessKey, + }; +} + +function uniqueBucket(prefix) { + return `${prefix}-${crypto.randomBytes(4).toString('hex')}`; +} + +function desertKey(n) { + return `${DESERT_PREFIX}${`00${n}`.slice(-3)}`; +} + +function putObject(bucket, key, cb) { + return s3 + .send(new PutObjectCommand({ Bucket: bucket, Key: key, Body: '123' })) + .then(data => cb(null, data.VersionId)) + .catch(cb); +} + +function deleteVersion(bucket, key, versionId, cb) { + return s3 + .send( + new DeleteObjectCommand({ + Bucket: bucket, + Key: key, + VersionId: versionId, + }), + ) + .then(() => cb()) + .catch(cb); +} + +/** + * Creates a dangling PHD master. Put one version, then delete that exact version. + * Metadata replaces the master with { isPHD: true } and starts its 15s repair. + * Until the repair runs, or a GET or HEAD triggers it, the key is a zero-version + * PHD master. These keys are the desert material for the tests below. + */ +function createDanglingPHD(bucket, key, cb) { + return putObject(bucket, key, (err, versionId) => (err ? cb(err) : deleteVersion(bucket, key, versionId, cb))); +} + +/** + * Seeds DESERT_SIZE dangling PHD masters. Returns the time seeding started. That + * time bounds the earliest repair deadline, because each key's own delete happens + * later. The bound is therefore safe. + */ +function seedDesert(bucket, cb) { + const seededAt = Date.now(); + return async.timesLimit( + DESERT_SIZE, + SEED_CONCURRENCY, + (n, next) => createDanglingPHD(bucket, desertKey(n), next), + err => cb(err, seededAt), + ); +} + +/** + * HEADs every desert key. A GET or HEAD on a PHD master triggers the metadata + * repair, which deletes a zero-version master. Every key returns 404. The code + * ignores errors on purpose. This clears the desert at once, instead of waiting + * for the repair timers. It also matters more than tidiness, because PHD masters + * outlive their bucket. + */ +function repairDesert(bucket, cb) { + return async.timesLimit( + DESERT_SIZE, + SEED_CONCURRENCY, + (n, next) => + s3 + .send(new HeadObjectCommand({ Bucket: bucket, Key: desertKey(n) })) + .then(() => next()) + .catch(() => next()), + cb, + ); +} + +function createOrphanDeleteMarker(bucket, key, cb) { + return putObject(bucket, key, (err, versionId) => { + if (err) { + return cb(err); + } + return s3 + .send(new DeleteObjectCommand({ Bucket: bucket, Key: key })) + .then(() => deleteVersion(bucket, key, versionId, cb)) + .catch(cb); + }); +} + +function createVersionedBucket(bucket, cb) { + return s3 + .send(new CreateBucketCommand({ Bucket: bucket })) + .then(() => + s3.send( + new PutBucketVersioningCommand({ + Bucket: bucket, + VersioningConfiguration: { Status: 'Enabled' }, + }), + ), + ) + .then(() => cb()) + .catch(cb); +} + +function cleanupBucket(bucket, cb) { + return async.series( + [ + next => repairDesert(bucket, next), + next => + removeAllVersionsPromise({ Bucket: bucket }) + .then(() => next()) + .catch(next), + next => + s3 + .send(new DeleteBucketCommand({ Bucket: bucket })) + .then(() => next()) + .catch(next), + ], + cb, + ); +} + +/** + * Reads every page of a lifecycle listing. Feeds each returned marker back into + * the next request. Checks the core invariant on every page: a truncated page + * must return a resume marker, and that marker must move forward. This is the + * regression itself -- before the fix, a desert truncated with no marker at all. + */ +function listAllPages(params, cb) { + const { bucket, listType, scanCap } = params; + const pages = []; + let keyMarker; + let versionIdMarker; + let done = false; + + return async.whilst( + () => !done, + next => { + const queryObj = { + 'list-type': listType, + 'max-scanned-lifecycle-listing-entries': scanCap, + }; + if (keyMarker !== undefined) { + if (listType === 'orphan') { + queryObj.marker = keyMarker; + } else { + queryObj['key-marker'] = keyMarker; + if (versionIdMarker !== undefined) { + queryObj['version-id-marker'] = versionIdMarker; + } + } + } + return makeBackbeatRequest( + { + method: 'GET', + bucket, + queryObj, + authCredentials: credentials, + }, + (err, response) => { + if (err) { + return next(err); + } + if (response.statusCode !== 200) { + return next( + new Error( + `${listType} listing returned ${response.statusCode}: ` + + `${String(response.body).slice(0, 200)}`, + ), + ); + } + const data = JSON.parse(response.body); + pages.push(data); + + if (pages.length > MAX_PAGES) { + return next( + new Error( + `listing did not terminate within ${MAX_PAGES} pages: ` + + 'markerless truncation restarts it from scratch', + ), + ); + } + + if (!data.IsTruncated) { + done = true; + return next(); + } + + // Report invariant violations through the callback, do not throw them. An + // assertion thrown in this HTTP callback becomes an uncaught exception, and + // mocha can blame another test for it. + const nextKeyMarker = listType === 'orphan' ? data.NextMarker : data.NextKeyMarker; + // The core invariant. A truncated listing must return a resume marker, + if (!nextKeyMarker) { + return next( + new Error( + `truncated ${listType} listing page ${pages.length} returned no marker ` + + `(request marker: ${keyMarker || ''})`, + ), + ); + } + // and that marker must move forward on every page. + if (keyMarker !== undefined) { + if (nextKeyMarker < keyMarker) { + return next(new Error(`marker went backwards: ${keyMarker} -> ${nextKeyMarker}`)); + } + const prevTuple = `${keyMarker}\0${versionIdMarker || ''}`; + const newTuple = `${nextKeyMarker}\0${data.NextVersionIdMarker || ''}`; + if (newTuple === prevTuple) { + return next( + new Error( + 'marker did not advance on truncated ' + + `${listType} page ${pages.length}: ${nextKeyMarker}`, + ), + ); + } + } + keyMarker = nextKeyMarker; + versionIdMarker = data.NextVersionIdMarker; + return next(); + }, + ); + }, + err => (err ? cb(err) : cb(null, pages)), + ); +} + +const seedDesertPromise = promisify(seedDesert); +const listAllPagesPromise = promisify(listAllPages); + +/** Seeds a fresh desert, then reads every page of a capped listing across it. */ +async function seedThenList(bucket, listType, scanCap) { + const seededAt = await seedDesertPromise(bucket); + const pages = await listAllPagesPromise({ bucket, listType, scanCap }); + return { pages, seededAt }; +} + +/** Proves the scan cap ran out inside the desert, so the assertions that follow mean something. */ +function assertDesertWasScanned(pages, seededAt, label) { + const markers = pages.map(page => page.NextMarker || page.NextKeyMarker).filter(Boolean); + if (markers.some(marker => marker.startsWith(DESERT_PREFIX))) { + return; + } + const elapsed = Date.now() - seededAt; + const cause = + elapsed >= PHD_REPAIR_WINDOW_MS + ? `seeding+listing took ${elapsed}ms, over the ${PHD_REPAIR_WINDOW_MS}ms metadata repair ` + + 'window: the desert was repaired before the listing ran (slow runner, not a code bug)' + : `only ${elapsed}ms elapsed, well inside the ${PHD_REPAIR_WINDOW_MS}ms repair window: the ` + + 'desert was never created, so this backend did not write PHD masters (v0 buckets only)'; + assert.fail( + `${label}: no resume marker landed inside the desert ` + `(markers: ${JSON.stringify(markers)}) -- ${cause}`, + ); +} + +function contentsKeys(pages) { + return pages.reduce((acc, page) => acc.concat((page.Contents || []).map(entry => entry.Key)), []); +} + +function contentsEntries(pages) { + return pages.reduce((acc, page) => acc.concat(page.Contents || []), []); +} + +describePHD('listLifecycle over a dangling-PHD desert', () => { + before(async () => { + credentials = await getCredentials(); + }); + + // scanCap = 5, desert = 12 dangling PHDs (phd-001..012) in both buckets below. + + describe('orphan crosses the desert', () => { + const bucket = uniqueBucket('lc-phd-orphan'); + + before(done => + async.series( + [ + next => createVersionedBucket(bucket, next), + // held as a candidate up to the desert + next => createOrphanDeleteMarker(bucket, 'aaa-dm', next), + // past the desert + next => createOrphanDeleteMarker(bucket, 'zzz-dm', next), + ], + done, + ), + ); + + after(done => cleanupBucket(bucket, done)); + + it('should list both orphan delete markers across the desert', async () => { + const { pages, seededAt } = await seedThenList(bucket, 'orphan', SCAN_CAP); + assertDesertWasScanned(pages, seededAt, 'orphan'); + assert.deepStrictEqual(contentsKeys(pages), ['aaa-dm', 'zzz-dm']); + }); + }); + + describe('noncurrent crosses the desert', () => { + const bucket = uniqueBucket('lc-phd-noncurrent'); + const aaaNcVersionIds = []; + const survivorVersionIds = []; + const zzzNcVersionIds = []; + + before(done => + async.series( + [ + next => createVersionedBucket(bucket, next), + // 2 versions before the desert, older one is noncurrent + next => + async.timesSeries( + 2, + (n, cb) => + putObject(bucket, 'aaa-nc', (err, versionId) => { + aaaNcVersionIds.push(versionId); + cb(err); + }), + next, + ), + // 3 puts past the desert, newest deleted by id -> PHD master with 2 + // survivors: oldest is noncurrent, newest must stay protected + next => + async.timesSeries( + 3, + (n, cb) => + putObject(bucket, 'www-survivors', (err, versionId) => { + survivorVersionIds.push(versionId); + cb(err); + }), + next, + ), + next => deleteVersion(bucket, 'www-survivors', survivorVersionIds[2], next), + // 2 more versions past the desert, older one is noncurrent + next => + async.timesSeries( + 2, + (n, cb) => + putObject(bucket, 'zzz-nc', (err, versionId) => { + zzzNcVersionIds.push(versionId); + cb(err); + }), + next, + ), + ], + done, + ), + ); + + after(done => cleanupBucket(bucket, done)); + + it('should list noncurrent versions on both sides of the desert and protect the PHD survivor', async () => { + const { pages, seededAt } = await seedThenList(bucket, 'noncurrent', SCAN_CAP); + assertDesertWasScanned(pages, seededAt, 'noncurrent'); + const listed = contentsEntries(pages); + assert.deepStrictEqual(listed.map(entry => `${entry.Key}:${entry.VersionId}`).sort(), [ + `aaa-nc:${aaaNcVersionIds[0]}`, + `www-survivors:${survivorVersionIds[0]}`, + `zzz-nc:${zzzNcVersionIds[0]}`, + ]); + const survivorVersions = listed + .filter(entry => entry.Key === 'www-survivors') + .map(entry => entry.VersionId); + assert( + !survivorVersions.includes(survivorVersionIds[1]), + 'newest surviving version under the PHD master listed as noncurrent: ' + 'NCVE would expire live data', + ); + }); + }); +}); diff --git a/yarn.lock b/yarn.lock index 9b0e3a73ce..233af7d2dc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6591,9 +6591,9 @@ arraybuffer.prototype.slice@^1.0.4: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/Arsenal#8.4.24": - version "8.4.24" - resolved "git+https://github.com/scality/Arsenal#8c136afde33cac95a5df62b0aab7f5a9805d4004" +"arsenal@git+https://github.com/scality/Arsenal#8.4.26": + version "8.4.26" + resolved "git+https://github.com/scality/Arsenal#7d29c31f1c099ee6e0ad3b60280c81cf5cc1c29e" dependencies: "@aws-sdk/client-kms" "^3.975.0" "@aws-sdk/client-s3" "^3.975.0" From e9af6e71b912d33053f923bea4f2d954624d1e99 Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Thu, 24 Sep 2026 11:14:20 +0200 Subject: [PATCH 2/3] CLDSRV-961 bump package version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0ced078fed..14cbfb313c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.3.20", + "version": "9.3.21", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": { From 3149195a8fff9ec679b3528af5f57448f750ff94 Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Thu, 24 Sep 2026 20:46:57 +0200 Subject: [PATCH 3/3] fixup! CLDSRV-961 Fix lifecycle listings stuck on PHD master keys --- .github/docker/docker-compose.yaml | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/docker/docker-compose.yaml b/.github/docker/docker-compose.yaml index 628d7abec7..2fcb7fe99f 100644 --- a/.github/docker/docker-compose.yaml +++ b/.github/docker/docker-compose.yaml @@ -1,7 +1,7 @@ services: cloudserver: image: ${CLOUDSERVER_IMAGE} - network_mode: "host" + network_mode: 'host' volumes: - /tmp/ssl:/ssl - /tmp/ssl-kmip:/tmp/ssl-kmip @@ -56,13 +56,13 @@ services: depends_on: - redis extra_hosts: - - "bucketwebsitetester.s3-website-us-east-1.amazonaws.com:127.0.0.1" - - "pykmip.local:127.0.0.1" + - 'bucketwebsitetester.s3-website-us-east-1.amazonaws.com:127.0.0.1' + - 'pykmip.local:127.0.0.1' redis: image: redis:alpine - network_mode: "host" + network_mode: 'host' squid: - network_mode: "host" + network_mode: 'host' profiles: ['ci-proxy'] image: scality/ci-squid command: >- @@ -76,7 +76,7 @@ services: volumes: - /tmp/ssl:/ssl pykmip: - network_mode: "host" + network_mode: 'host' profiles: ['pykmip'] image: ${PYKMIP_IMAGE:-ghcr.io/scality/cloudserver/pykmip} volumes: @@ -86,17 +86,17 @@ services: - ../pykmip/policy.json:/etc/pykmip/policies/policy.json - ../pykmip/server.conf:/etc/pykmip/server.conf localkms: - network_mode: "host" + network_mode: 'host' profiles: ['localkms'] image: ${KMS_IMAGE:-nsmithuk/local-kms:3.11.7} mongo: - network_mode: "host" + network_mode: 'host' profiles: ['mongo'] image: ${MONGODB_IMAGE} volumes: - /tmp/artifacts/${JOB_NAME}:/logs sproxyd: - network_mode: "host" + network_mode: 'host' profiles: ['sproxyd'] image: sproxyd-standalone build: ./sproxyd @@ -109,7 +109,7 @@ services: profiles: ['vault'] user: root command: sh -c "chmod 400 tests/utils/keyfile && yarn start > /artifacts/vault.log 2> /artifacts/vault-stderr.log" - network_mode: "host" + network_mode: 'host' volumes: - /tmp/artifacts/${JOB_NAME}:/artifacts - ./vault-config.json:/conf/config.json:ro