Repository: compose-spec/compose-go
Pull request #142 (2021-06-04 13:55:39)

run interpolation on raw yaml before parsing into map[string]interface{}
This move the interpolation process to take place _before_ parsing the yaml tree into a
`map[string]interface{}`, so that a variable which resolves to a numeric value will corectly be parsed as an `int`, not `string`.

tested with https://github.com/docker/compose-cli/pull/1738

see https://github.com/docker/compose-cli/issues/1725 for context


Patch:
diff --git a/cli/options.go b/cli/options.go
index 2c1d79c..c1c0e38 100644
--- a/cli/options.go
+++ b/cli/options.go
@@ -249,9 +249,28 @@ func ProjectFromOptions(options *ProjectOptions) (*types.Project, error) {
 		return nil, err
 	}
 
-	configs, err := parseConfigs(configPaths)
-	if err != nil {
-		return nil, err
+	var configs []types.ConfigFile
+	for _, f := range configPaths {
+		var b []byte
+		if f == "-" {
+			b, err = ioutil.ReadAll(os.Stdin)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			f, err := filepath.Abs(f)
+			if err != nil {
+				return nil, err
+			}
+			b, err = ioutil.ReadFile(f)
+			if err != nil {
+				return nil, err
+			}
+		}
+		configs = append(configs, types.ConfigFile{
+			Filename: f,
+			Content:  b,
+		})
 	}
 
 	workingDir, err := options.GetWorkingDir()
@@ -335,37 +354,6 @@ func absolutePaths(p []string, pwd string) ([]string, error) {
 	return paths, nil
 }
 
-func parseConfigs(configPaths []string) ([]types.ConfigFile, error) {
-	files := []types.ConfigFile{}
-	for _, f := range configPaths {
-		var (
-			b   []byte
-			err error
-		)
-		if f == "-" {
-			b, err = ioutil.ReadAll(os.Stdin)
-		} else {
-			b, err = ioutil.ReadFile(f)
-		}
-		if err != nil {
-			return nil, err
-		}
-		config, err := loader.ParseYAML(b)
-		if err != nil {
-			return nil, err
-		}
-		if !filepath.IsAbs(f) {
-			fAbs, err := filepath.Abs(f)
-			if err != nil {
-				return nil, err
-			}
-			f = fAbs
-		}
-		files = append(files, types.ConfigFile{Filename: f, Config: config})
-	}
-	return files, nil
-}
-
 // getAsEqualsMap split key=value formatted strings into a key : value map
 func getAsEqualsMap(em []string) map[string]string {
 	m := make(map[string]string)
diff --git a/loader/interpolate.go b/loader/interpolate.go
index e4e5ccb..b45172a 100644
--- a/loader/interpolate.go
+++ b/loader/interpolate.go
@@ -82,7 +82,3 @@ func toBoolean(value string) (interface{}, error) {
 		return nil, errors.Errorf("invalid boolean: %s", value)
 	}
 }
-
-func interpolateConfig(configDict map[string]interface{}, opts interp.Options) (map[string]interface{}, error) {
-	return interp.Interpolate(configDict, opts)
-}
diff --git a/loader/loader.go b/loader/loader.go
index 5713423..97ce633 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -140,15 +140,16 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
 	}
 
 	configs := []*types.Config{}
