build: do not attempt to push unnamed service images

When building, if images are being pushed, ensure that only
named images (i.e. services with a populated `image` field)
are attempted to be pushed.

Services without `image` get an auto-generated name, which
will be a "Docker library" reference since they're in the
format `$project-$service`, which is implicitly the same as
`docker.io/library/$project-$service`. A push for that is
never desirable / will always fail.

The key here is that we cannot overwrite the `<svc>.image`
field when doing builds, as we need to be able to check for
its presence to determine whether a push makes sense.

Fixes docker#10813.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>

Author: milas

cmd/compose/config.go

@@ -233,11 +233,7 @@ func runConfigImages(streams api.Streams, opts configOptions, services []string)
 		return err
 	}
 	for _, s := range project.Services {
-		if s.Image != "" {
-			fmt.Fprintln(streams.Out(), s.Image)
-		} else {
-			fmt.Fprintf(streams.Out(), "%s%s%s\n", project.Name, api.Separator, s.Name)
-		}
+		fmt.Fprintln(streams.Out(), api.GetImageNameOrDefault(s, project.Name))
 	}
 	return nil
 }

pkg/api/api.go

@@ -145,7 +145,6 @@ func (o BuildOptions) Apply(project *types.Project) error {
 		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)

pkg/compose/build.go

@@ -77,7 +77,7 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 		return nil, err
 	}
 
