Repository: compose-spec/compose-go
Pull request #371 (2023-03-23 08:47:34)

remove `extends` from model after service has been extended
- Makes `ExtendsConfig` a struct (not sure why this was set as a MappingWithEquals, while the json schema is strict)
- Removes `extends` attribute from model after service has been extended, so that we return the actual service definition.
- Fix broken test on master 😅


Patch:
diff --git a/loader/loader.go b/loader/loader.go
index 3007a63..82c0abd 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -589,16 +589,17 @@ func loadServiceWithExtends(filename, name string, servicesDict map[string]inter
 	}
 
 	if serviceConfig.Extends != nil && !opts.SkipExtends {
-		baseServiceName := *serviceConfig.Extends["service"]
+		baseServiceName := serviceConfig.Extends.Service
 		var baseService *types.ServiceConfig
-		if file := serviceConfig.Extends["file"]; file == nil {
+		file := serviceConfig.Extends.File
+		if file == "" {
 			baseService, err = loadServiceWithExtends(filename, baseServiceName, servicesDict, workingDir, lookupEnv, opts, ct)
 			if err != nil {
 				return nil, err
 			}
 		} else {
 			// Resolve the path to the imported file, and load it.
-			baseFilePath := absPath(workingDir, *file)
+			baseFilePath := absPath(workingDir, file)
 
 			b, err := os.ReadFile(baseFilePath)
 			if err != nil {
@@ -617,10 +618,10 @@ func loadServiceWithExtends(filename, name string, servicesDict map[string]inter
 			}
 
 			// Make paths relative to the importing Compose file. Note that we
-			// make the paths relative to `*file` rather than `baseFilePath` so
-			// that the resulting paths won't be absolute if `*file` isn't an
+			// make the paths relative to `file` rather than `baseFilePath` so
+			// that the resulting paths won't be absolute if `file` isn't an
 			// absolute path.
-			baseFileParent := filepath.Dir(*file)
+			baseFileParent := filepath.Dir(file)
 			if baseService.Build != nil {
 				baseService.Build.Context = resolveBuildContextPath(baseFileParent, baseService.Build.Context)
 			}
@@ -641,6 +642,7 @@ func loadServiceWithExtends(filename, name string, servicesDict map[string]inter
 		if err != nil {
 			return nil, err
 		}
+		serviceConfig.Extends = nil
 	}
 
 	return serviceConfig, nil
@@ -1048,14 +1050,15 @@ var transformDependsOnConfig TransformerFunc = func(data interface{}) (interface
 	}
 }
 
-var transformExtendsConfig TransformerFunc = func(data interface{}) (interface{}, error) {
-	switch data.(type) {
+var transformExtendsConfig TransformerFunc = func(value interface{}) (interface{}, error) {
+	switch value.(type) {
 	case string:
-		data = map[string]interface{}{
-			"service": data,
-		}
+		return map[string]interface{}{"service": value}, nil
+	case map[string]interface{}:
+		return value, nil
+	default:
+		return value, errors.Errorf("invalid type %T for extends", value)
 	}
-	return transformMappingOrListFunc("=", true)(data)
 }
 
 var transformServiceVolumeConfig TransformerFunc = func(data interface{}) (interface{}, error) {
diff --git a/loader/normalize.go b/loader/normalize.go
index 0c869f3..3c9e1ad 100644
--- a/loader/normalize.go
+++ b/loader/normalize.go
@@ -91,9 +91,8 @@ func Normalize(project *types.Project, resolvePaths bool) error {
 		}
 		s.Environment = s.Environment.Resolve(fn)
 
-		if extendFile := s.Extends["file"]; extendFile != nil && *extendFile != "" {
-			p := absPath(project.WorkingDir, *extendFile)
-			s.Extends["file"] = &p
+		if s.Extends != nil && s.Extends.File != "" {
+			s.Extends.File = absPath(project.WorkingDir, s.Extends.File)
 		}
 
 		for _, link := range s.Links {
diff --git a/types/types.go b/types/types.go
index 9176b27..f739959 100644
--- a/types/types.go
+++ b/types/types.go
@@ -132,7 +132,7 @@ type ServiceConfig struct {
 	Environment     MappingWithEquals                `yaml:",omitempty" json:"environment,omitempty"`
 	EnvFile         StringList                       `mapstructure:"env_file" yaml:"env_file,omitempty" json:"env_file,omitempty"`
 	Expose          StringOrNumberList               `yaml:",omitempty" json:"expose,omitempty"`
-	Extends         ExtendsConfig                    `yaml:"extends,omitempty" json:"extends,omitempty"`
+	Extends         *ExtendsConfig                   `yaml:"extends,omitempty" json:"extends,omitempty"`
 	ExternalLinks   []string                         `mapstructure:"external_links" yaml:"external_links,omitempty" json:"external_links,omitempty"`
 	ExtraHosts      HostsList                        `mapstructure:"extra_hosts" yaml:"extra_hosts,omitempty" json:"extra_hosts,omitempty"`
 	GroupAdd        []string                         `mapstructure:"group_add" yaml:"group_add,omitempty" json:"group_add,omitempty"`
@@ -1008,7 +1008,10 @@ type ServiceDependency struct {
 	Extensions Extensions `mapstructure:"#extensions" yaml:",inline" json:"-"`
 }
 
-type ExtendsConfig MappingWithEquals
+type ExtendsConfig struct {
+	File    string `yaml:",omitempty" json:"file,omitempty"`
+	Service string `yaml:",omitempty" json:"service,omitempty"`
+}
 
 // SecretConfig for a secret
 type SecretConfig FileObjectConfig


Test patch:
diff --git a/loader/loader_test.go b/loader/loader_test.go
index dcc4885..2956d7e 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -1934,19 +1934,10 @@ func TestLoadWithExtends(t *testing.T) {
 
 	expectedEnvFilePath := filepath.Join(extendsDir, "extra.env")
 
-	expectedExtendsPath := filepath.Join(
-		extendsDir,
-		"compose-test-extends-imported.yaml",
-	)
-
 	expServices := types.Services{
 		{
 			Name:  "importer",
 			Image: "nginx",
-			Extends: types.ExtendsConfig{
-				"file":    &expectedExtendsPath,
-				"service": strPtr("imported"),
-			},
 			Environment: types.MappingWithEquals{
 				"SOURCE": strPtr("extends"),
 			},
@@ -1979,13 +1970,6 @@ func TestLoadWithExtendsWithContextUrl(t *testing.T) {
 	actual, err := Load(configDetails)
 	assert.NilError(t, err)
 
-	expectedExtendsPath, err := filepath.Abs(
-		filepath.Join(
-			"testdata",
-			"compose-test-extends-with-context-url-imported.yaml",
-		),
-	)
-	assert.NilError(t, err)
 	expServices := types.Services{
 		{
 			Name: "importer-with-https-url",
@@ -1993,10 +1977,6 @@ func TestLoadWithExtendsWithContextUrl(t *testing.T) {
 				Context:    "https://github.com/docker/compose.git",
 				Dockerfile: "Dockerfile",
 			},
-			Extends: types.ExtendsConfig{
-				"file":    &expectedExtendsPath,
-				"service": strPtr("imported-with-https-url"),
-			},
 			Environment: types.MappingWithEquals{},
 			Networks:    map[string]*types.ServiceNetworkConfig{"default": nil},
 			Scale:       1,
@@ -2304,6 +2284,7 @@ volumes:
 
 func TestLoadServiceExtension(t *testing.T) {
 	dict := `
+name: test
 services:
   extension: # this name should be allowed
     image: web
diff --git a/loader/testdata/subdir/compose-test-extends-imported.yaml b/loader/testdata/subdir/compose-test-extends-imported.yaml
index 2ee24e8..d56b82a 100644
--- a/loader/testdata/subdir/compose-test-extends-imported.yaml
+++ b/loader/testdata/subdir/compose-test-extends-imported.yaml
@@ -2,6 +2,6 @@ services:
   imported:
     image: nginx
     env_file:
-      - extra.env
+      - ./extra.env # expected to be loaded relative to this file, not the extending one
     volumes:
       - /opt/data:/var/lib/mysql