Skip to content

Commit bfaf10b

Browse files
idsulikndeloof
idsulik
authored and
ndeloof
committed
Revert "run interpolation after merge, but for required attributes"
Signed-off-by: Suleiman Dibirov <[email protected]>
1 parent cf5a3d4 commit bfaf10b

File tree

7 files changed

+45
-106
lines changed

7 files changed

+45
-106
lines changed

‎cli/options_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func TestProjectWithDotEnv(t *testing.T) {
253253
"compose-with-variables.yaml",
254254
}, WithName("my_project"), WithEnvFiles(), WithDotEnv)
255255
assert.NilError(t, err)
256-
p, err := opts.LoadProject(context.TODO())
256+
p, err := ProjectFromOptions(context.TODO(), opts)
257257
assert.NilError(t, err)
258258
service, err := p.GetService("simple")
259259
assert.NilError(t, err)

‎cli/testdata/env-file/compose-with-env-files.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
version: "3"
12
services:
23
simple:
34
image: nginx

‎loader/extends.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ import (
2222
"path/filepath"
2323

2424
"github.com/compose-spec/compose-go/v2/consts"
25-
"github.com/compose-spec/compose-go/v2/interpolation"
2625
"github.com/compose-spec/compose-go/v2/override"
2726
"github.com/compose-spec/compose-go/v2/paths"
28-
"github.com/compose-spec/compose-go/v2/template"
29-
"github.com/compose-spec/compose-go/v2/transform"
3027
"github.com/compose-spec/compose-go/v2/types"
3128
)
3229

@@ -71,22 +68,10 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a
7168
)
7269
switch v := extends.(type) {
7370
case map[string]any:
74-
if opts.Interpolate != nil {
75-
v, err = interpolation.Interpolate(v, *opts.Interpolate)
76-
if err != nil {
77-
return nil, err
78-
}
79-
}
8071
ref = v["service"].(string)
8172
file = v["file"]
8273
opts.ProcessEvent("extends", v)
8374
case string:
84-
if opts.Interpolate != nil {
85-
v, err = opts.Interpolate.Substitute(v, template.Mapping(opts.Interpolate.LookupValue))
86-
if err != nil {
87-
return nil, err
88-
}
89-
}
9075
ref = v
9176
opts.ProcessEvent("extends", map[string]any{"service": ref})
9277
}
@@ -190,12 +175,6 @@ func getExtendsBaseFromFile(
190175
)
191176
}
192177

193-
// Attempt to make a canonical model so ResolveRelativePaths can operate on source:target short syntax
194-
source, err = transform.Canonical(source, true)
195-
if err != nil {
196-
return nil, nil, err
197-
}
198-
199178
var remotes []paths.RemoteResource
200179
for _, loader := range opts.RemoteResourceLoaders() {
201180
remotes = append(remotes, loader.Accept)

‎loader/include.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
)
3131

3232
// loadIncludeConfig parse the required config from raw yaml
33-
func loadIncludeConfig(source any, options *Options) ([]types.IncludeConfig, error) {
33+
func loadIncludeConfig(source any) ([]types.IncludeConfig, error) {
3434
if source == nil {
3535
return nil, nil
3636
}
@@ -45,23 +45,13 @@ func loadIncludeConfig(source any, options *Options) ([]types.IncludeConfig, err
4545
}
4646
}
4747
}
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-
5848
var requires []types.IncludeConfig
5949
err := Transform(source, &requires)
6050
return requires, err
6151
}
6252

6353
func ApplyInclude(ctx context.Context, workingDir string, environment types.Mapping, model map[string]any, options *Options, included []string) error {
64-
includeConfig, err := loadIncludeConfig(model["include"], options)
54+
includeConfig, err := loadIncludeConfig(model["include"])
6555
if err != nil {
6656
return err
6757
}

‎loader/loader.go

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -367,23 +367,6 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option
367367
}
368368
}
369369

370-
if opts.Interpolate != nil && !opts.SkipInterpolation {
371-
dict, err = interp.Interpolate(dict, *opts.Interpolate)
372-
if err != nil {
373-
return nil, err
374-
}
375-
}
376-
377-
dict, err = transform.Canonical(dict, opts.SkipInterpolation)
378-
if err != nil {
379-
return nil, err
380-
}
381-
382-
dict, err = override.EnforceUnicity(dict)
383-
if err != nil {
384-
return nil, err
385-
}
386-
387370
if !opts.SkipDefaultValues {
388371
dict, err = transform.SetDefaultValues(dict)
389372
if err != nil {
@@ -432,6 +415,13 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
432415
return errors.New("Top-level object must be a mapping")
433416
}
434417

418+
if opts.Interpolate != nil && !opts.SkipInterpolation {
419+
cfg, err = interp.Interpolate(cfg, *opts.Interpolate)
420+
if err != nil {
421+
return err
422+
}
423+
}
424+
435425
fixEmptyNotNull(cfg)
436426

437427
if !opts.SkipExtends {
@@ -460,6 +450,11 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
460450
return err
461451
}
462452

453+
dict, err = override.EnforceUnicity(dict)
454+
if err != nil {
455+
return err
456+
}
457+
463458
if !opts.SkipValidation {
464459
if err := schema.Validate(dict); err != nil {
465460
return fmt.Errorf("validating %s: %w", file.Filename, err)
@@ -469,7 +464,15 @@ func loadYamlFile(ctx context.Context, file types.ConfigFile, opts *Options, wor
469464
delete(dict, "version")
470465
}
471466
}
472-
return nil
467+
468+
dict, err = transform.Canonical(dict, opts.SkipInterpolation)
469+
if err != nil {
470+
return err
471+
}
472+
473+
// Canonical transformation can reveal duplicates, typically as ports can be a range and conflict with an override
474+
dict, err = override.EnforceUnicity(dict)
475+
return err
473476
}
474477

475478
var processor PostProcessor

‎loader/loader_yaml_test.go

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -56,46 +56,6 @@ services:
5656
})
5757
}
5858

