Merge pull request #9757 from jwhonce/wip/load

Cleanup /libpod/images/load handler

libpod/runtime_img.go

@@ -313,15 +313,23 @@ func (r *Runtime) LoadImageFromSingleImageArchive(ctx context.Context, writer io
 		func() (types.ImageReference, error) {
 			return layout.NewReference(inputFile, "")
 		},
+		func() (types.ImageReference, error) {
+			// This item needs to be last to break out of loop and report meaningful error message
+			return nil,
+				errors.New("payload does not match any of the supported image formats (oci-archive, oci-dir, docker-archive, docker-dir)")
+		},
 	} {
 		src, err := referenceFn()
-		if err == nil && src != nil {
-			newImages, err := r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer)
-			if err == nil {
-				return getImageNames(newImages), nil
-			}
+		if err != nil {
 			saveErr = err
+			continue
+		}
+
+		newImages, err := r.ImageRuntime().LoadFromArchiveReference(ctx, src, signaturePolicy, writer)
+		if err == nil {
+			return getImageNames(newImages), nil
 		}
+		saveErr = err
 	}
 	return "", errors.Wrapf(saveErr, "error pulling image")
 }

pkg/api/handlers/libpod/images.go

@@ -319,33 +319,22 @@ func ExportImages(w http.ResponseWriter, r *http.Request) {
 
 func ImagesLoad(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
-	decoder := r.Context().Value("decoder").(*schema.Decoder)
-	query := struct {
-		Reference string `schema:"reference"`
-	}{
-		// Add defaults here once needed.
-	}
-
-	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
-		utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest,
-			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
-		return
-	}
 
 	tmpfile, err := ioutil.TempFile("", "libpod-images-load.tar")
 	if err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "unable to create tempfile"))
 		return
 	}
 	defer os.Remove(tmpfile.Name())
-	defer tmpfile.Close()
 
-	if _, err := io.Copy(tmpfile, r.Body); err != nil && err != io.EOF {
+	_, err = io.Copy(tmpfile, r.Body)
+	tmpfile.Close()
+
+	if err != nil && err != io.EOF {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "unable to write archive to temporary file"))
 		return
 	}
 
-	tmpfile.Close()
 	loadedImage, err := runtime.LoadImage(context.Background(), tmpfile.Name(), os.Stderr, "")
 	if err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "unable to load image"))

pkg/api/server/register_images.go

@@ -810,11 +810,14 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	// summary: Load image
 	// description: Load an image (oci-archive or docker-archive) stream.
 	// parameters:
-	//   - in: formData
+	//   - in: body
 	//     name: upload
-	//     description: tarball of container image
-	//     type: file
 	//     required: true
+	//     description: tarball of container image
+	//     schema:
+	//       type: string
+	// consumes:
+	// - application/x-tar
 	// produces:
 	// - application/json
 	// responses:

test/python/docker/compat/test_images.py

@@ -1,11 +1,13 @@
 import collections
+import io
 import os
 import subprocess
 import sys
 import time
 import unittest
 
 from docker import DockerClient, errors
+from docker.errors import APIError
 
 from test.python.docker import Podman
 from test.python.docker.compat import common, constant
@@ -79,9 +81,7 @@ def test_list_images(self):
         self.assertEqual(len(self.client.images.list()), 2)
 
         # List images with filter
-        self.assertEqual(
-            len(self.client.images.list(filters={"reference": "alpine"})), 1
-        )
+        self.assertEqual(len(self.client.images.list(filters={"reference": "alpine"})), 1)
 
     def test_search_image(self):
         """Search for image"""
@@ -149,15 +149,22 @@ def test_load_image(self):
 
         self.assertEqual(len(self.client.images.list()), 2)
 
+    def test_load_corrupt_image(self):
+        """Import|Load Image failure"""
+        tarball = io.BytesIO("This is a corrupt tarball".encode("utf-8"))
+        with self.assertRaises(APIError):
+            self.client.images.load(tarball)
+
     def test_build_image(self):
         labels = {"apple": "red", "grape": "green"}
-        _ = self.client.images.build(path="test/python/docker/build_labels", labels=labels, tag="labels")
+        _ = self.client.images.build(
+            path="test/python/docker/build_labels", labels=labels, tag="labels"
+        )
         image = self.client.images.get("labels")
         self.assertEqual(image.labels["apple"], labels["apple"])
         self.assertEqual(image.labels["grape"], labels["grape"])
 
 
-
 if __name__ == "__main__":
     # Setup temporary space
     unittest.main()

test/system/120-load.bats

@@ -32,7 +32,7 @@ verify_iid_and_name() {
     echo "I am an invalid file and should cause a podman-load error" > $invalid
     run_podman 125 load -i $invalid
     # podman and podman-remote emit different messages; this is a common string
-    is "$output" ".*error pulling image: unable to pull .*" \
+    is "$output" ".*payload does not match any of the supported image formats .*" \
        "load -i INVALID fails with expected diagnostic"
 }
 
@@ -137,6 +137,13 @@ verify_iid_and_name() {
        "Diagnostic from 'podman load' without redirection or -i"
 }
 
+@test "podman load - redirect corrupt payload" {
+    run_podman 125 load <<< "Danger, Will Robinson!! This is a corrupt tarball!"
+    is "$output" \
+        ".*payload does not match any of the supported image formats .*" \
+        "Diagnostic from 'podman load' unknown/corrupt payload"
+}
+
 @test "podman load - multi-image archive" {
     img1="quay.io/libpod/testimage:00000000"
     img2="quay.io/libpod/testimage:20200902"