Merge "Wire in header microversion into negotiation"
This commit is contained in:
Zuul
Gerrit Code Review
commit
177162019b
ironicclient
@ -103,8 +103,8 @@ class VersionNegotiationMixin(object):
|
||||
|
||||
Assumption: Called after receiving a 406 error when doing a request.
|
||||
|
||||
param conn: A connection object
|
||||
param resp: The response object from http request
|
||||
:param conn: A connection object
|
||||
:param resp: The response object from http request
|
||||
"""
|
||||
def _query_server(conn):
|
||||
if (self.os_ironic_api_version and
|
||||
@ -116,6 +116,31 @@ class VersionNegotiationMixin(object):
|
||||
base_version = API_VERSION
|
||||
return self._make_simple_request(conn, 'GET', base_version)
|
||||
|
||||
version_overridden = False
|
||||
|
||||
if (resp and hasattr(resp, 'request') and
|
||||
hasattr(resp.request, 'headers')):
|
||||
orig_hdr = resp.request.headers
|
||||
# Get the version of the client's last request and fallback
|
||||
# to the default for things like unit tests to not cause
|
||||
# migraines.
|
||||
req_api_ver = orig_hdr.get('X-OpenStack-Ironic-API-Version',
|
||||
self.os_ironic_api_version)
|
||||
else:
|
||||
req_api_ver = self.os_ironic_api_version
|
||||
if (resp and req_api_ver != self.os_ironic_api_version and
|
||||
self.api_version_select_state == 'negotiated'):
|
||||
# If we have a non-standard api version on the request,
|
||||
# but we think we've negotiated, then the call was overriden.
|
||||
# We should report the error with the called version
|
||||
requested_version = req_api_ver
|
||||
# And then we shouldn't save the newly negotiated
|
||||
# version of this negotiation because we have been
|
||||
# overridden a request.
|
||||
version_overridden = True
|
||||
else:
|
||||
requested_version = self.os_ironic_api_version
|
||||
|
||||
if not resp:
|
||||
resp = _query_server(conn)
|
||||
if self.api_version_select_state not in API_VERSION_SELECTED_STATES:
|
||||
@ -147,34 +172,36 @@ class VersionNegotiationMixin(object):
|
||||
# be supported by the requested version.
|
||||
# TODO(TheJulia): We should break this method into several parts,
|
||||
# such as a sanity check/error method.
|
||||
if (self.api_version_select_state == 'user' and
|
||||
not self._must_negotiate_version()):
|
||||
if ((self.api_version_select_state == 'user' and
|
||||
not self._must_negotiate_version()) or
|
||||
(self.api_version_select_state == 'negotiated' and
|
||||
version_overridden)):
|
||||
raise exc.UnsupportedVersion(textwrap.fill(
|
||||
_("Requested API version %(req)s is not supported by the "
|
||||
"server, client, or the requested operation is not "
|
||||
"supported by the requested version."
|
||||
"supported by the requested version. "
|
||||
"Supported version range is %(min)s to "
|
||||
"%(max)s")
|
||||
% {'req': self.os_ironic_api_version,
|
||||
% {'req': requested_version,
|
||||
'min': min_ver, 'max': max_ver}))
|
||||
if self.api_version_select_state == 'negotiated':
|
||||
if (self.api_version_select_state == 'negotiated'):
|
||||
raise exc.UnsupportedVersion(textwrap.fill(
|
||||
_("No API version was specified and the requested operation "
|
||||
_("No API version was specified or the requested operation "
|
||||
"was not supported by the client's negotiated API version "
|
||||
"%(req)s. Supported version range is: %(min)s to %(max)s")
|
||||
% {'req': self.os_ironic_api_version,
|
||||
% {'req': requested_version,
|
||||
'min': min_ver, 'max': max_ver}))
|
||||
|
||||
if isinstance(self.os_ironic_api_version, six.string_types):
|
||||
if self.os_ironic_api_version == 'latest':
|
||||
if isinstance(requested_version, six.string_types):
|
||||
if requested_version == 'latest':
|
||||
negotiated_ver = max_ver
|
||||
else:
|
||||
negotiated_ver = str(
|
||||
min(StrictVersion(self.os_ironic_api_version),
|
||||
min(StrictVersion(requested_version),
|
||||
StrictVersion(max_ver)))
|
||||
|
||||
elif isinstance(self.os_ironic_api_version, list):
|
||||
if 'latest' in self.os_ironic_api_version:
|
||||
elif isinstance(requested_version, list):
|
||||
if 'latest' in requested_version:
|
||||
raise ValueError(textwrap.fill(
|
||||
_("The 'latest' API version can not be requested "
|
||||
"in a list of versions. Please explicitly request "
|
||||
@ -183,7 +210,7 @@ class VersionNegotiationMixin(object):
|
||||
% {'min': min_ver, 'max': max_ver}))
|
||||
|
||||
versions = []
|
||||
for version in self.os_ironic_api_version:
|
||||
for version in requested_version:
|
||||
if min_ver <= StrictVersion(version) <= max_ver:
|
||||
versions.append(StrictVersion(version))
|
||||
if versions:
|
||||
@ -194,7 +221,7 @@ class VersionNegotiationMixin(object):
|
||||
"operation was not supported by the client's "
|
||||
"requested API version %(req)s. Supported "
|
||||
"version range is: %(min)s to %(max)s")
|
||||
% {'req': self.os_ironic_api_version,
|
||||
% {'req': requested_version,
|
||||
'min': min_ver, 'max': max_ver}))
|
||||
|
||||
else:
|
||||
@ -202,7 +229,7 @@ class VersionNegotiationMixin(object):
|
||||
_("Requested API version %(req)s type is unsupported. "
|
||||
"Valid types are Strings such as '1.1', 'latest' "
|
||||
"or a list of string values representing API versions.")
|
||||
% {'req': self.os_ironic_api_version}))
|
||||
% {'req': requested_version}))
|
||||
|
||||
if StrictVersion(negotiated_ver) < StrictVersion(min_ver):
|
||||
negotiated_ver = min_ver
|
||||
@ -388,7 +415,6 @@ class HTTPClient(VersionNegotiationMixin):
|
||||
# the self.negotiate_version() call if negotiation occurs.
|
||||
if self.os_ironic_api_version and self._must_negotiate_version():
|
||||
self.negotiate_version(self.session, None)
|
||||
|
||||
# Copy the kwargs so we can reuse the original in case of redirects
|
||||
kwargs['headers'] = copy.deepcopy(kwargs.get('headers', {}))
|
||||
kwargs['headers'].setdefault('User-Agent', USER_AGENT)
|
||||
|
||||
@ -318,6 +318,29 @@ class VersionNegotiationMixinTest(utils.BaseTestCase):
|
||||
self.assertEqual(2, mock_pvh.call_count)
|
||||
self.assertEqual(0, mock_save_data.call_count)
|
||||
|
||||
@mock.patch.object(filecache, 'save_data', autospec=True)
|
||||
@mock.patch.object(http.VersionNegotiationMixin, '_make_simple_request',
|
||||
autospec=True)
|
||||
@mock.patch.object(http.VersionNegotiationMixin, '_parse_version_headers',
|
||||
autospec=True)
|
||||
def test_negotiate_version_explicit_version_request(
|
||||
self, mock_pvh, mock_msr, mock_save_data):
|
||||
mock_pvh.side_effect = iter([(None, None), ('1.1', '1.99')])
|
||||
mock_conn = mock.MagicMock()
|
||||
self.test_object.api_version_select_state = 'negotiated'
|
||||
self.test_object.os_ironic_api_version = '1.30'
|
||||
req_header = {'X-OpenStack-Ironic-API-Version': '1.29'}
|
||||
response = utils.FakeResponse(
|
||||
{}, status=http_client.NOT_ACCEPTABLE,
|
||||
request_headers=req_header)
|
||||
self.assertRaisesRegexp(exc.UnsupportedVersion,
|
||||
".*is not supported by the server.*",
|
||||
self.test_object.negotiate_version,
|
||||
mock_conn, response)
|
||||
self.assertTrue(mock_msr.called)
|
||||
self.assertEqual(2, mock_pvh.call_count)
|
||||
self.assertFalse(mock_save_data.called)
|
||||
|
||||
def test_get_server(self):
|
||||
host = 'ironic-host'
|
||||
port = '6385'
|
||||
@ -517,6 +540,24 @@ class HttpClientTest(utils.BaseTestCase):
|
||||
self.assertEqual(http_client.OK, response.status_code)
|
||||
self.assertEqual(1, mock_negotiate.call_count)
|
||||
|
||||
@mock.patch.object(requests.Session, 'request', autospec=True)
|
||||
@mock.patch.object(http.VersionNegotiationMixin, 'negotiate_version',
|
||||
autospec=False)
|
||||
def test__http_request_explicit_version(self, mock_negotiate,
|
||||
mock_session):
|
||||
headers = {'User-Agent': 'python-ironicclient',
|
||||
'X-OpenStack-Ironic-API-Version': '1.28'}
|
||||
kwargs = {'os_ironic_api_version': '1.30',
|
||||
'api_version_select_state': 'negotiated'}
|
||||
mock_session.return_value = utils.mockSessionResponse(
|
||||
{}, status_code=http_client.NO_CONTENT, version=1)
|
||||
client = http.HTTPClient('http://localhost/', **kwargs)
|
||||
response, body_iter = client._http_request('/v1/resources', 'GET',
|
||||
headers=headers)
|
||||
mock_session.assert_called_once_with(mock.ANY, 'GET',
|
||||
'http://localhost/v1/resources',
|
||||
headers=headers)
|
||||
|
||||
@mock.patch.object(http.LOG, 'debug', autospec=True)
|
||||
def test_log_curl_request_mask_password(self, mock_log):
|
||||
client = http.HTTPClient('http://localhost/')
|
||||
|
||||
@ -79,7 +79,7 @@ class FakeConnection(object):
|
||||
|
||||
class FakeResponse(object):
|
||||
def __init__(self, headers, body=None, version=None, status=None,
|
||||
reason=None):
|
||||
reason=None, request_headers={}):
|
||||
"""Fake object to help testing.
|
||||
|
||||
:param headers: dict representing HTTP response headers
|
||||
@ -91,6 +91,8 @@ class FakeResponse(object):
|
||||
self.raw.version = version
|
||||
self.status_code = status
|
||||
self.reason = reason
|
||||
self.request = mock.Mock()
|
||||
self.request.headers = request_headers
|
||||
|
||||
def getheaders(self):
|
||||
return copy.deepcopy(self.headers).items()
|
||||
@ -102,21 +104,26 @@ class FakeResponse(object):
|
||||
return self.body.read(amt)
|
||||
|
||||
def __repr__(self):
|
||||
return ("FakeResponse(%s, body=%s, version=%s, status=%s, reason=%s)" %
|
||||
(self.headers, self.body, self.version, self.status,
|
||||
self.reason))
|
||||
return ("FakeResponse(%s, body=%s, version=%s, status=%s, reason=%s, "
|
||||
"request_headers=%s)" %
|
||||
(self.headers, self.body, self.raw.version, self.status_code,
|
||||
self.reason, self.request.headers))
|
||||
|
||||
|
||||
def mockSessionResponse(headers, content=None, status_code=None, version=None):
|
||||
def mockSessionResponse(headers, content=None, status_code=None, version=None,
|
||||
request_headers={}):
|
||||
raw = mock.Mock()
|
||||
raw.version = version
|
||||
request = mock.Mock()
|
||||
request.headers = request_headers
|
||||
response = mock.Mock(spec=requests.Response,
|
||||
headers=headers,
|
||||
content=content,
|
||||
status_code=status_code,
|
||||
raw=raw,
|
||||
reason='',
|
||||
encoding='UTF-8')
|
||||
encoding='UTF-8',
|
||||
request=request)
|
||||
response.text = content
|
||||
|
||||
return response
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user