Skip to content

Commit

Permalink
Add nil args to env list as empty strings when calling buildMetaArgs
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 Oct 4, 2024
1 parent a38e4cc commit 2b57e4f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 17 deletions.
35 changes: 18 additions & 17 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,16 +263,15 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
shlex := shell.NewLex(dockerfile.EscapeToken)
outline := newOutlineCapture()

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

// Rebuild the arguments using the provided build arguments
// for the remainder of the build.
globalArgs, outline.allArgs, err = buildMetaArgs(globalArgs, shlex, argCmds, opt.BuildArgs)
if err != nil {
return nil, err
}
// Validate that base images continue to be valid even
// when no build arguments are used.
validateBaseImagesWithDefaultArgs(stages, shlex, outline.allArgs, lint)

metaResolver := opt.MetaResolver
if metaResolver == nil {
Expand All @@ -284,7 +283,7 @@ 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)
reportUnusedFromArgs(outline.allArgs, nameMatch.Unmatched, st.Location, lint)
used := nameMatch.Matched
if used == nil {
used = map[string]struct{}{}
Expand All @@ -311,7 +310,7 @@ 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)
reportUnusedFromArgs(outline.allArgs, 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 +2331,16 @@ 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 reportUnusedFromArgs(args map[string]argInfo, unmatched map[string]struct{}, location []parser.Range, lint *linter.Linter) {
argKeys := make([]string, 0, len(args))
for k := range args {
argKeys = append(argKeys, k)
}
for arg := range unmatched {
suggest, _ := suggest.Search(arg, args.Keys(), true)
if _, ok := args[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,15 +2462,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) {
// Build the arguments as if no build options were given
// and using only defaults.
args, _, err := buildMetaArgs(args, shlex, argsCmds, 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
// of the linting check.
return
func validateBaseImagesWithDefaultArgs(stages []instructions.Stage, shlex *shell.Lex, allArgs map[string]argInfo, lint *linter.Linter) {
args := &llb.EnvList{}
for key, info := range allArgs {
args = args.AddOrReplace(key, info.value)
}

for _, st := range stages {
Expand Down
31 changes: 31 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,13 @@ 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))
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 2b57e4f

Please sign in to comment.