Skip to content

Commit

Permalink
fix config validator for permitted properties
Browse files Browse the repository at this point in the history
  • Loading branch information
nklincoln committed Feb 18, 2020
1 parent d523eb2 commit 8d160e1
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 16 deletions.
19 changes: 14 additions & 5 deletions packages/caliper-fabric/lib/configValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ class ConfigValidator {
throw new Error('Use of service discovery is only valid with a `caliper-flow-only-test` flag');
}

// registrar requirement removed if only-test
let requireRegistrar = 'required';
if (flowOptions.performTest && (!flowOptions.performInit && !flowOptions.performInstall)) {
requireRegistrar = 'optional';
}

let tls; // undefined => we don't know yet
// the TLS setting might not be known after the individual section if they are missing
// the first existing node will determine its value, and after that every node is validated against that value
Expand All @@ -53,7 +59,7 @@ class ConfigValidator {
cas = Object.keys(config.certificateAuthorities);
for (let ca of cas) {
try {
ConfigValidator.validateCertificateAuthority(config.certificateAuthorities[ca], tls);
ConfigValidator.validateCertificateAuthority(config.certificateAuthorities[ca], tls, requireRegistrar);
tls = (tls || false) || config.certificateAuthorities[ca].url.startsWith('https://');
} catch (err) {
throw new Error(`Invalid "${ca}" CA configuration: ${err.message}`);
Expand Down Expand Up @@ -159,7 +165,7 @@ class ConfigValidator {
const schema = j.object().keys({
// simple attributes
name: j.string().min(1).required(),
version: j.string().valid('1.0').required(),
version: j.string().regex(/\d+\.\d+\.\d$/).required(),
'mutual-tls': j.boolean().valid(mutualTlsValid).optional(),
wallet: j.string().min(1).optional(),
caliper: j.object().keys({
Expand Down Expand Up @@ -206,6 +212,7 @@ class ConfigValidator {
const binary = !!config.configBinary;
const def = !!config.definition;
const ordererModif = discovery ? 'optional' : 'required';
const peerModif = discovery ? 'optional' : 'required';

let binaryModif;
let defModif;
Expand Down Expand Up @@ -306,7 +313,7 @@ class ConfigValidator {
})[defModif](),

orderers: j.array().sparse(false).items(j.string().valid(validOrderers)).unique()[ordererModif](),
peers: j.object().keys(createPeersSchema()).required(),
peers: j.object().keys(createPeersSchema()).required()[peerModif](),

// leave this embedded, so the validation error messages are more meaningful
chaincodes: j.array().sparse(false).items(j.object().keys({
Expand Down Expand Up @@ -358,11 +365,13 @@ class ConfigValidator {
* Validates the given CA configuration object.
* @param {object} config The configuration object.
* @param {boolean} tls Indicates whether TLS is enabled or known at this point.
* @param {string} requireRegistrar Indicates whether a registrar is optional or required.
*/
static validateCertificateAuthority(config, tls) {
static validateCertificateAuthority(config, tls, requireRegistrar) {
let urlRegex = tls === undefined ? /^(https|http):\/\// : (tls ? /^https:\/\// : /^http:\/\//);

const schema = j.object().keys({
caName: j.string().optional(),
url: j.string().uri().regex(urlRegex).required(),

httpOptions: j.object().optional(),
Expand All @@ -380,7 +389,7 @@ class ConfigValidator {
registrar: j.array().items(j.object().keys({
enrollId: j.string().min(1).required(),
enrollSecret: j.string().min(1).required()
})).min(1).sparse(false).unique('enrollId').required()
})).min(1).sparse(false).unique('enrollId').required()[requireRegistrar]()
});

let options = {
Expand Down
39 changes: 34 additions & 5 deletions packages/caliper-fabric/test/configValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Class: ConfigValidator', () => {
describe('Function: validateNetwork', () => {
let config = {
name: 'Fabric',
version: '1.0',
version: '1.0.0',
'mutual-tls': false,
caliper: {
blockchain: 'fabric'
Expand Down Expand Up @@ -437,7 +437,7 @@ describe('Class: ConfigValidator', () => {
// good practice for auto complete and easy backup
let config = {
name: 'Fabric',
version: '1.0',
version: '1.0.0',
'mutual-tls': false,
wallet: '/path',
caliper: {
Expand Down Expand Up @@ -502,12 +502,18 @@ describe('Class: ConfigValidator', () => {
call.should.throw(err);
});

it('should throw for an invalid string value', () => {
const err = 'child "version" fails because ["version" must be one of [1.0]]';
it('should throw for an invalid string value (non-semver)', () => {
const err = 'child "version" fails because ["version" with value "2.0" fails to match the required pattern: /\\d+\\.\\d+\\.\\d$/]';
config.version = '2.0';
call.should.throw(err);
});

it('should throw for an invalid string value (non-semver)', () => {
const err = 'child "version" fails because ["version" with value "2.1a8" fails to match the required pattern: /\\d+\\.\\d+\\.\\d$/]';
config.version = '2.1a8';
call.should.throw(err);
});

it('should throw for a non-string value', () => {
const err = 'child "version" fails because ["version" must be a string]';
config.version = true;
Expand Down Expand Up @@ -825,22 +831,45 @@ describe('Class: ConfigValidator', () => {
};
let configString = JSON.stringify(config);

let configNoRegistrar = {
url: 'https://localhost:7054',
httpOptions: {
verify: false
},
tlsCACerts: {
path: 'my/path/tocert'
}
};
let configStringNoRegistrar = JSON.stringify(configNoRegistrar);

// reset the local config before every test
beforeEach(() => {
config = JSON.parse(configString);
configNoRegistrar = JSON.parse(configStringNoRegistrar);
});

/**
* Wraps the actual call, so "should" can call this function without parameters
*/
function call() {
ConfigValidator.validateCertificateAuthority(config, tls);
ConfigValidator.validateCertificateAuthority(config, tls, 'required');
}

/**
* Wraps the actual call, so "should" can call this function without parameters
*/
function callNoRegistrar() {
ConfigValidator.validateCertificateAuthority(configNoRegistrar, tls, 'optional');
}

it('should not throw for a valid value', () => {
call.should.not.throw();
});

it('should not throw for a valid value', () => {
callNoRegistrar.should.not.throw();
});

it('should throw for an unknown child property', () => {
const err = '"unknown" is not allowed';
config.unknown = '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"

caliper:
blockchain: fabric
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"
mutual-tls: true

caliper:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"
mutual-tls: true

caliper:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"
mutual-tls: true

caliper:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"

caliper:
blockchain: fabric
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#

name: Fabric
version: "1.0"
version: "1.0.0"
mutual-tls: true

caliper:
Expand Down

0 comments on commit 8d160e1

Please sign in to comment.