Merge pull request #2218 from tonistiigi/error-suggest

dockerfile: add suggestions to how to fix certain errors

frontend/dockerfile/builder/build.go

@@ -425,7 +425,7 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
 				})
 
 				if err != nil {
-					return errors.Wrapf(err, "failed to create LLB definition")
+					return err
 				}
 
 				def, err := st.Marshal(ctx)

frontend/dockerfile/dockerfile2llb/convert.go

@@ -25,6 +25,7 @@ import (
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/util/apicaps"
+	"github.com/moby/buildkit/util/suggest"
 	"github.com/moby/buildkit/util/system"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -216,6 +217,13 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 		allDispatchStates.states[0].stageName = ""
 	}
 
+	allStageNames := make([]string, 0, len(allDispatchStates.states))
+	for _, s := range allDispatchStates.states {
+		if s.stageName != "" {
+			allStageNames = append(allStageNames, s.stageName)
+		}
+	}
+
 	eg, ctx := errgroup.WithContext(ctx)
 	for i, d := range allDispatchStates.states {
 		reachable := isReachable(target, d)
@@ -233,6 +241,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 							err = parser.WithLocation(err, d.stage.Location)
 						}
 					}()
+					origName := d.stage.BaseName
 					ref, err := reference.ParseNormalizedNamed(d.stage.BaseName)
 					if err != nil {
 						return errors.Wrapf(err, "failed to parse stage name %q", d.stage.BaseName)
@@ -243,7 +252,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 					}
 					d.stage.BaseName = reference.TagNameOnly(ref).String()
 					var isScratch bool
-					if metaResolver != nil && reachable && !d.unregistered {
+					if metaResolver != nil && reachable {
 						prefix := "["
 						if opt.PrefixPlatform && platform != nil {
 							prefix += platforms.Format(*platform) + " "
@@ -255,7 +264,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
 							LogName:     fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName),
 						})
 						if err != nil {
-							return errors.Wrap(err, d.stage.BaseName)
+							return suggest.WrapError(errors.Wrap(err, origName), origName, append(allStageNames, commonImageNames()...), true)
 						}
 						var img Image
 						if err := json.Unmarshal(dt, &img); err != nil {
@@ -1535,3 +1544,14 @@ func summarizeHeredoc(doc string) string {
 	}
 	return summary
 }
+
+func commonImageNames() []string {
+	repos := []string{
+		"alpine", "busybox", "centos", "debian", "golang", "ubuntu", "fedora",
+	}
+	out := make([]string, 0, len(repos)*4)
+	for _, name := range repos {
+		out = append(out, name, "docker.io/library"+name, name+":latest", "docker.io/library"+name+":latest")
+	}
+	return out
+}

frontend/dockerfile/dockerfile_mount_test.go

@@ -24,6 +24,7 @@ var mountTests = []integration.Test{
 	testMountEnvAcrossStages,
 	testMountMetaArg,
 	testMountFromError,
+	testMountInvalid,
 }
 
 func init() {
@@ -88,6 +89,77 @@ RUN [ ! -f /mytmp/foo ]
 	require.NoError(t, err)
 }
 
