From f6bb5797c1ed3534355ffc6a12dc5f3b08d99f58 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Tue, 13 Feb 2024 17:16:00 +0530 Subject: [PATCH] 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 --- qa/tasks/cephfs/test_admin.py | 7 +++++++ src/mds/MDSAuthCaps.cc | 30 ++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py index 98548df8cd3..37895a379f2 100644 --- 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 c65c7f86e2e..5e4bd995175 100644 --- 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), -- 2.39.5