Skip to content

Commit

Permalink
[FABN-976] Fix chaincode package install arg validation
Browse files Browse the repository at this point in the history 
Client.installChaincode requires a chaincode ID/version with chaincode
package. This is wrong, as the chaincode ID/version are not actually used
when you specify a chaincode package. The combo of required options
should be:

chaincodeId, chaincodeVersion, and chaincodePath
-or-
chaincodePackage

Also fixed the TypeScript definitions to allow for chaincode package
install without needing a chaincode ID/version, and did a bit of a
rework on the unit tests to cope with these changes - like ensuring
that errors are actually thrown!

Change-Id: I9632ed6b8897c2c3a6a71ce1db674b3f0e59e9a7
Signed-off-by: Simon Stone <sstone1@uk.ibm.com>
  • Loading branch information
Simon Stone committed Oct 18, 2018
1 parent 4f7c9aa commit 9b7a616
Show file tree
Hide file tree
Showing 3 changed files with 177 additions and 107 deletions.
108 changes: 56 additions & 52 deletions fabric-client/lib/Client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1029,12 +1029,27 @@ const Client = class extends BaseClient {
* @returns {Promise} A Promise for a {@link ProposalResponseObject}
*/
async installChaincode(request, timeout) {
logger.debug('installChaincode - start');
try {
logger.debug('installChaincode - start');

// must provide a valid request object with:
// chaincodePackage
// -or-
// chaincodeId, chaincodeVersion, and chaincodePath
if (!request) {
throw new Error('Missing input request object on install chaincode request');
} else if (request.chaincodePackage) {
logger.debug('installChaincode - installing chaincode package');
} else if (!request.chaincodeId) {
throw new Error('Missing "chaincodeId" parameter in the proposal request');
} else if (!request.chaincodeVersion) {
throw new Error('Missing "chaincodeVersion" parameter in the proposal request');
} else if (!request.chaincodePath) {
throw new Error('Missing "chaincodePath" parameter in the proposal request');
}

let error_msg = null;
let peers = null;
if (request) {
peers = this.getTargetPeers(request.targets);
console.log('request.targets', request.targets);
let peers = this.getTargetPeers(request.targets);
if (!peers && request.channelNames) {
peers = this.getPeersForOrgOnChannel(request.channelNames);
}
Expand All @@ -1043,58 +1058,47 @@ const Client = class extends BaseClient {
if (peers && peers.length > 0) {
logger.debug(`installChaincode - found peers ::${peers.length}`);
} else {
error_msg = 'Missing peer objects in install chaincode request';
throw new Error('Missing peer objects in install chaincode request');
}
} else {
error_msg = 'Missing input request object on install chaincode request';
}

if (!error_msg) {
error_msg = clientUtils.checkProposalRequest(request, false);
}
if (!error_msg) {
error_msg = clientUtils.checkInstallRequest(request);
}

if (error_msg) {
logger.error(`installChaincode error ${error_msg}`);
throw new Error(error_msg);
}
const cdsBytes = await _getChaincodeDeploymentSpec(request, this.isDevMode());
// TODO add ESCC/VSCC info here ??????
const lcccSpec = {
type: clientUtils.translateCCType(request.chaincodeType),
chaincode_id: {
name: Constants.LSCC
},
input: {
args: [Buffer.from('install', 'utf8'), cdsBytes]
}
};

const cdsBytes = await _getChaincodeDeploymentSpec(request, this.isDevMode());
// TODO add ESCC/VSCC info here ??????
const lcccSpec = {
type: clientUtils.translateCCType(request.chaincodeType),
chaincode_id: {
name: Constants.LSCC
},
input: {
args: [Buffer.from('install', 'utf8'), cdsBytes]
let signer;
let tx_id = request.txId;
if (!tx_id) {
signer = this._getSigningIdentity(true);
tx_id = new TransactionID(signer, true);
} else {
signer = this._getSigningIdentity(tx_id.isAdmin());
}
};

let signer;
let tx_id = request.txId;
if (!tx_id) {
signer = this._getSigningIdentity(true);
tx_id = new TransactionID(signer, true);
} else {
signer = this._getSigningIdentity(tx_id.isAdmin());
}

const channelHeader = clientUtils.buildChannelHeader(
_commonProto.HeaderType.ENDORSER_TRANSACTION,
'', //install does not target a channel
tx_id.getTransactionID(),
null,
Constants.LSCC
);
const header = clientUtils.buildHeader(signer, channelHeader, tx_id.getNonce());
const proposal = clientUtils.buildProposal(lcccSpec, header);
const signed_proposal = clientUtils.signProposal(signer, proposal);
logger.debug('installChaincode - about to sendPeersProposal');
const responses = await clientUtils.sendPeersProposal(peers, signed_proposal, timeout);
return [responses, proposal];
const channelHeader = clientUtils.buildChannelHeader(
_commonProto.HeaderType.ENDORSER_TRANSACTION,
'', //install does not target a channel
tx_id.getTransactionID(),
null,
Constants.LSCC
);
const header = clientUtils.buildHeader(signer, channelHeader, tx_id.getNonce());
const proposal = clientUtils.buildProposal(lcccSpec, header);
const signed_proposal = clientUtils.signProposal(signer, proposal);
logger.debug('installChaincode - about to sendPeersProposal');
const responses = await clientUtils.sendPeersProposal(peers, signed_proposal, timeout);
return [responses, proposal];
} catch (error) {
logger.error(`installChaincode error ${error.message}`);
throw error;
}
}

/**
Expand Down
156 changes: 107 additions & 49 deletions fabric-client/test/Client.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ const rewire = require('rewire');
const Client = rewire('../lib/Client');

const chai = require('chai');
const chaiAsPromised = require('chai-as-promised');
const sinon = require('sinon');
const should = chai.should();
chai.use(chaiAsPromised);

function propertiesToBeEqual(obj, properties, value) {
properties.forEach((prop) => {
Expand Down Expand Up @@ -1420,8 +1422,6 @@ describe('Client', () => {
describe('#installChaincode', () => {
let getTargetPeersStub;
let getPeersForOrgOnChannelStub;
let checkProposalRequestStub;
let checkInstallRequestStub;
let _getSigningIdentityStub;
let TransactionIDStub;
let isAdminStub;
Expand All @@ -1437,15 +1437,11 @@ describe('Client', () => {

let client;
beforeEach(() => {
checkProposalRequestStub = sandbox.stub();
revert.push(Client.__set__('clientUtils.checkProposalRequest', checkProposalRequestStub));
checkInstallRequestStub = sandbox.stub();
revert.push(Client.__set__('clientUtils.checkInstallRequest', checkInstallRequestStub));
getPeersForOrgOnChannelStub = sandbox.stub();
getTargetPeersStub = sandbox.stub();
_getSigningIdentityStub = sandbox.stub().returns('signer');
getNonceStub = sandbox.stub().returns('nonce');
isAdminStub = sandbox.stub().returns('is-admin');
isAdminStub = sandbox.stub().returns(true);
getTransactionIDStub = sandbox.stub().returns('txId');
TransactionIDStub = sandbox.stub().returns({ isAdmin: isAdminStub, getTransactionID: getTransactionIDStub, getNonce: getNonceStub });
revert.push(Client.__set__('TransactionID', TransactionIDStub));
Expand All @@ -1472,58 +1468,101 @@ describe('Client', () => {
});

it('should throw error if not request given', async () => {
try {
await client.installChaincode();
} catch (err) {
err.message.should.equal('Missing input request object on install chaincode request');
sinon.assert.calledWith(FakeLogger.error, 'installChaincode error Missing input request object on install chaincode request');
}
await client.installChaincode()
.should.be.rejectedWith(/Missing input request object on install chaincode request/);
sinon.assert.calledWith(FakeLogger.error, 'installChaincode error Missing input request object on install chaincode request');
});

it('should throw error if chaincodeId not specified', async () => {
await client.installChaincode({ chaincodeVersion: '0.0.1', chaincodePath: 'mycc' })
.should.be.rejectedWith(/Missing "chaincodeId" parameter in the proposal request/);
});

it('should throw error if chaincodeVersion not specified', async () => {
await client.installChaincode({ chaincodeId: 'mycc', chaincodePath: 'mycc' })
.should.be.rejectedWith(/Missing "chaincodeVersion" parameter in the proposal request/);
});

it('should throw error if chaincodePath not specified', async () => {
await client.installChaincode({ chaincodeId: 'mycc', chaincodeVersion: '0.0.1' })
.should.be.rejectedWith(/Missing "chaincodePath" parameter in the proposal request/);
});

it('should throw error if no peers found', async () => {
try {
await client.installChaincode({ targets: [], channelNames: [] });
} catch (err) {
err.message.should.equal('Missing peer objects in install chaincode request');
}
await client.installChaincode({ chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc' })
.should.be.rejectedWith(/Missing peer objects in install chaincode request/);
});

it('should throw error proposal request is invalid', async () => {
checkProposalRequestStub.returns('test error');
getTargetPeersStub.returns(['peer']);
try {
await client.installChaincode({ targets: [], channelNames: [] });
} catch (err) {
err.message.should.equal('test error');
sinon.assert.calledWith(FakeLogger.debug, 'installChaincode - found peers ::1');
sinon.assert.calledWith(checkProposalRequestStub, { targets: [], channelNames: [] }, false);
sinon.assert.calledWith(FakeLogger.error, 'installChaincode error test error');
}
it('should install using chaincode ID, chaincode version, and chaincode path', async () => {
getTargetPeersStub.withArgs(['peer']).returns(['peer']);
const request = { chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc', targets: ['peer'] };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, ['peer']);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, undefined);
sinon.assert.calledWith(_getSigningIdentityStub, true);
sinon.assert.calledWith(buildChannelHeaderStub, 'ENDORSER_TRANSACITON', '', 'txId', null, 'lscc');
sinon.assert.calledWith(buildHeaderStub, 'signer', 'channel-header', 'nonce');
sinon.assert.calledWith(buildProposalStub, {
chaincode_id: { name: 'lscc' },
input: { args: [Buffer.from('install', 'utf8'), 'cdsBytes'] },
type: 'go'
}, 'header');
sinon.assert.calledWith(signProposalStub, 'signer', 'proposal');
sinon.assert.calledWith(sendPeersProposalStub, ['peer'], 'signed-proposal', undefined);
response.should.deep.equal([['response'], 'proposal']);
});

it('should throw error install is invalid', async () => {
checkInstallRequestStub.returns('test error');
getTargetPeersStub.returns(['peer']);
try {
await client.installChaincode({ targets: [], channelNames: [] });
} catch (err) {
err.message.should.equal('test error');
sinon.assert.calledWith(FakeLogger.debug, 'installChaincode - found peers ::1');
sinon.assert.calledWith(checkProposalRequestStub, { targets: [], channelNames: [] });
sinon.assert.calledWith(FakeLogger.error, 'installChaincode error test error');
}
it('should install using chaincode ID, chaincode version, chaincode path, and chaincode type', async () => {
getTargetPeersStub.withArgs(['peer']).returns(['peer']);
const request = { chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc', chaincodeType: 'java', targets: ['peer'] };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, ['peer']);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, 'java');
sinon.assert.calledWith(_getSigningIdentityStub, true);
sinon.assert.calledWith(buildChannelHeaderStub, 'ENDORSER_TRANSACITON', '', 'txId', null, 'lscc');
sinon.assert.calledWith(buildHeaderStub, 'signer', 'channel-header', 'nonce');
sinon.assert.calledWith(buildProposalStub, {
chaincode_id: { name: 'lscc' },
input: { args: [Buffer.from('install', 'utf8'), 'cdsBytes'] },
type: 'go'
}, 'header');
sinon.assert.calledWith(signProposalStub, 'signer', 'proposal');
sinon.assert.calledWith(sendPeersProposalStub, ['peer'], 'signed-proposal', undefined);
response.should.deep.equal([['response'], 'proposal']);
});

it('should install using a chaincode package', async () => {
getTargetPeersStub.withArgs(['peer']).returns(['peer']);
const chaincodePackage = Buffer.from('hello world');
_getChaincodeDeploymentSpecStub.resolves(chaincodePackage);
const request = { chaincodePackage, targets: ['peer'] };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, ['peer']);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, undefined);
sinon.assert.calledWith(_getSigningIdentityStub, true);
sinon.assert.calledWith(buildChannelHeaderStub, 'ENDORSER_TRANSACITON', '', 'txId', null, 'lscc');
sinon.assert.calledWith(buildHeaderStub, 'signer', 'channel-header', 'nonce');
sinon.assert.calledWith(buildProposalStub, {
chaincode_id: { name: 'lscc' },
input: { args: [Buffer.from('install', 'utf8'), chaincodePackage] },
type: 'go'
}, 'header');
sinon.assert.calledWith(signProposalStub, 'signer', 'proposal');
sinon.assert.calledWith(sendPeersProposalStub, ['peer'], 'signed-proposal', undefined);
response.should.deep.equal([['response'], 'proposal']);
});

it('should return responses and a proposal when transactionId is given', async () => {
it('should install using an explicit transaction ID', async () => {
getTargetPeersStub.returns(['peer']);
const request = { targets: [], channelNames: [], txId: { isAdmin: isAdminStub, getNonce: getNonceStub, getTransactionID: getTransactionIDStub }, chaincodeType: 'go' };
const request = { chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc', targets: [], channelNames: [], txId: { isAdmin: isAdminStub, getNonce: getNonceStub, getTransactionID: getTransactionIDStub }, chaincodeType: 'go' };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, []);
sinon.assert.calledWith(checkProposalRequestStub, request, false);
sinon.assert.calledWith(checkInstallRequestStub, request);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, 'go');
sinon.assert.calledWith(_getSigningIdentityStub, 'is-admin');
sinon.assert.calledWith(_getSigningIdentityStub, true);
sinon.assert.calledWith(buildChannelHeaderStub, 'ENDORSER_TRANSACITON', '', 'txId', null, 'lscc');
sinon.assert.calledWith(buildHeaderStub, 'signer', 'channel-header', 'nonce');
sinon.assert.calledWith(buildProposalStub, {
Expand All @@ -1536,13 +1575,11 @@ describe('Client', () => {
response.should.deep.equal([['response'], 'proposal']);
});

it('should return responses and a proposal when transactionId is not given', async () => {
it('should install using the specified target peers', async () => {
getTargetPeersStub.returns(['peer']);
const request = { targets: [], channelNames: [], chaincodeType: 'go' };
const request = { chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc', targets: [], channelNames: [], txId: { isAdmin: isAdminStub, getNonce: getNonceStub, getTransactionID: getTransactionIDStub }, chaincodeType: 'go' };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, []);
sinon.assert.calledWith(checkProposalRequestStub, request, false);
sinon.assert.calledWith(checkInstallRequestStub, request);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, 'go');
sinon.assert.calledWith(_getSigningIdentityStub, true);
Expand All @@ -1557,6 +1594,27 @@ describe('Client', () => {
sinon.assert.calledWith(sendPeersProposalStub, ['peer'], 'signed-proposal', undefined);
response.should.deep.equal([['response'], 'proposal']);
});

it('should install using the peers discovered for the channel', async () => {
getTargetPeersStub.returns();
getPeersForOrgOnChannelStub.withArgs(['mychannel']).returns(['peer']);
const request = { chaincodeId: 'mycc', chaincodeVersion: '0.0.1', chaincodePath: 'mycc', channelNames: ['mychannel'] };
const response = await client.installChaincode(request);
sinon.assert.calledWith(getTargetPeersStub, undefined);
sinon.assert.calledWith(_getChaincodeDeploymentSpecStub, request, client.isDevMode());
sinon.assert.calledWith(translateCCTypeStub, undefined);
sinon.assert.calledWith(_getSigningIdentityStub, true);
sinon.assert.calledWith(buildChannelHeaderStub, 'ENDORSER_TRANSACITON', '', 'txId', null, 'lscc');
sinon.assert.calledWith(buildHeaderStub, 'signer', 'channel-header', 'nonce');
sinon.assert.calledWith(buildProposalStub, {
chaincode_id: { name: 'lscc' },
input: { args: [Buffer.from('install', 'utf8'), 'cdsBytes'] },
type: 'go'
}, 'header');
sinon.assert.calledWith(signProposalStub, 'signer', 'proposal');
sinon.assert.calledWith(sendPeersProposalStub, ['peer'], 'signed-proposal', undefined);
response.should.deep.equal([['response'], 'proposal']);
});
});

describe('#initCredentialStores', () => {
Expand Down
20 changes: 14 additions & 6 deletions fabric-client/types/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,18 +427,26 @@ declare namespace Client {
principal: Buffer;
}

export interface ChaincodeInstallRequest {
export interface ChaincodePackageInstallRequest {
targets?: Peer[] | string[];
chaincodePath: string;
metadataPath?: string;
channelNames?: string[] | string;
txId?: TransactionId;
chaincodePackage: Buffer;
}

export interface ChaincodePathInstallRequest {
targets?: Peer[] | string[];
channelNames?: string[] | string;
txId?: TransactionId;
chaincodeId: string;
chaincodeVersion: string;
chaincodePackage?: Buffer;
chaincodePath: string;
chaincodeType?: ChaincodeType;
channelNames?: string[] | string;
txId: TransactionId;
metadataPath?: string;
}

export type ChaincodeInstallRequest = ChaincodePackageInstallRequest | ChaincodePathInstallRequest;

export interface ChaincodeInstantiateUpgradeRequest {
targets?: Peer[] | string[];
chaincodeType?: ChaincodeType;
Expand Down

0 comments on commit 9b7a616

Please sign in to comment.