Merge PR ceph#58878 into main

* refs/pull/58878/head:
	client: Fix leading / issue with mds_check_access
	qa: Add mds caps test for testing fs read and a path rw

Reviewed-by: Venky Shankar <[email protected]>
Reviewed-by: Xiubo Li <[email protected]>
Reviewed-by: Patrick Donnelly <[email protected]>

Author: vshankar

qa/tasks/cephfs/caps_helper.py

@@ -204,11 +204,14 @@ def _create_test_files(self, mount, path):
         """
         # CephFS mount where read/write test will be conducted.
         self.mount = mount
+        # Set new file creation path
+        self.new_file = os_path_join(self.mount.hostfs_mntpt, path.lstrip('/'), 'new_file')
         # Path where out test file located.
         self.path = self._gen_test_file_path(path)
         # Data that out test file will contain.
         self.data = self._gen_test_file_data()
 
+
         self.mount.write_file(self.path, self.data)
         log.info(f'Test file has been created on FS '
                  f'"{self.mount.cephfs_name}" at path "{self.path}" with the '
@@ -242,7 +245,8 @@ def _gen_test_file_data(self):
             self.path = {self.path}
             cephfs_name = {self.mount.cephfs_name}
             cephfs_mntpt = {self.mount.cephfs_mntpt}
-            hostfs_mntpt = {self.mount.hostfs_mntpt}''')
+            hostfs_mntpt = {self.mount.hostfs_mntpt}
+            self.new_file_path = {self.new_file}''')
 
     def run_mds_cap_tests(self, perm, mntpt=None):
         """
@@ -260,11 +264,13 @@ def run_mds_cap_tests(self, perm, mntpt=None):
             #   cephfs dir serving as root for current mnt: /dir1/dir2
             #   therefore, final path: /mnt/cephfs_x/testdir
             self.path = self.path.replace(mntpt, '')
+            self.new_file = self.new_file.replace(mntpt, '')
 
         self.conduct_pos_test_for_read_caps()
 
         if perm == 'rw':
             self.conduct_pos_test_for_write_caps()
+            self.conduct_pos_test_for_new_file_creation()
         elif perm == 'r':
             self.conduct_neg_test_for_write_caps()
         else:
@@ -302,6 +308,12 @@ def conduct_neg_test_for_write_caps(self, sudo_write=False):
         log.info('absence of write perm was tested successfully: '
                  f'failed to be write data to file {self.path}.')
 
+    def conduct_pos_test_for_new_file_creation(self, sudo_write=False):
+        log.info(f'test write perm: try creating a new "{self.new_file}"')
+        self.mount.create_file(self.new_file)
+        log.info(f'write perm was tested successfully: new file "{self.new_file}" '
+                  'created successfully')
+
 
 class CapTester(MonCapTester, MdsCapTester):
     '''

qa/tasks/cephfs/test_admin.py

@@ -1818,6 +1818,28 @@ def test_single_path_authorize_on_nonalphanumeric_fsname(self):
         self._remount(keyring)
         self.captester.run_mds_cap_tests(PERM)
 
+    def test_fs_read_and_single_path_rw(self):
+        """
+        Tests the file creation using 'touch' cmd on a specific path
+        which has 'rw' caps and 'r' caps on the rest of the fs.
+
+        The mds auth caps with 'rw' caps on a specific path and 'r' caps
+        on the rest of the fs has an issue. The file creation using 'touch'
+        cmd on the fuse client used to fail while doing setattr.
+        Please see https://tracker.ceph.com/issues/67212
+
+        The new file creation test using 'touch' cmd is added to
+        'MdsCapTester.run_mds_cap_tests' which eventually gets
+        called by '_remount_and_run_tests'
+        """
+        FS_AUTH_CAPS = (('/', 'r'), ('/dir2', 'rw'))
+        self.mount_a.run_shell('mkdir -p ./dir2')
+        self.captesters = (CapTester(self.mount_a, '/'),
+                           CapTester(self.mount_a, '/dir2'))
+        keyring = self.fs.authorize(self.client_id, FS_AUTH_CAPS)
+
+        self._remount_and_run_tests(FS_AUTH_CAPS, keyring)
+
     def test_multiple_path_r(self):
         PERM = 'r'
         FS_AUTH_CAPS = (('/dir1/dir12', PERM), ('/dir2/dir22', PERM))

src/client/Client.cc

@@ -5933,6 +5933,11 @@ int Client::mds_check_access(std::string& path, const UserPerm& perms, int mask)
     }
   }
 
+  // drop any leading /
+  while (path.length() && path[0] == '/') {
+    path = path.substr(1);
+  }
+
   for (auto& s: cap_auths) {
     ldout(cct, 20) << __func__ << " auth match path " << s.match.path << " r: " << s.readable
                    << " w: " << s.writeable << dendl;
@@ -8039,6 +8044,10 @@ bool Client::make_absolute_path_string(Inode *in, std::string& path)
     return false;
   }
 
+  // Make sure this function returns path with single leading '/'
+  if (path.length() && path[0] == '/' && path[1] == '/')
+    path = path.substr(1);
+
   return true;
 }
 
@@ -8086,8 +8095,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask,
     std::string path;
     if (make_absolute_path_string(in, path)) {
       ldout(cct, 20) << " absolute path: " << path << dendl;
-      if (path.length())
-        path = path.substr(1);    // drop leading /
       res = mds_check_access(path, perms, MAY_WRITE);
       if (res) {
         goto out;
@@ -10329,8 +10336,6 @@ int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp,
     std::string path;
     if (make_absolute_path_string(in, path)) {
       ldout(cct, 20) << __func__ << " absolute path: " << path << dendl;
-      if (path.length())
-        path = path.substr(1);    // drop leading /
       result = mds_check_access(path, perms, mask);
       if (result) {
         return result;