Run classic builder with BuildConfig, not buildx.Options

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

Author: ndeloof

pkg/api/api.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/compose-spec/compose-go/types"
+	"github.com/docker/compose/v2/pkg/utils"
 )
 
 // Service manages a compose project
@@ -107,6 +108,38 @@ type BuildOptions struct {
 	SSHs []types.SSHKey
 }
 
+// Apply mutates project according to build options
+func (o BuildOptions) Apply(project *types.Project) error {
+	platform := project.Environment["DOCKER_DEFAULT_PLATFORM"]
+	for i, service := range project.Services {
+		if service.Image == "" && service.Build == nil {
+			return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name)
+		}
+
+		if service.Build == nil {
+			continue
+		}
+		service.Image = GetImageNameOrDefault(service, project.Name)
+		if platform != "" {
+			if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, platform) {
+				return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, platform)
+			}
+			service.Platform = platform
+		}
+		if service.Platform != "" {
+			if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, service.Platform) {
+				return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, service.Platform)
+			}
+		}
+
+		service.Build.Pull = service.Build.Pull || o.Pull
+		service.Build.NoCache = service.Build.NoCache || o.NoCache
+
+		project.Services[i] = service
+	}
+	return nil
+}
+
 // CreateOptions group options of the Create API
 type CreateOptions struct {
 	// Services defines the services user interacts with

pkg/compose/build.go

@@ -43,43 +43,63 @@ import (
 )
 
 func (s *composeService) Build(ctx context.Context, project *types.Project, options api.BuildOptions) error {
+	err := options.Apply(project)
+	if err != nil {
+		return err
+	}
 	return progress.Run(ctx, func(ctx context.Context) error {
 		_, err := s.build(ctx, project, options)
 		return err
 	}, s.stderr())
 }
 
 func (s *composeService) build(ctx context.Context, project *types.Project, options api.BuildOptions) (map[string]string, error) {
-	args := flatten(options.Args.Resolve(envResolver(project.Environment)))
+	args := options.Args.Resolve(envResolver(project.Environment))
 
+	buildkitEnabled, err := s.dockerCli.BuildKitEnabled()
+	if err != nil {
+		return nil, err
+	}
 	builtIDs := make([]string, len(project.Services))
-	err := InDependencyOrder(ctx, project, func(ctx context.Context, name string) error {
+	err = InDependencyOrder(ctx, project, func(ctx context.Context, name string) error {
 		if len(options.Services) > 0 && !utils.Contains(options.Services, name) {
 			return nil
 		}
 		for i, service := range project.Services {
 			if service.Name != name {
 				continue
 			}
-			service, err := project.GetService(name)
-			if err != nil {
-				return err
-			}
+
 			if service.Build == nil {
 				return nil
 			}
-			imageName := api.GetImageNameOrDefault(service, project.Name)
-			buildOptions, err := s.toBuildOptions(project, service, imageName, options)
+
+			if !buildkitEnabled {
+				service.Build.Args = service.Build.Args.OverrideBy(args)
+				id, err := s.doBuildClassic(ctx, service)
+				if err != nil {
+					return err
+				}
+				builtIDs[i] = id
+
+				if options.Push {
+					return s.push(ctx, project, api.PushOptions{})
+				}
+				return nil
+			}
+			buildOptions, err := s.toBuildOptions(project, service, options)
 			if err != nil {
 				return err
 			}
-			buildOptions.BuildArgs = mergeArgs(buildOptions.BuildArgs, args)
-			opts := map[string]build.Options{imageName: buildOptions}
-			ids, err := s.doBuild(ctx, project, opts, options.Progress)
+			buildOptions.BuildArgs = mergeArgs(buildOptions.BuildArgs, flatten(args))
+			opts := map[string]build.Options{service.Name: buildOptions}
+
+			ids, err := s.doBuildBuildkit(ctx, opts, options.Progress)
 			if err != nil {
 				return err
 			}
-			builtIDs[i] = ids[imageName]
+			builtIDs[i] = ids[service.Name]
+			return nil
 		}
 		return nil
 	}, func(traversal *graphTraversal) {
@@ -146,39 +166,26 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types.
 }
 
 func (s *composeService) prepareProjectForBuild(project *types.Project, images map[string]string) error {
-	platform := project.Environment["DOCKER_DEFAULT_PLATFORM"]
+	err := api.BuildOptions{}.Apply(project)
+	if err != nil {
+		return err
+	}
 	for i, service := range project.Services {
-		if service.Image == "" && service.Build == nil {
-			return fmt.Errorf("invalid service %q. Must specify either image or build", service.Name)
-		}
 		if service.Build == nil {
 			continue
 		}
 
-		imageName := api.GetImageNameOrDefault(service, project.Name)
-		service.Image = imageName
-
-		_, localImagePresent := images[imageName]
+		_, localImagePresent := images[service.Image]
 		if localImagePresent && service.PullPolicy != types.PullPolicyBuild {
 			service.Build = nil
 			project.Services[i] = service
 			continue
 		}
 
-		if platform != "" {
-			if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, platform) {
-				return fmt.Errorf("service %q build.platforms does not support value set by DOCKER_DEFAULT_PLATFORM: %s", service.Name, platform)
-			}
-			service.Platform = platform
-		}
-
 		if service.Platform == "" {
 			// let builder to build for default platform
 			service.Build.Platforms = nil
 		} else {
-			if len(service.Build.Platforms) > 0 && !utils.StringContains(service.Build.Platforms, service.Platform) {
-				return fmt.Errorf("service %q build configuration does not support platform: %s", service.Name, platform)
-			}
 			service.Build.Platforms = []string{service.Platform}
 		}
 		project.Services[i] = service
@@ -214,19 +221,8 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ
 	return images, nil
 }
 
-func (s *composeService) doBuild(ctx context.Context, project *types.Project, opts map[string]build.Options, mode string) (map[string]string, error) {
-	if len(opts) == 0 {
-		return nil, nil
-	}
-	if buildkitEnabled, err := s.dockerCli.BuildKitEnabled(); err != nil || !buildkitEnabled {
-		return s.doBuildClassic(ctx, project, opts)
-	}
-	return s.doBuildBuildkit(ctx, opts, mode)
-}
-
-func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, imageTag string, options api.BuildOptions) (build.Options, error) {
-	var tags []string
-	tags = append(tags, imageTag)
+func (s *composeService) toBuildOptions(project *types.Project, service types.ServiceConfig, options api.BuildOptions) (build.Options, error) {
+	tags := []string{service.Image}
 
 	buildArgs := flatten(service.Build.Args.Resolve(envResolver(project.Environment)))
 
@@ -304,8 +300,8 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 		},
 		CacheFrom:   cacheFrom,
 		CacheTo:     cacheTo,
-		NoCache:     service.Build.NoCache || options.NoCache,
-		Pull:        service.Build.Pull || options.Pull,
+		NoCache:     service.Build.NoCache,
+		Pull:        service.Build.Pull,
 		BuildArgs:   buildArgs,
 		Tags:        tags,
 		Target:      service.Build.Target,

pkg/compose/build_classic.go

@@ -27,55 +27,23 @@ import (
 	"strings"
 
 	"github.com/compose-spec/compose-go/types"
-	buildx "github.com/docker/buildx/build"
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command/image/build"
-	"github.com/docker/compose/v2/pkg/utils"
 	dockertypes "github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/remotecontext/urlutil"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
-	"github.com/hashicorp/go-multierror"
-	"github.com/moby/buildkit/util/entitlements"
 	"github.com/pkg/errors"
 
 	"github.com/docker/compose/v2/pkg/api"
 )
 
-func (s *composeService) doBuildClassic(ctx context.Context, project *types.Project, opts map[string]buildx.Options) (map[string]string, error) {
-	nameDigests := make(map[string]string)
-	var errs error
-	err := project.WithServices(nil, func(service types.ServiceConfig) error {
-		imageName := api.GetImageNameOrDefault(service, project.Name)
-		o, ok := opts[imageName]
-		if !ok {
-			return nil
-		}
-		digest, err := s.doBuildClassicSimpleImage(ctx, o)
-		if err != nil {
-			errs = multierror.Append(errs, err).ErrorOrNil()
-		}
-		nameDigests[imageName] = digest
-		if errs != nil {
-			return nil
-		}
-		if len(o.Exports) != 0 && o.Exports[0].Attrs["push"] == "true" {
-			return s.push(ctx, project, api.PushOptions{})
-		}
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	return nameDigests, errs
-}
-
 //nolint:gocyclo
-func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options buildx.Options) (string, error) {
+func (s *composeService) doBuildClassic(ctx context.Context, service types.ServiceConfig) (string, error) {
 	var (
 		buildCtx      io.ReadCloser
 		dockerfileCtx io.ReadCloser
@@ -86,31 +54,31 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
 		err error
 	)
 
-	dockerfileName := options.Inputs.DockerfilePath
-	specifiedContext := options.Inputs.ContextPath
+	dockerfileName := dockerFilePath(service.Build.Context, service.Build.Dockerfile)
+	specifiedContext := service.Build.Context
 	progBuff := s.stdout()
 	buildBuff := s.stdout()
-	if options.ImageIDFile != "" {
-		// Avoid leaving a stale file if we eventually fail
-		if err := os.Remove(options.ImageIDFile); err != nil && !os.IsNotExist(err) {
-			return "", errors.Wrap(err, "removing image ID file")
-		}
-	}
 
-	if len(options.Platforms) > 1 {
-		return "", errors.Errorf("this builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use multi-arch builder")
+	if len(service.Build.Platforms) > 1 {
+		return "", errors.Errorf("the classic builder doesn't support multi-arch build, set DOCKER_BUILDKIT=1 to use BuildKit")
+	}
+	if service.Build.Privileged {
+		return "", errors.Errorf("the classic builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use BuildKit")
+	}
+	if len(service.Build.AdditionalContexts) > 0 {
+		return "", errors.Errorf("the classic builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit")
 	}
-	if utils.Contains(options.Allow, entitlements.EntitlementSecurityInsecure) {
-		return "", errors.Errorf("this builder doesn't support privileged mode, set DOCKER_BUILDKIT=1 to use builder supporting privileged mode")
+	if len(service.Build.SSH) > 0 {
+		return "", errors.Errorf("the classic builder doesn't support SSH keys, set DOCKER_BUILDKIT=1 to use BuildKit")
 	}
-	if len(options.Inputs.NamedContexts) > 0 {
-		return "", errors.Errorf("this builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit which does")
+	if len(service.Build.Secrets) > 0 {
+		return "", errors.Errorf("the classic builder doesn't support secrets, set DOCKER_BUILDKIT=1 to use BuildKit")
 	}
 
-	if options.Labels == nil {
-		options.Labels = make(map[string]string)
+	if service.Build.Labels == nil {
+		service.Build.Labels = make(map[string]string)
 	}
-	options.Labels[api.ImageBuilderLabel] = "classic"
+	service.Build.Labels[api.ImageBuilderLabel] = "classic"
 
 	switch {
 	case isLocalDir(specifiedContext):
@@ -189,8 +157,8 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
 	for k, auth := range creds {
 		authConfigs[k] = dockertypes.AuthConfig(auth)
 	}
-	buildOptions := imageBuildOptions(options)
-	buildOptions.Version = dockertypes.BuilderV1
+	buildOptions := imageBuildOptions(service.Build)
+	buildOptions.Tags = append(buildOptions.Tags, service.Image)
 	buildOptions.Dockerfile = relDockerfile
 	buildOptions.AuthConfigs = authConfigs
 
@@ -235,15 +203,6 @@ func (s *composeService) doBuildClassicSimpleImage(ctx context.Context, options
 			"files and directories.")
 	}
 
-	if options.ImageIDFile != "" {
-		if imageID == "" {
-			return "", errors.Errorf("Server did not provide an image ID. Cannot write %s", options.ImageIDFile)
-		}
-		if err := os.WriteFile(options.ImageIDFile, []byte(imageID), 0o666); err != nil {
-			return "", err
-		}
-	}
-
 	return imageID, nil
 }
 
@@ -252,25 +211,18 @@ func isLocalDir(c string) bool {
 	return err == nil
 }
 
-func imageBuildOptions(options buildx.Options) dockertypes.ImageBuildOptions {
+func imageBuildOptions(config *types.BuildConfig) dockertypes.ImageBuildOptions {
 	return dockertypes.ImageBuildOptions{
-		Tags:        options.Tags,
-		NoCache:     options.NoCache,
+		Version:     dockertypes.BuilderV1,
+		Tags:        config.Tags,
+		NoCache:     config.NoCache,
 		Remove:      true,
-		PullParent:  options.Pull,
-		BuildArgs:   toMapStringStringPtr(options.BuildArgs),
-		Labels:      options.Labels,
-		NetworkMode: options.NetworkMode,
-		ExtraHosts:  options.ExtraHosts,
-		Target:      options.Target,
-	}
-}
-
-func toMapStringStringPtr(source map[string]string) map[string]*string {
-	dest := make(map[string]*string)
-	for k, v := range source {
-		v := v
-		dest[k] = &v
+		PullParent:  config.Pull,
+		BuildArgs:   config.Args,
+		Labels:      config.Labels,
+		NetworkMode: config.Network,
+		ExtraHosts:  config.ExtraHosts.AsList(),
+		Target:      config.Target,
+		Isolation:   container.Isolation(config.Isolation),
 	}
-	return dest
 }