-	builtIDs := make([]string, len(project.Services))
+	builtDigests := make([]string, len(project.Services))
 	err = InDependencyOrder(ctx, project, func(ctx context.Context, name string) error {
 		if len(options.Services) > 0 && !utils.Contains(options.Services, name) {
 			return nil
@@ -94,11 +94,11 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 			} else {
 				service.Build.Args = service.Build.Args.OverrideBy(args)
 			}
-			id, err := s.doBuildClassic(ctx, service, options)
+			id, err := s.doBuildClassic(ctx, project.Name, service, options)
 			if err != nil {
 				return err
 			}
-			builtIDs[idx] = id
+			builtDigests[idx] = id
 
 			if options.Push {
 				return s.push(ctx, project, api.PushOptions{})
@@ -120,7 +120,7 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 		if err != nil {
 			return err
 		}
-		builtIDs[idx] = digest
+		builtDigests[idx] = digest
 
 		return nil
 	}, func(traversal *graphTraversal) {
@@ -137,9 +137,10 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti
 	}
 
 	imageIDs := map[string]string{}
-	for i, d := range builtIDs {
-		if d != "" {
-			imageIDs[project.Services[i].Image] = d
+	for i, imageDigest := range builtDigests {
+		if imageDigest != "" {
+			imageRef := api.GetImageNameOrDefault(project.Services[i], project.Name)
+			imageIDs[imageRef] = imageDigest
 		}
 	}
 	return imageIDs, err
@@ -222,7 +223,8 @@ func (s *composeService) prepareProjectForBuild(project *types.Project, images m
 			continue
 		}
 
-		_, localImagePresent := images[service.Image]
+		image := api.GetImageNameOrDefault(service, project.Name)
+		_, localImagePresent := images[image]
 		if localImagePresent && service.PullPolicy != types.PullPolicyBuild {
 			service.Build = nil
 			project.Services[i] = service
@@ -292,8 +294,6 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ
 }
 
 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)))
 
 	for k, v := range storeutil.GetProxyConfig(s.dockerCli) {
@@ -335,6 +335,7 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 		sessionConfig = append(sessionConfig, secretsProvider)
 	}
 
+	tags := []string{api.GetImageNameOrDefault(service, project.Name)}
 	if len(service.Build.Tags) > 0 {
 		tags = append(tags, service.Build.Tags...)
 	}
@@ -345,18 +346,19 @@ func (s *composeService) toBuildOptions(project *types.Project, service types.Se
 
 	imageLabels := getImageBuildLabels(project, service)
 
+	push := options.Push && service.Image != ""
 	exports := []bclient.ExportEntry{{
 		Type: "docker",
 		Attrs: map[string]string{
 			"load": "true",
-			"push": fmt.Sprint(options.Push),
+			"push": fmt.Sprint(push),
 		},
 	}}
 	if len(service.Build.Platforms) > 1 {
 		exports = []bclient.ExportEntry{{
 			Type: "image",
 			Attrs: map[string]string{
-				"push": fmt.Sprint(options.Push),
+				"push": fmt.Sprint(push),
 			},
 		}}
 	}

pkg/compose/build_classic.go

@@ -45,7 +45,7 @@ import (
 )
 
 //nolint:gocyclo
-func (s *composeService) doBuildClassic(ctx context.Context, service types.ServiceConfig, options api.BuildOptions) (string, error) {
+func (s *composeService) doBuildClassic(ctx context.Context, projectName string, service types.ServiceConfig, options api.BuildOptions) (string, error) {
 	var (
 		buildCtx      io.ReadCloser
 		dockerfileCtx io.ReadCloser
@@ -160,7 +160,8 @@ func (s *composeService) doBuildClassic(ctx context.Context, service types.Servi
 		authConfigs[k] = registry.AuthConfig(auth)
 	}
 	buildOptions := imageBuildOptions(service.Build)
-	buildOptions.Tags = append(buildOptions.Tags, service.Image)
+	imageName := api.GetImageNameOrDefault(service, projectName)
+	buildOptions.Tags = append(buildOptions.Tags, imageName)
 	buildOptions.Dockerfile = relDockerfile
 	buildOptions.AuthConfigs = authConfigs
 	buildOptions.Memory = options.Memory

pkg/compose/pull.go

@@ -313,8 +313,15 @@ func isServiceImageToBuild(service types.ServiceConfig, services []types.Service
 		return true
 	}
 
-	for _, depService := range services {
-		if depService.Image == service.Image && depService.Build != nil {
+	if service.Image == "" {
+		// N.B. this should be impossible as service must have either `build` or `image` (or both)
+		return false
+	}
+
+	// look through the other services to see if another has a build definition for the same
+	// image name
+	for _, svc := range services {
+		if svc.Image == service.Image && svc.Build != nil {
 			return true
 		}
 	}

pkg/compose/viz.go

@@ -52,7 +52,7 @@ func (s *composeService) Viz(_ context.Context, project *types.Project, opts api
 	// dot is the perfect layout for this use case since graph is directed and hierarchical
 	graphBuilder.WriteString(opts.Indentation + "layout=dot;\n")
 
-	addNodes(&graphBuilder, graph, &opts)
+	addNodes(&graphBuilder, graph, project.Name, &opts)
 	graphBuilder.WriteByte('\n')
 
 	addEdges(&graphBuilder, graph, &opts)
@@ -63,7 +63,7 @@ func (s *composeService) Viz(_ context.Context, project *types.Project, opts api
 
 // addNodes adds the corresponding graphviz representation of all the nodes in the given graph to the graphBuilder
 // returns the same graphBuilder
-func addNodes(graphBuilder *strings.Builder, graph vizGraph, opts *api.VizOptions) *strings.Builder {
+func addNodes(graphBuilder *strings.Builder, graph vizGraph, projectName string, opts *api.VizOptions) *strings.Builder {
 	for serviceNode := range graph {
 		// write:
 		// "service name" [style="filled" label<<font point-size="15">service name</font>
@@ -107,7 +107,7 @@ func addNodes(graphBuilder *strings.Builder, graph vizGraph, opts *api.VizOption
 		if opts.IncludeImageName {
 			graphBuilder.WriteString("<font point-size=\"10\">")
 			graphBuilder.WriteString("<br/><br/><b>Image:</b><br/>")
-			graphBuilder.WriteString(serviceNode.Image)
+			graphBuilder.WriteString(api.GetImageNameOrDefault(*serviceNode, projectName))
 			graphBuilder.WriteString("</font>")
 		}
 

pkg/e2e/build_test.go

@@ -111,7 +111,7 @@ func TestLocalComposeBuild(t *testing.T) {
 		t.Run(env+" no rebuild when up again", func(t *testing.T) {
 			res := c.RunDockerComposeCmd(t, "--project-directory", "fixtures/build-test", "up", "-d")
 
-			assert.Assert(t, !strings.Contains(res.Stdout(), "COPY static"), res.Stdout())
+			assert.Assert(t, !strings.Contains(res.Stdout(), "COPY static"))
 		})
 
 		t.Run(env+" rebuild when up --build", func(t *testing.T) {
@@ -121,6 +121,11 @@ func TestLocalComposeBuild(t *testing.T) {
 			res.Assert(t, icmd.Expected{Out: "COPY static2 /usr/share/nginx/html"})
 		})
 
+		t.Run(env+" build --push ignored for unnamed images", func(t *testing.T) {
+			res := c.RunDockerComposeCmd(t, "--workdir", "fixtures/build-test", "build", "--push", "nginx")
+			assert.Assert(t, !strings.Contains(res.Stdout(), "failed to push"), res.Stdout())
+		})
+
 		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")