]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: make modified API endpoints backward compatible
authorAvan Thakkar <athakkar@redhat.com>
Thu, 23 Sep 2021 11:15:16 +0000 (16:45 +0530)
committerNizamudeen A <nia@redhat.com>
Thu, 14 Oct 2021 17:34:27 +0000 (23:04 +0530)
Fixes: https://tracker.ceph.com/issues/52480
Signed-off-by: Avan Thakkar <athakkar@redhat.com>
Introducing APIVersion class to handle versioning for API-endpints and making
them backward compatible.

13 files changed:
qa/tasks/mgr/dashboard/__init__.py
qa/tasks/mgr/dashboard/helper.py
qa/tasks/mgr/dashboard/test_api.py
qa/tasks/mgr/dashboard/test_requests.py
src/pybind/mgr/dashboard/__init__.py
src/pybind/mgr/dashboard/controllers/docs.py
src/pybind/mgr/dashboard/frontend/cypress/support/commands.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/host.service.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/classes/cd-helper.class.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/services/api-interceptor.service.ts
src/pybind/mgr/dashboard/tests/__init__.py
src/pybind/mgr/dashboard/tests/test_tools.py
src/pybind/mgr/dashboard/tests/test_versioning.py

index c066be5f49e36971c9488f03d3042a0bda257e97..2b022e02495e6d0eac4bdb3ba16fa0d7c71cc5cd 100644 (file)
@@ -1 +1 @@
-DEFAULT_VERSION = '1.0'
+DEFAULT_API_VERSION = '1.0'
index 2b6bedc948a37cf8c4d831f97a3e1137bc317073..2c6efa9015aaa175926a819146877a489eccbc24 100644 (file)
@@ -16,7 +16,7 @@ from tasks.mgr.mgr_test_case import MgrTestCase
 from teuthology.exceptions import \
     CommandFailedError  # pylint: disable=import-error
 
-from . import DEFAULT_VERSION
+from . import DEFAULT_API_VERSION
 
 log = logging.getLogger(__name__)
 
@@ -276,7 +276,7 @@ class DashboardTestCase(MgrTestCase):
 
     # pylint: disable=inconsistent-return-statements, too-many-arguments, too-many-branches
     @classmethod
