Repository: compose-spec/compose-go
Pull request #648 (2024-06-27 09:13:44)

Reset extends
This introduces `loadYamlFile` to load a single compose file, running _most_ of the processing. 
`!reset` tags collected by a post-processor is returned to caller. It is used when running the recursive extends processing outside the main loading-and-merge pipeline

fixes https://github.com/docker/compose/issues/11937

jhrotko: that's a big change! (nice) did you try running compose e2e tests against this branch?
ndeloof: > that's a big change! 

actually, I "just" extracted a long block into a new function. Github code review doesn't make this easy to notice

tested on https://github.com/docker/compose/pull/11950

Patch:
diff --git a/loader/extends.go b/loader/extends.go
index cdfed50..67d3387 100644
--- a/loader/extends.go
+++ b/loader/extends.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/compose-spec/compose-go/v2/consts"
 	"github.com/compose-spec/compose-go/v2/override"
+	"github.com/compose-spec/compose-go/v2/paths"
 	"github.com/compose-spec/compose-go/v2/types"
 )
 
@@ -75,10 +76,15 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
 		opts.ProcessEvent("extends", map[string]any{"service": ref})
 	}
 
-	var base any
+	var (
+		base      any
+		processor PostProcessor
+	)
+
 	if file != nil {
 		filename = file.(string)
-		services, err = getExtendsBaseFromFile(ctx, ref, filename, opts, tracker)
+		services, processor, err = getExtendsBaseFromFile(ctx, ref, filename, opts, tracker)
+		post = append(post, processor)
 		if err != nil {
 			return nil, err
 		}
@@ -121,14 +127,14 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
 	return merged, nil
 }
 
-func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts *Options, ct *cycleTracker) (map[string]any, error) {
+func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts *Options, ct *cycleTracker) (map[string]any, PostProcessor, error) {
 	for _, loader := range opts.ResourceLoaders {
 		if !loader.Accept(path) {
 			continue
 		}
 		local, err := loader.Load(ctx, path)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		localdir := filepath.Dir(local)
 		relworkingdir := loader.Dir(path)
@@ -138,30 +144,36 @@ func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts
 		extendsOpts.ResourceLoaders = append(opts.RemoteResourceLoaders(), localResourceLoader{
 			WorkingDir: localdir,
 		})
-		extendsOpts.ResolvePaths = true
+		extendsOpts.ResolvePaths = false // we do relative path resolution after file has been loaded
 		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
 		extendsOpts.SkipDefaultValues = true
-		source, err := loadYamlModel(ctx, types.ConfigDetails{
-			WorkingDir: relworkingdir,
-			ConfigFiles: []types.ConfigFile{
-				{Filename: local},
-			},
-		}, extendsOpts, ct, nil)
+		source, processor, err := loadYamlFile(ctx, types.ConfigFile{Filename: local},
+			extendsOpts, relworkingdir, nil, ct, map[string]any{}, nil)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		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 nil, nil, fmt.Errorf("cannot extend service %q in %s: service not found", name, path)
 		}
-		return services, nil
+
+		var remotes []paths.RemoteResource
+		for _, loader := range opts.RemoteResourceLoaders() {
+			remotes = append(remotes, loader.Accept)
+		}
+		err = paths.ResolveRelativePaths(source, relworkingdir, remotes)
+		if err != nil {
+			return nil, nil, err
+		}
+
+		return services, processor, nil
 	}
