From 0a6abbd5e7773271dc4f7c9bde1b5434cc98fa00 Mon Sep 17 00:00:00 2001 From: German Lashevich Date: Sun, 24 Dec 2023 20:18:34 +0100 Subject: [PATCH 1/5] fix: check overlapping paths separately for directories and contents Signed-off-by: German Lashevich --- pkg/vendir/config/config.go | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/vendir/config/config.go b/pkg/vendir/config/config.go index a591d338..ea84c1e7 100644 --- a/pkg/vendir/config/config.go +++ b/pkg/vendir/config/config.go @@ -9,9 +9,10 @@ import ( "reflect" "strings" - "carvel.dev/vendir/pkg/vendir/version" semver "github.com/hashicorp/go-version" "sigs.k8s.io/yaml" + + "carvel.dev/vendir/pkg/vendir/version" ) const ( @@ -259,20 +260,34 @@ func (c Config) Lock(lockConfig LockConfig) error { } func (c Config) checkOverlappingPaths() error { + checkPaths := func(paths []string) error { + for i, path := range paths { + for i2, path2 := range paths { + if i != i2 && strings.HasPrefix(path2+string(filepath.Separator), path+string(filepath.Separator)) { + return fmt.Errorf("Expected to not manage overlapping paths: '%s' and '%s'", path2, path) + } + } + } + return nil + } + paths := []string{} + for _, dir := range c.Directories { + paths = append(paths, dir.Path) + } + + if err := checkPaths(paths); err != nil { + return err + } for _, dir := range c.Directories { - for _, con := range dir.Contents { - paths = append(paths, filepath.Join(dir.Path, con.Path)) + paths = []string{} + for _, cont := range dir.Contents { + paths = append(paths, filepath.Join(dir.Path, cont.Path)) } - } - for i, path := range paths { - for i2, path2 := range paths { - if i != i2 && strings.HasPrefix(path2+string(filepath.Separator), path+string(filepath.Separator)) { - return fmt.Errorf("Expected to not "+ - "manage overlapping paths: '%s' and '%s'", path2, path) - } + if err := checkPaths(paths); err != nil { + return err } } From 59bb7bb5843b8a0a134a717f021b9cbceac05539 Mon Sep 17 00:00:00 2001 From: German Lashevich Date: Sun, 24 Dec 2023 21:03:47 +0100 Subject: [PATCH 2/5] fix: do not split directory into multiple when filtering paths Signed-off-by: German Lashevich --- pkg/vendir/config/config.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/vendir/config/config.go b/pkg/vendir/config/config.go index ea84c1e7..9a294a56 100644 --- a/pkg/vendir/config/config.go +++ b/pkg/vendir/config/config.go @@ -209,6 +209,9 @@ func (c Config) Subset(paths []string) (Config, error) { } for _, dir := range c.Directories { + newDir := dir + newDir.Contents = []DirectoryContents{} + for _, con := range dir.Contents { entirePath := filepath.Join(dir.Path, con.Path) @@ -221,13 +224,11 @@ func (c Config) Subset(paths []string) (Config, error) { } pathsToSeen[entirePath] = true - newCon := con // copy (but not deep unfortunately) - newCon.Path = con.Path + newDir.Contents = append(newDir.Contents, con) + } - result.Directories = append(result.Directories, Directory{ - Path: dir.Path, - Contents: []DirectoryContents{newCon}, - }) + if len(newDir.Contents) > 0 { + result.Directories = append(result.Directories, newDir) } } From 78ae3fbb13888546d378a89750458d3074f1a7df Mon Sep 17 00:00:00 2001 From: German Lashevich Date: Wed, 10 Jan 2024 21:49:03 +0100 Subject: [PATCH 3/5] refactor: revert changes to grouping of imports Signed-off-by: German Lashevich --- pkg/vendir/config/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/vendir/config/config.go b/pkg/vendir/config/config.go index 9a294a56..80aec701 100644 --- a/pkg/vendir/config/config.go +++ b/pkg/vendir/config/config.go @@ -9,10 +9,9 @@ import ( "reflect" "strings" + "carvel.dev/vendir/pkg/vendir/version" semver "github.com/hashicorp/go-version" "sigs.k8s.io/yaml" - - "carvel.dev/vendir/pkg/vendir/version" ) const ( From 5499ba36d10e7d4fd5e24dadd4a6c6d0a70a5180 Mon Sep 17 00:00:00 2001 From: German Lashevich Date: Wed, 10 Jan 2024 22:31:13 +0100 Subject: [PATCH 4/5] test: add e2e tests for overlapping paths Signed-off-by: German Lashevich --- test/e2e/overlapping_dir_err_test.go | 110 ++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 9 deletions(-) diff --git a/test/e2e/overlapping_dir_err_test.go b/test/e2e/overlapping_dir_err_test.go index 16a9fb97..e5d77082 100644 --- a/test/e2e/overlapping_dir_err_test.go +++ b/test/e2e/overlapping_dir_err_test.go @@ -6,19 +6,111 @@ package e2e import ( "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestOverlappingDirErr(t *testing.T) { +func TestOverlappingPathsErr(t *testing.T) { + tests := []struct { + name string + description string + yaml string + expectedErr string + }{ + { + name: "contents-paths", + description: "syncing config with overlapping contents paths", + yaml: ` +apiVersion: vendir.k14s.io/v1alpha1 +kind: Config +directories: +- path: vendor + contents: + - path: foo/bar/baz + inline: + paths: + test.txt: love + - path: foo + inline: + paths: + test.txt: peace +`, + expectedErr: `vendir: Error: Parsing resource config '-': + Unmarshaling config: + Validating config: + Expected to not manage overlapping paths: 'vendor/foo/bar/baz' and 'vendor/foo' +`, + }, + { + name: "directories-paths", + description: "syncing config with overlapping directories paths", + yaml: ` +apiVersion: vendir.k14s.io/v1alpha1 +kind: Config +directories: +- path: vendor + contents: + - path: foo/bar/baz + inline: + paths: + test.txt: love +- path: vendor/foo + contents: + - path: bar + inline: + paths: + test.txt: peace +`, + expectedErr: `vendir: Error: Parsing resource config '-': + Unmarshaling config: + Validating config: + Expected to not manage overlapping paths: 'vendor/foo' and 'vendor' +`, + }, + { + name: "same-directories-paths", + description: "syncing config with the same directories paths", + yaml: ` +apiVersion: vendir.k14s.io/v1alpha1 +kind: Config +directories: +- path: vendor + contents: + - path: foo + inline: + paths: + test.txt: love +- path: vendor + contents: + - path: bar + inline: + paths: + test.txt: peace +`, + expectedErr: `vendir: Error: Parsing resource config '-': + Unmarshaling config: + Validating config: + Expected to not manage overlapping paths: 'vendor' and 'vendor' +`, + }, + } + env := BuildEnv(t) vendir := Vendir{t, env.BinaryPath, Logger{}} - path := "../../examples/overlapping-dir" - - _, err := vendir.RunWithOpts([]string{"sync"}, RunOpts{Dir: path, AllowError: true}) - if err == nil { - t.Fatalf("Expected err") - } - if !strings.Contains(err.Error(), "Expected to not manage overlapping paths") { - t.Fatalf("Expected overlapping err: %s", err) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := vendir.RunWithOpts( + []string{"sync", "-f", "-"}, + RunOpts{ + Dir: t.TempDir(), + StdinReader: strings.NewReader(test.yaml), + AllowError: true, + }, + ) + require.Error(t, err, "Expected to err while %s", test.description) + assert.ErrorContains(t, err, test.expectedErr) + }) } } From 39ead71d8f07df437d788f10fce38c101aaa5072 Mon Sep 17 00:00:00 2001 From: German Lashevich Date: Wed, 10 Jan 2024 22:32:13 +0100 Subject: [PATCH 5/5] chore: remove overlapping-dir example as unused Signed-off-by: German Lashevich --- examples/overlapping-dir/README.md | 1 - examples/overlapping-dir/vendir.yml | 19 ------------------- 2 files changed, 20 deletions(-) delete mode 100644 examples/overlapping-dir/README.md delete mode 100644 examples/overlapping-dir/vendir.yml diff --git a/examples/overlapping-dir/README.md b/examples/overlapping-dir/README.md deleted file mode 100644 index c92f0de0..00000000 --- a/examples/overlapping-dir/README.md +++ /dev/null @@ -1 +0,0 @@ -`vendir sync` will fail because `vendir.yml` specifies overlapping paths. diff --git a/examples/overlapping-dir/vendir.yml b/examples/overlapping-dir/vendir.yml deleted file mode 100644 index c3a1464e..00000000 --- a/examples/overlapping-dir/vendir.yml +++ /dev/null @@ -1,19 +0,0 @@ -apiVersion: vendir.k14s.io/v1alpha1 -kind: Config -directories: -- path: vendor - contents: - - path: github.com/cloudfoundry/cf-k8s-networking - git: - url: https://github.com/cloudfoundry/cf-k8s-networking - ref: 2b009b61fa8afb330a4302c694ee61b11104c54c - includePaths: - - cfroutesync/crds/**/* - - install/ytt/networking/**/* - - path: github.com - git: - url: https://github.com/cloudfoundry/cf-k8s-networking - ref: 2b009b61fa8afb330a4302c694ee61b11104c54c - includePaths: - - cfroutesync/crds/**/* - - install/ytt/networking/**/*