Skip to content
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.

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

fix config validator for permitted properties #733

Merged
merged 1 commit into from
Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 13 additions & 4 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 @@ -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())[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(),
aklenik marked this conversation as resolved.
Show resolved Hide resolved
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')[requireRegistrar]()
});

let options = {
Expand Down
25 changes: 24 additions & 1 deletion packages/caliper-fabric/test/configValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -825,22 +825,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