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>

diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py
index 98548df8cd3cdd4beb4bec9f3a16f53aff5b3b26..37895a379f2fbb4fe36931abafa7a45dd93ca344 100644 (file)
--- a/qa/tasks/cephfs/test_admin.py
+++ b/qa/tasks/cephfs/test_admin.py
@@ -1486,6 +1486,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}')
@@ -1498,6 +1499,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),