diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index 98174dd6dd..aa7645e012 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -360,60 +360,86 @@ function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD, }); } +/** Return options to pass to Metadata layer for version-specific + * operations with the given requested version ID + * + * @param {object} objectMD - object metadata + * @param {boolean} nullVersionCompatMode - if true, behaves in null + * version compatibility mode + * @return {object} options object with params: + * {string} [options.versionId] - specific versionId to update + * {boolean} [options.isNull=true|false|undefined] - if set, tells the + * Metadata backend if we're updating or deleting a new-style null + * version (stored in master or null key), or not a null version. + */ +function getVersionSpecificMetadataOptions(objectMD, nullVersionCompatMode) { + // Use the internal versionId if it is a "real" null version (not + // non-versioned) + // + // If the target object is non-versioned: do not specify a + // "versionId" attribute nor "isNull" + // + // If the target version is a null version, i.e. has the "isNull" + // attribute: + // + // - send the "isNull=true" param to Metadata if the version is + // already a null key put by a non-compat mode Cloudserver, to + // let Metadata know that the null key is to be updated or + // deleted. This is the case if the "isNull2" metadata attribute + // exists + // + // - otherwise, do not send the "isNull" parameter to hint + // Metadata that it is a legacy null version + // + // If the target version is not a null version and is versioned: + // + // - send the "isNull=false" param to Metadata in non-compat + // mode (mandatory for v1 format) + // + // - otherwise, do not send the "isNull" parameter to hint + // Metadata that an existing null version may not be stored in a + // null key + // + // + if (objectMD.versionId === undefined) { + return {}; + } + const options = { versionId: objectMD.versionId }; + if (objectMD.isNull) { + if (objectMD.isNull2) { + options.isNull = true; + } + } else if (!nullVersionCompatMode) { + options.isNull = false; + } + return options; +} + /** preprocessingVersioningDelete - return versioning information for S3 to * manage deletion of objects and versions, including creation of delete markers * @param {string} bucketName - name of bucket * @param {object} bucketMD - bucket metadata * @param {object} objectMD - obj metadata * @param {string} [reqVersionId] - specific version ID sent as part of request - * @param {boolean} nullVersionCompatMode - if true, behaves in null - * version compatibility mode and return appropriate values: - * - in normal mode, returns an 'isNull' boolean sent to Metadata (true or false) - * - in compatibility mode, does not return an 'isNull' property + * @param {boolean} nullVersionCompatMode - if true, behaves in null version compatibility mode * @return {object} options object with params: * {boolean} [options.deleteData=true|undefined] - whether to delete data (if undefined * means creating a delete marker instead) * {string} [options.versionId] - specific versionId to delete * {boolean} [options.isNull=true|false|undefined] - if set, tells the - * Metadata backend if we're deleting a null version or not a null - * version. Not set if `nullVersionCompatMode` is true. + * Metadata backend if we're deleting a new-style null version (stored + * in master or null key), or not a null version. */ function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersionId, nullVersionCompatMode) { - const options = {}; + let options = {}; + if (bucketMD.getVersioningConfiguration() && reqVersionId) { + options = getVersionSpecificMetadataOptions(objectMD, nullVersionCompatMode); + } if (!bucketMD.getVersioningConfiguration() || reqVersionId) { // delete data if bucket is non-versioned or the request // deletes a specific version options.deleteData = true; } - if (bucketMD.getVersioningConfiguration() && reqVersionId) { - if (reqVersionId === 'null') { - // deleting the 'null' version if it exists: - // - // - use its internal versionId if it is a "real" null - // version (not non-versioned) - // - // - send the "isNull" param to Metadata if: - // - // - in non-compat mode (mandatory for v1 format) - // - // - OR if the version is already a null key put by a - // non-compat mode Cloudserver, to let Metadata know that - // the null key is to be deleted. This is the case if the - // "isNull2" param is set. - if (objectMD.versionId !== undefined) { - options.versionId = objectMD.versionId; - if (objectMD.isNull2) { - options.isNull = true; - } - } - } else { - // deleting a specific version - options.versionId = reqVersionId; - if (!nullVersionCompatMode) { - options.isNull = false; - } - } - } return options; } @@ -424,5 +450,6 @@ module.exports = { processVersioningState, getMasterState, versioningPreprocessing, + getVersionSpecificMetadataOptions, preprocessingVersioningDelete, }; diff --git a/lib/api/objectDeleteTagging.js b/lib/api/objectDeleteTagging.js index e16fd37df1..b2a465298d 100644 --- a/lib/api/objectDeleteTagging.js +++ b/lib/api/objectDeleteTagging.js @@ -1,7 +1,7 @@ const async = require('async'); const { errors } = require('arsenal'); -const { decodeVersionId, getVersionIdResHeader } +const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOptions } = require('./apiUtils/object/versioning'); const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); @@ -75,13 +75,7 @@ function objectDeleteTagging(authInfo, request, log, callback) { (bucket, objectMD, next) => { // eslint-disable-next-line no-param-reassign objectMD.tags = {}; - const params = {}; - if (objectMD.versionId) { - params.versionId = objectMD.versionId; - if (!config.nullVersionCompatMode) { - params.isNull = objectMD.isNull || false; - } - } + const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); const replicationInfo = getReplicationInfo(objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { diff --git a/lib/api/objectPutACL.js b/lib/api/objectPutACL.js index ec040cb322..52cbefc8be 100644 --- a/lib/api/objectPutACL.js +++ b/lib/api/objectPutACL.js @@ -7,7 +7,7 @@ const { pushMetric } = require('../utapi/utilities'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const constants = require('../../constants'); const vault = require('../auth/vault'); -const { decodeVersionId, getVersionIdResHeader } +const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOptions } = require('./apiUtils/object/versioning'); const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const monitoring = require('../utilities/metrics'); @@ -281,13 +281,7 @@ function objectPutACL(authInfo, request, log, cb) { }, function addAclsToObjMD(bucket, objectMD, ACLParams, next) { // Add acl's to object metadata - const params = {}; - if (objectMD.versionId) { - params.versionId = objectMD.versionId; - if (!config.nullVersionCompatMode) { - params.isNull = objectMD.isNull || false; - } - } + const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); 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 5d4a62708e..6d4c123de5 100644 --- a/lib/api/objectPutLegalHold.js +++ b/lib/api/objectPutLegalHold.js @@ -2,7 +2,7 @@ const async = require('async'); const { errors, s3middleware } = require('arsenal'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); -const { decodeVersionId, getVersionIdResHeader } = +const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOptions } = require('./apiUtils/object/versioning'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const metadata = require('../metadata/wrapper'); @@ -86,13 +86,7 @@ function objectPutLegalHold(authInfo, request, log, callback) { (bucket, legalHold, objectMD, next) => { // eslint-disable-next-line no-param-reassign objectMD.legalHold = legalHold; - const params = {}; - if (objectMD.versionId) { - params.versionId = objectMD.versionId; - if (!config.nullVersionCompatMode) { - params.isNull = objectMD.isNull || false; - } - } + const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); 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 da8eb79b23..b77cccbe46 100644 --- a/lib/api/objectPutRetention.js +++ b/lib/api/objectPutRetention.js @@ -1,7 +1,7 @@ const async = require('async'); const { errors, s3middleware } = require('arsenal'); -const { decodeVersionId, getVersionIdResHeader } = +const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOptions } = require('./apiUtils/object/versioning'); const { ObjectLockInfo, checkUserGovernanceBypass, hasGovernanceBypassHeader } = require('./apiUtils/object/objectLockHelpers'); @@ -116,13 +116,7 @@ function objectPutRetention(authInfo, request, log, callback) { /* eslint-disable no-param-reassign */ objectMD.retentionMode = retentionInfo.mode; objectMD.retentionDate = retentionInfo.date; - const params = {}; - if (objectMD.versionId) { - params.versionId = objectMD.versionId; - if (!config.nullVersionCompatMode) { - params.isNull = objectMD.isNull || false; - } - } + const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); 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 c37b11c569..038a43c612 100644 --- a/lib/api/objectPutTagging.js +++ b/lib/api/objectPutTagging.js @@ -1,7 +1,7 @@ const async = require('async'); const { errors, s3middleware } = require('arsenal'); -const { decodeVersionId, getVersionIdResHeader } = +const { decodeVersionId, getVersionIdResHeader, getVersionSpecificMetadataOptions } = require('./apiUtils/object/versioning'); const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils'); @@ -81,13 +81,7 @@ function objectPutTagging(authInfo, request, log, callback) { (bucket, tags, objectMD, next) => { // eslint-disable-next-line no-param-reassign objectMD.tags = tags; - const params = {}; - if (objectMD.versionId) { - params.versionId = objectMD.versionId; - if (!config.nullVersionCompatMode) { - params.isNull = objectMD.isNull || false; - } - } + const params = getVersionSpecificMetadataOptions(objectMD, config.nullVersionCompatMode); const replicationInfo = getReplicationInfo(objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { diff --git a/package.json b/package.json index ab391695c3..08ef3ae106 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.70.27", + "version": "7.70.28", "description": "S3 connector", "main": "index.js", "engines": { diff --git a/tests/functional/aws-node-sdk/test/versioning/legacyNullVersionCompat.js b/tests/functional/aws-node-sdk/test/versioning/legacyNullVersionCompat.js new file mode 100644 index 0000000000..ce77ac50fe --- /dev/null +++ b/tests/functional/aws-node-sdk/test/versioning/legacyNullVersionCompat.js @@ -0,0 +1,156 @@ +const assert = require('assert'); +const async = require('async'); + +const BucketUtility = require('../../lib/utility/bucket-util'); + +const { + removeAllVersions, + versioningEnabled, +} = require('../../lib/utility/versioning-util.js'); + +// This series of tests can only be enabled on an environment that has +// two Cloudserver instances, with one of them in null version +// compatibility mode. This is why they have to be explicitly enabled, +// which is done in a particular Integration test suite. This test +// suite makes the most sense in Integration because it tests the +// combination of Cloudserver requests to bucketd and the behavior of +// bucketd based on those requests. + +const describeSkipIfNotExplicitlyEnabled = + process.env.ENABLE_LEGACY_NULL_VERSION_COMPAT_TESTS ? describe : describe.skip; + +describeSkipIfNotExplicitlyEnabled('legacy null version compatibility tests', () => { + const bucketUtilCompat = new BucketUtility('default', { + endpoint: 'http://127.0.0.1:8001', + }); + const s3Compat = bucketUtilCompat.s3; + const bucketUtil = new BucketUtility('default', {}); + const s3 = bucketUtil.s3; + const bucket = `legacy-null-version-compat-${Date.now()}`; + + // In this series of tests, we first create a non-current null + // version in legacy format (with "nullVersionId" field in the + // master and no "isNull2" metadata attribute), by using the + // Cloudserver endpoint that is configured with null version + // compatibility mode enabled. + beforeEach(done => async.series([ + next => s3Compat.createBucket({ + Bucket: bucket, + }, next), + next => s3Compat.putObject({ + Bucket: bucket, + Key: 'obj', + Body: 'nullbody', + }, next), + next => s3Compat.putBucketVersioning({ + Bucket: bucket, + VersioningConfiguration: versioningEnabled, + }, next), + next => s3Compat.putObject({ + Bucket: bucket, + Key: 'obj', + Body: 'versionedbody', + }, next), + ], done)); + + afterEach(done => { + removeAllVersions({ Bucket: bucket }, err => { + if (err) { + return done(err); + } + return s3Compat.deleteBucket({ Bucket: bucket }, done); + }); + }); + + it('updating ACL of legacy null version with non-compat cloudserver', done => { + async.series([ + next => s3.putObjectAcl({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + ACL: 'public-read', + }, next), + next => s3.getObjectAcl({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, (err, acl) => { + assert.ifError(err); + // check that we fetched the updated null version + assert.strictEqual(acl.Grants.length, 2); + next(); + }), + next => s3.deleteObject({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, next), + next => s3.listObjectVersions({ + Bucket: bucket, + }, (err, listing) => { + assert.ifError(err); + // check that the null version has been correctly deleted + assert(listing.Versions.every(version => version.VersionId !== 'null')); + next(); + }), + ], done); + }); + + it('updating tags of legacy null version with non-compat cloudserver', done => { + const tagSet = [ + { + Key: 'newtag', + Value: 'newtagvalue', + }, + ]; + async.series([ + next => s3.putObjectTagging({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + Tagging: { + TagSet: tagSet, + }, + }, next), + next => s3.getObjectTagging({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, (err, tagging) => { + assert.ifError(err); + assert.deepStrictEqual(tagging.TagSet, tagSet); + next(); + }), + next => s3.deleteObjectTagging({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, err => { + assert.ifError(err); + next(); + }), + next => s3.getObjectTagging({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, (err, tagging) => { + assert.ifError(err); + assert.deepStrictEqual(tagging.TagSet, []); + next(); + }), + next => s3.deleteObject({ + Bucket: bucket, + Key: 'obj', + VersionId: 'null', + }, next), + next => s3.listObjectVersions({ + Bucket: bucket, + }, (err, listing) => { + assert.ifError(err); + // check that the null version has been correctly deleted + assert(listing.Versions.every(version => version.VersionId !== 'null')); + next(); + }), + ], done); + }); +}); diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index fca9e06c72..230c13b767 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -5,6 +5,7 @@ const { config } = require('../../../../lib/Config'); const INF_VID = versioning.VersionID.getInfVid(config.replicationGroupId); const { processVersioningState, getMasterState, + getVersionSpecificMetadataOptions, preprocessingVersioningDelete } = require('../../../../lib/api/apiUtils/object/versioning'); @@ -527,6 +528,68 @@ describe('versioning helpers', () => { })))); }); + describe('getVersionSpecificMetadataOptions', () => { + [ + { + description: 'object put before versioning was first enabled', + objMD: {}, + expectedRes: {}, + expectedResCompat: {}, + }, + { + description: 'non-null object version', + objMD: { + versionId: 'v1', + }, + expectedRes: { + versionId: 'v1', + isNull: false, + }, + expectedResCompat: { + versionId: 'v1', + }, + }, + { + description: 'legacy null object version', + objMD: { + versionId: 'vnull', + isNull: true, + }, + expectedRes: { + versionId: 'vnull', + }, + expectedResCompat: { + versionId: 'vnull', + }, + }, + { + description: 'null object version in null key', + objMD: { + versionId: 'vnull', + isNull: true, + isNull2: true, + }, + expectedRes: { + versionId: 'vnull', + isNull: true, + }, + expectedResCompat: { + versionId: 'vnull', + isNull: true, + }, + }, + ].forEach(testCase => + [false, true].forEach(nullVersionCompatMode => + it(`${testCase.description}${nullVersionCompatMode ? ' (null compat)' : ''}`, + () => { + const options = getVersionSpecificMetadataOptions( + testCase.objMD, nullVersionCompatMode); + const expectedResAttr = nullVersionCompatMode ? + 'expectedResCompat' : 'expectedRes'; + assert.deepStrictEqual(options, testCase[expectedResAttr]); + }))); + }); + describe('preprocessingVersioningDelete', () => { [ {