Skip to content

Commit

Permalink
Refactor various rulecheck related code to properly handle env vars.
Browse files Browse the repository at this point in the history
after EnvGetter refactoring

Signed-off-by: Talon Bowler <talon.bowler@docker.com>
  • Loading branch information
daghack committed Oct 8, 2024
1 parent a38e4cc commit 161fc50
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 7 deletions.
34 changes: 27 additions & 7 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

// Validate that base images continue to be valid even
// when no build arguments are used.
validateBaseImagesWithDefaultArgs(stages, shlex, argCmds, globalArgs, lint)
validateBaseImagesWithDefaultArgs(stages, shlex, globalArgs, argCmds, lint)

// Rebuild the arguments using the provided build arguments
// for the remainder of the build.
Expand All @@ -284,7 +284,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
// set base state for every image
for i, st := range stages {
nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, globalArgs)
reportUnusedFromArgs(globalArgs, nameMatch.Unmatched, st.Location, lint)
argKeys := unusedFromArgsCheckKeys(globalArgs, outline.allArgs)
reportUnusedFromArgs(argKeys, nameMatch.Unmatched, st.Location, lint)
used := nameMatch.Matched
if used == nil {
used = map[string]struct{}{}
Expand All @@ -311,7 +312,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

if v := st.Platform; v != "" {
platMatch, err := shlex.ProcessWordWithMatches(v, globalArgs)
reportUnusedFromArgs(globalArgs, platMatch.Unmatched, st.Location, lint)
argKeys := unusedFromArgsCheckKeys(globalArgs, outline.allArgs)
reportUnusedFromArgs(argKeys, platMatch.Unmatched, st.Location, lint)
reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, globalArgs, lint)
reportConstPlatformDisallowed(st.Name, platMatch, st.Location, lint)

Expand Down Expand Up @@ -2332,9 +2334,27 @@ func toPBLocation(sourceIndex int, location []parser.Range) pb.Location {
}
}

func reportUnusedFromArgs(args shell.EnvGetter, unmatched map[string]struct{}, location []parser.Range, lint *linter.Linter) {
func unusedFromArgsCheckKeys(env shell.EnvGetter, args map[string]argInfo) map[string]struct{} {
matched := make(map[string]struct{})
for _, arg := range args {
matched[arg.definition.Key] = struct{}{}
}
for _, k := range env.Keys() {
matched[k] = struct{}{}
}
return matched
}

func reportUnusedFromArgs(testArgKeys map[string]struct{}, unmatched map[string]struct{}, location []parser.Range, lint *linter.Linter) {
var argKeys []string
for arg := range testArgKeys {
argKeys = append(argKeys, arg)
}
for arg := range unmatched {
suggest, _ := suggest.Search(arg, args.Keys(), true)
if _, ok := testArgKeys[arg]; ok {
continue
}
suggest, _ := suggest.Search(arg, argKeys, true)
msg := linter.RuleUndefinedArgInFrom.Format(arg, suggest)
lint.Run(&linter.RuleUndefinedArgInFrom, location, msg)
}
Expand Down Expand Up @@ -2456,10 +2476,10 @@ func validateNoSecretKey(instruction, key string, location []parser.Range, lint
}
}

func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell.Lex, argsCmds []instructions.ArgCommand, args *llb.EnvList, lint *linter.Linter) {
func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell.Lex, env *llb.EnvList, argCmds []instructions.ArgCommand, lint *linter.Linter) {
// Build the arguments as if no build options were given
// and using only defaults.
args, _, err := buildMetaArgs(args, shlex, argsCmds, nil)
args, _, err := buildMetaArgs(env, shlex, argCmds, nil)
if err != nil {
// Abandon running the linter. We'll likely fail after this point
// with the same error but we shouldn't error here inside
Expand Down
36 changes: 36 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,27 @@ COPY Dockerfile .
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

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

dockerfile = []byte(`
ARG DEBUG
FROM scra${DEBUG:-tch}
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

dockerfile = []byte(`
ARG DEBUG=""
FROM scratch${DEBUG-@bogus}
COPY Dockerfile .
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})

dockerfile = []byte(`
FROM --platform=$BULIDPLATFORM scratch
COPY Dockerfile .
Expand Down Expand Up @@ -1456,6 +1477,9 @@ func checkProgressStream(t *testing.T, sb integration.Sandbox, lintTest *lintTes
},
}, status)
if lintTest.StreamBuildErr == "" && lintTest.StreamBuildErrRegexp == nil {
if err != nil {
t.Logf("expected no error, received: %v", err)
}
require.NoError(t, err)
} else {
if lintTest.StreamBuildErr != "" {
Expand All @@ -1471,6 +1495,18 @@ func checkProgressStream(t *testing.T, sb integration.Sandbox, lintTest *lintTes
t.Fatalf("timed out waiting for statusDone")
}

if len(lintTest.Warnings) != len(warnings) {
t.Logf("expected %d warnings, received:", len(lintTest.Warnings))
t.Logf("\texpected:")
for i, w := range lintTest.Warnings {
t.Logf("\t\t%d: %s", i, w.Detail)
}

t.Logf("\treceived:")
for i, w := range warnings {
t.Logf("\t%d: %s", i, w.Short)
}
}
require.Equal(t, len(lintTest.Warnings), len(warnings))
sort.Slice(warnings, func(i, j int) bool {
w1 := warnings[i]
Expand Down

0 comments on commit 161fc50

Please sign in to comment.