dockerfile: avoid evaluating ARG default if unused

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

frontend/dockerfile/dockerfile2llb/convert.go

@@ -733,7 +733,9 @@ type dispatchOpt struct {
 }
 
 func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
-	if ex, ok := cmd.Command.(instructions.SupportsSingleWordExpansion); ok {
+	// ARG command value could be ignored, so defer handling the expansion error
+	_, isArg := cmd.Command.(*instructions.ArgCommand)
+	if ex, ok := cmd.Command.(instructions.SupportsSingleWordExpansion); ok && !isArg {
 		err := ex.Expand(func(word string) (string, error) {
 			env, err := d.state.Env(context.TODO())
 			if err != nil {
@@ -820,7 +822,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.buildArgValues, opt.shlex)
 	case *instructions.CopyCommand:
 		l := opt.buildContext
 		if len(cmd.sources) != 0 {
@@ -1520,42 +1522,59 @@ 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, buildArgValues map[string]string, shlex *shell.Lex) error {
 	commitStrs := make([]string, 0, len(c.Args))
 	for _, arg := range c.Args {
-		buildArg := setKVValue(arg, buildArgValues)
-
-		commitStr := arg.Key
-		if arg.Value != nil {
-			commitStr += "=" + *arg.Value
-		}
-		commitStrs = append(commitStrs, commitStr)
+		_, hasValue := buildArgValues[arg.Key]
+		hasDefault := arg.Value != nil
 
 		skipArgInfo := false // skip the arg info if the arg is inherited from global scope
-		if buildArg.Value == nil {
+		if !hasDefault && !hasValue {
 			for _, ma := range metaArgs {
-				if ma.Key == buildArg.Key {
-					buildArg.Value = ma.Value
+				if ma.Key == arg.Key {
+					arg.Value = ma.Value
+					hasDefault = false
 					skipArgInfo = true
 				}
 			}
 		}
 
+		if hasValue {
+			v := buildArgValues[arg.Key]
+			arg.Value = &v
+		} else if hasDefault {
+			env, err := d.state.Env(context.TODO())
+			if err != nil {
+				return err
+			}
+			v, err := shlex.ProcessWord(*arg.Value, env)
+			if err != nil {
+				return err
+			}
+			arg.Value = &v
+		}
+
 		ai := argInfo{definition: arg, location: c.Location()}
 
-		if buildArg.Value != nil {
-			if _, ok := nonEnvArgs[buildArg.Key]; !ok {
-				d.state = d.state.AddEnv(buildArg.Key, *buildArg.Value)
+		if arg.Value != nil {
+			if _, ok := nonEnvArgs[arg.Key]; !ok {
+				d.state = d.state.AddEnv(arg.Key, *arg.Value)
 			}
-			ai.value = *buildArg.Value
+			ai.value = *arg.Value
 		}
 
 		if !skipArgInfo {
 			d.outline.allArgs[arg.Key] = ai
 		}
 		d.outline.usedArgs[arg.Key] = struct{}{}
 
-		d.buildArgs = append(d.buildArgs, buildArg)
+		d.buildArgs = append(d.buildArgs, arg)
+
+		commitStr := arg.Key
+		if arg.Value != nil {
+			commitStr += "=" + *arg.Value
+		}
+		commitStrs = append(commitStrs, commitStr)
 	}
 	return commitToHistory(&d.image, "ARG "+strings.Join(commitStrs, " "), false, nil, d.epoch)
 }

frontend/dockerfile/dockerfile_test.go

@@ -102,6 +102,7 @@ var allTests = integration.TestFuncs(
 	testExportMultiPlatform,
 	testQuotedMetaArgs,
 	testGlobalArgErrors,
+	testArgDefaultExpansion,
 	testIgnoreEntrypoint,
 	testSymlinkedDockerfile,
 	testEmptyWildcard,
@@ -1332,6 +1333,58 @@ FROM busybox
 	require.NoError(t, err)
 }
 
+func testArgDefaultExpansion(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM scratch
+ARG FOO
+ARG BAR=${FOO:?"foo missing"}
+`)
+
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.Error(t, err)
+
+	require.Contains(t, err.Error(), "FOO: foo missing")
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		FrontendAttrs: map[string]string{
+			"build-arg:FOO": "123",
+		},
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		FrontendAttrs: map[string]string{
+			"build-arg:BAR": "123",
+		},
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
 func testMultiArgs(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)