[FAB-2253] Track MSP ID per org and make immutable

https://jira.hyperledger.org/browse/FAB-2253

This CR does two things, it tracks the MSP per organization and checks
for mutation of the MSP ID.  It also exposes access to this MSP ID and
the organization name via the configtxapi interfaces.

Change-Id: Id8eb219199377beed163fb69e077875d7bb8e1f8
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

common/configtx/api/api.go

@@ -40,8 +40,18 @@ type ChannelConfig interface {
 	OrdererAddresses() []string
 }
 
+type OrgConfig interface {
+	// Name returns the name this org is referred to in config
+	Name() string
+
+	// MSPID returns the MSP ID associated with this org
+	MSPID() string
+}
+
 // ApplicationOrgConfig stores the per org application config
 type ApplicationOrgConfig interface {
+	OrgConfig
+
 	// AnchorPeers returns the list of gossip anchor peers
 	AnchorPeers() []*pb.AnchorPeer
 }

common/configtx/handlers/msp/config.go

@@ -20,65 +20,101 @@ import (
 	"fmt"
 	"reflect"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/msp"
-	"github.com/hyperledger/fabric/protos/common"
 	mspprotos "github.com/hyperledger/fabric/protos/msp"
 )
 
+type pendingMSPConfig struct {
+	mspConfig *mspprotos.MSPConfig
+	msp       msp.MSP
+}
+
+type mspConfigStore struct {
+	idMap       map[string]*pendingMSPConfig
+	proposedMgr msp.MSPManager
+}
+
 // MSPConfigHandler
 type MSPConfigHandler struct {
+	pendingConfig *mspConfigStore
 	msp.MSPManager
-	config      []*mspprotos.MSPConfig
-	proposedMgr msp.MSPManager
 }
 
 // BeginConfig called when a config proposal is begun
 func (bh *MSPConfigHandler) BeginConfig() {
-	if bh.config != nil || bh.proposedMgr != nil {
+	if bh.pendingConfig != nil {
 		panic("Programming error, called BeginConfig while a proposal was in process")
 	}
-	bh.config = make([]*mspprotos.MSPConfig, 0)
+	bh.pendingConfig = &mspConfigStore{
+		idMap: make(map[string]*pendingMSPConfig),
+	}
 }
 
 // RollbackConfig called when a config proposal is abandoned
 func (bh *MSPConfigHandler) RollbackConfig() {
-	bh.config = nil
-	bh.proposedMgr = nil
+	bh.pendingConfig = nil
 }
 
 // CommitConfig called when a config proposal is committed
 func (bh *MSPConfigHandler) CommitConfig() {
-	if bh.config == nil {
+	if bh.pendingConfig == nil {
 		panic("Programming error, called CommitConfig with no proposal in process")
 	}
 
-	bh.MSPManager = bh.proposedMgr
-	bh.config = nil
-	bh.proposedMgr = nil
+	bh.MSPManager = bh.pendingConfig.proposedMgr
+	bh.pendingConfig = nil
 }
 
 // ProposeConfig called when config is added to a proposal
-func (bh *MSPConfigHandler) ProposeConfig(key string, configValue *common.ConfigValue) error {
-	mspconfig := &mspprotos.MSPConfig{}
-	err := proto.Unmarshal(configValue.Value, mspconfig)
+func (bh *MSPConfigHandler) ProposeMSP(mspConfig *mspprotos.MSPConfig) (msp.MSP, error) {
+	// check that the type for that MSP is supported
+	if mspConfig.Type != int32(msp.FABRIC) {
+		return nil, fmt.Errorf("Setup error: unsupported msp type %d", mspConfig.Type)
+	}
+
+	// create the msp instance
+	mspInst, err := msp.NewBccspMsp()
 	if err != nil {
-		return fmt.Errorf("Error unmarshalling msp config item, err %s", err)
+		return nil, fmt.Errorf("Creating the MSP manager failed, err %s", err)
 	}
 
-	// TODO handle deduplication more gracefully
-	for _, oMsp := range bh.config {
-		if reflect.DeepEqual(oMsp, mspconfig) {
-			// The MSP is already in the list
-			return nil
+	// set it up
+	err = mspInst.Setup(mspConfig)
+	if err != nil {
+		return nil, fmt.Errorf("Setting up the MSP manager failed, err %s", err)
+	}
+
+	// add the MSP to the map of pending MSPs
+	mspID, err := mspInst.GetIdentifier()
+	if err != nil {
+		return nil, fmt.Errorf("Could not extract msp identifier, err %s", err)
+	}
+
+	existingPendingMSPConfig, ok := bh.pendingConfig.idMap[mspID]
+	if ok && !reflect.DeepEqual(existingPendingMSPConfig.mspConfig, mspConfig) {
+		return nil, fmt.Errorf("Attempted to define two different versions of MSP: %s", mspID)
+	} else {
+		bh.pendingConfig.idMap[mspID] = &pendingMSPConfig{
+			mspConfig: mspConfig,
+			msp:       mspInst,
 		}
 	}
 
-	bh.config = append(bh.config, []*mspprotos.MSPConfig{mspconfig}...)
+	mspList := make([]msp.MSP, len(bh.pendingConfig.idMap))
+	i := 0
+	for _, pendingMSP := range bh.pendingConfig.idMap {
+		mspList[i] = pendingMSP.msp
+		i++
+	}
+
 	// the only way to make sure that I have a
 	// workable config is to toss the proposed
 	// manager, create a new one, call setup on
 	// it and return whatever error setup gives me
-	bh.proposedMgr = msp.NewMSPManager()
-	return bh.proposedMgr.Setup(bh.config)
+	bh.pendingConfig.proposedMgr = msp.NewMSPManager()
+	err = bh.pendingConfig.proposedMgr.Setup(mspList)
+	if err != nil {
+		return nil, err
+	}
+	return mspInst, nil
 }

common/configtx/handlers/msp/config_test.go

@@ -19,29 +19,21 @@ package msp
 import (
 	"testing"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/msp"
-	"github.com/hyperledger/fabric/protos/common"
+	mspprotos "github.com/hyperledger/fabric/protos/msp"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestMSPConfigManager(t *testing.T) {
 	conf, err := msp.GetLocalMspConfig("../../../../msp/sampleconfig/", "DEFAULT")
 	assert.NoError(t, err)
 
-	confBytes, err := proto.Marshal(conf)
-	assert.NoError(t, err)
-
-	ci := &common.ConfigValue{Value: confBytes}
-
 	// test success:
 
 	// begin/propose/commit
-	key := "DEFAULT"
-
 	mspCH := &MSPConfigHandler{}
 	mspCH.BeginConfig()
-	err = mspCH.ProposeConfig(key, ci)
+	_, err = mspCH.ProposeMSP(conf)
 	assert.NoError(t, err)
 	mspCH.CommitConfig()
 
@@ -55,8 +47,8 @@ func TestMSPConfigManager(t *testing.T) {
 	// test failure
 	// begin/propose/commit
 	mspCH.BeginConfig()
-	err = mspCH.ProposeConfig(key, ci)
+	_, err = mspCH.ProposeMSP(conf)
 	assert.NoError(t, err)
-	err = mspCH.ProposeConfig(key, &common.ConfigValue{Value: []byte("BARF!")})
+	_, err = mspCH.ProposeMSP(&mspprotos.MSPConfig{Config: []byte("BARF!")})
 	assert.Error(t, err)
 }

common/configtx/handlers/organization.go

@@ -23,7 +23,9 @@ import (
 	mspconfig "github.com/hyperledger/fabric/common/configtx/handlers/msp"
 	"github.com/hyperledger/fabric/msp"
 	cb "github.com/hyperledger/fabric/protos/common"
+	mspprotos "github.com/hyperledger/fabric/protos/msp"
 
+	"github.com/golang/protobuf/proto"
 	"github.com/op/go-logging"
 )
 
@@ -43,6 +45,7 @@ type orgConfig struct {
 // In general, it should only be referenced as an Impl for the configtx.Manager
 type OrgConfig struct {
 	id            string
+	mspID         string
 	pendingConfig *orgConfig
 	config        *orgConfig
 
@@ -88,12 +91,53 @@ func (oc *OrgConfig) CommitConfig() {
 	oc.pendingConfig = nil
 }
 
+// Name returns the name this org is referred to in config
+func (oc *OrgConfig) Name() string {
+	return oc.id
+}
+
+// MSPID returns the MSP ID associated with this org
+func (oc *OrgConfig) MSPID() string {
+	return oc.mspID
+}
+
 // ProposeConfig is used to add new config to the config proposal
 func (oc *OrgConfig) ProposeConfig(key string, configValue *cb.ConfigValue) error {
 	switch key {
 	case MSPKey:
 		logger.Debugf("Initializing org MSP for id %s", oc.id)
-		return oc.mspConfig.ProposeConfig(key, configValue)
+
+		mspconfig := &mspprotos.MSPConfig{}
+		err := proto.Unmarshal(configValue.Value, mspconfig)
+		if err != nil {
+			return fmt.Errorf("Error unmarshalling msp config for org %s, err %s", oc.id, err)
+		}
+
+		logger.Debugf("Setting up MSP")
+		msp, err := oc.mspConfig.ProposeMSP(mspconfig)
+		if err != nil {
+			return err
+		}
+
+		mspID, err := msp.GetIdentifier()
+		if err != nil {
+			return fmt.Errorf("Could not extract msp identifier for org %s, err %s", oc.id, err)
+		}
+
+		if mspID == "" {
+			return fmt.Errorf("MSP for org %s has empty MSP ID", oc.id)
+		}
+
+		// If the mspID has never been initialized, store it to ensure immutability
+		if oc.mspID == "" {
+			oc.mspID = mspID
+		}
+
+		if mspID != oc.mspID {
+			return fmt.Errorf("MSP for org %s attempted to change its identifier from %s to %s", oc.id, oc.mspID, mspID)
+		}
+
+		oc.pendingConfig.msp = msp
 	default:
 		logger.Warningf("Uknown org config item with key %s", key)
 	}

gossip/service/eventer_test.go

@@ -32,6 +32,14 @@ func (ao applicationOrgs) AnchorPeers() []*peer.AnchorPeer {
 	return ao
 }
 
+func (ao applicationOrgs) MSPID() string {
+	panic("Unimplimented")
+}
+
+func (ao applicationOrgs) Name() string {
+	panic("Unimplimented")
+}
+
 type mockReceiver struct {
 	orgs     map[string]configtxapi.ApplicationOrgConfig
 	sequence uint64

msp/mgmt/testtools/config.go

@@ -23,7 +23,6 @@ import (
 	"github.com/hyperledger/fabric/common/util"
 	"github.com/hyperledger/fabric/msp"
 	"github.com/hyperledger/fabric/msp/mgmt"
-	mspprotos "github.com/hyperledger/fabric/protos/msp"
 )
 
 func getConfigPath(dir string) (string, error) {
@@ -59,9 +58,7 @@ func LoadMSPSetupForTesting(dir string) error {
 		return err
 	}
 
-	fakeConfig := []*mspprotos.MSPConfig{conf}
-
-	err = mgmt.GetManagerForChain(util.GetTestChainID()).Setup(fakeConfig)
+	err = mgmt.GetManagerForChain(util.GetTestChainID()).Setup([]msp.MSP{mgmt.GetLocalMSP()})
 	if err != nil {
 		return err
 	}

msp/msp.go

@@ -59,7 +59,7 @@ type MSPManager interface {
 	IdentityDeserializer
 
 	// Setup the MSP manager instance according to configuration information
-	Setup(msps []*msp.MSPConfig) error
+	Setup(msps []MSP) error
 
 	// GetMSPs Provides a list of Membership Service providers
 	GetMSPs() (map[string]MSP, error)
@@ -240,7 +240,7 @@ type IdentityIdentifier struct {
 // ProviderType indicates the type of an identity provider
 type ProviderType int
 
-// The ProviderTYpe of a member relative to the member API
+// The ProviderType of a member relative to the member API
 const (
 	FABRIC ProviderType = iota // MSP is of FABRIC type
 	OTHER                      // MSP is of OTHER TYPE

msp/msp_test.go

@@ -302,7 +302,7 @@ func TestMain(m *testing.M) {
 	}
 
 	mspMgr = NewMSPManager()
-	err = mspMgr.Setup([]*msp.MSPConfig{conf})
+	err = mspMgr.Setup([]MSP{localMsp})
 	if err != nil {
 		fmt.Printf("Setup for msp manager should have succeeded, got err %s instead", err)
 		os.Exit(-1)

msp/mspmgrimpl.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 
 	"github.com/golang/protobuf/proto"
-	"github.com/hyperledger/fabric/protos/msp"
 	"github.com/op/go-logging"
 )
 
@@ -42,7 +41,7 @@ func NewMSPManager() MSPManager {
 }
 
 // Setup initializes the internal data structures of this manager and creates MSPs
-func (mgr *mspManagerImpl) Setup(msps []*msp.MSPConfig) error {
+func (mgr *mspManagerImpl) Setup(msps []MSP) error {
 	if mgr.up {
 		mspLogger.Infof("MSP manager already up")
 		return nil
@@ -61,26 +60,7 @@ func (mgr *mspManagerImpl) Setup(msps []*msp.MSPConfig) error {
 	// create the map that assigns MSP IDs to their manager instance - once
 	mgr.mspsMap = make(map[string]MSP)
 
-	for _, mspConf := range msps {
-		// check that the type for that MSP is supported
-		if mspConf.Type != int32(FABRIC) {
-			return fmt.Errorf("Setup error: unsupported msp type %d", mspConf.Type)
-		}
-
-		mspLogger.Debugf("Setting up MSP")
-
-		// create the msp instance
-		msp, err := NewBccspMsp()
-		if err != nil {
-			return fmt.Errorf("Creating the MSP manager failed, err %s", err)
-		}
-
-		// set it up
-		err = msp.Setup(mspConf)
-		if err != nil {
-			return fmt.Errorf("Setting up the MSP manager failed, err %s", err)
-		}
-
+	for _, msp := range msps {
 		// add the MSP to the map of active MSPs
 		mspID, err := msp.GetIdentifier()
 		if err != nil {