Merge pull request #26035 from sakeven/fmt_dockerfile_error_message

Fmt dockerfile error message

builder/dockerfile/dispatchers.go

@@ -68,7 +68,7 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
 		// value ==> args[j+1]
 
 		if len(args[j]) == 0 {
-			return fmt.Errorf("ENV names can not be blank")
+			return errBlankCommandNames("ENV")
 		}
 
 		newVar := args[j] + "=" + args[j+1] + ""
@@ -136,7 +136,7 @@ func label(b *Builder, args []string, attributes map[string]bool, original strin
 		// value ==> args[j+1]
 
 		if len(args[j]) == 0 {
-			return fmt.Errorf("LABEL names can not be blank")
+			return errBlankCommandNames("LABEL")
 		}
 
 		newVar := args[j] + "=" + args[j+1] + ""
@@ -455,7 +455,7 @@ func parseOptInterval(f *Flag) (time.Duration, error) {
 //
 func healthcheck(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) == 0 {
-		return fmt.Errorf("HEALTHCHECK requires an argument")
+		return errAtLeastOneArgument("HEALTHCHECK")
 	}
 	typ := strings.ToUpper(args[0])
 	args = args[1:]
@@ -529,11 +529,7 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, original
 		b.runConfig.Healthcheck = &healthcheck
 	}
 
-	if err := b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck)); err != nil {
-		return err
-	}
-
-	return nil
+	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("HEALTHCHECK %q", b.runConfig.Healthcheck))
 }
 
 // ENTRYPOINT /usr/sbin/nginx
@@ -654,7 +650,7 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri
 	for _, v := range args {
 		v = strings.TrimSpace(v)
 		if v == "" {
-			return fmt.Errorf("Volume specified can not be an empty string")
+			return fmt.Errorf("VOLUME specified can not be an empty string")
 		}
 		b.runConfig.Volumes[v] = struct{}{}
 	}
