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 load with invalid input has 0 (success) exit code #9672

Closed
martinetd opened this issue Mar 9, 2021 · 2 comments · Fixed by #9677
Closed

podman load with invalid input has 0 (success) exit code #9672

martinetd opened this issue Mar 9, 2021 · 2 comments · Fixed by #9677
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@martinetd
Copy link

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature
(not quite sure there :D I swear I'm not just trying to use podman randomly like a fuzzer...!)

Description

if podman load failed to parse the input I believe it should error out not just in messages but also in its exit code
(mostly-unrelated: it's a shame saved images can't be signed somehow, or am I missing something?)

Steps to reproduce the issue:

$ echo garbled input | podman load
  open /var/tmp/podman877531368/manifest.json: not a directory
  open /var/tmp/podman877531368/index.json: not a directory
Loaded image(s): 

$ echo $?
0

I'm -very bad- at go, but pulling a debugger out it looks like a simple assignment problem in libpod/runtime_img.go (err goes out of scope after the if, so saveErr = err only ever saves "nil")?
With this diff:

diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go
index 6e1105b9e3f5..76cd03dc0ebd 100644
--- a/libpod/runtime_img.go
+++ b/libpod/runtime_img.go
@@ -316,7 +316,8 @@ func() (types.ImageReference, error) {
        } {
                src, err := referenceFn()
                if err == nil && src != nil {
-                       if newImages, err := r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer); err == nil {
+                       newImages, err := r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer)
+                       if err == nil {
                                return getImageNames(newImages), nil
                        }
                        saveErr = err

I get:

$ ./bin/podman load -i foo
  open foo/manifest.json: not a directory
  open foo/index.json: not a directory
Error: error pulling image: unable to pull oci:foo:: 1 error occurred:
	* Error initializing source oci:foo:: open foo/index.json: not a directory

$ echo $?
125

The error only displaying the last handler's error isn't necessarily the most obvious for users but at least an error bubbles up as it looks like the code originally intended. Happy to submit that as a PR if that makes sense (I'm not sure if this project has a policy to always refer to an issue for each PR? Some people don't like PRs that hadn't been discussed first)

Output of podman version:

Version:      3.1.0-dev
API Version:  3.1.0-dev
Go Version:   go1.15.8
Git Commit:   b7c00f2cc03499d5d385a7aa7e8cd35d0ab994d7-dirty
Built:        Mon Mar  8 19:01:05 2021
OS/Arch:      linux/amd64
@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 9, 2021
@vrothberg
Copy link
Member

Thanks for reporting, @martinetd!
I will take a look now.

@vrothberg vrothberg self-assigned this Mar 9, 2021
vrothberg added a commit to vrothberg/libpod that referenced this issue Mar 9, 2021
Make sure to properly return loading errors and to set the exit code
accordingly.

Fixes: containers#9672
Signed-off-by: Valentin Rothberg <[email protected]>
@vrothberg
Copy link
Member

Opened #9677.

edsantiago added a commit to edsantiago/libpod that referenced this issue Mar 15, 2021
- cp test: clean up stray image

- build test: add workaround for containers#9567 (ultra-slow ubuntu).
  We're seeing CI flakes (timeouts) due to ubuntu 2004 being
  absurdly slow. Workaround: double our timeout on one specific
  test when ubuntu + remote.

- build test: clean up new copy-from test (from containers#9275).
  The test was copy-pasted from buildah system tests, without
  really adapting for podman environment (e.g. it was using
  images that we don't use here, and would cause pulls, which
  will cause flakes). Rewrite test so it references only $IMAGE,
  remove some confusing/unnecessary stuff, selectively run
  parts of it even when rootless or remote, and add a
  test to confirm that copy-from succeeded.

- load test: add error-message test to new load-invalid (containers#9672).
  Basically, make sure the command fails for the right reason.

- play test (kube): use $IMAGE, not alpine; and add pause-image
  cleanup to teardown()

- apiv2 mounts test: add a maintainability comment in a tricky
  section of code; and tighten up the mount point test.

Signed-off-by: Ed Santiago <[email protected]>
@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
kind/feature Categorizes issue or PR as related to a new feature. 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 a pull request may close this issue.

3 participants