]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
client: hide alternate_name from API
authorPatrick Donnelly <pdonnell@redhat.com>
Tue, 10 Sep 2024 20:47:39 +0000 (16:47 -0400)
committerPatrick Donnelly <pdonnell@ibm.com>
Thu, 27 Feb 2025 18:41:54 +0000 (13:41 -0500)
The dentry alternate_name was exposed primarily to facilitate testing. Instead,
put these methods in the TestClient scaffolding to allow reading/manipulating
alternate_name.

Because we will be using alternate_name to handle encrypted names and the
normalized / case folded name, we do not want the application to change the
meaning of the metadata out-of-band.

Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>
Fixes: https://tracker.ceph.com/issues/66373
src/client/Client.cc
src/client/Client.h
src/test/client/TestClient.h
src/test/client/alternate_name.cc

index 17ade3a88285d69e5798931990f48365eacc7262..983ba70f414f9872893ae94195a0bbd3237345d9 100644 (file)
@@ -7603,7 +7603,7 @@ int Client::path_walk(const filepath& origpath, walk_dentry_result* result, cons
 
 // namespace ops
 
-int Client::link(const char *relexisting, const char *relpath, const UserPerm& perm, std::string alternate_name)
+int Client::do_link(const char *relexisting, const char *relpath, const UserPerm& perm, std::string alternate_name)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
   if (!mref_reader.is_state_satisfied())
@@ -7700,7 +7700,7 @@ int Client::unlinkat(int dirfd, const char *relpath, int flags, const UserPerm&
   return r;
 }
 
-int Client::rename(const char *relfrom, const char *relto, const UserPerm& perm, std::string alternate_name)
+int Client::do_rename(const char *relfrom, const char *relto, const UserPerm& perm, std::string alternate_name)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
   if (!mref_reader.is_state_satisfied())
@@ -7745,12 +7745,7 @@ out:
 
 // dirs
 
-int Client::mkdir(const char *relpath, mode_t mode, const UserPerm& perm, std::string alternate_name)
-{
-  return mkdirat(CEPHFS_AT_FDCWD, relpath, mode, perm, alternate_name);
-}
-
-int Client::mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm,
+int Client::do_mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm,
                     std::string alternate_name)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
