Repository: compose-spec/compose-go
Pull request #547 (2024-01-24 14:19:31)

process `extends` recursively
fixes https://github.com/docker/compose/issues/11394

issue takes place when there's a chain of extending services:
```yaml
services:
  level1:
    build:
      context: ..

  level2:
    extends: level1

  service:
    extends: level2
    build:
      target: target
```

The root cause for this issue is that we try to extends a service before the compose model has been fully resolved. Typically loading this illustration example, if first service (which go map iteration makes random) to be processed is `service`, it will merge with sibling `level2`. This results in a `build` mapping defined without a `context`, and will default to `.`. But if `level2` is loaded first, it will have `build.context` set to `..`, and then `service` will do as well.

This PR changes the logic so that we force `extends` to be applied recursively on services using new func `applyServiceExtends`. This one will process a service to apply `extends` if needed (otherwise is no-op). Once it has identified the base service used by extends, it executes recursively to enforce this one to be fully defined


Patch:
diff --git a/Makefile b/Makefile
index e31b823..437fd5b 100644
--- a/Makefile
+++ b/Makefile
@@ -15,8 +15,8 @@
 IMAGE_PREFIX=composespec/conformance-tests-
 
 .PHONY: build
-build: ## Run tests
-	go build ./...
+build: ## Build command line
+	go build -o compose-spec cmd/main.go
 
 .PHONY: test
 test: ## Run tests
