From 133ea31ffb9e6cd9ccbbc01053463f04c11f7f18 Mon Sep 17 00:00:00 2001 From: Ygal Blum Date: Thu, 19 Sep 2024 13:18:15 -0400 Subject: [PATCH] Quadlet - add full support for Symlinks Use os.ReadDir recursively instead of filepath.WalkDir Use map instead of list to easily find looped Symlinks Update existing tests and add a more elaborate one Update the man page Signed-off-by: Ygal Blum --- cmd/quadlet/main.go | 118 +++++++++++------- cmd/quadlet/main_test.go | 100 ++++++++++++--- docs/source/markdown/podman-systemd.unit.5.md | 3 +- test/e2e/quadlet_test.go | 2 +- 4 files changed, 159 insertions(+), 64 deletions(-) diff --git a/cmd/quadlet/main.go b/cmd/quadlet/main.go index bd19875599..a62a01a1d3 100644 --- a/cmd/quadlet/main.go +++ b/cmd/quadlet/main.go @@ -107,9 +107,11 @@ func Debugf(format string, a ...interface{}) { // 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) []string { +func getUnitDirs(rootless bool) map[string]struct{} { + dirs := make(map[string]struct{}, 0) + // Allow overriding source dir, this is mainly for the CI tests - if varExist, dirs := getDirsFromEnv(); varExist { + if getDirsFromEnv(dirs) { return dirs } @@ -119,61 +121,57 @@ func getUnitDirs(rootless bool) []string { if rootless { systemUserDirLevel := len(strings.Split(resolvedUnitDirAdminUser, string(os.PathSeparator))) nonNumericFilter := getNonNumericFilter(resolvedUnitDirAdminUser, systemUserDirLevel) - return getRootlessDirs(nonNumericFilter, userLevelFilter) + getRootlessDirs(dirs, nonNumericFilter, userLevelFilter) + } else { + getRootDirs(dirs, userLevelFilter) } - - return getRootDirs(userLevelFilter) + return dirs } -func getDirsFromEnv() (bool, []string) { +func getDirsFromEnv(dirs map[string]struct{}) bool { unitDirsEnv := os.Getenv("QUADLET_UNIT_DIRS") if len(unitDirsEnv) == 0 { - return false, nil + return false } - dirs := make([]string, 0) for _, eachUnitDir := range strings.Split(unitDirsEnv, ":") { if !filepath.IsAbs(eachUnitDir) { Logf("%s not a valid file path", eachUnitDir) - return true, nil + break } - dirs = appendSubPaths(dirs, eachUnitDir, false, nil) + appendSubPaths(dirs, eachUnitDir, false, nil) } - return true, dirs + return true } -func getRootlessDirs(nonNumericFilter, userLevelFilter func(string, bool) bool) []string { - dirs := make([]string, 0) - +func getRootlessDirs(dirs map[string]struct{}, nonNumericFilter, userLevelFilter func(string, bool) bool) { runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") if found { - dirs = appendSubPaths(dirs, path.Join(runtimeDir, "containers/systemd"), false, nil) + appendSubPaths(dirs, path.Join(runtimeDir, "containers/systemd"), false, nil) } configDir, err := os.UserConfigDir() if err != nil { fmt.Fprintf(os.Stderr, "Warning: %v", err) - return nil + return } - dirs = appendSubPaths(dirs, path.Join(configDir, "containers/systemd"), false, nil) + appendSubPaths(dirs, path.Join(configDir, "containers/systemd"), false, nil) u, err := user.Current() if err == nil { - dirs = appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) - dirs = appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) + appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) + appendSubPaths(dirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) } else { fmt.Fprintf(os.Stderr, "Warning: %v", err) } - return append(dirs, filepath.Join(quadlet.UnitDirAdmin, "users")) + dirs[filepath.Join(quadlet.UnitDirAdmin, "users")] = struct{}{} } -func getRootDirs(userLevelFilter func(string, bool) bool) []string { - dirs := make([]string, 0) - - dirs = appendSubPaths(dirs, quadlet.UnitDirTemp, false, userLevelFilter) - dirs = appendSubPaths(dirs, quadlet.UnitDirAdmin, false, userLevelFilter) - return appendSubPaths(dirs, quadlet.UnitDirDistro, false, nil) +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 resolveUnitDirAdminUser() string { @@ -189,7 +187,7 @@ func resolveUnitDirAdminUser() string { return resolvedUnitDirAdminUser } -func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func(string, bool) bool) []string { +func appendSubPaths(dirs map[string]struct{}, path string, isUserFlag bool, filterPtr func(string, bool) bool) { resolvedPath, err := filepath.EvalSymlinks(path) if err != nil { if !errors.Is(err, fs.ErrNotExist) { @@ -197,27 +195,55 @@ func appendSubPaths(dirs []string, path string, isUserFlag bool, filterPtr func( } // 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 = append(dirs, path) - return dirs + dirs[path] = struct{}{} + return } - err = filepath.WalkDir(resolvedPath, func(_path string, info os.DirEntry, err error) error { - // Ignore drop-in directory subpaths - if !strings.HasSuffix(_path, ".d") { - if info == nil || info.IsDir() { - if filterPtr == nil || filterPtr(_path, isUserFlag) { - dirs = append(dirs, _path) - } - } + // If the resolvedPath is already in the map no need to read it again + if _, already := dirs[resolvedPath]; already { + 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 err - }) + return + } + + // Not a directory nothing to add + if !stat.IsDir() { + return + } + + // Add the current directory + dirs[resolvedPath] = struct{}{} + + // Read the contents of the directory + entries, err := os.ReadDir(resolvedPath) if err != nil { if !errors.Is(err, os.ErrNotExist) { Debugf("Error occurred walking sub directories %q: %s", path, err) } + return + } + + // Recursively run through the contents of the directory + for _, entry := range entries { + fullPath := filepath.Join(resolvedPath, entry.Name()) + appendSubPaths(dirs, fullPath, isUserFlag, filterPtr) } - return dirs } func getNonNumericFilter(resolvedUnitDirAdminUser string, systemUserDirLevel int) func(string, bool) bool { @@ -297,7 +323,7 @@ func loadUnitsFromDir(sourcePath string) ([]*parser.UnitFile, error) { return units, prevError } -func loadUnitDropins(unit *parser.UnitFile, sourcePaths []string) error { +func loadUnitDropins(unit *parser.UnitFile, sourcePaths map[string]struct{}) error { var prevError error reportError := func(err error) { if prevError != nil { @@ -309,7 +335,7 @@ func loadUnitDropins(unit *parser.UnitFile, sourcePaths []string) error { dropinDirs := []string{} unitDropinPaths := unit.GetUnitDropinPaths() - for _, sourcePath := range sourcePaths { + for sourcePath := range sourcePaths { for _, dropinPath := range unitDropinPaths { dropinDirs = append(dropinDirs, path.Join(sourcePath, dropinPath)) } @@ -620,10 +646,10 @@ func process() error { Debugf("Starting quadlet-generator, output to: %s", outputPath) } - sourcePaths := getUnitDirs(isUserFlag) + sourcePathsMap := getUnitDirs(isUserFlag) var units []*parser.UnitFile - for _, d := range sourcePaths { + for d := range sourcePathsMap { if result, err := loadUnitsFromDir(d); err != nil { reportError(err) } else { @@ -634,12 +660,12 @@ func process() error { if len(units) == 0 { // containers/podman/issues/17374: exit cleanly but log that we // had nothing to do - Debugf("No files parsed from %s", sourcePaths) + Debugf("No files parsed from %s", sourcePathsMap) return prevError } for _, unit := range units { - if err := loadUnitDropins(unit, sourcePaths); err != nil { + if err := loadUnitDropins(unit, sourcePathsMap); err != nil { reportError(err) } } diff --git a/cmd/quadlet/main_test.go b/cmd/quadlet/main_test.go index 74fec0a022..e429c507db 100644 --- a/cmd/quadlet/main_test.go +++ b/cmd/quadlet/main_test.go @@ -64,31 +64,36 @@ func TestUnitDirs(t *testing.T) { resolvedUnitDirAdminUser := resolveUnitDirAdminUser() userLevelFilter := getUserLevelFilter(resolvedUnitDirAdminUser) - rootDirs := []string{} - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirTemp, false, userLevelFilter) - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirAdmin, false, userLevelFilter) - rootDirs = appendSubPaths(rootDirs, quadlet.UnitDirDistro, false, userLevelFilter) - assert.Equal(t, unitDirs, rootDirs, "rootful unit dirs should match") + 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") configDir, err := os.UserConfigDir() assert.Nil(t, err) - rootlessDirs := []string{} + rootlessDirs := make(map[string]struct{}, 0) systemUserDirLevel := len(strings.Split(resolvedUnitDirAdminUser, string(os.PathSeparator))) nonNumericFilter := getNonNumericFilter(resolvedUnitDirAdminUser, systemUserDirLevel) runtimeDir, found := os.LookupEnv("XDG_RUNTIME_DIR") if found { - rootlessDirs = appendSubPaths(rootlessDirs, path.Join(runtimeDir, "containers/systemd"), false, nil) + appendSubPaths(rootlessDirs, path.Join(runtimeDir, "containers/systemd"), false, nil) } - rootlessDirs = appendSubPaths(rootlessDirs, path.Join(configDir, "containers/systemd"), false, nil) - rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users"), true, nonNumericFilter) - rootlessDirs = appendSubPaths(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users", u.Uid), true, userLevelFilter) - rootlessDirs = append(rootlessDirs, filepath.Join(quadlet.UnitDirAdmin, "users")) + 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{}{} unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, rootlessDirs, "rootless unit dirs should match") + assert.Equal(t, rootlessDirs, 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) name, err := os.MkdirTemp("", "dir") assert.Nil(t, err) @@ -97,10 +102,10 @@ func TestUnitDirs(t *testing.T) { t.Setenv("QUADLET_UNIT_DIRS", name) unitDirs = getUnitDirs(false) - assert.Equal(t, unitDirs, []string{name}, "rootful should use environment variable") + assert.Equal(t, map[string]struct{}{name: {}}, unitDirs, "rootful should use environment variable") unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, []string{name}, "rootless should use environment variable") + assert.Equal(t, map[string]struct{}{name: {}}, unitDirs, "rootless should use environment variable") symLinkTestBaseDir, err := os.MkdirTemp("", "podman-symlinktest") assert.Nil(t, err) @@ -118,7 +123,72 @@ func TestUnitDirs(t *testing.T) { assert.Nil(t, err) t.Setenv("QUADLET_UNIT_DIRS", symlink) unitDirs = getUnitDirs(true) - assert.Equal(t, unitDirs, []string{actualDir, innerDir}, "directory resolution should follow symlink") + assert.Equal(t, map[string]struct{}{actualDir: {}, innerDir: {}}, unitDirs, "directory resolution should follow symlink") + + // Make a more elborate test with the following structure: + // /linkToDir - real directory to link to + // /linkToDir/a - real directory + // /linkToDir/b - link to /unitDir/b/a should be ignored + // /linkToDir/c - link to /unitDir should be ignored + // /unitDir - start from here + // /unitDir/a - real directory + // /unitDir/a/a - real directory + // /unitDir/a/a/a - real directory + // /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 { + dirName := filepath.Join(path, name) + assert.NotContains(t, dirs, dirName) + err = os.Mkdir(dirName, 0755) + assert.Nil(t, err) + dirs[dirName] = struct{}{} + return dirName + } + expectedDirs := make(map[string]struct{}, 0) + // Create /linkToDir + linkToDirPath := createDir(symLinkRecursiveTestBaseDir, "linkToDir", expectedDirs) + // Create /linkToDir/a + createDir(linkToDirPath, "a", expectedDirs) + // Create /unitDir + unitsDirPath := createDir(symLinkRecursiveTestBaseDir, "unitsDir", expectedDirs) + // Create /unitDir/a + aDirPath := createDir(unitsDirPath, "a", expectedDirs) + // Create /unitDir/a/a + aaDirPath := createDir(aDirPath, "a", expectedDirs) + // Create /unitDir/a/b + createDir(aDirPath, "b", expectedDirs) + // Create /unitDir/a/a/a + createDir(aaDirPath, "a", expectedDirs) + // Create /unitDir/b + bDirPath := createDir(unitsDirPath, "b", expectedDirs) + // Create /unitDir/b/a + baDirPath := createDir(bDirPath, "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 + linkDir(linkToDirPath, "b", baDirPath) + // Link /linkToDir/c to /unitDir + linkDir(linkToDirPath, "c", unitsDirPath) + // Link /unitDir/c to /linkToDir + linkDir(unitsDirPath, "c", linkToDirPath) + + t.Setenv("QUADLET_UNIT_DIRS", unitsDirPath) + unitDirs = getUnitDirs(true) + assert.Equal(t, expectedDirs, unitDirs, "directory resolution should follow symlink") + // remove the temporary directory at the end of the program + defer os.RemoveAll(symLinkTestBaseDir) // because chroot is only available for root, // unshare the namespace and map user to root diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index 846fdb97ba..22551ee193 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -35,8 +35,7 @@ Quadlet files for non-root users can be placed in the following directories ### Using symbolic links -Quadlet supports using symbolic links for the base of the search paths. -Symbolic links below the search paths are not supported. +Quadlet supports using symbolic links for the base of the search paths and inside them. ## DESCRIPTION diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index 51897716ed..f0b7f0ef30 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 [/something]" + expected := "No files parsed from map[/something:{}]" found := false for _, line := range current {