Merge remote-tracking branch 'gh/wip-osd-caps'

Reviewed-by: Sage Weil <sage@inktank.com>

3 files changed, 82 insertions, 17 deletions

diff --git a/src/osd/OSDCap.cc b/src/osd/OSDCap.cc
index 0687a3c0aab..f2ee2efc7e6 100644
--- a/src/osd/OSDCap.cc
+++ b/src/osd/OSDCap.cc
@@ -26,7 +26,7 @@ using std::vector;
 
 ostream& operator<<(ostream& out, rwxa_t p)
 { 
-  if (p & OSD_CAP_ANY)
+  if (p == OSD_CAP_ANY)
     return out << "*";
 
   if (p & OSD_CAP_R)
@@ -40,20 +40,11 @@ ostream& operator<<(ostream& out, rwxa_t p)
 
 ostream& operator<<(ostream& out, const OSDCapSpec& s)
 {
-  if (s.allow & OSD_CAP_ANY)
-    return out << "*";
-  if (s.allow) {
-    if (s.allow & OSD_CAP_R)
-      out << 'r';
-    if (s.allow & OSD_CAP_W)
-      out << 'w';
-    if (s.allow & OSD_CAP_X)
-      out << 'x';
-    return out;
-  }
+  if (s.allow)
+    return out << s.allow;
   if (s.class_name.length())
     return out << "class '" << s.class_name << "' '" << s.class_allow << "'";
-  return out << s.allow;
+  return out;
 }
 
 ostream& operator<<(ostream& out, const OSDCapMatch& m)
@@ -154,7 +145,7 @@ struct OSDCapParser : qi::grammar<Iterator, OSDCap(), ascii::space_type>
     unquoted_word %= +(alnum | '_' | '-');
     str %= quoted_string | unquoted_word;
 
-    // match := [pool <poolname> | uid <123>] [object_prefix <prefix>]
+    // match := [pool <poolname> | auid <123>] [object_prefix <prefix>]
     pool_name %= -(lit("pool") >> str);
     object_prefix %= -(lit("object_prefix") >> str);
     match = ( (lit("auid") >> int_ >> object_prefix)   [_val = phoenix::construct<OSDCapMatch>(_1, _2)] |
@@ -206,6 +197,9 @@ bool OSDCap::parse(const string& str, ostream *err)
   if (r && iter == end)
     return true;
 
+  // Make sure no grants are kept after parsing failed!
+  grants.clear();
+
   if (err)
     *err << "osdcap parse failed, stopped at '" << std::string(iter, end)
 	 << "' of '" << str << "'\n";
diff --git a/src/osd/OSDCap.h b/src/osd/OSDCap.h
index 7f2e9f76eb5..941428c3f18 100644
--- a/src/osd/OSDCap.h
+++ b/src/osd/OSDCap.h
@@ -50,7 +50,7 @@ struct OSDCapSpec {
   OSDCapSpec(std::string n, std::string a) : allow(0), class_name(n), class_allow(a) {}
 
   bool allow_all() const {
-    return allow & OSD_CAP_ANY;
+    return allow == OSD_CAP_ANY;
   }
 };
 
diff --git a/src/test/osd/osdcap.cc b/src/test/osd/osdcap.cc
index a3d9e9d5b83..a0866696fc6 100644
--- a/src/test/osd/osdcap.cc
+++ b/src/test/osd/osdcap.cc
@@ -12,6 +12,9 @@
  *
  */
 
+#include <iostream>
+
+#include "include/stringify.h"
 #include "osd/OSDCap.h"
 
 #include "gtest/gtest.h"
@@ -93,8 +96,35 @@ TEST(OSDCap, AllowAll) {
   OSDCap cap;
   ASSERT_FALSE(cap.allow_all());
 
-  bool r = cap.parse("allow *", NULL);
-  ASSERT_TRUE(r);
+  ASSERT_TRUE(cap.parse("allow r", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow w", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow x", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow rwx", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow rw", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow rx", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow wx", NULL));
+  ASSERT_FALSE(cap.allow_all());
+  cap.grants.clear();
+
+  ASSERT_TRUE(cap.parse("allow *", NULL));
   ASSERT_TRUE(cap.allow_all());
   ASSERT_TRUE(cap.is_capable("foo", 0, "asdf", true, true, true));
 }
@@ -157,3 +187,44 @@ TEST(OSDCap, ObjectPoolAndPrefix) {
   ASSERT_FALSE(cap.is_capable("baz", 0, "fo", true, true, true));
 }
 
+TEST(OSDCap, OutputParsed)
+{
+  struct CapsTest {
+    const char *input;
+    const char *output;
+  };
+  CapsTest test_values[] = {
+    {"allow *",
+     "osdcap[grant(*)]"},
+    {"allow r",
+     "osdcap[grant(r)]"},
+    {"allow rx",
+     "osdcap[grant(rx)]"},
+    {"allow rwx",
+     "osdcap[grant(rwx)]"},
+    {"allow rwx pool images",
+     "osdcap[grant(pool images rwx)]"},
+    {"allow r pool images",
+     "osdcap[grant(pool images r)]"},
+    {"allow pool images rwx",
+     "osdcap[grant(pool images rwx)]"},
+    {"allow pool images r",
+     "osdcap[grant(pool images r)]"},
+    {"allow pool images w",
+     "osdcap[grant(pool images w)]"},
+    {"allow pool images x",
+     "osdcap[grant(pool images x)]"},
+    {"allow pool images r; allow pool rbd rwx",
+     "osdcap[grant(pool images r),grant(pool rbd rwx)]"},
+    {"allow pool images r, allow pool rbd rwx",
+     "osdcap[grant(pool images r),grant(pool rbd rwx)]"}
+  };
+
+  size_t num_tests = sizeof(test_values) / sizeof(*test_values);
+  for (size_t i = 0; i < num_tests; ++i) {
+    OSDCap cap;
+    std::cout << "Testing input '" << test_values[i].input << "'" << std::endl;
+    ASSERT_TRUE(cap.parse(test_values[i].input));
+    ASSERT_EQ(test_values[i].output, stringify(cap));
+  }
+}