diff --git a/loader/extends.go b/loader/extends.go
index 97c6ca3..3982cd6 100644
--- a/loader/extends.go
+++ b/loader/extends.go
@@ -35,96 +35,125 @@ func ApplyExtends(ctx context.Context, dict map[string]any, opts *Options, track
 	if !ok {
 		return fmt.Errorf("services must be a mapping")
 	}
-	for name, s := range services {
-		service, ok := s.(map[string]any)
-		if !ok {
-			return fmt.Errorf("services.%s must be a mapping", name)
-		}
-		x, ok := service["extends"]
-		if !ok {
-			continue
-		}
-		ct, err := tracker.Add(ctx.Value(consts.ComposeFileKey{}).(string), name)
+	for name := range services {
+		merged, err := applyServiceExtends(ctx, name, services, opts, tracker, post...)
 		if err != nil {
 			return err
 		}
-		var (
-			ref  string
-			file any
-		)
-		switch v := x.(type) {
-		case map[string]any:
-			ref = v["service"].(string)
-			file = v["file"]
-		case string:
-			ref = v
+		services[name] = merged
+	}
+	dict["services"] = services
+	return nil
+}
+
+func applyServiceExtends(ctx context.Context, name string, services map[string]any, opts *Options, tracker *cycleTracker, post ...PostProcessor) (any, error) {
+	s := services[name]
+	if s == nil {
+		return nil, nil
+	}
+	service, ok := s.(map[string]any)
+	if !ok {
+		return nil, fmt.Errorf("services.%s must be a mapping", name)
+	}
+	extends, ok := service["extends"]
+	if !ok {
+		return s, nil
+	}
+	filename := ctx.Value(consts.ComposeFileKey{}).(string)
+	tracker, err := tracker.Add(filename, name)
+	if err != nil {
+		return nil, err
+	}
+	var (
+		ref  string
+		file any
+	)
+	switch v := extends.(type) {
+	case map[string]any:
+		ref = v["service"].(string)
+		file = v["file"]
+	case string:
+		ref = v
+	}
+
+	var base any
+	if file != nil {
+		path := file.(string)
+		services, err = getExtendsBaseFromFile(ctx, ref, path, opts, tracker)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		_, ok := services[ref]
+		if !ok {
+			return nil, fmt.Errorf("cannot extend service %q in %s: service not found", name, filename)
 		}
+	}
+	// recursively apply `extends`
+	base, err = applyServiceExtends(ctx, ref, services, opts, tracker, post...)
+	if err != nil {
+		return nil, err
+	}
 
-		var base any
-		if file != nil {
-			path := file.(string)
-			for _, loader := range opts.ResourceLoaders {
-				if !loader.Accept(path) {
-					continue
-				}
-				local, err := loader.Load(ctx, path)
-				if err != nil {
-					return err
-				}
-				localdir := filepath.Dir(local)
-				relworkingdir := loader.Dir(path)
+	if base == nil {
+		return service, nil
+	}
+	source := deepClone(base).(map[string]any)
+	for _, processor := range post {
+		processor.Apply(map[string]any{
+			"services": map[string]any{
+				name: source,
+			},
+		})
+	}
+	merged, err := override.ExtendService(source, service)
+	if err != nil {
+		return nil, err
+	}
+	delete(merged, "extends")
+	return merged, nil
+}
 
-				extendsOpts := opts.clone()
-				extendsOpts.ResourceLoaders = append([]ResourceLoader{}, opts.ResourceLoaders...)
-				// replace localResourceLoader with a new flavour, using extended file base path
-				extendsOpts.ResourceLoaders[len(opts.ResourceLoaders)-1] = localResourceLoader{
-					WorkingDir: localdir,
-				}
-				extendsOpts.ResolvePaths = true
-				extendsOpts.SkipNormalization = true
-				extendsOpts.SkipConsistencyCheck = true
-				extendsOpts.SkipInclude = true
-				source, err := loadYamlModel(ctx, types.ConfigDetails{
-					WorkingDir: relworkingdir,
-					ConfigFiles: []types.ConfigFile{
-						{Filename: local},
-					},
-				}, extendsOpts, ct, nil)
-				if err != nil {
-					return err
-				}
-				services := source["services"].(map[string]any)
-				base, ok = services[ref]
-				if !ok {
-					return fmt.Errorf("cannot extend service %q in %s: service not found", name, path)
-				}
-			}
-			if base == nil {
-				return fmt.Errorf("cannot read %s", path)
-			}
-		} else {
-			base, ok = services[ref]
-			if !ok {
-				return fmt.Errorf("cannot extend service %q in %s: service not found", name, "filename") // TODO track filename
-			}
+func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts *Options, ct *cycleTracker) (map[string]any, error) {
+	for _, loader := range opts.ResourceLoaders {
+		if !loader.Accept(path) {
+			continue
 		}
-		source := deepClone(base).(map[string]any)
-		for _, processor := range post {
-			processor.Apply(map[string]any{
-				"services": map[string]any{
-					name: source,
-				},
-			})
+		local, err := loader.Load(ctx, path)
+		if err != nil {
+			return nil, err
 		}
-		merged, err := override.ExtendService(source, service)
+		localdir := filepath.Dir(local)
+		relworkingdir := loader.Dir(path)
+
+		extendsOpts := opts.clone()
+		// replace localResourceLoader with a new flavour, using extended file base path
+		extendsOpts.ResourceLoaders = append(opts.RemoteResourceLoaders(), localResourceLoader{
+			WorkingDir: localdir,
+		})
+		extendsOpts.ResolvePaths = true
+		extendsOpts.SkipNormalization = true
+		extendsOpts.SkipConsistencyCheck = true
+		extendsOpts.SkipInclude = true
+		extendsOpts.SkipExtends = true    // we manage extends recursively based on raw service definition
+		extendsOpts.SkipValidation = true // we validate the merge result
+		source, err := loadYamlModel(ctx, types.ConfigDetails{
+			WorkingDir: relworkingdir,
+			ConfigFiles: []types.ConfigFile{
+				{Filename: local},
+			},
+		}, extendsOpts, ct, nil)
 		if err != nil {
-			return err
+			return nil, err
 		}
-		delete(merged, "extends")
-		services[name] = merged
+		services := source["services"].(map[string]any)
+		_, ok := services[name]
+		if !ok {
+			return nil, fmt.Errorf("cannot extend service %q in %s: service not found", name, path)
+		}
+		return services, nil
 	}
-	dict["services"] = services
-	return nil
+	return nil, fmt.Errorf("cannot read %s", path)
 }
 
 func deepClone(value any) any {
diff --git a/loader/loader.go b/loader/loader.go
index 5100d9f..e1aa766 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -40,6 +40,7 @@ import (
 	"github.com/compose-spec/compose-go/v2/types"
 	"github.com/compose-spec/compose-go/v2/validation"
 	"github.com/mitchellh/mapstructure"
+	"github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
 )
 
@@ -87,6 +88,21 @@ type ResourceLoader interface {
 	Dir(path string) string
 }
 
+// RemoteResourceLoaders excludes localResourceLoader from ResourceLoaders
+func (o Options) RemoteResourceLoaders() []ResourceLoader {
+	var loaders []ResourceLoader
+	for i, loader := range o.ResourceLoaders {
+		if _, ok := loader.(localResourceLoader); ok {
+			if i != len(o.ResourceLoaders)-1 {
+				logrus.Warning("misconfiguration of ResourceLoaders: localResourceLoader should be last")
+			}
+			continue
+		}
+		loaders = append(loaders, loader)
+	}
+	return loaders
+}
+
 type localResourceLoader struct {
 	WorkingDir string
 }
@@ -399,7 +415,11 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option
 	}
 
 	if opts.ResolvePaths {
-		err = paths.ResolveRelativePaths(dict, config.WorkingDir)
+		var remotes []paths.RemoteResource
+		for _, loader := range opts.RemoteResourceLoaders() {
+			remotes = append(remotes, loader.Accept)
+		}
+		err = paths.ResolveRelativePaths(dict, config.WorkingDir, remotes)
 		if err != nil {
 			return nil, err
 		}
diff --git a/paths/extends.go b/paths/extends.go
new file mode 100644
index 0000000..aa61a9f
--- /dev/null
+++ b/paths/extends.go
@@ -0,0 +1,25 @@
+/*
+   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 paths
+
+func (r *relativePathsResolver) absExtendsPath(value any) (any, error) {
+	v := value.(string)
+	if r.isRemoteResource(v) {
+		return v, nil
+	}
+	return r.absPath(v)
+}
diff --git a/paths/resolve.go b/paths/resolve.go
index d7e100f..ecfa0e9 100644
--- a/paths/resolve.go
+++ b/paths/resolve.go
@@ -28,13 +28,16 @@ import (
 type resolver func(any) (any, error)
 
 // ResolveRelativePaths make relative paths absolute
-func ResolveRelativePaths(project map[string]any, base string) error {
-	r := relativePathsResolver{workingDir: base}
+func ResolveRelativePaths(project map[string]any, base string, remotes []RemoteResource) error {
+	r := relativePathsResolver{
+		workingDir: base,
+		remotes:    remotes,
+	}
 	r.resolvers = map[tree.Path]resolver{
 		"services.*.build.context":               r.absContextPath,
 		"services.*.build.additional_contexts.*": r.absContextPath,
 		"services.*.env_file.*.path":             r.absPath,
-		"services.*.extends.file":                r.absPath,
+		"services.*.extends.file":                r.absExtendsPath,
 		"services.*.develop.watch.*.path":        r.absPath,
 		"services.*.volumes.*":                   r.absVolumeMount,
 		"configs.*.file":                         r.maybeUnixPath,
@@ -48,11 +51,23 @@ func ResolveRelativePaths(project map[string]any, base string) error {
 	return err
 }
 
+type RemoteResource func(path string) bool
+
 type relativePathsResolver struct {
 	workingDir string
+	remotes    []RemoteResource
 	resolvers  map[tree.Path]resolver
 }
 
+func (r *relativePathsResolver) isRemoteResource(path string) bool {
+	for _, remote := range r.remotes {
+		if remote(path) {
+			return true
+		}
+	}
+	return false
+}
+
 func (r *relativePathsResolver) resolveRelativePaths(value any, p tree.Path) (any, error) {
 	for pattern, resolver := range r.resolvers {
 		if p.Matches(pattern) {
diff --git a/transform/services.go b/transform/services.go
index 5a290ef..c141145 100644
--- a/transform/services.go
+++ b/transform/services.go
@@ -21,8 +21,12 @@ import (
 )
 
 func transformService(data any, p tree.Path) (any, error) {
-	value := data.(map[string]any)
-	return transformMapping(value, p)
+	switch value := data.(type) {
+	case map[string]any:
+		return transformMapping(value, p)
+	default:
+		return value, nil
+	}
 }
 
 func transformServiceNetworks(data any, _ tree.Path) (any, error) {


Test patch:
diff --git a/loader/extends_test.go b/loader/extends_test.go
index e5bafaa..4c0686c 100644
--- a/loader/extends_test.go
+++ b/loader/extends_test.go
@@ -142,3 +142,31 @@ services:
 	assert.NilError(t, err)
 	assert.Equal(t, p.Services["test"].Build.Context, filepath.Join("testdata", "extends"))
 }
+
+func TestExtendsNil(t *testing.T) {
+	yaml := `
+name: test-extends-port
+services:
+  test:
+    image: test
+    extends:
+      file: testdata/extends/base.yaml
+      service: nil
+`
+	abs, err := filepath.Abs(".")
+	assert.NilError(t, err)
+
+	_, err = LoadWithContext(context.Background(), types.ConfigDetails{
+		ConfigFiles: []types.ConfigFile{
+			{
+				Content:  []byte(yaml),
+				Filename: "(inline)",
+			},
+		},
+		WorkingDir: abs,
+	}, func(options *Options) {
+		options.ResolvePaths = false
+		options.SkipValidation = true
+	})
+	assert.NilError(t, err)
+}
diff --git a/loader/testdata/extends/base.yaml b/loader/testdata/extends/base.yaml
index 3c2f693..3886659 100644
--- a/loader/testdata/extends/base.yaml
+++ b/loader/testdata/extends/base.yaml
@@ -22,3 +22,4 @@ services:
       file: sibling.yaml
       service: test
 
+  nil: #left intentionally empty