run interpolation after merge, but for required attributes

ndeloof · ndeloof · commit 65600cee45d4 · 2024-07-08T11:48:38.000+02:00
Signed-off-by: Nicolas De Loof &lt;nicolas.deloof@gmail.com&gt;
diff --git a/cli/options_test.go b/cli/options_test.go
@@ -253,7 +253,7 @@ func TestProjectWithDotEnv(t *testing.T) {
 		"compose-with-variables.yaml",
 	}, WithName("my_project"), WithEnvFiles(), WithDotEnv)
 	assert.NilError(t, err)
-	p, err := ProjectFromOptions(context.TODO(), opts)
+	p, err := opts.LoadProject(context.TODO())
 	assert.NilError(t, err)
 	service, err := p.GetService("simple")
 	assert.NilError(t, err)
diff --git a/cli/testdata/env-file/compose-with-env-files.yaml b/cli/testdata/env-file/compose-with-env-files.yaml
@@ -1,4 +1,3 @@
-version: "3"
 services:
   simple:
     image: nginx
diff --git a/loader/extends.go b/loader/extends.go
@@ -22,8 +22,11 @@ import (
 	"path/filepath"
 
 	"github.com/compose-spec/compose-go/v2/consts"
+	"github.com/compose-spec/compose-go/v2/interpolation"
 	"github.com/compose-spec/compose-go/v2/override"
 	"github.com/compose-spec/compose-go/v2/paths"
+	"github.com/compose-spec/compose-go/v2/template"
+	"github.com/compose-spec/compose-go/v2/transform"
 	"github.com/compose-spec/compose-go/v2/types"
 )
 
@@ -68,10 +71,22 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
 	)
 	switch v := extends.(type) {
 	case map[string]any:
+		if opts.Interpolate != nil {
+			v, err = interpolation.Interpolate(v, *opts.Interpolate)
+			if err != nil {
+				return nil, err
+			}
+		}
 		ref = v["service"].(string)
 		file = v["file"]
 		opts.ProcessEvent("extends", v)
 	case string:
+		if opts.Interpolate != nil {
+			v, err = opts.Interpolate.Substitute(v, template.Mapping(opts.Interpolate.LookupValue))
+			if err != nil {
+				return nil, err
+			}
+		}
 		ref = v
 		opts.ProcessEvent("extends", map[string]any{"service": ref})
 	}
@@ -175,6 +190,12 @@ func getExtendsBaseFromFile(
 			)
 		}
 
+		// Attempt to make a canonical model so ResolveRelativePaths can operate on source:target short syntaxes
+		source, err = transform.Canonical(source, true)
+		if err != nil {
+			return nil, nil, err
+		}
+
 		var remotes []paths.RemoteResource
 		for _, loader := range opts.RemoteResourceLoaders() {
 			remotes = append(remotes, loader.Accept)
diff --git a/loader/include.go b/loader/include.go
@@ -30,7 +30,7 @@ import (
 )
 
 // loadIncludeConfig parse the required config from raw yaml
-func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {
+func loadIncludeConfig(source any, options *Options) ([]types.IncludeConfig, error) {
 	if source == nil {
 		return nil, nil
 	}
@@ -45,13 +45,23 @@ func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {
 			}
 		}
 	}
+	if options.Interpolate != nil {
+		for i, config := range configs {
+			interpolated, err := interp.Interpolate(config.(map[string]any), *options.Interpolate)
+			if err != nil {
+				return nil, err
+			}
+			configs[i] = interpolated
+		}
+	}
+
 	var requires []types.IncludeConfig
 	err := Transform(source, &requires)
 	return requires, err
 }
 
 func ApplyInclude(ctx context.Context, workingDir string, environment types.Mapping, model map[string]any, options *Options, included []string) error {
-	includeConfig, err := loadIncludeConfig(model["include"])
+	includeConfig, err := loadIncludeConfig(model["include"], options)
 	if err != nil {
 		return err
 	}
diff --git a/loader/loader.go b/loader/loader.go
@@ -367,6 +367,23 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option
 		}
 	}
 
+	if opts.Interpolate != nil && !opts.SkipInterpolation {
+		dict, err = interp.Interpolate(dict, *opts.Interpolate)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	dict, err = transform.Canonical(dict, opts.SkipInterpolation)
+	if err != nil {
+		return nil, err
+	}
+
+	dict, err = override.EnforceUnicity(dict)
+	if err != nil {
+		return nil, err
+	}
+
 	if !opts.SkipDefaultValues {
 		dict, err = transform.SetDefaultValues(dict)
 		if err != nil {
@@ -415,13 +432,6 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
 			return errors.New("Top-level object must be a mapping")
 		}
 
-		if opts.Interpolate != nil && !opts.SkipInterpolation {
-			cfg, err = interp.Interpolate(cfg, *opts.Interpolate)
-			if err != nil {
-				return err
-			}
-		}
-
 		fixEmptyNotNull(cfg)
 
 		if !opts.SkipExtends {
@@ -450,11 +460,6 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
 			return err
 		}
 
-		dict, err = override.EnforceUnicity(dict)
-		if err != nil {
-			return err
-		}
-
 		if !opts.SkipValidation {
 			if err := schema.Validate(dict); err != nil {
 				return fmt.Errorf("validating %s: %w", file.Filename, err)
@@ -464,15 +469,7 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
 				delete(dict, "version")
 			}
 		}
-
-		dict, err = transform.Canonical(dict, opts.SkipInterpolation)
-		if err != nil {
-			return err
-		}
-
-		// Canonical transformation can reveal duplicates, typically as ports can be a range and conflict with an override
-		dict, err = override.EnforceUnicity(dict)
-		return err
+		return nil
 	}
 
 	var processor PostProcessor
diff --git a/loader/loader_yaml_test.go b/loader/loader_yaml_test.go
@@ -56,6 +56,46 @@ services:
 	})
 }
 
+func TestParseYAMLFilesInterpolateAfterMerge(t *testing.T) {
+	model, err := loadYamlModel(
+		context.TODO(), types.ConfigDetails{
+			ConfigFiles: []types.ConfigFile{
+				{
+					Filename: "test.yaml",
+					Content: []byte(`
+services:
+  test:
+    image: foo
+    environment:
+      my_env: ${my_env?my_env must be set}
+`),
+				},
+				{
+					Filename: "override.yaml",
+					Content: []byte(`
+services:
+  test:
+    image: bar
+    environment:
+      my_env: ${my_env:-default}
+`),
+				},
+			},
+		}, &Options{}, &cycleTracker{}, nil,
+	)
+	assert.NilError(t, err)
+	assert.DeepEqual(
+		t, model, map[string]interface{}{
+			"services": map[string]interface{}{
+				"test": map[string]interface{}{
+					"image":       "bar",
+					"environment": []any{"my_env=${my_env:-default}"},
+				},
+			},
+		},
+	)
+}
+
 func TestParseYAMLFilesMergeOverride(t *testing.T) {
 	model, err := loadYamlModel(context.TODO(), types.ConfigDetails{
 		ConfigFiles: []types.ConfigFile{
diff --git a/schema/compose-spec.json b/schema/compose-spec.json
@@ -155,8 +155,8 @@
           },
           "additionalProperties": false
         },
-        "cap_add": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
-        "cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "cap_add": {"type": "array", "items": {"type": "string"}},
+        "cap_drop": {"type": "array", "items": {"type": "string"}},
         "cgroup": {"type": "string", "enum": ["host", "private"]},
         "cgroup_parent": {"type": "string"},
         "command": {"$ref": "#/definitions/command"},
@@ -215,9 +215,9 @@
           ]
         },
         "device_cgroup_rules": {"$ref": "#/definitions/list_of_strings"},
-        "devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "devices": {"type": "array", "items": {"type": "string"}},
         "dns": {"$ref": "#/definitions/string_or_list"},
-        "dns_opt": {"type": "array","items": {"type": "string"}, "uniqueItems": true},
+        "dns_opt": {"type": "array","items": {"type": "string"}},
         "dns_search": {"$ref": "#/definitions/string_or_list"},
         "domainname": {"type": "string"},
         "entrypoint": {"$ref": "#/definitions/command"},
