diff --git a/doc/source/devref/experimental_apis.rst b/doc/source/devref/experimental_apis.rst new file mode 100644 index 0000000000..74b8b0885d --- /dev/null +++ b/doc/source/devref/experimental_apis.rst @@ -0,0 +1,75 @@ +Experimental APIs +================= + +Background +---------- + +Manila uses API microversions to allow natural evolution of its REST APIs +over time. But microversions alone cannot solve the question of how to +ship APIs that are experimental in nature, are expected to change at any +time, and could even be removed entirely without a typical deprecation +period. + +In conjunction with microversions, Manila has added a facility for marking +individual REST APIs as experimental. To call an experimental API, clients +must include a specific HTTP header, ``X-OpenStack-Manila-API-Experimental``, +with a value of ``True``. If a user calls an experimental API without +including the experimental header, the server would respond with ``HTTP/404``. +This forces the client to acknowledge the experimental status of the API and +prevents anyone from building an application around a Manila feature without +realizing the feature could change significantly or even disappear. + +On the other hand, if a request is made to a non-experimental Manila API with +``X-OpenStack-Manila-API-Experimental: True``, the server would respond as if +the header had not been included. This is a convenience mechanism, as it +allows the client to specify both the requested API version as well as the +experimental header (if desired) in one place instead of having to set the +headers separately for each API call (although that would be fine, too). + +When do I need to set an API experimental? +------------------------------------------ + +An API should be marked as experimental if any of the following is true: + +- the API is not yet considered a stable, core API + +- the API is expected to change in later releases + +- the API could be removed altogether if a feature is redesigned + +- the API controls a feature that could change or be removed + +When do I need to remove the experimental annotation from an API? +----------------------------------------------------------------- + +When the community is satisfied that an experimental feature and its APIs +have had sufficient time to gather and incorporate user feedback to consider +it stable, which could be one or more OpenStack release cycles, any relevant +APIs must be re-released with a microversion bump and without the experimental +flag. The maturation period can vary between features, but experimental is NOT +a stable state, and an experimental feature should not be left in that state +any longer than necessary. + +Because experimental APIs have no conventional deprecation period, the Manila +core team may optionally choose to remove any experimental versions of an API +at the same time that a microversioned stable version is added. + +In Code +------- + +The ``@api_version`` decorator defined in ``manila/api/openstack/wsgi.py``, +which is used for specifying API versions on top-level Controller methods, +also allows for tagging an API as experimental. For example: + +In the controller class:: + + @wsgi.Controller.api_version("2.4", experimental=True) + def my_api_method(self, req, id): + .... + +This method would only be available if the caller had specified an +``X-OpenStack-Manila-API-Version`` of >= ``2.4``. and had also included +``X-OpenStack-Manila-API-Experimental: True``. If they had specified a +lower version (or not specified it and received a lower default version), +or if they had failed to include the experimental header, the server would +respond with ``HTTP/404``. diff --git a/doc/source/devref/index.rst b/doc/source/devref/index.rst index 79f04f39ee..d2a7592597 100644 --- a/doc/source/devref/index.rst +++ b/doc/source/devref/index.rst @@ -59,6 +59,7 @@ API Reference api api_microversion_dev api_microversion_history + experimental_apis Module Reference ---------------- diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 707fe0e841..d5d703c726 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -16,7 +16,9 @@ import re +from manila.api.openstack import versioned_method from manila import exception +from manila.i18n import _ from manila import utils # Define the minimum and maximum version of the API across all of the @@ -76,31 +78,55 @@ class APIVersionRequest(utils.ComparableMixin): def __init__(self, version_string=None): """Create an API version request object.""" - self.ver_major = None - self.ver_minor = None + self._ver_major = None + self._ver_minor = None + self._experimental = False if version_string is not None: match = re.match(r"^([1-9]\d*)\.([1-9]\d*|0)$", version_string) if match: - self.ver_major = int(match.group(1)) - self.ver_minor = int(match.group(2)) + self._ver_major = int(match.group(1)) + self._ver_minor = int(match.group(2)) else: raise exception.InvalidAPIVersionString(version=version_string) def __str__(self): """Debug/Logging representation of object.""" return ("API Version Request Major: %(major)s, Minor: %(minor)s" - % {'major': self.ver_major, 'minor': self.ver_minor}) + % {'major': self._ver_major, 'minor': self._ver_minor}) def is_null(self): - return self.ver_major is None and self.ver_minor is None + return self._ver_major is None and self._ver_minor is None def _cmpkey(self): """Return the value used by ComparableMixin for rich comparisons.""" - return self.ver_major, self.ver_minor + return self._ver_major, self._ver_minor - def matches(self, min_version, max_version): + @property + def experimental(self): + return self._experimental + + @experimental.setter + def experimental(self, value): + if type(value) != bool: + msg = _('The experimental property must be a bool value.') + raise exception.InvalidParameterValue(err=msg) + self._experimental = value + + def matches_versioned_method(self, method): + """Compares this version to that of a versioned method.""" + + if type(method) != versioned_method.VersionedMethod: + msg = _('An API version request must be compared ' + 'to a VersionedMethod object.') + raise exception.InvalidParameterValue(err=msg) + + return self.matches(method.start_version, + method.end_version, + method.experimental) + + def matches(self, min_version, max_version, experimental=False): """Compares this version to the specified min/max range. Returns whether the version object represents a version @@ -113,11 +139,17 @@ class APIVersionRequest(utils.ComparableMixin): :param min_version: Minimum acceptable version. :param max_version: Maximum acceptable version. + :param experimental: Whether to match experimental APIs. :returns: boolean """ if self.is_null(): raise ValueError + # NOTE(cknight): An experimental request should still match a + # non-experimental API, so the experimental check isn't just + # looking for equality. + if not self.experimental and experimental: + return False if max_version.is_null() and min_version.is_null(): return True elif max_version.is_null(): @@ -136,5 +168,5 @@ class APIVersionRequest(utils.ComparableMixin): if self.is_null(): raise ValueError return ("%(major)s.%(minor)s" % - {'major': self.ver_major, 'minor': self.ver_minor}) + {'major': self._ver_major, 'minor': self._ver_minor}) diff --git a/manila/api/openstack/versioned_method.py b/manila/api/openstack/versioned_method.py index 178bc408f0..ed096bbbf9 100644 --- a/manila/api/openstack/versioned_method.py +++ b/manila/api/openstack/versioned_method.py @@ -19,7 +19,7 @@ from manila import utils class VersionedMethod(utils.ComparableMixin): - def __init__(self, name, start_version, end_version, func): + def __init__(self, name, start_version, end_version, experimental, func): """Versioning information for a single method. Minimum and maximums are inclusive. @@ -27,11 +27,13 @@ class VersionedMethod(utils.ComparableMixin): :param name: Name of the method :param start_version: Minimum acceptable version :param end_version: Maximum acceptable_version + :param experimental: True if method is experimental :param func: Method to call """ self.name = name self.start_version = start_version self.end_version = end_version + self.experimental = experimental self.func = func def __str__(self): diff --git a/manila/api/openstack/wsgi.py b/manila/api/openstack/wsgi.py index e368226a47..cc5401dbb3 100644 --- a/manila/api/openstack/wsgi.py +++ b/manila/api/openstack/wsgi.py @@ -49,6 +49,7 @@ VER_METHOD_ATTR = 'versioned_methods' # Name of header used by clients to request a specific version # of the REST API API_VERSION_REQUEST_HEADER = 'X-OpenStack-Manila-API-Version' +EXPERIMENTAL_API_REQUEST_HEADER = 'X-OpenStack-Manila-API-Experimental' class Request(webob.Request): @@ -231,6 +232,11 @@ class Request(webob.Request): self.api_version_request = api_version.APIVersionRequest( api_version.DEFAULT_API_VERSION) + # Check if experimental API was requested + if EXPERIMENTAL_API_REQUEST_HEADER in self.headers: + self.api_version_request.experimental = strutils.bool_from_string( + self.headers[EXPERIMENTAL_API_REQUEST_HEADER]) + class ActionDispatcher(object): """Maps method name to local methods through action name.""" @@ -849,6 +855,9 @@ class Resource(wsgi.Application): if not request.api_version_request.is_null(): response.headers[API_VERSION_REQUEST_HEADER] = ( request.api_version_request.get_string()) + if request.api_version_request.experimental: + response.headers[EXPERIMENTAL_API_REQUEST_HEADER] = ( + request.api_version_request.experimental) response.headers['Vary'] = API_VERSION_REQUEST_HEADER return response @@ -1017,13 +1026,13 @@ class Controller(object): # object. The version for the request is attached to the # request object if len(args) == 0: - ver = kwargs['req'].api_version_request + version_request = kwargs['req'].api_version_request else: - ver = args[0].api_version_request + version_request = args[0].api_version_request func_list = self.versioned_methods[key] for func in func_list: - if ver.matches(func.start_version, func.end_version): + if version_request.matches_versioned_method(func): # Update the version_select wrapper function so # other decorator attributes like wsgi.response # are still respected. @@ -1031,7 +1040,8 @@ class Controller(object): return func.func(self, *args, **kwargs) # No version match - raise exception.VersionNotFoundForAPIMethod(version=ver) + raise exception.VersionNotFoundForAPIMethod( + version=version_request) try: version_meth_dict = object.__getattribute__(self, VER_METHOD_ATTR) @@ -1048,7 +1058,7 @@ class Controller(object): # NOTE(cyeoh): This decorator MUST appear first (the outermost # decorator) on an API method for it to work correctly @classmethod - def api_version(cls, min_ver, max_ver=None): + def api_version(cls, min_ver, max_ver=None, experimental=False): """Decorator for versioning API methods. Add the decorator to any method which takes a request object @@ -1057,6 +1067,8 @@ class Controller(object): :param min_ver: string representing minimum version :param max_ver: optional string representing maximum version + :param experimental: flag indicating an API is experimental and is + subject to change or removal at any time """ def decorator(f): @@ -1069,7 +1081,7 @@ class Controller(object): # Add to list of versioned methods registered func_name = f.__name__ new_func = versioned_method.VersionedMethod( - func_name, obj_min_ver, obj_max_ver, f) + func_name, obj_min_ver, obj_max_ver, experimental, f) func_dict = getattr(cls, VER_METHOD_ATTR, {}) if not func_dict: @@ -1145,6 +1157,9 @@ class Fault(webob.exc.HTTPException): if not req.api_version_request.is_null(): self.wrapped_exc.headers[API_VERSION_REQUEST_HEADER] = ( req.api_version_request.get_string()) + if req.api_version_request.experimental: + self.wrapped_exc.headers[EXPERIMENTAL_API_REQUEST_HEADER] = ( + req.api_version_request.experimental) self.wrapped_exc.headers['Vary'] = API_VERSION_REQUEST_HEADER content_type = req.best_match_content_type() diff --git a/manila/tests/api/openstack/test_api_version_request.py b/manila/tests/api/openstack/test_api_version_request.py index 4f4b4267a4..9193a5c821 100644 --- a/manila/tests/api/openstack/test_api_version_request.py +++ b/manila/tests/api/openstack/test_api_version_request.py @@ -18,6 +18,7 @@ import ddt import six from manila.api.openstack import api_version_request +from manila.api.openstack import versioned_method from manila import exception from manila import test @@ -25,6 +26,26 @@ from manila import test @ddt.ddt class APIVersionRequestTests(test.TestCase): + def test_init(self): + + result = api_version_request.APIVersionRequest() + + self.assertIsNone(result._ver_major) + self.assertIsNone(result._ver_minor) + self.assertFalse(result._experimental) + + def test_min_version(self): + + self.assertEqual( + api_version_request.APIVersionRequest(api_version_request._MIN_API_VERSION), + api_version_request.min_api_version()) + + def test_max_api_version(self): + + self.assertEqual( + api_version_request.APIVersionRequest(api_version_request._MAX_API_VERSION), + api_version_request.max_api_version()) + @ddt.data( ('1.1', 1, 1), ('2.10', 2, 10), @@ -38,8 +59,8 @@ class APIVersionRequestTests(test.TestCase): request = api_version_request.APIVersionRequest(version_string) - self.assertEqual(major, request.ver_major) - self.assertEqual(minor, request.ver_minor) + self.assertEqual(major, request._ver_major) + self.assertEqual(minor, request._ver_minor) def test_null_version(self): @@ -59,6 +80,23 @@ class APIVersionRequestTests(test.TestCase): request = api_version_request.APIVersionRequest('1.2') self.assertEqual((1, 2), request._cmpkey()) + @ddt.data(True, False) + def test_experimental_property(self, experimental): + + request = api_version_request.APIVersionRequest() + request.experimental = experimental + + self.assertEqual(experimental, request.experimental) + + def test_experimental_property_value_error(self): + + request = api_version_request.APIVersionRequest() + + def set_non_boolean(): + request.experimental = 'non_bool_value' + + self.assertRaises(exception.InvalidParameterValue, set_non_boolean) + def test_version_comparisons(self): v1 = api_version_request.APIVersionRequest('2.0') v2 = api_version_request.APIVersionRequest('2.5') @@ -100,6 +138,44 @@ class APIVersionRequestTests(test.TestCase): self.assertRaises(ValueError, v_null.matches, v1, v3) + def test_version_matches_experimental_request(self): + + experimental_request = api_version_request.APIVersionRequest('2.0') + experimental_request.experimental = True + + non_experimental_request = api_version_request.APIVersionRequest('2.0') + + experimental_function = versioned_method.VersionedMethod( + 'experimental_function', + api_version_request.APIVersionRequest('2.0'), + api_version_request.APIVersionRequest('2.1'), + True, + None) + + non_experimental_function = versioned_method.VersionedMethod( + 'non_experimental_function', + api_version_request.APIVersionRequest('2.0'), + api_version_request.APIVersionRequest('2.1'), + False, + None) + + self.assertTrue(experimental_request.matches_versioned_method( + experimental_function)) + self.assertTrue(experimental_request.matches_versioned_method( + non_experimental_function)) + self.assertTrue(non_experimental_request.matches_versioned_method( + non_experimental_function)) + self.assertFalse(non_experimental_request.matches_versioned_method( + experimental_function)) + + def test_matches_versioned_method(self): + + request = api_version_request.APIVersionRequest('2.0') + + self.assertRaises(exception.InvalidParameterValue, + request.matches_versioned_method, + 'fake_method') + def test_get_string(self): v1_string = '3.23' v1 = api_version_request.APIVersionRequest(v1_string) diff --git a/manila/tests/api/openstack/test_versioned_method.py b/manila/tests/api/openstack/test_versioned_method.py index fbcb896338..4bf6b725bf 100644 --- a/manila/tests/api/openstack/test_versioned_method.py +++ b/manila/tests/api/openstack/test_versioned_method.py @@ -23,7 +23,7 @@ class VersionedMethodTestCase(test.TestCase): def test_str(self): args = ('fake_name', 'fake_min', 'fake_max') - method = versioned_method.VersionedMethod(*(args + (None,))) + method = versioned_method.VersionedMethod(*(args + (False, None))) method_string = six.text_type(method) self.assertEqual('Version Method %s: min: %s, max: %s' % args, @@ -31,5 +31,6 @@ class VersionedMethodTestCase(test.TestCase): def test_cmpkey(self): method = versioned_method.VersionedMethod( - 'fake_name', 'fake_start_version', 'fake_end_version', 'fake_func') + 'fake_name', 'fake_start_version', 'fake_end_version', False, + 'fake_func') self.assertEqual('fake_start_version', method._cmpkey()) \ No newline at end of file diff --git a/manila/tests/api/test_versions.py b/manila/tests/api/test_versions.py index 458e67107c..7f620623a9 100644 --- a/manila/tests/api/test_versions.py +++ b/manila/tests/api/test_versions.py @@ -25,11 +25,13 @@ from manila import test from manila.tests.api import fakes +version_header_name = 'X-OpenStack-Manila-API-Version' +experimental_header_name = 'X-OpenStack-Manila-API-Experimental' + + @ddt.ddt class VersionsControllerTestCase(test.TestCase): - version_header_name = 'X-OpenStack-Manila-API-Version' - def setUp(self): super(VersionsControllerTestCase, self).setUp() self.wsgi_apps = (versions.VersionsRouter(), router.APIRouter()) @@ -57,7 +59,7 @@ class VersionsControllerTestCase(test.TestCase): req.method = 'GET' req.content_type = 'application/json' if include_header: - req.headers = {self.version_header_name: '1.0'} + req.headers = {version_header_name: '1.0'} for app in self.wsgi_apps: response = req.get_response(app) @@ -66,9 +68,8 @@ class VersionsControllerTestCase(test.TestCase): ids = [v['id'] for v in version_list] self.assertEqual({'v1.0'}, set(ids)) - self.assertEqual('1.0', response.headers[self.version_header_name]) - self.assertEqual(self.version_header_name, - response.headers['Vary']) + self.assertEqual('1.0', response.headers[version_header_name]) + self.assertEqual(version_header_name, response.headers['Vary']) self.assertIsNone(version_list[0].get('min_version')) self.assertIsNone(version_list[0].get('version')) @@ -83,7 +84,7 @@ class VersionsControllerTestCase(test.TestCase): req = fakes.HTTPRequest.blank('/', base_url=base_url) req.method = 'GET' req.content_type = 'application/json' - req.headers = {self.version_header_name: req_version} + req.headers = {version_header_name: req_version} for app in self.wsgi_apps: response = req.get_response(app) @@ -95,13 +96,12 @@ class VersionsControllerTestCase(test.TestCase): if req_version == 'latest': self.assertEqual(api_version_request._MAX_API_VERSION, - response.headers[self.version_header_name]) + response.headers[version_header_name]) else: self.assertEqual(req_version, - response.headers[self.version_header_name]) + response.headers[version_header_name]) - self.assertEqual(self.version_header_name, - response.headers['Vary']) + self.assertEqual(version_header_name, response.headers['Vary']) self.assertEqual(api_version_request._MIN_API_VERSION, version_list[0].get('min_version')) self.assertEqual(api_version_request._MAX_API_VERSION, @@ -112,30 +112,28 @@ class VersionsControllerTestCase(test.TestCase): req = fakes.HTTPRequest.blank('/', base_url=base_url) req.method = 'GET' req.content_type = 'application/json' - req.headers = {self.version_header_name: '2.0'} + req.headers = {version_header_name: '2.0'} for app in self.wsgi_apps: response = req.get_response(app) self.assertEqual(406, response.status_int) - self.assertEqual('2.0', response.headers[self.version_header_name]) - self.assertEqual(self.version_header_name, - response.headers['Vary']) + self.assertEqual('2.0', response.headers[version_header_name]) + self.assertEqual(version_header_name, response.headers['Vary']) @ddt.data('http://localhost/', None) def test_versions_index_invalid_version_request(self, base_url): req = fakes.HTTPRequest.blank('/', base_url=base_url) req.method = 'GET' req.content_type = 'application/json' - req.headers = {self.version_header_name: '2.0.1'} + req.headers = {version_header_name: '2.0.1'} for app in self.wsgi_apps: response = req.get_response(app) self.assertEqual(400, response.status_int) - self.assertEqual('1.0', response.headers[self.version_header_name]) - self.assertEqual(self.version_header_name, - response.headers['Vary']) + self.assertEqual('1.0', response.headers[version_header_name]) + self.assertEqual(version_header_name, response.headers['Vary']) def test_versions_version_not_found(self): api_version_request_3_0 = api_version_request.APIVersionRequest('3.0') @@ -149,8 +147,82 @@ class VersionsControllerTestCase(test.TestCase): return 'off' req = fakes.HTTPRequest.blank('/tests') - req.headers = {self.version_header_name: '2.0'} + req.headers = {version_header_name: '2.0'} app = fakes.TestRouter(Controller()) response = req.get_response(app) - self.assertEqual(404, response.status_int) \ No newline at end of file + self.assertEqual(404, response.status_int) + + +@ddt.ddt +class ExperimentalAPITestCase(test.TestCase): + + class Controller(wsgi.Controller): + @wsgi.Controller.api_version('1.0', '1.0') + def index(self, req): + return {'fake_key': 'fake_value'} + + @wsgi.Controller.api_version('1.1', '1.1', experimental=True) # noqa + def index(self, req): # pylint: disable=E0102 + return {'fake_key': 'fake_value'} + + def setUp(self): + super(ExperimentalAPITestCase, self).setUp() + self.app = fakes.TestRouter(ExperimentalAPITestCase.Controller()) + + @ddt.data(True, False) + def test_stable_api_always_called(self, experimental): + + req = fakes.HTTPRequest.blank('/tests') + req.headers = {version_header_name: '1.0'} + if experimental: + req.headers[experimental_header_name] = experimental + response = req.get_response(self.app) + + self.assertEqual(200, response.status_int) + self.assertEqual('1.0', response.headers[version_header_name]) + + if experimental: + self.assertEqual(experimental, + response.headers.get(experimental_header_name)) + else: + self.assertFalse(experimental_header_name in response.headers) + + def test_experimental_api_called_when_requested(self): + + req = fakes.HTTPRequest.blank('/tests') + req.headers = { + version_header_name: '1.1', + experimental_header_name: 'True', + } + response = req.get_response(self.app) + + self.assertEqual(200, response.status_int) + self.assertEqual('1.1', response.headers[version_header_name]) + self.assertTrue(response.headers.get(experimental_header_name)) + + def test_experimental_api_not_called_when_not_requested(self): + + req = fakes.HTTPRequest.blank('/tests') + req.headers = {version_header_name: '1.1'} + response = req.get_response(self.app) + + self.assertEqual(404, response.status_int) + self.assertFalse(experimental_header_name in response.headers) + + def test_experimental_header_returned_in_exception(self): + + api_version_request_3_0 = api_version_request.APIVersionRequest('3.0') + self.mock_object(api_version_request, + 'max_api_version', + mock.Mock(return_value=api_version_request_3_0)) + + req = fakes.HTTPRequest.blank('/tests') + req.headers = { + version_header_name: '1.2', + experimental_header_name: 'True', + } + response = req.get_response(self.app) + + self.assertEqual(404, response.status_int) + self.assertTrue(response.headers.get(experimental_header_name)) \ No newline at end of file