59-
func TestParseYAMLFilesInterpolateAfterMerge(t *testing.T) {
60-
model, err := loadYamlModel(
61-
context.TODO(), types.ConfigDetails{
62-
ConfigFiles: []types.ConfigFile{
63-
{
64-
Filename: "test.yaml",
65-
Content: []byte(`
66-
services:
67-
test:
68-
image: foo
69-
environment:
70-
my_env: ${my_env?my_env must be set}
71-
`),
72-
},
73-
{
74-
Filename: "override.yaml",
75-
Content: []byte(`
76-
services:
77-
test:
78-
image: bar
79-
environment:
80-
my_env: ${my_env:-default}
81-
`),
82-
},
83-
},
84-
}, &Options{}, &cycleTracker{}, nil,
85-
)
86-
assert.NilError(t, err)
87-
assert.DeepEqual(
88-
t, model, map[string]interface{}{
89-
"services": map[string]interface{}{
90-
"test": map[string]interface{}{
91-
"image": "bar",
92-
"environment": []any{"my_env=${my_env:-default}"},
93-
},
94-
},
95-
},
96-
)
97-
}
98-
9959
func TestParseYAMLFilesMergeOverride(t *testing.T) {
10060
model, err := loadYamlModel(context.TODO(), types.ConfigDetails{
10161
ConfigFiles: []types.ConfigFile{

‎schema/compose-spec.json

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@
155155
},
156156
"additionalProperties": false
157157
},
158-
"cap_add": {"type": "array", "items": {"type": "string"}},
159-
"cap_drop": {"type": "array", "items": {"type": "string"}},
158+
"cap_add": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
159+
"cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
160160
"cgroup": {"type": "string", "enum": ["host", "private"]},
161161
"cgroup_parent": {"type": "string"},
162162
"command": {"$ref": "#/definitions/command"},
@@ -216,9 +216,9 @@
216216
]
217217
},
218218
"device_cgroup_rules": {"$ref": "#/definitions/list_of_strings"},
219-
"devices": {"type": "array", "items": {"type": "string"}},
219+
"devices": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
220220
"dns": {"$ref": "#/definitions/string_or_list"},
221-
"dns_opt": {"type": "array","items": {"type": "string"}},
221+
"dns_opt": {"type": "array","items": {"type": "string"}, "uniqueItems": true},
222222
"dns_search": {"$ref": "#/definitions/string_or_list"},
223223
"domainname": {"type": "string"},
224224
"entrypoint": {"$ref": "#/definitions/command"},
@@ -230,7 +230,8 @@
230230
"items": {
231231
"type": ["string", "number"],
232232
"format": "expose"
233-
}
233+
},
234+
"uniqueItems": true
234235
},
235236
"extends": {
236237
"oneOf": [
@@ -247,13 +248,14 @@
247248
}
248249
]
249250
},
250-
"external_links": {"type": "array", "items": {"type": "string"}},
251+
"external_links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
251252
"extra_hosts": {"$ref": "#/definitions/list_or_dict"},
252253
"group_add": {
253254
"type": "array",
254255
"items": {
255256
"type": ["string", "number"]
256-
}
257+
},
258+
"uniqueItems": true
257259
},
258260
"healthcheck": {"$ref": "#/definitions/healthcheck"},
259261
"hostname": {"type": "string"},
@@ -262,7 +264,7 @@
262264
"ipc": {"type": "string"},
263265
"isolation": {"type": "string"},
264266
"labels": {"$ref": "#/definitions/list_or_dict"},
265-
"links": {"type": "array", "items": {"type": "string"}},
267+
"links": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
266268
"logging": {
267269
"type": "object",
268270

@@ -348,7 +350,8 @@
348350
"patternProperties": {"^x-": {}}
349351
}
350352
]
351-
}
353+
},
354+
"uniqueItems": true
352355
},
353356
"privileged": {"type": ["boolean", "string"]},
354357
"profiles": {"$ref": "#/definitions/list_of_strings"},
@@ -363,7 +366,7 @@
363366
"scale": {
364367
"type": ["integer", "string"]
365368
},
366-
"security_opt": {"type": "array", "items": {"type": "string"}},
369+
"security_opt": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
367370
"shm_size": {"type": ["number", "string"]},
368371
"secrets": {"$ref": "#/definitions/service_config_or_secret"},
369372
"sysctls": {"$ref": "#/definitions/list_or_dict"},
@@ -429,11 +432,13 @@
429432
"patternProperties": {"^x-": {}}
430433
}
431434
]
432-
}
435+
},
436+
"uniqueItems": true
433437
},
434438
"volumes_from": {
435439
"type": "array",
436-
"items": {"type": "string"}
440+
"items": {"type": "string"},
441+
"uniqueItems": true
437442
},
438443
"working_dir": {"type": "string"}
439444
},
@@ -828,7 +833,8 @@
828833

829834
"list_of_strings": {
830835
"type": "array",
831-
"items": {"type": "string"}
836+
"items": {"type": "string"},
837+
"uniqueItems": true
832838
},
833839

834840
"list_or_dict": {
@@ -842,7 +848,7 @@
842848
},
843849
"additionalProperties": false
844850
},
845-
{"type": "array", "items": {"type": "string"}}
851+
{"type": "array", "items": {"type": "string"}, "uniqueItems": true}
846852
]
847853
},
848854

0 commit comments

Comments
 (0)