Skip to content

Commit

Permalink
Merge pull request #24062 from ygalblum/quadlet-restore-dir-order
Browse files Browse the repository at this point in the history 
Quadlet - make sure the order of the UnitsDir is deterministic
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Sep 27, 2024
2 parents 13e4b08 + ebbec00 commit 87dcf9d
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 98 deletions.
129 changes: 78 additions & 51 deletions cmd/quadlet/main.go
View file
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
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
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

1 comment on commit 87dcf9d

@packit-as-a-service
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

podman-next COPR build failed. @containers/packit-build please check.

Please sign in to comment.