clean up minor issues based on feedback

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

frontend/dockerfile/dockerfile2llb/convert.go

@@ -49,8 +49,6 @@ const (
 
 	sbomScanContext = "BUILDKIT_SBOM_SCAN_CONTEXT"
 	sbomScanStage   = "BUILDKIT_SBOM_SCAN_STAGE"
-
-	keyDockerfileLintArg = "build-arg:BUILDKIT_DOCKERFILE_LINT"
 )
 
 var nonEnvArgs = map[string]struct{}{
@@ -164,10 +162,10 @@ func ListTargets(ctx context.Context, dt []byte) (*targets.List, error) {
 	return l, nil
 }
 
-func parseLintOptions(checkStr string) (*linter.LinterConfig, error) {
+func parseLintOptions(checkStr string) (*linter.Config, error) {
 	checkStr = strings.TrimSpace(checkStr)
 	if checkStr == "" {
-		return &linter.LinterConfig{}, nil
+		return &linter.Config{}, nil
 	}
 
 	parts := strings.SplitN(checkStr, ";", 2)
@@ -191,13 +189,16 @@ func parseLintOptions(checkStr string) (*linter.LinterConfig, error) {
 				}
 			}
 		case "error":
-			v = strings.TrimSpace(v)
-			errorOnWarn = v == "true"
+			v, err := strconv.ParseBool(strings.TrimSpace(v))
+			if err != nil {
+				return nil, errors.Wrapf(err, "failed to parse check option %q", p)
+			}
+			errorOnWarn = v
 		default:
 			return nil, errors.Errorf("invalid check option %q", k)
 		}
 	}
