diff --git a/config/shared/errors/errors.go b/config/shared/errors/errors.go index 85d110112..9edbb3d3a 100644 --- a/config/shared/errors/errors.go +++ b/config/shared/errors/errors.go @@ -85,6 +85,8 @@ var ( ErrInvalidProxy = errors.New("proxies must be http(s)") ErrInsecureProxy = errors.New("insecure plaintext HTTP proxy specified for HTTPS resources") ErrPathConflictsSystemd = errors.New("path conflicts with systemd unit or dropin") + ErrPathAlreadyExists = errors.New("path already exists") + ErrMissLabeledDir = errors.New("parent directory path matches configured file, check path, and ensure parent directory is configured") ErrCexWithClevis = errors.New("cannot use cex with clevis") ErrCexWithKeyFile = errors.New("cannot use key file with cex") diff --git a/config/v3_6_experimental/types/config.go b/config/v3_6_experimental/types/config.go index 9428b0bb2..5fb06b6fa 100644 --- a/config/v3_6_experimental/types/config.go +++ b/config/v3_6_experimental/types/config.go @@ -15,6 +15,11 @@ package types import ( + "fmt" + "path/filepath" + "sort" + "strings" + "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" @@ -61,5 +66,80 @@ func (cfg Config) Validate(c path.ContextPath) (r report.Report) { r.AddOnError(c.Append("storage", "links", i, "path"), errors.ErrPathConflictsSystemd) } } - return + + r.Merge(cfg.validateParents(c)) + + return r +} + +func (cfg Config) validateParents(c path.ContextPath) report.Report { + type entry struct { + Path string + Field string + Index int + } + + var entries []entry + paths := make(map[string]entry) + r := report.Report{} + + for i, f := range cfg.Storage.Files { + if _, exists := paths[f.Path]; exists { + r.AddOnError(c.Append("storage", "files", i, "path"), errors.ErrPathAlreadyExists) + } else { + paths[f.Path] = entry{Path: f.Path, Field: "files", Index: i} + entries = append(entries, entry{Path: f.Path, Field: "files", Index: i}) + } + } + + for i, d := range cfg.Storage.Directories { + if _, exists := paths[d.Path]; exists { + r.AddOnError(c.Append("storage", "directories", i, "path"), errors.ErrPathAlreadyExists) + } else { + paths[d.Path] = entry{Path: d.Path, Field: "directories", Index: i} + entries = append(entries, entry{Path: d.Path, Field: "directories", Index: i}) + } + } + + for i, l := range cfg.Storage.Links { + if _, exists := paths[l.Path]; exists { + r.AddOnError(c.Append("storage", "links", i, "path"), errors.ErrPathAlreadyExists) + } else { + paths[l.Path] = entry{Path: l.Path, Field: "links", Index: i} + entries = append(entries, entry{Path: l.Path, Field: "links", Index: i}) + } + } + + sort.Slice(entries, func(i, j int) bool { + return len(strings.Split(strings.Trim(entries[i].Path, "/"), "/")) < len(strings.Split(strings.Trim(entries[j].Path, "/"), "/")) + }) + + // Check for path conflicts where a file/link is used as a parent directory + for _, entry := range entries { + // Skip root path + if entry.Path == "/" { + continue + } + + // Check if any parent path exists as a file or link + parentPath := entry.Path + for { + newParentPath := filepath.Dir(parentPath) + if newParentPath == parentPath { + break + } + parentPath = newParentPath + + if parentEntry, exists := paths[parentPath]; exists { + // If the parent is not a directory, it's a conflict + if parentEntry.Field != "directories" { + errorMsg := fmt.Errorf("invalid entry at path %s: %w", entry.Path, errors.ErrMissLabeledDir) + r.AddOnError(c.Append("storage", entry.Field, entry.Index, "path"), errorMsg) + break + } + } + } + } + + return r } diff --git a/config/v3_6_experimental/types/config_test.go b/config/v3_6_experimental/types/config_test.go index 3d82627b2..558431bef 100644 --- a/config/v3_6_experimental/types/config_test.go +++ b/config/v3_6_experimental/types/config_test.go @@ -15,6 +15,7 @@ package types import ( + "fmt" "reflect" "testing" @@ -248,6 +249,120 @@ func TestConfigValidation(t *testing.T) { }, }, }, + + // test 7: file path conflicts with another file path, should error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar"}}, + {Node: Node{Path: "/foo/bar/baz"}}, + }, + }, + }, + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "files", 1, "path"), + }, + // test 8: file path conflicts with link path, should error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar"}}, + }, + Links: []Link{ + {Node: Node{Path: "/foo/bar/baz"}}, + }, + }, + }, + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "links", 0, "path"), + }, + + // test 9: file path conflicts with directory path, should error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar"}}, + }, + Directories: []Directory{ + {Node: Node{Path: "/foo/bar/baz"}}, + }, + }, + }, + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "directories", 0, "path"), + }, + + // test 10: non-conflicting scenarios with same parent directory, should not error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar/baz"}}, + }, + Directories: []Directory{ + {Node: Node{Path: "/foo/bar"}}, + }, + }, + }, + }, + // test 11: non-conflicting scenarios with a link, should not error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar"}}, + }, + Links: []Link{ + {Node: Node{Path: "/baz/qux"}}, + }, + }, + }, + }, + // test 12: deep path conflict - file conflicts with deeper nested path, should error + { + in: Config{ + Storage: Storage{ + Files: []File{ + {Node: Node{Path: "/foo/bar"}}, + {Node: Node{Path: "/foo/bar/baz/deep/file"}}, + }, + }, + }, + out: fmt.Errorf("invalid entry at path /foo/bar/baz/deep/file: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "files", 1, "path"), + }, + // test 13: multiple levels with directories, should not error + { + in: Config{ + Storage: Storage{ + Directories: []Directory{ + {Node: Node{Path: "/foo"}}, + {Node: Node{Path: "/foo/bar"}}, + }, + Files: []File{ + {Node: Node{Path: "/foo/bar/baz"}}, + }, + }, + }, + }, + // test 14: link conflicts with file parent, should error + { + in: Config{ + Storage: Storage{ + Links: []Link{ + {Node: Node{Path: "/foo/bar"}}, + }, + Files: []File{ + {Node: Node{Path: "/foo/bar/child"}}, + }, + }, + }, + out: fmt.Errorf("invalid entry at path /foo/bar/child: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "files", 0, "path"), + }, } for i, test := range tests { r := test.in.Validate(path.New("json")) diff --git a/docs/release-notes.md b/docs/release-notes.md index 52696cd70..93721cf89 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -14,6 +14,8 @@ nav_order: 9 ### Changes +- Fix validation to properly detect when a file/link path conflicts with a parent directory. + ### Bug fixes - Fix fetch-offline for Oracle Cloud Infrastructure