Repository: compose-spec/compose-go
Pull request #403 (2023-05-15 07:12:33)

fix os.Env precedence resolving variables in .env file
parsing dot.env file :
```
FOO=foo
BAR=${FOO}
``` 
and applying env var precedence rules documented [here](https://docs.docker.com/compose/environment-variables/envvars-precedence/), 
`FOO` set in os.Env should have precedence, and so value set by dotEnv file should not override AND `BAR` should be expanded using value from os.Env.

This rule is different from the one implemented in the original dotenv library, so the need to change the associated testcase.

closes https://github.com/docker/compose/issues/10505


Patch:
diff --git a/cli/options.go b/cli/options.go
index 50b6f07..0815dcc 100644
--- a/cli/options.go
+++ b/cli/options.go
@@ -255,9 +255,8 @@ func WithDotEnv(o *ProjectOptions) error {
 		return err
 	}
 	for k, v := range envMap {
-		o.Environment[k] = v
-		if osVal, ok := os.LookupEnv(k); ok {
-			o.Environment[k] = osVal
+		if _, set := o.Environment[k]; !set {
+			o.Environment[k] = v
 		}
 	}
 	return nil
@@ -304,15 +303,12 @@ func GetEnvFromFile(currentEnv map[string]string, workingDir string, filenames [
 		}
 
 		env, err := dotenv.ParseWithLookup(bytes.NewReader(b), func(k string) (string, bool) {
-			v, ok := envMap[k]
+			v, ok := currentEnv[k]
 			if ok {
 				return v, true
 			}
-			v, ok = currentEnv[k]
-			if !ok {
-				return "", false
-			}
-			return v, true
+			v, ok = envMap[k]
+			return v, ok
 		})
 		if err != nil {
 			return envMap, errors.Wrapf(err, "failed to read %s", dotEnvFile)
diff --git a/dotenv/godotenv.go b/dotenv/godotenv.go
index cd19cb4..9b95c99 100644
--- a/dotenv/godotenv.go
+++ b/dotenv/godotenv.go
@@ -162,10 +162,11 @@ func readFile(filename string, lookupFn LookupFn) (map[string]string, error) {
 
 func expandVariables(value string, envMap map[string]string, lookupFn LookupFn) (string, error) {
 	retVal, err := template.Substitute(value, func(k string) (string, bool) {
-		if v, ok := envMap[k]; ok {
-			return v, ok
+		if v, ok := lookupFn(k); ok {
+			return v, true
 		}
-		return lookupFn(k)
+		v, ok := envMap[k]
+		return v, ok
 	})
 	if err != nil {
 		return value, err


Test patch:
diff --git a/cli/options_test.go b/cli/options_test.go
index 458ed31..4f5fc18 100644
--- a/cli/options_test.go
+++ b/cli/options_test.go
@@ -322,3 +322,46 @@ func TestGetEnvFromFile(t *testing.T) {
 	_, err = GetEnvFromFile(nil, wd, []string{f})
 	assert.Check(t, strings.HasSuffix(err.Error(), ".env is a directory"))
 }
+
+func TestEnvVariablePrecedence(t *testing.T) {
+	testcases := []struct {
+		name     string
+		dotEnv   string
+		osEnv    []string
+		expected map[string]string
+	}{
+		{
+			"no value set in environment",
+			"FOO=foo\nBAR=${FOO}",
+			nil,
+			map[string]string{
+				"FOO": "foo",
+				"BAR": "foo",
+			},
+		},
+		{
+			"conflict with value set in environment",
+			"FOO=foo\nBAR=${FOO}",
+			[]string{"FOO=zot"},
+			map[string]string{
+				"FOO": "zot",
+				"BAR": "zot",
+			},
+		},
+	}
+
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+			wd := t.TempDir()
+			err := os.WriteFile(filepath.Join(wd, ".env"), []byte(test.dotEnv), 0o700)
+			assert.NilError(t, err)
+			options, err := NewProjectOptions(nil,
+				// First load os.Env variable, higher in precedence rule
+				WithEnv(test.osEnv),
+				// Then load dotEnv file
+				WithWorkingDirectory(wd), WithDotEnv)
+			assert.NilError(t, err)
+			assert.DeepEqual(t, test.expected, options.Environment)
+		})
+	}
+}
diff --git a/dotenv/godotenv_test.go b/dotenv/godotenv_test.go
index 994026d..e1187fe 100644
--- a/dotenv/godotenv_test.go
+++ b/dotenv/godotenv_test.go
@@ -587,9 +587,9 @@ func TestSubstitutionsWithEnvFilePrecedence(t *testing.T) {
 	envFileName := "fixtures/substitutions.env"
 	expectedValues := map[string]string{
 		"OPTION_A": "1",
-		"OPTION_B": "1",
-		"OPTION_C": "1",
-		"OPTION_D": "1_1",
+		"OPTION_B": "5",
+		"OPTION_C": "5",
+		"OPTION_D": "5_5",
 		"OPTION_E": "",
 	}