Merge pull request #69675 from NitzanMordhai/wip-nitzan-config-trim-key-trail-lead-spaces

mon/config: trim whitespace in config target

qa/workunits/mon/config.sh

@@ -73,6 +73,23 @@ ceph config rm client.foo debug_asok
 ceph config set client.foo debug_asok 66
 ceph config rm client.foo 'debug asok'
 
+# whitespace in who/section (e.g. "osd.0  " with stray trailing spaces)
+ceph config set 'osd.0  ' debug_asok 44
+ceph config get osd.0 debug_asok | grep 44
+while ! ceph tell osd.0 config get debug_asok | grep 44
+do
+    sleep 1
+done
+ceph config rm osd.0 debug_asok
+while ceph tell osd.0 config get debug_asok | grep 44
+do
+    sleep 1
+done
+
+ceph config set '  osd.0' debug_asok 55
+ceph config get osd.0 debug_asok | grep 55
+ceph config rm osd.0 debug_asok
+
 # help
 ceph config help debug_asok | grep debug_asok
 

src/mon/ConfigMap.cc

@@ -6,6 +6,7 @@
 #include "common/entity_name.h"
 
 #include <boost/algorithm/string/split.hpp>
+#include <boost/algorithm/string/trim.hpp>
 
 #define dout_subsys ceph_subsys_mon
 #undef dout_prefix
@@ -207,6 +208,7 @@ bool ConfigMap::parse_mask(
   boost::split(split, who, [](char c){ return c == '/'; });
   for (unsigned j = 0; j < split.size(); ++j) {
     auto& i = split[j];
+    boost::algorithm::trim(i);
     if (i == "global") {
       *section = "global";
       continue;

src/mon/ConfigMonitor.cc

@@ -19,6 +19,7 @@
 #include "crush/CrushWrapper.h"
 
 #include <boost/algorithm/string/predicate.hpp>
+#include <boost/algorithm/string/trim.hpp>
 
 #define dout_subsys ceph_subsys_mon
 #undef dout_prefix
@@ -296,6 +297,7 @@ bool ConfigMonitor::preprocess_command(MonOpRequestRef op)
   } else if (prefix == "config get") {
     string who, name;
     cmd_getval(cmdmap, "who", who);
+    boost::algorithm::trim(who);
 
     EntityName entity;
     if (!entity.from_str(who) &&

src/test/mon/test_config_map.cc

@@ -53,6 +53,67 @@ TEST(ConfigMap, parse_key)
   }
 }
 
+TEST(ConfigMap, parse_mask)
+{
+  {
+    std::string section;
+    OptionMask mask;
+    ASSERT_TRUE(ConfigMap::parse_mask("osd.2", &section, &mask));
+    ASSERT_EQ("osd.2", section);
+  }
+  {
+    std::string section;
+    OptionMask mask;
+    ASSERT_TRUE(ConfigMap::parse_mask("osd.2  ", &section, &mask));
+    ASSERT_EQ("osd.2", section);
+  }
+  {
+    std::string section;
+    OptionMask mask;
+    ASSERT_TRUE(ConfigMap::parse_mask("  osd.2", &section, &mask));
+    ASSERT_EQ("osd.2", section);
+  }
+  {
+    std::string section;
+    OptionMask mask;
+    ASSERT_TRUE(ConfigMap::parse_mask(" global ", &section, &mask));
+    ASSERT_EQ("global", section);
+  }
+}
+
+TEST(ConfigMap, add_option_who_whitespace)
+{
+  ConfigMap cm;
+  boost::intrusive_ptr<CephContext> cct{new CephContext(CEPH_ENTITY_TYPE_CLIENT), false};
+  auto crush = std::make_unique<CrushWrapper>();
+  crush->finalize();
+
+  int r = cm.add_option(
+    cct.get(), "debug_ms", "osd.2  ", "1/1",
+    [&](const std::string& name) {
+      return nullptr;
+    });
+  ASSERT_EQ(0, r);
+  ASSERT_EQ(1, cm.by_id.size());
+  ASSERT_EQ(1, cm.by_id["osd.2"].options.size());
+
+  EntityName n;
+  n.set(CEPH_ENTITY_TYPE_OSD, "2");
+  auto c = cm.generate_entity_map(n, {}, crush.get(), "none", nullptr);
+  ASSERT_EQ(1, c.size());
+  ASSERT_EQ("1/1", c["debug_ms"]);
+
+  r = cm.add_option(
+    cct.get(), "foo", "osd.2", "clean",
+    [&](const std::string& name) {
+      return nullptr;
+    });
+  ASSERT_EQ(0, r);
+
+  ASSERT_EQ(1, cm.by_id.size());
+  ASSERT_EQ(2, cm.by_id["osd.2"].options.size());
+}
+
 TEST(ConfigMap, add_option)
 {
   ConfigMap cm;