From be290456015aedfcf6fac5b5b434c043a1d0408f Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 25 Mar 2024 17:18:20 +0100 Subject: [PATCH] Handle NotAcceptable when Accept-Encoding: identity is not allowed Apparently, some HPE Gen 10 Plus machines do not allow identity encoding when BIOS registries are requested. Add a fallback to Connector. Change-Id: I7363df5f5f74705307990dda9dfc7baebd5c07a3 --- .../accept-encoding-4646ea43998f80bd.yaml | 5 + sushy/connector.py | 42 ++++++-- sushy/exceptions.py | 6 ++ sushy/tests/unit/test_connector.py | 101 ++++++++++++------ 4 files changed, 111 insertions(+), 43 deletions(-) create mode 100644 releasenotes/notes/accept-encoding-4646ea43998f80bd.yaml diff --git a/releasenotes/notes/accept-encoding-4646ea43998f80bd.yaml b/releasenotes/notes/accept-encoding-4646ea43998f80bd.yaml new file mode 100644 index 00000000..9d1781ab --- /dev/null +++ b/releasenotes/notes/accept-encoding-4646ea43998f80bd.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Adds a work-around for cases where ``Accept-Encoding: identity`` is not + accepted. diff --git a/sushy/connector.py b/sushy/connector.py index 75b341c4..71cec0ee 100644 --- a/sushy/connector.py +++ b/sushy/connector.py @@ -51,15 +51,6 @@ class Connector(object): # By default, we ask HTTP server to shut down HTTP connection we've # just used. self._session.headers['Connection'] = 'close' - # NOTE(TheJulia): Depending on the BMC, offering compression as an - # acceptable response changes the ETag behavior to offering an - # automatic "weak" ETag response, which is appropriate because the - # body content *may* not be a byte for byte match given compression. - # Overall, the value of compression is less than the value of concise - # interaction with the BMC. Setting to identity basically means - # "without modification or compression". By default, python-requests - # indicates responses can be compressed. - self._session.headers['Accept-Encoding'] = 'identity' if username or password: LOG.warning('Passing username and password to Connector is ' @@ -115,7 +106,8 @@ class Connector(object): PUT, PATCH, etc... :param path: The sub-URI or absolute URL path to the resource. :param data: Optional JSON data. - :param headers: Optional dictionary of headers. + :param headers: Optional dictionary of headers. Use None value + to remove a default header. :param blocking: Whether to block for asynchronous operations. :param timeout: Max time in seconds to wait for blocking async call or for requests library to connect and read. If a custom @@ -138,12 +130,25 @@ class Connector(object): url = path if urlparse.urlparse(path).netloc else urlparse.urljoin( self._url, path) - headers = headers or {} + headers = (headers or {}).copy() lc_headers = [k.lower() for k in headers] if data is not None and 'content-type' not in lc_headers: headers['Content-Type'] = 'application/json' if 'odata-version' not in lc_headers: headers['OData-Version'] = '4.0' + # NOTE(TheJulia): Depending on the BMC, offering compression as an + # acceptable response changes the ETag behavior to offering an + # automatic "weak" ETag response, which is appropriate because the + # body content *may* not be a byte for byte match given compression. + # Overall, the value of compression is less than the value of concise + # interaction with the BMC. Setting to identity basically means + # "without modification or compression". By default, python-requests + # indicates responses can be compressed. + if 'accept-encoding' not in lc_headers: + headers['Accept-Encoding'] = 'identity' + # Allow removing default headers + headers = {k: v for k, v in headers.items() if v is not None} + # TODO(lucasagomes): We should mask the data to remove sensitive # information LOG.debug('HTTP request: %(method)s %(url)s; headers: %(headers)s; ' @@ -268,6 +273,21 @@ class Connector(object): **extra_session_req_kwargs) else: raise + except exceptions.NotAcceptableError as e: + # NOTE(dtantsur): some HPE Gen 10 Plus machines do not allow + # identity encoding when fetching registries. + if (method.lower() == 'get' + and headers.get('Accept-Encoding') == 'identity'): + LOG.warning('Server has indicated a NotAcceptable for %s, ' + 'retrying without identity encoding', e) + headers = dict(headers, **{'Accept-Encoding': None}) + return self._op( + method, path, data=data, headers=headers, + blocking=blocking, timeout=timeout, + server_side_retries_left=server_side_retries_left, + **extra_session_req_kwargs) + else: + raise if blocking and response.status_code == 202: if not response.headers.get('Location'): m = ('HTTP response for %(method)s request to %(url)s ' diff --git a/sushy/exceptions.py b/sushy/exceptions.py index bef3b619..74bb2962 100644 --- a/sushy/exceptions.py +++ b/sushy/exceptions.py @@ -160,6 +160,10 @@ class AccessError(HTTPError): pass +class NotAcceptableError(HTTPError): + pass + + class MissingXAuthToken(HTTPError): message = ('No X-Auth-Token returned from remote host when ' 'attempting to establish a session. Error: %(error)s') @@ -176,6 +180,8 @@ def raise_for_response(method, url, response): elif response.status_code in (http_client.UNAUTHORIZED, http_client.FORBIDDEN): raise AccessError(method, url, response) + elif response.status_code == http_client.NOT_ACCEPTABLE: + raise NotAcceptableError(method, url, response) elif response.status_code >= http_client.INTERNAL_SERVER_ERROR: raise ServerSideError(method, url, response) else: diff --git a/sushy/tests/unit/test_connector.py b/sushy/tests/unit/test_connector.py index 16a7c780..6755817e 100644 --- a/sushy/tests/unit/test_connector.py +++ b/sushy/tests/unit/test_connector.py @@ -168,18 +168,25 @@ class ConnectorOpTestCase(base.TestCase): server_side_retries=10, server_side_retries_delay=3) self.conn._auth = mock_auth self.data = {'fake': 'data'} - self.headers = {'X-Fake': 'header'} + self.headers = {'Accept-Encoding': 'identity', 'OData-Version': '4.0'} self.session = mock.Mock(spec=requests.Session) self.conn._session = self.session self.request = self.session.request self.request.return_value.status_code = http_client.OK def test_ok_get(self): - self.conn._op('GET', path='fake/path', headers=self.headers) + self.conn._op('GET', path='fake/path') self.request.assert_called_once_with( 'GET', 'http://foo.bar:1234/fake/path', headers=self.headers, json=None, verify=True, timeout=60) + def test_ok_get_with_headers(self): + self.conn._op('GET', path='fake/path', headers={'answer': '42'}) + self.request.assert_called_once_with( + 'GET', 'http://foo.bar:1234/fake/path', + headers=dict(self.headers, answer='42'), + json=None, verify=True, timeout=60) + def test_response_callback(self): mock_response_callback = mock.MagicMock() self.conn._response_callback = mock_response_callback @@ -188,34 +195,42 @@ class ConnectorOpTestCase(base.TestCase): self.assertEqual(1, mock_response_callback.call_count) def test_ok_get_url_redirect_false(self): - self.conn._op('GET', path='fake/path', headers=self.headers, - allow_redirects=False) + self.conn._op('GET', path='fake/path', allow_redirects=False) self.request.assert_called_once_with( 'GET', 'http://foo.bar:1234/fake/path', headers=self.headers, json=None, allow_redirects=False, verify=True, timeout=60) def test_ok_post(self): - self.conn._op('POST', path='fake/path', data=self.data.copy(), - headers=self.headers) + self.conn._op('POST', path='fake/path', data=self.data.copy()) self.request.assert_called_once_with( 'POST', 'http://foo.bar:1234/fake/path', - json=self.data, headers=self.headers, verify=True, timeout=60) + json=self.data, + headers=dict(self.headers, **{'Content-Type': 'application/json'}), + verify=True, timeout=60) + + def test_ok_post_with_headers(self): + self.conn._op('POST', path='fake/path', data=self.data.copy(), + headers={'answer': 42}) + self.request.assert_called_once_with( + 'POST', 'http://foo.bar:1234/fake/path', + json=self.data, + headers=dict(self.headers, **{'Content-Type': 'application/json', + 'answer': 42}), + verify=True, timeout=60) def test_ok_put(self): - self.conn._op('PUT', path='fake/path', data=self.data.copy(), - headers=self.headers) + self.conn._op('PUT', path='fake/path', data=self.data.copy()) self.request.assert_called_once_with( 'PUT', 'http://foo.bar:1234/fake/path', - json=self.data, headers=self.headers, verify=True, timeout=60) + headers=dict(self.headers, **{'Content-Type': 'application/json'}), + json=self.data, verify=True, timeout=60) def test_ok_delete(self): - expected_headers = self.headers.copy() - expected_headers['OData-Version'] = '4.0' - self.conn._op('DELETE', path='fake/path', headers=self.headers.copy()) + self.conn._op('DELETE', path='fake/path') self.request.assert_called_once_with( 'DELETE', 'http://foo.bar:1234/fake/path', - headers=expected_headers, json=None, verify=True, timeout=60) + headers=self.headers, json=None, verify=True, timeout=60) def test_ok_post_with_session(self): self.conn._session.headers = {} @@ -242,23 +257,24 @@ class ConnectorOpTestCase(base.TestCase): 'GET', 'http://foo.bar:1234' + path, headers=expected_headers, json=None, verify=True, timeout=60) - def test_odata_version_header_redfish_no_headers(self): - path = '/redfish/v1/bar' - expected_headers = {'OData-Version': '4.0'} - self.conn._op('GET', path=path) - self.request.assert_called_once_with( - 'GET', 'http://foo.bar:1234' + path, - headers=expected_headers, json=None, verify=True, timeout=60) - def test_odata_version_header_redfish_existing_header(self): path = '/redfish/v1/foo' headers = {'OData-Version': '3.0'} - expected_headers = dict(headers) + expected_headers = dict(self.headers, **headers) self.conn._op('GET', path=path, headers=headers) self.request.assert_called_once_with( 'GET', 'http://foo.bar:1234' + path, headers=expected_headers, json=None, verify=True, timeout=60) + def test_remove_header_accept_encoding(self): + path = '/redfish/v1/foo' + headers = {'Accept-Encoding': None} + self.headers.pop('Accept-Encoding') + self.conn._op('GET', path=path, headers=headers) + self.request.assert_called_once_with( + 'GET', 'http://foo.bar:1234' + path, + headers=self.headers, json=None, verify=True, timeout=60) + def test_timed_out_session_unable_to_create_session(self): self.conn._auth.can_refresh_session.return_value = False self.session.auth = None @@ -522,6 +538,29 @@ class ConnectorOpTestCase(base.TestCase): self.assertEqual(0, mock_sleep.call_count) self.assertEqual(1, self.request.call_count) + def test_op_retry_without_identity(self): + self.request.side_effect = [ + mock.Mock(status_code=http_client.NOT_ACCEPTABLE), + mock.Mock(status_code=http_client.OK), + ] + self.conn._op('GET', 'http://foo.bar') + + self.assertEqual(2, self.request.call_count) + headers_no_accept = self.headers.copy() + headers_no_accept.pop('Accept-Encoding') + self.request.assert_has_calls([ + mock.call('GET', 'http://foo.bar', headers=self.headers, + json=None, verify=True, timeout=60), + mock.call('GET', 'http://foo.bar', headers=headers_no_accept, + json=None, verify=True, timeout=60), + ]) + + def test_op_retry_without_identity_fails(self): + self.request.return_value.status_code = http_client.NOT_ACCEPTABLE + self.assertRaises(exceptions.NotAcceptableError, self.conn._op, + 'GET', 'http://foo.bar') + self.assertEqual(2, self.request.call_count) + def test_access_error(self): self.conn._auth = None @@ -715,8 +754,7 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header', - 'If-Match': '"3d7b8a7360bf2941d"'}, + headers=dict(self.headers, **{'If-Match': '"3d7b8a7360bf2941d"'}), blocking=False, timeout=60) @@ -738,8 +776,8 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header', - 'If-Match': 'W/"3d7b8a7360bf2941d"'}, + headers=dict(self.headers, + **{'If-Match': 'W/"3d7b8a7360bf2941d"'}), blocking=False, timeout=60) @@ -759,7 +797,7 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header'}, + headers=self.headers, blocking=False, timeout=60) @@ -787,8 +825,7 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header', - 'If-Match': '"3d7b8a7360bf2941d"'}, + headers=dict(self.headers, **{'If-Match': '"3d7b8a7360bf2941d"'}), blocking=False, timeout=60) @@ -818,7 +855,7 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header'}, + headers=self.headers, blocking=False, timeout=60) @@ -846,7 +883,7 @@ class ConnectorOpTestCase(base.TestCase): '/redfish/v1/Systems/1', data={'Boot': {'BootSourceOverrideTarget': 'Cd', 'BootSourceOverrideEnabled': 'Once'}}, - headers={'X-Fake': 'header'}, + headers=self.headers, blocking=False, timeout=60)