From c49bd143f85316733157c43615cd617c3767f2fd Mon Sep 17 00:00:00 2001 From: Sharpz7 Date: Wed, 20 Nov 2024 19:47:27 +0000 Subject: [PATCH] api: Introduce new mechanism for API versioning ...and apply it to our first controller, the controller for the '/shards' API. Rather than having a check inside the method or overriding RestController._route, indicate the minimum (and potentially maximum API version supported by a decorator. This is similar to what Nova et al do, but without the descriptor protocol-derived hijinks. Doing things this way allows us to attach metadata to the controller which can later be inspected by a schema generator. Later changes can add more API versions. Related-Bug: 2086121 Change-Id: I9ccfe8240860d6300bbec5ae7d06f1dfc47f788c Signed-off-by: Stephen Finucane --- ironic/api/controllers/v1/shard.py | 21 +++--- ironic/api/validation/__init__.py | 72 +++++++++++++++++++ .../unit/api/controllers/v1/test_shard.py | 8 +++ 3 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 ironic/api/validation/__init__.py diff --git a/ironic/api/controllers/v1/shard.py b/ironic/api/controllers/v1/shard.py index 7aa0869970..71c91a12d2 100644 --- a/ironic/api/controllers/v1/shard.py +++ b/ironic/api/controllers/v1/shard.py @@ -13,11 +13,12 @@ from ironic_lib import metrics_utils from oslo_config import cfg import pecan -from webob import exc as webob_exc from ironic import api from ironic.api.controllers.v1 import utils as api_utils +from ironic.api.controllers.v1 import versions from ironic.api import method +from ironic.api import validation from ironic.common.i18n import _ @@ -29,18 +30,12 @@ METRICS = metrics_utils.get_metrics_logger(__name__) class ShardController(pecan.rest.RestController): """REST controller for shards.""" - @pecan.expose() - def _route(self, argv, request=None): - if not api_utils.allow_shards_endpoint(): - msg = _("The API version does not allow shards") - if api.request.method in "GET": - raise webob_exc.HTTPNotFound(msg) - else: - raise webob_exc.HTTPMethodNotAllowed(msg) - return super(ShardController, self)._route(argv, request) - @METRICS.timer('ShardController.get_all') @method.expose() + @validation.api_version( + min_version=versions.MINOR_82_NODE_SHARD, + message=_('The API version does not allow shards'), + ) def get_all(self): """Retrieve a list of shards. @@ -54,6 +49,10 @@ class ShardController(pecan.rest.RestController): @METRICS.timer('ShardController.get_one') @method.expose() + @validation.api_version( + min_version=versions.MINOR_82_NODE_SHARD, + message=_('The API version does not allow shards'), + ) def get_one(self, __): """Explicitly do not support getting one.""" pecan.abort(404) diff --git a/ironic/api/validation/__init__.py b/ironic/api/validation/__init__.py new file mode 100644 index 0000000000..e0c45831f3 --- /dev/null +++ b/ironic/api/validation/__init__.py @@ -0,0 +1,72 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, 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 functools +import typing as ty + +from webob import exc as webob_exc + +from ironic import api +from ironic.common.i18n import _ + + +def api_version( + min_version: ty.Optional[int], + max_version: ty.Optional[int] = None, + message: ty.Optional[str] = None, + exception_class: ty.Type[webob_exc.HTTPException] = webob_exc.HTTPNotFound, +): + """Decorator for marking lower and upper bounds on API methods. + + :param min_version: An integer representing the minimum API version that + the API is available under. + :param max_version: An integer representing the maximum API version that + the API is available under. + :param message: A message to return if the API is not supported. + :param exception_class: The exception class to raise if the API version is + not supported (default is HTTPNotFound). + """ + + # Ensure the provided status code is valid for the given exception class + assert isinstance( + exception_class, + type(webob_exc.HTTPException) + ), ( + "Invalid exception class provided, must be a " + "subclass of webob_exc.HTTPException." + ) + + def add_validator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + # Version checks + if ( + min_version and not api.request.version.minor >= min_version + ) or ( + max_version and not api.request.version.minor <= max_version + ): + # Raise provided exception with localized message + raise exception_class( + detail=_( + message + or 'The API is not supported for this version' + ) + ) + + return func(*args, **kwargs) + + wrapper.min_version = min_version + wrapper.max_version = max_version + + return wrapper + + return add_validator diff --git a/ironic/tests/unit/api/controllers/v1/test_shard.py b/ironic/tests/unit/api/controllers/v1/test_shard.py index 73d30f1063..f92cde6b5d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_shard.py +++ b/ironic/tests/unit/api/controllers/v1/test_shard.py @@ -69,6 +69,14 @@ class TestListShards(test_api_base.BaseApiTest): '/shards/shard1', expect_errors=True, headers=self.headers) self.assertEqual(http_client.NOT_FOUND, result.status_int) + def test_fail_get_one_wrong_version(self): + self._create_test_shard('shard1', 1) + # We should get the same response if we request with the wrong version + headers = {api_base.Version.string: '1.80'} + result = self.get_json( + '/shards/shard1', expect_errors=True, headers=headers) + self.assertEqual(http_client.NOT_FOUND, result.status_int) + def test_fail_post(self): result = self.post_json( '/shards', {}, expect_errors=True, headers=self.headers)