Browse Source

Fix issue in `docker import -c` with quoted flags

This fix tries to address the issue in 26173 where `docker import -c`
with quoted flags returns an error.

The issue was that in `api/client/image/import.go` the flag
`--change/-c` was handled by `StringSliceVarP` which does not handle
the quote well.

The similiar issue was enountered for 23309 (`docker commit`).

This fix takes the same approach as 23309 where `StringSliceVarP`
was replaced with `VarP` and `opts.ListOpts`.

An integration test has been added to cover the changes.

This fix fixes 26173.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Yong Tang 8 years ago
parent
commit
a79a27412d
2 changed files with 28 additions and 3 deletions
  1. 5 3
      api/client/image/import.go
  2. 23 0
      integration-cli/docker_cli_import_test.go

+ 5 - 3
api/client/image/import.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	dockeropts "github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/urlutil"
 	"github.com/docker/engine-api/types"
@@ -17,7 +18,7 @@ import (
 type importOptions struct {
 	source    string
 	reference string
-	changes   []string
+	changes   dockeropts.ListOpts
 	message   string
 }
 
@@ -40,7 +41,8 @@ func NewImportCommand(dockerCli *client.DockerCli) *cobra.Command {
 
 	flags := cmd.Flags()
 
-	flags.StringSliceVarP(&opts.changes, "change", "c", []string{}, "Apply Dockerfile instruction to the created image")
+	opts.changes = dockeropts.NewListOpts(nil)
+	flags.VarP(&opts.changes, "change", "c", "Apply Dockerfile instruction to the created image")
 	flags.StringVarP(&opts.message, "message", "m", "", "Set commit message for imported image")
 
 	return cmd
@@ -71,7 +73,7 @@ func runImport(dockerCli *client.DockerCli, opts importOptions) error {
 
 	options := types.ImageImportOptions{
 		Message: opts.message,
-		Changes: opts.changes,
+		Changes: opts.changes.GetAll(),
 	}
 
 	clnt := dockerCli.Client()

+ 23 - 0
integration-cli/docker_cli_import_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/docker/docker/pkg/integration/checker"
+	icmd "github.com/docker/docker/pkg/integration/cmd"
 	"github.com/go-check/check"
 )
 
@@ -125,3 +126,25 @@ func (s *DockerSuite) TestImportFileNonExistentFile(c *check.C) {
 	_, _, err := dockerCmdWithError("import", "example.com/myImage.tar")
 	c.Assert(err, checker.NotNil, check.Commentf("import non-existing file must failed"))
 }
+
+func (s *DockerSuite) TestImportWithQuotedChanges(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+	dockerCmd(c, "run", "--name", "test-import", "busybox", "true")
+
+	temporaryFile, err := ioutil.TempFile("", "exportImportTest")
+	c.Assert(err, checker.IsNil, check.Commentf("failed to create temporary file"))
+	defer os.Remove(temporaryFile.Name())
+
+	result := icmd.RunCmd(icmd.Cmd{
+		Command: binaryWithArgs("export", "test-import"),
+		Stdout:  bufio.NewWriter(temporaryFile),
+	})
+	c.Assert(result, icmd.Matches, icmd.Success)
+
+	result = dockerCmdWithResult("import", "-c", `ENTRYPOINT ["/bin/sh", "-c"]`, temporaryFile.Name())
+	c.Assert(result, icmd.Matches, icmd.Success)
+	image := strings.TrimSpace(result.Stdout())
+
+	result = dockerCmdWithResult("run", "--rm", image, "true")
+	c.Assert(result, icmd.Matches, icmd.Expected{Out: icmd.None})
+}