Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check overlapping paths separately for directories and contents #343

Merged
merged 5 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion examples/overlapping-dir/README.md

This file was deleted.

19 changes: 0 additions & 19 deletions examples/overlapping-dir/vendir.yml

This file was deleted.

45 changes: 30 additions & 15 deletions pkg/vendir/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,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)

Expand All @@ -220,13 +223,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)
}
}

Expand Down Expand Up @@ -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
}
}

Expand Down
110 changes: 101 additions & 9 deletions test/e2e/overlapping_dir_err_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Loading