Skip to content

INTEGRATION [PR#6222 > development/9.4] Handle BucketAlreadyExists races and bump arsenal for MPU afterEach fix #6234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bert-e wants to merge 5 commits into
base: development/9.4
Choose a base branch
from
Open

INTEGRATION [PR#6222 > development/9.4] Handle BucketAlreadyExists races and bump arsenal for MPU afterEach fix #6234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9b8e38
Handle BucketAlreadyExists races in CloudServer callers
benzekrimaha Jul 20, 2026
6e640a3
Format files touched by PR
benzekrimaha Jul 20, 2026
b778195
Bump Arsenal version
benzekrimaha Jul 20, 2026
a7978ae
Bump project version
benzekrimaha Jul 23, 2026
639cc42
Merge remote-tracking branch 'origin/improvement/CLDSRV-919-afterEach…
benzekrimaha Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
294 changes: 157 additions & 137 deletions lib/api/apiUtils/bucket/bucketCreation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
const zenkoSeparator = constants.zenkoSeparator;
const userBucketOwner = 'admin';


function addToUsersBucket(canonicalID, bucketName, bucketMD, log, cb) {
// BACKWARD: Simplify once do not have to deal with old
// usersbucket name and old splitter
Expand All @@ -28,8 +27,7 @@
if (err && !err.is.NoSuchBucket && !err.is.BucketAlreadyExists) {
return cb(err);
}
const splitter = usersBucketAttrs ?
constants.splitter : constants.oldSplitter;
const splitter = usersBucketAttrs ? constants.splitter : constants.oldSplitter;
let key = createKeyForUserBucket(canonicalID, splitter, bucketName);
const omVal = {
creationDate: new Date().toJSON(),
Expand All @@ -38,46 +36,44 @@
// If the new format usersbucket does not exist, try to put the
// key in the old usersBucket using the old splitter.
// Otherwise put the key in the new format usersBucket
const usersBucketBeingCalled = usersBucketAttrs ?
usersBucket : oldUsersBucket;
return metadata.putObjectMD(usersBucketBeingCalled, key,
omVal, {}, log, err => {
if (err?.is?.NoSuchBucket) {
// There must be no usersBucket so createBucket
// one using the new format
log.trace('users bucket does not exist, ' +
'creating users bucket');
key = `${canonicalID}${constants.splitter}` +
`${bucketName}`;
const creationDate = new Date().toJSON();
const freshBucket = new BucketInfo(usersBucket,
userBucketOwner, userBucketOwner, creationDate,
BucketInfo.currentModelVersion());
return metadata.createBucket(usersBucket,
freshBucket, log, err => {
// Note: In the event that two
// users' requests try to create the
// usersBucket at the same time,
// this will prevent one of the users
// from getting a BucketAlreadyExists
// error with respect
// to the usersBucket.
// TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver
if (err && !err.BucketAlreadyExists) {
log.error('error from metadata', {
error: err,
});
return cb(err);
}
log.trace('Users bucket created');
// Finally put the key in the new format
// usersBucket
return metadata.putObjectMD(usersBucket,
key, omVal, {}, log, cb);
const usersBucketBeingCalled = usersBucketAttrs ? usersBucket : oldUsersBucket;
return metadata.putObjectMD(usersBucketBeingCalled, key, omVal, {}, log, err => {
if (err?.is?.NoSuchBucket) {
// There must be no usersBucket so createBucket
// one using the new format
log.trace('users bucket does not exist, ' + 'creating users bucket');
key = `${canonicalID}${constants.splitter}` + `${bucketName}`;
const creationDate = new Date().toJSON();
const freshBucket = new BucketInfo(
usersBucket,
userBucketOwner,
userBucketOwner,
creationDate,
BucketInfo.currentModelVersion(),
);
return metadata.createBucket(usersBucket, freshBucket, log, err => {
// Note: In the event that two
// users' requests try to create the
// usersBucket at the same time,
// this will prevent one of the users
// from getting a BucketAlreadyExists
// error with respect
// to the usersBucket.
// TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver
if (err && !err.BucketAlreadyExists) {
log.error('error from metadata', {
error: err,
});
}
return cb(err);
});
return cb(err);
}
log.trace('Users bucket created');
// Finally put the key in the new format
// usersBucket
return metadata.putObjectMD(usersBucket, key, omVal, {}, log, cb);
});
}
return cb(err);
});
});
}

Expand All @@ -92,6 +88,12 @@
function freshStartCreateBucket(bucket, canonicalID, log, callback) {
const bucketName = bucket.getName();
metadata.createBucket(bucketName, bucket, log, err => {
if (err?.is?.BucketAlreadyExists) {
// The concurrent creator owns users-bucket registration and
// cleanup of the transient flag.
log.trace('bucket already exists in metadata');
return callback();
Comment thread
benzekrimaha marked this conversation as resolved.
}
if (err) {
log.debug('error from metadata', { error: err });
return callback(err);
Expand Down Expand Up @@ -139,16 +141,15 @@
* @callback called with (err, sseInfo: object)
*/
function bucketLevelServerSideEncryption(bucket, headers, log, cb) {
kms.bucketLevelEncryption(
bucket, headers, log, (err, sseInfo) => {
if (err) {
log.debug('error getting bucket encryption info', {
error: err,
});
return cb(err);
}
return cb(null, sseInfo);
});
kms.bucketLevelEncryption(bucket, headers, log, (err, sseInfo) => {
if (err) {
log.debug('error getting bucket encryption info', {
error: err,
});
return cb(err);
}
return cb(null, sseInfo);
});
}

/**
Expand All @@ -163,135 +164,154 @@
* @param {function} cb - callback to bucketPut
* @return {undefined}
*/
function createBucket(authInfo, bucketName, headers,
locationConstraint, log, cb) {
function createBucket(authInfo, bucketName, headers, locationConstraint, log, cb) {
// Prefer using undefined instead of null for unused properties so they are not present in metadata payload
log.trace('Creating bucket');
assert.strictEqual(typeof bucketName, 'string');
const canonicalID = authInfo.getCanonicalID();
const ownerDisplayName =
authInfo.getAccountDisplayName();
const ownerDisplayName = authInfo.getAccountDisplayName();
const creationDate = new Date().toJSON();
const isNFSEnabled = headers['x-scal-nfs-enabled'] === 'true' || undefined;
const headerObjectLock = headers['x-amz-bucket-object-lock-enabled'];
const objectLockEnabled
= headerObjectLock && headerObjectLock.toLowerCase() === 'true';
const bucket = new BucketInfo(bucketName, canonicalID, ownerDisplayName,
creationDate, BucketInfo.currentModelVersion(), undefined, undefined, undefined, undefined,
undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, isNFSEnabled,
undefined, undefined, objectLockEnabled);
const objectLockEnabled = headerObjectLock && headerObjectLock.toLowerCase() === 'true';
const bucket = new BucketInfo(
bucketName,
canonicalID,
ownerDisplayName,
creationDate,
BucketInfo.currentModelVersion(),
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
undefined,
isNFSEnabled,
undefined,
undefined,
objectLockEnabled,
);
let locationConstraintVal = null;

if (locationConstraint) {
const [locationConstraintStr, ingestion] =
locationConstraint.split(zenkoSeparator);
const [locationConstraintStr, ingestion] = locationConstraint.split(zenkoSeparator);
if (locationConstraintStr) {
locationConstraintVal = locationConstraintStr;
bucket.setLocationConstraint(locationConstraintStr);
}
if (ingestion === 'ingest') {
bucket.enableIngestion();
//automatically enable versioning for ingestion buckets
bucket.setVersioningConfiguration({ Status: 'Enabled' });
}
}
if (objectLockEnabled) {
// default versioning configuration AWS sets
// when a bucket is created with object lock
const versioningConfiguration = {
Status: 'Enabled',
MfaDelete: 'Disabled',
};
bucket.setVersioningConfiguration(versioningConfiguration);
}
const parseAclParams = {
headers,
resourceType: 'bucket',
acl: bucket.acl,
log,
};
async.parallel({
prepareNewBucketMD: function prepareNewBucketMD(callback) {
acl.parseAclFromHeaders(parseAclParams, (err, parsedACL) => {
if (err) {
log.debug('error parsing acl from headers', {
error: err,
});
return callback(err);
}
bucket.setFullAcl(parsedACL);
return callback(null, bucket);
});
},
getAnyExistingBucketInfo: function getAnyExistingBucketInfo(callback) {
metadata.getBucket(bucketName, log, (err, data) => {
// TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver
if (err && err.NoSuchBucket) {
return callback(null, 'NoBucketYet');
}
if (err) {
return callback(err);
}
return callback(null, data);
});
async.parallel(
{
prepareNewBucketMD: function prepareNewBucketMD(callback) {
acl.parseAclFromHeaders(parseAclParams, (err, parsedACL) => {
if (err) {
log.debug('error parsing acl from headers', {
error: err,
});
return callback(err);
}
bucket.setFullAcl(parsedACL);
return callback(null, bucket);
});
},

Check notice

Code scanning / CodeQL

Callback-style function (async migration) Note

This function uses a callback parameter ('callback'). Refactor to async/await.
Comment on lines +232 to -243
getAnyExistingBucketInfo: function getAnyExistingBucketInfo(callback) {
metadata.getBucket(bucketName, log, (err, data) => {
// TODO: move to `.is` once BKTCLT-9 is done and bumped in Cloudserver
if (err && err.NoSuchBucket) {
return callback(null, 'NoBucketYet');
}
if (err) {
return callback(err);
}
return callback(null, data);
});
},
},
},
// Function to run upon finishing both parallel requests
(err, results) => {
if (err) {
return cb(err);
}
const existingBucketMD = results.getAnyExistingBucketInfo;
if (existingBucketMD instanceof BucketInfo &&
existingBucketMD.getOwner() !== canonicalID &&
!isServiceAccount(canonicalID)) {
// return existingBucketMD to collect cors headers
return cb(errors.BucketAlreadyExists, existingBucketMD);
}
const newBucketMD = results.prepareNewBucketMD;
if (existingBucketMD === 'NoBucketYet') {
const bucketSseConfig = parseBucketEncryptionHeaders(headers);
// Function to run upon finishing both parallel requests
(err, results) => {
if (err) {
return cb(err);
}
const existingBucketMD = results.getAnyExistingBucketInfo;
if (
existingBucketMD instanceof BucketInfo &&
existingBucketMD.getOwner() !== canonicalID &&
!isServiceAccount(canonicalID)
) {
// return existingBucketMD to collect cors headers
return cb(errors.BucketAlreadyExists, existingBucketMD);
}
const newBucketMD = results.prepareNewBucketMD;
if (existingBucketMD === 'NoBucketYet') {
const bucketSseConfig = parseBucketEncryptionHeaders(headers);

// Apply global SSE configuration when global encryption is enabled
// and no SSE settings were specified during bucket creation.
// Bucket-specific SSE headers override the default encryption.
const sseConfig = config.globalEncryptionEnabled && !bucketSseConfig.algorithm
? {
algorithm: 'AES256',
mandatory: true,
} : bucketSseConfig;
// Apply global SSE configuration when global encryption is enabled
// and no SSE settings were specified during bucket creation.
// Bucket-specific SSE headers override the default encryption.
const sseConfig =
config.globalEncryptionEnabled && !bucketSseConfig.algorithm
? {
algorithm: 'AES256',
mandatory: true,
}
: bucketSseConfig;

return bucketLevelServerSideEncryption(
bucket, sseConfig, log,
(err, sseInfo) => {
return bucketLevelServerSideEncryption(bucket, sseConfig, log, (err, sseInfo) => {
if (err) {
return cb(err);
}
newBucketMD.setServerSideEncryption(sseInfo);
log.trace(
'new bucket without flags; adding transient label');
log.trace('new bucket without flags; adding transient label');
newBucketMD.addTransientFlag();
return freshStartCreateBucket(newBucketMD, canonicalID,
log, cb);
return freshStartCreateBucket(newBucketMD, canonicalID, log, cb);
});
}
if (existingBucketMD.hasTransientFlag() ||
existingBucketMD.hasDeletedFlag()) {
log.trace('bucket has transient flag or deleted flag. cleaning up');
return cleanUpBucket(newBucketMD, canonicalID, log, cb);
}
// If bucket already exists in non-transient and non-deleted
// state and owned by requester, then return BucketAlreadyOwnedByYou
// error unless old AWS behavior (us-east-1)
// Existing locationConstraint must have legacyAwsBehavior === true
// New locationConstraint should have legacyAwsBehavior === true
if (isLegacyAWSBehavior(locationConstraintVal) &&
isLegacyAWSBehavior(existingBucketMD.getLocationConstraint())) {
log.trace('returning 200 instead of 409 to mirror us-east-1');
return cb(null, existingBucketMD);
}
return cb(errors.BucketAlreadyOwnedByYou, existingBucketMD);
});
}
if (existingBucketMD.hasTransientFlag() || existingBucketMD.hasDeletedFlag()) {
log.trace('bucket has transient flag or deleted flag. cleaning up');
return cleanUpBucket(newBucketMD, canonicalID, log, cb);
}
// If bucket already exists in non-transient and non-deleted
// state and owned by requester, then return BucketAlreadyOwnedByYou
// error unless old AWS behavior (us-east-1)
// Existing locationConstraint must have legacyAwsBehavior === true
// New locationConstraint should have legacyAwsBehavior === true
if (
isLegacyAWSBehavior(locationConstraintVal) &&
isLegacyAWSBehavior(existingBucketMD.getLocationConstraint())
) {
log.trace('returning 200 instead of 409 to mirror us-east-1');
return cb(null, existingBucketMD);
}
return cb(errors.BucketAlreadyOwnedByYou, existingBucketMD);
},
);
}

Check notice

Code scanning / CodeQL

Callback-style function (async migration) Note

This function uses a callback parameter ('cb'). Refactor to async/await.

module.exports = {
Expand Down
4 changes: 4 additions & 0 deletions lib/services.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,6 +974,10 @@ const services = {
// bucket. This is the desired behavior since this should be
// a hidden bucket.
return metadata.createBucket(MPUBucketName, mpuBucket, log, err => {
if (err?.is?.BucketAlreadyExists) {
log.trace('mpu bucket already exists in metadata');
return cb(null, mpuBucket);
}
if (err) {
log.error('error from metadata', { error: err });
return cb(err);
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"@opentelemetry/instrumentation-ioredis": "~0.64.0",
"@opentelemetry/instrumentation-mongodb": "~0.69.0",
"@smithy/node-http-handler": "^3.0.0",
"arsenal": "git+https://github.com/scality/arsenal#8.5.6",
"arsenal": "git+https://github.com/scality/arsenal#8.5.12",
"async": "2.6.4",
"aws-crt": "^1.24.0",
"bucketclient": "scality/bucketclient#8.2.7",
Expand Down
Loading
Loading