From: Volker Theile <vtheile@suse.com>
Date: Fri, 13 Dec 2019 14:48:45 +0000 (+0100)
Subject: nautilus: mgr/dashboard: Hardening accessing the metadata
X-Git-Tag: v14.2.8~20^2~35^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F32128%2Fhead;p=ceph.git

nautilus: mgr/dashboard: Hardening accessing the metadata

Partly backported from https://tracker.ceph.com/issues/41372. This is done because we only want to backport the hardened code to access the metadata information.

Fixes: https://tracker.ceph.com/issues/43079

Signed-off-by: Volker Theile <vtheile@suse.com>
---

diff --git a/src/pybind/mgr/dashboard/controllers/cephfs.py b/src/pybind/mgr/dashboard/controllers/cephfs.py
index e16f04c61cc4..788a865a6fc5 100644
--- a/src/pybind/mgr/dashboard/controllers/cephfs.py
+++ b/src/pybind/mgr/dashboard/controllers/cephfs.py
@@ -101,6 +101,12 @@ class CephFS(RESTController):
 
         return names
 
+    def _append_mds_metadata(self, mds_versions, metadata_key):
+        metadata = mgr.get_metadata('mds', metadata_key)
+        if metadata is None:
+            return
+        mds_versions[metadata.get('ceph_version', 'unknown')].append(metadata_key)
+
     # pylint: disable=too-many-statements,too-many-branches
     def fs_status(self, fs_id):
         mds_versions = defaultdict(list)
@@ -155,9 +161,7 @@ class CephFS(RESTController):
                 else:
                     activity = 0.0
 
-                metadata = mgr.get_metadata('mds', info['name'])
-                mds_versions[metadata.get('ceph_version', 'unknown')].append(
-                    info['name'])
+                self._append_mds_metadata(mds_versions, info['name'])
                 rank_table.append(
                     {
                         "rank": rank,
@@ -224,10 +228,7 @@ class CephFS(RESTController):
 
         standby_table = []
         for standby in fsmap['standbys']:
-            metadata = mgr.get_metadata('mds', standby['name'])
-            mds_versions[metadata.get('ceph_version', 'unknown')].append(
-                standby['name'])
-
+            self._append_mds_metadata(mds_versions, standby['name'])
             standby_table.append({
                 'name': standby['name']
             })
diff --git a/src/pybind/mgr/dashboard/tests/test_cephfs.py b/src/pybind/mgr/dashboard/tests/test_cephfs.py
new file mode 100644
index 000000000000..e9abda538090
--- /dev/null
+++ b/src/pybind/mgr/dashboard/tests/test_cephfs.py
@@ -0,0 +1,48 @@
+# -*- coding: utf-8 -*-
+from collections import defaultdict
+try:
+    from mock import Mock
+except ImportError:
+    from unittest.mock import Mock
+
+from .. import mgr
+from . import ControllerTestCase
+from ..controllers.cephfs import CephFS
+
+
+class MetaDataMock(object):
+    def get(self, _x, _y):
+        return 'bar'
+
+
+def get_metadata_mock(key, meta_key):
+    return {
+        'mds': {
+            None: None,  # Unknown key
+            'foo': MetaDataMock()
+        }[meta_key]
+    }[key]
+
+
+class CephFsTest(ControllerTestCase):
+    cephFs = CephFS()
+
+    @classmethod
+    def setup_server(cls):
+        mgr.get_metadata = Mock(side_effect=get_metadata_mock)
+
+    def tearDown(self):
+        mgr.get_metadata.stop()
+
+    def test_append_of_mds_metadata_if_key_is_not_found(self):
+        mds_versions = defaultdict(list)
+        # pylint: disable=protected-access
+        self.cephFs._append_mds_metadata(mds_versions, None)
+        self.assertEqual(len(mds_versions), 0)
+
+    def test_append_of_mds_metadata_with_existing_metadata(self):
+        mds_versions = defaultdict(list)
+        # pylint: disable=protected-access
+        self.cephFs._append_mds_metadata(mds_versions, 'foo')
+        self.assertEqual(len(mds_versions), 1)
+        self.assertEqual(mds_versions['bar'], ['foo'])
diff --git a/src/pybind/mgr/dashboard/tests/test_rbd_mirroring.py b/src/pybind/mgr/dashboard/tests/test_rbd_mirroring.py
index bad16b746b26..b307c2097a27 100644
--- a/src/pybind/mgr/dashboard/tests/test_rbd_mirroring.py
+++ b/src/pybind/mgr/dashboard/tests/test_rbd_mirroring.py
@@ -48,7 +48,7 @@ class RbdMirroringSummaryControllerTest(ControllerTestCase):
     @classmethod
     def setup_server(cls):
         mgr.list_servers.return_value = mock_list_servers
-        mgr.get_metadata.return_value = mock_get_metadata
+        mgr.get_metadata = mock.Mock(return_value=mock_get_metadata)
         mgr.get_daemon_status.return_value = mock_get_daemon_status
         mgr.get.side_effect = lambda key: {
             'osd_map': mock_osd_map,