First pass at linter logic

Signed-off-by: Talon Bowler <talon.bowler@docker.com>

frontend/dockerfile/builder/build.go

@@ -15,6 +15,7 @@ import (
 	"github.com/moby/buildkit/frontend/dockerui"
 	"github.com/moby/buildkit/frontend/gateway/client"
 	gwpb "github.com/moby/buildkit/frontend/gateway/pb"
+	"github.com/moby/buildkit/frontend/subrequests/lint"
 	"github.com/moby/buildkit/frontend/subrequests/outline"
 	"github.com/moby/buildkit/frontend/subrequests/targets"
 	"github.com/moby/buildkit/solver/errdefs"
@@ -85,6 +86,9 @@ func Build(ctx context.Context, c client.Client) (_ *client.Result, err error) {
 		ListTargets: func(ctx context.Context) (*targets.List, error) {
 			return dockerfile2llb.ListTargets(ctx, src.Data)
 		},
+		Lint: func(ctx context.Context) (*lint.WarningList, error) {
+			return dockerfile2llb.DockerfileLint(ctx, src.Data, convertOpt)
+		},
 	}); err != nil {
 		return nil, err
 	} else if ok {

frontend/dockerfile/dockerfile2llb/convert.go

@@ -22,9 +22,11 @@ import (
 	"github.com/moby/buildkit/client/llb/imagemetaresolver"
 	"github.com/moby/buildkit/client/llb/sourceresolver"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
+	"github.com/moby/buildkit/frontend/dockerfile/linter"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
 	"github.com/moby/buildkit/frontend/dockerui"
+	"github.com/moby/buildkit/frontend/subrequests/lint"
 	"github.com/moby/buildkit/frontend/subrequests/outline"
 	"github.com/moby/buildkit/frontend/subrequests/targets"
 	"github.com/moby/buildkit/identity"
@@ -109,12 +111,51 @@ func Dockefile2Outline(ctx context.Context, dt []byte, opt ConvertOpt) (*outline
 	return &o, nil
 }
 
+func lintSourceInfo(srcMap *llb.SourceMap, srcRange *parser.Range) (filename string, source []string) {
+	if srcMap == nil {
+		return
+	}
+	filename = srcMap.Filename
+	if srcRange == nil {
+		return
+	}
+	lines := strings.Split(string(srcMap.Data), "\n")
+	start := srcRange.Start.Line
+	end := srcRange.End.Line
+	source = lines[start-1 : end]
+	return
+}
+
+func DockerfileLint(ctx context.Context, dt []byte, opt ConvertOpt) (*lint.WarningList, error) {
+	warnings := []lint.Warning{}
+	opt.Warn = func(short, url string, detail [][]byte, location *parser.Range) {
+		var source []string
+		var filename string
+		filename, source = lintSourceInfo(opt.SourceMap, location)
+		warnings = append(warnings, lint.Warning{
+			Short:    short,
+			Detail:   detail,
+			Location: location,
+			Filename: filename,
+			Source:   source,
+		})
+	}
+
+	_, err := toDispatchState(ctx, dt, opt)
+	if err != nil {
+		return nil, err
+	}
+	return &lint.WarningList{Warnings: warnings}, nil
+}
+
 func ListTargets(ctx context.Context, dt []byte) (*targets.List, error) {
 	dockerfile, err := parser.Parse(bytes.NewReader(dt))
 	if err != nil {
 		return nil, err
 	}
-	stages, _, err := instructions.Parse(dockerfile.AST)
+
+	noWarn := func(string, string, [][]byte, *parser.Range) {}
+	stages, _, err := instructions.Parse(dockerfile.AST, noWarn)
 	if err != nil {
 		return nil, err
 	}
@@ -180,13 +221,18 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 		return nil, err
 	}
 
+	for _, node := range dockerfile.AST.Children {
+		warnings := linter.ValidateNodeLintRules(node)
+		dockerfile.Warnings = append(dockerfile.Warnings, warnings...)
+	}
+
 	for _, w := range dockerfile.Warnings {
 		opt.Warn(w.Short, w.URL, w.Detail, w.Location)
 	}
 
 	proxyEnv := proxyEnvFromBuildArgs(opt.BuildArgs)
 
-	stages, metaArgs, err := instructions.Parse(dockerfile.AST)
+	stages, metaArgs, err := instructions.Parse(dockerfile.AST, opt.Warn)
 	if err != nil {
 		return nil, err
 	}

frontend/dockerfile/dockerfile2llb/convert_test.go

@@ -27,6 +27,57 @@ func toEnvMap(args []instructions.KeyValuePairOptional, env []string) map[string
 	return m
 }
 
+func TestDockerfileLinting(t *testing.T) {
+	t.Parallel()
+	df := `FROM scratch AS foo
+ENV FOO bar
+FROM foo
+COPY --from=foo f1 /
+	`
+	convertOpt := ConvertOpt{}
+	warningList, err := DockerfileLint(appcontext.Context(), []byte(df), convertOpt)
+	assert.NoError(t, err)
+	assert.Empty(t, warningList.Warnings)
+
+	df = `from scratch AS foo
+env FOO bar
+from foo
+copy --from=foo f1 /
+	`
+	warningList, err = DockerfileLint(appcontext.Context(), []byte(df), convertOpt)
+	assert.NoError(t, err)
+	assert.Empty(t, warningList.Warnings)
+
+	df = `FROM scratch AS FOO
+ENV FOO bar
+FROM foo
+COPY --from=FOO f1 /
+	`
+	convertOpt = ConvertOpt{}
+	warningList, err = DockerfileLint(appcontext.Context(), []byte(df), convertOpt)
+	assert.NoError(t, err)
+	assert.Len(t, warningList.Warnings, 2)
+
+	df = `FROM scratch AS Foo
+ENV FOO bar
+FROM foo
+COPY --from=Foo f1 /
+	`
+	convertOpt = ConvertOpt{}
+	warningList, err = DockerfileLint(appcontext.Context(), []byte(df), convertOpt)
+	assert.NoError(t, err)
+	assert.Len(t, warningList.Warnings, 2)
+
+	df = `FRom scratch AS foo
+env FOO bar
+from foo
+copy --from=foo f1 /
+	`
+	warningList, err = DockerfileLint(appcontext.Context(), []byte(df), convertOpt)
+	assert.NoError(t, err)
+	assert.Len(t, warningList.Warnings, 1)
+}
+
 func TestDockerfileParsing(t *testing.T) {
 	t.Parallel()
 	df := `FROM scratch

frontend/dockerfile/dockerfile_linter_test.go

@@ -0,0 +1,128 @@
+package dockerfile
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"sort"
+	"testing"
+
+	"github.com/containerd/continuity/fs/fstest"
+	"github.com/moby/buildkit/client"
+	"github.com/moby/buildkit/frontend/dockerfile/linter"
+	"github.com/moby/buildkit/frontend/dockerui"
+	gateway "github.com/moby/buildkit/frontend/gateway/client"
+	"github.com/moby/buildkit/frontend/subrequests/lint"
+	"github.com/moby/buildkit/util/testutil/integration"
+	"github.com/moby/buildkit/util/testutil/workers"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+	"github.com/tonistiigi/fsutil"
+)
+
+var lintTests = integration.TestFuncs(
+	testLintRules,
+)
+
+func testLintRules(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	workers.CheckFeatureCompat(t, sb, workers.FeatureFrontendOutline)
+	f := getFrontend(t, sb)
+	if _, ok := f.(*clientFrontend); !ok {
+		t.Skip("only test with client frontend")
+	}
+
+	dockerfile := []byte(`
+# 'First' should produce a lint warning (lowercase stage name)
+FROM scratch as First
+
+# 'Run' should produce a lint warning (inconsistent command case)
+Run echo "Hello"
+
+# No lint warning
+run echo "World"
+
+# No lint warning
+RUN touch /file
+
+# No lint warning
+FROM scratch as second
+
+# 'First' should produce a lint warning (lowercase flags)
+COPY --from=First /file /file
+
+# No lint warning
+FROM scratch as target
+
+# No lint warning
+COPY --from=second /file /file
+`)
+
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", []byte(dockerfile), 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	destDir, err := os.MkdirTemp("", "buildkit")
+	require.NoError(t, err)
+	defer os.RemoveAll(destDir)
+
+	called := false
+	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+		res, err := c.Solve(ctx, gateway.SolveRequest{
+			FrontendOpt: map[string]string{
+				"frontend.caps": "moby.buildkit.frontend.subrequests",
+				"requestid":     "frontend.lint",
+				"build-arg:BAR": "678",
+				"target":        "target",
+			},
+			Frontend: "dockerfile.v0",
+		})
+		require.NoError(t, err)
+
+		warnings, err := unmarshalLintWarnings(res)
+		require.NoError(t, err)
+		require.Len(t, warnings.Warnings, 3)
+
+		warningList := warnings.Warnings
+		sort.Slice(warningList, func(i, j int) bool {
+			return warningList[i].Location.Start.Line < warningList[j].Location.Start.Line
+		})
+
+		require.Equal(t, 3, warningList[0].Location.Start.Line)
+		require.Equal(t, linter.LinterRules[linter.RuleStageNameCasing].Short, warningList[0].Short)
+
+		require.Equal(t, 6, warningList[1].Location.Start.Line)
+		require.Equal(t, linter.LinterRules[linter.RuleCommandCasing].Short, warningList[1].Short)
+
+		require.Equal(t, 18, warningList[2].Location.Start.Line)
+		require.Equal(t, linter.LinterRules[linter.RuleFlagCasing].Short, warningList[2].Short)
+
+		called = true
+		return nil, nil
+	}
+	_, err = c.Build(sb.Context(), client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+		},
+	}, "", frontend, nil)
+	require.NoError(t, err)
+
+	require.True(t, called)
+}
+
+func unmarshalLintWarnings(res *gateway.Result) (*lint.WarningList, error) {
+	dt, ok := res.Metadata["result.json"]
+	if !ok {
+		return nil, errors.Errorf("missing frontend.lint")
+	}
+	var warnings lint.WarningList
+	if err := json.Unmarshal(dt, &warnings); err != nil {
+		return nil, err
+	}
+	return &warnings, nil
+}

frontend/dockerfile/dockerfile_test.go

@@ -246,6 +246,7 @@ func TestIntegration(t *testing.T) {
 		}))...)
 	integration.Run(t, heredocTests, opts...)
 	integration.Run(t, outlineTests, opts...)
+	integration.Run(t, lintTests, opts...)
 	integration.Run(t, targetsTests, opts...)
 
 	integration.Run(t, reproTests, append(opts,

frontend/dockerfile/instructions/parse.go

@@ -15,13 +15,16 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/moby/buildkit/frontend/dockerfile/command"
+	"github.com/moby/buildkit/frontend/dockerfile/linter"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
 	"github.com/moby/buildkit/util/suggest"
 	"github.com/pkg/errors"
 )
 
 var excludePatternsEnabled = false
 
+type warnCallback func(string, string, [][]byte, *parser.Range)
+
 type parseRequest struct {
 	command    string
 	args       []string
@@ -66,8 +69,13 @@ func newParseRequestFromNode(node *parser.Node) parseRequest {
 	}
 }
 
-// ParseInstruction converts an AST to a typed instruction (either a command or a build stage beginning when encountering a `FROM` statement)
 func ParseInstruction(node *parser.Node) (v interface{}, err error) {
+	noWarn := func(string, string, [][]byte, *parser.Range) {}
+	return ParseInstructionWithWarnings(node, noWarn)
+}
+
+// ParseInstruction converts an AST to a typed instruction (either a command or a build stage beginning when encountering a `FROM` statement)
+func ParseInstructionWithWarnings(node *parser.Node, warn warnCallback) (v interface{}, err error) {
 	defer func() {
 		err = parser.WithLocation(err, node.Location())
 	}()
@@ -84,6 +92,11 @@ func ParseInstruction(node *parser.Node) (v interface{}, err error) {
 	case command.Copy:
 		return parseCopy(req)
 	case command.From:
+		if !linter.ValidateStageNameCasing(req.args) {
+			location := linter.FirstNodeRange(node)
+			stageNameCasing := linter.LinterRules[linter.RuleStageNameCasing]
+			warn(stageNameCasing.Short, "", [][]byte{[]byte(stageNameCasing.Description)}, location)
+		}
 		return parseFrom(req)
 	case command.Onbuild:
 		return parseOnBuild(req)
@@ -150,9 +163,9 @@ func (e *parseError) Unwrap() error {
 
 // Parse a Dockerfile into a collection of buildable stages.
 // metaArgs is a collection of ARG instructions that occur before the first FROM.
-func Parse(ast *parser.Node) (stages []Stage, metaArgs []ArgCommand, err error) {
+func Parse(ast *parser.Node, warn warnCallback) (stages []Stage, metaArgs []ArgCommand, err error) {
 	for _, n := range ast.Children {
-		cmd, err := ParseInstruction(n)
+		cmd, err := ParseInstructionWithWarnings(n, warn)
 		if err != nil {
 			return nil, nil, &parseError{inner: err, node: n}
 		}

frontend/dockerfile/instructions/parse_test.go

@@ -161,7 +161,8 @@ ARG bar baz=123
 	ast, err := parser.Parse(bytes.NewBuffer([]byte(dt)))
 	require.NoError(t, err)
 
-	stages, meta, err := Parse(ast.AST)
+	emptyCallback := func(short, url string, detail [][]byte, location *parser.Range) {}
+	stages, meta, err := Parse(ast.AST, emptyCallback)
 	require.NoError(t, err)
 
 	require.Equal(t, "defines first stage", stages[0].Comment)