-	for _, file := range configDetails.ConfigFiles {
+	for i, file := range configDetails.ConfigFiles {
 		configDict := file.Config
-
-		if !opts.SkipInterpolation {
-			var err error
-			configDict, err = interpolateConfig(configDict, *opts.Interpolate)
+		if configDict == nil {
+			dict, err := parseConfig(file.Content, opts)
 			if err != nil {
 				return nil, err
 			}
+			configDict = dict
+			file.Config = dict
+			configDetails.ConfigFiles[i] = file
 		}
 
 		if !opts.SkipValidation {
@@ -214,6 +215,18 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
 	return project, nil
 }
 
+func parseConfig(b []byte, opts *Options) (map[string]interface{}, error) {
+	if !opts.SkipInterpolation {
+		substitute, err := opts.Interpolate.Substitute(string(b), template.Mapping(opts.Interpolate.LookupValue))
+		if err != nil {
+			return nil, err
+		}
+		b = []byte(substitute)
+	}
+
+	return ParseYAML(b)
+}
+
 func groupXFieldsIntoExtensions(dict map[string]interface{}) map[string]interface{} {
 	extras := map[string]interface{}{}
 	for key, value := range dict {
@@ -445,16 +458,18 @@ func loadServiceWithExtends(filename, name string, servicesDict map[string]inter
 			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)
+				substitute, err := opts.Interpolate.Substitute(string(bytes), template.Mapping(opts.Interpolate.LookupValue))
 				if err != nil {
 					return nil, err
 				}
+				bytes = []byte(substitute)
+			}
+
+			baseFile, err := ParseYAML(bytes)
+			if err != nil {
+				return nil, err
 			}
 
 			baseFileServices := getSection(baseFile, "services")
diff --git a/types/config.go b/types/config.go
index 067f339..21127f4 100644
--- a/types/config.go
+++ b/types/config.go
@@ -38,8 +38,12 @@ func (cd ConfigDetails) LookupEnv(key string) (string, bool) {
 
 // ConfigFile is a filename and the contents of the file as a Dict
 type ConfigFile struct {
+	// Filename is the name of the yaml configuration file
 	Filename string
-	Config   map[string]interface{}
+	// Content is the raw yaml content. Will be loaded from Filename if not set
+	Content []byte
+	// Config if the yaml tree for this config file. Will be parsed from Content if not set
+	Config map[string]interface{}
 }
 
 // Config is a full compose file configuration and model


Test patch:
diff --git a/compatibility/allowlist_test.go b/compatibility/allowlist_test.go
index b1cb992..592d3e9 100644
--- a/compatibility/allowlist_test.go
+++ b/compatibility/allowlist_test.go
@@ -36,19 +36,18 @@ func TestAllowList(t *testing.T) {
 			},
 		},
 	}
-	dict, err := loader.ParseYAML([]byte(`
+	dict := []byte(`
 services:
   foo:
     image: busybox
     network_mode: host
     privileged: true
     mac_address: "a:b:c:d"
-`))
-	assert.NilError(t, err)
+`)
 
 	project, err := loader.Load(types.ConfigDetails{
 		ConfigFiles: []types.ConfigFile{
-			{Filename: "filename.yml", Config: dict},
+			{Filename: "filename.yml", Content: dict},
 		},
 	})
 	assert.NilError(t, err)
diff --git a/loader/loader_test.go b/loader/loader_test.go
index 4ded0e0..61db73c 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -32,7 +32,7 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func buildConfigDetails(source map[string]interface{}, env map[string]string) types.ConfigDetails {
+func buildConfigDetails(yaml string, env map[string]string) types.ConfigDetails {
 	workingDir, err := os.Getwd()
 	if err != nil {
 		panic(err)
@@ -41,7 +41,7 @@ func buildConfigDetails(source map[string]interface{}, env map[string]string) ty
 	return types.ConfigDetails{
 		WorkingDir: workingDir,
 		ConfigFiles: []types.ConfigFile{
-			{Filename: "filename.yml", Config: source},
+			{Filename: "filename.yml", Content: []byte(yaml)},
 		},
 		Environment: env,
 	}
@@ -52,12 +52,7 @@ func loadYAML(yaml string) (*types.Project, error) {
 }
 
 func loadYAMLWithEnv(yaml string, env map[string]string) (*types.Project, error) {
-	dict, err := ParseYAML([]byte(yaml))
-	if err != nil {
-		return nil, err
-	}
-
-	return Load(buildConfigDetails(dict, env), func(options *Options) {
+	return Load(buildConfigDetails(yaml, env), func(options *Options) {
 		options.SkipConsistencyCheck = true
 		options.SkipNormalization = true
 	})
@@ -251,7 +246,7 @@ func TestParseYAML(t *testing.T) {
 }
 
 func TestLoad(t *testing.T) {
-	actual, err := Load(buildConfigDetails(sampleDict, nil), func(options *Options) {
+	actual, err := Load(buildConfigDetails(sampleYAML, nil), func(options *Options) {
 		options.SkipNormalization = true
 		options.SkipConsistencyCheck = true
 	})
@@ -519,7 +514,7 @@ volumes:
 }
 
 func TestLoadWithInterpolationCastFull(t *testing.T) {
-	dict, err := ParseYAML([]byte(`
+	dict := `
 services:
   web:
     configs:
@@ -581,8 +576,7 @@ networks:
     internal: $thebool
     attachable: $thebool
   back:
-`))
-	assert.NilError(t, err)
+`
 	env := map[string]string{
 		"theint":   "555",
 		"thefloat": "3.14",
@@ -686,7 +680,7 @@ networks:
 }
 
 func TestUnsupportedProperties(t *testing.T) {
-	dict, err := ParseYAML([]byte(`
+	dict := `
 services:
   web:
     image: web
@@ -699,30 +693,27 @@ services:
     image: db
     build:
      context: ./db
-`))
-	assert.NilError(t, err)
-
+`
 	configDetails := buildConfigDetails(dict, nil)
 
-	_, err = Load(configDetails)
+	_, err := Load(configDetails)
 	assert.NilError(t, err)
 }
 
 func TestDiscardEnvFileOption(t *testing.T) {
-	dict, err := ParseYAML([]byte(`services:
+	dict := `services:
   web:
     image: nginx
     env_file:
      - example1.env
      - example2.env
-`))
+`
 	expectedEnvironmentMap := types.MappingWithEquals{
 		"FOO": strPtr("foo_from_env_file"),
 		"BAZ": strPtr("baz_from_env_file"),
 		"BAR": strPtr("bar_from_env_file_2"), // Original value is overwritten by example2.env
 		"QUX": strPtr("quz_from_env_file_2"),
 	}
-	assert.NilError(t, err)
 	configDetails := buildConfigDetails(dict, nil)
 
 	// Default behavior keeps the `env_file` entries
@@ -740,7 +731,7 @@ func TestDiscardEnvFileOption(t *testing.T) {
 }
 
 func TestBuildProperties(t *testing.T) {
-	dict, err := ParseYAML([]byte(`
+	dict := `
 services:
   web:
     image: web
@@ -751,15 +742,14 @@ services:
     image: db
     build:
      context: ./db
-`))
-	assert.NilError(t, err)
+`
 	configDetails := buildConfigDetails(dict, nil)
-	_, err = Load(configDetails)
+	_, err := Load(configDetails)
 	assert.NilError(t, err)
 }
 
 func TestDeprecatedProperties(t *testing.T) {
-	dict, err := ParseYAML([]byte(`
+	dict := `
 services:
   web:
     image: web
@@ -768,12 +758,10 @@ services:
     image: db
     container_name: db
     expose: ["5434"]
-`))
-	assert.NilError(t, err)
-
+`
 	configDetails := buildConfigDetails(dict, nil)
 
-	_, err = Load(configDetails)
+	_, err := Load(configDetails)
 	assert.NilError(t, err)
 }
 
@@ -841,6 +829,18 @@ volumes:
 	assert.ErrorContains(t, err, "external_volume")
 }
 
+func TestInterpolateInt(t *testing.T) {
+	project, err := loadYAMLWithEnv(`
+services:
+  foo:
+    image: foo
+    scale: ${FOO_SCALE}
+`, map[string]string{"FOO_SCALE": "2"})
+
+	assert.NilError(t, err)
+	assert.Equal(t, project.Services[0].Scale, 2)
+}
+
 func durationPtr(value time.Duration) *types.Duration {
 	result := types.Duration(value)
 	return &result
@@ -1674,13 +1674,10 @@ 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},
+			{Filename: "testdata/compose-test-extends.yaml", Content: bytes},
 		},
 	}
 
diff --git a/loader/types_test.go b/loader/types_test.go
index c88b1ff..3f466d0 100644
--- a/loader/types_test.go
+++ b/loader/types_test.go
@@ -39,9 +39,7 @@ func TestMarshallConfig(t *testing.T) {
 	assert.Check(t, is.Equal(expected, string(actual)))
 
 	// Make sure the expected still
-	dict, err := ParseYAML([]byte(expected))
-	assert.NilError(t, err)
-	_, err = Load(buildConfigDetails(dict, map[string]string{}), func(options *Options) {
+	_, err = Load(buildConfigDetails(expected, map[string]string{}), func(options *Options) {
 		options.SkipNormalization = true
 		options.SkipConsistencyCheck = true
 	})
@@ -61,9 +59,7 @@ func TestJSONMarshallConfig(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(expected, string(actual)))
 
-	dict, err := ParseYAML([]byte(expected))
-	assert.NilError(t, err)
-	_, err = Load(buildConfigDetails(dict, map[string]string{}), func(options *Options) {
+	_, err = Load(buildConfigDetails(expected, map[string]string{}), func(options *Options) {
 		options.SkipNormalization = true
 		options.SkipConsistencyCheck = true
 	})