build: fix missing proxy build args for classic builder (docker#10887)

Refactor to use a consistent code path for determining the build
args for a service image regardless of whether BuildKit or the
classic builder is being used.

After recent changes, these code paths had diverged, so the classic
builder was missing the proxy variables from the Docker client
config.

Signed-off-by: Milas Bowman <[email protected]>

Author: milas

pkg/compose/build.go

@@ -22,18 +22,19 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/docker/buildx/builder"
-	"github.com/docker/compose/v2/internal/tracing"
-
-	"github.com/docker/buildx/controller/pb"
-
 	"github.com/compose-spec/compose-go/types"
 	"github.com/containerd/containerd/platforms"
 	"github.com/docker/buildx/build"
-	_ "github.com/docker/buildx/driver/docker" // required to get default driver registered
+	"github.com/docker/buildx/builder"
+	"github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/store/storeutil"
 	"github.com/docker/buildx/util/buildflags"
 	xprogress "github.com/docker/buildx/util/progress"
+	"github.com/docker/cli/cli/command"
+	"github.com/docker/compose/v2/internal/tracing"
+	"github.com/docker/compose/v2/pkg/api"
+	"github.com/docker/compose/v2/pkg/progress"
+	"github.com/docker/compose/v2/pkg/utils"
 	"github.com/docker/docker/builder/remotecontext/urlutil"
 	bclient "github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/session"
@@ -45,9 +46,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 
-	"github.com/docker/compose/v2/pkg/api"
-	"github.com/docker/compose/v2/pkg/progress"
-	"github.com/docker/compose/v2/pkg/utils"
+	// required to get default driver registered
+	_ "github.com/docker/buildx/driver/docker"
 )
 
 func (s *composeService) Build(ctx context.Context, project *types.Project, options api.BuildOptions) error {
@@ -61,9 +61,8 @@ func (s *composeService) Build(ctx context.Context, project *types.Project, opti
 	}, s.stdinfo(), "Building")
 }
 
