diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 0df35326b723a..358245d56a73b 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -364,8 +364,6 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS } } - validateCommandArgs(allDispatchStates, shlex, opt.Warn) - var target *dispatchState if opt.Target == "" { target = allDispatchStates.lastTarget() @@ -613,6 +611,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS cgroupParent: opt.CgroupParent, llbCaps: opt.LLBCaps, sourceMap: opt.SourceMap, + warn: opt.Warn, + argCmdVars: make(map[string]struct{}), } if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil { @@ -748,15 +748,17 @@ type dispatchOpt struct { cgroupParent string llbCaps *apicaps.CapSet sourceMap *llb.SourceMap + warn linter.LintWarnFunc + argCmdVars map[string]struct{} } func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { + env, err := d.state.Env(context.TODO()) + if err != nil { + return err + } if ex, ok := cmd.Command.(instructions.SupportsSingleWordExpansion); ok { err := ex.Expand(func(word string) (string, error) { - env, err := d.state.Env(context.TODO()) - if err != nil { - return "", err - } newword, _, err := opt.shlex.ProcessWord(word, env) return newword, err }) @@ -766,11 +768,6 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { } if ex, ok := cmd.Command.(instructions.SupportsSingleWordExpansionRaw); ok { err := ex.ExpandRaw(func(word string) (string, error) { - env, err := d.state.Env(context.TODO()) - if err != nil { - return "", err - } - lex := shell.NewLex('\\') lex.SkipProcessQuotes = true newword, _, err := lex.ProcessWord(word, env) @@ -780,8 +777,8 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { return err } } + validateCommandVar(cmd.Command, env, &opt) - var err error switch c := cmd.Command.(type) { case *instructions.MaintainerCommand: err = dispatchMaintainer(d, c) @@ -840,7 +837,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { case *instructions.ShellCommand: err = dispatchShell(d, c) case *instructions.ArgCommand: - err = dispatchArg(d, c, opt.metaArgs, opt.buildArgValues) + err = dispatchArg(d, c, opt.metaArgs, &opt) case *instructions.CopyCommand: l := opt.buildContext if len(cmd.sources) != 0 { @@ -1539,10 +1536,11 @@ func dispatchShell(d *dispatchState, c *instructions.ShellCommand) error { return commitToHistory(&d.image, fmt.Sprintf("SHELL %v", c.Shell), false, nil, d.epoch) } -func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instructions.KeyValuePairOptional, buildArgValues map[string]string) error { +func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instructions.KeyValuePairOptional, opt *dispatchOpt) error { commitStrs := make([]string, 0, len(c.Args)) for _, arg := range c.Args { - buildArg := setKVValue(arg, buildArgValues) + opt.argCmdVars[arg.Key] = struct{}{} + buildArg := setKVValue(arg, opt.buildArgValues) commitStr := arg.Key if arg.Value != nil { @@ -2038,27 +2036,15 @@ 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 validateCommandVar(cmd instructions.Command, env []string, opt *dispatchOpt) { + if cmdstr, ok := cmd.(fmt.Stringer); ok { + _, unmatched, _ := opt.shlex.ProcessWord(cmdstr.String(), env) + for arg := range unmatched { + _, argCmdOk := opt.argCmdVars[arg] + _, nonEnvOk := nonEnvArgs[arg] + if !argCmdOk && !nonEnvOk { + msg := linter.RuleUndefinedVar.Format(arg) + linter.RuleUndefinedVar.Run(opt.warn, cmd.Location(), msg) } } } diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index baa0fed7b92d4..5b859920883a6 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -31,7 +31,7 @@ var lintTests = integration.TestFuncs( testMaintainerDeprecated, testWarningsBeforeError, testUndeclaredArg, - testUndefinedArg, + testUndefinedVars, ) func testStageName(t *testing.T, sb integration.Sandbox) { @@ -419,7 +419,7 @@ COPY Dockerfile . }) } -func testUndefinedArg(t *testing.T, sb integration.Sandbox) { +func testUndefinedVars(t *testing.T, sb integration.Sandbox) { dockerfile := []byte(` FROM scratch ARG foo @@ -428,7 +428,22 @@ COPY Dockerfile${foo} . checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) dockerfile = []byte(` -FROM busybox +FROM alpine AS base +ARG foo=Dockerfile + +FROM base +COPY $foo . +`) + checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) + + dockerfile = []byte(` +FROM alpine +RUN echo $PATH +`) + checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) + + dockerfile = []byte(` +FROM alpine COPY $foo . ARG foo=bar RUN echo $foo @@ -437,9 +452,9 @@ RUN echo $foo Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "UndefinedArg", - Description: "ARGs should be defined before their use", - Detail: "Usage of undefined ARG 'foo'", + RuleName: "UndefinedVar", + Description: "Variables should be defined before their use", + Detail: "Usage of undefined variable '$foo'", Level: 1, Line: 3, }, @@ -476,6 +491,7 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara require.Less(t, lintResults.Error.Location.SourceIndex, int32(len(lintResults.Sources))) } require.Equal(t, len(lintTest.Warnings), len(lintResults.Warnings)) + sort.Slice(lintResults.Warnings, func(i, j int) bool { // sort by line number in ascending order firstRange := lintResults.Warnings[i].Location.Ranges[0] diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go index 07c3801934534..94e75ab52e3fb 100644 --- a/frontend/dockerfile/linter/ruleset.go +++ b/frontend/dockerfile/linter/ruleset.go @@ -70,11 +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", + RuleUndefinedVar = LinterRule[func(string) string]{ + Name: "UndefinedVar", + Description: "Variables should be defined before their use", Format: func(arg string) string { - return fmt.Sprintf("Usage of undefined ARG '%s'", arg) + return fmt.Sprintf("Usage of undefined variable '$%s'", arg) }, } )