Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quadlet - make sure the order of the UnitsDir is deterministic #24062

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Quadlet - make sure the order of the UnitsDir is deterministic #24062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 78 additions & 51 deletions cmd/quadlet/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -139,39 +162,39 @@ 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()
if err != nil {
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 {
Expand All @@ -187,48 +210,24 @@ 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) {
Debugf("Error occurred resolving path %q: %s", path, err)
}
// 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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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))
}
Expand Down Expand Up @@ -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 {
Expand Down
86 changes: 44 additions & 42 deletions cmd/quadlet/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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:
// <BASE>/linkToDir - real directory to link to
Expand All @@ -137,44 +137,46 @@ func TestUnitDirs(t *testing.T) {
// <BASE>/unitDir/b/a - real directory
// <BASE>/unitDir/b/b - link to <BASE>/unitDir/a/a should be ignored
// <BASE>/unitDir/c - link to <BASE>/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 <BASE>/linkToDir
linkToDirPath := createDir(symLinkRecursiveTestBaseDir, "linkToDir", expectedDirs)
// Create <BASE>/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 <BASE>/unitDir
unitsDirPath := createDir(symLinkRecursiveTestBaseDir, "unitsDir", expectedDirs)
unitsDirPath, expectedDirs := createDir(symLinkRecursiveTestBaseDir, "unitsDir", expectedDirs)
// Create <BASE>/unitDir/a
aDirPath := createDir(unitsDirPath, "a", expectedDirs)
aDirPath, expectedDirs := createDir(unitsDirPath, "a", expectedDirs)
// Create <BASE>/unitDir/a/a
aaDirPath := createDir(aDirPath, "a", expectedDirs)
// Create <BASE>/unitDir/a/b
createDir(aDirPath, "b", expectedDirs)
aaDirPath, expectedDirs := createDir(aDirPath, "a", expectedDirs)
// Create <BASE>/unitDir/a/a/a
createDir(aaDirPath, "a", expectedDirs)
_, expectedDirs = createDir(aaDirPath, "a", expectedDirs)
// Create <BASE>/unitDir/a/b
_, expectedDirs = createDir(aDirPath, "b", expectedDirs)
// Create <BASE>/unitDir/b
bDirPath := createDir(unitsDirPath, "b", expectedDirs)
bDirPath, expectedDirs := createDir(unitsDirPath, "b", expectedDirs)
// Create <BASE>/unitDir/b/a
baDirPath := createDir(bDirPath, "a", expectedDirs)
baDirPath, expectedDirs := createDir(bDirPath, "a", expectedDirs)
// Create <BASE>/linkToDir
linkToDirPath, expectedDirs := createDir(symLinkRecursiveTestBaseDir, "linkToDir", expectedDirs)
// Create <BASE>/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 <BASE>/unitDir/b/b to <BASE>/unitDir/a/a
linkDir(bDirPath, "b", aaDirPath)
// Link <BASE>/linkToDir/b to <BASE>/unitDir/b/a
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/quadlet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading