refactor: last gocognit candidates — bridge resources and TestViz

pkg/bridge/convert.go

@@ -162,47 +162,13 @@ func convert(ctx context.Context, dockerCli command.Cli, model map[string]any, o
 }
 
 // LoadAdditionalResources loads additional resources from the project, such as image references, secrets, configs and exposed ports
-// FIXME(ndeloof) complete migration to gocognit
-//
-//nolint:gocognit
 func LoadAdditionalResources(ctx context.Context, dockerCLI command.Cli, project *types.Project) (*types.Project, error) {
 	for name, service := range project.Services {
-		imageName := api.GetImageNameOrDefault(service, project.Name)
-
-		var inspect image.InspectResponse
-		if service.Build != nil && service.Image == "" {
-			result, err := dockerCLI.Client().ImageInspect(ctx, imageName)
-			if err != nil {
-				if !errdefs.IsNotFound(err) {
-					return nil, err
-				}
-				logrus.Warnf("image %s for service %s not found locally; Dockerfile-exposed ports will not be included — run `docker compose build` first to include them", imageName, name)
-			}
-			inspect = result.InspectResponse
-		} else {
-			var err error
-			inspect, err = inspectWithPull(ctx, dockerCLI, imageName)
-			if err != nil {
-				return nil, err
-			}
-		}
-		service.Image = imageName
-		exposed := utils.Set[string]{}
-		exposed.AddAll(service.Expose...)
-		if inspect.Config != nil {
-			for port := range inspect.Config.ExposedPorts {
-				p, err := network.ParsePort(port)
-				if err != nil {
-					return nil, err
-				}
-				exposed.Add(strconv.Itoa(int(p.Num())))
-			}
-		}
-		for _, port := range service.Ports {
-			exposed.Add(strconv.Itoa(int(port.Target)))
+		updated, err := loadServiceImageResources(ctx, dockerCLI, project.Name, name, service)
+		if err != nil {
+			return nil, err
 		}
-		service.Expose = exposed.Elements()
-		project.Services[name] = service
+		project.Services[name] = updated
 	}
 
 	for name, secret := range project.Secrets {
@@ -224,6 +190,51 @@ func LoadAdditionalResources(ctx context.Context, dockerCLI command.Cli, project
 	return project, nil
 }
 
+// loadServiceImageResources resolves the service image and merges the ports
+// it exposes into the service's Expose list
+func loadServiceImageResources(ctx context.Context, dockerCLI command.Cli, projectName, name string, service types.ServiceConfig) (types.ServiceConfig, error) {
+	imageName := api.GetImageNameOrDefault(service, projectName)
+
+	inspect, err := inspectServiceImage(ctx, dockerCLI, name, imageName, service)
+	if err != nil {
+		return service, err
+	}
+
+	service.Image = imageName
+	exposed := utils.Set[string]{}
+	exposed.AddAll(service.Expose...)
+	if inspect.Config != nil {
+		for port := range inspect.Config.ExposedPorts {
+			p, err := network.ParsePort(port)
+			if err != nil {
+				return service, err
+			}
+			exposed.Add(strconv.Itoa(int(p.Num())))
+		}
+	}
+	for _, port := range service.Ports {
+		exposed.Add(strconv.Itoa(int(port.Target)))
+	}
+	service.Expose = exposed.Elements()
+	return service, nil
+}
+
+// inspectServiceImage inspects the service image, pulling it when needed; a
+// buildable image missing locally only degrades to a warning
+func inspectServiceImage(ctx context.Context, dockerCLI command.Cli, name, imageName string, service types.ServiceConfig) (image.InspectResponse, error) {
+	if service.Build != nil && service.Image == "" {
+		result, err := dockerCLI.Client().ImageInspect(ctx, imageName)
+		if err != nil {
+			if !errdefs.IsNotFound(err) {
+				return image.InspectResponse{}, err
+			}
+			logrus.Warnf("image %s for service %s not found locally; Dockerfile-exposed ports will not be included — run `docker compose build` first to include them", imageName, name)
+		}
+		return result.InspectResponse, nil
+	}
+	return inspectWithPull(ctx, dockerCLI, imageName)
+}
+
 func loadFileObject(conf types.FileObjectConfig) (types.FileObjectConfig, error) {
 	if !conf.External {
 		switch {

pkg/compose/viz_test.go

@@ -29,9 +29,6 @@ import (
 	"github.com/docker/compose/v5/pkg/mocks"
 )
 
-// FIXME(ndeloof) complete migration to gocognit
-//
-//nolint:gocognit
 func TestViz(t *testing.T) {
 	project := types.Project{
 		Name:       "viz-test",
@@ -134,50 +131,14 @@ func TestViz(t *testing.T) {
 		assert.Check(t, is.Contains(graphStr, "\n  "))
 		assert.Check(t, !is.Contains(graphStr, "\n   ")().Success(), graphStr)
 
-		// check digraph name
-		assert.Check(t, is.Contains(graphStr, "digraph \""+project.Name+"\""))
-
-		// check nodes
-		for _, service := range project.Services {
-			assert.Check(t, is.Contains(graphStr, "\""+service.Name+"\" [style=\"filled\""))
-		}
+		assertVizGraphNodes(t, graphStr, project)
 
 		// check node attributes
 		assert.Check(t, !is.Contains(graphStr, "Networks")().Success())
 		assert.Check(t, !is.Contains(graphStr, "Image")().Success())
 		assert.Check(t, !is.Contains(graphStr, "Ports")().Success())
 
-		// check edges that SHOULD exist in the generated graph
-		allowedEdges := make(map[string][]string)
-		for name, service := range project.Services {
-			allowed := make([]string, 0, len(service.DependsOn))
-			for depName := range service.DependsOn {
-				allowed = append(allowed, depName)
-			}
-			allowedEdges[name] = allowed
-		}
-		for serviceName, dependencies := range allowedEdges {
-			for _, dependencyName := range dependencies {
-				assert.Check(t, is.Contains(graphStr, "\""+serviceName+"\" -> \""+dependencyName+"\""))
-			}
-		}
-
-		// check edges that SHOULD NOT exist in the generated graph
-		forbiddenEdges := make(map[string][]string)
-		for name, service := range project.Services {
-			forbiddenEdges[name] = make([]string, 0, len(project.ServiceNames())-len(service.DependsOn))
-			for _, serviceName := range project.ServiceNames() {
-				_, edgeExists := service.DependsOn[serviceName]
-				if !edgeExists {
-					forbiddenEdges[name] = append(forbiddenEdges[name], serviceName)
-				}
-			}
-		}
-		for serviceName, forbiddenDeps := range forbiddenEdges {
-			for _, forbiddenDep := range forbiddenDeps {
-				assert.Check(t, !is.Contains(graphStr, "\""+serviceName+"\" -> \""+forbiddenDep+"\"")().Success())
-			}
-		}
+		assertVizDependencyEdges(t, graphStr, project)
 	})
 
 	t.Run("viz (with ports, networks and image)", func(t *testing.T) {
@@ -193,13 +154,7 @@ func TestViz(t *testing.T) {
 		assert.Check(t, is.Contains(graphStr, "\n\t"))
 		assert.Check(t, !is.Contains(graphStr, "\n\t\t")().Success(), graphStr)
 
-		// check digraph name
-		assert.Check(t, is.Contains(graphStr, "digraph \""+project.Name+"\""))
-
-		// check nodes
-		for _, service := range project.Services {
-			assert.Check(t, is.Contains(graphStr, "\""+service.Name+"\" [style=\"filled\""))
-		}
+		assertVizGraphNodes(t, graphStr, project)
 
 		// check node attributes
 		assert.Check(t, is.Contains(graphStr, "Networks"))
@@ -218,3 +173,29 @@ func TestViz(t *testing.T) {
 		}
 	})
 }
+
+// assertVizGraphNodes checks the digraph is named after the project and has a
+// node per service
+func assertVizGraphNodes(t *testing.T, graphStr string, project types.Project) {
+	t.Helper()
+	assert.Check(t, is.Contains(graphStr, "digraph \""+project.Name+"\""))
+	for _, service := range project.Services {
+		assert.Check(t, is.Contains(graphStr, "\""+service.Name+"\" [style=\"filled\""))
+	}
+}
+
+// assertVizDependencyEdges checks the graph has an edge per depends_on
+// relation, and none between independent services
+func assertVizDependencyEdges(t *testing.T, graphStr string, project types.Project) {
+	t.Helper()
+	for name, service := range project.Services {
+		for _, other := range project.ServiceNames() {
+			edge := "\"" + name + "\" -> \"" + other + "\""
+			if _, expected := service.DependsOn[other]; expected {
+				assert.Check(t, is.Contains(graphStr, edge))
+			} else {
+				assert.Check(t, !is.Contains(graphStr, edge)().Success())
+			}
+		}
+	}
+}