Skip to content

Commit

Permalink
Add undefined arg lint rule
Browse files Browse the repository at this point in the history
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
  • Loading branch information
daghack committed May 2, 2024
1 parent 51d85d7 commit 39e7ce6
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 26 deletions.
36 changes: 33 additions & 3 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
}
}

validateCommandArgs(allDispatchStates, shlex, opt.Warn)

var target *dispatchState
if opt.Target == "" {
target = allDispatchStates.lastTarget()
Expand Down Expand Up @@ -755,7 +757,8 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
if err != nil {
return "", err
}
return opt.shlex.ProcessWord(word, env)
newword, _, err := opt.shlex.ProcessWord(word, env)
return newword, err
})
if err != nil {
return err
Expand All @@ -770,7 +773,8 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {

lex := shell.NewLex('\\')
lex.SkipProcessQuotes = true
return lex.ProcessWord(word, env)
newword, _, err := lex.ProcessWord(word, env)
return newword, err
})
if err != nil {
return err
Expand Down Expand Up @@ -1835,7 +1839,7 @@ func uppercaseCmd(str string) string {
}

func processCmdEnv(shlex *shell.Lex, cmd string, env []string) string {
w, err := shlex.ProcessWord(cmd, env)
w, _, err := shlex.ProcessWord(cmd, env)
if err != nil {
return cmd
}
Expand Down Expand Up @@ -2034,6 +2038,32 @@ func validateStageNames(stages []instructions.Stage, warn linter.LintWarnFunc) {
}
}

func validateCommandArgs(allDispatchStates *dispatchStates, shlex *shell.Lex, warn linter.LintWarnFunc) {
nonEnvArgs := map[string]struct{}{}
for _, d := range allDispatchStates.states {
env, err := d.state.Env(context.TODO())
if err != nil {
continue
}
for _, cmd := range d.stage.Commands {
if argCmd, ok := cmd.(*instructions.ArgCommand); ok {
for _, arg := range argCmd.Args {
nonEnvArgs[arg.Key] = struct{}{}
}
}
if cmdstr, ok := cmd.(fmt.Stringer); ok {
_, unmatched, _ := shlex.ProcessWord(cmdstr.String(), env)
for arg := range unmatched {
if _, ok := nonEnvArgs[arg]; !ok {
msg := linter.RuleUndefinedArg.Format(arg)
linter.RuleUndefinedArg.Run(warn, cmd.Location(), msg)
}
}
}
}
}
}

func mergeLocations(locations ...[]parser.Range) []parser.Range {
allRanges := []parser.Range{}
for _, ranges := range locations {
Expand Down
29 changes: 29 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var lintTests = integration.TestFuncs(
testMaintainerDeprecated,
testWarningsBeforeError,
testUndeclaredArg,
testUndefinedArg,
)

func testStageName(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -418,6 +419,34 @@ COPY Dockerfile .
})
}

func testUndefinedArg(t *testing.T, sb integration.Sandbox) {
dockerfile := []byte(`
FROM scratch
ARG foo
RUN echo $foo
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

dockerfile = []byte(`
FROM busybox
COPY $ABSENT .
ARG foo=bar
RUN echo "$foo"
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
{
RuleName: "UndefinedArg",
Description: "ARGs should be defined before their use",
Detail: "Usage of undefined ARG 'ABSENT'",
Level: 1,
Line: 3,
},
},
})
}

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 @@ -70,4 +70,11 @@ var (
return fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
},
}
RuleUndefinedArg = LinterRule[func(string) string]{
Name: "UndefinedArg",
Description: "ARGs should be defined before their use",
Format: func(arg string) string {
return fmt.Sprintf("Usage of undefined ARG '%s'", arg)
},
}
)
42 changes: 26 additions & 16 deletions frontend/dockerfile/shell/lex.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ func NewLex(escapeToken rune) *Lex {
}

