From 9e03afa242ff37e4217570540bfdb65145e4cd61 Mon Sep 17 00:00:00 2001 From: yuntongjin Date: Tue, 19 May 2015 16:15:15 +0800 Subject: [PATCH] Add support for API microversions This patch adds support in the REST API to accept a new X-OpenStack-Magnum-API-Version header. This header is also returned to clients as a hint to improve client response handling. Additionally, new Minimum-Version and Maximum-Version headers are also now returned, so as to inform clients of the supported range. The requested version is stashed in the pecan.request.version object. For reference as to why this approach was chosen, see the Nova spec on microversioning: http://specs.openstack.org/openstack/nova-specs/specs/kilo/approved/api-microversions.html Ironic: https://review.openstack.org/#/c/150821 Implements: blueprint api-microversions https://blueprints.launchpad.net/magnum/+spec/api-microversions Co-Authored-By: ShaoHe Feng Change-Id: I795f83cd829098e3d1f08d89ea33a8d986797d9f --- magnum/api/controllers/base.py | 75 +++++++++++++++++++ magnum/api/controllers/v1/__init__.py | 67 +++++++++++++++++ magnum/common/exception.py | 7 ++ magnum/tests/fakes.py | 2 + magnum/tests/unit/api/base.py | 9 ++- .../tests/unit/api/controllers/test_root.py | 58 ++++++++++++++ 6 files changed, 216 insertions(+), 2 deletions(-) diff --git a/magnum/api/controllers/base.py b/magnum/api/controllers/base.py index a132a339f0..ecc69a4de7 100644 --- a/magnum/api/controllers/base.py +++ b/magnum/api/controllers/base.py @@ -14,9 +14,13 @@ import datetime +from webob import exc import wsme from wsme import types as wtypes +from magnum.common.exception import NotAcceptable +from magnum.i18n import _ + class APIBase(wtypes.Base): @@ -45,3 +49,74 @@ class APIBase(wtypes.Base): for k in self.as_dict(): if k not in except_list: setattr(self, k, wsme.Unset) + + +class Version(object): + """API Version object.""" + + string = 'X-OpenStack-Magnum-API-Version' + """HTTP Header string carrying the requested version""" + + min_string = 'X-OpenStack-Magnum-API-Minimum-Version' + """HTTP response header""" + + max_string = 'X-OpenStack-Magnum-API-Maximum-Version' + """HTTP response header""" + + def __init__(self, headers, default_version, latest_version): + """Create an API Version object from the supplied headers. + + :param headers: webob headers + :param default_version: version to use if not specified in headers + :param latest_version: version to use if latest is requested + :raises: webob.HTTPNotAcceptable + """ + (self.major, self.minor) = Version.parse_headers(headers, + default_version, + latest_version) + + def __repr__(self): + return '%s.%s' % (self.major, self.minor) + + @staticmethod + def parse_headers(headers, default_version, latest_version): + """Determine the API version requested based on the headers supplied. + + :param headers: webob headers + :param default_version: version to use if not specified in headers + :param latest_version: version to use if latest is requested + :returns: a tupe of (major, minor) version numbers + :raises: webob.HTTPNotAcceptable + """ + version_str = headers.get(Version.string, default_version) + + if version_str.lower() == 'latest': + parse_str = latest_version + else: + parse_str = version_str + + try: + version = tuple(int(i) for i in parse_str.split('.')) + except ValueError: + version = () + + if len(version) != 2: + raise exc.HTTPNotAcceptable(_( + "Invalid value for %s header") % Version.string) + return version + + def __lt__(a, b): + if a.major != b.major: + raise NotAcceptable() + + if (a.major == b.major and a.minor < b.minor): + return True + return False + + def __gt__(a, b): + if a.major != b.major: + raise NotAcceptable() + + if (a.major == b.major and a.minor > b.minor): + return True + return False diff --git a/magnum/api/controllers/v1/__init__.py b/magnum/api/controllers/v1/__init__.py index de9f84cd5b..9d23b1650a 100644 --- a/magnum/api/controllers/v1/__init__.py +++ b/magnum/api/controllers/v1/__init__.py @@ -22,10 +22,12 @@ import datetime import pecan from pecan import rest +from webob import exc import wsme from wsme import types as wtypes import wsmeext.pecan as wsme_pecan +from magnum.api.controllers import base as controllers_base from magnum.api.controllers import link from magnum.api.controllers.v1 import bay from magnum.api.controllers.v1 import baymodel @@ -34,6 +36,32 @@ from magnum.api.controllers.v1 import node from magnum.api.controllers.v1 import pod from magnum.api.controllers.v1 import replicationcontroller as rc from magnum.api.controllers.v1 import service +from magnum.i18n import _ + + +BASE_VERSION = 1 + +# NOTE(yuntong): v1.0 is reserved to indicate Kilo's API, but is not presently +# supported by the API service. All changes between Kilo and the +# point where we added microversioning are considered backwards- +# compatible, but are not specifically discoverable at this time. +# +# The v1.1 version indicates this "initial" version as being +# different from Kilo (v1.0), and includes the following changes: +# + +# v1.1: API at the point in time when microversioning support was added +MIN_VER_STR = '1.1' + +# v1.1: Add API changelog here +MAX_VER_STR = '1.1' + + +MIN_VER = controllers_base.Version( + {controllers_base.Version.string: MIN_VER_STR}, MIN_VER_STR, MAX_VER_STR) +MAX_VER = controllers_base.Version( + {controllers_base.Version.string: MAX_VER_STR}, + MIN_VER_STR, MAX_VER_STR) class APIBase(wtypes.Base): @@ -176,4 +204,43 @@ class Controller(rest.RestController): # the request object to make the links. return V1.convert() + def _check_version(self, version, headers=None): + if headers is None: + headers = {} + # ensure that major version in the URL matches the header + if version.major != BASE_VERSION: + raise exc.HTTPNotAcceptable(_( + "Mutually exclusive versions requested. Version %(ver)s " + "requested but not supported by this service." + "The supported version range is: " + "[%(min)s, %(max)s].") % {'ver': version, + 'min': MIN_VER_STR, + 'max': MAX_VER_STR}, + headers=headers) + # ensure the minor version is within the supported range + if version < MIN_VER or version > MAX_VER: + raise exc.HTTPNotAcceptable(_( + "Version %(ver)s was requested but the minor version is not " + "supported by this service. The supported version range is: " + "[%(min)s, %(max)s].") % {'ver': version, 'min': MIN_VER_STR, + 'max': MAX_VER_STR}, headers=headers) + + @pecan.expose() + def _route(self, args): + version = controllers_base.Version( + pecan.request.headers, MIN_VER_STR, MAX_VER_STR) + + # Always set the min and max headers + pecan.response.headers[ + controllers_base.Version.min_string] = MIN_VER_STR + pecan.response.headers[ + controllers_base.Version.max_string] = MAX_VER_STR + + # assert that requested version is supported + self._check_version(version, pecan.response.headers) + pecan.response.headers[controllers_base.Version.string] = str(version) + pecan.request.version = version + + return super(Controller, self)._route(args) + __all__ = (Controller) diff --git a/magnum/common/exception.py b/magnum/common/exception.py index 33d07d0d62..bab4784af7 100644 --- a/magnum/common/exception.py +++ b/magnum/common/exception.py @@ -334,6 +334,13 @@ class NotAuthorized(MagnumException): code = 403 +class NotAcceptable(MagnumException): + # TODO(yuntongjin): We need to set response headers + # in the API for this exception + message = _("Request not acceptable.") + code = 404 + + class OperationNotPermitted(NotAuthorized): message = _("Operation not permitted.") diff --git a/magnum/tests/fakes.py b/magnum/tests/fakes.py index b9615a826e..4f53011456 100644 --- a/magnum/tests/fakes.py +++ b/magnum/tests/fakes.py @@ -27,6 +27,7 @@ fakeAuthTokenHeaders = {'X-User-Id': u'773a902f022949619b5c2f32cd89d419', 'X-User-Domain-Name': 'domain', 'X-Project-Domain-Id': 'project_domain_id', 'X-User-Domain-Id': 'user_domain_id', + 'X-OpenStack-Magnum-API-Version': '1.0' } @@ -42,6 +43,7 @@ class FakePecanRequest(mock.Mock): self.path = '/v1/services' self.headers = fakeAuthTokenHeaders self.environ = {} + self.version = (1, 0) def __setitem__(self, index, value): setattr(self, index, value) diff --git a/magnum/tests/unit/api/base.py b/magnum/tests/unit/api/base.py index 8bbfd47040..ff1be21cba 100644 --- a/magnum/tests/unit/api/base.py +++ b/magnum/tests/unit/api/base.py @@ -15,11 +15,12 @@ """Base classes for API tests.""" # NOTE: Ported from ceilometer/tests/api.py (subsequently moved to -# ceilometer/tests/api/__init__.py). This should be oslo'ified: -# https://bugs.launchpad.net/ironic/+bug/1255115. +# ceilometer/tests/api/__init__.py). This should be oslo'ified: +# https://bugs.launchpad.net/ironic/+bug/1255115. # NOTE(deva): import auth_token so we can override a config option from keystonemiddleware import auth_token # noqa +import mock from oslo_config import cfg import pecan import pecan.testing @@ -54,6 +55,10 @@ class FunctionalTest(base.DbTestCase): self.addCleanup(reset_pecan) + p = mock.patch('magnum.api.controllers.v1.Controller._check_version') + self._check_version = p.start() + self.addCleanup(p.stop) + def _make_app(self, enable_acl=False): # Determine where we are so we can set up paths in the config root_dir = self.path_get() diff --git a/magnum/tests/unit/api/controllers/test_root.py b/magnum/tests/unit/api/controllers/test_root.py index a714d99ea0..623084b1e0 100644 --- a/magnum/tests/unit/api/controllers/test_root.py +++ b/magnum/tests/unit/api/controllers/test_root.py @@ -9,7 +9,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +import mock +from webob import exc as webob_exc + +from magnum.api.controllers import v1 as v1_api from magnum import tests +from magnum.tests import base as test_base +from magnum.tests.unit.api import base as api_base class TestRootController(tests.FunctionalTest): @@ -71,3 +78,54 @@ class TestRootController(tests.FunctionalTest): def test_get_not_found(self): response = self.app.get('/a/bogus/url', expect_errors=True) assert response.status_int == 404 + + +class TestV1Routing(api_base.FunctionalTest): + def setUp(self): + super(TestV1Routing, self).setUp() + + def test_route_checks_version(self): + self.get_json('/') + self._check_version.assert_called_once_with(mock.ANY, + mock.ANY) + + +class TestCheckVersions(test_base.TestCase): + + def setUp(self): + super(TestCheckVersions, self).setUp() + + class ver(object): + major = None + minor = None + + self.version = ver() + + def test_check_version_invalid_major_version(self): + self.version.major = v1_api.BASE_VERSION + 1 + self.version.minor = v1_api.MIN_VER.minor + self.assertRaises(webob_exc.HTTPNotAcceptable, + v1_api.Controller()._check_version, + self.version) + + def test_check_version_too_low(self): + self.version.major = v1_api.BASE_VERSION + self.version.minor = v1_api.MIN_VER.minor - 1 + self.assertRaises(webob_exc.HTTPNotAcceptable, + v1_api.Controller()._check_version, + self.version) + + def test_check_version_too_high(self): + self.version.major = v1_api.BASE_VERSION + self.version.minor = v1_api.MAX_VER.minor + 1 + e = self.assertRaises(webob_exc.HTTPNotAcceptable, + v1_api.Controller()._check_version, + self.version, {'fake-headers': + v1_api.MAX_VER.minor}) + + self.assertEqual(v1_api.MAX_VER.minor, e.headers['fake-headers']) + + def test_check_version_ok(self): + self.version.major = v1_api.BASE_VERSION + self.version.minor = v1_api.MIN_VER.minor + v1_api.Controller()._check_version(self.version)