From 7918d5ed56fc64f296eaaa8670a6a485c5707ad3 Mon Sep 17 00:00:00 2001 From: Jason Yellick Date: Wed, 7 Jun 2017 23:44:11 -0400 Subject: [PATCH] [FAB-3831] Report empty application/orderer groups The config interface currently returns the application/orderer config groups as an interface definition. Because of this, a simple nil check will not generally determine whether the config is actually set, and consequently attempting to access the config may cause a nil pointer dereference (even when the nil check returns false). This CR adds a bool to this interface, to allow the caller to determine whether the interface has been populated. Change-Id: I37e0ba25dcacffb273e5712f072d65786206b05b Signed-off-by: Jason Yellick --- common/configtx/api/api.go | 6 ++++-- common/configtx/initializer.go | 16 +++++++++++---- common/mocks/configtx/configtx.go | 8 ++++---- core/peer/peer.go | 33 ++++++++++++++++++++++--------- orderer/multichain/manager.go | 6 +++++- 5 files changed, 49 insertions(+), 20 deletions(-) diff --git a/common/configtx/api/api.go b/common/configtx/api/api.go index bdbbe94c7e5..b5901e8457f 100644 --- a/common/configtx/api/api.go +++ b/common/configtx/api/api.go @@ -59,14 +59,16 @@ type Resources interface { ChannelConfig() config.Channel // OrdererConfig returns the config.Orderer for the channel - OrdererConfig() config.Orderer + // and whether the Orderer config exists + OrdererConfig() (config.Orderer, bool) // ConsortiumsConfig() returns the config.Consortiums for the channel // and whether the consortiums config exists ConsortiumsConfig() (config.Consortiums, bool) // ApplicationConfig returns the configtxapplication.SharedConfig for the channel - ApplicationConfig() config.Application + // and whether the Application config exists + ApplicationConfig() (config.Application, bool) // MSPManager returns the msp.MSPManager for the chain MSPManager() msp.MSPManager diff --git a/common/configtx/initializer.go b/common/configtx/initializer.go index d2fea9ab7e4..420fad3e84d 100644 --- a/common/configtx/initializer.go +++ b/common/configtx/initializer.go @@ -47,13 +47,21 @@ func (r *resources) ChannelConfig() config.Channel { } // OrdererConfig returns the api.OrdererConfig for the chain -func (r *resources) OrdererConfig() config.Orderer { - return r.configRoot.Orderer() +func (r *resources) OrdererConfig() (config.Orderer, bool) { + result := r.configRoot.Orderer() + if result == nil { + return nil, false + } + return result, true } // ApplicationConfig returns the api.ApplicationConfig for the chain -func (r *resources) ApplicationConfig() config.Application { - return r.configRoot.Application() +func (r *resources) ApplicationConfig() (config.Application, bool) { + result := r.configRoot.Application() + if result == nil { + return nil, false + } + return result, true } // ConsortiumsConfig returns the api.ConsortiumsConfig for the chain and whether or not diff --git a/common/mocks/configtx/configtx.go b/common/mocks/configtx/configtx.go index 2f4294b2720..2bfdc85f6a5 100644 --- a/common/mocks/configtx/configtx.go +++ b/common/mocks/configtx/configtx.go @@ -57,13 +57,13 @@ func (r *Resources) ChannelConfig() config.Channel { } // Returns the OrdererConfigVal -func (r *Resources) OrdererConfig() config.Orderer { - return r.OrdererConfigVal +func (r *Resources) OrdererConfig() (config.Orderer, bool) { + return r.OrdererConfigVal, r.OrdererConfigVal == nil } // Returns the ApplicationConfigVal -func (r *Resources) ApplicationConfig() config.Application { - return r.ApplicationConfigVal +func (r *Resources) ApplicationConfig() (config.Application, bool) { + return r.ApplicationConfigVal, r.ApplicationConfigVal == nil } func (r *Resources) ConsortiumsConfig() (config.Consortiums, bool) { diff --git a/core/peer/peer.go b/core/peer/peer.go index c19af5492ec..e04810a7daa 100644 --- a/core/peer/peer.go +++ b/core/peer/peer.go @@ -185,9 +185,14 @@ func createChain(cid string, ledger ledger.PeerLedger, cb *common.Block) error { gossipEventer := service.GetGossipService().NewConfigEventer() gossipCallbackWrapper := func(cm configtxapi.Manager) { + ac, ok := configtxInitializer.ApplicationConfig() + if !ok { + // TODO, handle a missing ApplicationConfig more gracefully + ac = nil + } gossipEventer.ProcessConfigUpdate(&chainSupport{ Manager: cm, - Application: configtxInitializer.ApplicationConfig(), + Application: ac, }) service.GetGossipService().SuspectPeers(func(identity api.PeerIdentityType) bool { // TODO: this is a place-holder that would somehow make the MSP layer suspect @@ -214,9 +219,13 @@ func createChain(cid string, ledger ledger.PeerLedger, cb *common.Block) error { // TODO remove once all references to mspmgmt are gone from peer code mspmgmt.XXXSetMSPManager(cid, configtxManager.MSPManager()) + ac, ok := configtxInitializer.ApplicationConfig() + if !ok { + ac = nil + } cs := &chainSupport{ Manager: configtxManager, - Application: configtxManager.ApplicationConfig(), // TODO, refactor as this is accessible through Manager + Application: ac, // TODO, refactor as this is accessible through Manager ledger: ledger, } @@ -386,10 +395,14 @@ func buildTrustedRootsForChain(cm configtxapi.Manager) { ordererRootCAs := [][]byte{} appOrgMSPs := make(map[string]struct{}) - //loop through app orgs and build map of MSPIDs - for _, appOrg := range cm.ApplicationConfig().Organizations() { - appOrgMSPs[appOrg.MSPID()] = struct{}{} + ac, ok := cm.ApplicationConfig() + if ok { + //loop through app orgs and build map of MSPIDs + for _, appOrg := range ac.Organizations() { + appOrgMSPs[appOrg.MSPID()] = struct{}{} + } } + cid := cm.ChainID() peerLogger.Debugf("updating root CAs for channel [%s]", cid) msps, err := cm.MSPManager().GetMSPs() @@ -452,13 +465,15 @@ func GetMSPIDs(cid string) []string { return mockMSPIDGetter(cid) } if c, ok := chains.list[cid]; ok { - if c == nil || c.cs == nil || - c.cs.ApplicationConfig() == nil || - c.cs.ApplicationConfig().Organizations() == nil { + if c == nil || c.cs == nil { + return nil + } + ac, ok := c.cs.ApplicationConfig() + if !ok || ac.Organizations() == nil { return nil } - orgs := c.cs.ApplicationConfig().Organizations() + orgs := ac.Organizations() toret := make([]string, len(orgs)) i := 0 for _, org := range orgs { diff --git a/orderer/multichain/manager.go b/orderer/multichain/manager.go index 1bf03a13e9d..0e34c15a6f4 100644 --- a/orderer/multichain/manager.go +++ b/orderer/multichain/manager.go @@ -57,7 +57,11 @@ type configResources struct { } func (cr *configResources) SharedConfig() config.Orderer { - return cr.OrdererConfig() + oc, ok := cr.OrdererConfig() + if !ok { + logger.Panicf("[channel %s] has no orderer configuration", cr.ChainID()) + } + return oc } type ledgerResources struct {