Merge pull request #6334 from jsternberg/disable-checks

dockerfile: add syntax to skip a check

Author: tonistiigi

frontend/dockerfile/dockerfile2llb/convert.go

@@ -177,7 +177,7 @@ func ListTargets(ctx context.Context, dt []byte) (*targets.List, error) {
 	for i, s := range stages {
 		t := targets.Target{
 			Name:        s.Name,
-			Description: s.Comment,
+			Description: s.DocComment,
 			Default:     i == len(stages)-1,
 			Base:        s.BaseName,
 			Platform:    s.Platform,
@@ -296,6 +296,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 
 	// set base state for every image
 	for i, st := range stages {
+		lint := lint.WithMergedConfigFromComments(st.Comments)
+
 		nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, globalArgs)
 		argKeys := unusedFromArgsCheckKeys(globalArgs, outline.allArgs)
 		reportUnusedFromArgs(argKeys, nameMatch.Unmatched, st.Location, lint)
@@ -914,6 +916,8 @@ func (e *envsFromState) Keys() []string {
 }
 
 func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
+	opt.lint = opt.lint.WithMergedConfigFromComments(cmd.Comments())
+
 	d.cmdIsOnBuild = cmd.isOnBuild
 	var err error
 	// ARG command value could be ignored, so defer handling the expansion error

frontend/dockerfile/dockerfile2llb/outline.go

@@ -75,7 +75,7 @@ func (ds *dispatchState) args(visited map[string]struct{}) []outline.Arg {
 				args = append(args, outline.Arg{
 					Name:        a.definition.Key,
 					Value:       a.value,
-					Description: a.definition.Comment,
+					Description: a.definition.DocComment,
 					Location:    toSourceLocation(a.location),
 				})
 				visited[k] = struct{}{}
@@ -153,7 +153,7 @@ func (ds *dispatchState) Outline(dt []byte) outline.Outline {
 
 	out := outline.Outline{
 		Name:        ds.stage.Name,
-		Description: ds.stage.Comment,
+		Description: ds.stage.DocComment,
 		Sources:     [][]byte{dt},
 		Args:        args,
 		Secrets:     secrets,

frontend/dockerfile/dockerfile_lint_test.go

@@ -242,6 +242,14 @@ ENV apikey=bar sunflower=foo
 ENV git_key=
 ENV PUBLIC_KEY=
 ARG public_token
+# check=skip=SecretsUsedInArgOrEnv // allow secret in environment
+ENV password=bar
+# check=skip=SecretsUsedInArgOrEnv // allow secret in arg
+ARG password
+# check=skip=all // is local to only this instruction
+ENV alternate_password=bar
+# check=skip=all // is local to only this instruction
+ARG alternate_password
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{
 		Dockerfile: dockerfile,
@@ -843,6 +851,27 @@ MAINTAINER [email protected]
 	dockerfile = []byte(`
 FROM scratch
 LABEL org.opencontainers.image.authors="[email protected]"
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch
+# check=skip=JSONArgsRecommended
+CMD mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch
+# check=skip=JSONArgsRecommended
+ENTRYPOINT mycommand
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch
+# check=skip=MaintainerDeprecated
+MAINTAINER [email protected]
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }
@@ -1018,6 +1047,13 @@ WORKDIR /app
 
 FROM a AS b
 WORKDIR subdir/
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
+
+	dockerfile = []byte(`
+FROM scratch
+# check=skip=WorkdirRelativePath
+WORKDIR app/
 `)
 	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }
@@ -1252,6 +1288,15 @@ FROM a AS c
 			},
 		},
 	})
+
+	dockerfile = []byte(`
+FROM scratch
+# check=skip=LegacyKeyValueFormat
+ENV testkey value
+# check=skip=LegacyKeyValueFormat
+LABEL key value
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }
 
 func testRedundantTargetPlatform(t *testing.T, sb integration.Sandbox) {
@@ -1288,6 +1333,12 @@ FROM --platform=${TARGETPLATFORM} scratch
 			},
 		},
 	})
+
+	dockerfile = []byte(`
+# check=skip=RedundantTargetPlatform
+FROM --platform=$TARGETPLATFORM scratch
+`)
+	checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
 }
 
 func testInvalidDefaultArgInFrom(t *testing.T, sb integration.Sandbox) {

frontend/dockerfile/dockerfile_outline_test.go

@@ -54,6 +54,11 @@ RUN true
 
 FROM scratch AS third
 ARG ABC=a
+# check=skip=all
+ARG password
+# alternate_password will show up in the outline
+# check=skip=all
+ARG alternate_password
 
 # target defines build target
 FROM third AS target
@@ -81,6 +86,11 @@ RUN exit 0
 
 FROM nanoserver AS third
 ARG ABC=a
+# check=skip=all
+ARG password
+# alternate_password will show up in the outline
+# check=skip=all
+ARG alternate_password
 
 # target defines build target
 FROM third AS target
@@ -125,7 +135,7 @@ FROM second
 		require.Equal(t, 1, len(outline.Sources))
 		require.Equal(t, dockerfile, outline.Sources[0])
 
-		require.Equal(t, 5, len(outline.Args))
+		require.Equal(t, 7, len(outline.Args))
 
 		arg := outline.Args[0]
 		require.Equal(t, "inherited", arg.Name)
@@ -155,6 +165,18 @@ FROM second
 		require.Equal(t, "ABC", arg.Name)
 		require.Equal(t, "a", arg.Value)
 
+		arg = outline.Args[5]
+		require.Equal(t, "password", arg.Name)
+		require.Equal(t, "", arg.Value)
+		require.Equal(t, "", arg.Description)
+		require.Equal(t, int32(22), arg.Location.Ranges[0].Start.Line)
+
+		arg = outline.Args[6]
+		require.Equal(t, "alternate_password", arg.Name)
+		require.Equal(t, "", arg.Value)
+		require.Equal(t, "will show up in the outline", arg.Description)
+		require.Equal(t, int32(25), arg.Location.Ranges[0].Start.Line)
+
 		called = true
 		return nil, nil
 	}

frontend/dockerfile/instructions/commands.go

@@ -24,9 +24,9 @@ func (kvp *KeyValuePair) String() string {
 
 // KeyValuePairOptional is identical to KeyValuePair, but allows for optional values.
 type KeyValuePairOptional struct {
-	Key     string
-	Value   *string
-	Comment string
+	Key        string
+	Value      *string
+	DocComment string
 }
 
 func (kvpo *KeyValuePairOptional) String() string {
@@ -49,6 +49,7 @@ func (kvpo *KeyValuePairOptional) ValueString() string {
 type Command interface {
 	Name() string
 	Location() []parser.Range
+	Comments() []string
 }
 
 // KeyValuePairs is a slice of KeyValuePair
@@ -59,6 +60,7 @@ type withNameAndCode struct {
 	code     string
 	name     string
 	location []parser.Range
+	comments []string
 }
 
 func (c *withNameAndCode) String() string {
@@ -75,8 +77,17 @@ func (c *withNameAndCode) Location() []parser.Range {
 	return c.location
 }
 
+func (c *withNameAndCode) Comments() []string {
+	return c.comments
+}
+
 func newWithNameAndCode(req parseRequest) withNameAndCode {
-	return withNameAndCode{code: strings.TrimSpace(req.original), name: req.command, location: req.location}
+	return withNameAndCode{
+		code:     strings.TrimSpace(req.original),
+		name:     req.command,
+		location: req.location,
+		comments: req.comments,
+	}
 }
 
 // SingleWordExpander is a provider for variable expansion where a single word
@@ -511,10 +522,11 @@ type Stage struct {
 	BaseName string    // name of the base stage or source
 	Platform string    // platform of base source to use
 
-	Comment string // doc-comment directly above the stage
+	DocComment string // doc-comment directly above the stage
 
 	SourceCode string         // contents of the defining FROM command
 	Location   []parser.Range // location of the defining FROM command
+	Comments   []string
 }
 
 // AddCommand appends a command to the stage.

frontend/dockerfile/instructions/parse.go

@@ -70,6 +70,8 @@ func ParseInstruction(node *parser.Node) (v any, err error) {
 
 // ParseInstruction converts an AST to a typed instruction (either a command or a build stage beginning when encountering a `FROM` statement)
 func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v any, err error) {
+	lint = lint.WithMergedConfigFromComments(node.PrevComment)
+
 	defer func() {
 		if err != nil {
 			err = parser.WithLocation(err, node.Location())
@@ -410,7 +412,8 @@ func parseFrom(req parseRequest) (*Stage, error) {
 		Commands:   []Command{},
 		Platform:   flPlatform.Value,
 		Location:   req.location,
-		Comment:    getComment(req.comments, stageName),
+		Comments:   req.comments,
+		DocComment: getDocComment(req.comments, stageName),
 	}, nil
 }
 
@@ -761,7 +764,7 @@ func parseArg(req parseRequest) (*ArgCommand, error) {
 		} else {
 			kvpo.Key = arg
 		}
-		kvpo.Comment = getComment(req.comments, kvpo.Key)
+		kvpo.DocComment = getDocComment(req.comments, kvpo.Key)
 		pairs[i] = kvpo
 	}
 
@@ -817,7 +820,7 @@ func errTooManyArguments(command string) error {
 	return errors.Errorf("Bad input to %s, too many arguments", command)
 }
 
-func getComment(comments []string, name string) string {
+func getDocComment(comments []string, name string) string {
 	if name == "" {
 		return ""
 	}

frontend/dockerfile/instructions/parse_test.go

@@ -193,18 +193,18 @@ ARG bar baz=123
 	stages, meta, err := Parse(ast.AST, nil)
 	require.NoError(t, err)
 
-	require.Equal(t, "defines first stage", stages[0].Comment)
+	require.Equal(t, "defines first stage", stages[0].DocComment)
 	require.Equal(t, "foo", meta[0].Args[0].Key)
-	require.Equal(t, "sets foo", meta[0].Args[0].Comment)
+	require.Equal(t, "sets foo", meta[0].Args[0].DocComment)
 
 	st := stages[0]
 
 	require.Equal(t, "foo", st.Commands[0].(*ArgCommand).Args[0].Key)
-	require.Equal(t, "", st.Commands[0].(*ArgCommand).Args[0].Comment)
+	require.Equal(t, "", st.Commands[0].(*ArgCommand).Args[0].DocComment)
 	require.Equal(t, "bar", st.Commands[1].(*ArgCommand).Args[0].Key)
-	require.Equal(t, "defines bar", st.Commands[1].(*ArgCommand).Args[0].Comment)
+	require.Equal(t, "defines bar", st.Commands[1].(*ArgCommand).Args[0].DocComment)
 	require.Equal(t, "baz", st.Commands[1].(*ArgCommand).Args[1].Key)
-	require.Equal(t, "is something else", st.Commands[1].(*ArgCommand).Args[1].Comment)
+	require.Equal(t, "is something else", st.Commands[1].(*ArgCommand).Args[1].DocComment)
 }
 
 func TestErrorCases(t *testing.T) {