Repository: compose-spec/compose-go
Pull request #446 (2023-08-21 19:32:39)

load: move env var profile detection to option
Instead of reading `COMPOSE_PROFILES` environment variable directly during `load()`, rely exclusively on `opts.Profiles`.

A new loader option, `WithDefaultProfiles`, allows passing profile name(s) via varargs. If none are provided, it will fallback to using any profiles set via `COMPOSE_PROFILES`.

This improves purity of the loader and fixes issues with profiles being ignored for `include` (docker/compose#10906).

milas: Also, technically `COMPOSE_PROFILES` is not part of the spec (we don't specify HOW things like CLI should work).

At the same time, one thing we've learned from `docker/cli` is that it's really annoying if a bunch of conventions (`DOCKER_CONTEXT`) aren't supported by the "lower-level" client (`moby/moby`). So that's why I went with the `WithDefaultProfiles` loader option here - it keeps the logic in `compose-go` for the broader ecosystem, but it's also easy to avoid by using `WithProfiles` instead.

Patch:
diff --git a/cli/options.go b/cli/options.go
index f20803b..332ea3b 100644
--- a/cli/options.go
+++ b/cli/options.go
@@ -66,7 +66,7 @@ type ProjectOptions struct {
 	// NOTE: For security, the loader does not automatically expose any
 	// process environment variables. For convenience, WithOsEnv can be
 	// used if appropriate.
-	Environment map[string]string
+	Environment types.Mapping
 
 	// EnvFiles are file paths to ".env" files with additional environment
 	// variable data.
@@ -194,7 +194,7 @@ func WithEnv(env []string) ProjectOptionsFn {
 	}
 }
 
-// WithDiscardEnvFiles sets discards the `env_file` section after resolving to
+// WithDiscardEnvFile sets discards the `env_file` section after resolving to
 // the `environment` section
 func WithDiscardEnvFile(o *ProjectOptions) error {
 	o.loadOptions = append(o.loadOptions, loader.WithDiscardEnvFiles)
@@ -209,6 +209,15 @@ func WithLoadOptions(loadOptions ...func(*loader.Options)) ProjectOptionsFn {
 	}
 }
 
+// WithDefaultProfiles uses the provided profiles (if any), and falls back to
+// profiles specified via the COMPOSE_PROFILES environment variable otherwise.
+func WithDefaultProfiles(profile ...string) ProjectOptionsFn {
+	if len(profile) == 0 {
+		profile = strings.Split(os.Getenv(consts.ComposeProfiles), ",")
+	}
+	return WithProfiles(profile)
+}
+
 // WithProfiles sets profiles to be activated
 func WithProfiles(profiles []string) ProjectOptionsFn {
 	return func(o *ProjectOptions) error {
@@ -228,8 +237,9 @@ func WithOsEnv(o *ProjectOptions) error {
 	return nil
 }
 
-// WithEnvFile set an alternate env file
-// deprecated - use WithEnvFiles
+// WithEnvFile sets an alternate env file.
+//
+// Deprecated: use WithEnvFiles instead.
 func WithEnvFile(file string) ProjectOptionsFn {
 	var files []string
 	if file != "" {
@@ -256,11 +266,7 @@ func WithDotEnv(o *ProjectOptions) error {
 	if err != nil {
 		return err
 	}
-	for k, v := range envMap {
-		if _, set := o.Environment[k]; !set {
-			o.Environment[k] = v
-		}
-	}
+	o.Environment.Merge(envMap)
 	return nil
 }
 
diff --git a/loader/include.go b/loader/include.go
index 7a736aa..ea78414 100644
--- a/loader/include.go
+++ b/loader/include.go
@@ -73,7 +73,7 @@ func loadInclude(ctx context.Context, filename string, configDetails types.Confi
 		loadOptions.SkipNormalization = true
 		loadOptions.SkipConsistencyCheck = true
 
-		env, err := dotenv.GetEnvFromFile(configDetails.Environment, r.ProjectDirectory, r.EnvFile)
+		envFromFile, err := dotenv.GetEnvFromFile(configDetails.Environment, r.ProjectDirectory, r.EnvFile)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -81,7 +81,7 @@ func loadInclude(ctx context.Context, filename string, configDetails types.Confi
 		config := types.ConfigDetails{
 			WorkingDir:  r.ProjectDirectory,
 			ConfigFiles: types.ToConfigFiles(r.Path),
-			Environment: env,
+			Environment: configDetails.Environment.Clone().Merge(envFromFile),
 		}
 		loadOptions.Interpolate = &interp.Options{
 			Substitute:      options.Interpolate.Substitute,
@@ -114,6 +114,12 @@ func importResources(model *types.Config, imported *types.Project, path []string
 		}
 		model.Services = append(model.Services, service)
 	}
+	for _, service := range imported.DisabledServices {
+		if _, ok := services[service.Name]; ok {
+			return fmt.Errorf("imported compose file %s defines conflicting service %s", path, service.Name)
+		}
+		model.Services = append(model.Services, service)
+	}
 	for n, network := range imported.Networks {
 		if _, ok := model.Networks[n]; ok {
 			return fmt.Errorf("imported compose file %s defines conflicting network %s", path, n)
diff --git a/loader/loader.go b/loader/loader.go
index 6b80ab2..74dc5c8 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -360,9 +360,6 @@ func load(ctx context.Context, configDetails types.ConfigDetails, opts *Options,
 		}
 	}
 
-	if profiles, ok := project.Environment[consts.ComposeProfiles]; ok && len(opts.Profiles) == 0 {
-		opts.Profiles = strings.Split(profiles, ",")
-	}
 	project.ApplyProfiles(opts.Profiles)
 
 	err := project.ResolveServicesEnvironment(opts.discardEnvFiles)
diff --git a/types/config.go b/types/config.go
index 517e612..25e6f82 100644
--- a/types/config.go
+++ b/types/config.go
@@ -34,7 +34,7 @@ type ConfigDetails struct {
 	Version     string
 	WorkingDir  string
 	ConfigFiles []ConfigFile
-	Environment map[string]string
+	Environment Mapping
 }
 
 // LookupEnv provides a lookup function for environment variables
diff --git a/types/types.go b/types/types.go
index 517d9a4..5864dc0 100644
--- a/types/types.go
+++ b/types/types.go
@@ -516,6 +516,24 @@ func (m Mapping) Resolve(s string) (string, bool) {
 	return v, ok
 }
 
+func (m Mapping) Clone() Mapping {
+	clone := Mapping{}
+	for k, v := range m {
+		clone[k] = v
+	}
+	return clone
+}
+
+// Merge adds all values from second mapping which are not already defined
+func (m Mapping) Merge(o Mapping) Mapping {
+	for k, v := range o {
+		if _, set := m[k]; !set {
+			m[k] = v
+		}
+	}
+	return m
+}
+
 // Labels is a mapping type for labels
 type Labels map[string]string
 


Test patch:
diff --git a/cli/options_test.go b/cli/options_test.go
index 893e073..5608759 100644
--- a/cli/options_test.go
+++ b/cli/options_test.go
@@ -22,6 +22,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/compose-spec/compose-go/types"
 	"gotest.tools/v3/assert"
 
 	"github.com/compose-spec/compose-go/consts"
@@ -314,13 +315,13 @@ func TestEnvVariablePrecedence(t *testing.T) {
 		name     string
 		dotEnv   string
 		osEnv    []string
-		expected map[string]string
+		expected types.Mapping
 	}{
 		{
 			"no value set in environment",
 			"FOO=foo\nBAR=${FOO}",
 			nil,
-			map[string]string{
+			types.Mapping{
 				"FOO": "foo",
 				"BAR": "foo",
 			},
@@ -329,7 +330,7 @@ func TestEnvVariablePrecedence(t *testing.T) {
 			"conflict with value set in environment",
 			"FOO=foo\nBAR=${FOO}",
 			[]string{"FOO=zot"},
-			map[string]string{
+			types.Mapping{
 				"FOO": "zot",
 				"BAR": "zot",
 			},
diff --git a/loader/loader_test.go b/loader/loader_test.go
index 130c778..6dad1e7 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -2555,6 +2555,25 @@ services:
 		},
 	})
 	assert.NilError(t, err)
+
+	p, err = Load(buildConfigDetails(`
+name: 'test-include'
+
+include:
+  - path: ./testdata/subdir/compose-test-extends-imported.yaml
+    env_file: ./testdata/subdir/extra.env
+
+services:
+  foo:
+    image: busybox
+    depends_on:
+      - imported
+`, map[string]string{"SOURCE": "override"}), func(options *Options) {
+		options.SkipNormalization = true
+		options.ResolvePaths = true
+	})
+	assert.NilError(t, err)
+	assert.Equal(t, p.Services[1].ContainerName, "override")
 }
 
 func TestLoadWithIncludeCycle(t *testing.T) {