@@ -7888,12 +7883,7 @@ int Client::mknod(const char *relpath, mode_t mode, const UserPerm& perms, dev_t
 
 // symlinks
   
-int Client::symlink(const char *target, const char *relpath, const UserPerm& perms, std::string alternate_name)
-{
-  return symlinkat(target, CEPHFS_AT_FDCWD, relpath, perms, alternate_name);
-}
-
-int Client::symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms,
+int Client::do_symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms,
                       std::string alternate_name)
 {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
@@ -10114,15 +10104,7 @@ int Client::create_and_open(int dirfd, const char *relpath, int flags,
   return r;
 }
 
-int Client::open(const char *relpath, int flags, const UserPerm& perms,
-                mode_t mode, int stripe_unit, int stripe_count,
-                int object_size, const char *data_pool, std::string alternate_name)
-{
-  return openat(CEPHFS_AT_FDCWD, relpath, flags, perms, mode, stripe_unit,
-                stripe_count, object_size, data_pool, std::move(alternate_name));
-}
-
-int Client::openat(int dirfd, const char *relpath, int flags, const UserPerm& perms,
+int Client::do_openat(int dirfd, const char *relpath, int flags, const UserPerm& perms,
                    mode_t mode, int stripe_unit, int stripe_count, int object_size,
                    const char *data_pool, std::string alternate_name) {
   RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
index c731def9b61a9be7b4c138611ddf3627721226fe..84c09e9250e80a816c92046285c4db56390a7dd1 100644 (file)
@@ -394,15 +394,22 @@ public:
   void seekdir(dir_result_t *dirp, loff_t offset);
 
   int may_delete(const char *relpath, const UserPerm& perms);
-  int link(const char *existing, const char *newname, const UserPerm& perm, std::string alternate_name="");
+  int link(const char *oldpath, const char *newpath, const UserPerm& perm) {
+    return do_link(oldpath, newpath, perm);
+  }
   int unlink(const char *path, const UserPerm& perm);
   int unlinkat(int dirfd, const char *relpath, int flags, const UserPerm& perm);
-  int rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name="");
+  int rename(const char *from, const char *to, const UserPerm& perm) {
+    return do_rename(from, to, perm);
+  }
 
   // dirs
-  int mkdir(const char *path, mode_t mode, const UserPerm& perm, std::string alternate_name="");
-  int mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm,
-              std::string alternate_name="");
+  int mkdir(const char *path, mode_t mode, const UserPerm& perm) {
+    return mkdirat(CEPHFS_AT_FDCWD, path, mode, perm);
+  }
+  int mkdirat(int dirfd, const char *path, mode_t mode, const UserPerm& perm) {
+    return do_mkdirat(dirfd, path, mode, perm);
+  }
   int mkdirs(const char *path, mode_t mode, const UserPerm& perms);
   int rmdir(const char *path, const UserPerm& perms);
 
@@ -410,12 +417,12 @@ public:
   int readlink(const char *path, char *buf, loff_t size, const UserPerm& perms);
   int readlinkat(int dirfd, const char *relpath, char *buf, loff_t size, const UserPerm& perms);
 
-  int symlink(const char *existing, const char *newname, const UserPerm& perms, std::string alternate_name="");
-  int symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms,
-                std::string alternate_name="");
-
-  // path traversal for high-level interface
-  int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true);
+  int symlink(const char *target, const char *linkpath, const UserPerm& perms) {
+    return symlinkat(target, CEPHFS_AT_FDCWD, linkpath, perms);
+  }
+  int symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms) {
+    return do_symlinkat(target, dirfd, linkpath, perms);
+  }
 
   // inode stuff
   unsigned statx_to_mask(unsigned int flags, unsigned int want);
@@ -459,21 +466,21 @@ public:
   // file ops
   int mknod(const char *path, mode_t mode, const UserPerm& perms, dev_t rdev=0);
 
