summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2022-01-27 07:48:50 -0500
committerGitHub <noreply@github.com>2022-01-27 07:48:50 -0500
commit5659b0734c628f3f42fd976b6ce91372be3019ae (patch)
tree4f259d6360a4711981d1564b709249d67a7fbb06
parent0d96c46c7c4de3f4464e8d55b6134b9bd082cf34 (diff)
parent77cd38d40c00290873226d610fc47098232b7c7a (diff)
downloadpodman-5659b0734c628f3f42fd976b6ce91372be3019ae.tar.gz
podman-5659b0734c628f3f42fd976b6ce91372be3019ae.tar.bz2
podman-5659b0734c628f3f42fd976b6ce91372be3019ae.zip
Merge pull request #12867 from cdoern/scp
switch podman image scp from depending on machinectl to just os/exec
-rw-r--r--cmd/podman/images/scp.go19
-rw-r--r--pkg/domain/infra/abi/images.go69
-rw-r--r--test/system/120-load.bats36
-rw-r--r--utils/utils.go15
4 files changed, 64 insertions, 75 deletions
diff --git a/cmd/podman/images/scp.go b/cmd/podman/images/scp.go
index 1481e71c7..81dcda123 100644
--- a/cmd/podman/images/scp.go
+++ b/cmd/podman/images/scp.go
@@ -17,7 +17,6 @@ import (
"github.com/containers/podman/v4/cmd/podman/system/connection"
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/pkg/domain/entities"
- "github.com/containers/podman/v4/pkg/rootless"
"github.com/containers/podman/v4/utils"
scpD "github.com/dtylman/scp"
"github.com/pkg/errors"
@@ -337,21 +336,9 @@ func GetServiceInformation(cliConnections []string, cfg *config.Config) (map[str
// execPodman executes the podman save/load command given the podman binary
func execPodman(podman string, command []string) error {
- if rootless.IsRootless() {
- cmd := exec.Command(podman)
- utils.CreateSCPCommand(cmd, command[1:])
- logrus.Debug("Executing podman command")
- return cmd.Run()
- }
- machinectl, err := exec.LookPath("machinectl")
- if err != nil {
- cmd := exec.Command("su", "-l", "root", "--command")
- cmd = utils.CreateSCPCommand(cmd, []string{strings.Join(command, " ")})
- return cmd.Run()
- }
- cmd := exec.Command(machinectl, "shell", "-q", "root@.host")
- cmd = utils.CreateSCPCommand(cmd, command)
- logrus.Debug("Executing load command machinectl")
+ cmd := exec.Command(podman)
+ utils.CreateSCPCommand(cmd, command[1:])
+ logrus.Debugf("Executing podman command: %q", cmd)
return cmd.Run()
}
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index b9c6d3ac7..0b1281aac 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/pkg/domain/infra/abi/images.go
@@ -12,6 +12,7 @@ import (
"path/filepath"
"strconv"
"strings"
+ "syscall"
"github.com/containers/common/libimage"
"github.com/containers/common/pkg/config"
@@ -782,7 +783,7 @@ func transferRootless(source entities.ImageScpOptions, dest entities.ImageScpOpt
return cmdLoad.Run()
}
-// TransferRootful creates new podman processes using exec.Command and su/machinectl, transferring images between the given source and destination users
+// TransferRootful creates new podman processes using exec.Command and a new uid/gid alongside a cleared environment
func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOptions, podman string, parentFlags []string) error {
basicCommand := []string{podman}
basicCommand = append(basicCommand, parentFlags...)
@@ -794,12 +795,9 @@ func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOpti
}
saveCommand = append(saveCommand, []string{"--output", source.File, source.Image}...)
loadCommand = append(loadCommand, []string{"--input", dest.File}...)
- save := []string{strings.Join(saveCommand, " ")}
- load := []string{strings.Join(loadCommand, " ")}
- // if executing using sudo or transferring between two users, the TransferRootless approach will not work, default to using machinectl or su as necessary.
- // the approach using sudo is preferable and more straightforward. There is no reason for using sudo in these situations
- // since the feature is meant to transfer from root to rootless an vice versa without explicit sudo evocaiton.
+ // if executing using sudo or transferring between two users, the TransferRootless approach will not work, the new process needs to be set up
+ // with the proper uid and gid as well as environmental variables.
var uSave *user.User
var uLoad *user.User
var err error
@@ -830,20 +828,11 @@ func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOpti
return err
}
}
- machinectl, err := exec.LookPath("machinectl")
- if err != nil {
- logrus.Warn("defaulting to su since machinectl is not available, su will fail if no user session is available")
- err = execSu(uSave, save)
- if err != nil {
- return err
- }
- return execSu(uLoad, load)
- }
- err = execMachine(uSave, saveCommand, machinectl)
+ err = execPodman(uSave, saveCommand)
if err != nil {
return err
}
- return execMachine(uLoad, loadCommand, machinectl)
+ return execPodman(uLoad, loadCommand)
}
func lookupUser(u string) (*user.User, error) {
@@ -853,21 +842,37 @@ func lookupUser(u string) (*user.User, error) {
return user.Lookup(u)
}
-func execSu(execUser *user.User, command []string) error {
- cmd := exec.Command("su", "-l", execUser.Username, "--command")
- cmd = utils.CreateSCPCommand(cmd, command)
- logrus.Debugf("Executing via su: %q", cmd)
- return cmd.Run()
-}
-
-func execMachine(execUser *user.User, command []string, machinectl string) error {
- verb := machinectl
- args := []string{"shell", "-q", execUser.Username + "@.host"}
- if execUser.Uid == "0" {
- args = append([]string{verb}, args...)
- verb = "sudo"
+func execPodman(execUser *user.User, command []string) error {
+ cmdLogin, err := utils.LoginUser(execUser.Username)
+ if err != nil {
+ return err
+ }
+ defer func() error {
+ err := cmdLogin.Process.Kill()
+ if err != nil {
+ return err
+ }
+ return cmdLogin.Wait()
+ }()
+ cmd := exec.Command(command[0], command[1:]...)
+ cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "TERM=" + os.Getenv("TERM")}
+ cmd.Stderr = os.Stderr
+ cmd.Stdout = os.Stdout
+ uid, err := strconv.ParseInt(execUser.Uid, 10, 32)
+ if err != nil {
+ return err
+ }
+ gid, err := strconv.ParseInt(execUser.Gid, 10, 32)
+ if err != nil {
+ return err
+ }
+ cmd.SysProcAttr = &syscall.SysProcAttr{
+ Credential: &syscall.Credential{
+ Uid: uint32(uid),
+ Gid: uint32(gid),
+ Groups: nil,
+ NoSetGroups: false,
+ },
}
- cmd := utils.CreateSCPCommand(exec.Command(verb, args...), command)
- logrus.Debugf("Executing via machinectl: %q", cmd)
return cmd.Run()
}
diff --git a/test/system/120-load.bats b/test/system/120-load.bats
index f5ba93d8a..8bf785081 100644
--- a/test/system/120-load.bats
+++ b/test/system/120-load.bats
@@ -104,10 +104,6 @@ verify_iid_and_name() {
# If we can't sudo, we can't test.
_sudo true || skip "cannot sudo to $notme"
- # FIXME FIXME FIXME: it'd be reeeeeeally nice if we could pass --root
- # to the non-self user, hence avoid vandalizing
- # their storage.
-
# Preserve digest of original image; we will compare against it later
run_podman image inspect --format '{{.Digest}}' $IMAGE
src_digest=$output
@@ -117,12 +113,8 @@ verify_iid_and_name() {
run_podman tag $IMAGE $newname
# Copy it there.
- # FIXME: the first '.*' in the expect string below is unfortunate; it's
- # a workaround for Ubuntu which gripes:
- # "warning.*defaulting to su since machinectl is not available"
- # Reexamine this once #12829 is fixed
run_podman image scp $newname ${notme}@localhost::
- is "$output" ".*Copying blob .*Copying config.*Writing manifest.*Storing signatures"
+ is "$output" "Copying blob .*Copying config.*Writing manifest.*Storing signatures"
# confirm that image was copied. FIXME: also try $PODMAN image inspect?
_sudo $PODMAN image exists $newname
@@ -132,13 +124,6 @@ verify_iid_and_name() {
run_podman image scp -q ${notme}@localhost::$newname
expect="Loaded image(s): $newname"
- # FIXME FIXME FIXME: ubuntu has no machinectl, emits useless warning message instead
- if ! is_rootless; then
- # FIXME: root on fedora uses machinectl, which emits useless \n and \r (#12829)
- NL=$'\n'
- CR=$'\r'
- expect="$NL$expect$CR"
- fi
is "$output" "$expect" "-q silences output"
# Confirm that we have it, and that its digest matches our original
@@ -150,17 +135,14 @@ verify_iid_and_name() {
run_podman untag $IMAGE $newname
# Negative test for nonexistent image.
- # FIXME FIXME: cannot test on root, because it uses machinectl (#12829)
- if is_rootless; then
- # FIXME: error message is 2 lines, the 2nd being "exit status 125".
- # FIXME: is that fixable, or do we have to live with it?
- nope="nope.nope/nonesuch:notag"
- run_podman 125 image scp ${notme}@localhost::$nope
- is "$output" "Error: $nope: image not known.*" "Pulling nonexistent image"
-
- run_podman 125 image scp $nope ${notme}@localhost::
- is "$output" "Error: $nope: image not known.*" "Pushing nonexistent image"
- fi
+ # FIXME: error message is 2 lines, the 2nd being "exit status 125".
+ # FIXME: is that fixable, or do we have to live with it?
+ nope="nope.nope/nonesuch:notag"
+ run_podman 125 image scp ${notme}@localhost::$nope
+ is "$output" "Error: $nope: image not known.*" "Pulling nonexistent image"
+
+ run_podman 125 image scp $nope ${notme}@localhost::
+ is "$output" "Error: $nope: image not known.*" "Pushing nonexistent image"
# Negative test for copying to a different name
run_podman 125 image scp $IMAGE ${notme}@localhost::newname:newtag
diff --git a/utils/utils.go b/utils/utils.go
index caf63c975..52586b937 100644
--- a/utils/utils.go
+++ b/utils/utils.go
@@ -238,3 +238,18 @@ func CreateSCPCommand(cmd *exec.Cmd, command []string) *exec.Cmd {
cmd.Stdout = os.Stdout
return cmd
}
+
+// LoginUser starts the user process on the host so that image scp can use systemd-run
+func LoginUser(user string) (*exec.Cmd, error) {
+ sleep, err := exec.LookPath("sleep")
+ if err != nil {
+ return nil, err
+ }
+ machinectl, err := exec.LookPath("machinectl")
+ if err != nil {
+ return nil, err
+ }
+ cmd := exec.Command(machinectl, "shell", "-q", user+"@.host", sleep, "inf")
+ err = cmd.Start()
+ return cmd, err
+}