@@ -669,7 +665,7 @@ func volume(b *Builder, args []string, attributes map[string]bool, original stri
 // Set the signal that will be used to kill the container.
 func stopSignal(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) != 1 {
-		return fmt.Errorf("STOPSIGNAL requires exactly one argument")
+		return errExactlyOneArgument("STOPSIGNAL")
 	}
 
 	sig := args[0]
@@ -689,7 +685,7 @@ func stopSignal(b *Builder, args []string, attributes map[string]bool, original
 // Dockerfile author may optionally set a default value of this variable.
 func arg(b *Builder, args []string, attributes map[string]bool, original string) error {
 	if len(args) != 1 {
-		return fmt.Errorf("ARG requires exactly one argument definition")
+		return errExactlyOneArgument("ARG")
 	}
 
 	var (
@@ -707,7 +703,7 @@ func arg(b *Builder, args []string, attributes map[string]bool, original string)
 	if strings.Contains(arg, "=") {
 		parts := strings.SplitN(arg, "=", 2)
 		if len(parts[0]) == 0 {
-			return fmt.Errorf("ARG names can not be blank")
+			return errBlankCommandNames("ARG")
 		}
 
 		name = parts[0]
@@ -764,6 +760,10 @@ func errAtLeastTwoArguments(command string) error {
 	return fmt.Errorf("%s requires at least two arguments", command)
 }
 
+func errBlankCommandNames(command string) error {
+	return fmt.Errorf("%s names can not be blank", command)
+}
+
 func errTooManyArguments(command string) error {
 	return fmt.Errorf("Bad input to %s, too many arguments", command)
 }

builder/dockerfile/dispatchers_test.go

@@ -22,7 +22,8 @@ func TestCommandsExactlyOneArgument(t *testing.T) {
 		{"MAINTAINER", func(args []string) error { return maintainer(nil, args, nil, "") }},
 		{"FROM", func(args []string) error { return from(nil, args, nil, "") }},
 		{"WORKDIR", func(args []string) error { return workdir(nil, args, nil, "") }},
-		{"USER", func(args []string) error { return user(nil, args, nil, "") }}}
+		{"USER", func(args []string) error { return user(nil, args, nil, "") }},
+		{"STOPSIGNAL", func(args []string) error { return stopSignal(nil, args, nil, "") }}}
 
 	for _, command := range commands {
 		err := command.function([]string{})
@@ -31,9 +32,9 @@ func TestCommandsExactlyOneArgument(t *testing.T) {
 			t.Fatalf("Error should be present for %s command", command.name)
 		}
 
-		expectedError := fmt.Sprintf("%s requires exactly one argument", command.name)
+		expectedError := errExactlyOneArgument(command.name)
 
-		if err.Error() != expectedError {
+		if err.Error() != expectedError.Error() {
 			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
 		}
 	}
@@ -44,6 +45,7 @@ func TestCommandsAtLeastOneArgument(t *testing.T) {
 		{"ENV", func(args []string) error { return env(nil, args, nil, "") }},
 		{"LABEL", func(args []string) error { return label(nil, args, nil, "") }},
 		{"ONBUILD", func(args []string) error { return onbuild(nil, args, nil, "") }},
+		{"HEALTHCHECK", func(args []string) error { return healthcheck(nil, args, nil, "") }},
 		{"EXPOSE", func(args []string) error { return expose(nil, args, nil, "") }},
 		{"VOLUME", func(args []string) error { return volume(nil, args, nil, "") }}}
 
@@ -54,9 +56,9 @@ func TestCommandsAtLeastOneArgument(t *testing.T) {
 			t.Fatalf("Error should be present for %s command", command.name)
 		}
 
-		expectedError := fmt.Sprintf("%s requires at least one argument", command.name)
+		expectedError := errAtLeastOneArgument(command.name)
 
-		if err.Error() != expectedError {
+		if err.Error() != expectedError.Error() {
 			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
 		}
 	}
@@ -74,9 +76,9 @@ func TestCommandsAtLeastTwoArguments(t *testing.T) {
 			t.Fatalf("Error should be present for %s command", command.name)
 		}
 
-		expectedError := fmt.Sprintf("%s requires at least two arguments", command.name)
+		expectedError := errAtLeastTwoArguments(command.name)
 
-		if err.Error() != expectedError {
+		if err.Error() != expectedError.Error() {
 			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
 		}
 	}
@@ -94,9 +96,35 @@ func TestCommandsTooManyArguments(t *testing.T) {
 			t.Fatalf("Error should be present for %s command", command.name)
 		}
 
-		expectedError := fmt.Sprintf("Bad input to %s, too many arguments", command.name)
+		expectedError := errTooManyArguments(command.name)
 
-		if err.Error() != expectedError {
+		if err.Error() != expectedError.Error() {
+			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
+		}
+	}
+}
+
+func TestCommandseBlankNames(t *testing.T) {
+	bflags := &BFlags{}
+	config := &container.Config{}
+
+	b := &Builder{flags: bflags, runConfig: config, disableCommit: true}
+
+	commands := []commandWithFunction{
+		{"ENV", func(args []string) error { return env(b, args, nil, "") }},
+		{"LABEL", func(args []string) error { return label(b, args, nil, "") }},
+	}
+
+	for _, command := range commands {
+		err := command.function([]string{"", ""})
+
+		if err == nil {
+			t.Fatalf("Error should be present for %s command", command.name)
+		}
+
+		expectedError := errBlankCommandNames(command.name)
+
+		if err.Error() != expectedError.Error() {
 			t.Fatalf("Wrong error message for %s. Got: %s. Should be: %s", command.name, err.Error(), expectedError)
 		}
 	}

builder/dockerfile/evaluator_test.go

@@ -45,7 +45,7 @@ func initDispatchTestCases() []dispatchTestCase {
 		{
 			name:          "ARG two arguments",
 			dockerfile:    "ARG foo bar",
-			expectedError: "ARG requires exactly one argument definition",
+			expectedError: "ARG requires exactly one argument",
 			files:         nil,
 		},
 		{