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 <ridave@redhat.com>

diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py
index c92ef8621f479292aade6ccf0aa5b593c8e49b77..4a18faa273178d5ac4e853e996d76378c5142c14 100644 (file)
--- 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 d1ae65713353893d0af0210c4c90e571e9618156..9b2bf70d23c8a9833812ad4d3178e1ebe68095ba 100644 (file)
--- 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<string, string>& 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<string, string>& caps, MonOpRequestRef op, stringstream& ds,
-  bufferlist* rdata, Formatter* fmtr, bool create_entity)
+  const map<string, string>& 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<string, string>& caps, MonOpRequestRef op, stringstream& ds,
-  bufferlist* rdata, Formatter* fmtr)
+  const map<string, string>& 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<string, string>& caps, MonOpRequestRef op, stringstream& ds,
-  bufferlist* rdata, Formatter* fmtr)
+  const map<string, string>& 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 b48ab914e9b60d7bab8d0c98059a98bb75653122..3a88edb873d2812f52c86482dacfb45f738f654f 100644 (file)
--- 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<std::string, std::string>& 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<std::string, std::string>& 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<std::string, std::string>& 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<std::string, std::string>& newcaps, std::ostream& out);