From 61dee39f2bd4670c27b5d035827c87f63c066b96 Mon Sep 17 00:00:00 2001 From: Yasmin Valim Date: Thu, 11 Apr 2024 10:07:44 -0300 Subject: [PATCH] v3_5_experimental: add validation, unit tests and error --- config/shared/errors/errors.go | 3 +- config/v3_5_experimental/types/config.go | 79 +++++++++++-------- config/v3_5_experimental/types/config_test.go | 15 ++-- docs/release-notes.md | 1 + 4 files changed, 55 insertions(+), 43 deletions(-) diff --git a/config/shared/errors/errors.go b/config/shared/errors/errors.go index 1f3ec0a95..da26eaa28 100644 --- a/config/shared/errors/errors.go +++ b/config/shared/errors/errors.go @@ -85,7 +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") - ErrPathConflictsParentDir = errors.New("path conflicts with parent directory of another file, link, or directory") + ErrPathAlreadyExists = errors.New("path already exists") + ErrMissLabeledDir = errors.New("parent directory path matches configured file, check path, and ensure parent directory is configured") // Systemd section errors ErrInvalidSystemdExt = errors.New("invalid systemd unit extension") diff --git a/config/v3_5_experimental/types/config.go b/config/v3_5_experimental/types/config.go index 352f73bda..cd98ce4ee 100644 --- a/config/v3_5_experimental/types/config.go +++ b/config/v3_5_experimental/types/config.go @@ -15,6 +15,7 @@ package types import ( + "fmt" "path/filepath" "sort" "strings" @@ -35,6 +36,8 @@ var ( } ) +var paths = map[string]struct{}{} + func (cfg Config) Validate(c path.ContextPath) (r report.Report) { systemdPath := "/etc/systemd/system/" unitPaths := map[string]struct{}{} @@ -76,43 +79,24 @@ func (cfg Config) validateParents(c path.ContextPath) report.Report { Path string Field string } - paths := map[string]struct{}{} 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.ErrPathConflictsParentDir) //TODO: should add different error? - return r - } - paths[f.Path] = struct{}{} - entries = append(entries, struct { - Path string - Field string - }{Path: f.Path, Field: "files"}) + fmt.Println("File variable value:", f) // Added print statement + r = handlePathConflict(f.Path, "files", i, c, r, errors.ErrPathAlreadyExists) + addPathAndEntry(f.Path, "files", &entries) } for i, d := range cfg.Storage.Directories { - if _, exists := paths[d.Path]; exists { - r.AddOnError(c.Append("storage", "directories", i, "path"), errors.ErrPathConflictsParentDir) //TODO: should add different error? - return r - } - paths[d.Path] = struct{}{} - entries = append(entries, struct { - Path string - Field string - }{Path: d.Path, Field: "directories"}) + fmt.Println("Directory variable value:", d) // Added print statement + r = handlePathConflict(d.Path, "directories", i, c, r, errors.ErrPathAlreadyExists) + addPathAndEntry(d.Path, "directories", &entries) } for i, l := range cfg.Storage.Links { - if _, exists := paths[l.Path]; exists { - r.AddOnError(c.Append("storage", "links", i, "path"), errors.ErrPathConflictsParentDir) //TODO: error to already exist path - return r - } - paths[l.Path] = struct{}{} - entries = append(entries, struct { - Path string - Field string - }{Path: l.Path, Field: "links"}) + fmt.Println("Link variable value:", l) // Added print statement + r = handlePathConflict(l.Path, "links", i, c, r, errors.ErrPathAlreadyExists) + addPathAndEntry(l.Path, "links", &entries) } sort.Slice(entries, func(i, j int) bool { @@ -120,9 +104,12 @@ func (cfg Config) validateParents(c path.ContextPath) report.Report { }) for i, entry := range entries { + fmt.Println("Entry variable value:", entry) // Added print statement if i > 0 && isWithin(entry.Path, entries[i-1].Path) { if entries[i-1].Field != "directories" { - r.AddOnError(c.Append("storage", entry.Field, i, "path"), errors.ErrPathConflictsParentDir) //TODO: conflict parent directories error + errorMsg := fmt.Errorf("invalid entry at path %s: %v", entry.Path, errors.ErrMissLabeledDir) + r.AddOnError(c.Append("storage", entry.Field, i, "path"), errorMsg) + fmt.Println("Error message:", errorMsg) // Added print statement return r } } @@ -131,16 +118,42 @@ func (cfg Config) validateParents(c path.ContextPath) report.Report { return r } -// check the depth +func handlePathConflict(path, fieldName string, index int, c path.ContextPath, r report.Report, err error) report.Report { + fmt.Println("Path variable value:", path) // Added print statement + if _, exists := paths[path]; exists { + r.AddOnError(c.Append("storage", fieldName, index, "path"), err) + fmt.Println("Error:", err) // Added print statement + } + return r +} + +func addPathAndEntry(path, fieldName string, entries *[]struct{ Path, Field string }) { + *entries = append(*entries, struct { + Path string + Field string + }{Path: path, Field: fieldName}) + fmt.Println("Added entry:", path) // Added print statement +} + func depth(path string) uint { var count uint - for p := filepath.Clean(path); p != "/" && p != "."; count++ { - p = filepath.Dir(p) + cleanedPath := filepath.FromSlash(filepath.Clean(path)) + sep := string(filepath.Separator) + + volume := filepath.VolumeName(cleanedPath) + if volume != "" { + cleanedPath = cleanedPath[len(volume):] + } + + for cleanedPath != sep && cleanedPath != "." { + cleanedPath = filepath.Dir(cleanedPath) + count++ } return count } -// isWithin checks if newPath is within prevPath. func isWithin(newPath, prevPath string) bool { + fmt.Println("New path variable value:", newPath) // Added print statement + fmt.Println("Previous path variable value:", prevPath) // Added print statement return strings.HasPrefix(newPath, prevPath) && newPath != prevPath } diff --git a/config/v3_5_experimental/types/config_test.go b/config/v3_5_experimental/types/config_test.go index 3db9f6e3c..780cbb2e4 100644 --- a/config/v3_5_experimental/types/config_test.go +++ b/config/v3_5_experimental/types/config_test.go @@ -15,12 +15,12 @@ package types import ( + "fmt" "reflect" "testing" "github.com/coreos/ignition/v2/config/shared/errors" "github.com/coreos/ignition/v2/config/util" - "github.com/coreos/vcontext/path" "github.com/coreos/vcontext/report" ) @@ -194,10 +194,9 @@ func TestConfigValidation(t *testing.T) { }, }, }, - out: errors.ErrPathConflictsParentDir, + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), at: path.New("json", "storage", "files", 1, "path"), }, - // test 7: file path conflicts with link path, should error { in: Config{ @@ -210,8 +209,8 @@ func TestConfigValidation(t *testing.T) { }, }, }, - out: errors.ErrPathConflictsParentDir, - at: path.New("json", "storage", "links", 0, "path"), + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "links", 1, "path"), }, // test 8: file path conflicts with directory path, should error @@ -220,15 +219,14 @@ func TestConfigValidation(t *testing.T) { Storage: Storage{ Files: []File{ {Node: Node{Path: "/foo/bar"}}, - {Node: Node{Path: "/foo/bar"}}, }, Directories: []Directory{ {Node: Node{Path: "/foo/bar/baz"}}, }, }, }, - out: errors.ErrPathConflictsParentDir, - at: path.New("json", "storage", "directories", 0, "path"), + out: fmt.Errorf("invalid entry at path /foo/bar/baz: %w", errors.ErrMissLabeledDir), + at: path.New("json", "storage", "directories", 1, "path"), }, // test 9: non-conflicting scenarios with systemd unit and systemd dropin file, should not error @@ -302,7 +300,6 @@ func TestConfigValidation(t *testing.T) { in: Config{ Storage: Storage{ Files: []File{ - {Node: Node{Path: "/foo/bar"}}, {Node: Node{Path: "/foo/bar/baz"}}, }, Directories: []Directory{ diff --git a/docs/release-notes.md b/docs/release-notes.md index ca6281f12..cb909cc81 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -24,6 +24,7 @@ nav_order: 9 - Fix failure when config only disables units already disabled - Retry HTTP requests on Azure on status codes 404, 410, and 429 +- Fix validation to catch conflicts with the parent directory of another file, link or directories ## Ignition 2.17.0 (2023-11-20)