-  int create_and_open(int dirfd, const char *relpath, int flags, const UserPerm& perms,
-                      mode_t mode, int stripe_unit, int stripe_count, int object_size,
-                      const char *data_pool, std::string alternate_name);
-  int open(const char *path, int flags, const UserPerm& perms, mode_t mode=0, std::string alternate_name="") {
-    return open(path, flags, perms, mode, 0, 0, 0, NULL, alternate_name);
+  int open(const char *path, int flags, const UserPerm& perms, mode_t mode=0) {
+    return open(path, flags, perms, mode, 0, 0, 0, NULL);
   }
   int open(const char *path, int flags, const UserPerm& perms,
           mode_t mode, int stripe_unit, int stripe_count, int object_size,
-          const char *data_pool, std::string alternate_name="");
-  int openat(int dirfd, const char *relpath, int flags, const UserPerm& perms,
+          const char *data_pool) {
+    return openat(CEPHFS_AT_FDCWD, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool);
+  }
+  int openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode=0) {
+    return openat(dirfd, path, flags, perms, mode, 0, 0, 0, NULL);
+  }
+  int openat(int dirfd, const char *path, int flags, const UserPerm& perms,
              mode_t mode, int stripe_unit, int stripe_count,
-             int object_size, const char *data_pool, std::string alternate_name);
-  int openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode=0,
-             std::string alternate_name="") {
-    return openat(dirfd, path, flags, perms, mode, 0, 0, 0, NULL, alternate_name);
+             int object_size, const char *data_pool) {
+    return do_openat(dirfd, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool);
   }
 
   int lookup_hash(inodeno_t ino, inodeno_t dirino, const char *name,
@@ -988,6 +995,9 @@ protected:
   void dump_mds_requests(Formatter *f);
   void dump_mds_sessions(Formatter *f, bool cap_dump=false);
 
+  // path traversal for testing, looking at walk_dentry_result
+  int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true);
+
   int make_request(MetaRequest *req, const UserPerm& perms,
                    InodeRef *ptarget = 0, bool *pcreated = 0,
                    mds_rank_t use_mds=-1, bufferlist *pdirbl=0,
@@ -1016,6 +1026,16 @@ protected:
   void handle_client_reply(const MConstRef<MClientReply>& reply);
   bool is_dir_operation(MetaRequest *request);
 
+  int create_and_open(int dirfd, const char *relpath, int flags, const UserPerm& perms,
+                      mode_t mode, int stripe_unit, int stripe_count, int object_size,
+                      const char *data_pool, std::string alternate_name);
+
+  int do_mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm, std::string alternate_name="");
+  int do_rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name="");
+  int do_link(const char *existing, const char *newname, const UserPerm& perm, std::string alternate_name="");
+  int do_symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms, std::string alternate_name="");
+  int do_openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode, int stripe_unit, int stripe_count, int object_size, const char *data_pool, std::string alternate_name="");
+
   int path_walk(const filepath& fp, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true, int mask=0,
                 InodeRef dirinode=nullptr);
   int path_walk(const filepath& fp, InodeRef *end, const UserPerm& perms,
index 67cadc7baafcba60097b7ca652bc5072b0b21e36..7de90373d5d6a573a71fd4c2ebd9bfef7e58a831 100644 (file)
@@ -34,6 +34,8 @@ namespace ca = ceph::async;
 
 class ClientScaffold : public Client {  
 public:
+    using Client::walk_dentry_result;
+
     ClientScaffold(Messenger *m, MonClient *mc, Objecter *objecter_) : Client(m, mc, objecter_) {}
     virtual ~ClientScaffold()
     { }
@@ -87,6 +89,31 @@ public:
       wait_on_list(waiting_for_reclaim);
       return session->reclaim_state == MetaSession::RECLAIM_FAIL ? true : false;
     }
+
+    /* Expose alternate_name for testing. There is no need to use virtual
+     * methods as we will call these only from the Derived class.
+     */
+    int symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms, std::string alternate_name) {
+      return do_symlinkat(target, dirfd, linkpath, perms, std::move(alternate_name));
+    }
+    int mkdirat(int dirfd, const char *path, mode_t mode, const UserPerm& perm, std::string alternate_name) {
+      return do_mkdirat(CEPHFS_AT_FDCWD, path, mode, perm, std::move(alternate_name));
+    }
+    int rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name) {
+      return do_rename(from, to, perm, std::move(alternate_name));
+    }
+    int link(const char *oldpath, const char *newpath, const UserPerm& perm, std::string alternate_name) {
+      return do_link(oldpath, newpath, perm, std::move(alternate_name));
+    }
+    int openat(int dirfd, const char *path, int flags, const UserPerm& perms,
+               mode_t mode, int stripe_unit, int stripe_count,
+               int object_size, const char *data_pool, std::string alternate_name) {
+      return do_openat(dirfd, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool, std::move(alternate_name));
+    }
+
+    int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true) {
+      return Client::walk(path, result, perms, followsym);
+    }
 };
 
 class TestClient : public ::testing::Test {
@@ -141,6 +168,7 @@ public:
       delete messenger;
       messenger = nullptr;
     }
+    // TODO expose altname versions
 protected:
     static inline ceph::async::io_context_pool icp;
     static inline UserPerm myperm{0,0};
index 43f4280120ca664527951e3f591c67fed28802a8..57c29a44c48379f2062b269a52a4f66939cb3f37 100644 (file)
@@ -24,7 +24,7 @@
 TEST_F(TestClient, AlternateNameRemount) {
   auto altname = std::string("foo");
   auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, altname));
+  ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, altname));
 
   client->unmount();
   TearDown();
