Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ const constants = {
// for external backends, don't call unless at least 1 minute
// (60,000 milliseconds) since last call
externalBackendHealthCheckInterval: 60000,
versioningNotImplBackends: { azure: true, gcp: true },
versioningNotImplBackends: { azure: true },
mpuMDStoredExternallyBackend: { aws_s3: true, gcp: true },
skipBatchDeleteBackends: { azure: true, gcp: true },
s3HandledBackends: { azure: true, gcp: true },
Expand Down
7 changes: 5 additions & 2 deletions lib/api/apiUtils/object/createAndStoreObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const {
const { externalBackends, versioningNotImplBackends } = constants;

const externalVersioningErrorMessage =
'We do not currently support putting a versioned object to a location-constraint of type Azure or GCP.';
'We do not currently support putting a versioned object to a location-constraint of type Azure.';

/**
* Validate and compute the checksum for a zero-size object body.
Expand Down Expand Up @@ -322,7 +322,10 @@ function createAndStoreObject(
}
}

const headerChecksum = areChecksumsEnabled() ? getChecksumDataFromHeaders(request.headers) : null;
// A delete marker has no body: an x-amz-checksum-* header
// describes the request payload (e.g. a multi-object delete XML).
const headerChecksum =
isDeleteMarker || !areChecksumsEnabled() ? null : getChecksumDataFromHeaders(request.headers);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not an issue for other data locations as well? Ie a separate bugfix for integrity check feature?

if (headerChecksum && headerChecksum.error) {
return next(arsenalErrorFromChecksumError(headerChecksum));
}
Expand Down
195 changes: 95 additions & 100 deletions lib/api/bucketPutVersioning.js

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the GPC removal should not go in 9.4, but 9.5
  • I wonder if we should build the strings, instead of having the source of truth in 2 constants (one in arsenal + one in cloudserver) AND having manually duplicated that info in multiple error strings...

Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
const metadata = require('../metadata/wrapper');
const { standardMetadataValidateBucket } = require('../metadata/metadataUtils');
const { pushMetric } = require('../utapi/utilities');
const versioningNotImplBackends =
require('../../constants').versioningNotImplBackends;
const versioningNotImplBackends = require('../../constants').versioningNotImplBackends;
const { config } = require('../Config');
const monitoring = require('../utilities/monitoringHandler');

const externalVersioningErrorMessage = 'We do not currently support putting ' +
'a versioned object to a location-constraint of type Azure or GCP.';
const externalVersioningErrorMessage =
'We do not currently support putting ' + 'a versioned object to a location-constraint of type Azure.';

const replicationVersioningErrorMessage = 'A replication configuration is ' +
'present on this bucket, so you cannot change the versioning state. To ' +
'change the versioning state, first delete the replication configuration.';
const replicationVersioningErrorMessage =
'A replication configuration is ' +
'present on this bucket, so you cannot change the versioning state. To ' +
'change the versioning state, first delete the replication configuration.';

const ingestionVersioningErrorMessage = 'Versioning cannot be suspended for '
+ 'buckets setup with Out of Band updates from a location';
const ingestionVersioningErrorMessage =
'Versioning cannot be suspended for ' + 'buckets setup with Out of Band updates from a location';

const objectLockErrorMessage = 'An Object Lock configuration is present on ' +
'this bucket, so the versioning state cannot be changed.';
const objectLockErrorMessage =
'An Object Lock configuration is present on ' + 'this bucket, so the versioning state cannot be changed.';

/**
* Format of xml request:
Expand All @@ -47,21 +47,17 @@ function _parseXML(request, log, cb) {
return cb(errors.MalformedXML);
}
const versioningConf = result.VersioningConfiguration;
const status = versioningConf.Status ?
versioningConf.Status[0] : undefined;
const mfaDelete = versioningConf.MfaDelete ?
versioningConf.MfaDelete[0] : undefined;
const status = versioningConf.Status ? versioningConf.Status[0] : undefined;
const mfaDelete = versioningConf.MfaDelete ? versioningConf.MfaDelete[0] : undefined;
const validStatuses = ['Enabled', 'Suspended'];
const validMfaDeletes = [undefined, 'Enabled', 'Disabled'];
if (validStatuses.indexOf(status) < 0 ||
validMfaDeletes.indexOf(mfaDelete) < 0) {
if (validStatuses.indexOf(status) < 0 || validMfaDeletes.indexOf(mfaDelete) < 0) {
log.debug('illegal versioning configuration');
return cb(errors.IllegalVersioningConfigurationException);
}
if (versioningConf && mfaDelete === 'Enabled') {
log.debug('mfa deletion is not implemented');
return cb(errorInstances.NotImplemented
.customizeDescription('MFA Deletion is not supported yet.'));
return cb(errorInstances.NotImplemented.customizeDescription('MFA Deletion is not supported yet.'));
}
return process.nextTick(() => cb(null));
});
Expand Down Expand Up @@ -103,90 +99,89 @@ function bucketPutVersioning(authInfo, request, log, callback) {
requestType: request.apiMethods || 'bucketPutVersioning',
request,
};
return waterfall([
next => _parseXML(request, log, next),
next => standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log,
(err, bucket) => next(err, bucket)), // ignore extra null object,
(bucket, next) => parseString(request.post, (err, result) => {
// just for linting; there should not be any parsing error here
if (err) {
return next(err, bucket);
}
// prevent enabling versioning on an nfs exported bucket
if (bucket.isNFS()) {
const error = new Error();
error.code = 'NFSBUCKET';
return next(error);
}
// _checkBackendVersioningImplemented returns false if versioning
// is not implemented on the bucket backend
if (!_checkBackendVersioningImplemented(bucket)) {
log.debug(externalVersioningErrorMessage,
{ method: 'bucketPutVersioning',
error: errors.NotImplemented });
const error = errorInstances.NotImplemented.customizeDescription(
externalVersioningErrorMessage);
return next(error, bucket);
}
const versioningConfiguration = {};
if (result.VersioningConfiguration.Status) {
versioningConfiguration.Status =
result.VersioningConfiguration.Status[0];
}
if (result.VersioningConfiguration.MfaDelete) {
versioningConfiguration.MfaDelete =
result.VersioningConfiguration.MfaDelete[0];
}
// the configuration has been checked before
return next(null, bucket, versioningConfiguration);
}),
(bucket, versioningConfiguration, next) => {
// check if replication is enabled if versioning is being suspended
const replicationConfig = bucket.getReplicationConfiguration();
const isIngestionBucket = bucket.isIngestionBucket && bucket.isIngestionBucket();
const invalidAction =
versioningConfiguration.Status === 'Suspended'
&& (isIngestionBucket || replicationConfig?.rules?.some(r => r.enabled));
if (invalidAction) {
const errorMsg = isIngestionBucket ?
ingestionVersioningErrorMessage : replicationVersioningErrorMessage;
next(errorInstances.InvalidBucketState
.customizeDescription(errorMsg));
return;
return waterfall(
[
next => _parseXML(request, log, next),
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
next =>
standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) =>
next(err, bucket),
), // ignore extra null object,
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
(bucket, next) =>
parseString(request.post, (err, result) => {
// just for linting; there should not be any parsing error here
if (err) {
return next(err, bucket);
}
// prevent enabling versioning on an nfs exported bucket
if (bucket.isNFS()) {
const error = new Error();
error.code = 'NFSBUCKET';
return next(error);
}
// _checkBackendVersioningImplemented returns false if versioning
// is not implemented on the bucket backend
if (!_checkBackendVersioningImplemented(bucket)) {
log.debug(externalVersioningErrorMessage, {
method: 'bucketPutVersioning',
error: errors.NotImplemented,
});
const error =
errorInstances.NotImplemented.customizeDescription(externalVersioningErrorMessage);
return next(error, bucket);
}
const versioningConfiguration = {};
if (result.VersioningConfiguration.Status) {
versioningConfiguration.Status = result.VersioningConfiguration.Status[0];
}
if (result.VersioningConfiguration.MfaDelete) {
versioningConfiguration.MfaDelete = result.VersioningConfiguration.MfaDelete[0];
}
// the configuration has been checked before
return next(null, bucket, versioningConfiguration);
}),
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
(bucket, versioningConfiguration, next) => {
// check if replication is enabled if versioning is being suspended
const replicationConfig = bucket.getReplicationConfiguration();
const isIngestionBucket = bucket.isIngestionBucket && bucket.isIngestionBucket();
const invalidAction =
versioningConfiguration.Status === 'Suspended' &&
(isIngestionBucket || replicationConfig?.rules?.some(r => r.enabled));
if (invalidAction) {
const errorMsg = isIngestionBucket
? ingestionVersioningErrorMessage
: replicationVersioningErrorMessage;
next(errorInstances.InvalidBucketState.customizeDescription(errorMsg));
return;
}
const objectLockEnabled = bucket.isObjectLockEnabled();
if (objectLockEnabled) {
next(errorInstances.InvalidBucketState.customizeDescription(objectLockErrorMessage));
return;
}
bucket.setVersioningConfiguration(versioningConfiguration);
// TODO all metadata updates of bucket should be using CAS
metadata.updateBucket(bucket.getName(), bucket, log, err => next(err, bucket));
},
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
],
(err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin, request.method, bucket);
if (err && err.code === 'NFSBUCKET') {
log.trace('skipping versioning for nfs exported bucket');
return callback(null, corsHeaders);
}
const objectLockEnabled = bucket.isObjectLockEnabled();
if (objectLockEnabled) {
next(errorInstances.InvalidBucketState
.customizeDescription(objectLockErrorMessage));
return;
if (err) {
log.trace('error processing request', { error: err, method: 'bucketPutVersioning' });
monitoring.promMetrics('PUT', bucketName, err.code, 'putBucketVersioning');
} else {
pushMetric('putBucketVersioning', log, {
authInfo,
bucket: bucketName,
});
monitoring.promMetrics('PUT', bucketName, '200', 'putBucketVersioning');
}
bucket.setVersioningConfiguration(versioningConfiguration);
// TODO all metadata updates of bucket should be using CAS
metadata.updateBucket(bucket.getName(), bucket, log, err =>
next(err, bucket));
return callback(err, corsHeaders);
},
], (err, bucket) => {
const corsHeaders = collectCorsHeaders(request.headers.origin,
request.method, bucket);
if (err && err.code === 'NFSBUCKET') {
log.trace('skipping versioning for nfs exported bucket');
return callback(null, corsHeaders);
}
if (err) {
log.trace('error processing request', { error: err,
method: 'bucketPutVersioning' });
monitoring.promMetrics(
'PUT', bucketName, err.code, 'putBucketVersioning');
} else {
pushMetric('putBucketVersioning', log, {
authInfo,
bucket: bucketName,
});
monitoring.promMetrics(
'PUT', bucketName, '200', 'putBucketVersioning');
}
return callback(err, corsHeaders);
});
);
}

module.exports = bucketPutVersioning;
2 changes: 1 addition & 1 deletion lib/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ function getObjMetadataAndDelete(
overheadField,
log,
's3:ObjectRemoved:DeleteMarkerCreated',
(err, result) => callback(err, objMD, deleteInfo, result.versionId),
(err, result) => callback(err, objMD, deleteInfo, result?.versionId),
);
},
],
Expand Down
2 changes: 1 addition & 1 deletion lib/api/objectCopy.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const versionIdUtils = versioning.VersionID;
const locationHeader = constants.objectLocationConstraintHeader;
const versioningNotImplBackends = constants.versioningNotImplBackends;
const externalVersioningErrorMessage =
'We do not currently support putting a versioned object to a location-constraint of type AWS or Azure or GCP.';
'We do not currently support putting a versioned object to a location-constraint of type Azure.';

/**
* Compute the prior data locations that are orphaned.
Expand Down
2 changes: 1 addition & 1 deletion package.json
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.15",
"arsenal": "git+https://github.com/scality/arsenal#2136c02ff1d3dfd43a5ff7ca59f955d105946dba",
Comment thread
maeldonn marked this conversation as resolved.
"async": "2.6.4",
"aws-crt": "^1.24.0",
"bucketclient": "scality/bucketclient#8.2.7",
Expand Down
Loading
Loading