Merge pull request #12867 from cdoern/scp

switch podman image scp from depending on machinectl to just os/exec
This commit is contained in:
OpenShift Merge Robot 2022-01-27 07:48:50 -05:00 committed by GitHub
commit 5659b0734c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 74 deletions

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")
cmd := exec.Command(podman)
utils.CreateSCPCommand(cmd, command[1:])
logrus.Debugf("Executing podman command: %q", cmd)
return cmd.Run()
}

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)
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()
}

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,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"
# 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
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

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
}