From 2fbd5ce8ccf2d37c60deac7c34257c8886c54e8c Mon Sep 17 00:00:00 2001 From: Rahul Padigela Date: Thu, 5 Apr 2018 11:48:19 -0700 Subject: [PATCH] ft: provide pathStyle option to support S3C This feature allows setting pathStyle requests option for location constraints so that it can be relaxed for non-AWS S3 backends (for example S3 Connector) --- lib/Config.js | 7 +++ lib/data/locationConstraintParser.js | 2 + tests/locationConfig/locationConfigTests.json | 11 ++++ tests/unit/testConfigs/locConstraintAssert.js | 61 ++++++++++++++++++- 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/Config.js b/lib/Config.js index 36d73611e6..ad719d154a 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -195,6 +195,13 @@ function locationConstraintAssert(locationConstraints) { // eslint-disable-next-line no-param-reassign locationConstraints[l].details.https = true; } + if (details.pathStyle !== undefined) { + assert(typeof details.pathStyle === 'boolean', 'bad config: ' + + 'locationConstraints[region].pathStyle must be a boolean'); + } else { + // eslint-disable-next-line no-param-reassign + locationConstraints[l].details.pathStyle = false; + } if (locationConstraints[l].type === 'azure') { azureLocationConstraintAssert(l, locationConstraints[l]); } diff --git a/lib/data/locationConstraintParser.js b/lib/data/locationConstraintParser.js index a4b021130a..c29e1d427f 100644 --- a/lib/data/locationConstraintParser.js +++ b/lib/data/locationConstraintParser.js @@ -54,6 +54,7 @@ function parseLC() { timeout: 0 } : { agent: connectionAgent, timeout: 0 }; const sslEnabled = locationObj.details.https === true; + const pathStyle = locationObj.details.pathStyle; // TODO: HTTP requests to AWS are not supported with V4 auth for // non-file streams which are used by Backbeat. This option will be // removed once CA certs, proxy setup feature is implemented. @@ -73,6 +74,7 @@ function parseLC() { signatureVersion, sslEnabled, maxRetries: 0, + s3ForcePathStyle: pathStyle, }; // users can either include the desired profile name from their // ~/.aws/credentials file or include the accessKeyId and diff --git a/tests/locationConfig/locationConfigTests.json b/tests/locationConfig/locationConfigTests.json index 3797901c1e..5a39aedb98 100644 --- a/tests/locationConfig/locationConfigTests.json +++ b/tests/locationConfig/locationConfigTests.json @@ -65,6 +65,17 @@ "credentialsProfile": "default_2" } }, + "awsbackendPathStyle": { + "type": "aws_s3", + "legacyAwsBehavior": true, + "details": { + "awsEndpoint": "s3.amazonaws.com", + "bucketName": "multitester555", + "bucketMatch": true, + "credentialsProfile": "default", + "pathStyle": true + } + }, "azurebackend": { "type": "azure", "legacyAwsBehavior": true, diff --git a/tests/unit/testConfigs/locConstraintAssert.js b/tests/unit/testConfigs/locConstraintAssert.js index a190ac5cb5..b3905e3054 100644 --- a/tests/unit/testConfigs/locConstraintAssert.js +++ b/tests/unit/testConfigs/locConstraintAssert.js @@ -5,11 +5,11 @@ class LocationConstraint { constructor(type, legacyAwsBehavior, details) { this.type = type || 'scality'; this.legacyAwsBehavior = legacyAwsBehavior || false; - this.details = details || { + this.details = Object.assign({}, { awsEndpoint: 's3.amazonaws.com', bucketName: 'tester', credentialsProfile: 'default', - }; + }, details || {}); } } @@ -224,4 +224,61 @@ describe('locationConstraintAssert', () => { '/bad location constraint: "azurefaketest" ' + 'azureStorageAccessKey is not a valid base64 string/'); }); + + it('should set https to true by default', () => { + const usEast1 = new LocationConstraint(); + const locationConstraint = new LocationConstraint('aws_s3', true); + assert.doesNotThrow(() => { + locationConstraintAssert({ + 'us-east-1': usEast1, + 'awshttpsDefault': locationConstraint, + }); + }, '/bad location constraint awshttpsDefault,' + + 'incorrect default config for https'); + assert.strictEqual(locationConstraint.details.https, true, + 'https config should be true'); + }); + + it('should override default if https is set to false', () => { + const usEast1 = new LocationConstraint(); + const locationConstraint = new LocationConstraint('aws_s3', true, { + https: false, + }); + assert.doesNotThrow(() => { + locationConstraintAssert({ + 'us-east-1': usEast1, + 'awshttpsFalse': locationConstraint, + }); + }, '/bad location constraint awshttpsFalse,' + + 'incorrect config for https'); + assert.strictEqual(locationConstraint.details.https, false, + 'https config should be false'); + }); + + it('should set pathStyle config option to false by default', () => { + const usEast1 = new LocationConstraint(); + const locationConstraint = new LocationConstraint('aws_s3', true); + assert.doesNotThrow(() => { + locationConstraintAssert({ + 'us-east-1': usEast1, + 'awsdefaultstyle': locationConstraint, + }); + }, '/bad location constraint, unable to set default config'); + assert.strictEqual(locationConstraint.details.pathStyle, false, + 'pathstyle config should be false'); + }); + + it('should override default if pathStyle is set to true', () => { + const usEast1 = new LocationConstraint(); + const locationConstraint = new LocationConstraint('aws_s3', true, + { pathStyle: true }); + assert.doesNotThrow(() => { + locationConstraintAssert({ + 'us-east-1': usEast1, + 'awspathstyle': locationConstraint, + }); + }, '/bad location constraint, unable to set pathSytle config'); + assert.strictEqual(locationConstraint.details.pathStyle, true, + 'pathstyle config should be true'); + }); });