diff --git a/core/container/api/core.go b/core/container/api/core.go index 6a92202e223..352dca38e53 100644 --- 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 index 11e548acf08..ab3dc27b981 100644 --- 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 index 45e1508c2ec..9e7df67746c 100644 --- 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 index 91862b3095e..ddbd47cf0cb 100644 --- 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 index 848bf738284..ddd391b66b9 100644 --- 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 }