diff --git a/heat/api/aws/ec2token.py b/heat/api/aws/ec2token.py index 39b240f5aa..27ac014597 100644 --- a/heat/api/aws/ec2token.py +++ b/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'] diff --git a/heat/common/config.py b/heat/common/config.py index c6a1dfcfe4..e2499afa0c 100644 --- a/heat/common/config.py +++ b/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', diff --git a/heat/common/urlfetch.py b/heat/common/urlfetch.py index 633c309c0c..17e4f3c309 100644 --- a/heat/common/urlfetch.py +++ b/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 diff --git a/heat/engine/service_software_config.py b/heat/engine/service_software_config.py index ce5001bbb8..a859e6f41b 100644 --- a/heat/engine/service_software_config.py +++ b/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: diff --git a/heat/tests/api/aws/test_api_ec2token.py b/heat/tests/api/aws/test_api_ec2token.py index af1305f609..1e01de4bc6 100644 --- a/heat/tests/api/aws/test_api_ec2token.py +++ b/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={}) diff --git a/heat/tests/engine/service/test_software_config.py b/heat/tests/engine/service/test_software_config.py index 90645c3dbe..5ec6a7bbc7 100644 --- a/heat/tests/engine/service/test_software_config.py +++ b/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') diff --git a/heat/tests/test_urlfetch.py b/heat/tests/test_urlfetch.py index ac3f75bd21..9cc7ec111b 100644 --- a/heat/tests/test_urlfetch.py +++ b/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) diff --git a/releasenotes/notes/requests-timeout-5d586f59dbe9bbfd.yaml b/releasenotes/notes/requests-timeout-5d586f59dbe9bbfd.yaml new file mode 100644 index 0000000000..8bc38ca951 --- /dev/null +++ b/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`` diff --git a/tox.ini b/tox.ini index 7dbbd6c212..a9646dbda7 100644 --- a/tox.ini +++ b/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