+func testMountInvalid(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM scratch
+RUN --mont=target=/mytmp,type=tmpfs /bin/true
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unknown flag: mont")
+	require.Contains(t, err.Error(), "did you mean mount?")
+
+	dockerfile = []byte(`
+	FROM scratch
+	RUN --mount=typ=tmpfs /bin/true
+	`)
+
+	dir, err = tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unexpected key 'typ'")
+	require.Contains(t, err.Error(), "did you mean type?")
+
+	dockerfile = []byte(`
+	FROM scratch
+	RUN --mount=type=tmp /bin/true
+	`)
+
+	dir, err = tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unsupported mount type \"tmp\"")
+	require.Contains(t, err.Error(), "did you mean tmpfs?")
+}
+
 func testMountRWCache(t *testing.T, sb integration.Sandbox) {
 	f := getFrontend(t, sb)
 

frontend/dockerfile/dockerfile_test.go

@@ -110,6 +110,7 @@ var allTests = []integration.Test{
 	testDockerfileLowercase,
 	testExportCacheLoop,
 	testWildcardRenameCache,
+	testDockerfileInvalidInstruction,
 }
 
 var fileOpTests = []integration.Test{
@@ -2152,6 +2153,37 @@ func testDockerfileInvalidCommand(t *testing.T, sb integration.Sandbox) {
 	require.Contains(t, stdout.String(), "did not complete successfully")
 }
 
+func testDockerfileInvalidInstruction(t *testing.T, sb integration.Sandbox) {
+	skipDockerd(t, sb)
+	f := getFrontend(t, sb)
+	f.RequiresBuildctl(t)
+	dockerfile := []byte(`
+	FROM scratch
+	FNTRYPOINT ["/bin/sh", "-c", "echo invalidinstruction"]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "unknown instruction: FNTRYPOINT")
+	require.Contains(t, err.Error(), "did you mean ENTRYPOINT?")
+}
+
 func testDockerfileADDFromURL(t *testing.T, sb integration.Sandbox) {
 	skipDockerd(t, sb)
 	f := getFrontend(t, sb)

frontend/dockerfile/instructions/bflag.go

@@ -3,6 +3,8 @@ package instructions
 import (
 	"fmt"
 	"strings"
+
+	"github.com/moby/buildkit/util/suggest"
 )
 
 // FlagType is the type of the build flag
@@ -139,12 +141,12 @@ func (bf *BFlags) Parse() error {
 	// go ahead and bubble it back up here since we didn't do it
 	// earlier in the processing
 	if bf.Err != nil {
-		return fmt.Errorf("Error setting up flags: %s", bf.Err)
+		return fmt.Errorf("error setting up flags: %s", bf.Err)
 	}
 
 	for _, arg := range bf.Args {
 		if !strings.HasPrefix(arg, "--") {
-			return fmt.Errorf("Arg should start with -- : %s", arg)
+			return fmt.Errorf("arg should start with -- : %s", arg)
 		}
 
 		if arg == "--" {
@@ -162,11 +164,11 @@ func (bf *BFlags) Parse() error {
 
 		flag, ok := bf.flags[arg]
 		if !ok {
-			return fmt.Errorf("Unknown flag: %s", arg)
+			return suggest.WrapError(fmt.Errorf("unknown flag: %s", arg), arg, allFlags(bf.flags), true)
 		}
 
 		if _, ok = bf.used[arg]; ok && flag.flagType != stringsType {
-			return fmt.Errorf("Duplicate flag specified: %s", arg)
+			return fmt.Errorf("duplicate flag specified: %s", arg)
 		}
 
 		bf.used[arg] = flag
@@ -175,7 +177,7 @@ func (bf *BFlags) Parse() error {
 		case boolType:
 			// value == "" is only ok if no "=" was specified
 			if index >= 0 && value == "" {
-				return fmt.Errorf("Missing a value on flag: %s", arg)
+				return fmt.Errorf("missing a value on flag: %s", arg)
 			}
 
 			lower := strings.ToLower(value)
@@ -184,18 +186,18 @@ func (bf *BFlags) Parse() error {
 			} else if lower == "true" || lower == "false" {
 				flag.Value = lower
 			} else {
-				return fmt.Errorf("Expecting boolean value for flag %s, not: %s", arg, value)
+				return fmt.Errorf("expecting boolean value for flag %s, not: %s", arg, value)
 			}
 
 		case stringType:
 			if index < 0 {
-				return fmt.Errorf("Missing a value on flag: %s", arg)
+				return fmt.Errorf("missing a value on flag: %s", arg)
 			}
 			flag.Value = value
 
 		case stringsType:
 			if index < 0 {
-				return fmt.Errorf("Missing a value on flag: %s", arg)
+				return fmt.Errorf("missing a value on flag: %s", arg)
 			}
 			flag.StringValues = append(flag.StringValues, value)
 
@@ -207,3 +209,11 @@ func (bf *BFlags) Parse() error {
 
 	return nil
 }
+
+func allFlags(flags map[string]*Flag) []string {
+	var names []string
+	for name := range flags {
+		names = append(names, name)
+	}
+	return names
+}

frontend/dockerfile/instructions/commands_runmount.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/moby/buildkit/util/suggest"
 	"github.com/pkg/errors"
 )
 
@@ -57,6 +58,20 @@ func isValidMountType(s string) bool {
 	return ok
 }
 
+func allMountTypes() []string {
+	types := make([]string, 0, len(allowedMountTypes)+2)
+	for k := range allowedMountTypes {
+		types = append(types, k)
+	}
+	if isSecretMountsSupported() {
+		types = append(types, "secret")
+	}
+	if isSSHMountsSupported() {
+		types = append(types, "ssh")
+	}
+	return types
+}
+
 func runMountPreHook(cmd *RunCommand, req parseRequest) error {
 	st := &mountState{}
 	st.flag = req.flags.AddStrings("mount")
@@ -173,10 +188,11 @@ func parseMount(value string, expander SingleWordExpander) (*Mount, error) {
 			// if we don't have an expander, defer evaluation to later
 			continue
 		}
+
 		switch key {
 		case "type":
 			if !isValidMountType(strings.ToLower(value)) {
-				return nil, errors.Errorf("unsupported mount type %q", value)
+				return nil, suggest.WrapError(errors.Errorf("unsupported mount type %q", value), value, allMountTypes(), true)
 			}
 			m.Type = strings.ToLower(value)
 		case "from":
@@ -234,7 +250,10 @@ func parseMount(value string, expander SingleWordExpander) (*Mount, error) {
 			}
 			m.GID = &gid
 		default:
-			return nil, errors.Errorf("unexpected key '%s' in '%s'", key, field)
+			allKeys := []string{
+				"type", "from", "source", "target", "readonly", "id", "sharing", "required", "mode", "uid", "gid", "src", "dst", "ro", "rw", "readwrite",
+			}
+			return nil, suggest.WrapError(errors.Errorf("unexpected key '%s' in '%s'", key, field), key, allKeys, true)
 		}
 	}
 

frontend/dockerfile/instructions/parse.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/moby/buildkit/frontend/dockerfile/command"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
+	"github.com/moby/buildkit/util/suggest"
 	"github.com/pkg/errors"
 )
 
@@ -63,7 +64,7 @@ func ParseInstruction(node *parser.Node) (v interface{}, err error) {
 		err = parser.WithLocation(err, node.Location())
 	}()
 	req := newParseRequestFromNode(node)
-	switch node.Value {
+	switch strings.ToLower(node.Value) {
 	case command.Env:
 		return parseEnv(req)
 	case command.Maintainer:
@@ -101,8 +102,7 @@ func ParseInstruction(node *parser.Node) (v interface{}, err error) {
 	case command.Shell:
 		return parseShell(req)
 	}
-
-	return nil, &UnknownInstruction{Instruction: node.Value, Line: node.StartLine}
+	return nil, suggest.WrapError(&UnknownInstruction{Instruction: node.Value, Line: node.StartLine}, node.Value, allInstructionNames(), false)
 }
 
 // ParseCommand converts an AST to a typed Command
@@ -124,7 +124,7 @@ type UnknownInstruction struct {
 }
 
 func (e *UnknownInstruction) Error() string {
-	return fmt.Sprintf("unknown instruction: %s", strings.ToUpper(e.Instruction))
+	return fmt.Sprintf("unknown instruction: %s", e.Instruction)
 }
 
 type parseError struct {
@@ -133,7 +133,7 @@ type parseError struct {
 }
 
 func (e *parseError) Error() string {
-	return fmt.Sprintf("dockerfile parse error line %d: %v", e.node.StartLine, e.inner.Error())
+	return fmt.Sprintf("dockerfile parse error on line %d: %v", e.node.StartLine, e.inner.Error())
 }
 
 func (e *parseError) Unwrap() error {
@@ -755,3 +755,13 @@ func getComment(comments []string, name string) string {
 	}
 	return ""
 }
+
+func allInstructionNames() []string {
+	out := make([]string, len(command.Commands))
+	i := 0
+	for name := range command.Commands {
+		out[i] = strings.ToUpper(name)
+		i++
+	}
+	return out
+}