-	return nil, fmt.Errorf("cannot read %s", path)
+	return nil, nil, fmt.Errorf("cannot read %s", path)
 }
 
 func deepClone(value any) any {
diff --git a/loader/include.go b/loader/include.go
index d40b24e..823c2f7 100644
--- a/loader/include.go
+++ b/loader/include.go
@@ -50,16 +50,17 @@ func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {
 	return requires, err
 }
 
-func ApplyInclude(ctx context.Context, configDetails types.ConfigDetails, model map[string]any, options *Options, included []string) error {
+func ApplyInclude(ctx context.Context, workingDir string, environment types.Mapping, model map[string]any, options *Options, included []string) error {
 	includeConfig, err := loadIncludeConfig(model["include"])
 	if err != nil {
 		return err
 	}
+
 	for _, r := range includeConfig {
 		for _, listener := range options.Listeners {
 			listener("include", map[string]any{
 				"path":       r.Path,
-				"workingdir": configDetails.WorkingDir,
+				"workingdir": workingDir,
 			})
 		}
 
@@ -83,7 +84,7 @@ func ApplyInclude(ctx context.Context, configDetails types.ConfigDetails, model
 						r.ProjectDirectory = filepath.Dir(path)
 					case !filepath.IsAbs(r.ProjectDirectory):
 						relworkingdir = loader.Dir(r.ProjectDirectory)
-						r.ProjectDirectory = filepath.Join(configDetails.WorkingDir, r.ProjectDirectory)
+						r.ProjectDirectory = filepath.Join(workingDir, r.ProjectDirectory)
 
 					default:
 						relworkingdir = r.ProjectDirectory
@@ -117,7 +118,7 @@ func ApplyInclude(ctx context.Context, configDetails types.ConfigDetails, model
 			envFile := []string{}
 			for _, f := range r.EnvFile {
 				if !filepath.IsAbs(f) {
-					f = filepath.Join(configDetails.WorkingDir, f)
+					f = filepath.Join(workingDir, f)
 					s, err := os.Stat(f)
 					if err != nil {
 						return err
@@ -131,7 +132,7 @@ func ApplyInclude(ctx context.Context, configDetails types.ConfigDetails, model
 			r.EnvFile = envFile
 		}
 
-		envFromFile, err := dotenv.GetEnvFromFile(configDetails.Environment, r.EnvFile)
+		envFromFile, err := dotenv.GetEnvFromFile(environment, r.EnvFile)
 		if err != nil {
 			return err
 		}
@@ -139,7 +140,7 @@ func ApplyInclude(ctx context.Context, configDetails types.ConfigDetails, model
 		config := types.ConfigDetails{
 			WorkingDir:  relworkingdir,
 			ConfigFiles: types.ToConfigFiles(r.Path),
-			Environment: configDetails.Environment.Clone().Merge(envFromFile),
+			Environment: environment.Clone().Merge(envFromFile),
 		}
 		loadOptions.Interpolate = &interp.Options{
 			Substitute:      options.Interpolate.Substitute,
diff --git a/loader/loader.go b/loader/loader.go
index 2556568..11317c5 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -358,140 +358,148 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option
 		dict = map[string]interface{}{}
 		err  error
 	)
+	workingDir, environment := config.WorkingDir, config.Environment
+
 	for _, file := range config.ConfigFiles {
-		fctx := context.WithValue(ctx, consts.ComposeFileKey{}, file.Filename)
-		if file.Content == nil && file.Config == nil {
-			content, err := os.ReadFile(file.Filename)
-			if err != nil {
-				return nil, err
-			}
-			file.Content = content
+		dict, _, err = loadYamlFile(ctx, file, opts, workingDir, environment, ct, dict, included)
+		if err != nil {
+			return nil, err
 		}
+	}
 
-		processRawYaml := func(raw interface{}, processors ...PostProcessor) error {
-			converted, err := convertToStringKeysRecursive(raw, "")
-			if err != nil {
-				return err
-			}
-			cfg, ok := converted.(map[string]interface{})
-			if !ok {
-				return errors.New("Top-level object must be a mapping")
-			}
+	if !opts.SkipDefaultValues {
+		dict, err = transform.SetDefaultValues(dict)
+		if err != nil {
+			return nil, err
+		}
+	}
 
-			if opts.Interpolate != nil && !opts.SkipInterpolation {
-				cfg, err = interp.Interpolate(cfg, *opts.Interpolate)
-				if err != nil {
-					return err
-				}
-			}
+	if !opts.SkipValidation {
+		if err := validation.Validate(dict); err != nil {
+			return nil, err
+		}
+	}
 
-			fixEmptyNotNull(cfg)
+	if opts.ResolvePaths {
+		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
+		}
+	}
+	resolveServicesEnvironment(dict, config)
 
-			if !opts.SkipExtends {
-				err = ApplyExtends(fctx, cfg, opts, ct, processors...)
-				if err != nil {
-					return err
-				}
-			}
+	return dict, nil
+}
 
-			for _, processor := range processors {
-				if err := processor.Apply(dict); err != nil {
-					return err
-				}
-			}
+func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, workingDir string, environment types.Mapping, ct *cycleTracker, dict map[string]interface{}, included []string) (map[string]interface{}, PostProcessor, error) {
+	ctx = context.WithValue(ctx, consts.ComposeFileKey{}, file.Filename)
+	if file.Content == nil && file.Config == nil {
+		content, err := os.ReadFile(file.Filename)
+		if err != nil {
+			return nil, nil, err
+		}
+		file.Content = content
+	}
 
-			if !opts.SkipInclude {
-				included = append(included, config.ConfigFiles[0].Filename)
-				err = ApplyInclude(ctx, config, cfg, opts, included)
-				if err != nil {
-					return err
-				}
-			}
+	processRawYaml := func(raw interface{}, processors ...PostProcessor) error {
+		converted, err := convertToStringKeysRecursive(raw, "")
+		if err != nil {
+			return err
+		}
+		cfg, ok := converted.(map[string]interface{})
+		if !ok {
+			return errors.New("Top-level object must be a mapping")
+		}
 
-			dict, err = override.Merge(dict, cfg)
+		if opts.Interpolate != nil && !opts.SkipInterpolation {
+			cfg, err = interp.Interpolate(cfg, *opts.Interpolate)
 			if err != nil {
 				return err
 			}
+		}
 
-			dict, err = override.EnforceUnicity(dict)
+		fixEmptyNotNull(cfg)
+
+		if !opts.SkipExtends {
+			err = ApplyExtends(ctx, cfg, opts, ct, processors...)
 			if err != nil {
 				return err
 			}
+		}
 
-			if !opts.SkipValidation {
-				if err := schema.Validate(dict); err != nil {
-					return fmt.Errorf("validating %s: %w", file.Filename, err)
-				}
-				if _, ok := dict["version"]; ok {
-					opts.warnObsoleteVersion(file.Filename)
-					delete(dict, "version")
-				}
+		for _, processor := range processors {
+			if err := processor.Apply(dict); err != nil {
+				return err
+			}
+		}
+
+		if !opts.SkipInclude {
+			included = append(included, file.Filename)
+			err = ApplyInclude(ctx, workingDir, environment, cfg, opts, included)
+			if err != nil {
+				return err
 			}
+		}
 
+		dict, err = override.Merge(dict, cfg)
+		if err != nil {
 			return err
 		}
 
-		if file.Config == nil {
-			r := bytes.NewReader(file.Content)
-			decoder := yaml.NewDecoder(r)
-			for {
-				var raw interface{}
-				processor := &ResetProcessor{target: &raw}
-				err := decoder.Decode(processor)
-				if err != nil && errors.Is(err, io.EOF) {
-					break
-				}
-				if err != nil {
-					return nil, err
-				}
-				if err := processRawYaml(raw, processor); err != nil {
-					return nil, err
-				}
+		dict, err = override.EnforceUnicity(dict)
+		if err != nil {
+			return err
+		}
+
+		if !opts.SkipValidation {
+			if err := schema.Validate(dict); err != nil {
+				return fmt.Errorf("validating %s: %w", file.Filename, err)
 			}
-		} else {
-			if err := processRawYaml(file.Config); err != nil {
-				return nil, err
+			if _, ok := dict["version"]; ok {
+				opts.warnObsoleteVersion(file.Filename)
+				delete(dict, "version")
 			}
 		}
-	}
-
-	dict, err = transform.Canonical(dict, opts.SkipInterpolation)
-	if err != nil {
-		return nil, err
-	}
 
-	// Canonical transformation can reveal duplicates, typically as ports can be a range and conflict with an override
-	dict, err = override.EnforceUnicity(dict)
-	if err != nil {
-		return nil, err
-	}
-
-	if !opts.SkipDefaultValues {
-		dict, err = transform.SetDefaultValues(dict)
+		dict, err = transform.Canonical(dict, opts.SkipInterpolation)
 		if err != nil {
-			return nil, err
+			return err
 		}
-	}
 
-	if !opts.SkipValidation {
-		if err := validation.Validate(dict); err != nil {
-			return nil, err
-		}
+		// Canonical transformation can reveal duplicates, typically as ports can be a range and conflict with an override
+		dict, err = override.EnforceUnicity(dict)
+		return err
 	}
 
-	if opts.ResolvePaths {
-		var remotes []paths.RemoteResource
-		for _, loader := range opts.RemoteResourceLoaders() {
-			remotes = append(remotes, loader.Accept)
+	var processor PostProcessor
+	if file.Config == nil {
+		r := bytes.NewReader(file.Content)
+		decoder := yaml.NewDecoder(r)
+		for {
+			var raw interface{}
+			reset := &ResetProcessor{target: &raw}
+			err := decoder.Decode(reset)
+			if err != nil && errors.Is(err, io.EOF) {
+				break
+			}
+			if err != nil {
+				return nil, nil, err
+			}
+			processor = reset
+			if err := processRawYaml(raw, processor); err != nil {
+				return nil, nil, err
+			}
 		}
-		err = paths.ResolveRelativePaths(dict, config.WorkingDir, remotes)
-		if err != nil {
-			return nil, err
+	} else {
+		if err := processRawYaml(file.Config); err != nil {
+			return nil, nil, err
 		}
 	}
-	resolveServicesEnvironment(dict, config)
-
-	return dict, nil
+	return dict, processor, nil
 }
 
 func load(ctx context.Context, configDetails types.ConfigDetails, opts *Options, loaded []string) (map[string]interface{}, error) {


Test patch:
diff --git a/loader/extends_test.go b/loader/extends_test.go
index 22900e9..3990b3d 100644
--- a/loader/extends_test.go
+++ b/loader/extends_test.go
@@ -465,3 +465,22 @@ services:
 	assert.NilError(t, err)
 	assert.Equal(t, extendsCount, 2)
 }
+
+func TestExtendsReset(t *testing.T) {
+	yaml := `
+name: test-extends-reset
+services:
+  test:
+    extends:
+      file: testdata/extends/reset.yaml
+      service: base
+`
+	p, err := Load(types.ConfigDetails{
+		ConfigFiles: []types.ConfigFile{{
+			Content:  []byte(yaml),
+			Filename: "-",
+		}},
+	})
+	assert.NilError(t, err)
+	assert.Check(t, p.Services["test"].Command == nil)
+}
diff --git a/loader/testdata/extends/reset.yaml b/loader/testdata/extends/reset.yaml
new file mode 100644
index 0000000..c1b9ee2
--- /dev/null
+++ b/loader/testdata/extends/reset.yaml
@@ -0,0 +1,7 @@
+services:
+  init:
+    image: alpine:latest
+    command: "sleep infinity"
+  base:
+    extends: { service: init }
+    command: !reset
\ No newline at end of file