Skip to content

Commit 3fd2e85

Browse files
yasminvalimyasminvalim
committed
v3_6_experimental: fix path conflict validation bug
 Fix validation to properly detect when a file/link path conflicts with a parent directory required by another storage entry. Fixes: #145
1 parent b98238a commit 3fd2e85

File tree

3 files changed

+197
-1
lines changed

3 files changed

+197
-1
lines changed

‎config/shared/errors/errors.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ var (
8585
ErrInvalidProxy = errors.New("proxies must be http(s)")
8686
ErrInsecureProxy = errors.New("insecure plaintext HTTP proxy specified for HTTPS resources")
8787
ErrPathConflictsSystemd = errors.New("path conflicts with systemd unit or dropin")
88+
ErrPathAlreadyExists = errors.New("path already exists")
89+
ErrMissLabeledDir = errors.New("parent directory path matches configured file, check path, and ensure parent directory is configured")
8890
ErrCexWithClevis = errors.New("cannot use cex with clevis")
8991
ErrCexWithKeyFile = errors.New("cannot use key file with cex")
9092

‎config/v3_6_experimental/types/config.go‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
package types
1616

1717
import (
18+
"fmt"
19+
"path/filepath"
20+
"sort"
21+
"strings"
22+
1823
"github.com/coreos/ignition/v2/config/shared/errors"
1924
"github.com/coreos/ignition/v2/config/util"
2025

@@ -61,5 +66,79 @@ func (cfg Config) Validate(c path.ContextPath) (r report.Report) {
6166
r.AddOnError(c.Append("storage", "links", i, "path"), errors.ErrPathConflictsSystemd)
6267
}
6368
}
64-
return
69+
70+
r.Merge(cfg.validateParents(c))
71+
72+
return r
73+
}
74+
75+
func (cfg Config) validateParents(c path.ContextPath) report.Report {
76+
type entry struct {
77+
Path string
78+
Field string
79+
Index int
80+
}
81+
82+
var entries []entry
83+
paths := make(map[string]entry)
84+
r := report.Report{}
85+
86+
for i, f := range cfg.Storage.Files {
87+
if _, exists := paths[f.Path]; exists {
88+
r.AddOnError(c.Append("storage", "files", i, "path"), errors.ErrPathAlreadyExists)
89+
} else {
90+
paths[f.Path] = entry{Path: f.Path, Field: "files", Index: i}
91+
entries = append(entries, entry{Path: f.Path, Field: "files", Index: i})
92+
}
93+
}
94+
95+
for i, d := range cfg.Storage.Directories {
96+
if _, exists := paths[d.Path]; exists {
97+
r.AddOnError(c.Append("storage", "directories", i, "path"), errors.ErrPathAlreadyExists)
98+
} else {
99+
paths[d.Path] = entry{Path: d.Path, Field: "directories", Index: i}
100+
entries = append(entries, entry{Path: d.Path, Field: "directories", Index: i})
101+
}
102+
}
103+
104+
for i, l := range cfg.Storage.Links {
105+
if _, exists := paths[l.Path]; exists {
106+
r.AddOnError(c.Append("storage", "links", i, "path"), errors.ErrPathAlreadyExists)
107+
} else {
108+
paths[l.Path] = entry{Path: l.Path, Field: "links", Index: i}
109+
entries = append(entries, entry{Path: l.Path, Field: "links", Index: i})
110+
}
111+
}
112+
113+
sort.Slice(entries, func(i, j int) bool {
114+
return len(strings.Split(strings.Trim(entries[i].Path, "/"), "/")) < len(strings.Split(strings.Trim(entries[j].Path, "/"), "/"))
115+
})
116+
117+
// Check for path conflicts where a file/link is used as a parent directory
118+
for _, entry := range entries {
119+
// Skip root path
120+
if entry.Path == "/" {
121+
continue
122+
}
123+
124+
// Check if any parent path exists as a file or link
125+
parentPath := entry.Path
126+
for {
127+
parentPath = filepath.Dir(parentPath)
128+
if parentPath == "/" || parentPath == "." {
129+
break
130+
}
131+
132+
if parentEntry, exists := paths[parentPath]; exists {
133+
// If the parent is not a directory, it's a conflict
134+
if parentEntry.Field != "directories" {
135+
errorMsg := fmt.Errorf("invalid entry at path %s: %w", entry.Path, errors.ErrMissLabeledDir)
136+
r.AddOnError(c.Append("storage", entry.Field, entry.Index, "path"), errorMsg)
137+
break
138+
}
139+
}
140+
}
141+
}
142+
143+
return r
65144
}

