diff --git a/internal/blueprint/fsnode_customizations.go b/internal/blueprint/fsnode_customizations.go index 92ba2f676..f83695b96 100644 --- a/internal/blueprint/fsnode_customizations.go +++ b/internal/blueprint/fsnode_customizations.go @@ -4,11 +4,8 @@ import ( "encoding/json" "fmt" "os" - "path" "regexp" - "sort" "strconv" - "strings" "github.com/osbuild/osbuild-composer/internal/common" "github.com/osbuild/osbuild-composer/internal/fsnode" @@ -333,104 +330,3 @@ 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 1f25730ab..040f68e4a 100644 --- a/internal/blueprint/fsnode_customizations_test.go +++ b/internal/blueprint/fsnode_customizations_test.go @@ -975,160 +975,3 @@ 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) - } - }) - } -}