Merge pull request #10561 from duglin/SupportNoArgCmds

Add support for no-arg commands in Dockerfile

builder/dispatchers.go

@@ -39,7 +39,7 @@ func nullDispatch(b *Builder, args []string, attributes map[string]bool, origina
 //
 func env(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) == 0 {
-		return fmt.Errorf("ENV is missing arguments")
+		return fmt.Errorf("ENV requires at least one argument")
 	}
 
 	if len(args)%2 != 0 {
@@ -78,7 +78,7 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
 // Sets the maintainer metadata.
 func maintainer(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) != 1 {
-		return fmt.Errorf("MAINTAINER requires only one argument")
+		return fmt.Errorf("MAINTAINER requires exactly one argument")
 	}
 
 	b.maintainer = args[0]
@@ -159,6 +159,10 @@ func from(b *Builder, args []string, attributes map[string]bool, original string
 // cases.
 //
 func onbuild(b *Builder, args []string, attributes map[string]bool, original string) error {
+	if len(args) == 0 {
+		return fmt.Errorf("ONBUILD requires at least one argument")
+	}
+
 	triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0]))
 	switch triggerInstruction {
 	case "ONBUILD":
@@ -327,6 +331,10 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original
 func expose(b *Builder, args []string, attributes map[string]bool, original string) error {
 	portsTab := args
 
+	if len(args) == 0 {
+		return fmt.Errorf("EXPOSE requires at least one argument")
+	}
+
 	if b.Config.ExposedPorts == nil {
 		b.Config.ExposedPorts = make(nat.PortSet)
 	}
@@ -373,7 +381,7 @@ func user(b *Builder, args []string, attributes map[string]bool, original string
 //
 func volume(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) == 0 {
-		return fmt.Errorf("Volume cannot be empty")
+		return fmt.Errorf("VOLUME requires at least one argument")
 	}
 
 	if b.Config.Volumes == nil {

builder/evaluator.go

@@ -54,10 +54,12 @@ var replaceEnvAllowed = map[string]struct{}{
 	"user":    {},
 }
 
-var evaluateTable map[string]func(*Builder, []string, map[string]bool, string) error
+// EvaluateTable is public so that we can get the list of Dockerfile
+// commands from within the test cases
+var EvaluateTable map[string]func(*Builder, []string, map[string]bool, string) error
 
 func init() {
-	evaluateTable = map[string]func(*Builder, []string, map[string]bool, string) error{
+	EvaluateTable = map[string]func(*Builder, []string, map[string]bool, string) error{
 		"env":        env,
 		"maintainer": maintainer,
 		"add":        add,
@@ -224,7 +226,7 @@ func (b *Builder) readDockerfile(origFile string) error {
 // Child[Node, Node, Node] where Child is from parser.Node.Children and each
 // node comes from parser.Node.Next. This forms a "line" with a statement and
 // arguments and we process them in this normalized form by hitting
-// evaluateTable with the leaf nodes of the command and the Builder object.
+// EvaluateTable with the leaf nodes of the command and the Builder object.
 //
 // ONBUILD is a special case; in this case the parser will emit:
 // Child[Node, Child[Node, Node...]] where the first node is the literal
@@ -240,6 +242,9 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 	msg := fmt.Sprintf("Step %d : %s", stepN, strings.ToUpper(cmd))
 
 	if cmd == "onbuild" {
+		if ast.Next == nil {
+			return fmt.Errorf("ONBUILD requires at least one argument")
+		}
 		ast = ast.Next.Children[0]
 		strs = append(strs, ast.Value)
 		msg += " " + ast.Value
@@ -277,7 +282,7 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 
 	// XXX yes, we skip any cmds that are not valid; the parser should have
 	// picked these out already.
-	if f, ok := evaluateTable[cmd]; ok {
+	if f, ok := EvaluateTable[cmd]; ok {
 		return f(b, strList, attrs, original)
 	}
 

builder/parser/line_parsers.go

@@ -30,6 +30,10 @@ func parseIgnore(rest string) (*Node, map[string]bool, error) {
 // ONBUILD RUN foo bar -> (onbuild (run foo bar))
 //
 func parseSubCommand(rest string) (*Node, map[string]bool, error) {
+	if rest == "" {
+		return nil, nil, nil
+	}
+
 	_, child, err := parseLine(rest)
 	if err != nil {
 		return nil, nil, err
@@ -133,7 +137,7 @@ func parseEnv(rest string) (*Node, map[string]bool, error) {
 	}
 
 	if len(words) == 0 {
-		return nil, nil, fmt.Errorf("ENV must have some arguments")
+		return nil, nil, fmt.Errorf("ENV requires at least one argument")
 	}
 
 	// Old format (ENV name value)
@@ -181,6 +185,10 @@ func parseEnv(rest string) (*Node, map[string]bool, error) {
 // parses a whitespace-delimited set of arguments. The result is effectively a
 // linked list of string arguments.
 func parseStringsWhitespaceDelimited(rest string) (*Node, map[string]bool, error) {
+	if rest == "" {
+		return nil, nil, nil
+	}
+
 	node := &Node{}
 	rootnode := node
 	prevnode := node
@@ -201,6 +209,9 @@ func parseStringsWhitespaceDelimited(rest string) (*Node, map[string]bool, error
 
 // parsestring just wraps the string in quotes and returns a working node.
 func parseString(rest string) (*Node, map[string]bool, error) {
+	if rest == "" {
+		return nil, nil, nil
+	}
 	n := &Node{}
 	n.Value = rest
 	return n, nil, nil
@@ -235,7 +246,9 @@ func parseJSON(rest string) (*Node, map[string]bool, error) {
 // so, passes to parseJSON; if not, quotes the result and returns a single
 // node.
 func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
-	rest = strings.TrimSpace(rest)
+	if rest == "" {
+		return nil, nil, nil
+	}
 
 	node, attrs, err := parseJSON(rest)
 
@@ -255,8 +268,6 @@ func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
 // so, passes to parseJSON; if not, attmpts to parse it as a whitespace
 // delimited string.
 func parseMaybeJSONToList(rest string) (*Node, map[string]bool, error) {
-	rest = strings.TrimSpace(rest)
-
 	node, attrs, err := parseJSON(rest)
 
 	if err == nil {

builder/parser/parser.go

@@ -3,7 +3,6 @@ package parser
 
 import (
 	"bufio"
-	"fmt"
 	"io"
 	"regexp"
 	"strings"
@@ -78,10 +77,6 @@ func parseLine(line string) (string, *Node, error) {
 		return "", nil, err
 	}
 
-	if len(args) == 0 {
-		return "", nil, fmt.Errorf("Instruction %q is empty; cannot continue", cmd)
-	}
-
 	node := &Node{}
 	node.Value = cmd
 

builder/parser/testfiles-negative/empty-instruction/Dockerfile

@@ -1,8 +0,0 @@
-FROM dockerfile/rabbitmq
-
-RUN
-  rabbitmq-plugins enable \
-   rabbitmq_shovel \
-   rabbitmq_shovel_management \
-   rabbitmq_federation \
-   rabbitmq_federation_management

builder/parser/testfiles-negative/html-page-yes-really-thanks-lk4d4/Dockerfile

@@ -1,2 +0,0 @@
-<html>
-</html>

builder/parser/utils.go

@@ -1,7 +1,6 @@
 package parser
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 )
@@ -50,17 +49,19 @@ func fullDispatch(cmd, args string) (*Node, map[string]bool, error) {
 // splitCommand takes a single line of text and parses out the cmd and args,
 // which are used for dispatching to more exact parsing functions.
 func splitCommand(line string) (string, string, error) {
+	var args string
+
 	// Make sure we get the same results irrespective of leading/trailing spaces
 	cmdline := TOKEN_WHITESPACE.Split(strings.TrimSpace(line), 2)
+	cmd := strings.ToLower(cmdline[0])
 
-	if len(cmdline) != 2 {
-		return "", "", fmt.Errorf("We do not understand this file. Please ensure it is a valid Dockerfile. Parser error at %q", line)
+	if len(cmdline) == 2 {
+		args = strings.TrimSpace(cmdline[1])
 	}
 
-	cmd := strings.ToLower(cmdline[0])
 	// the cmd should never have whitespace, but it's possible for the args to
 	// have trailing whitespace.
-	return cmd, strings.TrimSpace(cmdline[1]), nil
+	return cmd, args, nil
 }
 
 // covers comments and empty lines. Lines should be trimmed before passing to

integration-cli/docker_cli_build_test.go

@@ -19,6 +19,7 @@ import (
 	"text/template"
 	"time"
 
+	"github.com/docker/docker/builder"
 	"github.com/docker/docker/pkg/archive"
 )
 
@@ -49,14 +50,14 @@ func TestBuildEmptyWhitespace(t *testing.T) {
 		name,
 		`
     FROM busybox
-    RUN 
+    COPY
       quux \
       bar
     `,
 		true)
 
 	if err == nil {
-		t.Fatal("no error when dealing with a RUN statement with no content on the same line")
+		t.Fatal("no error when dealing with a COPY statement with no content on the same line")
 	}
 
 	logDone("build - statements with whitespace and no content should generate a parse error")
@@ -4726,9 +4727,9 @@ func TestBuildSpaces(t *testing.T) {
 
 	name := "testspaces"
 	defer deleteImages(name)
-	ctx, err := fakeContext("FROM busybox\nRUN\n",
+	ctx, err := fakeContext("FROM busybox\nCOPY\n",
 		map[string]string{
-			"Dockerfile": "FROM busybox\nRUN\n",
+			"Dockerfile": "FROM busybox\nCOPY\n",
 		})
 	if err != nil {
 		t.Fatal(err)
@@ -4739,7 +4740,7 @@ func TestBuildSpaces(t *testing.T) {
 		t.Fatal("Build 1 was supposed to fail, but didn't")
 	}
 
-	ctx.Add("Dockerfile", "FROM busybox\nRUN    ")
+	ctx.Add("Dockerfile", "FROM busybox\nCOPY    ")
 	if _, err2 = buildImageFromContext(name, ctx, false); err2 == nil {
 		t.Fatal("Build 2 was supposed to fail, but didn't")
 	}
@@ -4753,7 +4754,7 @@ func TestBuildSpaces(t *testing.T) {
 		t.Fatalf("Build 2's error wasn't the same as build 1's\n1:%s\n2:%s", err1, err2)
 	}
 
-	ctx.Add("Dockerfile", "FROM busybox\n   RUN")
+	ctx.Add("Dockerfile", "FROM busybox\n   COPY")
 	if _, err2 = buildImageFromContext(name, ctx, false); err2 == nil {
 		t.Fatal("Build 3 was supposed to fail, but didn't")
 	}
@@ -4767,7 +4768,7 @@ func TestBuildSpaces(t *testing.T) {
 		t.Fatalf("Build 3's error wasn't the same as build 1's\n1:%s\n3:%s", err1, err2)
 	}
 
-	ctx.Add("Dockerfile", "FROM busybox\n   RUN    ")
+	ctx.Add("Dockerfile", "FROM busybox\n   COPY    ")
 	if _, err2 = buildImageFromContext(name, ctx, false); err2 == nil {
 		t.Fatal("Build 4 was supposed to fail, but didn't")
 	}
@@ -4822,3 +4823,50 @@ func TestBuildVolumeFileExistsinContainer(t *testing.T) {
 
 	logDone("build - errors when volume is specified where a file exists")
 }
+
+func TestBuildMissingArgs(t *testing.T) {
+	// Test to make sure that all Dockerfile commands (except the ones listed
+	// in skipCmds) will generate an error if no args are provided.
+	// Note: INSERT is deprecated so we exclude it because of that.
+
+	skipCmds := map[string]struct{}{
+		"CMD":        {},
+		"RUN":        {},
+		"ENTRYPOINT": {},
+		"INSERT":     {},
+	}
+
+	defer deleteAllContainers()
+
+	for cmd := range builder.EvaluateTable {
+		var dockerfile string
+
+		cmd = strings.ToUpper(cmd)
+
+		if _, ok := skipCmds[cmd]; ok {
+			continue
+		}
+
+		if cmd == "FROM" {
+			dockerfile = cmd
+		} else {
+			// Add FROM to make sure we don't complain about it missing
+			dockerfile = "FROM busybox\n" + cmd
+		}
+
+		ctx, err := fakeContext(dockerfile, map[string]string{})
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer ctx.Close()
+		var out string
+		if out, err = buildImageFromContext("args", ctx, true); err == nil {
+			t.Fatalf("%s was supposed to fail. Out:%s", cmd, out)
+		}
+		if !strings.Contains(err.Error(), cmd+" requires") {
+			t.Fatalf("%s returned the wrong type of error:%s", cmd, err)
+		}
+	}
+
+	logDone("build - verify missing args")
+}