opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-05-17 15:18:43 +08:00
Code Issues Packages Projects Releases Wiki Activity

switch podman image scp from depending on machinectl to just os/exec

Browse Source 
machinectl does not propogate error messages and adds extra lines in the output, exec.Cmd is able to clear the env besides PATH and TERM,
and use the given UID and GID to execute the command properly.

machinectl is still used to create a user session. Ubuntu support is limited by this.

Signed-off-by: cdoern <cdoern@redhat.com>
This commit is contained in:
cdoern
2022-01-14 12:45:57 -05:00
committed by cdoern
parent 09589fccfd
commit 77cd38d40c
4 changed files with 63 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
cmd/podman/images/scp.go
View File

@ -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")
logrus.Debugf("Executing podman command: %q", cmd)
return cmd.Run()
}

69
pkg/domain/infra/abi/images.go
View File

@ -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)
err = execPodman(uSave, saveCommand)
if err != nil {
return err
}
return execSu(uLoad, load)
}
err = execMachine(uSave, saveCommand, machinectl)
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()
}

20
test/system/120-load.bats
View File

@ -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,8 +135,6 @@ 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"
@ -160,7 +143,6 @@ verify_iid_and_name() {
run_podman 125 image scp $nope ${notme}@localhost::
is "$output" "Error: $nope: image not known.*" "Pushing nonexistent image"
fi
# Negative test for copying to a different name
run_podman 125 image scp $IMAGE ${notme}@localhost::newname:newtag

15
utils/utils.go
View File

@ -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
}