openstack/ironic
Code Issues Proposed changes

Fix requests calls with timeouts

Browse Source 
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
This commit is contained in:
Julia Kreger 2023-04-04 10:35:04 -07:00
parent 17cabc4b81
commit 9f422a8dfd
6 changed files with 37 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
ironic/common/kickstart_utils.py
View File

@ -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'. "

6
ironic/common/molds.py
View File

@ -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)

5
ironic/conf/default.py
View File

@ -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.')),
]

3
ironic/drivers/modules/ansible/playbooks/library/stream_url.py
View File

@ -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)

3
ironic/tests/unit/common/test_kickstart_utils.py
View File

@ -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)

35
ironic/tests/unit/common/test_molds.py
View File

@ -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)