build: fix missing proxy build args for classic builder (#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 <milas.bowman@docker.com>
This commit is contained in:
Milas Bowman 2023-08-10 08:57:28 -04:00 committed by GitHub
parent 0be8e4a676
commit bfa54081d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 51 deletions

View File

@ -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 ""

View File

@ -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(),

View File

@ -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")
})
}