Skip to content

Commit

Permalink
add linting rules for undeclared args in from
Browse files Browse the repository at this point in the history
Signed-off-by: Talon Bowler <[email protected]>
  • Loading branch information
daghack committed Apr 18, 2024
1 parent 71f99c5 commit 606efdd
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
25 changes: 22 additions & 3 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
info := argInfo{definition: metaArg, location: cmd.Location()}
if v, ok := opt.BuildArgs[metaArg.Key]; !ok {
if metaArg.Value != nil {
*metaArg.Value, info.deps, _ = shlex.ProcessWordWithMatches(*metaArg.Value, metaArgsToMap(optMetaArgs))
result, _ := shlex.ProcessWordWithMatches(*metaArg.Value, metaArgsToMap(optMetaArgs))
*metaArg.Value = result.Word
info.deps = result.Matched
}
} else {
metaArg.Value = &v
Expand All @@ -248,7 +250,15 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

// set base state for every image
for i, st := range stages {
name, used, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToMap(optMetaArgs))
result, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToMap(optMetaArgs))
name := result.Word
used := result.Matched
if len(result.Unmatched) > 0 {
for unmatched := range result.Unmatched {
msg := linter.RuleUndeclaredArgInFrom.Format(unmatched)
linter.RuleUndeclaredArgInFrom.Run(opt.Warn, st.Location, msg)
}
}
if err != nil {
return nil, parser.WithLocation(err, st.Location)
}
Expand All @@ -269,7 +279,16 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
}

if v := st.Platform; v != "" {
v, u, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
result, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
v := result.Word
u := result.Matched

if len(result.Unmatched) > 0 {
for unmatched := range result.Unmatched {
msg := linter.RuleUndeclaredArgInFrom.Format(unmatched)
linter.RuleUndeclaredArgInFrom.Run(opt.Warn, st.Location, msg)
}
}
if err != nil {
return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", v), st.Location)
}
Expand Down
32 changes: 32 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var lintTests = integration.TestFuncs(
testNoEmptyContinuations,
testSelfConsistentCommandCasing,
testFileConsistentCommandCasing,
testUndeclaredArg,
)

func testStageName(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -208,6 +209,37 @@ COPY Dockerfile /bar
checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})
}

func testUndeclaredArg(t *testing.T, sb integration.Sandbox) {
dockerfile := []byte(`
ARG base=scratch
FROM $base
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})

dockerfile = []byte(`
ARG platform=linux/amd64
FROM --platform=$platform scratch
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})

dockerfile = []byte(`
ARG tag=latest
FROM busybox:${tag}${version} AS b
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
{
RuleName: "UndeclaredArgInFrom",
Description: "FROM command must use declared ARGs",
Detail: "FROM argument 'version' is not declared",
Level: 1,
Line: 3,
},
})
}

func checkUnmarshal(t *testing.T, sb integration.Sandbox, c *client.Client, dir *integration.TmpDirWithName, expected []expectedLintWarning) {
destDir, err := os.MkdirTemp("", "buildkit")
require.NoError(t, err)
Expand Down
4 changes: 4 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ func init() {
}
}

func TestLint(t *testing.T) {
integration.Run(t, lintTests, opts...)
}

func TestIntegration(t *testing.T) {
integration.Run(t, allTests, opts...)

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 @@ -41,4 +41,11 @@ var (
return fmt.Sprintf("Command '%s' should match the case of the command majority (%s)", violatingCommand, correctCasing)
},
}
RuleUndeclaredArgInFrom = LinterRule[func(string) string]{
Name: "UndeclaredArgInFrom",
Description: "FROM command must use declared ARGs",
Format: func(baseArg string) string {
return fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
},
}
)
17 changes: 15 additions & 2 deletions frontend/dockerfile/shell/lex.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,22 @@ func (s *Lex) ProcessWordWithMap(word string, env map[string]string) (string, er
return word, err
}

type ProcessWordMatchesResult struct {
Word 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) (string, map[string]struct{}, error) {
func (s *Lex) ProcessWordWithMatches(word string, env map[string]string) (ProcessWordMatchesResult, error) {
sw := s.init(word, env)
word, _, err := sw.process(word)
return word, sw.matches, err
return ProcessWordMatchesResult{
Word: word,
Matched: sw.matches,
Unmatched: sw.nonmatches,
}, err
}

func (s *Lex) ProcessWordsWithMap(word string, env map[string]string) ([]string, error) {
Expand All @@ -79,6 +89,7 @@ func (s *Lex) init(word string, env map[string]string) *shellWord {
rawQuotes: s.RawQuotes,
rawEscapes: s.RawEscapes,
matches: make(map[string]struct{}),
nonmatches: make(map[string]struct{}),
}
sw.scanner.Init(strings.NewReader(word))
return sw
Expand All @@ -98,6 +109,7 @@ type shellWord struct {
skipUnsetEnv bool
skipProcessQuotes bool
matches map[string]struct{}
nonmatches map[string]struct{}
}

func (sw *shellWord) process(source string) (string, []string, error) {
Expand Down Expand Up @@ -511,6 +523,7 @@ func (sw *shellWord) getEnv(name string) (string, bool) {
return value, true
}
}
sw.nonmatches[name] = struct{}{}
return "", false
}

Expand Down
18 changes: 10 additions & 8 deletions frontend/dockerfile/shell/lex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func TestShellParser4Words(t *testing.T) {
}

func TestGetEnv(t *testing.T) {
sw := &shellWord{envs: nil, matches: make(map[string]struct{})}
sw := &shellWord{envs: nil, matches: make(map[string]struct{}), nonmatches: make(map[string]struct{})}

getEnv := func(name string) string {
value, _ := sw.getEnv(name)
Expand Down Expand Up @@ -480,7 +480,9 @@ func TestProcessWithMatches(t *testing.T) {
for _, c := range tc {
c := c
t.Run(c.input, func(t *testing.T) {
w, matches, err := shlex.ProcessWordWithMatches(c.input, c.envs)
result, err := shlex.ProcessWordWithMatches(c.input, c.envs)
w := result.Word
matches := result.Matched
if c.expectedErr {
require.Error(t, err)
return
Expand All @@ -505,30 +507,30 @@ func TestProcessWithMatchesPlatform(t *testing.T) {
version = "v1.2.3"
)

w, _, err := shlex.ProcessWordWithMatches(release, map[string]string{
results, err := shlex.ProcessWordWithMatches(release, map[string]string{
"VERSION": version,
"TARGETOS": "linux",
"TARGETARCH": "arm",
"TARGETVARIANT": "v7",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm-v7.tar.gz", w)
require.Equal(t, "something-v1.2.3.linux-arm-v7.tar.gz", results.Word)

w, _, err = shlex.ProcessWordWithMatches(release, map[string]string{
results, err = shlex.ProcessWordWithMatches(release, map[string]string{
"VERSION": version,
"TARGETOS": "linux",
"TARGETARCH": "arm64",
"TARGETVARIANT": "",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", w)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Word)

w, _, err = shlex.ProcessWordWithMatches(release, map[string]string{
results, err = shlex.ProcessWordWithMatches(release, map[string]string{
"VERSION": version,
"TARGETOS": "linux",
"TARGETARCH": "arm64",
// No "TARGETVARIANT": "",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", w)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Word)
}

0 comments on commit 606efdd

Please sign in to comment.