Skip to content

Commit fdfb965

Browse files
williamlardierwilliamlardier
committed
AbortMPU to properly cleanup existing s3 object
- CompleteMPU was and is still, at the time of this commit, wrongly promoting s3 objects when completing a MPU even if the parts are not valid. - The AbortMPU cleanup logic was added as an effort to cleanup new occurences of this problem, and still handle old ones not yet aborted. - The logic currently is unable to clean up if the current master version has a different upload id: in case a new version was pushed, we end up never cleaning the ghost. - This commit introduces a dynamic listing of the versionIDs of the object. We use the delimiter versions algorithm and the versionID separator to list only the current object versions, with pagination support, and then we detect the one to cleanup. Issue: CLDSRV-669
1 parent c47dff0 commit fdfb965

File tree

1 file changed

+118
-30
lines changed

1 file changed

+118
-30
lines changed

‎lib/api/apiUtils/object/abortMultipartUpload.js‎

Lines changed: 118 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { standardMetadataValidateBucketAndObj } =
88
const { validateQuotas } = require('../quotas/quotaUtils');
99
const services = require('../../../services');
1010
const metadata = require('../../../metadata/wrapper');
11+
const { versioning } = require('arsenal');
1112

1213
function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
1314
callback, request) {
@@ -61,61 +62,146 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
6162
});
6263
},
6364
function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket, objectMD,
64-
next) {
65+
next) {
6566
const location = mpuOverviewObj.controllingLocationConstraint;
6667
const originalIdentityAuthzResults = request.actionImplicitDenies;
6768
// eslint-disable-next-line no-param-reassign
6869
delete request.actionImplicitDenies;
6970
return data.abortMPU(objectKey, uploadId, location, bucketName,
70-
request, destBucket, locationConstraintCheck, log,
71-
(err, skipDataDelete) => {
72-
// eslint-disable-next-line no-param-reassign
73-
request.actionImplicitDenies = originalIdentityAuthzResults;
74-
if (err) {
75-
log.error('error aborting MPU', { error: err });
76-
return next(err, destBucket);
77-
}
78-
// for Azure and GCP we do not need to delete data
79-
// for all other backends, skipDataDelete will be set to false
80-
return next(null, mpuBucket, destBucket, objectMD, skipDataDelete);
81-
});
71+
request, destBucket, locationConstraintCheck, log,
72+
(err, skipDataDelete) => {
73+
// eslint-disable-next-line no-param-reassign
74+
request.actionImplicitDenies = originalIdentityAuthzResults;
75+
if (err) {
76+
log.error('error aborting MPU', { error: err });
77+
return next(err, destBucket);
78+
}
79+
// for Azure and GCP we do not need to delete data
80+
// for all other backends, skipDataDelete will be set to false
81+
return next(null, mpuBucket, mpuOverviewObj, destBucket, objectMD, skipDataDelete);
82+
});
8283
},
83-
function getPartLocations(mpuBucket, destBucket, objectMD, skipDataDelete,
84-
next) {
84+
function getPartLocations(mpuBucket, mpuOverviewObj, destBucket, objectMD, skipDataDelete,
85+
next) {
8586
services.getMPUparts(mpuBucket.getName(), uploadId, log,
8687
(err, result) => {
8788
if (err) {
8889
log.error('error getting parts', { error: err });
8990
return next(err, destBucket);
9091
}
9192
const storedParts = result.Contents;
92-
return next(null, mpuBucket, storedParts, destBucket, objectMD,
93-
skipDataDelete);
93+
return next(null, mpuBucket, mpuOverviewObj, storedParts, destBucket, objectMD,
94+
skipDataDelete);
9495
});
9596
},
96-
function deleteObjectMetadata(mpuBucket, storedParts, destBucket, objectMD, skipDataDelete, next) {
97-
if (!objectMD || metadataValMPUparams.uploadId !== objectMD.uploadId) {
97+
// During Abort, we dynamically detect if the previous CompleteMPU call
98+
// created potential object metadata wrongly, e.g. by creating
99+
// an object version when some of the parts are missing.
100+
// By passing a null objectMD, we tell the subsequent steps
101+
// to skip the cleanup.
102+
// Another approach is possible, but not supported by all backends:
103+
// to honor the uploadId filter in standardMetadataValidateBucketAndObj
104+
// ensuring the objMD returned has the right uploadId. But this is not
105+
// supported by Metadata.
106+
function ensureCleanupIsRequired(mpuBucket, mpuOverviewObj, storedParts, destBucket,
107+
objectMD, skipDataDelete, next) {
108+
// If no overview object, skip - this wasn't a failed completeMPU
109+
if (!mpuOverviewObj) {
110+
return next(null, mpuBucket, storedParts, destBucket, null, skipDataDelete);
111+
}
112+
113+
// If objectMD exists and has matching uploadId, use it directly
114+
// This handles all non-versioned cases, and some versioned cases.
115+
if (objectMD && objectMD.uploadId === uploadId) {
98116
return next(null, mpuBucket, storedParts, destBucket, objectMD, skipDataDelete);
99117
}
100-
// In case there has been an error during cleanup after a complete MPU
101-
// (e.g. failure to delete MPU MD in shadow bucket),
102-
// we need to ensure that the MPU metadata is deleted.
103-
log.debug('Object has existing metadata, deleting them', {
118+
119+
// If bucket is not versioned, no need to check versions:
120+
// as the uploadId is not the same, we skip the cleanup.
121+
if (!destBucket.isVersioningEnabled()) {
122+
return next(null, mpuBucket, storedParts, destBucket, null, skipDataDelete);
123+
}
124+
125+
// Otherwise, we list all versions of the object. We try to stop as early
126+
// as possible, by using pagination.
127+
let keyMarker = null;
128+
let versionIdMarker = null;
129+
let foundVersion = null;
130+
let shouldContinue = true;
131+
132+
return async.whilst(
133+
() => shouldContinue && !foundVersion,
134+
callback => {
135+
const listParams = {
136+
listingType: 'DelimiterVersions',
137+
// To only list the specific key, we need to add the versionId separator
138+
prefix: `${objectKey}${versioning.VersioningConstants.VersionId.Separator}`,
139+
maxKeys: 1000,
140+
};
141+
142+
if (keyMarker) {
143+
listParams.keyMarker = keyMarker;
144+
}
145+
if (versionIdMarker) {
146+
listParams.versionIdMarker = versionIdMarker;
147+
}
148+
149+
return services.getObjectListing(bucketName, listParams, log, (err, listResponse) => {
150+
if (err) {
151+
log.error('error listing object versions', { error: err });
152+
return callback(err);
153+
}
154+
155+
// Check each version in current batch for matching uploadId
156+
const matchedVersion = (listResponse.Versions || []).find(version =>
157+
version.key === objectKey &&
158+
version.value &&
159+
version.value.uploadId === uploadId
160+
);
161+
162+
if (matchedVersion) {
163+
foundVersion = matchedVersion.value;
164+
}
165+
166+
// Set up for next iteration if needed
167+
if (listResponse.IsTruncated && !foundVersion) {
168+
keyMarker = listResponse.NextKeyMarker;
169+
versionIdMarker = listResponse.NextVersionIdMarker;
170+
} else {
171+
shouldContinue = false;
172+
}
173+
174+
return callback();
175+
});
176+
},
177+
err => next(null, mpuBucket, storedParts, destBucket, err ? null : foundVersion, skipDataDelete)
178+
);
179+
},
180+
function deleteObjectMetadata(mpuBucket, storedParts, destBucket, objMDWithMatchingUploadID,
181+
skipDataDelete, next) {
182+
// If no objMDWithMatchingUploadID, nothing to delete
183+
if (!objMDWithMatchingUploadID) {
184+
return next(null, mpuBucket, storedParts, destBucket, objMDWithMatchingUploadID, skipDataDelete);
185+
}
186+
187+
log.debug('Object has existing metadata with matching uploadId, deleting them', {
104188
method: 'abortMultipartUpload',
105189
bucketName,
106190
objectKey,
107191
uploadId,
108-
versionId: objectMD.versionId,
192+
versionId: objMDWithMatchingUploadID.versionId,
109193
});
110-
return metadata.deleteObjectMD(bucketName, objectKey, { versionId: objectMD.versionId }, log, err => {
194+
return metadata.deleteObjectMD(bucketName, objectKey, {
195+
versionId: objMDWithMatchingUploadID.versionId,
196+
}, log, err => {
111197
if (err) {
112198
log.error('error deleting object metadata', { error: err });
113199
}
114-
return next(err, mpuBucket, storedParts, destBucket, objectMD, skipDataDelete);
200+
return next(err, mpuBucket, storedParts, destBucket, objMDWithMatchingUploadID, skipDataDelete);
115201
});
116202
},
117-
function deleteData(mpuBucket, storedParts, destBucket, objectMD,
118-
skipDataDelete, next) {
203+
function deleteData(mpuBucket, storedParts, destBucket, objMDWithMatchingUploadID,
204+
skipDataDelete, next) {
119205
if (skipDataDelete) {
120206
return next(null, mpuBucket, storedParts, destBucket);
121207
}
@@ -126,9 +212,11 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
126212
return next(null, mpuBucket, storedParts, destBucket);
127213
}
128214

129-
if (objectMD && objectMD.location && objectMD.uploadId === metadataValMPUparams.uploadId) {
215+
// Add object data locations if they exist and we can trust uploadId matches
216+
if (objMDWithMatchingUploadID && objMDWithMatchingUploadID.location) {
130217
const existingLocations = new Set(locations.map(loc => loc.key));
131-
const remainingObjectLocations = objectMD.location.filter(loc => !existingLocations.has(loc.key));
218+
const remainingObjectLocations = objMDWithMatchingUploadID.
219+
location.filter(loc => !existingLocations.has(loc.key));
132220
locations.push(...remainingObjectLocations);
133221
}
134222

0 commit comments

Comments
 (0)