@@ -229,8 +229,7 @@
           "items": {
             "type": ["string", "number"],
             "format": "expose"
-          },
-          "uniqueItems": true
+          }
         },
         "extends": {
           "oneOf": [
@@ -247,14 +246,13 @@
             }
           ]
         },
-        "external_links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "external_links": {"type": "array", "items": {"type": "string"}},
         "extra_hosts": {"$ref": "#/definitions/list_or_dict"},
         "group_add": {
           "type": "array",
           "items": {
             "type": ["string", "number"]
-          },
-          "uniqueItems": true
+          }
         },
         "healthcheck": {"$ref": "#/definitions/healthcheck"},
         "hostname": {"type": "string"},
@@ -263,7 +261,7 @@
         "ipc": {"type": "string"},
         "isolation": {"type": "string"},
         "labels": {"$ref": "#/definitions/list_or_dict"},
-        "links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "links": {"type": "array", "items": {"type": "string"}},
         "logging": {
           "type": "object",
 
@@ -349,8 +347,7 @@
                 "patternProperties": {"^x-": {}}
               }
             ]
-          },
-          "uniqueItems": true
+          }
         },
         "privileged": {"type": ["boolean", "string"]},
         "profiles": {"$ref": "#/definitions/list_of_strings"},
@@ -365,7 +362,7 @@
         "scale": {
           "type": ["integer", "string"]
         },
-        "security_opt": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
+        "security_opt": {"type": "array", "items": {"type": "string"}},
         "shm_size": {"type": ["number", "string"]},
         "secrets": {"$ref": "#/definitions/service_config_or_secret"},
         "sysctls": {"$ref": "#/definitions/list_or_dict"},
@@ -431,13 +428,11 @@
                 "patternProperties": {"^x-": {}}
               }
             ]
-          },
-          "uniqueItems": true
+          }
         },
         "volumes_from": {
           "type": "array",
-          "items": {"type": "string"},
-          "uniqueItems": true
+          "items": {"type": "string"}
         },
         "working_dir": {"type": "string"}
       },
@@ -832,8 +827,7 @@
 
     "list_of_strings": {
       "type": "array",
-      "items": {"type": "string"},
-      "uniqueItems": true
+      "items": {"type": "string"}
     },
 
     "list_or_dict": {
@@ -847,7 +841,7 @@
           },
           "additionalProperties": false
         },
-        {"type": "array", "items": {"type": "string"}, "uniqueItems": true}
+        {"type": "array", "items": {"type": "string"}}
       ]
     },
 

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-version: "3"`
`2`	`1`	`services:`
`3`	`2`	`simple:`
`4`	`3`	`image: nginx`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ import (`
`30`	`30`	`)`
`31`	`31`
`32`	`32`	`// loadIncludeConfig parse the required config from raw yaml`
`33`		`-func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {`
	`33`	`+func loadIncludeConfig(source any, options *Options) ([]types.IncludeConfig, error) {`
`34`	`34`	`if source == nil {`
`35`	`35`	`return nil, nil`
`36`	`36`	`}`
`@@ -45,13 +45,23 @@ func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`	`}`
	`48`	`+ if options.Interpolate != nil {`
	`49`	`+ for i, config := range configs {`
	`50`	`+ interpolated, err := interp.Interpolate(config.(map[string]any), *options.Interpolate)`
	`51`	`+ if err != nil {`
	`52`	`+ return nil, err`
	`53`	`+ }`
	`54`	`+ configs[i] = interpolated`
	`55`	`+ }`
	`56`	`+ }`
	`57`	`+`
`48`	`58`	`var requires []types.IncludeConfig`
`49`	`59`	`err := Transform(source, &requires)`
`50`	`60`	`return requires, err`
`51`	`61`	`}`
`52`	`62`
`53`	`63`	`func ApplyInclude(ctx context.Context, workingDir string, environment types.Mapping, model map[string]any, options *Options, included []string) error {`
`54`		`- includeConfig, err := loadIncludeConfig(model["include"])`
	`64`	`+ includeConfig, err := loadIncludeConfig(model["include"], options)`
`55`	`65`	`if err != nil {`
`56`	`66`	`return err`
`57`	`67`	`}`