diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index a62a01a1d3..4433fb9a15 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -103,16 +103,39 @@ func Debugf(format string, a ...interface{}) { } } +type searchPaths struct { + sorted []string + // map to store paths so we can quickly check if we saw them already and not loop in case of symlinks + visitedDirs map[string]struct{} +} + +func newSearchPaths() *searchPaths { + return &searchPaths{ + sorted: make([]string, 0), + visitedDirs: make(map[string]struct{}, 0), + } +} + +func (s *searchPaths) Add(path string) { + s.sorted = append(s.sorted, path) + s.visitedDirs[path] = struct{}{} +} + +func (s *searchPaths) Visited(path string) bool { + _, visited := s.visitedDirs[path] + return visited +} + // This returns the directories where we read quadlet .container and .volumes from // For system generators these are in /usr/share/containers/systemd (for distro files) // and /etc/containers/systemd (for sysadmin files). // For user generators these can live in $XDG_RUNTIME_DIR/containers/systemd, /etc/containers/systemd/users, /etc/containers/systemd/users/$UID, and $XDG_CONFIG_HOME/containers/systemd -func getUnitDirs(rootless bool) map[string]struct{} { - dirs := make(map[string]struct{}, 0) +func getUnitDirs(rootless bool) []string { + paths := newSearchPaths() // Allow overriding source dir, this is mainly for the CI tests - if getDirsFromEnv(dirs) { - return dirs + if getDirsFromEnv(paths) { + return paths.sorted } resolvedUnitDirAdminUser := resolveUnitDirAdminUser() @@ -121,14 +144,14 @@ func getUnitDirs(rootless bool) map[string]struct{} { if rootless { systemUserDirLevel := len(strings.Split(resolvedUnitDirAdminUser, string(os.PathSeparator))) nonNumericFilter := getNonNumericFilter(resolvedUnitDirAdminUser, systemUserDirLevel) - getRootlessDirs(dirs, nonNumericFilter, userLevelFilter) + getRootlessDirs(paths, nonNumericFilter, userLevelFilter) } else { - getRootDirs(dirs, userLevelFilter) + getRootDirs(paths, userLevelFilter) } - return dirs + return paths.sorted } -func getDirsFromEnv(dirs map[string]struct{}) bool { +func getDirsFromEnv(paths *searchPaths) bool { unitDirsEnv := os.Getenv("QUADLET_UNIT_DIRS") if len(unitDirsEnv) == 0 { return false @@ -139,15 +162,15 @@ func getDirsFromEnv(dirs map[string]struct{}) bool { Logf("%s not a valid file path", eachUnitDir) break } - appendSubPaths(dirs, eachUnitDir, false, nil) + appendSubPaths(paths, eachUnitDir, false, nil) } return true } -func getRootlessDirs(dirs map[string]struct{}, nonNumericFilter, userLevelFilter func(string, bool) bool) { +func getRootlessDirs(paths *searchPaths, nonNumericFilter, userLevelFilter func(string, bool) bool) { runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") if found { - appendSubPaths(dirs, path.Join(runtimeDir, "containers/systemd"), false, nil) + appendSubPaths(paths, path.Join(runtimeDir, "containers/systemd"), false, nil) } configDir, err := os.UserConfigDir() @@ -155,23 +178,23 @@ func getRootlessDirs(dirs map[string]struct{}, nonNumericFilter, userLevelFilter fmt.Fprintf(os.Stderr, "Warning: %v", err) return } - appendSubPaths(dirs, path.Join(configDir, "containers/systemd"), false, nil) + appendSubPaths(paths, path.Join(configDir, "containers/systemd"), false, nil) u, err := user.Current() if err == nil { - appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) - appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) + appendSubPaths(paths, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) + appendSubPaths(paths, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) } else { fmt.Fprintf(os.Stderr, "Warning: %v", err) } - dirs[filepath.Join(quadlet.UnitDirAdmin, "users")] = struct{}{} + paths.Add(filepath.Join(quadlet.UnitDirAdmin, "users")) } -func getRootDirs(dirs map[string]struct{}, userLevelFilter func(string, bool) bool) { - appendSubPaths(dirs, quadlet.UnitDirTemp, false, userLevelFilter) - appendSubPaths(dirs, quadlet.UnitDirAdmin, false, userLevelFilter) - appendSubPaths(dirs, quadlet.UnitDirDistro, false, nil) +func getRootDirs(paths *searchPaths, userLevelFilter func(string, bool) bool) { + appendSubPaths(paths, quadlet.UnitDirTemp, false, userLevelFilter) + appendSubPaths(paths, quadlet.UnitDirAdmin, false, userLevelFilter) + appendSubPaths(paths, quadlet.UnitDirDistro, false, nil) } func resolveUnitDirAdminUser() string { @@ -187,7 +210,7 @@ func resolveUnitDirAdminUser() string { return resolvedUnitDirAdminUser } -func appendSubPaths(dirs map[string]struct{}, path string, isUserFlag bool, filterPtr func(string, bool) bool) { +func appendSubPaths(paths *searchPaths, path string, isUserFlag bool, filterPtr func(string, bool) bool) { resolvedPath, err := filepath.EvalSymlinks(path) if err != nil { if !errors.Is(err, fs.ErrNotExist) { @@ -195,40 +218,16 @@ func appendSubPaths(dirs map[string]struct{}, path string, isUserFlag bool, filt } // Despite the failure add the path to the list for logging purposes // This is the equivalent of adding the path when info==nil below - dirs[path] = struct{}{} - return - } - - // If the resolvedPath is already in the map no need to read it again - if _, already := dirs[resolvedPath]; already { + paths.Add(path) return } - // Don't traverse drop-in directories - if strings.HasSuffix(resolvedPath, ".d") { - return - } - - // Check if the directory should be filtered out - if filterPtr != nil && !filterPtr(resolvedPath, isUserFlag) { - return - } - - stat, err := os.Stat(resolvedPath) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - Debugf("Error occurred resolving path %q: %s", path, err) - } - return - } - - // Not a directory nothing to add - if !stat.IsDir() { + if skipPath(paths, resolvedPath, isUserFlag, filterPtr) { return } // Add the current directory - dirs[resolvedPath] = struct{}{} + paths.Add(resolvedPath) // Read the contents of the directory entries, err := os.ReadDir(resolvedPath) @@ -242,8 +241,36 @@ func appendSubPaths(dirs map[string]struct{}, path string, isUserFlag bool, filt // Recursively run through the contents of the directory for _, entry := range entries { fullPath := filepath.Join(resolvedPath, entry.Name()) - appendSubPaths(dirs, fullPath, isUserFlag, filterPtr) + appendSubPaths(paths, fullPath, isUserFlag, filterPtr) + } +} + +func skipPath(paths *searchPaths, path string, isUserFlag bool, filterPtr func(string, bool) bool) bool { + // If the path is already in the map no need to read it again + if paths.Visited(path) { + return true + } + + // Don't traverse drop-in directories + if strings.HasSuffix(path, ".d") { + return true + } + + // Check if the directory should be filtered out + if filterPtr != nil && !filterPtr(path, isUserFlag) { + return true + } + + stat, err := os.Stat(path) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + Debugf("Error occurred resolving path %q: %s", path, err) + } + return true } + + // Not a directory nothing to add + return !stat.IsDir() } func getNonNumericFilter(resolvedUnitDirAdminUser string, systemUserDirLevel int) func(string, bool) bool { @@ -323,7 +350,7 @@ func loadUnitsFromDir(sourcePath string) ([]*parser.UnitFile, error) { return units, prevError } -func loadUnitDropins(unit *parser.UnitFile, sourcePaths map[string]struct{}) error { +func loadUnitDropins(unit *parser.UnitFile, sourcePaths []string) error { var prevError error reportError := func(err error) { if prevError != nil { @@ -335,7 +362,7 @@ func loadUnitDropins(unit *parser.UnitFile, sourcePaths map[string]struct{}) err dropinDirs := []string{} unitDropinPaths := unit.GetUnitDropinPaths() - for sourcePath := range sourcePaths { + for _, sourcePath := range sourcePaths { for _, dropinPath := range unitDropinPaths { dropinDirs = append(dropinDirs, path.Join(sourcePath, dropinPath)) } @@ -649,7 +676,7 @@ func process() error { sourcePathsMap := getUnitDirs(isUserFlag) var units []*parser.UnitFile - for d := range sourcePathsMap { + for _, d := range sourcePathsMap { if result, err := loadUnitsFromDir(d); err != nil { reportError(err) } else { diff --git a/cmd/quadlet/main_test.go b/cmd/quadlet/main_test.go index e429c507db..d7efc80cc8 100644 --- a/cmd/quadlet/main_test.go +++ b/cmd/quadlet/main_test.go @@ -64,36 +64,36 @@ func TestUnitDirs(t *testing.T) { resolvedUnitDirAdminUser := resolveUnitDirAdminUser() userLevelFilter := getUserLevelFilter(resolvedUnitDirAdminUser) - rootDirs := make(map[string]struct{}, 0) - appendSubPaths(rootDirs, quadlet.UnitDirTemp, false, userLevelFilter) - appendSubPaths(rootDirs, quadlet.UnitDirAdmin, false, userLevelFilter) - appendSubPaths(rootDirs, quadlet.UnitDirDistro, false, userLevelFilter) - assert.Equal(t, rootDirs, unitDirs, "rootful unit dirs should match") + rootfulPaths := newSearchPaths() + appendSubPaths(rootfulPaths, quadlet.UnitDirTemp, false, userLevelFilter) + appendSubPaths(rootfulPaths, quadlet.UnitDirAdmin, false, userLevelFilter) + appendSubPaths(rootfulPaths, quadlet.UnitDirDistro, false, userLevelFilter) + assert.Equal(t, rootfulPaths.sorted, unitDirs, "rootful unit dirs should match") configDir, err := os.UserConfigDir() assert.Nil(t, err) - rootlessDirs := make(map[string]struct{}, 0) + rootlessPaths := newSearchPaths() systemUserDirLevel := len(strings.Split(resolvedUnitDirAdminUser, string(os.PathSeparator))) nonNumericFilter := getNonNumericFilter(resolvedUnitDirAdminUser, systemUserDirLevel) runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") if found { - appendSubPaths(rootlessDirs, path.Join(runtimeDir, "containers/systemd"), false, nil) + appendSubPaths(rootlessPaths, path.Join(runtimeDir, "containers/systemd"), false, nil) } - appendSubPaths(rootlessDirs, path.Join(configDir, "containers/systemd"), false, nil) - appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) - appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) - rootlessDirs[filepath.Join(quadlet.UnitDirAdmin, "users")] = struct{}{} + appendSubPaths(rootlessPaths, path.Join(configDir, "containers/systemd"), false, nil) + appendSubPaths(rootlessPaths, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) + appendSubPaths(rootlessPaths, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) + rootlessPaths.Add(filepath.Join(quadlet.UnitDirAdmin, "users")) unitDirs = getUnitDirs(true) - assert.Equal(t, rootlessDirs, unitDirs, "rootless unit dirs should match") + assert.Equal(t, rootlessPaths.sorted, unitDirs, "rootless unit dirs should match") // Test that relative path returns an empty list t.Setenv("QUADLET_UNIT_DIRS", "./relative/path") unitDirs = getUnitDirs(false) - assert.Equal(t, map[string]struct{}{}, unitDirs) + assert.Equal(t, []string{}, unitDirs) name, err := os.MkdirTemp("", "dir") assert.Nil(t, err) @@ -102,10 +102,10 @@ func TestUnitDirs(t *testing.T) { t.Setenv("QUADLET_UNIT_DIRS", name) unitDirs = getUnitDirs(false) - assert.Equal(t, map[string]struct{}{name: {}}, unitDirs, "rootful should use environment variable") + assert.Equal(t, []string{name}, unitDirs, "rootful should use environment variable") unitDirs = getUnitDirs(true) - assert.Equal(t, map[string]struct{}{name: {}}, unitDirs, "rootless should use environment variable") + assert.Equal(t, []string{name}, unitDirs, "rootless should use environment variable") symLinkTestBaseDir, err := os.MkdirTemp("", "podman-symlinktest") assert.Nil(t, err) @@ -123,7 +123,7 @@ func TestUnitDirs(t *testing.T) { assert.Nil(t, err) t.Setenv("QUADLET_UNIT_DIRS", symlink) unitDirs = getUnitDirs(true) - assert.Equal(t, map[string]struct{}{actualDir: {}, innerDir: {}}, unitDirs, "directory resolution should follow symlink") + assert.Equal(t, []string{actualDir, innerDir}, unitDirs, "directory resolution should follow symlink") // Make a more elborate test with the following structure: // /linkToDir - real directory to link to @@ -137,44 +137,46 @@ func TestUnitDirs(t *testing.T) { // /unitDir/b/a - real directory // /unitDir/b/b - link to /unitDir/a/a should be ignored // /unitDir/c - link to /linkToDir - symLinkRecursiveTestBaseDir, err := os.MkdirTemp("", "podman-symlink-recursive-test") - assert.Nil(t, err) - // remove the temporary directory at the end of the program - defer os.RemoveAll(symLinkRecursiveTestBaseDir) - - createDir := func(path, name string, dirs map[string]struct{}) string { + createDir := func(path, name string, dirs []string) (string, []string) { dirName := filepath.Join(path, name) assert.NotContains(t, dirs, dirName) err = os.Mkdir(dirName, 0755) assert.Nil(t, err) - dirs[dirName] = struct{}{} - return dirName + dirs = append(dirs, dirName) + return dirName, dirs } - expectedDirs := make(map[string]struct{}, 0) - // Create /linkToDir - linkToDirPath := createDir(symLinkRecursiveTestBaseDir, "linkToDir", expectedDirs) - // Create /linkToDir/a - createDir(linkToDirPath, "a", expectedDirs) + + linkDir := func(path, name, target string) { + linkName := filepath.Join(path, name) + err = os.Symlink(target, linkName) + assert.Nil(t, err) + } + + symLinkRecursiveTestBaseDir, err := os.MkdirTemp("", "podman-symlink-recursive-test") + assert.Nil(t, err) + // remove the temporary directory at the end of the program + defer os.RemoveAll(symLinkRecursiveTestBaseDir) + + expectedDirs := make([]string, 0) // Create /unitDir - unitsDirPath := createDir(symLinkRecursiveTestBaseDir, "unitsDir", expectedDirs) + unitsDirPath, expectedDirs := createDir(symLinkRecursiveTestBaseDir, "unitsDir", expectedDirs) // Create /unitDir/a - aDirPath := createDir(unitsDirPath, "a", expectedDirs) + aDirPath, expectedDirs := createDir(unitsDirPath, "a", expectedDirs) // Create /unitDir/a/a - aaDirPath := createDir(aDirPath, "a", expectedDirs) - // Create /unitDir/a/b - createDir(aDirPath, "b", expectedDirs) + aaDirPath, expectedDirs := createDir(aDirPath, "a", expectedDirs) // Create /unitDir/a/a/a - createDir(aaDirPath, "a", expectedDirs) + _, expectedDirs = createDir(aaDirPath, "a", expectedDirs) + // Create /unitDir/a/b + _, expectedDirs = createDir(aDirPath, "b", expectedDirs) // Create /unitDir/b - bDirPath := createDir(unitsDirPath, "b", expectedDirs) + bDirPath, expectedDirs := createDir(unitsDirPath, "b", expectedDirs) // Create /unitDir/b/a - baDirPath := createDir(bDirPath, "a", expectedDirs) + baDirPath, expectedDirs := createDir(bDirPath, "a", expectedDirs) + // Create /linkToDir + linkToDirPath, expectedDirs := createDir(symLinkRecursiveTestBaseDir, "linkToDir", expectedDirs) + // Create /linkToDir/a + _, expectedDirs = createDir(linkToDirPath, "a", expectedDirs) - linkDir := func(path, name, target string) { - linkName := filepath.Join(path, name) - err = os.Symlink(target, linkName) - assert.Nil(t, err) - } // Link /unitDir/b/b to /unitDir/a/a linkDir(bDirPath, "b", aaDirPath) // Link /linkToDir/b to /unitDir/b/a diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index f0b7f0ef30..51897716ed 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -713,7 +713,7 @@ var _ = Describe("quadlet system generator", func() { Expect(session).Should(Exit(0)) current := session.ErrorToStringArray() - expected := "No files parsed from map[/something:{}]" + expected := "No files parsed from [/something]" found := false for _, line := range current { diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats index b282cbd0eb..ada1198106 100644 --- a/test/system/252-quadlet.bats +++ b/test/system/252-quadlet.bats @@ -221,8 +221,6 @@ EOF } @test "quadlet conflict names" { - skip "FIXME: #24047, temporary skip because this is an intense flake" - # If two directories in the search have files with the same name, quadlet should # only process the first name dir1=$PODMAN_TMPDIR/$(random_string) @@ -232,13 +230,13 @@ EOF cat > $dir1/$quadlet_file < $dir2/$quadlet_file <