From a6dd4943c5d7e62f29b6b11e613a0d3fca0cd4e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Hozza?= Date: Fri, 3 Feb 2023 14:38:31 +0100 Subject: [PATCH] blueprint: add function for validating dir and file customizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a helper function for validating the user-provided directory and file customizations. This is necessary to fail early on invalid input, instead of when building the image. The function ensures that: - No file path is a prefix of another file or directory path - There are no duplicate file or directory paths in the customizations Signed-off-by: Tomáš Hozza --- internal/blueprint/fsnode_customizations.go | 104 ++++++++++++ .../blueprint/fsnode_customizations_test.go | 157 ++++++++++++++++++ 2 files changed, 261 insertions(+) diff --git a/internal/blueprint/fsnode_customizations.go b/internal/blueprint/fsnode_customizations.go index f83695b96..92ba2f676 100644 --- a/internal/blueprint/fsnode_customizations.go +++ b/internal/blueprint/fsnode_customizations.go @@ -4,8 +4,11 @@ import ( "encoding/json" "fmt" "os" + "path" "regexp" + "sort" "strconv" + "strings" "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/fsnode" @@ -330,3 +333,104 @@ func FileCustomizationsToFsNodeFiles(files []FileCustomization) ([]*fsnode.File, return fsFiles, nil } + +// ValidateDirFileCustomizations validates the given Directory and File customizations. +// If the customizations are invalid, an error is returned. Otherwise, nil is returned. +// +// It currently ensures that: +// - No file path is a prefix of another file or directory path +// - There are no duplicate file or directory paths in the customizations +func ValidateDirFileCustomizations(dirs []DirectoryCustomization, files []FileCustomization) error { + fsNodesMap := make(map[string]interface{}, len(dirs)+len(files)) + nodesPaths := make([]string, 0, len(dirs)+len(files)) + + // First check for duplicate paths + duplicatePaths := make([]string, 0) + for _, dir := range dirs { + if _, ok := fsNodesMap[dir.Path]; ok { + duplicatePaths = append(duplicatePaths, dir.Path) + } + fsNodesMap[dir.Path] = dir + nodesPaths = append(nodesPaths, dir.Path) + } + + for _, file := range files { + if _, ok := fsNodesMap[file.Path]; ok { + duplicatePaths = append(duplicatePaths, file.Path) + } + fsNodesMap[file.Path] = file + nodesPaths = append(nodesPaths, file.Path) + } + + // There is no point in continuing if there are duplicate paths, + // since the fsNodesMap will not be valid. + if len(duplicatePaths) > 0 { + return fmt.Errorf("duplicate files / directory customization paths: %v", duplicatePaths) + } + + invalidFSNodes := make([]string, 0) + checkedPaths := make(map[string]bool) + // Sort the paths so that we always check the longest paths first. This + // ensures that we don't check a parent path before we check the child + // path. Reverse sort the slice based on directory depth. + sort.Slice(nodesPaths, func(i, j int) bool { + return strings.Count(nodesPaths[i], "/") > strings.Count(nodesPaths[j], "/") + }) + + for _, nodePath := range nodesPaths { + // Skip paths that we have already checked + if checkedPaths[nodePath] { + continue + } + + // Check all parent paths of the current path. If any of them have + // already been checked, then we do not need to check them again. + // This is because we always check the longest paths first. If a parent + // path exists in the filesystem nodes map and it is a File, + // then it is an error because it is a parent of a Directory or File. + // Parent paths can be only Directories. + parentPath := nodePath + for { + parentPath = path.Dir(parentPath) + + // "." is returned only when the path is relative and we reached + // the root directory. This should never happen because File + // and Directory customization paths are validated as part of + // the unmarshalling process from JSON and TOML. + if parentPath == "." { + panic("filesystem node has relative path set.") + } + + if parentPath == "/" { + break + } + + if checkedPaths[parentPath] { + break + } + + // If the node is not a Directory, then it is an error because + // it is a parent of a Directory or File. + if node, ok := fsNodesMap[parentPath]; ok { + switch node.(type) { + case DirectoryCustomization: + break + case FileCustomization: + invalidFSNodes = append(invalidFSNodes, nodePath) + default: + panic(fmt.Sprintf("unexpected filesystem node customization type: %T", node)) + } + } + + checkedPaths[parentPath] = true + } + + checkedPaths[nodePath] = true + } + + if len(invalidFSNodes) > 0 { + return fmt.Errorf("the following filesystem nodes are parents of another node and are not directories: %s", invalidFSNodes) + } + + return nil +} diff --git a/internal/blueprint/fsnode_customizations_test.go b/internal/blueprint/fsnode_customizations_test.go index 040f68e4a..1f25730ab 100644 --- a/internal/blueprint/fsnode_customizations_test.go +++ b/internal/blueprint/fsnode_customizations_test.go @@ -975,3 +975,160 @@ func TestFileCustomizationUnmarshalJSON(t *testing.T) { }) } } + +func TestValidateDirFileCustomizations(t *testing.T) { + testCases := []struct { + Name string + Files []FileCustomization + Dirs []DirectoryCustomization + Error bool + }{ + { + Name: "empty-customizations", + }, + { + Name: "valid-file-customizations-only", + Files: []FileCustomization{ + { + Path: "/etc/file1", + }, + { + Path: "/etc/file2", + }, + }, + }, + { + Name: "valid-dir-customizations-only", + Dirs: []DirectoryCustomization{ + { + Path: "/etc/dir1", + }, + { + Path: "/etc/dir2", + }, + }, + }, + { + Name: "valid-file-and-dir-customizations", + Files: []FileCustomization{ + { + Path: "/etc/named/path1/path2/file", + }, + { + Path: "/etc/named/path1/file", + }, + { + Path: "/etc/named/path1/path2/file2", + }, + { + Path: "/etc/named/path1/file2", + }, + { + Path: "/etc/named/file", + }, + }, + Dirs: []DirectoryCustomization{ + { + Path: "/etc/named/path1/path2", + }, + { + Path: "/etc/named/path1", + }, + }, + }, + // Errors + { + Name: "file-parent-of-file", + Files: []FileCustomization{ + { + Path: "/etc/file1/file2", + }, + { + Path: "/etc/file1", + }, + }, + Error: true, + }, + { + Name: "file-parent-of-dir", + Files: []FileCustomization{ + { + Path: "/etc/file2", + }, + { + Path: "/etc/dir1", + }, + }, + Dirs: []DirectoryCustomization{ + { + Path: "/etc/dir1/dir2", + }, + { + Path: "/etc/dir3", + }, + }, + Error: true, + }, + { + Name: "duplicate-file-paths", + Files: []FileCustomization{ + { + Path: "/etc/file1", + }, + { + Path: "/etc/file2", + }, + { + Path: "/etc/file1", + }, + }, + Error: true, + }, + { + Name: "duplicate-dir-paths", + Dirs: []DirectoryCustomization{ + { + Path: "/etc/dir1", + }, + { + Path: "/etc/dir2", + }, + { + Path: "/etc/dir1", + }, + }, + Error: true, + }, + { + Name: "duplicate-file-and-dir-paths", + Files: []FileCustomization{ + { + Path: "/etc/path1", + }, + { + Path: "/etc/path2", + }, + }, + Dirs: []DirectoryCustomization{ + { + Path: "/etc/path3", + }, + { + Path: "/etc/path2", + }, + }, + Error: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + err := ValidateDirFileCustomizations(tc.Dirs, tc.Files) + if tc.Error { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}