Fix Bandit errors
Bandit 1.7.5 released with a timeout check for all requests and urllib calls. Fixed those. In the process, then exposed a bandit b310 issue, which was already covered by the code, but explicitly marked it as such. Also, enables bandit checks to be voting for CI.. Change-Id: If0e87790191f5f3648366d571e1d85dd7393a548
This commit is contained in:
parent
03e88b579e
commit
78c1343a54
ironic_python_agent
releasenotes/notes
zuul.d
@ -332,6 +332,12 @@ cli_opts = [
|
|||||||
default=True,
|
default=True,
|
||||||
help='If the MD5 algorithm is enabled for file checksums. '
|
help='If the MD5 algorithm is enabled for file checksums. '
|
||||||
'Will be changed to False in the future.'),
|
'Will be changed to False in the future.'),
|
||||||
|
cfg.IntOpt('http_request_timeout',
|
||||||
|
default=APARAMS.get('ipa-http-request-timeout', 30),
|
||||||
|
min=1,
|
||||||
|
help='Time in seconds to wait for an HTTP request TCP socket '
|
||||||
|
'used by an API request to a remote service to enter '
|
||||||
|
'a state where a request can be transmitted.'),
|
||||||
]
|
]
|
||||||
|
|
||||||
CONF.register_cli_opts(cli_opts)
|
CONF.register_cli_opts(cli_opts)
|
||||||
|
@ -371,7 +371,7 @@ class ImageDownload(object):
|
|||||||
# Realistically, this should never happen, but for
|
# Realistically, this should never happen, but for
|
||||||
# compatability...
|
# compatability...
|
||||||
# TODO(TheJulia): Remove for a 2024 release.
|
# TODO(TheJulia): Remove for a 2024 release.
|
||||||
self._hash_algo = hashlib.new('md5')
|
self._hash_algo = hashlib.new('md5') # nosec
|
||||||
else:
|
else:
|
||||||
self._hash_algo = new_algo
|
self._hash_algo = new_algo
|
||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
|
@ -23,11 +23,15 @@ from urllib import request
|
|||||||
from ironic_lib.common.i18n import _
|
from ironic_lib.common.i18n import _
|
||||||
from ironic_lib.exception import IronicException
|
from ironic_lib.exception import IronicException
|
||||||
from oslo_concurrency import processutils
|
from oslo_concurrency import processutils
|
||||||
|
from oslo_config import cfg
|
||||||
from oslo_log import log
|
from oslo_log import log
|
||||||
from oslo_utils import fileutils
|
from oslo_utils import fileutils
|
||||||
|
|
||||||
from ironic_python_agent import utils
|
from ironic_python_agent import utils
|
||||||
|
|
||||||
|
|
||||||
|
CONF = cfg.CONF
|
||||||
|
|
||||||
FW_VERSION_REGEX = r'FW Version:\s*\t*(?P<fw_ver>\d+\.\d+\.\d+)'
|
FW_VERSION_REGEX = r'FW Version:\s*\t*(?P<fw_ver>\d+\.\d+\.\d+)'
|
||||||
RUNNING_FW_VERSION_REGEX = \
|
RUNNING_FW_VERSION_REGEX = \
|
||||||
r'FW Version\(Running\):\s*\t*(?P<fw_ver>\d+\.\d+\.\d+)'
|
r'FW Version\(Running\):\s*\t*(?P<fw_ver>\d+\.\d+\.\d+)'
|
||||||
@ -415,7 +419,10 @@ class NvidiaNicFirmwareBinary(object):
|
|||||||
try:
|
try:
|
||||||
LOG.info('Downloading file: %s to %s', self.url,
|
LOG.info('Downloading file: %s to %s', self.url,
|
||||||
self.dest_file_path)
|
self.dest_file_path)
|
||||||
url_data = request.urlopen(self.url)
|
# NOTE(TheJulia: nosec b310 rule below is covered by _process_url
|
||||||
|
url_data = request.urlopen(
|
||||||
|
self.url,
|
||||||
|
timeout=CONF.image_download_connection_timeout) # nosec
|
||||||
except urlError.URLError as url_error:
|
except urlError.URLError as url_error:
|
||||||
LOG.error('Failed to open URL data: %s', url_error)
|
LOG.error('Failed to open URL data: %s', url_error)
|
||||||
raise url_error
|
raise url_error
|
||||||
|
@ -139,8 +139,10 @@ def call_inspector(data, failures):
|
|||||||
wait=tenacity.wait_fixed(_RETRY_WAIT),
|
wait=tenacity.wait_fixed(_RETRY_WAIT),
|
||||||
reraise=True)
|
reraise=True)
|
||||||
def _post_to_inspector():
|
def _post_to_inspector():
|
||||||
return requests.post(CONF.inspection_callback_url, data=data,
|
return requests.post(
|
||||||
verify=verify, cert=cert)
|
CONF.inspection_callback_url, data=data,
|
||||||
|
verify=verify, cert=cert,
|
||||||
|
timeout=CONF.http_request_timeout)
|
||||||
|
|
||||||
resp = _post_to_inspector()
|
resp = _post_to_inspector()
|
||||||
if resp.status_code >= 400:
|
if resp.status_code >= 400:
|
||||||
|
@ -80,6 +80,7 @@ class APIClient(object):
|
|||||||
data=data,
|
data=data,
|
||||||
verify=verify,
|
verify=verify,
|
||||||
cert=cert,
|
cert=cert,
|
||||||
|
timeout=CONF.http_request_timeout,
|
||||||
**kwargs)
|
**kwargs)
|
||||||
|
|
||||||
def _get_ironic_api_version_header(self, version=None):
|
def _get_ironic_api_version_header(self, version=None):
|
||||||
|
@ -163,7 +163,8 @@ class TestCallInspector(base.IronicAgentTest):
|
|||||||
|
|
||||||
mock_post.assert_called_once_with('url',
|
mock_post.assert_called_once_with('url',
|
||||||
cert=None, verify=True,
|
cert=None, verify=True,
|
||||||
data='{"data": 42, "error": null}')
|
data='{"data": 42, "error": null}',
|
||||||
|
timeout=30)
|
||||||
self.assertEqual(mock_post.return_value.json.return_value, res)
|
self.assertEqual(mock_post.return_value.json.return_value, res)
|
||||||
|
|
||||||
def test_send_failure(self, mock_post):
|
def test_send_failure(self, mock_post):
|
||||||
@ -176,7 +177,8 @@ class TestCallInspector(base.IronicAgentTest):
|
|||||||
|
|
||||||
mock_post.assert_called_once_with('url',
|
mock_post.assert_called_once_with('url',
|
||||||
cert=None, verify=True,
|
cert=None, verify=True,
|
||||||
data='{"data": 42, "error": "boom"}')
|
data='{"data": 42, "error": "boom"}',
|
||||||
|
timeout=30)
|
||||||
self.assertEqual(mock_post.return_value.json.return_value, res)
|
self.assertEqual(mock_post.return_value.json.return_value, res)
|
||||||
|
|
||||||
def test_inspector_error(self, mock_post):
|
def test_inspector_error(self, mock_post):
|
||||||
@ -188,7 +190,8 @@ class TestCallInspector(base.IronicAgentTest):
|
|||||||
|
|
||||||
mock_post.assert_called_once_with('url',
|
mock_post.assert_called_once_with('url',
|
||||||
cert=None, verify=True,
|
cert=None, verify=True,
|
||||||
data='{"data": 42, "error": null}')
|
data='{"data": 42, "error": null}',
|
||||||
|
timeout=30)
|
||||||
self.assertIsNone(res)
|
self.assertIsNone(res)
|
||||||
|
|
||||||
@mock.patch.object(inspector, '_RETRY_WAIT', 0.01)
|
@mock.patch.object(inspector, '_RETRY_WAIT', 0.01)
|
||||||
|
17
releasenotes/notes/bandit-fixes-a971142075b29ca9.yaml
Normal file
17
releasenotes/notes/bandit-fixes-a971142075b29ca9.yaml
Normal file
@ -0,0 +1,17 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixes timeout declarations for Bandit 1.7.5 rule additions.
|
||||||
|
- |
|
||||||
|
Adds a new configuration option ``http_request_timeout``
|
||||||
|
to allow for operators to set the amount of time to wait
|
||||||
|
for a new request socket to wait. This helps prevent prevent
|
||||||
|
a possible hanged connection should the initial packets be
|
||||||
|
lost in tranist.
|
||||||
|
other:
|
||||||
|
- |
|
||||||
|
Adds a new configuration option ``http_request_timeout``
|
||||||
|
which is also accessible utilizing the kernel command line
|
||||||
|
option ``ipa-http-request-timeout``. This option helps prevent
|
||||||
|
failed connections from hanging the agent. The default is 30
|
||||||
|
seconds.
|
@ -9,6 +9,7 @@
|
|||||||
check:
|
check:
|
||||||
jobs:
|
jobs:
|
||||||
- openstack-tox-functional
|
- openstack-tox-functional
|
||||||
|
- ipa-tox-bandit
|
||||||
- ipa-tox-examples
|
- ipa-tox-examples
|
||||||
# NOTE(iurygregory) Only run this two jobs since we are testing
|
# NOTE(iurygregory) Only run this two jobs since we are testing
|
||||||
# wholedisk + partition on tempest
|
# wholedisk + partition on tempest
|
||||||
@ -26,11 +27,10 @@
|
|||||||
# Non-voting jobs
|
# Non-voting jobs
|
||||||
- ipa-tempest-ironic-inspector-src:
|
- ipa-tempest-ironic-inspector-src:
|
||||||
voting: false
|
voting: false
|
||||||
- ipa-tox-bandit:
|
|
||||||
voting: false
|
|
||||||
gate:
|
gate:
|
||||||
jobs:
|
jobs:
|
||||||
- openstack-tox-functional
|
- openstack-tox-functional
|
||||||
|
- ipa-tox-bandit
|
||||||
- ipa-tox-examples
|
- ipa-tox-examples
|
||||||
- ipa-tempest-bios-ipmi-direct-src
|
- ipa-tempest-bios-ipmi-direct-src
|
||||||
- ipa-tempest-uefi-redfish-vmedia-src
|
- ipa-tempest-uefi-redfish-vmedia-src
|
||||||
|
Loading…
x
Reference in New Issue
Block a user