From e404413e89dcc02b13ce0c1a67dbe3b03f8ce346 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Tue, 12 Sep 2023 01:31:37 +0530 Subject: [PATCH] mon/AuthMonitor: make "ceph auth caps" print error messsages... generated by MDSAuthCaps::parse(). Specifically, in this case, this command is supposed to print an error message when permission flag passed for MDS capability is incorrect. This needs the method AuthMonitor::_update_or_create_entity() to be fixed. The issue is that stderr stream is not passed from the block under "else if" clause of "ceph auth caps" command to MDSAuthCaps::_parse() method. Let's fix this by adding a parameter for stderr stream to every method in stack that is between this block and MDSAuthCaps::parse() method. qa.tasks.cephfs.test_admin.TestPermErrMsg contains tests that checks whether or not all the Ceph commands that accept MDS capabilites print the error message when permission flag in this capaibility is incorrect. Add a test to this class to check if "ceph auth caps" also print an error message when perm flag in MDS capability sting is incorrect. Fixes: https://tracker.ceph.com/issues/63020 Signed-off-by: Rishabh Dave --- qa/tasks/cephfs/test_admin.py | 10 ++++++++++ src/mon/AuthMonitor.cc | 23 +++++++++++------------ src/mon/AuthMonitor.h | 10 ++++++---- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py index c92ef8621f479..4a18faa273178 100644 --- a/qa/tasks/cephfs/test_admin.py +++ b/qa/tasks/cephfs/test_admin.py @@ -1849,6 +1849,16 @@ class TestPermErrMsg(CephFSTestCase): for mdscap in self.MDSCAPS: self._negtestcmd('auth add', mdscap) + def test_auth_caps(self): + for mdscap in self.MDSCAPS: + self.fs.mon_manager.run_cluster_cmd( + args=f'auth add {self.CLIENT_NAME}') + + self._negtestcmd('auth caps', mdscap) + + self.fs.mon_manager.run_cluster_cmd( + args=f'auth rm {self.CLIENT_NAME}') + def test_auth_get_or_create(self): for mdscap in self.MDSCAPS: self._negtestcmd('auth get-or-create', mdscap) diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc index d1ae657133538..9b2bf70d23c8a 100644 --- a/src/mon/AuthMonitor.cc +++ b/src/mon/AuthMonitor.cc @@ -1801,7 +1801,7 @@ bool AuthMonitor::prepare_command(MonOpRequestRef op) dout(20) << it.first << " cap = \"" << it.second << "\"" << dendl; } - err = _update_caps(entity, newcaps, op, ds, &rdata, f.get()); + err = _update_caps(entity, newcaps, op, ss, ds, &rdata, f.get()); if (err == 0) { return true; } else { @@ -1809,14 +1809,14 @@ bool AuthMonitor::prepare_command(MonOpRequestRef op) } } - err = _create_entity(entity, newcaps, op, ds, &rdata, f.get()); + err = _create_entity(entity, newcaps, op, ss, ds, &rdata, f.get()); if (err == 0) { return true; } else { goto done; } } else if (prefix == "auth caps" && !entity_name.empty()) { - err = _update_caps(entity, ceph_caps, op, ds, &rdata, f.get()); + err = _update_caps(entity, ceph_caps, op, ss, ds, &rdata, f.get()); if (err == 0) { return true; } else { @@ -1976,10 +1976,9 @@ int AuthMonitor::_check_and_encode_caps(const map& caps, * update and set create to True to allow authorizing a new entity instead * of updating its caps. */ int AuthMonitor::_update_or_create_entity(const EntityName& entity, - const map& caps, MonOpRequestRef op, stringstream& ds, - bufferlist* rdata, Formatter* fmtr, bool create_entity) + const map& caps, MonOpRequestRef op, stringstream& ss, + stringstream& ds, bufferlist* rdata, Formatter* fmtr, bool create_entity) { - stringstream ss; KeyServerData::Incremental auth_inc; auth_inc.name = entity; @@ -2019,18 +2018,18 @@ int AuthMonitor::_update_or_create_entity(const EntityName& entity, } int AuthMonitor::_update_caps(const EntityName& entity, - const map& caps, MonOpRequestRef op, stringstream& ds, - bufferlist* rdata, Formatter* fmtr) + const map& caps, MonOpRequestRef op, stringstream& ss, + stringstream& ds, bufferlist* rdata, Formatter* fmtr) { - return _update_or_create_entity(entity, caps, op, ds, rdata, fmtr, + return _update_or_create_entity(entity, caps, op, ss, ds, rdata, fmtr, false); } int AuthMonitor::_create_entity(const EntityName& entity, - const map& caps, MonOpRequestRef op, stringstream& ds, - bufferlist* rdata, Formatter* fmtr) + const map& caps, MonOpRequestRef op, stringstream& ss, + stringstream& ds, bufferlist* rdata, Formatter* fmtr) { - return _update_or_create_entity(entity, caps, op, ds, rdata, fmtr, + return _update_or_create_entity(entity, caps, op, ss, ds, rdata, fmtr, true); } diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h index b48ab914e9b60..3a88edb873d28 100644 --- a/src/mon/AuthMonitor.h +++ b/src/mon/AuthMonitor.h @@ -191,14 +191,16 @@ private: int _update_or_create_entity(const EntityName& entity, const std::map& caps, MonOpRequestRef op, - std::stringstream& ds, bufferlist* rdata=nullptr, Formatter* fmtr=nullptr, - bool create_entity=false); + std::stringstream& ss, std::stringstream& ds, bufferlist* rdata=nullptr, + Formatter* fmtr=nullptr, bool create_entity=false); int _create_entity(const EntityName& entity, const std::map& caps, MonOpRequestRef op, - std::stringstream& ds, bufferlist* rdata, Formatter* fmtr); + std::stringstream& ss, std::stringstream& ds, bufferlist* rdata, + Formatter* fmtr); int _update_caps(const EntityName& entity, const std::map& caps, MonOpRequestRef op, - std::stringstream& ds, bufferlist* rdata, Formatter* fmtr); + std::stringstream& ss, std::stringstream& ds, bufferlist* rdata, + Formatter* fmtr); caps_update _gen_wanted_caps(EntityAuth& e_auth, std::map& newcaps, std::ostream& out); -- 2.39.5