‎config/v3_6_experimental/types/config_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package types
1616

1717
import (
18+
"fmt"
1819
"reflect"
1920
"testing"
2021

@@ -248,6 +249,120 @@ func TestConfigValidation(t *testing.T) {
248249
},
249250
},
250251
},
252+
253+
// test 7: file path conflicts with another file path, should error
254+
{
255+
in: Config{
256+
Storage: Storage{
257+
Files: []File{
258+
{Node: Node{Path: "/foo/bar"}},
259+
{Node: Node{Path: "/foo/bar/baz"}},
260+
},
261+
},
262+
},
263+
out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir),
264+
at: path.New("json", "storage", "files", 1, "path"),
265+
},
266+
// test 8: file path conflicts with link path, should error
267+
{
268+
in: Config{
269+
Storage: Storage{
270+
Files: []File{
271+
{Node: Node{Path: "/foo/bar"}},
272+
},
273+
Links: []Link{
274+
{Node: Node{Path: "/foo/bar/baz"}},
275+
},
276+
},
277+
},
278+
out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir),
279+
at: path.New("json", "storage", "links", 0, "path"),
280+
},
281+
282+
// test 9: file path conflicts with directory path, should error
283+
{
284+
in: Config{
285+
Storage: Storage{
286+
Files: []File{
287+
{Node: Node{Path: "/foo/bar"}},
288+
},
289+
Directories: []Directory{
290+
{Node: Node{Path: "/foo/bar/baz"}},
291+
},
292+
},
293+
},
294+
out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir),
295+
at: path.New("json", "storage", "directories", 0, "path"),
296+
},
297+
298+
// test 10: non-conflicting scenarios with same parent directory, should not error
299+
{
300+
in: Config{
301+
Storage: Storage{
302+
Files: []File{
303+
{Node: Node{Path: "/foo/bar/baz"}},
304+
},
305+
Directories: []Directory{
306+
{Node: Node{Path: "/foo/bar"}},
307+
},
308+
},
309+
},
310+
},
311+
// test 11: non-conflicting scenarios with a link, should not error
312+
{
313+
in: Config{
314+
Storage: Storage{
315+
Files: []File{
316+
{Node: Node{Path: "/foo/bar"}},
317+
},
318+
Links: []Link{
319+
{Node: Node{Path: "/baz/qux"}},
320+
},
321+
},
322+
},
323+
},
324+
// test 12: deep path conflict - file conflicts with deeper nested path, should error
325+
{
326+
in: Config{
327+
Storage: Storage{
328+
Files: []File{
329+
{Node: Node{Path: "/foo/bar"}},
330+
{Node: Node{Path: "/foo/bar/baz/deep/file"}},
331+
},
332+
},
333+
},
334+
out: fmt.Errorf("invalid entry at path /foo/bar/baz/deep/file: %w", errors.ErrMissLabeledDir),
335+
at: path.New("json", "storage", "files", 1, "path"),
336+
},
337+
// test 13: multiple levels with directories, should not error
338+
{
339+
in: Config{
340+
Storage: Storage{
341+
Directories: []Directory{
342+
{Node: Node{Path: "/foo"}},
343+
{Node: Node{Path: "/foo/bar"}},
344+
},
345+
Files: []File{
346+
{Node: Node{Path: "/foo/bar/baz"}},
347+
},
348+
},
349+
},
350+
},
351+
// test 14: link conflicts with file parent, should error
352+
{
353+
in: Config{
354+
Storage: Storage{
355+
Links: []Link{
356+
{Node: Node{Path: "/foo/bar"}},
357+
},
358+
Files: []File{
359+
{Node: Node{Path: "/foo/bar/child"}},
360+
},
361+
},
362+
},
363+
out: fmt.Errorf("invalid entry at path /foo/bar/child: %w", errors.ErrMissLabeledDir),
364+
at: path.New("json", "storage", "files", 0, "path"),
365+
},
251366
}
252367
for i, test := range tests {
253368
r := test.in.Validate(path.New("json"))

0 commit comments

Comments
 (0)