From 9765947ac0af25ab95771b9745efb1a55cf6b14d Mon Sep 17 00:00:00 2001
From: Felix Fontein <felix@fontein.de>
Date: Wed, 23 Dec 2020 10:36:35 +0100
Subject: [PATCH] Check output of load_image and react upon.

---
 .../fragments/55-docker_image-loading.yml     |  3 ++
 plugins/modules/docker_image.py               | 38 ++++++++++++++++---
 .../docker_image/tasks/tests/options.yml      | 28 +++++++++++++-
 3 files changed, 63 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/fragments/55-docker_image-loading.yml

diff --git a/changelogs/fragments/55-docker_image-loading.yml b/changelogs/fragments/55-docker_image-loading.yml
new file mode 100644
index 000000000..c7694f059
--- /dev/null
+++ b/changelogs/fragments/55-docker_image-loading.yml
@@ -0,0 +1,3 @@
+bugfixes:
+- "docker_image - report error when loading a broken archive that contains no image (https://github.com/ansible-collections/community.docker/issues/46, https://github.com/ansible-collections/community.docker/pull/55)."
+- "docker_image - report error when the loaded archive does not contain the specified image (https://github.com/ansible-collections/community.docker/issues/41, https://github.com/ansible-collections/community.docker/pull/55)."
diff --git a/plugins/modules/docker_image.py b/plugins/modules/docker_image.py
index 3daf00904..0ee048bab 100644
--- a/plugins/modules/docker_image.py
+++ b/plugins/modules/docker_image.py
@@ -640,7 +640,7 @@ def build_image(self):
             # line = json.loads(line)
             self.log(line, pretty_print=True)
             if "stream" in line or "status" in line:
-                build_line = line.get("stream") or line.get("status")
+                build_line = line.get("stream") or line.get("status") or ''
                 build_output.append(build_line)
 
             if line.get('error'):
@@ -665,17 +665,45 @@ def load_image(self):
 
         :return: image dict
         '''
+        # Load image(s) from file
+        load_output = []
         try:
             self.log("Opening image %s" % self.load_path)
             with open(self.load_path, 'rb') as image_tar:
                 self.log("Loading image from %s" % self.load_path)
-                self.client.load_image(image_tar)
+                for line in self.client.load_image(image_tar):
+                    self.log(line, pretty_print=True)
+                    if "stream" in line or "status" in line:
+                        load_line = line.get("stream") or line.get("status") or ''
+                        load_output.append(load_line)
         except EnvironmentError as exc:
             if exc.errno == errno.ENOENT:
-                self.fail("Error opening image %s - %s" % (self.load_path, str(exc)))
-            self.fail("Error loading image %s - %s" % (self.name, str(exc)))
+                self.client.fail("Error opening image %s - %s" % (self.load_path, str(exc)))
+            self.client.fail("Error loading image %s - %s" % (self.name, str(exc)), stdout='\n'.join(load_output))
         except Exception as exc:
-            self.fail("Error loading image %s - %s" % (self.name, str(exc)))
+            self.client.fail("Error loading image %s - %s" % (self.name, str(exc)), stdout='\n'.join(load_output))
+
+        # Collect loaded images
+        loaded_images = set()
+        for line in load_output:
+            if line.startswith('Loaded image:'):
+                loaded_images.add(line[len('Loaded image:'):].strip())
+
+        if not loaded_images:
+            self.client.fail("Detected no loaded images. Archive potentially corrupt?", stdout='\n'.join(load_output))
+
+        expected_image = '%s:%s' % (self.name, self.tag)
+        if expected_image not in loaded_images:
+            self.client.fail(
+                "The archive did not contain image '%s'. Instead, found %s." % (
+                    expected_image, ', '.join(["'%s'" % image for image in sorted(loaded_images)])),
+                stdout='\n'.join(load_output))
+        loaded_images.remove(expected_image)
+
+        if loaded_images:
+            self.client.module.warn(
+                "The archive contained more images than specified: %s" % (
+                    ', '.join(["'%s'" % image for image in sorted(loaded_images)]), ))
 
         return self.client.find_image(self.name, self.tag)
 
diff --git a/tests/integration/targets/docker_image/tasks/tests/options.yml b/tests/integration/targets/docker_image/tasks/tests/options.yml
index b04fa8ea4..576f5d459 100644
--- a/tests/integration/targets/docker_image/tasks/tests/options.yml
+++ b/tests/integration/targets/docker_image/tasks/tests/options.yml
@@ -189,6 +189,11 @@
     source: pull
   register: archive_image
 
+- name: Create invalid archive
+  copy:
+    dest: "{{ output_dir }}/image-invalid.tar"
+    content: "this is not a valid image"
+
 - name: remove image
   docker_image:
     name: "{{ docker_test_image_hello_world }}"
@@ -209,11 +214,32 @@
     source: load
   register: load_image_1
 
+- name: load image (wrong name)
+  docker_image:
+    name: foo:bar
+    load_path: "{{ output_dir }}/image.tar"
+    source: load
+  register: load_image_2
+  ignore_errors: true
+
+- name: load image (invalid image)
+  docker_image:
+    name: foo:bar
+    load_path: "{{ output_dir }}/image-invalid.tar"
+    source: load
+  register: load_image_3
+  ignore_errors: true
+
 - assert:
     that:
     - load_image is changed
-    - load_image_1 is not changed
     - archive_image['image']['Id'] == load_image['image']['Id']
+    - load_image_1 is not changed
+    - load_image_2 is failed
+    - >-
+      "The archive did not contain image 'foo:bar'. Instead, found '" ~ docker_test_image_hello_world ~ "'." == load_image_2.msg
+    - load_image_3 is failed
+    - '"Detected no loaded images. Archive potentially corrupt?" == load_image_3.msg'
 
 ####################################################################
 ## path ############################################################