]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: fix error when typing existing paths in the Ganesha form 37688/head
authorKiefer Chang <kiefer.chang@suse.com>
Wed, 9 Sep 2020 08:20:32 +0000 (16:20 +0800)
committerKiefer Chang <kiefer.chang@suse.com>
Fri, 16 Oct 2020 06:44:11 +0000 (14:44 +0800)
- The `CephFS.ls_dir()` implementation in the backend had changed, the code in the
  UI endpoint `/ui-api/nfs-ganesha/lsdir` needs to adapt.
- Add fs_name as resource_id in `/ui-api/nfs-ganesha/lsdir/<fs_name>` to distinguish FS.
- Add more checks and unit tests.

Fixes: https://tracker.ceph.com/issues/47372
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
(cherry picked from commit ee14ef7c6095d0572e90b29580f2d8d6982dcbda)

Conflicts:
src/pybind/mgr/dashboard/controllers/nfsganesha.py
src/pybind/mgr/dashboard/tests/test_ganesha.py
        - Conflict due to import order changed in the master

qa/tasks/mgr/dashboard/test_ganesha.py
src/pybind/mgr/dashboard/controllers/nfsganesha.py
src/pybind/mgr/dashboard/frontend/src/app/ceph/nfs/nfs-form/nfs-form.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/nfs.service.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/nfs.service.ts
src/pybind/mgr/dashboard/tests/test_ganesha.py

index c99e3651ff1442185681b6d2ee378a3bdd7e3a6e..581cf4464d236896a73f1614cccd31305347fd74 100644 (file)
@@ -181,8 +181,13 @@ class GaneshaTest(DashboardTestCase):
         })))
 
     def test_ganesha_lsdir(self):
-        self._get('/ui-api/nfs-ganesha/lsdir')
-        self.assertStatus(500)
+        fss = self._get('/ui-api/nfs-ganesha/cephfs/filesystems')
+        self.assertStatus(200)
+        for fs in fss:
+            data = self._get('/ui-api/nfs-ganesha/lsdir/{}'.format(fs['name']))
+            self.assertStatus(200)
+            self.assertSchema(data, JObj({'paths': JList(str)}))
+            self.assertEqual(data['paths'][0], '/')
 
     def test_ganesha_buckets(self):
         data = self._get('/ui-api/nfs-ganesha/rgw/buckets')
index 7e4fd8b8660b8fab43d3fde2946537bb3c0e4c45..bef77791fb3cbbd606008401daaa1dc06fde1eac 100644 (file)
@@ -1,8 +1,9 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import
 
-from functools import partial
 import logging
+import os
+from functools import partial
 
 import cherrypy
 import cephfs
@@ -12,7 +13,7 @@ from . import ApiController, RESTController, UiApiController, BaseController, \
 from ..security import Scope
 from ..services.cephfs import CephFS
 from ..services.cephx import CephX
-from ..services.exception import serialize_dashboard_exception
+from ..services.exception import DashboardException, serialize_dashboard_exception
 from ..services.ganesha import Ganesha, GaneshaConf, NFSException
 from ..services.rgw_client import RgwClient
 
@@ -282,21 +283,35 @@ class NFSGaneshaUi(BaseController):
 
     @Endpoint('GET', '/lsdir')
     @ReadPermission
-    def lsdir(self, root_dir=None, depth=1):  # pragma: no cover
+    def lsdir(self, fs_name, root_dir=None, depth=1):  # pragma: no cover
         if root_dir is None:
             root_dir = "/"
-        depth = int(depth)
-        if depth > 5:
-            logger.warning("Limiting depth to maximum value of 5: "
-                           "input depth=%s", depth)
-            depth = 5
-        root_dir = '{}{}'.format(root_dir.rstrip('/'), '/')
+        if not root_dir.startswith('/'):
+            root_dir = '/{}'.format(root_dir)
+        root_dir = os.path.normpath(root_dir)
+
+        try:
+            depth = int(depth)
+            error_msg = ''
+            if depth < 0:
+                error_msg = '`depth` must be greater or equal to 0.'
+            if depth > 5:
+                logger.warning("Limiting depth to maximum value of 5: "
+                               "input depth=%s", depth)
+                depth = 5
+        except ValueError:
+            error_msg = '`depth` must be an integer.'
+        finally:
+            if error_msg:
+                raise DashboardException(code=400,
+                                         component='nfsganesha',
+                                         msg=error_msg)
+
         try:
-            cfs = CephFS()
-            root_dir = root_dir.encode()
-            paths = cfs.ls_dir(root_dir, depth)
-            # Convert (bytes => string) and prettify paths (strip slashes).
-            paths = [p.decode().rstrip('/') for p in paths if p != root_dir]
+            cfs = CephFS(fs_name)
+            paths = [root_dir]
+            paths.extend([p['path'].rstrip('/')
+                          for p in cfs.ls_dir(root_dir, depth)])
         except (cephfs.ObjectNotFound, cephfs.PermissionError):
             paths = []
         return {'paths': paths}
index 8467d46e481b109a408061fe4efd50c32c14abbb..a85a32fc6f62499d5c35dae3cfeb2b6b4064759b 100644 (file)
@@ -377,7 +377,8 @@ export class NfsFormComponent implements OnInit {
       return of([]);
     }
 
-    return this.nfsService.lsDir(path);
+    const fsName = this.nfsForm.getValue('fsal').fs_name;
+    return this.nfsService.lsDir(fsName, path);
   }
 
   pathChangeHandler() {
index e5f039ff39e97243f15bdfae06b637f08747c4c3..ca5b25eb5de20081e75e757b3b9423805cb6b9b6 100644 (file)
@@ -59,8 +59,8 @@ describe('NfsService', () => {
   });
 
   it('should call lsDir', () => {
-    service.lsDir('foo_dir').subscribe();
-    const req = httpTesting.expectOne('ui-api/nfs-ganesha/lsdir?root_dir=foo_dir');
+    service.lsDir('a', 'foo_dir').subscribe();
+    const req = httpTesting.expectOne('ui-api/nfs-ganesha/lsdir/a?root_dir=foo_dir');
     expect(req.request.method).toBe('GET');
   });
 
index 1a53047ecdce0ea14f2bbc2d7a0d57ed9f7e1b09..7c00659d6e7cfd0c8248472d9f2ac1c97177006d 100644 (file)
@@ -75,8 +75,8 @@ export class NfsService {
     });
   }
 
