Merge pull request moby#5387 from daghack/nil-args-to-empty-strings

frontend: update lint checks to correctly utilize EnvList and EnvSetter

Author: tonistiigi

frontend/dockerfile/dockerfile2llb/convert.go

@@ -266,7 +266,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.
@@ -285,7 +285,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{}{}
@@ -312,7 +313,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)
 
@@ -2353,9 +2355,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)
 	}
@@ -2483,10 +2503,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

frontend/dockerfile/dockerfile_lint_test.go

@@ -885,6 +885,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 .
@@ -1542,6 +1563,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 != "" {
@@ -1557,6 +1581,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]