Parse multiple volumes via semicolon delimeter.

Fixes issue openshift#872
pahoughton · May 1, 2018 · be7f83a · be7f83a
1 parent 0c48fd2
commit be7f83a
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 7 deletions.
diff --git a/hack/test-stirunimage.sh b/hack/test-stirunimage.sh
@@ -140,6  140,10 @@ test_debug "s2i build with remote git repo"
 s2i build https://github.com/openshift/cakephp-ex docker.io/centos/php-70-centos7 test --loglevel=5 &> "${WORK_DIR}/s2i-git-proto.log"
 check_result $? "${WORK_DIR}/s2i-git-proto.log"
 
 test_debug "s2i build with runtime image"
 s2i build --ref=10.x --context-dir=helloworld https://github.com/wildfly/quickstart docker.io/openshift/wildfly-101-centos7 test-jee-app-thin --runtime-image=docker.io/openshift/wildfly-101-centos7 &> "${WORK_DIR}/s2i-runtime-image.log"
 check_result $? "${WORK_DIR}/s2i-runtime-image.log"
 
 test_debug "s2i build with --run==true option"
 if [[ "$OSTYPE" == "cygwin" ]]; then
   ( cd hack/windows/sigintwrap && make )

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -527,8  527,22 @@ func IsInvalidFilename(name string) bool {
 // When the destination is not specified, the source get copied into current
 // working directory in container.
 func (l *VolumeList) Set(value string) error {
 	volumes := strings.Split(value, ";")
 	newVols := make([]VolumeSpec, len(volumes))
 	for i, v := range volumes {
 		spec, err := l.parseSpec(v)
 		if err != nil {
 			return err
 		}
 		newVols[i] = *spec
 	}
 	*l = append(*l, newVols...)
 	return nil
 }
 
 func (l *VolumeList) parseSpec(value string) (*VolumeSpec, error) {
 	if len(value) == 0 {
-		return errors.New("invalid format, must be source:destination")
 		return nil, errors.New("invalid format, must be source:destination")
 	}
 	var mount []string
 	pos := strings.LastIndex(value, ":")
@@ -539,12  553,11 @@ func (l *VolumeList) Set(value string) error {
 	}
 	mount[0] = strings.Trim(mount[0], `"'`)
 	mount[1] = strings.Trim(mount[1], `"'`)
-	s := VolumeSpec{Source: filepath.Clean(mount[0]), Destination: filepath.ToSlash(filepath.Clean(mount[1]))}
 	s := &VolumeSpec{Source: filepath.Clean(mount[0]), Destination: filepath.ToSlash(filepath.Clean(mount[1]))}
 	if IsInvalidFilename(s.Source) || IsInvalidFilename(s.Destination) {
-		return fmt.Errorf("invalid characters in filename: %q", value)
 		return nil, fmt.Errorf("invalid characters in filename: %q", value)
 	}
-	*l = append(*l, s)
-	return nil
 	return s, nil
 }
 
 // String implements the String() function of pflags.Value interface.

diff --git a/pkg/api/types_test.go b/pkg/api/types_test.go
@@ -23,8  23,15 @@ func TestVolumeListSet(t *testing.T) {
 		{`C:\test:/bar`, VolumeList{{Source: `C:\test`, Destination: "/bar"}}},
 		{`C:\test:bar`, VolumeList{{Source: `C:\test`, Destination: "bar"}}},
 		{`"/te"st":"/foo"`, VolumeList{}},
-		{"/test/foo:/ss;ss", VolumeList{}},
-		{"/test;foo:/ssss", VolumeList{}},
 		{"/test/foo:/ss;ss", VolumeList{
 			{Source: "/test/foo", Destination: "/ss"},
 			{Source: "ss", Destination: "."},
 		}},
 		{"/test;foo:/ssss", VolumeList{
 			{Source: "/test", Destination: "."},
 			{Source: "foo", Destination: "/ssss"},
 		}},
 		{"/test;foo:b@!dF1nl3m!", VolumeList{}},
 	}
 	for _, test := range table {
 		if len(test.Expected) != 0 {

diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go
@@ -114,6  114,7 @@ func TestInjectionBuild(t *testing.T) {
 	integration(t).exerciseInjectionBuild(TagCleanBuild, FakeBuilderImage, []string{
 		tempdir   ":/tmp",
 		tempdir   ":",
 		tempdir   ":test;"   tempdir   ":test2",
 	})
 }