Repository: compose-spec/compose-go
Pull request #432 (2023-07-11 09:52:24)

filter depends_on services when required is not set
If a `depends_on` is marked as non-required, we shouldn't return an error if not found


Patch:
diff --git a/go.mod b/go.mod
index fc0730d..7ac23dc 100644
--- a/go.mod
+++ b/go.mod
@@ -15,6 +15,7 @@ require (
 	github.com/sirupsen/logrus v1.9.0
 	github.com/stretchr/testify v1.8.4
 	github.com/xeipuuv/gojsonschema v1.2.0
+	golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1
 	golang.org/x/sync v0.3.0
 	gopkg.in/yaml.v3 v3.0.1
 	gotest.tools/v3 v3.4.0
diff --git a/go.sum b/go.sum
index 7d64833..55ff4f1 100644
--- a/go.sum
+++ b/go.sum
@@ -41,6 +41,8 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 h1:MGwJjxBy0HJshjDNfLsYO8xppfqWlA5ZT9OhtUUhTNw=
+golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
diff --git a/types/project.go b/types/project.go
index bf5b8a7..42c2471 100644
--- a/types/project.go
+++ b/types/project.go
@@ -24,6 +24,8 @@ import (
 	"path/filepath"
 	"sort"
 
+	"github.com/compose-spec/compose-go/utils"
+
 	"github.com/compose-spec/compose-go/dotenv"
 	"github.com/distribution/distribution/v3/reference"
 	godigest "github.com/opencontainers/go-digest"
@@ -102,10 +104,19 @@ func (p *Project) ConfigNames() []string {
 
 // GetServices retrieve services by names, or return all services if no name specified
 func (p *Project) GetServices(names ...string) (Services, error) {
+	services, servicesNotFound := p.getServicesByNames(names...)
+	if len(servicesNotFound) > 0 {
+		return services, fmt.Errorf("no such service: %s", servicesNotFound[0])
+	}
+	return services, nil
+}
+
+func (p *Project) getServicesByNames(names ...string) (Services, []string) {
 	if len(names) == 0 {
 		return p.Services, nil
 	}
 	services := Services{}
+	var servicesNotFound []string
 	for _, name := range names {
 		var serviceConfig *ServiceConfig
 		for _, s := range p.Services {
@@ -115,11 +126,12 @@ func (p *Project) GetServices(names ...string) (Services, error) {
 			}
 		}
 		if serviceConfig == nil {
-			return services, fmt.Errorf("no such service: %s", name)
+			servicesNotFound = append(servicesNotFound, name)
+			continue
 		}
 		services = append(services, *serviceConfig)
 	}
-	return services, nil
+	return services, servicesNotFound
 }
 
 // GetDisabledService retrieve disabled service by name
@@ -159,26 +171,30 @@ func (p *Project) WithServices(names []string, fn ServiceFunc, options ...Depend
 		// backward compatibility
 		options = []DependencyOption{IncludeDependencies}
 	}
-	return p.withServices(names, fn, map[string]bool{}, options)
+	return p.withServices(names, fn, map[string]bool{}, options, map[string]ServiceDependency{})
 }
 
-func (p *Project) withServices(names []string, fn ServiceFunc, seen map[string]bool, options []DependencyOption) error {
-	services, err := p.GetServices(names...)
-	if err != nil {
-		return err
+func (p *Project) withServices(names []string, fn ServiceFunc, seen map[string]bool, options []DependencyOption, dependencies map[string]ServiceDependency) error {
+	services, servicesNotFound := p.getServicesByNames(names...)
+	if len(servicesNotFound) > 0 {
+		for _, serviceNotFound := range servicesNotFound {
+			if dependency, ok := dependencies[serviceNotFound]; !ok || dependency.Required {
+				return fmt.Errorf("no such service: %s", serviceNotFound)
+			}
+		}
 	}
 	for _, service := range services {
 		if seen[service.Name] {
 			continue
 		}
 		seen[service.Name] = true
-		var dependencies []string
+		var dependencies map[string]ServiceDependency
 		for _, policy := range options {
 			switch policy {
 			case IncludeDependents:
-				dependencies = append(dependencies, p.GetDependentsForService(service)...)
+				dependencies = utils.MapsAppend(dependencies, p.dependentsForService(service))
 			case IncludeDependencies:
-				dependencies = append(dependencies, service.GetDependencies()...)
+				dependencies = utils.MapsAppend(dependencies, service.DependsOn)
 			case IgnoreDependencies:
 				// Noop
 			default:
@@ -186,7 +202,7 @@ func (p *Project) withServices(names []string, fn ServiceFunc, seen map[string]b
 			}
 		}
 		if len(dependencies) > 0 {
-			err := p.withServices(dependencies, fn, seen, options)
+			err := p.withServices(utils.MapKeys(dependencies), fn, seen, options, dependencies)
 			if err != nil {
 				return err
 			}
@@ -199,11 +215,15 @@ func (p *Project) withServices(names []string, fn ServiceFunc, seen map[string]b
 }
 
 func (p *Project) GetDependentsForService(s ServiceConfig) []string {
-	var dependent []string
+	return utils.MapKeys(p.dependentsForService(s))
+}
+
+func (p *Project) dependentsForService(s ServiceConfig) map[string]ServiceDependency {
+	dependent := make(map[string]ServiceDependency)
 	for _, service := range p.Services {
-		for name := range service.DependsOn {
+		for name, dependency := range service.DependsOn {
 			if name == s.Name {
-				dependent = append(dependent, service.Name)
+				dependent[service.Name] = dependency
 			}
 		}
 	}
diff --git a/utils/collectionutils.go b/utils/collectionutils.go
new file mode 100644
index 0000000..3436922
--- /dev/null
+++ b/utils/collectionutils.go
@@ -0,0 +1,51 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package utils
+
+import "golang.org/x/exp/slices"
+
+func MapKeys[T comparable, U any](theMap map[T]U) []T {
+	var result []T
+	for key := range theMap {
+		result = append(result, key)
+	}
+	return result
+}
+
+func MapsAppend[T comparable, U any](target map[T]U, source map[T]U) map[T]U {
+	if target == nil {
+		return source
+	}
+	if source == nil {
+		return target
+	}
+	for key, value := range source {
+		if _, ok := target[key]; !ok {
+			target[key] = value
+		}
+	}
+	return target
+}
+
+func ArrayContains[T comparable](source []T, toCheck []T) bool {
+	for _, value := range toCheck {
+		if !slices.Contains(source, value) {
+			return false
+		}
+	}
+	return true
+}


Test patch:
diff --git a/types/config_test.go b/types/config_test.go
index 7743bf4..239b86c 100644
--- a/types/config_test.go
+++ b/types/config_test.go
@@ -30,6 +30,7 @@ func Test_WithServices(t *testing.T) {
 				DependsOn: map[string]ServiceDependency{
 					"service_3": {
 						Condition: ServiceConditionStarted,
+						Required:  true,
 					},
 				},
 			}, ServiceConfig{
@@ -39,6 +40,7 @@ func Test_WithServices(t *testing.T) {
 				DependsOn: map[string]ServiceDependency{
 					"service_2": {
 						Condition: ServiceConditionStarted,
+						Required:  true,
 					},
 				},
 			}),
diff --git a/types/project_test.go b/types/project_test.go
index b8421bc..bef04b3 100644
--- a/types/project_test.go
+++ b/types/project_test.go
@@ -20,6 +20,7 @@ import (
 	_ "crypto/sha256"
 	"testing"
 
+	"github.com/compose-spec/compose-go/utils"
 	"github.com/distribution/distribution/v3/reference"
 	"github.com/opencontainers/go-digest"
 	"gotest.tools/v3/assert"
@@ -114,14 +115,15 @@ func makeProject() Project {
 			}, ServiceConfig{
 				Name:      "service_2",
 				Profiles:  []string{"foo"},
-				DependsOn: map[string]ServiceDependency{"service_1": {}},
+				DependsOn: map[string]ServiceDependency{"service_1": {Required: true}},
 			}, ServiceConfig{
 				Name:      "service_3",
 				Profiles:  []string{"bar"},
-				DependsOn: map[string]ServiceDependency{"service_2": {}},
+				DependsOn: map[string]ServiceDependency{"service_2": {Required: true}},
 			}, ServiceConfig{
-				Name:     "service_4",
-				Profiles: []string{"zot"},
+				Name:      "service_4",
+				Profiles:  []string{"zot"},
+				DependsOn: map[string]ServiceDependency{"service_2": {Required: false}},
 			}, ServiceConfig{
 				Name:     "service_5",
 				Profiles: []string{"zot"},
@@ -189,7 +191,8 @@ func TestWithServices(t *testing.T) {
 		return nil
 	}, IncludeDependents)
 	assert.NilError(t, err)
-	assert.DeepEqual(t, seen, []string{"service_3", "service_2", "service_1"})
+	// Order of service_3 and service_4 may change because there both depending on service_2
+	assert.Check(t, utils.ArrayContains(seen, []string{"service_3", "service_4", "service_2", "service_1"}))
 
 	seen = []string{}
 	err = p.WithServices([]string{"service_1"}, func(service ServiceConfig) error {
@@ -198,4 +201,12 @@ func TestWithServices(t *testing.T) {
 	}, IgnoreDependencies)
 	assert.NilError(t, err)
 	assert.DeepEqual(t, seen, []string{"service_1"})
+
+	seen = []string{}
+	err = p.WithServices([]string{"service_4"}, func(service ServiceConfig) error {
+		seen = append(seen, service.Name)
+		return nil
+	}, IncludeDependencies)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, seen, []string{"service_1", "service_2", "service_4"})
 }