[FAB-10451] discovery config uses MSP IDs

The discovery config returns a mapping with keys which are organization names,
but it should really return keys that are the MSP IDs.

This change set makes it so, and also adds a test that:
- Generates the e2e_cli layout for 4 organizations:
  - Orderer org
  - BCCSP MSP org1, org2
  - Idemix MSP org3

The names of the organizations in the config block are randomized,
and the test ensures that:
- The discovery processing ignores idemix MSPs when parsing config
- The discovery processing uses the MSP IDs and not the org names.

Change-Id: Ie4e15c7fa2987af3363fd7aae5183bafd22a0124
Signed-off-by: yacovm <yacovm@il.ibm.com>

Author: yacovm

discovery/support/config/support.go

@@ -13,12 +13,16 @@ import (
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/channelconfig"
+	"github.com/hyperledger/fabric/common/flogging"
+	mspconstants "github.com/hyperledger/fabric/msp"
 	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/discovery"
 	"github.com/hyperledger/fabric/protos/msp"
 	"github.com/pkg/errors"
 )
 
+var logger = flogging.MustGetLogger("discovery/config")
+
 // CurrentConfigBlockGetter enables to fetch the last config block
 type CurrentConfigBlockGetter interface {
 	// GetCurrConfigBlock returns the current config block for the given channel
@@ -99,8 +103,23 @@ func (s *DiscoverySupport) Config(channel string) (*discovery.ConfigResult, erro
 
 func computeOrdererEndpoints(ordererGrp map[string]*common.ConfigGroup, ordererAddresses *common.OrdererAddresses) (map[string]*discovery.Endpoints, error) {
 	res := make(map[string]*discovery.Endpoints)
-	for ordererOrg := range ordererGrp {
-		res[ordererOrg] = &discovery.Endpoints{}
+	for name, group := range ordererGrp {
+		mspConfig := &msp.MSPConfig{}
+		if err := proto.Unmarshal(group.Values[channelconfig.MSPKey].Value, mspConfig); err != nil {
+			return nil, errors.Wrap(err, "failed parsing MSPConfig")
+		}
+		// Skip non fabric MSPs, as they don't carry useful information for service discovery.
+		// An idemix MSP shouldn't appear inside an orderer group, but this isn't a fatal error
+		// for the discovery service and we can just ignore it.
+		if mspConfig.Type != int32(mspconstants.FABRIC) {
+			logger.Error("Orderer group", name, "is not a FABRIC MSP, but is of type", mspConfig.Type)
+			continue
+		}
+		fabricConfig := &msp.FabricMSPConfig{}
+		if err := proto.Unmarshal(mspConfig.Config, fabricConfig); err != nil {
+			return nil, errors.Wrap(err, "failed marshaling FabricMSPConfig")
+		}
+		res[fabricConfig.Name] = &discovery.Endpoints{}
 		for _, endpoint := range ordererAddresses.Addresses {
 			host, portStr, err := net.SplitHostPort(endpoint)
 			if err != nil {
@@ -110,7 +129,7 @@ func computeOrdererEndpoints(ordererGrp map[string]*common.ConfigGroup, ordererA
 			if err != nil {
 				return nil, errors.Errorf("%s is not a valid port number", portStr)
 			}
-			res[ordererOrg].Endpoint = append(res[ordererOrg].Endpoint, &discovery.Endpoint{
+			res[fabricConfig.Name].Endpoint = append(res[fabricConfig.Name].Endpoint, &discovery.Endpoint{
 				Host: host,
 				Port: uint32(port),
 			})
@@ -121,19 +140,23 @@ func computeOrdererEndpoints(ordererGrp map[string]*common.ConfigGroup, ordererA
 
 func appendMSPConfigs(ordererGrp, appGrp map[string]*common.ConfigGroup, output map[string]*msp.FabricMSPConfig) error {
 	for _, group := range []map[string]*common.ConfigGroup{ordererGrp, appGrp} {
-		for orgID, grp := range group {
+		for _, grp := range group {
 			mspConfig := &msp.MSPConfig{}
 			if err := proto.Unmarshal(grp.Values[channelconfig.MSPKey].Value, mspConfig); err != nil {
 				return errors.Wrap(err, "failed parsing MSPConfig")
 			}
+			// Skip non fabric MSPs, as they don't carry useful information for service discovery
+			if mspConfig.Type != int32(mspconstants.FABRIC) {
+				continue
+			}
 			fabricConfig := &msp.FabricMSPConfig{}
 			if err := proto.Unmarshal(mspConfig.Config, fabricConfig); err != nil {
 				return errors.Wrap(err, "failed marshaling FabricMSPConfig")
 			}
-			if _, exists := output[orgID]; exists {
+			if _, exists := output[fabricConfig.Name]; exists {
 				continue
 			}
-			output[orgID] = fabricConfig
+			output[fabricConfig.Name] = fabricConfig
 		}
 	}
 

discovery/support/config/support_test.go

@@ -7,14 +7,23 @@ SPDX-License-Identifier: Apache-2.0
 package config_test
 
 import (
+	"crypto/rand"
+	"encoding/hex"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
 	"testing"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/channelconfig"
 	"github.com/hyperledger/fabric/common/configtx/test"
+	"github.com/hyperledger/fabric/common/tools/configtxgen/encoder"
+	genesisconfig "github.com/hyperledger/fabric/common/tools/configtxgen/localconfig"
 	"github.com/hyperledger/fabric/discovery/support/config"
 	"github.com/hyperledger/fabric/discovery/support/mocks"
 	"github.com/hyperledger/fabric/protos/common"
+	"github.com/onsi/gomega/gexec"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -46,6 +55,83 @@ func blockWithConfigEnvelope() *common.Block {
 	}
 }
 
+func TestMSPIDMapping(t *testing.T) {
+	randString := func() string {
+		buff := make([]byte, 10)
+		rand.Read(buff)
+		return hex.EncodeToString(buff)
+	}
+
+	dir := filepath.Join(os.TempDir(), fmt.Sprintf("TestMSPIDMapping_%s", randString()))
+	os.Mkdir(dir, 0700)
+	defer os.RemoveAll(dir)
+
+	cryptogen, err := gexec.Build(filepath.Join("github.com", "hyperledger", "fabric", "common", "tools", "cryptogen"))
+	assert.NoError(t, err)
+	defer os.Remove(cryptogen)
+
+	idemixgen, err := gexec.Build(filepath.Join("github.com", "hyperledger", "fabric", "common", "tools", "idemixgen"))
+	assert.NoError(t, err)
+	defer os.Remove(idemixgen)
+
+	cryptoConfigDir := filepath.Join(dir, "crypto-config")
+	b, err := exec.Command(cryptogen, "generate", fmt.Sprintf("--output=%s", cryptoConfigDir)).CombinedOutput()
+	assert.NoError(t, err, string(b))
+
+	idemixConfigDir := filepath.Join(dir, "crypto-config", "idemix")
+	b, err = exec.Command(idemixgen, "ca-keygen", fmt.Sprintf("--output=%s", idemixConfigDir)).CombinedOutput()
+	assert.NoError(t, err, string(b))
+
+	profileConfig := genesisconfig.Load("TwoOrgsChannel", "../../../examples/e2e_cli/")
+	ordererConfig := genesisconfig.Load("TwoOrgsOrdererGenesis", "../../../examples/e2e_cli/")
+	profileConfig.Orderer = ordererConfig.Orderer
+
+	// Override the MSP directory with our randomly generated and populated path
+	for _, org := range ordererConfig.Orderer.Organizations {
+		org.MSPDir = filepath.Join(cryptoConfigDir, "ordererOrganizations", "example.com", "msp")
+		org.Name = randString()
+	}
+
+	// Randomize organization names
+	for _, org := range profileConfig.Application.Organizations {
+		org.Name = randString()
+		// Non bccsp-msp orgs don't have the crypto material produced by cryptogen,
+		// we need to use the idemix crypto folder instead.
+		if org.MSPType != "bccsp" {
+			org.MSPDir = filepath.Join(idemixConfigDir)
+			continue
+		}
+		org.MSPDir = filepath.Join(cryptoConfigDir, "peerOrganizations", "org1.example.com", "msp")
+	}
+
+	gen := encoder.New(profileConfig)
+	block := gen.GenesisBlockForChannel("mychannel")
+
+	fakeBlockGetter := &mocks.ConfigBlockGetter{}
+	fakeBlockGetter.GetCurrConfigBlockReturnsOnCall(0, block)
+
+	cs := config.NewDiscoverySupport(fakeBlockGetter)
+	res, err := cs.Config("mychannel")
+
+	actualKeys := make(map[string]struct{})
+	for key := range res.Orderers {
+		actualKeys[key] = struct{}{}
+	}
+
+	for key := range res.Msps {
+		actualKeys[key] = struct{}{}
+	}
+
+	// Note that Org3MSP is an idemix org, but it shouldn't be listed here
+	// because peers can't have idemix credentials
+	expected := map[string]struct{}{
+		"OrdererMSP": {},
+		"Org1MSP":    {},
+		"Org2MSP":    {},
+	}
+	assert.Equal(t, expected, actualKeys)
+}
+
 func TestSupportGreenPath(t *testing.T) {
 	fakeBlockGetter := &mocks.ConfigBlockGetter{}
 	fakeBlockGetter.GetCurrConfigBlockReturnsOnCall(0, nil)