mds/MDSAuthCaps: allow updating root_squash in entity's MDS cap

Command "ceph fs authorize" can update fsname, path and perm/spec in a
MDS cap but it can't update "root_squash" in it. Add code to allow this.

Specifically, "MDSAuthCaps::merge_one_cap_grant()" doesn't check if
the value for "root_squash" needs an update while attempting to merge
the new MDS cap into the list of MDS caps already held by an entity. It
should do so to ensure correct behaviour.

Fixes: https://tracker.ceph.com/issues/64417
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit f6bb5797c1ed3534355ffc6a12dc5f3b08d99f58)

diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py
index 6c8e669376e1ad71ab9cf67dcabece932ed6edcc..4a73e60bca81b929b8e59f7afc0dec0ccbde2420 100644 (file)
--- a/qa/tasks/cephfs/test_admin.py
+++ b/qa/tasks/cephfs/test_admin.py
@@ -1859,6 +1859,7 @@ class TestFsAuthorize(CephFSTestCase):
         the entity's caps when multiple caps are passed to it in one go.
 
         Tests: https://tracker.ceph.com/issues/64127
+        Tests: https://tracker.ceph.com/issues/64417
         '''
         DIR = 'dir1'
         self.mount_a.run_shell(f'mkdir {DIR}')
@@ -1871,6 +1872,12 @@ class TestFsAuthorize(CephFSTestCase):
         # https://tracker.ceph.com/issues/64127 has been fixed
         keyring = self.fs.authorize(self.client_id, FS_AUTH_CAPS)
 
+        # The fact that following lines passes means
+        # https://tracker.ceph.com/issues/64417 has been fixed.
+        mdscaps = (f'allow rw fsname={self.fs.name} root_squash, '
+                   f'allow rw fsname={self.fs.name} path={DIR}')
+        self.assertIn(mdscaps, keyring)
+
         self._remount_and_run_tests(FS_AUTH_CAPS, keyring)
 
     def _remount_and_run_tests(self, fs_auth_caps, keyring):

diff --git a/src/mds/MDSAuthCaps.cc b/src/mds/MDSAuthCaps.cc
index c65c7f86e2ea0f45a049d18f08e44780c81c14c5..5e4bd995175c263c5ded004a2d2594b3592ebe49 100644 (file)
--- a/src/mds/MDSAuthCaps.cc
+++ b/src/mds/MDSAuthCaps.cc
@@ -392,18 +392,36 @@ bool MDSAuthCaps::merge_one_cap_grant(MDSCapGrant ng)
   // check if "ng" is already present in this cap object.
   for (auto& g : grants) {
     if (g.match.fs_name == ng.match.fs_name && g.match.path == ng.match.path) {
-      if (g.spec.get_caps() == ng.spec.get_caps()) {
-       // no update required. maintaining idempotency.
+      if (g.spec.get_caps() == ng.spec.get_caps() &&
+         g.match.root_squash == ng.match.root_squash) {
+       // Since all components of MDS caps (fsname, path, perm/spec and
+       // root_squash) matched, it means cap same as "ng" is present in MDS
+       // cap grant list. No need to look further in MDS cap grant list.
+       // No update is required. Maintain idempotency.
        return false;
-       } else {
-       // "ng" is present but perm/spec is different. update it.
+       }
+
+      // fsname and path match but perm/spec is different. update the cap
+      // with new perm/spec.
+      if (g.spec.get_caps() != ng.spec.get_caps()) {
        g.spec.set_caps(ng.spec.get_caps());
-       return true;
       }
+
+      // fsname and path match but value of root_squash is different. update
+      // its value.
+      if (g.match.root_squash != ng.match.root_squash) {
+       g.match.root_squash = ng.match.root_squash;
+      }
+
+      // Since fsname and path matched and either perm/spec or root_squash
+      // or both has been updated, cap from "ng" has been incorporated
+      // into this cap grant list. Time to return.
+      return true;
     }
   }
 
-  // since "ng" is absent in this cap object, add it.
+  // Since a cap grant like "ng" is absent in this cap object's grant list,
+  // add "ng" to the cap grant list.
   grants.push_back(MDSCapGrant(
     MDSCapSpec(ng.spec.get_caps()),
     MDSCapMatch(ng.match.fs_name, ng.match.path, ng.match.root_squash),