Skip to content

Commit

Permalink
update lint tests to accept a struct instead of an increasing number …
Browse files Browse the repository at this point in the history 
…of parameters

Signed-off-by: Talon Bowler <[email protected]>
  • Loading branch information
@daghack
daghack committed Apr 30, 2024
1 parent 763ca51 commit 52948d4
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 45 deletions.
49 changes: 23 additions & 26 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
if v, ok := opt.BuildArgs[metaArg.Key]; !ok {
if metaArg.Value != nil {
result, _ := shlex.ProcessWordWithMatches(*metaArg.Value, metaArgsToMap(optMetaArgs))
*metaArg.Value = result.Word
*metaArg.Value = result.Result
info.deps = result.Matched
}
} else {
Expand All @@ -260,22 +260,17 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

// set base state for every image
for i, st := range stages {
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)
}
}
nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToMap(optMetaArgs))
reportUnusedFromArgs(nameMatch.Unmatched, st.Location, opt.Warn)
used := nameMatch.Matched

if err != nil {
return nil, parser.WithLocation(err, st.Location)
}
if name == "" {
if nameMatch.Result == "" {
return nil, parser.WithLocation(errors.Errorf("base name (%s) should not be blank", st.BaseName), st.Location)
}
st.BaseName = name
st.BaseName = nameMatch.Result

ds := &dispatchState{
stage: st,
Expand All @@ -289,27 +284,22 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
}

if v := st.Platform; v != "" {
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)
}
}
platMatch, err := shlex.ProcessWordWithMatches(v, metaArgsToMap(optMetaArgs))
reportUnusedFromArgs(platMatch.Unmatched, st.Location, opt.Warn)

if err != nil {
return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", v), st.Location)
return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", platMatch.Result), st.Location)
}

p, err := platforms.Parse(v)
p, err := platforms.Parse(platMatch.Result)
if err != nil {
return nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location)
return nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", platMatch.Result), st.Location)
}
for k := range u {

for k := range platMatch.Matched {
used[k] = struct{}{}
}

ds.platform = &p
}

Expand Down Expand Up @@ -2093,3 +2083,10 @@ func toPBLocation(sourceIndex int, location []parser.Range) pb.Location {
Ranges: loc,
}
}

func reportUnusedFromArgs(unmatched map[string]struct{}, location []parser.Range, warn linter.LintWarnFunc) {
for arg := range unmatched {
msg := linter.RuleUndeclaredArgInFrom.Format(arg)
linter.RuleUndeclaredArgInFrom.Run(warn, location, msg)
}
}
39 changes: 36 additions & 3 deletions frontend/dockerfile/dockerfile_lint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,13 @@ FROM ${BAR} AS base
Line: 3,
Level: 1,
},
{
RuleName: "UndeclaredArgInFrom",
Description: "FROM command must use declared ARGs",
Detail: "FROM argument 'BAR' is not declared",
Level: 1,
Line: 4,
},
},
StreamBuildErr: "failed to solve: base name (${BAR}) should not be blank",
UnmarshalBuildErr: "base name (${BAR}) should not be blank",
Expand All @@ -360,12 +367,38 @@ COPY Dockerfile .
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

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

dockerfile = []byte(`
FROM --platform=$BUILDPLATFORM scratch
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

dockerfile = []byte(`
FROM --platform=$BULIDPLATFORM scratch
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
{
RuleName: "UndeclaredArgInFrom",
Description: "FROM command must use declared ARGs",
Detail: "FROM argument 'BULIDPLATFORM' is not declared",
Level: 1,
Line: 2,
},
},
StreamBuildErr: "failed to solve: failed to parse platform : \"\" is an invalid component of \"\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument",
UnmarshalBuildErr: "failed to parse platform : \"\" is an invalid component of \"\": platform specifier component must match \"^[A-Za-z0-9_-]+$\": invalid argument",
BuildErrLocation: 2,
})

