Merge pull request #7806 from erikh/fix_cmd_handling_in_parser

Fix cmd and entrypoint handling in parser

builder/dispatchers.go

@@ -166,13 +166,19 @@ func workdir(b *Builder, args []string, attributes map[string]bool) error {
 // RUN [ "echo", "hi" ] # echo hi
 //
 func run(b *Builder, args []string, attributes map[string]bool) error {
-	args = handleJsonArgs(args, attributes)
-
 	if b.image == "" {
 		return fmt.Errorf("Please provide a source image with `from` prior to run")
 	}
 
-	config, _, _, err := runconfig.Parse(append([]string{b.image}, args...), nil)
+	args = handleJsonArgs(args, attributes)
+
+	if len(args) == 1 {
+		args = append([]string{"/bin/sh", "-c"}, args[0])
+	}
+
+	args = append([]string{b.image}, args...)
+
+	config, _, _, err := runconfig.Parse(args, nil)
 	if err != nil {
 		return err
 	}
@@ -223,11 +229,18 @@ func run(b *Builder, args []string, attributes map[string]bool) error {
 func cmd(b *Builder, args []string, attributes map[string]bool) error {
 	b.Config.Cmd = handleJsonArgs(args, attributes)
 
-	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("CMD %v", cmd)); err != nil {
+	if !attributes["json"] && len(b.Config.Entrypoint) == 0 {
+		b.Config.Entrypoint = []string{"/bin/sh", "-c"}
+	}
+
+	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("CMD %v", b.Config.Cmd)); err != nil {
 		return err
 	}
 
-	b.cmdSet = true
+	if len(args) != 0 {
+		b.cmdSet = true
+	}
+
 	return nil
 }
 
@@ -242,8 +255,9 @@ func cmd(b *Builder, args []string, attributes map[string]bool) error {
 func entrypoint(b *Builder, args []string, attributes map[string]bool) error {
 	b.Config.Entrypoint = handleJsonArgs(args, attributes)
 
-	// if there is no cmd in current Dockerfile - cleanup cmd
-	if !b.cmdSet {
+	if len(b.Config.Entrypoint) == 0 && len(b.Config.Cmd) == 0 {
+		b.Config.Entrypoint = []string{"/bin/sh", "-c"}
+	} else if !b.cmdSet {
 		b.Config.Cmd = nil
 	}
 

builder/evaluator.go

@@ -144,7 +144,7 @@ func (b *Builder) Run(context io.Reader) (string, error) {
 	b.dockerfile = ast
 
 	// some initializations that would not have been supplied by the caller.
-	b.Config = &runconfig.Config{}
+	b.Config = &runconfig.Config{Entrypoint: []string{}, Cmd: []string{"/bin/sh", "-c"}}
 	b.TmpContainers = map[string]struct{}{}
 
 	for i, n := range b.dockerfile.Children {

builder/internals.go

@@ -89,6 +89,7 @@ func (b *Builder) commit(id string, autoCmd []string, comment string) error {
 	// Note: Actually copy the struct
 	autoConfig := *b.Config
 	autoConfig.Cmd = autoCmd
+
 	// Commit the container
 	image, err := b.Daemon.Commit(container, "", "", "", b.maintainer, true, &autoConfig)
 	if err != nil {

builder/parser/dumper/main.go

@@ -26,7 +26,7 @@ func main() {
 		if err != nil {
 			panic(err)
 		} else {
-			fmt.Print(ast.Dump())
+			fmt.Println(ast.Dump())
 		}
 	}
 }

builder/parser/parser.go

@@ -81,8 +81,10 @@ func parseLine(line string) (string, *Node, error) {
 		return "", nil, err
 	}
 
-	node.Next = sexp
-	node.Attributes = attrs
+	if sexp.Value != "" || sexp.Next != nil || sexp.Children != nil {
+		node.Next = sexp
+		node.Attributes = attrs
+	}
 
 	return "", node, nil
 }

builder/parser/parser_test.go

@@ -1,6 +1,7 @@
 package parser
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -69,7 +70,9 @@ func TestTestData(t *testing.T) {
 			t.Fatalf("Error reading %s's result file: %s", dir.Name(), err.Error())
 		}
 
-		if ast.Dump() != string(content) {
+		if ast.Dump()+"\n" != string(content) {
+			fmt.Fprintln(os.Stderr, ast.Dump())
+			fmt.Fprintln(os.Stderr, string(content))
 			t.Fatalf("%s: AST dump of dockerfile does not match result", dir.Name())
 		}
 

builder/parser/testfiles/brimstone-consuldock/result

@@ -2,4 +2,4 @@
 (maintainer "brimstone@the.narro.ws")
 (env "GOPATH" "/go")
 (entrypoint "/usr/local/bin/consuldock")
-(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/brimstone/consuldock && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH")
+(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/brimstone/consuldock && mv $GOPATH/bin/consuldock /usr/local/bin/consuldock && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH")

builder/parser/testfiles/brimstone-docker-consul/result

@@ -1,9 +1,9 @@
 (from "brimstone/ubuntu:14.04")
-(cmd "")
+(cmd)
 (entrypoint "/usr/bin/consul" "agent" "-server" "-data-dir=/consul" "-client=0.0.0.0" "-ui-dir=/webui")
 (expose "8500" "8600" "8400" "8301" "8302")
 (run "apt-get update && apt-get install -y unzip wget && apt-get clean && rm -rf /var/lib/apt/lists")
 (run "cd /tmp && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip -O web_ui.zip && unzip web_ui.zip && mv dist /webui && rm web_ui.zip")
 (run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends unzip wget && apt-get clean && rm -rf /var/lib/apt/lists && cd /tmp && wget https://dl.bintray.com/mitchellh/consul/0.3.1_web_ui.zip -O web_ui.zip && unzip web_ui.zip && mv dist /webui && rm web_ui.zip && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.*")
 (env "GOPATH" "/go")
-(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates build-essential && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/hashicorp/consul && mv $GOPATH/bin/consul /usr/bin/consul && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH")
+(run "apt-get update && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.clean && apt-get install -y --no-install-recommends git golang ca-certificates build-essential && apt-get clean && rm -rf /var/lib/apt/lists && go get -v github.com/hashicorp/consul && mv $GOPATH/bin/consul /usr/bin/consul && dpkg -l | awk '/^ii/ {print $2}' > /tmp/dpkg.dirty && apt-get remove --purge -y $(diff /tmp/dpkg.clean /tmp/dpkg.dirty | awk '/^>/ {print $2}') && rm /tmp/dpkg.* && rm -rf $GOPATH")

builder/parser/testfiles/cpuguy83-nagios/result

@@ -37,4 +37,4 @@
 (env "APACHE_LOG_DIR" "/var/log/apache2")
 (expose "80")
 (volume "/opt/nagios/var" "/opt/nagios/etc" "/opt/nagios/libexec" "/var/log/apache2" "/usr/share/snmp/mibs")
-(cmd "/usr/local/bin/start_nagios")
+(cmd "/usr/local/bin/start_nagios")

builder/parser/testfiles/docker/result

@@ -22,4 +22,4 @@
 (workdir "/go/src/github.com/docker/docker")
 (env "DOCKER_BUILDTAGS" "apparmor selinux")
 (entrypoint "hack/dind")
-(copy "." "/go/src/github.com/docker/docker")
+(copy "." "/go/src/github.com/docker/docker")

builder/parser/testfiles/escapes/result

@@ -2,4 +2,4 @@
 (maintainer "Erik \\\\Hollensbe <erik@hollensbe.org>\\\"")
 (run "apt-get \\update && apt-get \\\"install znc -y")
 (add "\\conf\\\\\"" "/.znc")
-(cmd "/usr\\\"/bin/znc" "-f" "-r")
+(cmd "/usr\\\"/bin/znc" "-f" "-r")

builder/parser/testfiles/influxdb/result

@@ -8,4 +8,4 @@
 (expose "8083")
 (expose "8086")
 (expose "8090")
-(expose "8099")
+(expose "8099")

builder/parser/testfiles/jeztah-invalid-json-json-inside-string-double/result

@@ -1 +1 @@
-(cmd "\"[\\\"echo\\\", \\\"Phew, I just managed to escaped those double quotes\\\"]\"")
+(cmd "\"[\\\"echo\\\", \\\"Phew, I just managed to escaped those double quotes\\\"]\"")

builder/parser/testfiles/jeztah-invalid-json-json-inside-string/result

@@ -1 +1 @@
-(cmd "'[\"echo\", \"Well, JSON in a string is JSON too?\"]'")
+(cmd "'[\"echo\", \"Well, JSON in a string is JSON too?\"]'")

builder/parser/testfiles/jeztah-invalid-json-single-quotes/result

@@ -1 +1 @@
-(cmd "['echo','single quotes are invalid JSON']")
+(cmd "['echo','single quotes are invalid JSON']")

builder/parser/testfiles/jeztah-invalid-json-unterminated-bracket/result

@@ -1 +1 @@
-(cmd "[\"echo\", \"Please, close the brackets when you're done\"")
+(cmd "[\"echo\", \"Please, close the brackets when you're done\"")

builder/parser/testfiles/jeztah-invalid-json-unterminated-string/result

@@ -1 +1 @@
-(cmd "[\"echo\", \"look ma, no quote!]")
+(cmd "[\"echo\", \"look ma, no quote!]")

builder/parser/testfiles/kartar-entrypoint-oddities/result

@@ -4,4 +4,4 @@
 (run "apt-get update")
 (run "apt-get -y install redis-server redis-tools")
 (expose "6379")
-(entrypoint "/usr/bin/redis-server")
+(entrypoint "/usr/bin/redis-server")

builder/parser/testfiles/lk4d4-the-edge-case-generator/result

@@ -26,4 +26,4 @@
 (volume "/test3")
 (workdir "/test")
 (add "." "/")
-(copy "." "copy")
+(copy "." "copy")

builder/parser/testfiles/mail/result

@@ -11,4 +11,4 @@
 (run "mkdir /Mail")
 (run "mkdir /.offlineimap")
 (run "echo \"export TERM=screen-256color\" >/.zshenv")
-(cmd "setsid cron; tmux -2")
+(cmd "setsid cron; tmux -2")

builder/parser/testfiles/mumble/result

@@ -1,4 +1,4 @@
 (from "ubuntu:14.04")
 (run "apt-get update && apt-get install libcap2-bin mumble-server -y")
 (add "./mumble-server.ini" "/etc/mumble-server.ini")
-(cmd "/usr/sbin/murmurd")
+(cmd "/usr/sbin/murmurd")

builder/parser/testfiles/nginx/result

@@ -8,4 +8,4 @@
 (run "mkdir /www")
 (cmd "/usr/sbin/nginx")
 (volume "/www")
-(expose "80")
+(expose "80")

builder/parser/testfiles/tf2/result

@@ -17,4 +17,4 @@
 (add "./configs" "/steam/tf2/tf/addons/sourcemod/configs")
 (run "mkdir -p /steam/tf2/tf/addons/sourcemod/translations/en")
 (run "cp /steam/tf2/tf/addons/sourcemod/translations/*.txt /steam/tf2/tf/addons/sourcemod/translations/en")
-(cmd "cd /steam/tf2 && ./srcds_run -port 27015 +ip 0.0.0.0 +map ctf_2fort -autoupdate -steam_dir /steam -steamcmd_script /steam/script +tf_bot_quota 12 +tf_bot_quota_mode fill")
+(cmd "cd /steam/tf2 && ./srcds_run -port 27015 +ip 0.0.0.0 +map ctf_2fort -autoupdate -steam_dir /steam -steamcmd_script /steam/script +tf_bot_quota 12 +tf_bot_quota_mode fill")

builder/parser/testfiles/weechat/result

@@ -3,4 +3,4 @@
 (add ".weechat" "/.weechat")
 (add ".tmux.conf" "/")
 (run "echo \"export TERM=screen-256color\" >/.zshenv")
-(cmd "zsh -c weechat")
+(cmd "zsh -c weechat")

builder/parser/testfiles/znc/result

@@ -2,4 +2,4 @@
 (maintainer "Erik Hollensbe <erik@hollensbe.org>")
 (run "apt-get update && apt-get install znc -y")
 (add "conf" "/.znc")
-(cmd "/usr/bin/znc" "-f" "-r")
+(cmd "/usr/bin/znc" "-f" "-r")

builder/support.go

@@ -28,10 +28,14 @@ func (b *Builder) replaceEnv(str string) string {
 }
 
 func handleJsonArgs(args []string, attributes map[string]bool) []string {
+	if len(args) == 0 {
+		return []string{}
+	}
+
 	if attributes != nil && attributes["json"] {
 		return args
 	}
 
 	// literal string command, not an exec array
-	return append([]string{"/bin/sh", "-c", strings.Join(args, " ")})
+	return []string{strings.Join(args, " ")}
 }

integration-cli/docker_cli_build_test.go

@@ -773,6 +773,29 @@ func TestBuildExpose(t *testing.T) {
 	logDone("build - expose")
 }
 
+func TestBuildEmptyEntrypoint(t *testing.T) {
+	name := "testbuildentrypoint"
+	defer deleteImages(name)
+	expected := "[]"
+
+	_, err := buildImage(name,
+		`FROM busybox
+        ENTRYPOINT []`,
+		true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	res, err := inspectField(name, "Config.Entrypoint")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res != expected {
+		t.Fatalf("Entrypoint %s, expected %s", res, expected)
+	}
+
+	logDone("build - empty entrypoint")
+}
+
 func TestBuildEntrypoint(t *testing.T) {
 	name := "testbuildentrypoint"
 	expected := "[/bin/echo]"
@@ -791,6 +814,7 @@ func TestBuildEntrypoint(t *testing.T) {
 	if res != expected {
 		t.Fatalf("Entrypoint %s, expected %s", res, expected)
 	}
+
 	logDone("build - entrypoint")
 }
 
@@ -1184,7 +1208,7 @@ func TestContextTarNoCompression(t *testing.T) {
 	testContextTar(t, archive.Uncompressed)
 }
 
-func TestNoContext(t *testing.T) {
+func TestBuildNoContext(t *testing.T) {
 	buildCmd := exec.Command(dockerBinary, "build", "-t", "nocontext", "-")
 	buildCmd.Stdin = strings.NewReader("FROM busybox\nCMD echo ok\n")
 
@@ -1899,3 +1923,24 @@ func TestBuildCleanupCmdOnEntrypoint(t *testing.T) {
 	}
 	logDone("build - cleanup cmd on ENTRYPOINT")
 }
+
+func TestBuildClearCmd(t *testing.T) {
+	name := "testbuildclearcmd"
+	defer deleteImages(name)
+	_, err := buildImage(name,
+		`From scratch
+   ENTRYPOINT ["/bin/bash"]
+   CMD []`,
+		true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	res, err := inspectFieldJSON(name, "Config.Cmd")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res != "[]" {
+		t.Fatalf("Cmd %s, expected %s", res, "[]")
+	}
+	logDone("build - clearcmd")
+}