Skip to content

Commit

Permalink
Merge pull request #214 from opentable/s3-validation
Browse files Browse the repository at this point in the history
S3 validation
  • Loading branch information
jankowiakmaria committed Mar 23, 2016
2 parents 32dc75d + feaf944 commit 5427ad1
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 16 deletions.
6 changes: 6 additions & 0 deletions registry/domain/validators/registry-configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,11 @@ module.exports = function(conf){
});
}

if(!conf.local){
if(!conf.s3 || !conf.s3.bucket || !conf.s3.key || !conf.s3.region || !conf.s3.secret){
return returnError(strings.errors.registry.CONFIGURATION_S3_NOT_VALID);
}
}

return response;
};
1 change: 1 addition & 0 deletions resources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ module.exports = {
CONFIGURATION_ROUTES_HANDLER_MUST_BE_FUNCTION: 'Registry configuration is not valid: handler should be a function',
CONFIGURATION_ROUTES_NOT_VALID: 'Registry configuration is not valid: each route should contain route, method and handler',
CONFIGURATION_ROUTES_MUST_BE_ARRAY: 'Registry configuration is not valid: routes must be an array',
CONFIGURATION_S3_NOT_VALID: 'Registry configuration is not valid: S3 configuration is not valid',
DATA_OBJECT_IS_UNDEFINED: 'data object is undefined',
DEPENDENCY_NOT_FOUND: 'Component is trying to use unavailable dependencies: {0}',
DEPENDENCY_NOT_FOUND_CODE: 'DEPENDENCY_MISSING_FROM_REGISTRY',
Expand Down
115 changes: 99 additions & 16 deletions test/unit/registry-domain-validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ var injectr = require('injectr');

describe('registry : domain : validator', function(){

var validator = require('../../registry/domain/validators');
var validator = require('../../registry/domain/validators'),
baseS3Conf = {
bucket: 'oc-components',
key: 's3-key',
region: 'us-west2',
secret: 's3-secret'
};

describe('when validating registry configuration', function(){

Expand Down Expand Up @@ -34,7 +40,7 @@ describe('registry : domain : validator', function(){
describe('prefix', function(){
describe('when prefix does not start with /', function(){

var conf = { prefix: 'hello/' };
var conf = { prefix: 'hello/', s3: baseS3Conf };

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -44,7 +50,7 @@ describe('registry : domain : validator', function(){

describe('when prefix does not end with /', function(){

var conf = { prefix: '/hello' };
var conf = { prefix: '/hello', s3: baseS3Conf };

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -56,7 +62,7 @@ describe('registry : domain : validator', function(){
describe('publishAuth', function(){
describe('when not specified', function(){

var conf = { publishAuth: null };
var conf = { publishAuth: null, s3: baseS3Conf };

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
Expand All @@ -65,7 +71,7 @@ describe('registry : domain : validator', function(){

describe('when specified and not supported', function(){

var conf = { publishAuth: { type: 'blarg' }};
var conf = { publishAuth: { type: 'blarg' }, s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -77,7 +83,7 @@ describe('registry : domain : validator', function(){

describe('when username and password specified', function(){

var conf = { publishAuth: { type: 'basic', username: 'a', password: 'b' }};
var conf = { publishAuth: { type: 'basic', username: 'a', password: 'b' }, s3: baseS3Conf};

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
Expand All @@ -86,7 +92,7 @@ describe('registry : domain : validator', function(){

describe('when username and password not specified', function(){

var conf = { publishAuth: { type: 'basic', a: '' }};
var conf = { publishAuth: { type: 'basic', a: '' }, s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -99,7 +105,7 @@ describe('registry : domain : validator', function(){
describe('dependencies', function(){
describe('when not specified', function(){

var conf = { dependencies: null };
var conf = { dependencies: null, s3: baseS3Conf };

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
Expand All @@ -110,7 +116,7 @@ describe('registry : domain : validator', function(){

describe('when it is an array', function(){

var conf = { dependencies: ['hello']};
var conf = { dependencies: ['hello'], s3: baseS3Conf};

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
Expand All @@ -119,7 +125,7 @@ describe('registry : domain : validator', function(){

describe('when it is not an array', function(){

var conf = { dependencies: { hello: 'world' }};
var conf = { dependencies: { hello: 'world' }, s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -129,10 +135,87 @@ describe('registry : domain : validator', function(){
});
});

describe('s3', function(){
describe('when local=true', function(){

var conf = { local: true };

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
});
});

describe('when not in local mode', function(){

var errorMessage = 'Registry configuration is not valid: S3 configuration is not valid';

describe('when s3 settings empty', function(){
var conf = { publishAuth: false, s3: {}};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal(errorMessage);
});
});

describe('when s3 setting is missing bucket', function(){
var conf = { publishAuth: false, s3: {
key: 's3-key', region: 'us-west2', secret: 's3-secret'
}};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal(errorMessage);
});
});

describe('when s3 setting is missing key', function(){
var conf = { publishAuth: false, s3: {
bucket: 'oc-registry', region: 'us-west2', secret: 's3-secret'
}};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal(errorMessage);
});
});

describe('when s3 setting is missing region', function(){
var conf = { publishAuth: false, s3: {
bucket: 'oc-registry', key: 's3-key', secret: 's3-secret'
}};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal(errorMessage);
});
});

describe('when s3 setting is missing secret', function(){
var conf = { publishAuth: false, s3: {
bucket: 'oc-registry', key: 's3-key', region: 'us-west2'
}};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
expect(validate(conf).message).to.equal(errorMessage);
});
});

describe('when s3 setting contains all properties', function(){
var conf = { publishAuth: false, s3: baseS3Conf};

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
});
});
});
});

describe('routes', function(){
describe('when not specified', function(){

var conf = { routes: null };
var conf = { routes: null, s3: baseS3Conf };

it('should be valid', function(){
expect(validate(conf).isValid).to.be.true;
Expand All @@ -143,7 +226,7 @@ describe('registry : domain : validator', function(){

describe('when not an array', function(){

var conf = { routes: {thisis: 'anobject' }};
var conf = { routes: {thisis: 'anobject', s3: baseS3Conf }};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -153,7 +236,7 @@ describe('registry : domain : validator', function(){

describe('when route does not contain route', function(){

var conf = { routes: [{ method: 'get', handler: function(){}}]};
var conf = { routes: [{ method: 'get', handler: function(){}}], s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -163,7 +246,7 @@ describe('registry : domain : validator', function(){

describe('when route does not contain handler', function(){

var conf = { routes: [{ method: 'get', route: '/hello'}]};
var conf = { routes: [{ method: 'get', route: '/hello'}], s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -173,7 +256,7 @@ describe('registry : domain : validator', function(){

describe('when route does not contain method', function(){

var conf = { routes: [{ route: '/hello', handler: function(){}}]};
var conf = { routes: [{ route: '/hello', handler: function(){}}], s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand All @@ -183,7 +266,7 @@ describe('registry : domain : validator', function(){

describe('when route contains handler that is not a function', function(){

var conf = { routes: [{ route: '/hello', method: 'get', handler: 'hello' }]};
var conf = { routes: [{ route: '/hello', method: 'get', handler: 'hello' }], s3: baseS3Conf};

it('should not be valid', function(){
expect(validate(conf).isValid).to.be.false;
Expand Down

0 comments on commit 5427ad1

Please sign in to comment.