Resolve B113 error (Requests call without timeout)

Add tunable timeout options to all request calls, so that slow response
does not hung up heat.

Change-Id: I16911cfe8e47f74f758103090bcba27b1750a350

heat/api/aws/ec2token.py

@@ -49,6 +49,10 @@ opts = [
                 default=False,
                 help=_('If set, then the server\'s certificate will not '
                        'be verified.')),
+    cfg.FloatOpt('timeout',
+                 default=60,
+                 min=0,
+                 help=_('Timeout in seconds for HTTP requests.')),
 ]
 cfg.CONF.register_opts(opts, group='ec2authtoken')
 
@@ -205,11 +209,13 @@ class EC2Token(wsgi.Middleware):
         headers = {'Content-Type': 'application/json'}
 
         keystone_ec2_uri = self._conf_get_keystone_ec2_uri(auth_uri)
+        timeout = self._conf_get('timeout')
         LOG.info('Authenticating with %s', keystone_ec2_uri)
         response = requests.post(keystone_ec2_uri, data=creds_json,
                                  headers=headers,
                                  verify=self.ssl_options['verify'],
-                                 cert=self.ssl_options['cert'])
+                                 cert=self.ssl_options['cert'],
+                                 timeout=timeout)
         result = response.json()
         try:
             token_id = response.headers['X-Subject-Token']

heat/common/config.py

@@ -88,6 +88,10 @@ service_opts = [
     cfg.IntOpt('max_nested_stack_depth',
                default=5,
                help=_('Maximum depth allowed when using nested stacks.')),
+    cfg.FloatOpt('template_fetch_timeout',
+                 default=60,
+                 min=0,
+                 help=_('Timeout in seconds for template download.')),
     cfg.IntOpt('num_engine_workers',
                help=_('Number of heat-engine processes to fork and run. '
                       'Will default to either to 4 or number of CPUs on '
@@ -319,7 +323,13 @@ engine_opts = [
                 default=False,
                 help=_('Encrypt template parameters that were marked as'
                        ' hidden and also all the resource properties before'
-                       ' storing them in database.'))]
+                       ' storing them in database.')),
+    cfg.FloatOpt('metadata_put_timeout',
+                 default=60,
+                 min=0,
+                 help=_('Timeout in seconds for metadata update for '
+                        'software deployment'))
+    ]
 
 rpc_opts = [
     cfg.StrOpt('host',

heat/common/urlfetch.py

@@ -57,7 +57,8 @@ def get(url, allowed_schemes=('http', 'https')):
             raise URLFetchError(_('Failed to retrieve template: %s') % uex)
 
     try:
-        resp = requests.get(url, stream=True)
+        resp = requests.get(url, stream=True,
+                            timeout=cfg.CONF.template_fetch_timeout)
         resp.raise_for_status()
 
         # We cannot use resp.text here because it would download the

heat/engine/service_software_config.py

@@ -14,6 +14,7 @@
 import itertools
 import uuid
 
+from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import timeutils
@@ -35,6 +36,8 @@ from heat.rpc import api as rpc_api
 
 LOG = logging.getLogger(__name__)
 
+cfg.CONF.import_opt('metadata_put_timeout', 'heat.common.config')
+
 
 class SoftwareConfigService(object):
 
@@ -135,10 +138,11 @@ class SoftwareConfigService(object):
 
         if metadata_put_url:
             json_md = jsonutils.dumps(md)
-            resp = requests.put(metadata_put_url, json_md)
             try:
+                resp = requests.put(metadata_put_url, json_md,
+                                    timeout=cfg.CONF.metadata_put_timeout)
                 resp.raise_for_status()
-            except requests.HTTPError as exc:
+            except requests.RequestException as exc:
                 LOG.error('Failed to deliver deployment data to '
                           'server %s: %s', server_id, exc)
         if metadata_queue_id:

heat/tests/api/aws/test_api_ec2token.py

@@ -288,7 +288,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_ok_roles(self):
         dummy_conf = {'auth_uri': 'http://123:5000/v2.0'}
@@ -317,7 +318,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_err_tokenid(self):
         dummy_conf = {'auth_uri': 'http://123:5000/v2.0/'}
@@ -342,7 +344,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_err_signature(self):
         dummy_conf = {'auth_uri': 'http://123:5000/v2.0'}
@@ -367,7 +370,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_err_denied(self):
         dummy_conf = {'auth_uri': 'http://123:5000/v2.0'}
@@ -391,7 +395,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_ok_v2(self):
         dummy_conf = {'auth_uri': 'http://123:5000/v2.0'}
@@ -411,7 +416,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_once_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_ok_multicloud(self):
         dummy_conf = {
@@ -451,7 +457,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_err_multicloud(self):
         dummy_conf = {
@@ -492,7 +499,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_err_multicloud_none_allowed(self):
         dummy_conf = {
@@ -541,7 +549,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_ok_auth_uri_ec2authtoken_long(self):
         # Prove we tolerate a url which already includes the /ec2tokens path
@@ -564,7 +573,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_call_ok_auth_uri_ks_authtoken(self):
         # Import auth_token to have keystone_authtoken settings setup.
@@ -592,7 +602,8 @@ class Ec2TokenTest(common.HeatTestCase):
         requests.post.assert_called_with(
             self.verify_req_url, data=self.verify_data,
             verify=self.verify_verify,
-            cert=self.verify_cert, headers=self.verify_req_headers)
+            cert=self.verify_cert, headers=self.verify_req_headers,
+            timeout=60)
 
     def test_filter_factory(self):
         ec2_filter = ec2token.EC2Token_filter_factory(global_conf={})

heat/tests/engine/service/test_software_config.py

@@ -778,7 +778,8 @@ class SoftwareConfigServiceTest(common.HeatTestCase):
         ])
 
         put.assert_called_once_with(
-            'http://192.168.2.2/foo/bar', json.dumps(result_metadata))
+            'http://192.168.2.2/foo/bar', json.dumps(result_metadata),
+            timeout=60)
 
     @mock.patch.object(service_software_config.SoftwareConfigService,
                        'metadata_software_deployments')

heat/tests/test_urlfetch.py

@@ -65,7 +65,7 @@ class UrlFetchTest(common.HeatTestCase):
         mock_get = self.patchobject(requests, 'get')
         mock_get.return_value = response
         self.assertEqual(data, urlfetch.get(url))
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)
 
     def test_https_scheme(self):
         url = 'https://example.com/template'
