Repository: compose-spec/compose-go
Pull request #72 (2020-09-01 00:44:38)

Extends fixes
These are various fixes that I've made on my fork of compose-go.

I'd loved to get them merged upstream. I'm happy to write tests, make changes, etc -- I assume I'll need to make some changes. I just made these changes in the moment based on the path of least resistance.

The last commit in particular feels ugly to me, especially since it changes the public API.


Patch:
diff --git a/loader/loader.go b/loader/loader.go
index e174626..25399ef 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -52,6 +52,8 @@ type Options struct {
 	SkipNormalization bool
 	// Skip consistency check
 	SkipConsistencyCheck bool
+	// Skip extends
+	SkipExtends bool
 	// Interpolation options
 	Interpolate *interp.Options
 	// Discard 'env_file' entries after resolving to 'environment' section
@@ -60,6 +62,42 @@ type Options struct {
 	Name string
 }
 
+// serviceRef identifies a reference to a service. It's used to detect cyclic
+// references in "extends".
+type serviceRef struct {
+	filename string
+	service  string
+}
+
+type cycleTracker struct {
+	loaded []serviceRef
+}
+
+func (ct *cycleTracker) Add(filename, service string) error {
+	toAdd := serviceRef{filename: filename, service: service}
+	for _, loaded := range ct.loaded {
+		if toAdd == loaded {
+			// Create an error message of the form:
+			// Circular reference:
+			//   service-a in docker-compose.yml
+			//   extends service-b in docker-compose.yml
+			//   extends service-a in docker-compose.yml
+			errLines := []string{
+				"Circular reference:",
+				fmt.Sprintf("  %s in %s", ct.loaded[0].service, ct.loaded[0].filename),
+			}
+			for _, service := range append(ct.loaded[1:], toAdd) {
+				errLines = append(errLines, fmt.Sprintf("  extends %s in %s", service.service, service.filename))
+			}
+
+			return errors.New(strings.Join(errLines, "\n"))
+		}
+	}
+
+	ct.loaded = append(ct.loaded, toAdd)
+	return nil
+}
+
 // WithDiscardEnvFiles sets the Options to discard the `env_file` section after resolving to
 // the `environment` section
 func WithDiscardEnvFiles(opts *Options) {
@@ -122,11 +160,10 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
 
 		configDict = groupXFieldsIntoExtensions(configDict)
 
-		cfg, err := loadSections(configDict, configDetails)
+		cfg, err := loadSections(file.Filename, configDict, configDetails, opts)
 		if err != nil {
 			return nil, err
 		}
-		cfg.Filename = file.Filename
 		if opts.discardEnvFiles {
 			for i := range cfg.Services {
 				cfg.Services[i].EnvFile = nil
@@ -186,9 +223,11 @@ func groupXFieldsIntoExtensions(dict map[string]interface{}) map[string]interfac
 	return dict
 }
 
-func loadSections(config map[string]interface{}, configDetails types.ConfigDetails) (*types.Config, error) {
+func loadSections(filename string, config map[string]interface{}, configDetails types.ConfigDetails, opts *Options) (*types.Config, error) {
 	var err error
-	cfg := types.Config{}
+	cfg := types.Config{
+		Filename: filename,
+	}
 
 	var loaders = []struct {
 		key string
@@ -197,7 +236,7 @@ func loadSections(config map[string]interface{}, configDetails types.ConfigDetai
 		{
 			key: "services",
 			fnc: func(config map[string]interface{}) error {
-				cfg.Services, err = LoadServices(config, configDetails.WorkingDir, configDetails.LookupEnv)
+				cfg.Services, err = LoadServices(filename, config, configDetails.WorkingDir, configDetails.LookupEnv, opts)
 				return err
 			},
 		},
@@ -350,6 +389,7 @@ func createTransformHook(additionalTransformers ...Transformer) mapstructure.Dec
 		reflect.TypeOf(types.BuildConfig{}):                      transformBuildConfig,
 		reflect.TypeOf(types.Duration(0)):                        transformStringToDuration,
 		reflect.TypeOf(types.DependsOnConfig{}):                  transformDependsOnConfig,
+		reflect.TypeOf(types.ExtendsConfig{}):                    transformExtendsConfig,
 	}
 
 	for _, transformer := range additionalTransformers {
@@ -415,51 +455,97 @@ func formatInvalidKeyError(keyPrefix string, key interface{}) error {
 
 // LoadServices produces a ServiceConfig map from a compose file Dict
 // the servicesDict is not validated if directly used. Use Load() to enable validation
-func LoadServices(servicesDict map[string]interface{}, workingDir string, lookupEnv template.Mapping) ([]types.ServiceConfig, error) {
+func LoadServices(filename string, servicesDict map[string]interface{}, workingDir string, lookupEnv template.Mapping, opts *Options) ([]types.ServiceConfig, error) {
 	var services []types.ServiceConfig
 
-	for name, serviceDef := range servicesDict {
-		serviceConfig, err := LoadService(name, serviceDef.(map[string]interface{}), workingDir, lookupEnv)
+	for name := range servicesDict {
+		serviceConfig, err := loadServiceWithExtends(filename, name, servicesDict, workingDir, lookupEnv, opts, &cycleTracker{})
 		if err != nil {
 			return nil, err
 		}
 
-		if serviceConfig.Extends != nil {
-			file := serviceConfig.Extends["file"]
-			service := serviceConfig.Extends["service"]
-			var source interface{}
-			if file == nil {
-				// extends a service from same file
-				source = servicesDict[*service]
-			} else {
-				if !filepath.IsAbs(*file) {
-					absolute := filepath.Join(workingDir, *file)
-					file = &absolute
-				}
-				bytes, err := ioutil.ReadFile(*file)
-				if err != nil {
-					return nil, err
-				}
-				source, err = ParseYAML(bytes)
+		services = append(services, *serviceConfig)
+	}
+
+	return services, nil
+}
+
+func loadServiceWithExtends(filename, name string, servicesDict map[string]interface{}, workingDir string, lookupEnv template.Mapping, opts *Options, ct *cycleTracker) (*types.ServiceConfig, error) {
+	if err := ct.Add(filename, name); err != nil {
+		return nil, err
+	}
+
+	serviceConfig, err := LoadService(name, servicesDict[name].(map[string]interface{}), workingDir, lookupEnv)
+	if err != nil {
+		return nil, err
+	}
+
+	if serviceConfig.Extends != nil && !opts.SkipExtends {
+		baseServiceName := *serviceConfig.Extends["service"]
+		var baseService *types.ServiceConfig
+		if file := serviceConfig.Extends["file"]; file == nil {
+			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 := *file
+			if !filepath.IsAbs(*file) {
+				baseFilePath = filepath.Join(workingDir, *file)
+			}
+
+			bytes, err := ioutil.ReadFile(baseFilePath)
+			if err != nil {
+				return nil, err
+			}
+			baseFile, err := ParseYAML(bytes)
+			if err != nil {
+				return nil, err
+			}
+
+			if !opts.SkipInterpolation {
+				baseFile, err = interpolateConfig(baseFile, *opts.Interpolate)
 				if err != nil {
 					return nil, err
 				}
 			}
-			baseService, err := LoadService(name, source.(map[string]interface{}), workingDir, lookupEnv)
+
+			baseFileServices := getSection(baseFile, "services")
+			baseService, err = loadServiceWithExtends(baseFilePath, baseServiceName, baseFileServices, filepath.Dir(baseFilePath), lookupEnv, opts, ct)
 			if err != nil {
 				return nil, err
 			}
 
-			if err := mergo.Merge(baseService, serviceConfig, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(serviceSpecials)); err != nil {
-				return nil, errors.Wrapf(err, "cannot merge service %s", name)
+			// 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
+			// absolute path.
+			baseFileParent := filepath.Dir(*file)
+			if baseService.Build != nil && !filepath.IsAbs(baseService.Build.Context) {
+				// Note that the Dockerfile is always defined relative to the
+				// build context, so there's no need to update the Dockerfile field.
+				baseService.Build.Context = filepath.Join(baseFileParent, baseService.Build.Context)
+			}
+
+			for i, vol := range baseService.Volumes {
+				if vol.Type != types.VolumeTypeBind {
+					continue
+				}
+
+				if !filepath.IsAbs(baseService.Volumes[i].Source) {
+					baseService.Volumes[i].Source = filepath.Join(baseFileParent, vol.Source)
+				}
 			}
-			serviceConfig = baseService
 		}
 
-		services = append(services, *serviceConfig)
+		if err := mergo.Merge(baseService, serviceConfig, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(serviceSpecials)); err != nil {
+			return nil, errors.Wrapf(err, "cannot merge service %s", name)
+		}
+		serviceConfig = baseService
 	}
 
-	return services, nil
+	return serviceConfig, nil
 }
 
 // LoadService produces a single ServiceConfig from a compose file Dict
@@ -798,6 +884,16 @@ var transformDependsOnConfig TransformerFunc = func(data interface{}) (interface
 	}
 }
 
+var transformExtendsConfig TransformerFunc = func(data interface{}) (interface{}, error) {
+	switch data.(type) {
+	case string:
+		data = map[string]interface{}{
+			"service": data,
+		}
+	}
+	return transformMappingOrListFunc("=", true)(data)
+}
+
 var transformServiceVolumeConfig TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case string:
diff --git a/types/types.go b/types/types.go
index a645b1f..30a2edd 100644
--- a/types/types.go
+++ b/types/types.go
@@ -107,7 +107,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         MappingWithEquals                `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_app" yaml:"group_add,omitempty" json:"group_add,omitempty"`
@@ -676,6 +676,8 @@ type ServiceDependency struct {
 	Extensions map[string]interface{} `yaml:",inline" json:"-"`
 }
 
+type ExtendsConfig MappingWithEquals
+
 // SecretConfig for a secret
 type SecretConfig FileObjectConfig
 


Test patch:
diff --git a/loader/loader_test.go b/loader/loader_test.go
index fec44c0..af1e4eb 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -1665,3 +1665,35 @@ func TestComposeFileWithVersion(t *testing.T) {
 	assert.Check(t, is.DeepEqual(expectedConfig.Networks, config.Networks))
 	assert.Check(t, is.DeepEqual(expectedConfig.Volumes, config.Volumes))
 }
+
+func TestLoadWithExtends(t *testing.T) {
+	bytes, err := ioutil.ReadFile("testdata/compose-test-extends.yaml")
+	assert.NilError(t, err)
+
+	dict, err := ParseYAML(bytes)
+	assert.NilError(t, err)
+
+	configDetails := types.ConfigDetails{
+		WorkingDir: "testdata",
+		ConfigFiles: []types.ConfigFile{
+			{Filename: "testdata/compose-test-extends.yaml", Config: dict},
+		},
+	}
+
+	actual, err := Load(configDetails)
+	assert.NilError(t, err)
+
+	expServices := types.Services{
+		{
+			Name:  "importer",
+			Image: "nginx",
+			Extends: types.ExtendsConfig{
+				"file":    strPtr("compose-test-extends-imported.yaml"),
+				"service": strPtr("imported"),
+			},
+			Environment: types.MappingWithEquals{},
+			Networks:    map[string]*types.ServiceNetworkConfig{"default": nil},
+		},
+	}
+	assert.Check(t, is.DeepEqual(expServices, actual.Services))
+}
diff --git a/loader/testdata/compose-test-extends-imported.yaml b/loader/testdata/compose-test-extends-imported.yaml
new file mode 100644
index 0000000..03f2ea2
--- /dev/null
+++ b/loader/testdata/compose-test-extends-imported.yaml
@@ -0,0 +1,3 @@
+services:
+  imported:
+    image: nginx
diff --git a/loader/testdata/compose-test-extends.yaml b/loader/testdata/compose-test-extends.yaml
new file mode 100644
index 0000000..d167cdc
--- /dev/null
+++ b/loader/testdata/compose-test-extends.yaml
@@ -0,0 +1,5 @@
+services:
+  importer:
+    extends:
+      file: compose-test-extends-imported.yaml
+      service: imported