Merge pull request #4001 from kunalkushwaha/podman-import-fix

podman import syntax fix

5 files changed, 199 insertions, 24 deletions

diff --git a/cmd/podman/import.go b/cmd/podman/import.go
index d49792f27..027fa7299 100644
--- a/cmd/podman/import.go
+++ b/cmd/podman/import.go
@@ -39,7 +39,7 @@ func init() {
 	importCommand.SetHelpTemplate(HelpTemplate())
 	importCommand.SetUsageTemplate(UsageTemplate())
 	flags := importCommand.Flags()
-	flags.StringSliceVarP(&importCommand.Change, "change", "c", []string{}, "Apply the following possible instructions to the created image (default []): CMD | ENTRYPOINT | ENV | EXPOSE | LABEL | STOPSIGNAL | USER | VOLUME | WORKDIR")
+	flags.StringArrayVarP(&importCommand.Change, "change", "c", []string{}, "Apply the following possible instructions to the created image (default []): CMD | ENTRYPOINT | ENV | EXPOSE | LABEL | STOPSIGNAL | USER | VOLUME | WORKDIR")
 	flags.StringVarP(&importCommand.Message, "message", "m", "", "Set commit message for imported image")
 	flags.BoolVarP(&importCommand.Quiet, "quiet", "q", false, "Suppress output")
 
@@ -56,7 +56,6 @@ func importCmd(c *cliconfig.ImportValues) error {
 		source    string
 		reference string
 	)
-
 	args := c.InputArgs
 	switch len(args) {
 	case 0:
@@ -81,7 +80,7 @@ func importCmd(c *cliconfig.ImportValues) error {
 	if runtime.Remote {
 		quiet = false
 	}
-	iid, err := runtime.Import(getContext(), source, reference, c.StringSlice("change"), c.String("message"), quiet)
+	iid, err := runtime.Import(getContext(), source, reference, importCommand.Change, c.String("message"), quiet)
 	if err == nil {
 		fmt.Println(iid)
 	}
diff --git a/docs/podman-import.1.md b/docs/podman-import.1.md
index 5e57c1bcb..946b680dd 100644
--- a/docs/podman-import.1.md
+++ b/docs/podman-import.1.md
@@ -55,6 +55,26 @@ db65d991f3bbf7f31ed1064db9a6ced7652e3f8166c4736aa9133dadd3c7acb3
 ```
 
 ```
+$ podman import --change "ENTRYPOINT ["/bin/sh","-c","test-image"]"  --change LABEL=blue=image test-image.tar image-imported
+Getting image source signatures
+Copying blob e3b0c44298fc skipped: already exists
+Copying config 1105523502 done
+Writing manifest to image destination
+Storing signatures
+110552350206337183ceadc0bdd646dc356e06514c548b69a8917b4182414b
+```
+```
+$ podman import --change "CMD /bin/sh"  --change LABEL=blue=image test-image.tar image-imported
+Getting image source signatures
+Copying blob e3b0c44298fc skipped: already exists
+Copying config ae9a27e249 done
+Writing manifest to image destination
+Storing signatures
+ae9a27e249f801aff11a4ba54a81751ea9fbc9db45a6df3f1bfd63fc2437bb9c
+```
+
+
+```
 $ cat ctr.tar | podman -q import --message "importing the ctr.tar tarball" - image-imported
 db65d991f3bbf7f31ed1064db9a6ced7652e3f8166c4736aa9133dadd3c7acb3
 ```
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 583bf5d18..edcad1d1b 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -16,7 +17,7 @@ import (
 	"github.com/containers/libpod/pkg/rootless"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/idtools"
-	"github.com/opencontainers/image-spec/specs-go/v1"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
@@ -69,6 +70,50 @@ func StringInSlice(s string, sl []string) bool {
 	return false
 }
 
+// ParseChanges returns key, value(s) pair for given option.
+func ParseChanges(option string) (key string, vals []string, err error) {
+	// Supported format as below
+	// 1. key=value
+	// 2. key value
+	// 3. key ["value","value1"]
+	if strings.Contains(option, " ") {
+		// This handles 2 & 3 conditions.
+		var val string
+		tokens := strings.SplitAfterN(option, " ", 2)
+		if len(tokens) < 2 {
+			return "", []string{}, fmt.Errorf("invalid key value %s", option)
+		}
+		key = strings.Trim(tokens[0], " ") // Need to trim whitespace part of delimeter.
+		val = tokens[1]
+		if strings.Contains(tokens[1], "[") && strings.Contains(tokens[1], "]") {
+			//Trim '[',']' if exist.
+			val = strings.TrimLeft(strings.TrimRight(tokens[1], "]"), "[")
+		}
+		vals = strings.Split(val, ",")
+	} else if strings.Contains(option, "=") {
+		// handles condition 1.
+		tokens := strings.Split(option, "=")
+		key = tokens[0]
+		vals = tokens[1:]
+	} else {
+		// either ` ` or `=` must be provided after command
+		return "", []string{}, fmt.Errorf("invalid format %s", option)
+	}
+
+	if len(vals) == 0 {
+		return "", []string{}, errors.Errorf("no value given for instruction %q", key)
+	}
+
+	for _, v := range vals {
+		//each option must not have ' '., `[`` or `]` & empty strings
+		whitespaces := regexp.MustCompile(`[\[\s\]]`)
+		if whitespaces.MatchString(v) || len(v) == 0 {
+			return "", []string{}, fmt.Errorf("invalid value %s", v)
+		}
+	}
+	return key, vals, nil
+}
+
 // GetImageConfig converts the --change flag values in the format "CMD=/bin/bash USER=example"
 // to a type v1.ImageConfig
 func GetImageConfig(changes []string) (v1.ImageConfig, error) {
@@ -87,40 +132,42 @@ func GetImageConfig(changes []string) (v1.ImageConfig, error) {
 	exposedPorts := make(map[string]struct{})
 	volumes := make(map[string]struct{})
 	labels := make(map[string]string)
-
 	for _, ch := range changes {
-		pair := strings.Split(ch, "=")
-		if len(pair) == 1 {
-			return v1.ImageConfig{}, errors.Errorf("no value given for instruction %q", ch)
+		key, vals, err := ParseChanges(ch)
+		if err != nil {
+			return v1.ImageConfig{}, err
 		}
-		switch pair[0] {
+
+		switch key {
 		case "USER":
-			user = pair[1]
+			user = vals[0]
 		case "EXPOSE":
 			var st struct{}
-			exposedPorts[pair[1]] = st
+			exposedPorts[vals[0]] = st
 		case "ENV":
-			if len(pair) < 3 {
-				return v1.ImageConfig{}, errors.Errorf("no value given for environment variable %q", pair[1])
+			if len(vals) < 2 {
+				return v1.ImageConfig{}, errors.Errorf("no value given for environment variable %q", vals[0])
 			}
-			env = append(env, strings.Join(pair[1:], "="))
+			env = append(env, strings.Join(vals[0:], "="))
 		case "ENTRYPOINT":
-			entrypoint = append(entrypoint, pair[1])
+			// ENTRYPOINT and CMD can have array of strings
+			entrypoint = append(entrypoint, vals...)
 		case "CMD":
-			cmd = append(cmd, pair[1])
+			// ENTRYPOINT and CMD can have array of strings
+			cmd = append(cmd, vals...)
 		case "VOLUME":
 			var st struct{}
-			volumes[pair[1]] = st
+			volumes[vals[0]] = st
 		case "WORKDIR":
-			workingDir = pair[1]
+			workingDir = vals[0]
 		case "LABEL":
-			if len(pair) == 3 {
-				labels[pair[1]] = pair[2]
+			if len(vals) == 2 {
+				labels[vals[0]] = vals[1]
 			} else {
-				labels[pair[1]] = ""
+				labels[vals[0]] = ""
 			}
 		case "STOPSIGNAL":
-			stopSignal = pair[1]
+			stopSignal = vals[0]
 		}
 	}
 
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index f47c0b7ad..c938dc592 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -1,8 +1,9 @@
 package util
 
 import (
-	"github.com/stretchr/testify/assert"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 var (
@@ -17,3 +18,71 @@ func TestStringInSlice(t *testing.T) {
 	// string is not in empty slice
 	assert.False(t, StringInSlice("one", []string{}))
 }
+
+func TestParseChanges(t *testing.T) {
+	// CMD=/bin/sh
+	_, vals, err := ParseChanges("CMD=/bin/sh")
+	assert.EqualValues(t, []string{"/bin/sh"}, vals)
+	assert.NoError(t, err)
+
+	// CMD [/bin/sh]
+	_, vals, err = ParseChanges("CMD [/bin/sh]")
+	assert.EqualValues(t, []string{"/bin/sh"}, vals)
+	assert.NoError(t, err)
+
+	// CMD ["/bin/sh"]
+	_, vals, err = ParseChanges(`CMD ["/bin/sh"]`)
+	assert.EqualValues(t, []string{`"/bin/sh"`}, vals)
+	assert.NoError(t, err)
+
+	// CMD ["/bin/sh","-c","ls"]
+	_, vals, err = ParseChanges(`CMD ["/bin/sh","c","ls"]`)
+	assert.EqualValues(t, []string{`"/bin/sh"`, `"c"`, `"ls"`}, vals)
+	assert.NoError(t, err)
+
+	// CMD ["/bin/sh","arg-with,comma"]
+	_, vals, err = ParseChanges(`CMD ["/bin/sh","arg-with,comma"]`)
+	assert.EqualValues(t, []string{`"/bin/sh"`, `"arg-with`, `comma"`}, vals)
+	assert.NoError(t, err)
+
+	// CMD "/bin/sh"]
+	_, _, err = ParseChanges(`CMD "/bin/sh"]`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value "/bin/sh"]`, err.Error())
+
+	// CMD [bin/sh
+	_, _, err = ParseChanges(`CMD "/bin/sh"]`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value "/bin/sh"]`, err.Error())
+
+	// CMD ["/bin /sh"]
+	_, _, err = ParseChanges(`CMD ["/bin /sh"]`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value "/bin /sh"`, err.Error())
+
+	// CMD ["/bin/sh", "-c","ls"] whitespace between values
+	_, vals, err = ParseChanges(`CMD ["/bin/sh", "c","ls"]`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value  "c"`, err.Error())
+
+	// CMD?
+	_, _, err = ParseChanges(`CMD?`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid format CMD?`, err.Error())
+
+	// empty values for CMD
+	_, _, err = ParseChanges(`CMD `)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value `, err.Error())
+
+	// LABEL=blue=image
+	_, vals, err = ParseChanges(`LABEL=blue=image`)
+	assert.EqualValues(t, []string{"blue", "image"}, vals)
+	assert.NoError(t, err)
+
+	// LABEL = blue=image
+	_, vals, err = ParseChanges(`LABEL = blue=image`)
+	assert.Error(t, err)
+	assert.Equal(t, `invalid value = blue=image`, err.Error())
+
+}
diff --git a/test/e2e/import_test.go b/test/e2e/import_test.go
index 84a91a783..979440a50 100644
--- a/test/e2e/import_test.go
+++ b/test/e2e/import_test.go
@@ -88,7 +88,7 @@ var _ = Describe("Podman import", func() {
 		Expect(results.LineInOuputStartsWith("importing container test message")).To(BeTrue())
 	})
 
-	It("podman import with change flag", func() {
+	It("podman import with change flag CMD=<path>", func() {
 		outfile := filepath.Join(podmanTest.TempDir, "container.tar")
 		_, ec, cid := podmanTest.RunLsContainer("")
 		Expect(ec).To(Equal(0))
@@ -108,4 +108,44 @@ var _ = Describe("Podman import", func() {
 		Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/bash"))
 	})
 
+	It("podman import with change flag CMD <path>", func() {
+		outfile := filepath.Join(podmanTest.TempDir, "container.tar")
+		_, ec, cid := podmanTest.RunLsContainer("")
+		Expect(ec).To(Equal(0))
+
+		export := podmanTest.Podman([]string{"export", "-o", outfile, cid})
+		export.WaitWithDefaultTimeout()
+		Expect(export.ExitCode()).To(Equal(0))
+
+		importImage := podmanTest.Podman([]string{"import", "--change", "CMD /bin/sh", outfile, "imported-image"})
+		importImage.WaitWithDefaultTimeout()
+		Expect(importImage.ExitCode()).To(Equal(0))
+
+		results := podmanTest.Podman([]string{"inspect", "imported-image"})
+		results.WaitWithDefaultTimeout()
+		Expect(results.ExitCode()).To(Equal(0))
+		imageData := results.InspectImageJSON()
+		Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/sh"))
+	})
+
+	It("podman import with change flag CMD [\"path\",\"path'\"", func() {
+		outfile := filepath.Join(podmanTest.TempDir, "container.tar")
+		_, ec, cid := podmanTest.RunLsContainer("")
+		Expect(ec).To(Equal(0))
+
+		export := podmanTest.Podman([]string{"export", "-o", outfile, cid})
+		export.WaitWithDefaultTimeout()
+		Expect(export.ExitCode()).To(Equal(0))
+
+		importImage := podmanTest.Podman([]string{"import", "--change", "CMD [/bin/bash]", outfile, "imported-image"})
+		importImage.WaitWithDefaultTimeout()
+		Expect(importImage.ExitCode()).To(Equal(0))
+
+		results := podmanTest.Podman([]string{"inspect", "imported-image"})
+		results.WaitWithDefaultTimeout()
+		Expect(results.ExitCode()).To(Equal(0))
+		imageData := results.InspectImageJSON()
+		Expect(imageData[0].Config.Cmd[0]).To(Equal("/bin/bash"))
+	})
+
 })