@@ -32,7 +32,7 @@ TEST_F(TestClient, AlternateNameRemount) {
   client->mount("/", myperm, true);
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname);
   }
@@ -43,10 +43,10 @@ TEST_F(TestClient, AlternateNameRemount) {
 
 TEST_F(TestClient, AlternateNameMkdir) {
   auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, "foo"));
+  ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, "foo"));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, "foo");
   }
@@ -57,10 +57,10 @@ TEST_F(TestClient, AlternateNameMkdir) {
 TEST_F(TestClient, AlternateNameLong) {
   auto altname = std::string(4096+1024, '-');
   auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, altname));
+  ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, altname));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname);
   }
@@ -71,13 +71,13 @@ TEST_F(TestClient, AlternateNameLong) {
 TEST_F(TestClient, AlternateNameCreat) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file, &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname);
   }
@@ -86,17 +86,17 @@ TEST_F(TestClient, AlternateNameCreat) {
 TEST_F(TestClient, AlternateNameSymlink) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
 
   auto file2 = file+"2";
   auto altname2 = altname+"2";
-  ASSERT_EQ(0, client->symlink(file.c_str(), file2.c_str(), myperm, altname2));
+  ASSERT_EQ(0, client->symlinkat(file.c_str(), CEPHFS_AT_FDCWD, file2.c_str(), myperm, altname2));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file2, &wdr, myperm, false));
     ASSERT_EQ(wdr.alternate_name, altname2);
     ASSERT_EQ(0, client->walk(file, &wdr, myperm));
@@ -107,7 +107,7 @@ TEST_F(TestClient, AlternateNameSymlink) {
 TEST_F(TestClient, AlternateNameRename) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -118,7 +118,7 @@ TEST_F(TestClient, AlternateNameRename) {
   ASSERT_EQ(0, client->rename(file.c_str(), file2.c_str(), myperm, altname2));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file2, &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname2);
   }
@@ -127,7 +127,7 @@ TEST_F(TestClient, AlternateNameRename) {
 TEST_F(TestClient, AlternateNameRenameExistMatch) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -135,7 +135,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) {
   auto file2 = file+"2";
   auto altname2 = altname+"2";
 
-  fd = client->open(file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname2);
+  fd = client->openat(CEPHFS_AT_FDCWD, file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname2);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -143,7 +143,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) {
   ASSERT_EQ(0, client->rename(file.c_str(), file2.c_str(), myperm, altname2));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file2, &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname2);
   }
@@ -152,7 +152,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) {
 TEST_F(TestClient, AlternateNameRenameExistMisMatch) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -160,7 +160,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) {
   auto file2 = file+"2";
   auto altname2 = altname+"2";
 
-  fd = client->open(file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname+"mismatch");
+  fd = client->openat(CEPHFS_AT_FDCWD, file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname+"mismatch");
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -168,7 +168,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) {
   ASSERT_EQ(-EINVAL, client->rename(file.c_str(), file2.c_str(), myperm, altname2));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file2, &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname+"mismatch");
   }
@@ -177,7 +177,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) {
 TEST_F(TestClient, AlternateNameLink) {
   auto altname = std::string("foo");
   auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid());
-  int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname);
+  int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname);
   ASSERT_LE(0, fd);
   ASSERT_EQ(3, client->write(fd, "baz", 3, 0));
   ASSERT_EQ(0, client->close(fd));
@@ -188,7 +188,7 @@ TEST_F(TestClient, AlternateNameLink) {
   ASSERT_EQ(0, client->link(file.c_str(), file2.c_str(), myperm, altname2));
 
   {
-    Client::walk_dentry_result wdr;
+    ClientScaffold::walk_dentry_result wdr;
     ASSERT_EQ(0, client->walk(file2, &wdr, myperm));
     ASSERT_EQ(wdr.alternate_name, altname2);
     ASSERT_EQ(0, client->walk(file, &wdr, myperm));