dockerfile = []byte(`
ARG tag=latest
FROM busybox:${tag}${version} AS b
Expand Down Expand Up @@ -434,7 +467,6 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara
},
}, "", frontend, nil)
require.NoError(t, err)

require.True(t, called)
}

Expand Down Expand Up @@ -503,6 +535,7 @@ func checkProgressStream(t *testing.T, sb integration.Sandbox, lintTest *lintTes
}

func checkLinterWarnings(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
t.Helper()
sort.Slice(lintTest.Warnings, func(i, j int) bool {
return lintTest.Warnings[i].Line < lintTest.Warnings[j].Line
})
Expand Down
4 changes: 2 additions & 2 deletions frontend/dockerfile/shell/lex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *Lex) ProcessWordWithMap(word string, env map[string]string) (string, er
}

type ProcessWordMatchesResult struct {
Word string
Result string
Matched map[string]struct{}
Unmatched map[string]struct{}
}
Expand All @@ -69,7 +69,7 @@ func (s *Lex) ProcessWordWithMatches(word string, env map[string]string) (Proces
sw := s.init(word, env)
word, _, err := sw.process(word)
return ProcessWordMatchesResult{
Word: word,
Result: word,
Matched: sw.matches,
Unmatched: sw.nonmatches,
}, err
Expand Down
48 changes: 34 additions & 14 deletions frontend/dockerfile/shell/lex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func TestProcessWithMatches(t *testing.T) {
expected string
expectedErr bool
matches map[string]struct{}
unmatched map[string]struct{}
}{
{
input: "x",
Expand All @@ -279,10 +280,11 @@ func TestProcessWithMatches(t *testing.T) {
matches: nil,
},
{
input: "x ${UNUSED}",
envs: map[string]string{"DUMMY": "dummy"},
expected: "x ",
matches: nil,
input: "x ${UNUSED}",
envs: map[string]string{"DUMMY": "dummy"},
expected: "x ",
matches: nil,
unmatched: map[string]struct{}{"UNUSED": {}},
},
{
input: "x ${FOO}",
Expand All @@ -306,8 +308,9 @@ func TestProcessWithMatches(t *testing.T) {
"FOO": "xxx",
"BAR": "",
},
expected: "xxx bbb ccc",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
expected: "xxx bbb ccc",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
unmatched: map[string]struct{}{"BAZ": {}},
},

{
Expand All @@ -316,17 +319,19 @@ func TestProcessWithMatches(t *testing.T) {
"FOO": "xxx",
"BAR": "",
},
expected: "aaa bbb ",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
expected: "aaa bbb ",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
unmatched: map[string]struct{}{"BAZ": {}},
},
{
input: "${FOO:+aaa} ${BAR:+bbb} ${BAZ:+ccc}",
envs: map[string]string{
"FOO": "xxx",
"BAR": "",
},
expected: "aaa ",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
expected: "aaa ",
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
unmatched: map[string]struct{}{"BAZ": {}},
},

{
Expand Down Expand Up @@ -354,6 +359,7 @@ func TestProcessWithMatches(t *testing.T) {
"BAR": "",
},
expectedErr: true,
unmatched: map[string]struct{}{"BAZ": {}},
},
{
input: "${FOO:?aaa}",
Expand All @@ -379,6 +385,7 @@ func TestProcessWithMatches(t *testing.T) {
"BAR": "",
},
expectedErr: true,
unmatched: map[string]struct{}{"BAZ": {}},
},

{
Expand Down Expand Up @@ -475,14 +482,22 @@ func TestProcessWithMatches(t *testing.T) {
expected: "\\/tmp\\/foo.txt",
matches: map[string]struct{}{"FOO": {}},
},
{
input: "${FOO/$NEEDLE} = ${FOO//$NEEDLE/}",
envs: map[string]string{"BAR": "bar"},
expected: "",
matches: nil,
unmatched: map[string]struct{}{"FOO": {}, "NEEDLE": {}},
},
}

for _, c := range tc {
c := c
t.Run(c.input, func(t *testing.T) {
result, err := shlex.ProcessWordWithMatches(c.input, c.envs)
w := result.Word
w := result.Result
matches := result.Matched
unmatched := result.Unmatched
if c.expectedErr {
require.Error(t, err)
return
Expand All @@ -494,6 +509,11 @@ func TestProcessWithMatches(t *testing.T) {
for k := range c.matches {
require.Contains(t, matches, k)
}

require.Equal(t, len(c.unmatched), len(unmatched))

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test-windows-amd64 (./..., 1)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} 

=== RUN   TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    lex_test.go:513: 
        	Error Trace:	D:/a/buildkit/buildkit/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    --- FAIL: TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} (0.00s)

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test-windows-amd64 (./..., 1)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} 

=== RUN   TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    lex_test.go:513: 
        	Error Trace:	D:/a/buildkit/buildkit/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    --- FAIL: TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} (0.00s)

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test / run (./..., 1, integration gateway)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} 

=== RUN   TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    lex_test.go:513: 
        	Error Trace:	/src/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    --- FAIL: TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} (0.00s)

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test / run (./..., 1, integration gateway)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} 

=== RUN   TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    lex_test.go:513: 
        	Error Trace:	/src/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    --- FAIL: TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} (0.00s)

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test / run (./..., nydus, 1, integration)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} 

=== RUN   TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    lex_test.go:513: 
        	Error Trace:	/src/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc}
    --- FAIL: TestProcessWithMatches/${FOO-aaa}_${BAR-bbb}_${BAZ-ccc} (0.00s)

Check failure on line 513 in frontend/dockerfile/shell/lex_test.go

View workflow job for this annotation

GitHub Actions / test / run (./..., nydus, 1, integration)

Failed: frontend/dockerfile/shell/TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} 

=== RUN   TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    lex_test.go:513: 
        	Error Trace:	/src/frontend/dockerfile/shell/lex_test.go:513
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.}
    --- FAIL: TestProcessWithMatches/${ABC:-.}${FOO%x}${ABC:-.} (0.00s)
for k := range c.unmatched {
require.Contains(t, unmatched, k)
}
})
}
}
Expand All @@ -514,7 +534,7 @@ func TestProcessWithMatchesPlatform(t *testing.T) {
"TARGETVARIANT": "v7",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm-v7.tar.gz", results.Word)
require.Equal(t, "something-v1.2.3.linux-arm-v7.tar.gz", results.Result)

results, err = shlex.ProcessWordWithMatches(release, map[string]string{
"VERSION": version,
Expand All @@ -523,7 +543,7 @@ func TestProcessWithMatchesPlatform(t *testing.T) {
"TARGETVARIANT": "",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Word)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Result)

results, err = shlex.ProcessWordWithMatches(release, map[string]string{
"VERSION": version,
Expand All @@ -532,5 +552,5 @@ func TestProcessWithMatchesPlatform(t *testing.T) {
// No "TARGETVARIANT": "",
})
require.NoError(t, err)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Word)
require.Equal(t, "something-v1.2.3.linux-arm64.tar.gz", results.Result)
}

0 comments on commit 52948d4

Please sign in to comment.