Skip to content

Commit

Permalink
Merge branches 'development/8.8' and 'w/8.7/bugfix/CLDSRV-529/bump_ut…
Browse files Browse the repository at this point in the history
…api' into tmp/octopus/w/8.8/bugfix/CLDSRV-529/bump_utapi
  • Loading branch information
bert-e committed May 17, 2024
3 parents 8f25892 + 926242b + 47c628e commit 09dc452
Show file tree
Hide file tree
Showing 11 changed files with 203 additions and 31 deletions.
4 changes: 2 additions & 2 deletions lib/api/apiUtils/object/objectRestore.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ function objectRestore(metadata, mdUtils, userInfo, request, log, callback) {
const actions = Array.isArray(mdValueParams.requestType) ?
mdValueParams.requestType : [mdValueParams.requestType];
const bytes = processBytesToWrite(request.apiMethod, bucketMD, mdValueParams.versionId, 0, objectMD);
return validateQuotas(request, bucketMD, request.accountQuotas, actions, request.apiMethod, bytes, log,
err => next(err, bucketMD, objectMD));
return validateQuotas(request, bucketMD, request.accountQuotas, actions, request.apiMethod, bytes,
false, log, err => next(err, bucketMD, objectMD));
},
function updateObjectMD(bucketMD, objectMD, next) {
const params = objectMD.versionId ? { versionId: objectMD.versionId } : {};
Expand Down
14 changes: 11 additions & 3 deletions lib/api/apiUtils/quotas/quotaUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,14 @@ function monitorQuotaEvaluationDuration(apiMethod, type, code, duration) {
* @param {array} apiNames - action names: operations to authorize
* @param {string} apiMethod - the main API call
* @param {number} inflight - inflight bytes
* @param {boolean} isStorageReserved - Flag to check if the current quota, minus
* the incoming bytes, are under the limit.
* @param {Logger} log - logger
* @param {function} callback - callback function
* @returns {boolean} - true if the quota is valid, false otherwise
*/
function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight, log, callback) {
if (!config.isQuotaEnabled() || !inflight) {
function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight, isStorageReserved, log, callback) {
if (!config.isQuotaEnabled() || (!inflight && isStorageReserved)) {
return callback(null);
}
let type;
Expand Down Expand Up @@ -228,6 +230,11 @@ function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight,
return callback(null);
}

if (isStorageReserved) {
// eslint-disable-next-line no-param-reassign
inflight = 0;
}

return async.forEach(apiNames, (apiName, done) => {
// Object copy operations first check the target object,
// meaning the source object, containing the current bytes,
Expand All @@ -248,7 +255,8 @@ function validateQuotas(request, bucket, account, apiNames, apiMethod, inflight,
let _inflights = shouldSendInflights ? inflight : undefined;
const inflightForCheck = shouldSendInflights ? 0 : inflight;
return _evaluateQuotas(bucketQuota, accountQuota, bucket, account, _inflights,
inflightForCheck, apiName, log, (err, _bucketQuotaExceeded, _accountQuotaExceeded) => {
inflightForCheck, apiName, log,
(err, _bucketQuotaExceeded, _accountQuotaExceeded) => {
if (err) {
return done(err);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request,
},
(objMD, versionId, callback) => validateQuotas(
request, bucket, request.accountQuotas, ['objectDelete'], 'objectDelete',
-objMD?.['content-length'] || 0, log, err => callback(err, objMD, versionId)),
-objMD?.['content-length'] || 0, false, log, err => callback(err, objMD, versionId)),
(objMD, versionId, callback) => {
const options = preprocessingVersioningDelete(
bucketName, bucket, objMD, versionId, config.nullVersionCompatMode);
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectPut.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function objectPut(authInfo, request, streamingV4Params, log, callback) {
'The encryption method specified is not supported');
const requestType = request.apiMethods || 'objectPut';
const valParams = { authInfo, bucketName, objectKey, versionId,
requestType, request };
requestType, request, withVersionId: isPutVersion };
const canonicalID = authInfo.getCanonicalID();

if (hasNonPrintables(objectKey)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectPutCopyPart.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket,
copyObjectSize, sourceVerId,
sourceLocationConstraintName, sourceObjMD, next) {
return validateQuotas(request, destBucketMD, request.accountQuotas, valPutParams.requestType,
request.apiMethod, sourceObjMD?.['content-length'] || 0, log, err =>
request.apiMethod, sourceObjMD?.['content-length'] || 0, false, log, err =>
next(err, dataLocator, destBucketMD, copyObjectSize, sourceVerId, sourceLocationConstraintName));
},
// get MPU shadow bucket to get splitter based on MD version
Expand Down
5 changes: 4 additions & 1 deletion lib/api/objectPutPart.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ function objectPutPart(authInfo, request, streamingV4Params, log,
log.debug('processing request', { method: 'objectPutPart' });
const size = request.parsedContentLength;

const putVersionId = request.headers['x-scal-s3-version-id'];
const isPutVersion = putVersionId || putVersionId === '';

if (Number.parseInt(size, 10) > constants.maximumAllowedPartSize) {
log.debug('put part size too large', { size });
monitoring.promMetrics('PUT', request.bucketName, 400,
Expand Down Expand Up @@ -134,7 +137,7 @@ function objectPutPart(authInfo, request, streamingV4Params, log,
return next(null, destinationBucket);
},
(destinationBucket, next) => validateQuotas(request, destinationBucket, request.accountQuotas,
requestType, request.apiMethod, size, log, err => next(err, destinationBucket)),
requestType, request.apiMethod, size, isPutVersion, log, err => next(err, destinationBucket)),
// Get bucket server-side encryption, if it exists.
(destinationBucket, next) => getObjectSSEConfiguration(
request.headers, destinationBucket, log,
Expand Down
7 changes: 5 additions & 2 deletions lib/metadata/metadataUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function validateBucket(bucket, params, log, actionImplicitDenies = {}) {
* @return {undefined} - and call callback with params err, bucket md
*/
function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log, callback) {
const { authInfo, bucketName, objectKey, versionId, getDeleteMarker, request } = params;
const { authInfo, bucketName, objectKey, versionId, getDeleteMarker, request, withVersionId } = params;
let requestType = params.requestType;
if (!Array.isArray(requestType)) {
requestType = [requestType];
Expand Down Expand Up @@ -242,13 +242,16 @@ function standardMetadataValidateBucketAndObj(params, actionImplicitDenies, log,
const needQuotaCheck = requestType => requestType.some(type => actionNeedQuotaCheck[type] ||
actionWithDataDeletion[type]);
const checkQuota = params.checkQuota === undefined ? needQuotaCheck(requestType) : params.checkQuota;
// withVersionId cover cases when an object is being restored with a specific version ID.
// In this case, the storage space was already accounted for when the RestoreObject API call
// was made, so we don't need to add any inflight, but quota must be evaluated.
if (!checkQuota) {
return next(null, bucket, objMD);
}
const contentLength = processBytesToWrite(request.apiMethod, bucket, versionId,
request?.parsedContentLength || 0, objMD, params.destObjMD);
return validateQuotas(request, bucket, request.accountQuotas, requestType, request.apiMethod,
contentLength, log, err => next(err, bucket, objMD));
contentLength, withVersionId, log, err => next(err, bucket, objMD));
},
], (err, bucket, objMD) => {
if (err) {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zenko/cloudserver",
"version": "8.8.23",
"version": "8.8.24",
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
"main": "index.js",
"engines": {
Expand Down Expand Up @@ -43,7 +43,7 @@
"request": "^2.81.0",
"scubaclient": "git+https://github.com/scality/scubaclient.git",
"sql-where-parser": "~2.2.1",
"utapi": "git+https://github.com/scality/utapi#8.1.14",
"utapi": "github:scality/utapi#8.1.14",
"utf-8-validate": "^5.0.8",
"utf8": "~2.1.1",
"uuid": "^8.3.2",
Expand Down
102 changes: 102 additions & 0 deletions tests/quota/awsNodeSdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,25 @@ function putObject(bucket, key, size, cb) {
});
}

function putObjectWithCustomHeader(bucket, key, size, vID, cb) {
const request = s3Client.putObject({
Bucket: bucket,
Key: key,
Body: Buffer.alloc(size),
});

request.on('build', () => {
request.httpRequest.headers['x-scal-s3-version-id'] = vID;
});

return request.send((err, data) => {
if (!err && !s3Config.isQuotaInflightEnabled()) {
mockScuba.incrementBytesForBucket(bucket, 0);
}
return cb(err, data);
});
}

function copyObject(bucket, key, sourceSize, cb) {
return s3Client.copyObject({
Bucket: bucket,
Expand Down Expand Up @@ -680,4 +699,87 @@ function multiObjectDelete(bucket, keys, size, callback) {
},
], done);
});

it('should only evaluate quota and not update inflights for PutObject with the x-scal-s3-version-id header',
done => {
const bucket = 'quota-test-bucket13';
const key = 'quota-test-object';
const size = 100;
let vID = null;
return async.series([
next => createBucket(bucket, true, next),
next => sendRequest(putQuotaVerb, '127.0.0.1:8000', `/${bucket}/?quota=true`,
JSON.stringify(quota), config).then(() => next()).catch(err => next(err)),
next => putObject(bucket, key, size, (err, val) => {
assert.ifError(err);
vID = val.VersionId;
return next();
}),
next => wait(inflightFlushFrequencyMS * 2, next),
next => {
assert.strictEqual(scuba.getInflightsForBucket(bucket), size);
return next();
},
next => fakeMetadataArchive(bucket, key, vID, {
archiveInfo: {},
restoreRequestedAt: new Date(0).toISOString(),
restoreRequestedDays: 7,
}, next),
// Simulate the real restore
next => putObjectWithCustomHeader(bucket, key, size, vID, err => {
assert.ifError(err);
return next();
}),
next => {
assert.strictEqual(scuba.getInflightsForBucket(bucket), size);
return next();
},
next => deleteVersionID(bucket, key, vID, size, next),
next => deleteBucket(bucket, next),
], done);
});

it('should allow a restore if the quota is full but the objet fits with its reserved storage space',
done => {
const bucket = 'quota-test-bucket15';
const key = 'quota-test-object';
const size = 1000;
let vID = null;
return async.series([
next => createBucket(bucket, true, next),
next => sendRequest(putQuotaVerb, '127.0.0.1:8000', `/${bucket}/?quota=true`,
JSON.stringify(quota), config).then(() => next()).catch(err => next(err)),
next => putObject(bucket, key, size, (err, val) => {
assert.ifError(err);
vID = val.VersionId;
return next();
}),
next => wait(inflightFlushFrequencyMS * 2, next),
next => {
assert.strictEqual(scuba.getInflightsForBucket(bucket), size);
return next();
},
next => fakeMetadataArchive(bucket, key, vID, {
archiveInfo: {},
restoreRequestedAt: new Date(0).toISOString(),
restoreRequestedDays: 7,
}, next),
// Put an object, the quota should be exceeded
next => putObject(bucket, `${key}-2`, size, err => {
assert.strictEqual(err.code, 'QuotaExceeded');
return next();
}),
// Simulate the real restore
next => putObjectWithCustomHeader(bucket, key, size, vID, err => {
assert.ifError(err);
return next();
}),
next => {
assert.strictEqual(scuba.getInflightsForBucket(bucket), size);
return next();
},
next => deleteVersionID(bucket, key, vID, size, next),
next => deleteBucket(bucket, next),
], done);
});
});
Loading

0 comments on commit 09dc452

Please sign in to comment.