@@ -74,21 +74,21 @@ class UrlFetchTest(common.HeatTestCase):
         mock_get = self.patchobject(requests, 'get')
         mock_get.return_value = response
         self.assertEqual(data, urlfetch.get(url))
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)
 
     def test_http_error(self):
         url = 'http://example.com/template'
         mock_get = self.patchobject(requests, 'get')
         mock_get.side_effect = exceptions.HTTPError()
         self.assertRaises(urlfetch.URLFetchError, urlfetch.get, url)
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)
 
     def test_non_exist_url(self):
         url = 'http://non-exist.com/template'
         mock_get = self.patchobject(requests, 'get')
         mock_get.side_effect = exceptions.Timeout()
         self.assertRaises(urlfetch.URLFetchError, urlfetch.get, url)
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)
 
     def test_garbage(self):
         self.assertRaises(urlfetch.URLFetchError, urlfetch.get, 'wibble')
@@ -101,7 +101,7 @@ class UrlFetchTest(common.HeatTestCase):
         mock_get = self.patchobject(requests, 'get')
         mock_get.return_value = response
         urlfetch.get(url)
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)
 
     def test_max_fetch_size_error(self):
         url = 'http://example.com/template'
@@ -113,4 +113,4 @@ class UrlFetchTest(common.HeatTestCase):
         exception = self.assertRaises(urlfetch.URLFetchError,
                                       urlfetch.get, url)
         self.assertIn("Template exceeds", str(exception))
-        mock_get.assert_called_once_with(url, stream=True)
+        mock_get.assert_called_once_with(url, stream=True, timeout=60)

releasenotes/notes/requests-timeout-5d586f59dbe9bbfd.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    The following parameters have been added, to define timeout in internal
+    HTTP requests.
+
+    - ``[DEFAULT] metadata_put_timeout``
+    - ``[DEFAULT] template_fetch_timeout``
+    - ``[ec2authtoken] timeout``

tox.ini

@@ -32,7 +32,6 @@ commands =
     # B104: Test for binding to all interfaces
     # B107: Test for use of hard-coded password argument defaults
     # B110: Try, Except, Pass detected.
-    # B113: Requests call without timeout
     # B310: Audit url open for permitted schemes
     # B311: Standard pseudo-random generators are not suitable for security/cryptographic purposes
     # B404: Import of subprocess module
@@ -41,7 +40,7 @@ commands =
     # B506: Test for use of yaml load
     # B603: Test for use of subprocess with shell equals true
     # B607: Test for starting a process with a partial path
-    bandit -r heat -x tests --skip B101,B104,B107,B110,B113,B310,B311,B404,B410,B504,B506,B603,B607
+    bandit -r heat -x tests --skip B101,B104,B107,B110,B310,B311,B404,B410,B504,B506,B603,B607
     doc8 {posargs}
 
 [testenv:venv]
@@ -102,7 +101,6 @@ deps =
 # B101: Test for use of assert
 # B104: Test for binding to all interfaces
 # B110: Try, Except, Pass detected.
-# B113: Requests call without timeout
 # B310: Audit url open for permitted schemes
 # B311: Standard pseudo-random generators are not suitable for security/cryptographic purposes
 # B404: Import of subprocess module
@@ -111,7 +109,7 @@ deps =
 # B506: Test for use of yaml load
 # B603: Test for use of subprocess with shell equals true
 # B607: Test for starting a process with a partial path
-commands = bandit -r heat -x tests --skip B101,B104,B110,B113,B310,B311,B404,B410,B504,B506,B603,B607
+commands = bandit -r heat -x tests --skip B101,B104,B110,B310,B311,B404,B410,B504,B506,B603,B607
 
 [flake8]
 show-source = true