Skip to content

Allow enabling versioning on buckets with a GCP location constraint #6266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
maeldonn wants to merge 2 commits into
base: development/9.5
Choose a base branch
from
Draft

Allow enabling versioning on buckets with a GCP location constraint #6266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f8cff2f
Reformat files with prettier
maeldonn Aug 31, 2026
58e07b2
Allow enabling versioning on buckets with a GCP location constraint
maeldonn Aug 24, 2026
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
4 changes: 3 additions & 1 deletion constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ 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 },
externalVersioningErrorMessage:
'We do not currently support putting a versioned object to a location-constraint of type Azure.',
mpuMDStoredExternallyBackend: { aws_s3: true, gcp: true },
skipBatchDeleteBackends: { azure: true, gcp: true },
s3HandledBackends: { azure: true, gcp: true },
Expand Down
5 changes: 1 addition & 4 deletions lib/api/apiUtils/object/createAndStoreObject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ const {
areChecksumsEnabled,
} = require('../integrity/validateChecksums');

const { externalBackends, versioningNotImplBackends } = constants;

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

/**
* Validate and compute the checksum for a zero-size object body.
Expand Down
194 changes: 93 additions & 101 deletions lib/api/bucketPutVersioning.js
View file
Open in desktop
Comment thread
maeldonn marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,20 @@ 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, externalVersioningErrorMessage } = require('../../constants');
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 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 +44,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 +96,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];
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);

@claude claude Bot Sep 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

_checkBackendVersioningImplemented can now reject a GCP location (when supportsVersioning is false/undefined) at the second check. Before this PR, GCP was caught by the versioningNotImplBackends lookup and never reached the supportsVersioning path. Now it can, and this error message says "Azure" — confusing for a user operating on a GCP bucket.

Consider making the message generic when the rejection comes from the supportsVersioning check (e.g. return the backend type from _checkBackendVersioningImplemented and build the message accordingly), or use a backend-agnostic message like "Versioning is not supported for this location constraint."

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);
}
// 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;
}
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;
3 changes: 1 addition & 2 deletions lib/api/objectCopy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ const kms = require('../kms/wrapper');
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.';
const externalVersioningErrorMessage = constants.externalVersioningErrorMessage;

/**
* Compute the prior data locations that are orphaned.
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
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