Skip to content

Make S3Overrides accessible via E-Var or option AND tests #40

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

Closed
wants to merge 24 commits into from
Closed

Make S3Overrides accessible via E-Var or option AND tests #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
14f3f56
Make S3Overrides accessible via E-Var or option.
mrmarcsmith Dec 30, 2016
4abb443
Merge pull request #1 from mrmarcsmith/mrmarcsmith-s3override-access
mrmarcsmith Dec 30, 2016
fd7ba2b
Update index.js
mrmarcsmith Dec 30, 2016
87e3b2a
Update index.js
mrmarcsmith Dec 30, 2016
7986b61
Update index.js
mrmarcsmith Dec 31, 2016
03fd287
Update index.js
mrmarcsmith Dec 31, 2016
4ab310e
Update index.js
mrmarcsmith Dec 31, 2016
e18efe8
Update index.js
mrmarcsmith Dec 31, 2016
eae68ec
Update index.js
mrmarcsmith Dec 31, 2016
311a44d
Update index.js
mrmarcsmith Dec 31, 2016
bcc7ef4
Update index.js
mrmarcsmith Dec 31, 2016
0ee6e5e
Update index.js
mrmarcsmith Jan 2, 2017
5690044
Update index.js
mrmarcsmith Jan 6, 2017
2b9fe8f
Removed debugging Logs
mrmarcsmith Jan 26, 2017
078a342
added tests for SOverrides as Env or an option
mrmarcsmith Jan 26, 2017
344cfb3
moved function so auto merge can happen
mrmarcsmith Jan 26, 2017
d4a3881
removed trailing spaces
mrmarcsmith Jan 26, 2017
a4bce1e
Debuggin CI Error
mrmarcsmith Jan 26, 2017
a19d31d
Fix
mrmarcsmith Jan 26, 2017
803523a
removed trailing space
mrmarcsmith Jan 26, 2017
747a17e
revert CI debug
mrmarcsmith Jan 26, 2017
ddc96f6
options now yield to overrides
mrmarcsmith Jan 26, 2017
8725059
Added tests to ensure overrides take priority
mrmarcsmith Jan 26, 2017
fcf3d39
removed trailing spaces
mrmarcsmith Jan 26, 2017
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
13 changes: 10 additions & 3 deletions lib/optionsFromArguments.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ function requiredOrFromEnvironment(options, key, env) {

function fromEnvironmentOrDefault(options, key, env, defaultValue) {
options[key] = options[key] || process.env[env] || defaultValue;
// If we used the overrides,
// make sure they are consistent
if(options.s3overrides){
if(options.s3overrides[key]){
options[key] = options.s3overrides[key];
}
}

return options;
}

Expand Down Expand Up @@ -60,6 +68,7 @@ const optionsFromArguments = function optionsFromArguments(args) {
}
}

options = fromEnvironmentOrDefault(options, 's3overrides', 'S3_OVERRIDES', s3overrides);
options = requiredOrFromEnvironment(options, 'bucket', 'S3_BUCKET');
options = fromEnvironmentOrDefault(options, 'accessKey', 'S3_ACCESS_KEY', null);
options = fromEnvironmentOrDefault(options, 'secretKey', 'S3_SECRET_KEY', null);
Expand All @@ -69,9 +78,7 @@ const optionsFromArguments = function optionsFromArguments(args) {
options = fromEnvironmentOrDefault(options, 'baseUrl', 'S3_BASE_URL', null);
options = fromEnvironmentOrDefault(options, 'baseUrlDirect', 'S3_BASE_URL_DIRECT', false);
options = fromEnvironmentOrDefault(options, 'signatureVersion', 'S3_SIGNATURE_VERSION', 'v4');
options = fromEnvironmentOrDefault(
options, 'globalCacheControl', 'S3_GLOBAL_CACHE_CONTROL', null);
options.s3overrides = s3overrides;
options = fromEnvironmentOrDefault(options, 'globalCacheControl', 'S3_GLOBAL_CACHE_CONTROL', null);

return options;
}
Expand Down
19 changes: 19 additions & 0 deletions spec/test.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,25 @@ describe('S3Adapter tests', () => {
expect(options.bucketPrefix).toEqual('test/');
});

it('should accept options and overrides as an option in args', () => {
var confObj = { bucketPrefix: 'test/', bucket: 'bucket-1', secretKey: 'secret-1', accessKey: 'key-1' ,s3overrides: { secretAccessKey: 'secret-2', accessKeyId: 'key-2', params: { Bucket: 'bucket-2' }} };
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add a space after the comma

it should be
, s3over....
not
,s3over...

var s3 = new S3Adapter(confObj);
expect(s3._s3Client.config.accessKeyId).toEqual('key-2');
expect(s3._s3Client.config.secretAccessKey).toEqual('secret-2');
expect(s3._s3Client.config.params.Bucket).toEqual('bucket-2');
expect(s3._bucketPrefix).toEqual('test/');
});

it('should accept options and overrides as an Enviromental Variable', () => {
var confObj = { bucketPrefix: 'test/', bucket: 'bucket-1', secretKey: 'secret-1', accessKey: 'key-1'};
Copy link
Contributor

@acinader acinader Jan 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

why'd you take out that trailing space? for consistency, there should be a space between the 1' };

that's how all the other object spacing is done in the file. we just want to be consistent, when possible we put those consistency rules into lint so we don't have to think about it......

process.env.S3_OVERRIDES = { secretAccessKey: 'secret-2', accessKeyId: 'key-2', params: { Bucket: 'bucket-2' }};
var s3 = new S3Adapter(confObj);
expect(s3._s3Client.config.accessKeyId).toEqual('key-2');
expect(s3._s3Client.config.secretAccessKey).toEqual('secret-2');
expect(s3._s3Client.config.params.Bucket).toEqual('bucket-2');
expect(s3._bucketPrefix).toEqual('test/');
});

it('should accept options and overrides as args', () => {
var confObj = { bucketPrefix: 'test/', bucket: 'bucket-1', secretKey: 'secret-1', accessKey: 'key-1' };
var overridesObj = { secretAccessKey: 'secret-2', accessKeyId: 'key-2', params: { Bucket: 'bucket-2' }};
Expand Down