From 5b9f21b38b59322d6c9423aa4f5b028e72dd01f6 Mon Sep 17 00:00:00 2001
From: Stuart McLaren <stuart.mclaren@hp.com>
Date: Mon, 7 Dec 2015 16:46:54 +0000
Subject: [PATCH] Handle 403 forbidden on download

A download of a deactivated image may result in a 403.
The cli should catch this error rather than stack trace.

We also catch other unexpected http responses.

Change-Id: If33fbc3a56cdb02b3ab32a6479a67fff20b4b1a9
Closes-bug: 1523612
---
 glanceclient/common/utils.py                |  2 +-
 glanceclient/tests/unit/v2/test_images.py   |  9 +++++++
 glanceclient/tests/unit/v2/test_shell_v2.py | 30 +++++++++++++++++++++
 glanceclient/v2/shell.py                    |  7 ++++-
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/glanceclient/common/utils.py b/glanceclient/common/utils.py
index 88144ef8..0534d357 100644
--- a/glanceclient/common/utils.py
+++ b/glanceclient/common/utils.py
@@ -283,7 +283,7 @@ def import_versioned_module(version, submodule=None):
 
 def exit(msg='', exit_code=1):
     if msg:
-        print(encodeutils.safe_decode(msg), file=sys.stderr)
+        print_err(msg)
     sys.exit(exit_code)
 
 
diff --git a/glanceclient/tests/unit/v2/test_images.py b/glanceclient/tests/unit/v2/test_images.py
index 15732280..049528bf 100644
--- a/glanceclient/tests/unit/v2/test_images.py
+++ b/glanceclient/tests/unit/v2/test_images.py
@@ -872,6 +872,15 @@ class TestController(testtools.TestCase):
         body = self.controller.data('image_id')
         self.assertEqual(None, body)
 
+    def test_download_forbidden(self):
+        self.controller.http_client.get = mock.Mock(
+            side_effect=exc.HTTPForbidden())
+        try:
+            self.controller.data('image_id')
+            self.fail('No forbidden exception raised.')
+        except exc.HTTPForbidden:
+            pass
+
     def test_update_replace_prop(self):
         image_id = '3a4560a1-e585-443e-9b39-553b46ec92d1'
         params = {'name': 'pong'}
diff --git a/glanceclient/tests/unit/v2/test_shell_v2.py b/glanceclient/tests/unit/v2/test_shell_v2.py
index cdd0ba2f..b473019d 100644
--- a/glanceclient/tests/unit/v2/test_shell_v2.py
+++ b/glanceclient/tests/unit/v2/test_shell_v2.py
@@ -652,6 +652,36 @@ class ShellV2Test(testtools.TestCase):
             self.assert_exits_with_msg(func=test_shell.do_image_delete,
                                        func_args=args)
 
+    @mock.patch.object(utils, 'print_err')
+    def test_do_image_download_with_forbidden_id(self, mocked_print_err):
+        args = self._make_args({'id': 'IMG-01', 'file': None,
+                                'progress': False})
+        with mock.patch.object(self.gc.images, 'data') as mocked_data:
+            mocked_data.side_effect = exc.HTTPForbidden
+            try:
+                test_shell.do_image_download(self.gc, args)
+                self.fail('Exit not called')
+            except SystemExit:
+                pass
+
+            self.assertEqual(1, mocked_data.call_count)
+            self.assertEqual(1, mocked_print_err.call_count)
+
+    @mock.patch.object(utils, 'print_err')
+    def test_do_image_download_with_500(self, mocked_print_err):
+        args = self._make_args({'id': 'IMG-01', 'file': None,
+                                'progress': False})
+        with mock.patch.object(self.gc.images, 'data') as mocked_data:
+            mocked_data.side_effect = exc.HTTPInternalServerError
+            try:
+                test_shell.do_image_download(self.gc, args)
+                self.fail('Exit not called')
+            except SystemExit:
+                pass
+
+            self.assertEqual(1, mocked_data.call_count)
+            self.assertEqual(1, mocked_print_err.call_count)
+
     def test_do_member_list(self):
         args = self._make_args({'image_id': 'IMG-01'})
         with mock.patch.object(self.gc.image_members, 'list') as mocked_list:
diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py
index 99ca3974..f9b9318f 100644
--- a/glanceclient/v2/shell.py
+++ b/glanceclient/v2/shell.py
@@ -275,7 +275,12 @@ def do_explain(gc, args):
            help=_('Show download progress bar.'))
 def do_image_download(gc, args):
     """Download a specific image."""
-    body = gc.images.data(args.id)
+    try:
+        body = gc.images.data(args.id)
+    except (exc.HTTPForbidden, exc.HTTPException) as e:
+        msg = "Unable to download image '%s'. (%s)" % (args.id, e)
+        utils.exit(msg)
+
     if body is None:
         msg = ('Image %s has no data.' % args.id)
         utils.exit(msg)