-func (s *composeService) build(ctx context.Context, project *types.Project, options api.BuildOptions) (map[string]string, error) { //nolint:gocyclo
-	args := options.Args.Resolve(envResolver(project.Environment))
-
+//nolint:gocyclo
+func (s *composeService) build(ctx context.Context, project *types.Project, options api.BuildOptions) (map[string]string, error) {
 	buildkitEnabled, err := s.dockerCli.BuildKitEnabled()
 	if err != nil {
 		return nil, err
@@ -119,12 +118,7 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 		}
 
 		if !buildkitEnabled {
-			if service.Build.Args == nil {
-				service.Build.Args = args
-			} else {
-				service.Build.Args = service.Build.Args.OverrideBy(args)
-			}
-			id, err := s.doBuildClassic(ctx, project.Name, service, options)
+			id, err := s.doBuildClassic(ctx, project, service, options)
 			if err != nil {
 				return err
 			}
@@ -144,7 +138,6 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 		if err != nil {
 			return err
 		}
-		buildOptions.BuildArgs = mergeArgs(buildOptions.BuildArgs, flatten(args))
 
 		digest, err := s.doBuildBuildkit(ctx, service.Name, buildOptions, w, nodes)
 		if err != nil {
@@ -337,15 +330,37 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ
 	return images, nil
 }
 
-func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, options api.BuildOptions) (build.Options, error) {
-	buildArgs := flatten(service.Build.Args.Resolve(envResolver(project.Environment)))
-
-	for k, v := range storeutil.GetProxyConfig(s.dockerCli) {
-		if _, ok := buildArgs[k]; !ok {
-			buildArgs[k] = v
-		}
-	}
+// resolveAndMergeBuildArgs returns the final set of build arguments to use for the service image build.
+//
+// First, args directly defined via `build.args` in YAML are considered.
+// Then, any explicitly passed args in opts (e.g. via `--build-arg` on the CLI) are merged, overwriting any
+// keys that already exist.
+// Next, any keys without a value are resolved using the project environment.
+//
+// Finally, standard proxy variables based on the Docker client configuration are added, but will not overwrite
+// any values if already present.
+func resolveAndMergeBuildArgs(
+	dockerCli command.Cli,
+	project *types.Project,
+	service types.ServiceConfig,
+	opts api.BuildOptions,
+) types.MappingWithEquals {
+	result := make(types.MappingWithEquals).
+		OverrideBy(service.Build.Args).
+		OverrideBy(opts.Args).
+		Resolve(envResolver(project.Environment))
+
+	// proxy arguments do NOT override and should NOT have env resolution applied,
+	// so they're handled last
+	for k, v := range storeutil.GetProxyConfig(dockerCli) {
+		if _, ok := result[k]; !ok {
+			result[k] = &v
+		}
+	}
+	return result
+}
 
+func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, options api.BuildOptions) (build.Options, error) {
 	plats, err := addPlatforms(project, service)
 	if err != nil {
 		return build.Options{}, err
@@ -418,7 +433,7 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 		CacheTo:     pb.CreateCaches(cacheTo),
 		NoCache:     service.Build.NoCache,
 		Pull:        service.Build.Pull,
-		BuildArgs:   buildArgs,
+		BuildArgs:   flatten(resolveAndMergeBuildArgs(s.dockerCli, project, service, options)),
 		Tags:        tags,
 		Target:      service.Build.Target,
 		Exports:     exports,
@@ -445,16 +460,6 @@ func flatten(in types.MappingWithEquals) types.Mapping {
 	return out
 }
 
-func mergeArgs(m ...types.Mapping) types.Mapping {
-	merged := types.Mapping{}
-	for _, mapping := range m {
-		for key, val := range mapping {
-			merged[key] = val
-		}
-	}
-	return merged
-}
-
 func dockerFilePath(ctxName string, dockerfile string) string {
 	if dockerfile == "" {
 		return ""

pkg/compose/build_classic.go

@@ -26,6 +26,8 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/docker/cli/cli/command"
+
 	"github.com/docker/docker/api/types/registry"
 
 	"github.com/compose-spec/compose-go/types"
@@ -45,7 +47,7 @@ import (
 )
 
 //nolint:gocyclo
-func (s *composeService) doBuildClassic(ctx context.Context, projectName string, service types.ServiceConfig, options api.BuildOptions) (string, error) {
+func (s *composeService) doBuildClassic(ctx context.Context, project *types.Project, service types.ServiceConfig, options api.BuildOptions) (string, error) {
 	var (
 		buildCtx      io.ReadCloser
 		dockerfileCtx io.ReadCloser
@@ -159,8 +161,8 @@ func (s *composeService) doBuildClassic(ctx context.Context, projectName string,
 	for k, auth := range creds {
 		authConfigs[k] = registry.AuthConfig(auth)
 	}
-	buildOptions := imageBuildOptions(service.Build)
-	imageName := api.GetImageNameOrDefault(service, projectName)
+	buildOptions := imageBuildOptions(s.dockerCli, project, service, options)
+	imageName := api.GetImageNameOrDefault(service, project.Name)
 	buildOptions.Tags = append(buildOptions.Tags, imageName)
 	buildOptions.Dockerfile = relDockerfile
 	buildOptions.AuthConfigs = authConfigs
@@ -215,14 +217,15 @@ func isLocalDir(c string) bool {
 	return err == nil
 }
 
-func imageBuildOptions(config *types.BuildConfig) dockertypes.ImageBuildOptions {
+func imageBuildOptions(dockerCli command.Cli, project *types.Project, service types.ServiceConfig, options api.BuildOptions) dockertypes.ImageBuildOptions {
+	config := service.Build
 	return dockertypes.ImageBuildOptions{
 		Version:     dockertypes.BuilderV1,
 		Tags:        config.Tags,
 		NoCache:     config.NoCache,
 		Remove:      true,
 		PullParent:  config.Pull,
-		BuildArgs:   config.Args,
+		BuildArgs:   resolveAndMergeBuildArgs(dockerCli, project, service, options),
 		Labels:      config.Labels,
 		NetworkMode: config.Network,
 		ExtraHosts:  config.ExtraHosts.AsList(),

pkg/e2e/build_test.go

@@ -38,8 +38,8 @@ func TestLocalComposeBuild(t *testing.T) {
 
 		t.Run(env+" build named and unnamed images", func(t *testing.T) {
 			// ensure local test run does not reuse previously build image
-			c.RunDockerOrExitError(t, "rmi", "build-test-nginx")
-			c.RunDockerOrExitError(t, "rmi", "custom-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "build-test-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "custom-nginx")
 
 			res := c.RunDockerComposeCmd(t, "--project-directory", "fixtures/build-test", "build")
 
@@ -50,8 +50,8 @@ func TestLocalComposeBuild(t *testing.T) {
 
 		t.Run(env+" build with build-arg", func(t *testing.T) {
 			// ensure local test run does not reuse previously build image
-			c.RunDockerOrExitError(t, "rmi", "build-test-nginx")
-			c.RunDockerOrExitError(t, "rmi", "custom-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "build-test-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "custom-nginx")
 
 			c.RunDockerComposeCmd(t, "--project-directory", "fixtures/build-test", "build", "--build-arg", "FOO=BAR")
 
@@ -61,8 +61,8 @@ func TestLocalComposeBuild(t *testing.T) {
 
 		t.Run(env+" build with build-arg set by env", func(t *testing.T) {
 			// ensure local test run does not reuse previously build image
-			c.RunDockerOrExitError(t, "rmi", "build-test-nginx")
-			c.RunDockerOrExitError(t, "rmi", "custom-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "build-test-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "custom-nginx")
 
 			icmd.RunCmd(c.NewDockerComposeCmd(t,
 				"--project-directory",
@@ -72,7 +72,7 @@ func TestLocalComposeBuild(t *testing.T) {
 				"FOO"),
 				func(cmd *icmd.Cmd) {
 					cmd.Env = append(cmd.Env, "FOO=BAR")
-				})
+				}).Assert(t, icmd.Success)
 
 			res := c.RunDockerCmd(t, "image", "inspect", "build-test-nginx")
 			res.Assert(t, icmd.Expected{Out: `"FOO": "BAR"`})
@@ -92,8 +92,9 @@ func TestLocalComposeBuild(t *testing.T) {
 		})
 
 		t.Run(env+" build as part of up", func(t *testing.T) {
-			c.RunDockerOrExitError(t, "rmi", "build-test-nginx")
-			c.RunDockerOrExitError(t, "rmi", "custom-nginx")
+			// ensure local test run does not reuse previously build image
+			c.RunDockerOrExitError(t, "rmi", "-f", "build-test-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "custom-nginx")
 
 			res := c.RunDockerComposeCmd(t, "--project-directory", "fixtures/build-test", "up", "-d")
 			t.Cleanup(func() {
@@ -130,8 +131,8 @@ func TestLocalComposeBuild(t *testing.T) {
 
 		t.Run(env+" cleanup build project", func(t *testing.T) {
 			c.RunDockerComposeCmd(t, "--project-directory", "fixtures/build-test", "down")
-			c.RunDockerCmd(t, "rmi", "build-test-nginx")
-			c.RunDockerCmd(t, "rmi", "custom-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "build-test-nginx")
+			c.RunDockerOrExitError(t, "rmi", "-f", "custom-nginx")
 		})
 	}