From afcb1470ee90d7d70c77da721aad1c3ff2e313b4 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 24 Jan 2024 15:07:42 +0100 Subject: [PATCH 1/2] process `extends` recursively Signed-off-by: Nicolas De Loof --- Makefile | 4 +- loader/extends.go | 181 ++++++++++++++++++++++++++-------------------- loader/loader.go | 22 +++++- paths/extends.go | 25 +++++++ paths/resolve.go | 21 +++++- 5 files changed, 168 insertions(+), 85 deletions(-) create mode 100644 paths/extends.go diff --git a/Makefile b/Makefile index e31b8236..437fd5bd 100644 --- a/Makefile +++ b/Makefile @@ -15,8 +15,8 @@ IMAGE_PREFIX=composespec/conformance-tests- .PHONY: build -build: ## Run tests - go build ./... +build: ## Build command line + go build -o compose-spec cmd/main.go .PHONY: test test: ## Run tests diff --git a/loader/extends.go b/loader/extends.go index 97c6ca36..ebb60251 100644 --- a/loader/extends.go +++ b/loader/extends.go @@ -35,96 +35,119 @@ func ApplyExtends(ctx context.Context, dict map[string]any, opts *Options, track if !ok { return fmt.Errorf("services must be a mapping") } - for name, s := range services { - service, ok := s.(map[string]any) - if !ok { - return fmt.Errorf("services.%s must be a mapping", name) - } - x, ok := service["extends"] - if !ok { - continue - } - ct, err := tracker.Add(ctx.Value(consts.ComposeFileKey{}).(string), name) + for name := range services { + merged, err := applyServiceExtends(ctx, name, services, opts, tracker, post...) if err != nil { return err } - var ( - ref string - file any - ) - switch v := x.(type) { - case map[string]any: - ref = v["service"].(string) - file = v["file"] - case string: - ref = v + services[name] = merged + } + dict["services"] = services + return nil +} + +func applyServiceExtends(ctx context.Context, name string, services map[string]any, opts *Options, tracker *cycleTracker, post ...PostProcessor) (any, error) { + s := services[name] + service, ok := s.(map[string]any) + if !ok { + return nil, fmt.Errorf("services.%s must be a mapping", name) + } + extends, ok := service["extends"] + if !ok { + return s, nil + } + filename := ctx.Value(consts.ComposeFileKey{}).(string) + tracker, err := tracker.Add(filename, name) + if err != nil { + return nil, err + } + var ( + ref string + file any + ) + switch v := extends.(type) { + case map[string]any: + ref = v["service"].(string) + file = v["file"] + case string: + ref = v + } + + var base any + if file != nil { + path := file.(string) + services, err = getExtendsBaseFromFile(ctx, ref, path, opts, tracker) + if err != nil { + return nil, err + } + } else { + _, ok := services[ref] + if !ok { + return nil, fmt.Errorf("cannot extend service %q in %s: service not found", name, filename) } + } + // recursively apply `extends` + base, err = applyServiceExtends(ctx, ref, services, opts, tracker, post...) + if err != nil { + return nil, err + } - var base any - if file != nil { - path := file.(string) - for _, loader := range opts.ResourceLoaders { - if !loader.Accept(path) { - continue - } - local, err := loader.Load(ctx, path) - if err != nil { - return err - } - localdir := filepath.Dir(local) - relworkingdir := loader.Dir(path) + source := deepClone(base).(map[string]any) + for _, processor := range post { + processor.Apply(map[string]any{ + "services": map[string]any{ + name: source, + }, + }) + } + merged, err := override.ExtendService(source, service) + if err != nil { + return nil, err + } + delete(merged, "extends") + return merged, nil +} - extendsOpts := opts.clone() - extendsOpts.ResourceLoaders = append([]ResourceLoader{}, opts.ResourceLoaders...) - // replace localResourceLoader with a new flavour, using extended file base path - extendsOpts.ResourceLoaders[len(opts.ResourceLoaders)-1] = localResourceLoader{ - WorkingDir: localdir, - } - extendsOpts.ResolvePaths = true - extendsOpts.SkipNormalization = true - extendsOpts.SkipConsistencyCheck = true - extendsOpts.SkipInclude = true - source, err := loadYamlModel(ctx, types.ConfigDetails{ - WorkingDir: relworkingdir, - ConfigFiles: []types.ConfigFile{ - {Filename: local}, - }, - }, extendsOpts, ct, nil) - if err != nil { - return err - } - services := source["services"].(map[string]any) - base, ok = services[ref] - if !ok { - return fmt.Errorf("cannot extend service %q in %s: service not found", name, path) - } - } - if base == nil { - return fmt.Errorf("cannot read %s", path) - } - } else { - base, ok = services[ref] - if !ok { - return fmt.Errorf("cannot extend service %q in %s: service not found", name, "filename") // TODO track filename - } +func getExtendsBaseFromFile(ctx context.Context, name string, path string, opts *Options, ct *cycleTracker) (map[string]any, error) { + for _, loader := range opts.ResourceLoaders { + if !loader.Accept(path) { + continue } - source := deepClone(base).(map[string]any) - for _, processor := range post { - processor.Apply(map[string]any{ - "services": map[string]any{ - name: source, - }, - }) + local, err := loader.Load(ctx, path) + if err != nil { + return nil, err } - merged, err := override.ExtendService(source, service) + localdir := filepath.Dir(local) + relworkingdir := loader.Dir(path) + + extendsOpts := opts.clone() + // replace localResourceLoader with a new flavour, using extended file base path + extendsOpts.ResourceLoaders = append(opts.RemoteResourceLoaders(), localResourceLoader{ + WorkingDir: localdir, + }) + extendsOpts.ResolvePaths = true + extendsOpts.SkipNormalization = true + extendsOpts.SkipConsistencyCheck = true + extendsOpts.SkipInclude = true + extendsOpts.SkipExtends = true // we manage extends recursively based on raw service definition + extendsOpts.SkipValidation = true // we validate the merge result + source, err := loadYamlModel(ctx, types.ConfigDetails{ + WorkingDir: relworkingdir, + ConfigFiles: []types.ConfigFile{ + {Filename: local}, + }, + }, extendsOpts, ct, nil) if err != nil { - return err + return nil, err } - delete(merged, "extends") - services[name] = merged + services := source["services"].(map[string]any) + _, ok := services[name] + if !ok { + return nil, fmt.Errorf("cannot extend service %q in %s: service not found", name, path) + } + return services, nil } - dict["services"] = services - return nil + return nil, fmt.Errorf("cannot read %s", path) } func deepClone(value any) any { diff --git a/loader/loader.go b/loader/loader.go index 5100d9f5..e1aa7663 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -40,6 +40,7 @@ import ( "github.com/compose-spec/compose-go/v2/types" "github.com/compose-spec/compose-go/v2/validation" "github.com/mitchellh/mapstructure" + "github.com/sirupsen/logrus" "gopkg.in/yaml.v3" ) @@ -87,6 +88,21 @@ type ResourceLoader interface { Dir(path string) string } +// RemoteResourceLoaders excludes localResourceLoader from ResourceLoaders +func (o Options) RemoteResourceLoaders() []ResourceLoader { + var loaders []ResourceLoader + for i, loader := range o.ResourceLoaders { + if _, ok := loader.(localResourceLoader); ok { + if i != len(o.ResourceLoaders)-1 { + logrus.Warning("misconfiguration of ResourceLoaders: localResourceLoader should be last") + } + continue + } + loaders = append(loaders, loader) + } + return loaders +} + type localResourceLoader struct { WorkingDir string } @@ -399,7 +415,11 @@ func loadYamlModel(ctx context.Context, config types.ConfigDetails, opts *Option } if opts.ResolvePaths { - err = paths.ResolveRelativePaths(dict, config.WorkingDir) + var remotes []paths.RemoteResource + for _, loader := range opts.RemoteResourceLoaders() { + remotes = append(remotes, loader.Accept) + } + err = paths.ResolveRelativePaths(dict, config.WorkingDir, remotes) if err != nil { return nil, err } diff --git a/paths/extends.go b/paths/extends.go new file mode 100644 index 00000000..aa61a9f9 --- /dev/null +++ b/paths/extends.go @@ -0,0 +1,25 @@ +/* + Copyright 2020 The Compose Specification Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package paths + +func (r *relativePathsResolver) absExtendsPath(value any) (any, error) { + v := value.(string) + if r.isRemoteResource(v) { + return v, nil + } + return r.absPath(v) +} diff --git a/paths/resolve.go b/paths/resolve.go index d7e100f7..ecfa0e9b 100644 --- a/paths/resolve.go +++ b/paths/resolve.go @@ -28,13 +28,16 @@ import ( type resolver func(any) (any, error) // ResolveRelativePaths make relative paths absolute -func ResolveRelativePaths(project map[string]any, base string) error { - r := relativePathsResolver{workingDir: base} +func ResolveRelativePaths(project map[string]any, base string, remotes []RemoteResource) error { + r := relativePathsResolver{ + workingDir: base, + remotes: remotes, + } r.resolvers = map[tree.Path]resolver{ "services.*.build.context": r.absContextPath, "services.*.build.additional_contexts.*": r.absContextPath, "services.*.env_file.*.path": r.absPath, - "services.*.extends.file": r.absPath, + "services.*.extends.file": r.absExtendsPath, "services.*.develop.watch.*.path": r.absPath, "services.*.volumes.*": r.absVolumeMount, "configs.*.file": r.maybeUnixPath, @@ -48,11 +51,23 @@ func ResolveRelativePaths(project map[string]any, base string) error { return err } +type RemoteResource func(path string) bool + type relativePathsResolver struct { workingDir string + remotes []RemoteResource resolvers map[tree.Path]resolver } +func (r *relativePathsResolver) isRemoteResource(path string) bool { + for _, remote := range r.remotes { + if remote(path) { + return true + } + } + return false +} + func (r *relativePathsResolver) resolveRelativePaths(value any, p tree.Path) (any, error) { for pattern, resolver := range r.resolvers { if p.Matches(pattern) { From 25f05966ff47ce7530d174ab46c274a1f39381e9 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Thu, 25 Jan 2024 14:57:45 +0100 Subject: [PATCH 2/2] manage corner case using an empty (null) service definition Signed-off-by: Nicolas De Loof --- loader/extends.go | 6 ++++++ loader/extends_test.go | 28 ++++++++++++++++++++++++++++ loader/testdata/extends/base.yaml | 1 + transform/services.go | 8 ++++++-- 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/loader/extends.go b/loader/extends.go index ebb60251..3982cd67 100644 --- a/loader/extends.go +++ b/loader/extends.go @@ -48,6 +48,9 @@ func ApplyExtends(ctx context.Context, dict map[string]any, opts *Options, track func applyServiceExtends(ctx context.Context, name string, services map[string]any, opts *Options, tracker *cycleTracker, post ...PostProcessor) (any, error) { s := services[name] + if s == nil { + return nil, nil + } service, ok := s.(map[string]any) if !ok { return nil, fmt.Errorf("services.%s must be a mapping", name) @@ -92,6 +95,9 @@ func applyServiceExtends(ctx context.Context, name string, services map[string]a return nil, err } + if base == nil { + return service, nil + } source := deepClone(base).(map[string]any) for _, processor := range post { processor.Apply(map[string]any{ diff --git a/loader/extends_test.go b/loader/extends_test.go index e5bafaa1..4c0686c3 100644 --- a/loader/extends_test.go +++ b/loader/extends_test.go @@ -142,3 +142,31 @@ services: assert.NilError(t, err) assert.Equal(t, p.Services["test"].Build.Context, filepath.Join("testdata", "extends")) } + +func TestExtendsNil(t *testing.T) { + yaml := ` +name: test-extends-port +services: + test: + image: test + extends: + file: testdata/extends/base.yaml + service: nil +` + abs, err := filepath.Abs(".") + assert.NilError(t, err) + + _, err = LoadWithContext(context.Background(), types.ConfigDetails{ + ConfigFiles: []types.ConfigFile{ + { + Content: []byte(yaml), + Filename: "(inline)", + }, + }, + WorkingDir: abs, + }, func(options *Options) { + options.ResolvePaths = false + options.SkipValidation = true + }) + assert.NilError(t, err) +} diff --git a/loader/testdata/extends/base.yaml b/loader/testdata/extends/base.yaml index 3c2f6930..3886659d 100644 --- a/loader/testdata/extends/base.yaml +++ b/loader/testdata/extends/base.yaml @@ -22,3 +22,4 @@ services: file: sibling.yaml service: test + nil: #left intentionally empty diff --git a/transform/services.go b/transform/services.go index 5a290efa..c1411452 100644 --- a/transform/services.go +++ b/transform/services.go @@ -21,8 +21,12 @@ import ( ) func transformService(data any, p tree.Path) (any, error) { - value := data.(map[string]any) - return transformMapping(value, p) + switch value := data.(type) { + case map[string]any: + return transformMapping(value, p) + default: + return value, nil + } } func transformServiceNetworks(data any, _ tree.Path) (any, error) {