-    def _request(cls, url, method, data=None, params=None, version=DEFAULT_VERSION,
+    def _request(cls, url, method, data=None, params=None, version=DEFAULT_API_VERSION,
                  set_cookies=False):
         url = "{}{}".format(cls._base_uri, url)
         log.debug("Request %s to %s", method, url)
@@ -336,7 +336,7 @@ class DashboardTestCase(MgrTestCase):
             raise ex
 
     @classmethod
-    def _get(cls, url, params=None, version=DEFAULT_VERSION, set_cookies=False):
+    def _get(cls, url, params=None, version=DEFAULT_API_VERSION, set_cookies=False):
         return cls._request(url, 'GET', params=params, version=version, set_cookies=set_cookies)
 
     @classmethod
@@ -344,7 +344,7 @@ class DashboardTestCase(MgrTestCase):
         retry = True
         while retry and retries > 0:
             retry = False
-            res = cls._get(url, version=DEFAULT_VERSION)
+            res = cls._get(url, version=DEFAULT_API_VERSION)
             if isinstance(res, dict):
                 res = [res]
             for view in res:
@@ -358,15 +358,15 @@ class DashboardTestCase(MgrTestCase):
         return res
 
     @classmethod
-    def _post(cls, url, data=None, params=None, version=DEFAULT_VERSION, set_cookies=False):
+    def _post(cls, url, data=None, params=None, version=DEFAULT_API_VERSION, set_cookies=False):
         cls._request(url, 'POST', data, params, version=version, set_cookies=set_cookies)
 
     @classmethod
-    def _delete(cls, url, data=None, params=None, version=DEFAULT_VERSION, set_cookies=False):
+    def _delete(cls, url, data=None, params=None, version=DEFAULT_API_VERSION, set_cookies=False):
         cls._request(url, 'DELETE', data, params, version=version, set_cookies=set_cookies)
 
     @classmethod
-    def _put(cls, url, data=None, params=None, version=DEFAULT_VERSION, set_cookies=False):
+    def _put(cls, url, data=None, params=None, version=DEFAULT_API_VERSION, set_cookies=False):
         cls._request(url, 'PUT', data, params, version=version, set_cookies=set_cookies)
 
     @classmethod
@@ -386,7 +386,8 @@ class DashboardTestCase(MgrTestCase):
 
     # pylint: disable=too-many-arguments
     @classmethod
-    def _task_request(cls, method, url, data, timeout, version=DEFAULT_VERSION, set_cookies=False):
+    def _task_request(cls, method, url, data, timeout, version=DEFAULT_API_VERSION,
+                      set_cookies=False):
         res = cls._request(url, method, data, version=version, set_cookies=set_cookies)
         cls._assertIn(cls._resp.status_code, [200, 201, 202, 204, 400, 403, 404])
 
@@ -438,17 +439,17 @@ class DashboardTestCase(MgrTestCase):
         return res_task['exception']
 
     @classmethod
-    def _task_post(cls, url, data=None, timeout=60, version=DEFAULT_VERSION, set_cookies=False):
+    def _task_post(cls, url, data=None, timeout=60, version=DEFAULT_API_VERSION, set_cookies=False):
         return cls._task_request('POST', url, data, timeout, version=version,
                                  set_cookies=set_cookies)
 
     @classmethod
-    def _task_delete(cls, url, timeout=60, version=DEFAULT_VERSION, set_cookies=False):
+    def _task_delete(cls, url, timeout=60, version=DEFAULT_API_VERSION, set_cookies=False):
         return cls._task_request('DELETE', url, None, timeout, version=version,
                                  set_cookies=set_cookies)
 
     @classmethod
-    def _task_put(cls, url, data=None, timeout=60, version=DEFAULT_VERSION, set_cookies=False):
+    def _task_put(cls, url, data=None, timeout=60, version=DEFAULT_API_VERSION, set_cookies=False):
         return cls._task_request('PUT', url, data, timeout, version=version,
                                  set_cookies=set_cookies)
 
index 2fe1a78be54843aac167dd9f897cf1fbc4f55e63..22f23569874d54eb4590616ddf0085b62709e1e1 100644 (file)
@@ -4,14 +4,14 @@ from __future__ import absolute_import
 
 import unittest
 
-from . import DEFAULT_VERSION
+from . import DEFAULT_API_VERSION
 from .helper import DashboardTestCase
 
 
 class VersionReqTest(DashboardTestCase, unittest.TestCase):
     def test_version(self):
         for (version, expected_status) in [
-                (DEFAULT_VERSION, 200),
+                (DEFAULT_API_VERSION, 200),
                 (None, 415),
                 ("99.99", 415)
         ]:
index 93b175bfda0ea635248d1a3f921fdc2dc7144684..0d7fb75adc8edfbcc34ec3029b93e26614d8c27a 100644 (file)
@@ -2,7 +2,7 @@
 
 from __future__ import absolute_import
 
-from . import DEFAULT_VERSION
+from . import DEFAULT_API_VERSION
 from .helper import DashboardTestCase
 
 
@@ -11,7 +11,7 @@ class RequestsTest(DashboardTestCase):
         self._get('/api/summary')
         self.assertHeaders({
             'Content-Encoding': 'gzip',
-            'Content-Type': 'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION)
+            'Content-Type': 'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION)
         })
 
     def test_force_no_gzip(self):
@@ -27,7 +27,7 @@ class RequestsTest(DashboardTestCase):
         self._get('/api/summary')
         self.assertHeaders({
             'server': 'Ceph-Dashboard',
-            'Content-Type': 'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION),
+            'Content-Type': 'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION),
             'Content-Security-Policy': "frame-ancestors 'self';",
             'X-Content-Type-Options': 'nosniff',
             'Strict-Transport-Security': 'max-age=63072000; includeSubDomains; preload'
index 34485327e3535bac9d49194c191d7f60362c7360..b760ae661cd98dbcbcb07d5f95a06f26fb73af5a 100644 (file)
@@ -6,10 +6,35 @@ ceph dashboard module
 from __future__ import absolute_import
 
 import os
+import re
+from typing import NamedTuple
 
 import cherrypy
 
-DEFAULT_VERSION = '1.0'
+DEFAULT_API_VERSION = '1.0'
+
+
+class APIVersion(NamedTuple):
+    major: int = 1
+    minor: int = 0
+
+    @classmethod
+    def from_string(cls, version_string):
+        result = re.match(r'application/vnd\.ceph\.api\.v(\d+)\.(\d+)\+json', version_string)
+        if result:
+            return cls(*(int(s) for s in result.groups()))
+        return None
+
+    def __str__(self):
+        return f'{self.major}.{self.minor}'
+
+    def supports(self, client_version):
+        return self.major == client_version.major and client_version.minor <= self.minor
+
+    @staticmethod
+    def to_tuple(version: str):
+        return APIVersion(int(version.split('.')[0]), int(version.split('.')[1]))
+
 
 if 'COVERAGE_ENABLED' in os.environ:
     import coverage  # pylint: disable=import-error
index 7ab6946aadbd1d1183e7b140fe93fdcf401d1d4c..22aafeec6c3512eca64f3fe92cebe42bed198971 100644 (file)
@@ -6,7 +6,7 @@ from typing import Any, Dict, List, Union
 
 import cherrypy
 
-from .. import DEFAULT_VERSION, mgr
+from .. import DEFAULT_API_VERSION, mgr
 from ..api.doc import Schema, SchemaInput, SchemaType
 from . import ENDPOINT_MAP, BaseController, Endpoint, Router
 from ._version import APIVersion
@@ -206,7 +206,7 @@ class Docs(BaseController):
         }
 
         if not version:
-            version = DEFAULT_VERSION
+            version = DEFAULT_API_VERSION
 
         if method.lower() == 'get':
             resp['200'] = {'description': "OK",
index 60fa7a746c043cba2f39e5f45040e09865c576a6..a4c8ff332c49613d9f6bbbfce9ca84d2f2385f4a 100644 (file)
@@ -7,6 +7,7 @@ declare global {
   }
 }
 
+import { CdHelperClass } from '~/app/shared/classes/cd-helper.class';
 import { Permissions } from '~/app/shared/models/permissions';
 
 let auth: any;
@@ -27,7 +28,7 @@ Cypress.Commands.add('login', () => {
     cy.request({
       method: 'POST',
       url: 'api/auth',
-      headers: { Accept: 'application/vnd.ceph.api.v1.0+json' },
+      headers: { Accept: CdHelperClass.cdVersionHeader('1', '0') },
       body: { username: username, password: password }
     }).then((resp) => {
       auth = resp.body;
index 7f8956baa263d1919219e475b87088d05b408384..984ee88fff11b5b2feed2976a0a7d3dfcbe47c8f 100644 (file)
@@ -7,6 +7,7 @@ import { map, mergeMap, toArray } from 'rxjs/operators';
 
 import { InventoryDevice } from '~/app/ceph/cluster/inventory/inventory-devices/inventory-device.model';
 import { InventoryHost } from '~/app/ceph/cluster/inventory/inventory-host.model';
+import { CdHelperClass } from '~/app/shared/classes/cd-helper.class';
 import { Daemon } from '../models/daemon.interface';
 import { CdDevice } from '../models/devices';
 import { SmartDataResponseV1 } from '../models/smart';
@@ -29,7 +30,7 @@ export class HostService {
     return this.http.post(
       this.baseURL,
       { hostname: hostname, addr: addr, labels: labels, status: status },
-      { observe: 'response', headers: { Accept: 'application/vnd.ceph.api.v0.1+json' } }
+      { observe: 'response', headers: { Accept: CdHelperClass.cdVersionHeader('0', '1') } }
     );
   }
 
index 5c872b42fd83a98fb1cc6038f9b3998b4df180fb..25057312536ac11acb3faba7a1e22f8b2ca6e8a7 100644 (file)
@@ -21,4 +21,8 @@ export class CdHelperClass {
 
     return hasChanges;
   }
+
+  static cdVersionHeader(major_ver: string, minor_ver: string) {
+    return `application/vnd.ceph.api.v${major_ver}.${minor_ver}+json`;
+  }
 }
index cf7ddd342e0ea3b024a31332e8acdee0bd01ab4c..fb7a9f73395f00ca5b381cac6b5f6bdd2202082f 100644 (file)
@@ -12,6 +12,7 @@ import _ from 'lodash';
 import { Observable, throwError as observableThrowError } from 'rxjs';
 import { catchError } from 'rxjs/operators';
 
+import { CdHelperClass } from '~/app/shared/classes/cd-helper.class';
 import { NotificationType } from '../enum/notification-type.enum';
 import { CdNotificationConfig } from '../models/cd-notification';
 import { FinishedTask } from '../models/finished-task';
@@ -34,7 +35,6 @@ export class ApiInterceptorService implements HttpInterceptor {
   ) {}
 
   intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
-    const defaultVersion = '1.0';
     const acceptHeader = request.headers.get('Accept');
     let reqWithVersion: HttpRequest<any>;
     if (acceptHeader && acceptHeader.startsWith('application/vnd.ceph.api.v')) {
@@ -42,7 +42,7 @@ export class ApiInterceptorService implements HttpInterceptor {
     } else {
       reqWithVersion = request.clone({
         setHeaders: {
-          Accept: `application/vnd.ceph.api.v${defaultVersion}+json`
+          Accept: CdHelperClass.cdVersionHeader('1', '0')
         }
       });
     }
index 742ac371864eb5f1751168fdec6e39ec17a2ae99..a09fe6a9adb0c45162fdec181b2d24898846d95d 100644 (file)
@@ -14,7 +14,7 @@ from cherrypy.test import helper
 from mgr_module import HandleCommandResult
 from pyfakefs import fake_filesystem
 
-from .. import mgr
+from .. import DEFAULT_API_VERSION, mgr
 from ..controllers import generate_controller_routes, json_error_page
 from ..controllers._version import APIVersion
 from ..module import Module
@@ -154,7 +154,7 @@ class ControllerTestCase(helper.CPWebCase):
         if cls._request_logging:
             cherrypy.config.update({'tools.request_logging.on': False})
 
-    def _request(self, url, method, data=None, headers=None, version=DEFAULT_VERSION):
+    def _request(self, url, method, data=None, headers=None, version=DEFAULT_API_VERSION):
         if not data:
             b = None
             if version:
@@ -177,19 +177,19 @@ class ControllerTestCase(helper.CPWebCase):
             h = headers
         self.getPage(url, method=method, body=b, headers=h)
 
-    def _get(self, url, headers=None, version=DEFAULT_VERSION):
+    def _get(self, url, headers=None, version=DEFAULT_API_VERSION):
         self._request(url, 'GET', headers=headers, version=version)
 
-    def _post(self, url, data=None, version=DEFAULT_VERSION):
+    def _post(self, url, data=None, version=DEFAULT_API_VERSION):
         self._request(url, 'POST', data, version=version)
 
-    def _delete(self, url, data=None, version=DEFAULT_VERSION):
+    def _delete(self, url, data=None, version=DEFAULT_API_VERSION):
         self._request(url, 'DELETE', data, version=version)
 
-    def _put(self, url, data=None, version=DEFAULT_VERSION):
+    def _put(self, url, data=None, version=DEFAULT_API_VERSION):
         self._request(url, 'PUT', data, version=version)
 
-    def _task_request(self, method, url, data, timeout, version=DEFAULT_VERSION):
+    def _task_request(self, method, url, data, timeout, version=DEFAULT_API_VERSION):
         self._request(url, method, data, version=version)
         if self.status != '202 Accepted':
             logger.info("task finished immediately")
@@ -255,13 +255,13 @@ class ControllerTestCase(helper.CPWebCase):
             self.status = 500
         self.body = json.dumps(thread.res_task['exception'])
 
-    def _task_post(self, url, data=None, timeout=60, version=DEFAULT_VERSION):
+    def _task_post(self, url, data=None, timeout=60, version=DEFAULT_API_VERSION):
         self._task_request('POST', url, data, timeout, version=version)
 
-    def _task_delete(self, url, timeout=60, version=DEFAULT_VERSION):
+    def _task_delete(self, url, timeout=60, version=DEFAULT_API_VERSION):
         self._task_request('DELETE', url, None, timeout, version=version)
 
-    def _task_put(self, url, data=None, timeout=60, version=DEFAULT_VERSION):
+    def _task_put(self, url, data=None, timeout=60, version=DEFAULT_API_VERSION):
         self._task_request('PUT', url, data, timeout, version=version)
 
     def json_body(self):
index 0313e10ddcc3526cca347af475b6aa7b83e49b0f..4edb458b955d766df2262047e9b0443da927f0d6 100644 (file)
@@ -11,6 +11,7 @@ try:
 except ImportError:
     from unittest.mock import patch
 
+from .. import DEFAULT_API_VERSION
 from ..controllers import APIRouter, BaseController, Proxy, RESTController, Router
 from ..controllers._version import APIVersion
 from ..services.exception import handle_rados_error
@@ -88,7 +89,7 @@ class RESTControllerTest(ControllerTestCase):
         self._get("/foo")
         self.assertStatus('200 OK')
         self.assertHeader('Content-Type',
-                          'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION))
+                          'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION))
         self.assertBody('[]')
 
     def test_fill(self):
@@ -100,18 +101,18 @@ class RESTControllerTest(ControllerTestCase):
                 self.assertJsonBody(data)
                 self.assertStatus(201)
                 self.assertHeader('Content-Type',
-                                  'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION))
+                                  'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION))
 
             self._get("/foo")
             self.assertStatus('200 OK')
             self.assertHeader('Content-Type',
-                              'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION))
+                              'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION))
             self.assertJsonBody([data] * 5)
 
             self._put('/foo/0', {'newdata': 'newdata'})
             self.assertStatus('200 OK')
             self.assertHeader('Content-Type',
-                              'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION))
+                              'application/vnd.ceph.api.v{}+json'.format(DEFAULT_API_VERSION))
             self.assertJsonBody({'newdata': 'newdata', 'key': '0'})
 
     def test_not_implemented(self):
index b3e6f6f8a6fa2ef843cab3f9974604891d338479..ae153e3e18d9af2f60edc5d5ac93947070fda369 100644 (file)
@@ -24,6 +24,10 @@ class VTest(RESTController):
     def vmethod(self):
         return {'version': '1.0'}
 
+    @RESTController.Collection('GET', version="1.1")
+    def vmethodv1_1(self):
+        return {'version': '1.1'}
+
     @RESTController.Collection('GET', version="2.0")
     def vmethodv2(self):
         return {'version': '2.0'}
@@ -60,3 +64,13 @@ class RESTVersioningTest(ControllerTestCase, unittest.TestCase):
             with self.subTest(version=version):
                 self._get('/test/api/vtest/vmethodv2', version=version)
                 self.assertStatus(expected_status)
+
+    def test_backward_compatibility(self):
+        for (version, expected_status) in [
+                ("1.1", 200),
+                ("1.0", 200),
+                ("2.0", 415)
+        ]:
+            with self.subTest(version=version):
+                self._get('/test/api/vtest/vmethodv1_1', version=version)
+                self.assertStatus(expected_status)