From f2d552f5db4c037fbddb87fb45d7ddcbea62dc0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 22 Jan 2025 22:13:52 +0100 Subject: [PATCH] Turn PodmanAsUserBase into PodmanExecBaseWithOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... replacing the many parameters with a struct with named fields. This makes the meaning of parameters more explicit, and more importantly it makes it easier to just edit _one_ of the parameters without requiring specialized wrappers for every single case. Should not change behavior. Signed-off-by: Miloslav Trmač --- test/e2e/common_test.go | 7 ++++- test/e2e/libpod_suite_remote_test.go | 9 ++++-- test/e2e/libpod_suite_test.go | 9 ++++-- test/utils/utils.go | 46 ++++++++++++++++++---------- 4 files changed, 50 insertions(+), 21 deletions(-) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 65a04de847..10dec2449a 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -1060,7 +1060,12 @@ func SkipIfNetavark(p *PodmanTestIntegration) { // PodmanAsUser is the exec call to podman on the filesystem with the specified uid/gid and environment func (p *PodmanTestIntegration) PodmanAsUser(args []string, uid, gid uint32, cwd string, env []string) *PodmanSessionIntegration { - podmanSession := p.PodmanAsUserBase(args, uid, gid, cwd, env, false, false, nil, nil) + podmanSession := p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + UID: uid, + GID: gid, + CWD: cwd, + Env: env, + }) return &PodmanSessionIntegration{podmanSession} } diff --git a/test/e2e/libpod_suite_remote_test.go b/test/e2e/libpod_suite_remote_test.go index ceaa5851ea..26eeecb8a9 100644 --- a/test/e2e/libpod_suite_remote_test.go +++ b/test/e2e/libpod_suite_remote_test.go @@ -13,6 +13,7 @@ import ( "syscall" "time" + . "github.com/containers/podman/v5/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -37,14 +38,18 @@ func (p *PodmanTestIntegration) PodmanSystemdScope(args []string) *PodmanSession wrapper = []string{"systemd-run", "--scope", "--user"} } - podmanSession := p.PodmanAsUserBase(args, 0, 0, "", nil, false, false, wrapper, nil) + podmanSession := p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + Wrapper: wrapper, + }) return &PodmanSessionIntegration{podmanSession} } // PodmanExtraFiles is the exec call to podman on the filesystem and passes down extra files func (p *PodmanTestIntegration) PodmanExtraFiles(args []string, extraFiles []*os.File) *PodmanSessionIntegration { args = p.makeOptions(args, false, false) - podmanSession := p.PodmanAsUserBase(args, 0, 0, "", nil, false, false, nil, extraFiles) + podmanSession := p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + ExtraFiles: extraFiles, + }) return &PodmanSessionIntegration{podmanSession} } diff --git a/test/e2e/libpod_suite_test.go b/test/e2e/libpod_suite_test.go index 3b11b3952b..41dcbe7a36 100644 --- a/test/e2e/libpod_suite_test.go +++ b/test/e2e/libpod_suite_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + . "github.com/containers/podman/v5/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -26,13 +27,17 @@ func (p *PodmanTestIntegration) PodmanSystemdScope(args []string) *PodmanSession if isRootless() { wrapper = []string{"systemd-run", "--scope", "--user"} } - podmanSession := p.PodmanAsUserBase(args, 0, 0, "", nil, false, false, wrapper, nil) + podmanSession := p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + Wrapper: wrapper, + }) return &PodmanSessionIntegration{podmanSession} } // PodmanExtraFiles is the exec call to podman on the filesystem and passes down extra files func (p *PodmanTestIntegration) PodmanExtraFiles(args []string, extraFiles []*os.File) *PodmanSessionIntegration { - podmanSession := p.PodmanAsUserBase(args, 0, 0, "", nil, false, false, nil, extraFiles) + podmanSession := p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + ExtraFiles: extraFiles, + }) return &PodmanSessionIntegration{podmanSession} } diff --git a/test/utils/utils.go b/test/utils/utils.go index 30a0107596..bc8049b38c 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -94,16 +94,28 @@ func (p *PodmanTest) MakeOptions(args []string, noEvents, noCache bool) []string return p.PodmanMakeOptions(args, noEvents, noCache) } -// PodmanAsUserBase exec podman as user. uid and gid is set for credentials usage. env is used -// to record the env for debugging -func (p *PodmanTest) PodmanAsUserBase(args []string, uid, gid uint32, cwd string, env []string, noEvents, noCache bool, wrapper []string, extraFiles []*os.File) *PodmanSession { +// PodmanExecOptions modify behavior of PodmanTest.PodmanExecBaseWithOptions and its callers. +// Users should typically leave most fields default-initialized, and only set those that are relevant to them. +type PodmanExecOptions struct { + UID, GID uint32 // default: inherited form the current process + CWD string // default: inherited form the current process + Env []string // default: inherited form the current process + NoEvents bool + NoCache bool + Wrapper []string // A command to run, receiving the Podman command line. default: none + ExtraFiles []*os.File +} + +// PodmanExecBaseWithOptions execs podman with the specified args, and in an environment defined by options +func (p *PodmanTest) PodmanExecBaseWithOptions(args []string, options PodmanExecOptions) *PodmanSession { var command *exec.Cmd - podmanOptions := p.MakeOptions(args, noEvents, noCache) + podmanOptions := p.MakeOptions(args, options.NoEvents, options.NoCache) podmanBinary := p.PodmanBinary if p.RemoteTest { podmanBinary = p.RemotePodmanBinary } + runCmd := options.Wrapper if timeDir := os.Getenv(EnvTimeDir); timeDir != "" { timeFile, err := os.CreateTemp(timeDir, ".time") if err != nil { @@ -111,18 +123,17 @@ func (p *PodmanTest) PodmanAsUserBase(args []string, uid, gid uint32, cwd string } timeArgs := []string{"-f", "%M", "-o", timeFile.Name()} timeCmd := append([]string{"/usr/bin/time"}, timeArgs...) - wrapper = append(timeCmd, wrapper...) + runCmd = append(timeCmd, runCmd...) } - runCmd := wrapper runCmd = append(runCmd, podmanBinary) - if env == nil { + if options.Env == nil { GinkgoWriter.Printf("Running: %s %s\n", strings.Join(runCmd, " "), strings.Join(podmanOptions, " ")) } else { - GinkgoWriter.Printf("Running: (env: %v) %s %s\n", env, strings.Join(runCmd, " "), strings.Join(podmanOptions, " ")) + GinkgoWriter.Printf("Running: (env: %v) %s %s\n", options.Env, strings.Join(runCmd, " "), strings.Join(podmanOptions, " ")) } - if uid != 0 || gid != 0 { - pythonCmd := fmt.Sprintf("import os; import sys; uid = %d; gid = %d; cwd = '%s'; os.setgid(gid); os.setuid(uid); os.chdir(cwd) if len(cwd)>0 else True; os.execv(sys.argv[1], sys.argv[1:])", gid, uid, cwd) + if options.UID != 0 || options.GID != 0 { + pythonCmd := fmt.Sprintf("import os; import sys; uid = %d; gid = %d; cwd = '%s'; os.setgid(gid); os.setuid(uid); os.chdir(cwd) if len(cwd)>0 else True; os.execv(sys.argv[1], sys.argv[1:])", options.GID, options.UID, options.CWD) runCmd = append(runCmd, podmanOptions...) nsEnterOpts := append([]string{"-c", pythonCmd}, runCmd...) command = exec.Command("python", nsEnterOpts...) @@ -130,14 +141,14 @@ func (p *PodmanTest) PodmanAsUserBase(args []string, uid, gid uint32, cwd string runCmd = append(runCmd, podmanOptions...) command = exec.Command(runCmd[0], runCmd[1:]...) } - if env != nil { - command.Env = env + if options.Env != nil { + command.Env = options.Env } - if cwd != "" { - command.Dir = cwd + if options.CWD != "" { + command.Dir = options.CWD } - command.ExtraFiles = extraFiles + command.ExtraFiles = options.ExtraFiles session, err := Start(command, GinkgoWriter, GinkgoWriter) if err != nil { @@ -148,7 +159,10 @@ func (p *PodmanTest) PodmanAsUserBase(args []string, uid, gid uint32, cwd string // PodmanBase exec podman with default env. func (p *PodmanTest) PodmanBase(args []string, noEvents, noCache bool) *PodmanSession { - return p.PodmanAsUserBase(args, 0, 0, "", nil, noEvents, noCache, nil, nil) + return p.PodmanExecBaseWithOptions(args, PodmanExecOptions{ + NoEvents: noEvents, + NoCache: noCache, + }) } // WaitForContainer waits on a started container