From f5c1240c0152b262b11adfe94eed7a69d17dfade Mon Sep 17 00:00:00 2001 From: Dimitrios Vasilas Date: Wed, 18 Dec 2024 13:31:55 +0200 Subject: [PATCH 1/2] CLDSRV-598: Add overhreadField to md-only ops Metadata-only operations (objectPutRetention, objectPutLegalHold, objectPutTagging, objectPutACL) overwrite the metadata of an object. These operations currently do not attach the overheadField to metadata put operations. As a result, the entries in the metadata op log for those operations do not include overhead information, and scuba interprets them as object puts, resulting in wrong metric updates. --- lib/api/objectPutACL.js | 2 ++ lib/api/objectPutLegalHold.js | 2 ++ lib/api/objectPutRetention.js | 2 ++ lib/api/objectPutTagging.js | 3 ++ tests/unit/api/objectPutACL.js | 52 ++++++++++++++++++++++++++++ tests/unit/api/objectPutLegalHold.js | 30 ++++++++++++++++ tests/unit/api/objectPutRetention.js | 30 ++++++++++++++++ tests/unit/api/objectPutTagging.js | 33 ++++++++++++++++++ 8 files changed, 154 insertions(+) diff --git a/lib/api/objectPutACL.js b/lib/api/objectPutACL.js index b5fbf1e815..4a1826a898 100644 --- a/lib/api/objectPutACL.js +++ b/lib/api/objectPutACL.js @@ -12,6 +12,7 @@ const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOption const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const monitoring = require('../utilities/metrics'); const { config } = require('../Config'); +const { overheadField } = require('../../constants'); /* Format of xml request: @@ -282,6 +283,7 @@ function objectPutACL(authInfo, request, log, cb) { function addAclsToObjMD(bucket, objectMD, ACLParams, next) { // Add acl's to object metadata const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); + params.overheadField = overheadField; acl.addObjectACL(bucket, objectKey, objectMD, ACLParams, params, log, err => next(err, bucket, objectMD)); }, diff --git a/lib/api/objectPutLegalHold.js b/lib/api/objectPutLegalHold.js index e5ba15b44b..686e19d716 100644 --- a/lib/api/objectPutLegalHold.js +++ b/lib/api/objectPutLegalHold.js @@ -9,6 +9,7 @@ const metadata = require('../metadata/wrapper'); const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); const { config } = require('../Config'); +const { overheadField } = require('../../constants'); const { parseLegalHoldXml } = s3middleware.objectLegalHold; @@ -87,6 +88,7 @@ function objectPutLegalHold(authInfo, request, log, callback) { // eslint-disable-next-line no-param-reassign objectMD.legalHold = legalHold; const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); + params.overheadField = overheadField; const replicationInfo = getReplicationInfo(objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { diff --git a/lib/api/objectPutRetention.js b/lib/api/objectPutRetention.js index 9bbf9fca14..685a8a95fe 100644 --- a/lib/api/objectPutRetention.js +++ b/lib/api/objectPutRetention.js @@ -11,6 +11,7 @@ const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); const { config } = require('../Config'); +const { overheadField } = require('../../constants'); const { parseRetentionXml } = s3middleware.retention; const REPLICATION_ACTION = 'PUT_RETENTION'; @@ -125,6 +126,7 @@ function objectPutRetention(authInfo, request, log, callback) { objectMD.retentionMode = retentionInfo.mode; objectMD.retentionDate = retentionInfo.date; const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); + params.overheadField = overheadField; const replicationInfo = getReplicationInfo(objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { diff --git a/lib/api/objectPutTagging.js b/lib/api/objectPutTagging.js index d5881ef51e..a93073a851 100644 --- a/lib/api/objectPutTagging.js +++ b/lib/api/objectPutTagging.js @@ -13,6 +13,8 @@ const metadata = require('../metadata/wrapper'); const { data } = require('../data/wrapper'); const { parseTagXml } = s3middleware.tagging; const { config } = require('../Config'); +const { overheadField } = require('../../constants'); + const REPLICATION_ACTION = 'PUT_TAGGING'; /** @@ -81,6 +83,7 @@ function objectPutTagging(authInfo, request, log, callback) { // eslint-disable-next-line no-param-reassign objectMD.tags = tags; const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); + params.overheadField = overheadField; const replicationInfo = getReplicationInfo(objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { diff --git a/tests/unit/api/objectPutACL.js b/tests/unit/api/objectPutACL.js index 83a2d8caea..a6541b6023 100644 --- a/tests/unit/api/objectPutACL.js +++ b/tests/unit/api/objectPutACL.js @@ -1,5 +1,6 @@ const assert = require('assert'); const async = require('async'); +const sinon = require('sinon'); const { errors } = require('arsenal'); const AuthInfo = require('arsenal').auth.AuthInfo; @@ -480,6 +481,57 @@ describe('putObjectACL API', () => { }); }); }); + + describe('overheadField', () => { + before(done => { + cleanup(); + sinon.spy(metadata, 'putObjectMD'); + return done(); + }); + + after(() => { + metadata.putObjectMD.restore(); + cleanup(); + }); + + it('should pass overheadField', done => { + const testObjACLRequest = { + bucketName, + namespace, + objectKey: objectName, + headers: { + 'x-amz-grant-full-control': + 'emailaddress="sampleaccount1@sampling.com"' + + ',emailaddress="sampleaccount2@sampling.com"', + 'x-amz-grant-read': `uri=${constants.logId}`, + 'x-amz-grant-read-acp': `id=${ownerID}`, + 'x-amz-grant-write-acp': `id=${anotherID}`, + }, + url: `/${bucketName}/${objectName}?acl`, + query: { acl: '' }, + actionImplicitDenies: false, + }; + bucketPut(authInfo, testPutBucketRequest, log, () => { + objectPut(authInfo, testPutObjectRequest, undefined, log, + (err) => { + assert.ifError(err); + objectPutACL(authInfo, testObjACLRequest, log, err => { + assert.ifError(err); + sinon.assert.calledWith( + metadata.putObjectMD, + sinon.match.string, + objectName, + sinon.match.any, + sinon.match({ overheadField: sinon.match.array }), + sinon.match.any, + sinon.match.any + ); + done(); + }); + }); + }); + }); + }); }); describe('with bucket policy', () => { diff --git a/tests/unit/api/objectPutLegalHold.js b/tests/unit/api/objectPutLegalHold.js index 839d7d7edf..7ae0affcf3 100644 --- a/tests/unit/api/objectPutLegalHold.js +++ b/tests/unit/api/objectPutLegalHold.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const sinon = require('sinon'); const { bucketPut } = require('../../../lib/api/bucketPut'); const objectPut = require('../../../lib/api/objectPut'); @@ -98,5 +99,34 @@ describe('putObjectLegalHold API', () => { }); }); }); + + describe('overheadField', () => { + before(done => { + cleanup(); + sinon.spy(metadata, 'putObjectMD'); + return done(); + }); + + after(() => { + metadata.putObjectMD.restore(); + cleanup(); + }); + + it('should pass overheadField', done => { + objectPutLegalHold(authInfo, putLegalHoldReq('OFF'), log, err => { + assert.ifError(err); + sinon.assert.calledWith( + metadata.putObjectMD, + sinon.match.string, + objectName, + sinon.match.any, + sinon.match({ overheadField: sinon.match.array }), + sinon.match.any, + sinon.match.any + ); + done(); + }); + }); + }); }); }); diff --git a/tests/unit/api/objectPutRetention.js b/tests/unit/api/objectPutRetention.js index 9a24d7a340..537b0bc659 100644 --- a/tests/unit/api/objectPutRetention.js +++ b/tests/unit/api/objectPutRetention.js @@ -1,5 +1,6 @@ const assert = require('assert'); const moment = require('moment'); +const sinon = require('sinon'); const { errors } = require('arsenal'); const { bucketPut } = require('../../../lib/api/bucketPut'); @@ -223,5 +224,34 @@ describe('putObjectRetention API', () => { }); }); }); + + describe('overheadField', () => { + before(done => { + cleanup(); + sinon.spy(metadata, 'putObjectMD'); + return done(); + }); + + after(() => { + metadata.putObjectMD.restore(); + cleanup(); + }); + + it('should pass overheadField', done => { + objectPutRetention(authInfo, putObjRetRequestGovernance, log, err => { + assert.ifError(err); + sinon.assert.calledWith( + metadata.putObjectMD, + sinon.match.string, + objectName, + sinon.match.any, + sinon.match({ overheadField: sinon.match.array }), + sinon.match.any, + sinon.match.any + ); + done(); + }); + }); + }); }); }); diff --git a/tests/unit/api/objectPutTagging.js b/tests/unit/api/objectPutTagging.js index faa6d9a188..b5cdca44cb 100644 --- a/tests/unit/api/objectPutTagging.js +++ b/tests/unit/api/objectPutTagging.js @@ -1,4 +1,5 @@ const assert = require('assert'); +const sinon = require('sinon'); const { bucketPut } = require('../../../lib/api/bucketPut'); const objectPut = require('../../../lib/api/objectPut'); @@ -91,6 +92,38 @@ describe('putObjectTagging API', () => { }); }); }); + + describe('overheadField', () => { + before(done => { + cleanup(); + sinon.spy(metadata, 'putObjectMD'); + return done(); + }); + + after(() => { + metadata.putObjectMD.restore(); + cleanup(); + }); + + it('should pass overheadField', done => { + const taggingUtil = new TaggingConfigTester(); + const testObjectPutTaggingRequest = taggingUtil + .createObjectTaggingRequest('PUT', bucketName, objectName); + objectPutTagging(authInfo, testObjectPutTaggingRequest, log, err => { + assert.ifError(err); + sinon.assert.calledWith( + metadata.putObjectMD, + sinon.match.string, + objectName, + sinon.match.any, + sinon.match({ overheadField: sinon.match.array }), + sinon.match.any, + sinon.match.any + ); + done(); + }); + }); + }); }); describe('PUT object tagging :: helper validation functions ', () => { From 1681dac2fd54236a90e06251468d151d031069ca Mon Sep 17 00:00:00 2001 From: Dimitrios Vasilas Date: Wed, 18 Dec 2024 16:42:53 +0200 Subject: [PATCH 2/2] CLDSRV-598: Bump version to 7.70.59 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6a4ca3a19e..1a01de72c0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.70.58", + "version": "7.70.59", "description": "S3 connector", "main": "index.js", "engines": {