mds/MDSAuthCaps: allow adding multiple caps via "fs auth" cmd

When multiple caps are passed to "ceph fs authorize" command, the
command hangs because the MON crashes due to this command. Underlying
code assumes that only one cap can be passed at a time by the user
through the "ceph fs authorize" command. "ceph_assert()" checks if only
one cap is passed. If not, the code crashes.

Fixes: https://tracker.ceph.com/issues/64127
Signed-off-by: Rishabh Dave <[email protected]>

Author: rishabh-d-dave

qa/tasks/cephfs/test_admin.py

@@ -1480,6 +1480,26 @@ def test_when_MDS_caps_needs_update_but_others_dont(self):
             FS_AUTH_CAPS[2], self.captesters[2], self.fs2.name, self.mount_b,
             keyring)
 
+    def test_adding_multiple_caps(self):
+        '''
+        Test that the command "ceph fs authorize" is successful in updating
+        the entity's caps when multiple caps are passed to it in one go.
+
+        Tests: https://tracker.ceph.com/issues/64127
+        '''
+        DIR = 'dir1'
+        self.mount_a.run_shell(f'mkdir {DIR}')
+        self.captesters = (CapTester(self.mount_a, '/'),
+                           CapTester(self.mount_a, f'/{DIR}'))
+        self.fs.authorize(self.client_id, ('/', 'rw'))
+
+        FS_AUTH_CAPS = ('/', 'rw', 'root_squash'), (f'/{DIR}', 'rw' )
+        # The fact that following line passes means
+        # https://tracker.ceph.com/issues/64127 has been fixed
+        keyring = self.fs.authorize(self.client_id, FS_AUTH_CAPS)
+
+        self._remount_and_run_tests(FS_AUTH_CAPS, keyring)
+
     def _remount_and_run_tests(self, fs_auth_caps, keyring):
         '''
         This method is specifically designed to meet needs of most of the

src/mds/MDSAuthCaps.cc

@@ -382,25 +382,28 @@ bool MDSAuthCaps::parse(string_view str, ostream *err)
   }
 }
 
-bool MDSAuthCaps::merge(MDSAuthCaps newcap)
+/* Check if the "cap grant" is already present in this cap object. If it is,
+ * return false. If not, add it and return true.
+ *
+ * ng = new grant, new mds cap grant.
+ */
+bool MDSAuthCaps::merge_one_cap_grant(MDSCapGrant ng)
 {
-  ceph_assert(newcap.grants.size() == 1);
-  auto ng = newcap.grants[0];
-
+  // 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.
 	return false;
        } else {
-	// cap for given fs name is present, let's update it.
+	// "ng" is present but perm/spec is different. update it.
 	g.spec.set_caps(ng.spec.get_caps());
 	return true;
       }
     }
   }
 
-  // cap for given fs name and/or path is absent, let's add a new cap for it.
+  // since "ng" is absent in this cap object, add it.
   grants.push_back(MDSCapGrant(
     MDSCapSpec(ng.spec.get_caps()),
     MDSCapMatch(ng.match.fs_name, ng.match.path, ng.match.root_squash),
@@ -409,6 +412,29 @@ bool MDSAuthCaps::merge(MDSAuthCaps newcap)
   return true;
 }
 
+/* User can pass one or MDS caps that it wishes to add to entity's keyring.
+ * Merge all of these caps one by one. Return value indicates whether or not
+ * AuthMonitor must update the entity's keyring.
+ *
+ * If all caps do not merge (that is, underlying helper method returns false
+ * after attempting merge), no update is required. Return false so that
+ * AuthMonitor doesn't run the update procedure for caps.
+ *
+ * If even one cap is merged (that is, underlying method returns true even
+ * once), an update to the entity's keyring is required. Return true so that
+ * AuthMonitor runs the update procedure.
+ */
+bool MDSAuthCaps::merge(MDSAuthCaps newcaps)
+{
+  bool were_caps_merged = false;
+
+  for (auto& ng : newcaps.grants) {
+      were_caps_merged |= merge_one_cap_grant(ng);
+  }
+
+  return were_caps_merged;
+}
+
 string MDSCapMatch::to_string()
 {
   string str = "";

src/mds/MDSAuthCaps.h

@@ -259,7 +259,8 @@ class MDSAuthCaps
 
   void set_allow_all();
   bool parse(std::string_view str, std::ostream *err);
-  bool merge(MDSAuthCaps newcap);
+  bool merge_one_cap_grant(MDSCapGrant ng);
+  bool merge(MDSAuthCaps newcaps);
 
   bool allow_all() const;
   bool is_capable(std::string_view inode_path,