Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Podman Image SCP transfer patch #12224

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Conversation

cdoern
Copy link
Contributor

@cdoern cdoern commented Nov 8, 2021

Fixed syntax so that podman image scp transfer works with no user specified.

This command can only be executed as root so to obtain the default user, I searched for
the SUDO_USER environmental variable.

If that is not found, we error out and inform the user
to set this variable and make sure they are running as root.

the two functional syntax forms are now:

sudo podman image scp root@localhost::IMAGE

and

sudo podman image scp root@localhost::IMAGE USER@localhost::

Signed-off-by: cdoern [email protected]

@cdoern cdoern requested a review from rhatdan November 8, 2021 16:32
@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2021

Still blows up for me.

 ./bin/podman image scp myimage root@localhost::
panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
github.com/containers/podman/v3/cmd/podman/images.parseArgs(0xc0003e1aa0, 0x2, 0x2, 0xc00035cc00, 0x1f4a170, 0xc00059c6f0, 0x0)
	/home/dwalsh/podman/cmd/podman/images/scp.go:303 +0x1625
github.com/containers/podman/v3/cmd/podman/images.scp(0x298b040, 0xc0003e1aa0, 0x2, 0x2, 0x0, 0x0)
	/home/dwalsh/podman/cmd/podman/images/scp.go:91 +0x245
github.com/spf13/cobra.(*Command).execute(0x298b040, 0xc00003c0d0, 0x2, 0x2, 0x298b040, 0xc00003c0d0)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:856 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0x299acc0, 0xc0000400d8, 0x19f14e0, 0x2a5df10)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:974 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:902
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:895
main.Execute()
	/home/dwalsh/podman/cmd/podman/root.go:91 +0xe7
main.main()
	/home/dwalsh/podman/cmd/podman/main.go:39 +0x92

@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2021

$ ./bin/podman image scp root@localhost::alpine .
Error: could not save image as specified: alpine: image not known
$ ./bin/podman image scp root@localhost::alpine
Error: could not save image as specified: alpine: image not known

@cdoern
Copy link
Contributor Author

cdoern commented Nov 8, 2021

@rhatdan The transfer command has to be run as root which explains the alpine not found error, and the syntax you're trying to use of podman image scp image root@localhost:: also does nothing because we can't transfer images to root which is that that is trying to do.

But that panic should not be happening even with the incorrect arguments, I'll take another look.

@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2021

Why would I need to run as root? This makes the command a lot less useful if it needs to be run as root.
Why wouldn't it work with sudo and move images back and forth?

If this only works as root, then it should have failed with that message.

@cdoern
Copy link
Contributor Author

cdoern commented Nov 8, 2021

@rhatdan sorry, I am confused, are you running those commands you are showing me using sudo? The command was designed to be run using sudo. Any attempt to invoke sudo within the rootless proces led to the following error:

sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: /etc/sudo.conf is owned by uid 65534, should be 0
sudo: error in /etc/sudo.conf, line 0 while loading plugin "sudoers_policy"
sudo: /usr/libexec/sudo/sudoers.so must be owned by uid 0
sudo: fatal error, unable to load plugins
Error: exit status 1

I can try to mess around with this again but that led me down the rabbit hole of creating a new user NS which most people agreed was too complicated for the task at hand.

@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2021

I am not running this command as sudo. I am running it as a rootless user.
There is nothing in the man page about sudo and there is nothing to tell me I did anything wrong. So for now, lets fix the man page and make podman image scp blowup if I run it as non root, in this circumstance.
And then let's fix the fact that I have to run it with sudo.

@rhatdan
Copy link
Member

rhatdan commented Nov 8, 2021

I think we can figure out how to not enter the user namespace in the situation where we want to use sudo. But for now fix the crash.

@cdoern
Copy link
Contributor Author

cdoern commented Nov 8, 2021

I think we can figure out how to not enter the user namespace in the situation where we want to use sudo. But for now fix the crash.

okay, I will look into this a bit because I agree it is much less useful this way.

@umohnani8
Copy link
Member

Looks like this is ready - LGTM
@rhatdan @mheon PTAL

@rhatdan
Copy link
Member

rhatdan commented Nov 11, 2021

Still segfaulting:

./bin/podman image scp alpine root@localhost::alpine
panic: runtime error: index out of range [1] with length 1

goroutine 1 [running]:
github.com/containers/podman/v3/cmd/podman/images.parseArgs(0xc0002d2560, 0x2, 0x2, 0xc0001a3800, 0x1f47230, 0xc000010ad8, 0x0)
	/home/dwalsh/podman/cmd/podman/images/scp.go:299 +0x1625
github.com/containers/podman/v3/cmd/podman/images.scp(0x2983060, 0xc0002d2560, 0x2, 0x2, 0x0, 0x0)
	/home/dwalsh/podman/cmd/podman/images/scp.go:91 +0x245
github.com/spf13/cobra.(*Command).execute(0x2983060, 0xc000114080, 0x2, 0x2, 0x2983060, 0xc000114080)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:856 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0x2992ce0, 0xc000138030, 0x19ef3e0, 0x2a55f30)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:974 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:902
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/home/dwalsh/podman/vendor/github.com/spf13/cobra/command.go:895
main.Execute()
	/home/dwalsh/podman/cmd/podman/root.go:91 +0xe7
main.main()
	/home/dwalsh/podman/cmd/podman/main.go:39 +0x92

@cdoern
Copy link
Contributor Author

cdoern commented Nov 11, 2021

@rhatdan I am not getting that error but I am pushing a different fix for syntax, let me know if that fixes your issue.

@rhatdan
Copy link
Member

rhatdan commented Nov 11, 2021

That is better at least gets rid of the crash, but the error makes not sense to a human being.

Error: cannot transfer images from any user besides root using sudo: invalid argument

As a user of this tool, I would have no idea what that meant.

Fixed syntax so that podman image scp transfer works with no user specified.
This command can only be executed as root so to obtain the default user, I searched for
the SUDO_USER environmental variable. If that is not found, we error out and inform the user
to set this variable and make sure they are running as root

Signed-off-by: cdoern <[email protected]>
Signed-off-by: cdoern <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Nov 12, 2021

Ok I will merge, but this is still needs to work for rootless users.

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 12, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 12, 2021
@openshift-merge-robot openshift-merge-robot merged commit 0aecacb into containers:main Nov 12, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants