Merge pull request #343 from mykso/fix-overlapping-paths-check

fix: check overlapping paths separately for directories and contents

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)
 		}
 	}
 
@@ -260,20 +261,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
 		}
 	}
 

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)
+		})
 	}
 }