-  lsDir(root_dir: string) {
-    return this.http.get(`${this.uiApiPath}/lsdir?root_dir=${root_dir}`);
+  lsDir(fs_name: string, root_dir: string) {
+    return this.http.get(`${this.uiApiPath}/lsdir/${fs_name}?root_dir=${root_dir}`);
   }
 
   buckets(user_id: string) {
index 03f1f5b5513e8863d447d109012b180c1df1d389..2090f1ee60d1858774a89f8cbb797d0184c2e5db 100644 (file)
@@ -2,18 +2,23 @@
 from __future__ import absolute_import
 
 import unittest
+from urllib.parse import urlencode
 
 try:
-    from mock import MagicMock, Mock
+    from mock import MagicMock, Mock, patch
 except ImportError:
-    from unittest.mock import MagicMock, Mock
+    # pylint: disable=ungrouped-imports
+    from unittest.mock import MagicMock, Mock, patch
 
 import orchestrator
-from . import KVStoreMockMixin
+
 from .. import mgr
-from ..settings import Settings
+from ..controllers.nfsganesha import NFSGaneshaUi
 from ..services import ganesha
-from ..services.ganesha import GaneshaConf, Export, GaneshaConfParser
+from ..services.ganesha import Export, GaneshaConf, GaneshaConfParser
+from ..settings import Settings
+from . import ControllerTestCase  # pylint: disable=no-name-in-module
+from . import KVStoreMockMixin  # pylint: disable=no-name-in-module
 
 
 class GaneshaConfTest(unittest.TestCase, KVStoreMockMixin):
@@ -643,3 +648,97 @@ EXPORT
         self.assertEqual(export.cluster_id, '_default_')
         self.assertEqual(export.attr_expiration_time, 0)
         self.assertEqual(export.security_label, True)
+
+
+class NFSGaneshaUiControllerTest(ControllerTestCase):
+    @classmethod
+    def setup_server(cls):
+        # pylint: disable=protected-access
+        NFSGaneshaUi._cp_config['tools.authenticate.on'] = False
+        cls.setup_controllers([NFSGaneshaUi])
+
+    @classmethod
+    def _create_ls_dir_url(cls, fs_name, query_params):
+        api_url = '/ui-api/nfs-ganesha/lsdir/{}'.format(fs_name)
+        if query_params is not None:
+            return '{}?{}'.format(api_url, urlencode(query_params))
+        return api_url
+
+    @patch('dashboard.controllers.nfsganesha.CephFS')
+    def test_lsdir(self, cephfs_class):
+        cephfs_class.return_value.ls_dir.return_value = [
+            {'path': '/foo'},
+            {'path': '/foo/bar'}
+        ]
+        mocked_ls_dir = cephfs_class.return_value.ls_dir
+
+        reqs = [
+            {
+                'params': None,
+                'cephfs_ls_dir_args': ['/', 1],
+                'path0': '/',
+                'status': 200
+            },
+            {
+                'params': {'root_dir': '/', 'depth': '1'},
+                'cephfs_ls_dir_args': ['/', 1],
+                'path0': '/',
+                'status': 200
+            },
+            {
+                'params': {'root_dir': '', 'depth': '1'},
+                'cephfs_ls_dir_args': ['/', 1],
+                'path0': '/',
+                'status': 200
+            },
+            {
+                'params': {'root_dir': '/foo', 'depth': '3'},
+                'cephfs_ls_dir_args': ['/foo', 3],
+                'path0': '/foo',
+                'status': 200
+            },
+            {
+                'params': {'root_dir': 'foo', 'depth': '6'},
+                'cephfs_ls_dir_args': ['/foo', 5],
+                'path0': '/foo',
+                'status': 200
+            },
+            {
+                'params': {'root_dir': '/', 'depth': '-1'},
+                'status': 400
+            },
+            {
+                'params': {'root_dir': '/', 'depth': 'abc'},
+                'status': 400
+            }
+        ]
+
+        for req in reqs:
+            self._get(self._create_ls_dir_url('a', req['params']))
+            self.assertStatus(req['status'])
+
+            # Returned paths should contain root_dir as first element
+            if req['status'] == 200:
+                paths = self.json_body()['paths']
+                self.assertEqual(paths[0], req['path0'])
+                cephfs_class.assert_called_once_with('a')
+
+            # Check the arguments passed to `CephFS.ls_dir`.
+            if req.get('cephfs_ls_dir_args'):
+                mocked_ls_dir.assert_called_once_with(*req['cephfs_ls_dir_args'])
+            else:
+                mocked_ls_dir.assert_not_called()
+            mocked_ls_dir.reset_mock()
+            cephfs_class.reset_mock()
+
+    @patch('dashboard.controllers.nfsganesha.cephfs')
+    @patch('dashboard.controllers.nfsganesha.CephFS')
+    def test_lsdir_non_existed_dir(self, cephfs_class, cephfs):
+        cephfs.ObjectNotFound = Exception
+        cephfs.PermissionError = Exception
+        cephfs_class.return_value.ls_dir.side_effect = cephfs.ObjectNotFound()
+        self._get(self._create_ls_dir_url('a', {'root_dir': '/foo', 'depth': '3'}))
+        cephfs_class.assert_called_once_with('a')
+        cephfs_class.return_value.ls_dir.assert_called_once_with('/foo', 3)
+        self.assertStatus(200)
+        self.assertJsonBody({'paths': []})