Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 32 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
case *instructions.EntrypointCommand:
err = dispatchEntrypoint(d, c, opt.lintWarn)
case *instructions.HealthCheckCommand:
err = dispatchHealthcheck(d, c)
err = dispatchHealthcheck(d, c, opt.lintWarn)
case *instructions.ExposeCommand:
err = dispatchExpose(d, c, opt.shlex)
case *instructions.UserCommand:
Expand Down Expand Up @@ -891,7 +891,6 @@ type dispatchState struct {
// paths marks the paths that are used by this dispatchState.
paths map[string]struct{}
ignoreCache bool
cmdSet bool
unregistered bool
stageName string
cmdIndex int
Expand All @@ -904,6 +903,10 @@ type dispatchState struct {
// workdirSet is set to true if a workdir has been set
// within the current dockerfile.
workdirSet bool

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible follow-up: workdirSet to healthcheck seem like fields with similar usage. I think they could be all combined into one struct that is included here.

entrypoint instructionTracker
cmd instructionTracker
healthcheck instructionTracker
}

func (ds *dispatchState) asyncLocalOpts() []llb.LocalOption {
Expand Down Expand Up @@ -1473,6 +1476,8 @@ func dispatchOnbuild(d *dispatchState, c *instructions.OnbuildCommand) error {
}

func dispatchCmd(d *dispatchState, c *instructions.CmdCommand, warn linter.LintWarnFunc) error {
validateUsedOnce(c, &d.cmd, warn)

var args []string = c.CmdLine
if c.PrependShell {
if len(d.image.Config.Shell) == 0 {
Expand All @@ -1483,11 +1488,12 @@ func dispatchCmd(d *dispatchState, c *instructions.CmdCommand, warn linter.LintW
}
d.image.Config.Cmd = args
d.image.Config.ArgsEscaped = true //nolint:staticcheck // ignore SA1019: field is deprecated in OCI Image spec, but used for backward-compatibility with Docker image spec.
d.cmdSet = true
return commitToHistory(&d.image, fmt.Sprintf("CMD %q", args), false, nil, d.epoch)
}

func dispatchEntrypoint(d *dispatchState, c *instructions.EntrypointCommand, warn linter.LintWarnFunc) error {
validateUsedOnce(c, &d.entrypoint, warn)

var args []string = c.CmdLine
if c.PrependShell {
if len(d.image.Config.Shell) == 0 {
Expand All @@ -1497,13 +1503,14 @@ func dispatchEntrypoint(d *dispatchState, c *instructions.EntrypointCommand, war
args = withShell(d.image, args)
}
d.image.Config.Entrypoint = args
if !d.cmdSet {
if !d.cmd.IsSet {
d.image.Config.Cmd = nil
}
return commitToHistory(&d.image, fmt.Sprintf("ENTRYPOINT %q", args), false, nil, d.epoch)
}

func dispatchHealthcheck(d *dispatchState, c *instructions.HealthCheckCommand) error {
func dispatchHealthcheck(d *dispatchState, c *instructions.HealthCheckCommand, warn linter.LintWarnFunc) error {
validateUsedOnce(c, &d.healthcheck, warn)
d.image.Config.Healthcheck = &dockerspec.HealthcheckConfig{
Test: c.Health.Test,
Interval: c.Health.Interval,
Expand Down Expand Up @@ -2151,3 +2158,23 @@ func reportUnusedFromArgs(unmatched map[string]struct{}, location []parser.Range
linter.RuleUndeclaredArgInFrom.Run(warn, location, msg)
}
}

type instructionTracker struct {
Loc []parser.Range
IsSet bool
}

func (v *instructionTracker) MarkUsed(loc []parser.Range) {
v.Loc = loc
v.IsSet = true
}

func validateUsedOnce(c instructions.Command, loc *instructionTracker, warn linter.LintWarnFunc) {
if loc.IsSet {
msg := linter.RuleMultipleInstructionsDisallowed.Format(c.Name())
// Report the location of the previous invocation because it is the one
// that will be ignored.
linter.RuleMultipleInstructionsDisallowed.Run(warn, loc.Loc, msg)
}
loc.MarkUsed(c.Location())
}
68 changes: 68 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package dockerfile
import (
"context"
"encoding/json"
"fmt"
"os"
"sort"
"testing"
Expand Down Expand Up @@ -34,6 +35,7 @@ var lintTests = integration.TestFuncs(
testUndeclaredArg,
testWorkdirRelativePath,
testUnmatchedVars,
testMultipleInstructionsDisallowed,
)

func testStageName(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -560,6 +562,72 @@ RUN echo $foo
})
}

func testMultipleInstructionsDisallowed(t *testing.T, sb integration.Sandbox) {
makeLintWarning := func(instructionName string, line int) expectedLintWarning {
return expectedLintWarning{
RuleName: "MultipleInstructionsDisallowed",
Description: "Multiple instructions of the same type should not be used in the same stage",
Detail: fmt.Sprintf("Multiple %s instructions should not be used in the same stage because only the last one will be used", instructionName),
Level: 1,
Line: line,
}
}

dockerfile := []byte(`
FROM scratch
ENTRYPOINT ["/myapp"]
ENTRYPOINT ["/myotherapp"]
CMD ["/myapp"]
CMD ["/myotherapp"]
HEALTHCHECK CMD ["/myapp"]
HEALTHCHECK CMD ["/myotherapp"]
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
makeLintWarning("ENTRYPOINT", 3),
makeLintWarning("CMD", 5),
makeLintWarning("HEALTHCHECK", 7),
},
})

// Still a linter warning even when broken up with another
// command. Entrypoint is only used by the resulting image.
dockerfile = []byte(`
FROM scratch
ENTRYPOINT ["/myapp"]
CMD ["/myapp"]
HEALTHCHECK CMD ["/myapp"]
COPY <<EOF /a.txt
Hello, World!
EOF
ENTRYPOINT ["/myotherapp"]
CMD ["/myotherapp"]
HEALTHCHECK CMD ["/myotherapp"]
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
makeLintWarning("ENTRYPOINT", 3),
makeLintWarning("CMD", 4),
makeLintWarning("HEALTHCHECK", 5),
},
})

dockerfile = []byte(`
FROM scratch AS a
ENTRYPOINT ["/myapp"]
CMD ["/myapp"]
HEALTHCHECK CMD ["/myapp"]

FROM a AS b
ENTRYPOINT ["/myotherapp"]
CMD ["/myotherapp"]
HEALTHCHECK CMD ["/myotherapp"]
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
}

func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
destDir, err := os.MkdirTemp("", "buildkit")
require.NoError(t, err)
Expand Down
7 changes: 7 additions & 0 deletions frontend/dockerfile/linter/ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,11 @@ var (
return fmt.Sprintf("Usage of undefined variable '$%s'", arg)
},
}
RuleMultipleInstructionsDisallowed = LinterRule[func(instructionName string) string]{
Name: "MultipleInstructionsDisallowed",
Description: "Multiple instructions of the same type should not be used in the same stage",
Format: func(instructionName string) string {
return fmt.Sprintf("Multiple %s instructions should not be used in the same stage because only the last one will be used", instructionName)
},
}
)