// ProcessWord will use the 'env' list of environment variables,
// and replace any env var references in 'word'.
func (s *Lex) ProcessWord(word string, env []string) (string, error) {
word, _, err := s.process(word, BuildEnvs(env))
return word, err
// and replace any env var references in 'word'. It will also
// return variables in word which were not found in the 'env' list,
// which is useful in later linting.
func (s *Lex) ProcessWord(word string, env []string) (string, map[string]struct{}, error) {
result, err := s.process(word, BuildEnvs(env))
return result.Result, result.Unmatched, err
}

// ProcessWords will use the 'env' list of environment variables,
Expand All @@ -46,38 +48,40 @@ func (s *Lex) ProcessWord(word string, env []string) (string, error) {
// Note, each one is trimmed to remove leading and trailing spaces (unless
// they are quoted", but ProcessWord retains spaces between words.
func (s *Lex) ProcessWords(word string, env []string) ([]string, error) {
_, words, err := s.process(word, BuildEnvs(env))
return words, err
result, err := s.process(word, BuildEnvs(env))
return result.Words, err
}

// ProcessWordWithMap will use the 'env' list of environment variables,
// and replace any env var references in 'word'.
func (s *Lex) ProcessWordWithMap(word string, env map[string]string) (string, error) {
word, _, err := s.process(word, env)
return word, err
result, err := s.process(word, env)
return result.Result, err
}

type ProcessWordMatchesResult struct {
type ProcessWordResult struct {
Result string
Words []string
Matched map[string]struct{}
Unmatched map[string]struct{}
}

// ProcessWordWithMatches will use the 'env' list of environment variables,
// replace any env var references in 'word' and return the env that were used.
func (s *Lex) ProcessWordWithMatches(word string, env map[string]string) (ProcessWordMatchesResult, error) {
func (s *Lex) ProcessWordWithMatches(word string, env map[string]string) (ProcessWordResult, error) {
sw := s.init(word, env)
word, _, err := sw.process(word)
return ProcessWordMatchesResult{
word, words, err := sw.process(word)
return ProcessWordResult{
Result: word,
Words: words,
Matched: sw.matches,
Unmatched: sw.nonmatches,
}, err
}

func (s *Lex) ProcessWordsWithMap(word string, env map[string]string) ([]string, error) {
_, words, err := s.process(word, env)
return words, err
result, err := s.process(word, env)
return result.Words, err
}

func (s *Lex) init(word string, env map[string]string) *shellWord {
Expand All @@ -95,9 +99,15 @@ func (s *Lex) init(word string, env map[string]string) *shellWord {
return sw
}

func (s *Lex) process(word string, env map[string]string) (string, []string, error) {
func (s *Lex) process(word string, env map[string]string) (*ProcessWordResult, error) {
sw := s.init(word, env)
return sw.process(word)
word, words, err := sw.process(word)
return &ProcessWordResult{
Result: word,
Words: words,
Matched: sw.matches,
Unmatched: sw.nonmatches,
}, err
}

type shellWord struct {
Expand Down
14 changes: 7 additions & 7 deletions frontend/dockerfile/shell/lex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,26 @@ func TestShellParserMandatoryEnvVars(t *testing.T) {
noUnset := "${VAR?message here$ARG}"

// disallow empty
newWord, err = shlex.ProcessWord(noEmpty, setEnvs)
newWord, _, err = shlex.ProcessWord(noEmpty, setEnvs)
require.NoError(t, err)
require.Equal(t, "plain", newWord)

_, err = shlex.ProcessWord(noEmpty, emptyEnvs)
_, _, err = shlex.ProcessWord(noEmpty, emptyEnvs)
require.ErrorContains(t, err, "message herex")

_, err = shlex.ProcessWord(noEmpty, unsetEnvs)
_, _, err = shlex.ProcessWord(noEmpty, unsetEnvs)
require.ErrorContains(t, err, "message herex")

// disallow unset
newWord, err = shlex.ProcessWord(noUnset, setEnvs)
newWord, _, err = shlex.ProcessWord(noUnset, setEnvs)
require.NoError(t, err)
require.Equal(t, "plain", newWord)

newWord, err = shlex.ProcessWord(noUnset, emptyEnvs)
newWord, _, err = shlex.ProcessWord(noUnset, emptyEnvs)
require.NoError(t, err)
require.Empty(t, newWord)

_, err = shlex.ProcessWord(noUnset, unsetEnvs)
_, _, err = shlex.ProcessWord(noUnset, unsetEnvs)
require.ErrorContains(t, err, "message herex")
}

Expand Down Expand Up @@ -123,7 +123,7 @@ func TestShellParser4EnvVars(t *testing.T) {

if ((platform == "W" || platform == "A") && runtime.GOOS == "windows") ||
((platform == "U" || platform == "A") && runtime.GOOS != "windows") {
newWord, err := shlex.ProcessWord(source, envs)
newWord, _, err := shlex.ProcessWord(source, envs)
if expected == "error" {
require.Errorf(t, err, "input: %q, result: %q", source, newWord)
} else {
Expand Down

0 comments on commit 39e7ce6

Please sign in to comment.