diff --git a/pkg/compose/build.go b/pkg/compose/build.go index 3b19c75a..cf2f8a36 100644 --- a/pkg/compose/build.go +++ b/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))) +// 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)) - for k, v := range storeutil.GetProxyConfig(s.dockerCli) { - if _, ok := buildArgs[k]; !ok { - buildArgs[k] = v + // 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 "" diff --git a/pkg/compose/build_classic.go b/pkg/compose/build_classic.go index 2f08837a..243ffb17 100644 --- a/pkg/compose/build_classic.go +++ b/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(), diff --git a/pkg/e2e/build_test.go b/pkg/e2e/build_test.go index 0f4702c3..7e6d5590 100644 --- a/pkg/e2e/build_test.go +++ b/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") }) }