From 9f422a8dfd3dad9072d391e68fb7fb14b6a4cedb Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 4 Apr 2023 10:35:04 -0700 Subject: Fix requests calls with timeouts Bandit 1.7.5 dropped with logic to check requests invocations. Specifically if a timeout is not explicitly set, then it results in an error. This should cause our bandit job to go green. Closes-Bug: 2015284 Change-Id: I1dcb3075de63aae97bb22012a54736c293393185 --- ironic/common/kickstart_utils.py | 4 ++- ironic/common/molds.py | 6 ++-- ironic/conf/default.py | 5 ++-- .../ansible/playbooks/library/stream_url.py | 3 +- ironic/tests/unit/common/test_kickstart_utils.py | 3 +- ironic/tests/unit/common/test_molds.py | 35 ++++++++++++++-------- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/ironic/common/kickstart_utils.py b/ironic/common/kickstart_utils.py index 433cf2390..4e02e2ea7 100644 --- a/ironic/common/kickstart_utils.py +++ b/ironic/common/kickstart_utils.py @@ -23,6 +23,7 @@ import pycdlib import requests from ironic.common import exception +from ironic.conf import CONF LOG = logging.getLogger(__name__) @@ -107,7 +108,8 @@ def decode_and_extract_config_drive_iso(config_drive_iso_gz): def _fetch_config_drive_from_url(url): try: - config_drive = requests.get(url).content + config_drive = requests.get( + url, timeout=CONF.webserver_connection_timeout).content except requests.exceptions.RequestException as e: raise exception.InstanceDeployFailure( "Can't download the configdrive content from '%(url)s'. " diff --git a/ironic/common/molds.py b/ironic/common/molds.py index 234fcc6e3..a77e42a63 100644 --- a/ironic/common/molds.py +++ b/ironic/common/molds.py @@ -49,7 +49,8 @@ def save_configuration(task, url, data): ) def _request(url, data, auth_header): return requests.put( - url, data=json.dumps(data, indent=2), headers=auth_header) + url, data=json.dumps(data, indent=2), headers=auth_header, + timeout=CONF.webserver_connection_timeout) auth_header = _get_auth_header(task) response = _request(url, data, auth_header) @@ -76,7 +77,8 @@ def get_configuration(task, url): reraise=True ) def _request(url, auth_header): - return requests.get(url, headers=auth_header) + return requests.get(url, headers=auth_header, + timeout=CONF.webserver_connection_timeout) auth_header = _get_auth_header(task) response = _request(url, auth_header) diff --git a/ironic/conf/default.py b/ironic/conf/default.py index c7aff69cc..5b40c1f31 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -426,8 +426,9 @@ webserver_opts = [ 'Defaults to True.')), cfg.IntOpt('webserver_connection_timeout', default=60, - help=_('Connection timeout when accessing remote web servers ' - 'with images.')), + help=_('Connection timeout when accessing/interacting with ' + 'remote web servers with images or other artifacts ' + 'being accessed.')), ] diff --git a/ironic/drivers/modules/ansible/playbooks/library/stream_url.py b/ironic/drivers/modules/ansible/playbooks/library/stream_url.py index 0da3cc4dd..786b65013 100644 --- a/ironic/drivers/modules/ansible/playbooks/library/stream_url.py +++ b/ironic/drivers/modules/ansible/playbooks/library/stream_url.py @@ -31,7 +31,8 @@ class StreamingDownloader(object): else: self.hasher = None self.chunksize = chunksize - resp = requests.get(url, stream=True, verify=verify, cert=certs) + resp = requests.get(url, stream=True, verify=verify, cert=certs, + timeout=30) if resp.status_code != 200: raise Exception('Invalid response code: %s' % resp.status_code) diff --git a/ironic/tests/unit/common/test_kickstart_utils.py b/ironic/tests/unit/common/test_kickstart_utils.py index 0dd1ac572..db6123b9d 100644 --- a/ironic/tests/unit/common/test_kickstart_utils.py +++ b/ironic/tests/unit/common/test_kickstart_utils.py @@ -129,4 +129,5 @@ echo $CONTENT | /usr/bin/base64 --decode > {file_path}\n\ task.node.instance_info = i_info task.node.save() self.assertEqual(expected, ks_utils.prepare_config_drive(task)) - mock_get.assert_called_with('http://server/fake-configdrive-url') + mock_get.assert_called_with('http://server/fake-configdrive-url', + timeout=60) diff --git a/ironic/tests/unit/common/test_molds.py b/ironic/tests/unit/common/test_molds.py index 810dd61bc..2323c2fa8 100644 --- a/ironic/tests/unit/common/test_molds.py +++ b/ironic/tests/unit/common/test_molds.py @@ -46,7 +46,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): molds.save_configuration(task, url, data) mock_put.assert_called_once_with(url, '{\n "key": "value"\n}', - headers={'X-Auth-Token': 'token'}) + headers={'X-Auth-Token': 'token'}, + timeout=60) @mock.patch.object(swift, 'get_swift_session', autospec=True) @mock.patch.object(requests, 'put', autospec=True) @@ -77,7 +78,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): mock_put.assert_called_once_with( url, '{\n "key": "value"\n}', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) @mock.patch.object(requests, 'put', autospec=True) def test_save_configuration_http_noauth(self, mock_put): @@ -91,7 +93,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): molds.save_configuration(task, url, data) mock_put.assert_called_once_with( url, '{\n "key": "value"\n}', - headers=None) + headers=None, + timeout=60) @mock.patch.object(requests, 'put', autospec=True) def test_save_configuration_http_error(self, mock_put): @@ -112,7 +115,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): {'key': 'value'}) mock_put.assert_called_once_with( 'https://example.com/file2', '{\n "key": "value"\n}', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) @mock.patch.object(requests, 'put', autospec=True) def test_save_configuration_connection_error(self, mock_put): @@ -132,7 +136,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): task, 'https://example.com/file2', {'key': 'value'}) mock_put.assert_called_with( 'https://example.com/file2', '{\n "key": "value"\n}', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) self.assertEqual(mock_put.call_count, 3) @mock.patch.object(requests, 'put', autospec=True) @@ -155,7 +160,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): {'key': 'value'}) mock_put.assert_called_with( 'https://example.com/file2', '{\n "key": "value"\n}', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) self.assertEqual(mock_put.call_count, 2) @mock.patch.object(swift, 'get_swift_session', autospec=True) @@ -176,7 +182,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): result = molds.get_configuration(task, url) mock_get.assert_called_once_with( - url, headers={'X-Auth-Token': 'token'}) + url, headers={'X-Auth-Token': 'token'}, + timeout=60) self.assertJsonEqual({'key': 'value'}, result) @mock.patch.object(swift, 'get_swift_session', autospec=True) @@ -210,7 +217,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): result = molds.get_configuration(task, url) mock_get.assert_called_once_with( - url, headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + url, headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) self.assertJsonEqual({"key": "value"}, result) @mock.patch.object(requests, 'get', autospec=True) @@ -228,7 +236,7 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid) as task: result = molds.get_configuration(task, url) - mock_get.assert_called_once_with(url, headers=None) + mock_get.assert_called_once_with(url, headers=None, timeout=60) self.assertJsonEqual({"key": "value"}, result) @mock.patch.object(requests, 'get', autospec=True) @@ -249,7 +257,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): 'https://example.com/file2') mock_get.assert_called_once_with( 'https://example.com/file2', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) @mock.patch.object(requests, 'get', autospec=True) def test_get_configuration_connection_error(self, mock_get): @@ -269,7 +278,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): task, 'https://example.com/file2') mock_get.assert_called_with( 'https://example.com/file2', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) self.assertEqual(mock_get.call_count, 3) @mock.patch.object(requests, 'get', autospec=True) @@ -291,7 +301,8 @@ class ConfigurationMoldTestCase(db_base.DbTestCase): 'https://example.com/file2') mock_get.assert_called_with( 'https://example.com/file2', - headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}) + headers={'Authorization': 'Basic dXNlcjpwYXNzd29yZA=='}, + timeout=60) self.assertEqual(mock_get.call_count, 2) @mock.patch.object(requests, 'get', autospec=True) -- cgit v1.2.1