Merge "[FAB-5329] Able to instantiate on a taken chaincode ID"

hyperledger · Jul 26, 2017 · 627b474 · 627b474
2 parents dda9ddc + 36e5140
commit 627b474
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 34 deletions.
diff --git a/core/container/api/core.go b/core/container/api/core.go
@@ -27,11 +27,11 @@ import (
 type BuildSpecFactory func() (io.Reader, error)
 type PrelaunchFunc func() error
 
-//abstract virtual image for supporting arbitrary virual machines
+//VM is an abstract virtual image for supporting arbitrary virual machines
 type VM interface {
 	Deploy(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, reader io.Reader) error
 	Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, builder BuildSpecFactory, preLaunchFunc PrelaunchFunc) error
 	Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error
 	Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error
-	GetVMName(ccID ccintf.CCID) (string, error)
+	GetVMName(ccID ccintf.CCID, format func(string) (string, error)) (string, error)
 }
diff --git a/core/container/controller.go b/core/container/controller.go
@@ -245,7 +245,7 @@ func VMCProcess(ctxt context.Context, vmtype string, req VMCReqIntf) (interface{
 	go func() {
 		defer close(c)
 
-		id, err := v.GetVMName(req.getCCID())
+		id, err := v.GetVMName(req.getCCID(), nil)
 		if err != nil {
 			resp = VMCResp{Err: err}
 			return

diff --git a/core/container/dockercontroller/dockercontroller.go b/core/container/dockercontroller/dockercontroller.go
@@ -18,6 +18,7 @@ package dockercontroller
 
 import (
 	"bytes"
+	"encoding/hex"
 	"fmt"
 	"io"
 	"strings"
@@ -29,6 +30,7 @@ import (
 
 	"github.com/fsouza/go-dockerclient"
 	"github.com/hyperledger/fabric/common/flogging"
+	"github.com/hyperledger/fabric/common/util"
 	container "github.com/hyperledger/fabric/core/container/api"
 	"github.com/hyperledger/fabric/core/container/ccintf"
 	cutil "github.com/hyperledger/fabric/core/container/util"
@@ -40,6 +42,8 @@ import (
 var (
 	dockerLogger = flogging.MustGetLogger("dockercontroller")
 	hostConfig   *docker.HostConfig
+	vmRegExp     = regexp.MustCompile("[^a-zA-Z0-9-_.]")
+	imageRegExp  = regexp.MustCompile("^[a-z0-9]+(([._-][a-z0-9]+)+)?$")
 )
 
 // getClient returns an instance that implements dockerClient interface
@@ -163,7 +167,7 @@ func (vm *DockerVM) createContainer(ctxt context.Context, client dockerClient,
 
 func (vm *DockerVM) deployImage(client dockerClient, ccid ccintf.CCID,
 	args []string, env []string, reader io.Reader) error {
-	id, err := vm.GetVMName(ccid)
+	id, err := vm.GetVMName(ccid, formatImageName)
 	if err != nil {
 		return err
 	}
@@ -208,7 +212,7 @@ func (vm *DockerVM) Deploy(ctxt context.Context, ccid ccintf.CCID,
 //Start starts a container using a previously created docker image
 func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
 	args []string, env []string, builder container.BuildSpecFactory, prelaunchFunc container.PrelaunchFunc) error {
-	imageID, err := vm.GetVMName(ccid)
+	imageID, err := vm.GetVMName(ccid, formatImageName)
 	if err != nil {
 		return err
 	}
@@ -219,7 +223,11 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
 		return err
 	}
 
-	containerID := strings.Replace(imageID, ":", "_", -1)
+	containerID, err := vm.GetVMName(ccid, nil)
+	if err != nil {
+		return err
+	}
+
 	attachStdout := viper.GetBool("vm.docker.attachStdout")
 
 	//stop,force remove if necessary
@@ -349,7 +357,7 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
 
 //Stop stops a running chaincode
 func (vm *DockerVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error {
-	id, err := vm.GetVMName(ccid)
+	id, err := vm.GetVMName(ccid, nil)
 	if err != nil {
 		return err
 	}
@@ -395,7 +403,7 @@ func (vm *DockerVM) stopInternal(ctxt context.Context, client dockerClient,
 
 //Destroy destroys an image
 func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error {
-	id, err := vm.GetVMName(ccid)
+	id, err := vm.GetVMName(ccid, formatImageName)
 	if err != nil {
 		return err
 	}
@@ -418,34 +426,48 @@ func (vm *DockerVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
 	return err
 }
 
-//GetVMName generates the docker image from peer information given the hashcode. This is needed to
-//keep image name's unique in a single host, multi-peer environment (such as a development environment)
-func (vm *DockerVM) GetVMName(ccid ccintf.CCID) (string, error) {
+// GetVMName generates the VM name from peer information. It accepts a format
+// function parameter to allow different formatting based on the desired use of
+// the name.
+func (vm *DockerVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
 	name := ccid.GetName()
 
-	//Convert to lowercase and replace any invalid characters with "-"
-	r := regexp.MustCompile("[^a-zA-Z0-9-_.]")
-
+	// replace any invalid characters with "-"
 	if ccid.NetworkID != "" && ccid.PeerID != "" {
-		name = strings.ToLower(
-			r.ReplaceAllString(
-				fmt.Sprintf("%s-%s-%s", ccid.NetworkID, ccid.PeerID, name), "-"))
+		name = vmRegExp.ReplaceAllString(
+			fmt.Sprintf("%s-%s-%s", ccid.NetworkID, ccid.PeerID, name), "-")
 	} else if ccid.NetworkID != "" {
-		name = strings.ToLower(
-			r.ReplaceAllString(
-				fmt.Sprintf("%s-%s", ccid.NetworkID, name), "-"))
+		name = vmRegExp.ReplaceAllString(
+			fmt.Sprintf("%s-%s", ccid.NetworkID, name), "-")
 	} else if ccid.PeerID != "" {
-		name = strings.ToLower(
-			r.ReplaceAllString(
-				fmt.Sprintf("%s-%s", ccid.PeerID, name), "-"))
+		name = vmRegExp.ReplaceAllString(
+			fmt.Sprintf("%s-%s", ccid.PeerID, name), "-")
 	}
 
-	// Check name complies with Docker's repository naming rules
-	r = regexp.MustCompile("^[a-z0-9]+(([._-][a-z0-9]+)+)?$")
+	if format != nil {
+		formattedName, err := format(name)
+		if err != nil {
+			return formattedName, err
+		}
+		// check to ensure format function didn't add any invalid characters
+		name = vmRegExp.ReplaceAllString(formattedName, "-")
+	}
+	return name, nil
+}
 
-	if !r.MatchString(name) {
+// formatImageName formats the docker image from peer information. This is
+// needed to keep image (repository) names unique in a single host, multi-peer
+// environment (such as a development environment). It computes the hash for the
+// supplied image name and then appends it to the lowercase image name to ensure
+// uniqueness.
+func formatImageName(name string) (string, error) {
+	imageName := strings.ToLower(fmt.Sprintf("%s-%s", name, hex.EncodeToString(util.ComputeSHA256([]byte(name)))))
+
+	// Check that name complies with Docker's repository naming rules
+	if !imageRegExp.MatchString(imageName) {
 		dockerLogger.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", name)
-		return name, fmt.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", name)
+		return imageName, fmt.Errorf("Error constructing Docker VM Name. '%s' breaks Docker's repository naming rules", imageName)
 	}
-	return name, nil
+
+	return imageName, nil
 }
diff --git a/core/container/dockercontroller/dockercontroller_test.go b/core/container/dockercontroller/dockercontroller_test.go
@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"compress/gzip"
 	"context"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"io"
@@ -226,6 +227,39 @@ func Test_Destroy(t *testing.T) {
 	testerr(t, err, true)
 }
 
+type testCase struct {
+	name           string
+	ccid           ccintf.CCID
+	formatFunc     func(string) (string, error)
+	expectedOutput string
+}
+
+func TestGetVMName(t *testing.T) {
+	dvm := DockerVM{}
+	var tc []testCase
+
+	tc = append(tc,
+		testCase{"mycc", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, NetworkID: "dev", PeerID: "peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-mycc-1.0"))))},
+		testCase{"mycc-nonetworkid", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, PeerID: "peer1", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "peer1-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("peer1-mycc-1.0"))))},
+		testCase{"myCC", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("Dev-Peer0-myCC-1.0"))))},
+		testCase{"mycc-nopeerid", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "mycc"}}, NetworkID: "dev", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-mycc-1.0"))))},
+		testCase{"myCC", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "dev", PeerID: "peer0", Version: "1.0"}, formatImageName, fmt.Sprintf("%s-%s", "dev-peer0-mycc-1.0", hex.EncodeToString(util.ComputeSHA256([]byte("dev-peer0-myCC-1.0"))))},
+		testCase{"myCC-preserveCase", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, nil, fmt.Sprintf("%s", "Dev-Peer0-myCC-1.0")},
+		testCase{"invalidCharsFormatFunction", ccintf.CCID{ChaincodeSpec: &pb.ChaincodeSpec{ChaincodeId: &pb.ChaincodeID{Name: "myCC"}}, NetworkID: "Dev", PeerID: "Peer0", Version: "1.0"}, formatInvalidChars, fmt.Sprintf("%s", "inv-lid-character--")})
+
+	for _, test := range tc {
+		name, err := dvm.GetVMName(test.ccid, test.formatFunc)
+		assert.Nil(t, err, "Expected nil error")
+		assert.Equal(t, test.expectedOutput, name, "Unexpected output for test case name: %s", test.name)
+	}
+
+}
+
+func TestFormatImageName_invalidChars(t *testing.T) {
+	_, err := formatImageName("invalid*chars")
+	assert.NotNil(t, err, "Expected error")
+}
+
 func getCodeChainBytesInMem() io.Reader {
 	startTime := time.Now()
 	inputbuf := bytes.NewBuffer(nil)
@@ -322,3 +356,7 @@ func (c *mockClient) RemoveContainer(opts docker.RemoveContainerOptions) error {
 	}
 	return nil
 }
+
+func formatInvalidChars(name string) (string, error) {
+	return "inv@lid*character$/", nil
+}
diff --git a/core/container/inproccontroller/inproccontroller.go b/core/container/inproccontroller/inproccontroller.go
@@ -93,7 +93,7 @@ func (vm *InprocVM) Deploy(ctxt context.Context, ccid ccintf.CCID, args []string
 		return fmt.Errorf(fmt.Sprintf("%s system chaincode does not contain chaincode instance", path))
 	}
 
-	instName, _ := vm.GetVMName(ccid)
+	instName, _ := vm.GetVMName(ccid, nil)
 	_, err := vm.getInstance(ctxt, ipctemplate, instName, args, env)
 
 	//FUTURE ... here is where we might check code for safety
@@ -163,7 +163,7 @@ func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string,
 		return fmt.Errorf(fmt.Sprintf("%s not registered", path))
 	}
 
-	instName, _ := vm.GetVMName(ccid)
+	instName, _ := vm.GetVMName(ccid, nil)
 
 	ipc, err := vm.getInstance(ctxt, ipctemplate, instName, args, env)
 
@@ -211,7 +211,7 @@ func (vm *InprocVM) Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, d
 		return fmt.Errorf("%s not registered", path)
 	}
 
-	instName, _ := vm.GetVMName(ccid)
+	instName, _ := vm.GetVMName(ccid, nil)
 
 	ipc := instRegistry[instName]
 
@@ -236,7 +236,17 @@ func (vm *InprocVM) Destroy(ctxt context.Context, ccid ccintf.CCID, force bool,
 	return nil
 }
 
-//GetVMName ignores the peer and network name as it just needs to be unique in process
-func (vm *InprocVM) GetVMName(ccid ccintf.CCID) (string, error) {
-	return ccid.GetName(), nil
+// GetVMName ignores the peer and network name as it just needs to be unique in
+// process.  It accepts a format function parameter to allow different
+// formatting based on the desired use of the name.
+func (vm *InprocVM) GetVMName(ccid ccintf.CCID, format func(string) (string, error)) (string, error) {
+	name := ccid.GetName()
+	if format != nil {
+		formattedName, err := format(name)
+		if err != nil {
+			return formattedName, err
+		}
+		name = formattedName
+	}
+	return name, nil
 }