-	return &linter.LinterConfig{
+	return &linter.Config{
 		SkipRules:     skipSet,
 		SkipAll:       skipAll,
 		ReturnAsError: errorOnWarn,
@@ -206,12 +207,11 @@ func parseLintOptions(checkStr string) (*linter.LinterConfig, error) {
 
 func newRuleLinter(dt []byte, opt *ConvertOpt) (*linter.Linter, error) {
 	var lintOptionStr string
-	if opt.Client != nil {
-		opts := opt.Client.BuildOpts().Opts
-		lintOptionStr = opts[keyDockerfileLintArg]
+	if opt.Client != nil && opt.Client.LinterConfig != "" {
+		lintOptionStr = opt.Client.LinterConfig
 	}
 	if lintOptionStr == "" {
-		if dfLintOption, _, _, ok := parser.DetectCheck(dt); ok {
+		if dfLintOption, _, _, ok := parser.ParseDirective("check", dt); ok {
 			lintOptionStr = dfLintOption
 		}
 	}
@@ -720,8 +720,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 	// there were warnings, and that our linter has been
 	// configured to return an error on warnings,
 	// so we appropriately return that error here.
-	err = lint.Error()
-	if err != nil {
+	if err := lint.Error(); err != nil {
 		return nil, err
 	}
 

frontend/dockerfile/dockerfile_lint_test.go

@@ -121,7 +121,7 @@ copy Dockerfile .
 		UnmarshalBuildErr: "lint violation found for rules: FromAsCasing",
 		BuildErrLocation:  2,
 		FrontendAttrs: map[string]string{
-			"build-arg:BUILDKIT_DOCKERFILE_LINT": "skip=FileConsistentCommandCasing;error=true",
+			"build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=FileConsistentCommandCasing;error=true",
 		},
 	})
 
@@ -132,7 +132,7 @@ copy Dockerfile .
 	checkLinterWarnings(t, sb, &lintTestParams{
 		Dockerfile: dockerfile,
 		FrontendAttrs: map[string]string{
-			"build-arg:BUILDKIT_DOCKERFILE_LINT": "skip=all",
+			"build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=all",
 		},
 	})
 }

frontend/dockerfile/linter/linter.go

@@ -8,7 +8,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-type LinterConfig struct {
+type Config struct {
 	Warn          LintWarnFunc
 	SkipRules     []string
 	SkipAll       bool
@@ -23,7 +23,7 @@ type Linter struct {
 	Warn          LintWarnFunc
 }
 
-func New(config *LinterConfig) *Linter {
+func New(config *Config) *Linter {
 	toret := &Linter{
 		SkippedRules: map[string]struct{}{},
 		CalledRules:  map[string]struct{}{},

frontend/dockerfile/linter/ruleset.go

@@ -4,8 +4,6 @@ import (
 	"fmt"
 )
 
-var ErrorOnLintWarn = false
-
 var (
 	RuleStageNameCasing = LinterRule[func(string) string]{
 		Name:        "StageNameCasing",

frontend/dockerfile/parser/directives.go

@@ -112,16 +112,10 @@ func (d *DirectiveParser) ParseAll(data []byte) ([]*Directive, error) {
 // This allows for a flexible range of input formats, and appropriate syntax
 // selection.
 func DetectSyntax(dt []byte) (string, string, []Range, bool) {
-	return detectDirective(keySyntax, dt)
+	return ParseDirective(keySyntax, dt)
 }
 
-// DetectCheck returns the check options of provided Dockerfile.
-// Follows all the same rules as the syntax directive.
-func DetectCheck(dt []byte) (string, string, []Range, bool) {
-	return detectDirective(keyCheck, dt)
-}
-
-func detectDirective(key string, dt []byte) (string, string, []Range, bool) {
+func ParseDirective(key string, dt []byte) (string, string, []Range, bool) {
 	dt, hadShebang, err := discardShebang(dt)
 	if err != nil {
 		return "", "", nil, false

frontend/dockerfile/parser/directives_test.go

@@ -104,13 +104,13 @@ RUN ls
 	require.False(t, ok)
 }
 
-func TestDetectLint(t *testing.T) {
+func TestParseDirective(t *testing.T) {
 	t.Parallel()
 
 	dt := `#check = skip=all // opts
 FROM busybox
 `
-	ref, cmdline, loc, ok := DetectCheck([]byte(dt))
+	ref, cmdline, loc, ok := ParseDirective("check", []byte(dt))
 	require.True(t, ok)
 	require.Equal(t, ref, "skip=all")
 	require.Equal(t, cmdline, "skip=all // opts")
@@ -121,7 +121,7 @@ FROM busybox
 # check = skip=all
 FROM busybox
 `
-	ref, _, loc, ok = DetectCheck([]byte(dt))
+	ref, _, loc, ok = ParseDirective("check", []byte(dt))
 	require.True(t, ok)
 	require.Equal(t, ref, "skip=all")
 	require.Equal(t, 2, loc[0].Start.Line)
@@ -131,40 +131,40 @@ FROM busybox
 
 # check = skip=all
 `
-	_, _, _, ok = DetectCheck([]byte(dt))
+	_, _, _, ok = ParseDirective("check", []byte(dt))
 	require.False(t, ok)
 
 	dt = `FROM busybox
 RUN ls
 `
-	ref, cmdline, _, ok = DetectCheck([]byte(dt))
+	ref, cmdline, _, ok = ParseDirective("check", []byte(dt))
 	require.False(t, ok)
 	require.Equal(t, ref, "")
 	require.Equal(t, cmdline, "")
 
 	dt = `//check=skip=all
 //key=value`
-	ref, _, _, ok = DetectCheck([]byte(dt))
+	ref, _, _, ok = ParseDirective("check", []byte(dt))
 	require.True(t, ok)
 	require.Equal(t, ref, "skip=all")
 
 	dt = `#!/bin/sh
 //check=skip=all`
-	ref, _, _, ok = DetectCheck([]byte(dt))
+	ref, _, _, ok = ParseDirective("check", []byte(dt))
 	require.True(t, ok)
 	require.Equal(t, ref, "skip=all")
 
 	dt = `{"check": "skip=all"}`
-	ref, _, _, ok = DetectCheck([]byte(dt))
+	ref, _, _, ok = ParseDirective("check", []byte(dt))
 	require.True(t, ok)
 	require.Equal(t, ref, "skip=all")
 
 	dt = `{"check": "foo"`
-	_, _, _, ok = DetectCheck([]byte(dt))
+	_, _, _, ok = ParseDirective("check", []byte(dt))
 	require.False(t, ok)
 
 	dt = `{"check": "foo"}
 # syntax=bar`
-	_, _, _, ok = DetectCheck([]byte(dt))
+	_, _, _, ok = ParseDirective("check", []byte(dt))
 	require.False(t, ok)
 }

frontend/dockerui/config.go

@@ -47,6 +47,7 @@ const (
 	keyCacheNSArg           = "build-arg:BUILDKIT_CACHE_MOUNT_NS"
 	keyMultiPlatformArg     = "build-arg:BUILDKIT_MULTI_PLATFORM"
 	keyHostnameArg          = "build-arg:BUILDKIT_SANDBOX_HOSTNAME"
+	keyDockerfileLintArg    = "build-arg:BUILDKIT_DOCKERFILE_CHECK"
 	keyContextKeepGitDirArg = "build-arg:BUILDKIT_CONTEXT_KEEP_GIT_DIR"
 	keySourceDateEpoch      = "build-arg:SOURCE_DATE_EPOCH"
 )
@@ -64,6 +65,7 @@ type Config struct {
 	ShmSize          int64
 	Target           string
 	Ulimits          []pb.Ulimit
+	LinterConfig     string
 
 	CacheImports           []client.CacheOptionsEntry
 	TargetPlatforms        []ocispecs.Platform // nil means default
@@ -277,6 +279,10 @@ func (bc *Client) init() error {
 		opts[keyHostname] = v
 	}
 	bc.Hostname = opts[keyHostname]
+
+	if v, ok := opts[keyDockerfileLintArg]; ok {
+		bc.LinterConfig = v
+	}
 	return nil
 }