Fix interpolation priorities

Signed-off-by: Nicolas De Loof <[email protected]>

Author: ndeloof

Diff for: dotenv/format.go

@@ -24,28 +24,28 @@ import (
 const DotEnv = ".env"
 
 var formats = map[string]Parser{
-	DotEnv: func(r io.Reader, filename string, lookup func(key string) (string, bool)) (map[string]string, error) {
-		m, err := ParseWithLookup(r, lookup)
+	DotEnv: func(r io.Reader, filename string, vars map[string]string, lookup func(key string) (string, bool)) error {
+		err := parseWithLookup(r, vars, lookup)
 		if err != nil {
-			return nil, fmt.Errorf("failed to read %s: %w", filename, err)
+			return fmt.Errorf("failed to read %s: %w", filename, err)
 		}
-		return m, nil
+		return nil
 	},
 }
 
-type Parser func(r io.Reader, filename string, lookup func(key string) (string, bool)) (map[string]string, error)
+type Parser func(r io.Reader, filename string, vars map[string]string, lookup func(key string) (string, bool)) error
 
 func RegisterFormat(format string, p Parser) {
 	formats[format] = p
 }
 
-func ParseWithFormat(r io.Reader, filename string, resolve LookupFn, format string) (map[string]string, error) {
+func ParseWithFormat(r io.Reader, filename string, vars map[string]string, resolve LookupFn, format string) error {
 	if format == "" {
 		format = DotEnv
 	}
 	fn, ok := formats[format]
 	if !ok {
-		return nil, fmt.Errorf("unsupported env_file format %q", format)
+		return fmt.Errorf("unsupported env_file format %q", format)
 	}
-	return fn(r, filename, resolve)
+	return fn(r, filename, vars, resolve)
 }

Diff for: dotenv/godotenv.go

@@ -41,16 +41,23 @@ func Parse(r io.Reader) (map[string]string, error) {
 
 // ParseWithLookup reads an env file from io.Reader, returning a map of keys and values.
 func ParseWithLookup(r io.Reader, lookupFn LookupFn) (map[string]string, error) {
+	vars := map[string]string{}
+	err := parseWithLookup(r, vars, lookupFn)
+	return vars, err
+}
+
+// ParseWithLookup reads an env file from io.Reader, returning a map of keys and values.
+func parseWithLookup(r io.Reader, vars map[string]string, lookupFn LookupFn) error {
 	data, err := io.ReadAll(r)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// seek past the UTF-8 BOM if it exists (particularly on Windows, some
 	// editors tend to add it, and it'll cause parsing to fail)
 	data = bytes.TrimPrefix(data, utf8BOM)
 
-	return UnmarshalBytesWithLookup(data, lookupFn)
+	return newParser().parse(string(data), vars, lookupFn)
 }
 
 // Load will read your env file(s) and load them into ENV for this process.

Diff for: dotenv/godotenv_test.go

@@ -717,8 +717,7 @@ func TestLoadWithFormat(t *testing.T) {
 		"ZOT": "QIX",
 	}
 
-	custom := func(r io.Reader, _ string, lookup func(key string) (string, bool)) (map[string]string, error) {
-		vars := map[string]string{}
+	custom := func(r io.Reader, _ string, vars map[string]string, lookup func(key string) (string, bool)) error {
 		scanner := bufio.NewScanner(r)
 		for scanner.Scan() {
 			key, value, found := strings.Cut(scanner.Text(), ":")
@@ -730,14 +729,15 @@ func TestLoadWithFormat(t *testing.T) {
 			}
 			vars[key] = value
 		}
-		return vars, nil
+		return nil
 	}
 
 	RegisterFormat("custom", custom)
 
 	f, err := os.Open(envFileName)
 	assert.NilError(t, err)
-	env, err := ParseWithFormat(f, envFileName, nil, "custom")
+	env := map[string]string{}
+	err = ParseWithFormat(f, envFileName, env, nil, "custom")
 	assert.NilError(t, err)
 	assert.DeepEqual(t, expectedValues, env)
 }

Diff for: loader/loader_test.go

@@ -596,7 +596,6 @@ services:
 		"BAR": strPtr("2"),
 		"GA":  strPtr("2.5"),
 		"BU":  strPtr(""),
-		"ZO":  nil,
 		"MEU": strPtr("Shadoks"),
 	}
 

Diff for: types/fixtures/base.env

@@ -0,0 +1,6 @@
+FOO=foo_from_base.env
+INTERPOLATED_FOO=${FOO}
+BAR=bar_from_base.env
+INTERPOLATED_BAR=${BAR}
+ZOT=zot_from_base.env
+INTERPOLATED_ZOT=${ZOT}

Diff for: types/fixtures/override.env

@@ -0,0 +1,4 @@
+FOO=foo_from_override.env
+INTERPOLATED_FOO=${FOO}
+BAR=bar_from_override.env
+INTERPOLATED_BAR=${BAR}

Diff for: types/project.go

@@ -639,27 +639,27 @@ func (p *Project) MarshalJSON(options ...func(*marshallOptions)) ([]byte, error)
 func (p Project) WithServicesEnvironmentResolved(discardEnvFiles bool) (*Project, error) {
 	newProject := p.deepCopy()
 	for i, service := range newProject.Services {
-		service.Environment = service.Environment.Resolve(newProject.Environment.Resolve)
-
-		environment := MappingWithEquals{}
-		// resolve variables based on other files we already parsed, + project's environment
-		var resolve dotenv.LookupFn = func(s string) (string, bool) {
-			v, ok := environment[s]
-			if ok && v != nil {
-				return *v, ok
-			}
-			return newProject.Environment.Resolve(s)
-		}
+		service.Environment = service.Environment.Resolve(newProject.Environment.Resolve).RemoveEmpty()
 
+		environment := service.Environment.ToMapping()
 		for _, envFile := range service.EnvFiles {
-			vars, err := loadEnvFile(envFile, resolve)
+			err := loadEnvFile(envFile, environment, func(k string) (string, bool) {
+				// project.env has precedence doing interpolation
+				if resolve, ok := p.Environment.Resolve(k); ok {
+					return resolve, true
+				}
+				// then service.environment
+				if s, ok := service.Environment[k]; ok {
+					return *s, true
+				}
+				return "", false
+			})
 			if err != nil {
 				return nil, err
 			}
-			environment.OverrideBy(vars.ToMappingWithEquals())
 		}
 
-		service.Environment = environment.OverrideBy(service.Environment)
+		service.Environment = environment.ToMappingWithEquals().OverrideBy(service.Environment)
 
 		if discardEnvFiles {
 			service.EnvFiles = nil
@@ -707,37 +707,36 @@ func (p Project) WithServicesLabelsResolved(discardLabelFiles bool) (*Project, e
 	return newProject, nil
 }
 
-func loadEnvFile(envFile EnvFile, resolve dotenv.LookupFn) (Mapping, error) {
+func loadEnvFile(envFile EnvFile, environment Mapping, resolve dotenv.LookupFn) error {
 	if _, err := os.Stat(envFile.Path); os.IsNotExist(err) {
 		if envFile.Required {
-			return nil, fmt.Errorf("env file %s not found: %w", envFile.Path, err)
+			return fmt.Errorf("env file %s not found: %w", envFile.Path, err)
 		}
-		return nil, nil
+		return nil
 	}
 
-	return loadMappingFile(envFile.Path, envFile.Format, resolve)
+	err := loadMappingFile(envFile.Path, envFile.Format, environment, resolve)
+	return err
 }
 
 func loadLabelFile(labelFile string, resolve dotenv.LookupFn) (Mapping, error) {
 	if _, err := os.Stat(labelFile); os.IsNotExist(err) {
 		return nil, fmt.Errorf("label file %s not found: %w", labelFile, err)
 	}
 
-	return loadMappingFile(labelFile, "", resolve)
+	labels := Mapping{}
+	err := loadMappingFile(labelFile, "", labels, resolve)
+	return labels, err
 }
 
-func loadMappingFile(path string, format string, resolve dotenv.LookupFn) (Mapping, error) {
+func loadMappingFile(path string, format string, vars Mapping, resolve dotenv.LookupFn) error {
 	file, err := os.Open(path)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	defer file.Close()
 
-	fileVars, err := dotenv.ParseWithFormat(file, path, resolve, format)
-	if err != nil {
-		return nil, err
-	}
-	return fileVars, nil
+	return dotenv.ParseWithFormat(file, path, vars, resolve, format)
 }
 
 func (p *Project) deepCopy() *Project {

Diff for: types/project_test.go

@@ -432,3 +432,67 @@ secrets:
     content: SECRET`
 	assert.Equal(t, strings.TrimSpace(string(yaml)), strings.TrimSpace(expected))
 }
+
+func TestProject_WithServicesEnvironmentResolved(t *testing.T) {
+	p := &Project{
+		Services: Services{
+			"base": ServiceConfig{
+				Environment: MappingWithEquals{
+					"FOO": ptr("foo_from_environment"),
+					"BAR": ptr("bar_from_environment"),
+					"QIX": nil,
+				},
+				EnvFiles: []EnvFile{
+					{Path: "fixtures/base.env"},
+				},
+			},
+			"override": ServiceConfig{
+				Environment: MappingWithEquals{
+					"FOO": ptr("foo_from_environment"),
+				},
+				EnvFiles: []EnvFile{
+					{Path: "fixtures/base.env"},
+					{Path: "fixtures/override.env"},
+				},
+			},
+		},
+		Environment: map[string]string{
+			"FOO": "FOO_from_os.env",
+			"QIX": "QIX_from_os.env",
+		},
+	}
+	p, err := p.WithServicesEnvironmentResolved(true)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, p.Services["base"].Environment, MappingWithEquals{
+		// service.environment has precedence over env_file
+		"FOO": ptr("foo_from_environment"),
+		"BAR": ptr("bar_from_environment"),
+		// service.environment without a value propagates os.env to container
+		"QIX": ptr("QIX_from_os.env"),
+		// value from env_file is loaded
+		"ZOT": ptr("zot_from_base.env"),
+		// os.env is always preferred for interpolation
+		"INTERPOLATED_FOO": ptr("FOO_from_os.env"),
+		// then service.environment is preferred for interpolation
+		"INTERPOLATED_BAR": ptr("bar_from_environment"),
+		// interpolation uses the value previously set in env file
+		"INTERPOLATED_ZOT": ptr("zot_from_base.env"),
+	})
+	assert.DeepEqual(t, p.Services["override"].Environment, MappingWithEquals{
+		// service.environment has precedence over env_file
+		"FOO": ptr("foo_from_environment"),
+		// value from env_file are loaded, with override
+		"BAR": ptr("bar_from_override.env"),
+		"ZOT": ptr("zot_from_base.env"),
+		// os.env is always preferred for interpolation
+		"INTERPOLATED_FOO": ptr("FOO_from_os.env"),
+		// interpolation uses the overridden value from override.env
+		"INTERPOLATED_BAR": ptr("bar_from_override.env"),
+		// interpolation uses the value previously set in env file
+		"INTERPOLATED_ZOT": ptr("zot_from_base.env"),
+	})
+}
+
+func ptr[T any](s T) *T {
+	return &s
+}