config: fix metavariable substitution

common_init: Do metavariable expansion as a separate step after the
configuration, argv, and env have been read. This ensures that we get
the correct values for the metavariables.

Change librados to follow suit.

config: Enforce the "no metavariables in default values" restriction.

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/common/common_init.cc b/src/common/common_init.cc
index b09a0876eb5160855812658b9d499bf3fbfe03c6..32f75428f98b495382acd1e7c70eb2cd13d24b6f 100644 (file)
--- a/src/common/common_init.cc
+++ b/src/common/common_init.cc
@@ -142,6 +142,8 @@ void common_init(std::vector < const char* >& args,
     conf->log_per_instance = false;
   }
 
+  conf->expand_all_meta();
+
   if (conf->log_to_syslog || conf->clog_to_syslog) {
     closelog();
     openlog(g_conf.name->to_cstr(), LOG_ODELAY | LOG_PID, LOG_USER);

diff --git a/src/common/config.cc b/src/common/config.cc
index 0d7bb9e826438a2114e7fd370ffacc3afdd16694..5814a5bb5109b1892b454c3ef8b60491a03cc8d8 100644 (file)
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -540,9 +540,6 @@ parse_config_files(const std::list<std::string> &conf_files)
     if (ret == 0) {
       set_val_impl(val.c_str(), opt);
     }
-    else if (ret != -ENOENT) {
-      // TODO: complain about parse error
-    }
   }
 
   // FIXME: This bit of global fiddling needs to go somewhere else eventually.
@@ -753,10 +750,8 @@ get_val_from_conf_file(const std::vector <std::string> &sections,
   std::vector <std::string>::const_iterator s_end = sections.end();
   for (; s != s_end; ++s) {
     int ret = cf->read(s->c_str(), key, out);
-    if (ret == 0) {
-      conf_post_process_val(out);
+    if (ret == 0)
       return 0;
-    }
     else if (ret != -ENOENT)
       return ret;
   }
@@ -773,9 +768,25 @@ set_val_from_default(const config_option *opt)
     case OPT_LONGLONG:
       *(long long*)opt->val_ptr = opt->def_longlong;
       break;
-    case OPT_STR:
-      *(std::string *)opt->val_ptr = opt->def_str ? opt->def_str : "";
+    case OPT_STR: {
+      std::string *str = (std::string *)opt->val_ptr;
+      *str = opt->def_str ? opt->def_str : "";
+      if (expand_meta(*str)) {
+       // We don't allow metavariables in default values.  The reason for this
+       // is that default values take effect at global constructor time.  At
+       // global constructor time, we don't have valid values for $host,
+       // $name, $id and so forth. So trying to set default variables would
+       // inevitably lead to the wrong behavior.  Once g_conf is
+       // de-globalized, and we don't have to worry about global constructor
+       // time, this restriction can be eased.
+       ostringstream oss;
+       oss << "found metavariables in the default value for '"
+           << opt->name << "'. " << "metavariables cannot be "
+           << "used in default values.";
+       assert(oss.str().c_str() == 0);
+      }
       break;
+    }
     case OPT_FLOAT:
       *(float *)opt->val_ptr = (float)opt->def_double;
       break;
@@ -883,14 +894,27 @@ set_val_impl(const char *val, const config_option *opt)
   return -ENOSYS;
 }
 
+void md_config_t::
+expand_all_meta()
+{
+  for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) {
+    config_option *opt = config_optionsp + i;
+    if (opt->type == OPT_STR) {
+      std::string *str = (std::string *)opt->val_ptr;
+      expand_meta(*str);
+    }
+  }
+}
+
 static const char *CONF_METAVARIABLES[] =
       { "type", "name", "host", "num", "id" };
 static const int NUM_CONF_METAVARIABLES =
       (sizeof(CONF_METAVARIABLES) / sizeof(CONF_METAVARIABLES[0]));
 
-void md_config_t::
-conf_post_process_val(std::string &val) const
+bool md_config_t::
+expand_meta(std::string &val) const
 {
+  bool found_meta = false;
   string out;
   string::size_type sz = val.size();
   out.reserve(sz);
@@ -919,6 +943,7 @@ conf_post_process_val(std::string &val) const
        out += name->get_id().c_str();
       else
        assert(0); // unreachable
+      found_meta = true;
       break;
     }
     if (i == NUM_CONF_METAVARIABLES)
@@ -927,4 +952,5 @@ conf_post_process_val(std::string &val) const
       s += strlen(CONF_METAVARIABLES[i]) + 1;
   }
   val = out;
+  return found_meta;
 }

diff --git a/src/common/config.h b/src/common/config.h
index 28fb999a566c21511fe988bcf4d0b5359e87a85b..d200b5a8b37f8183d460f75114107e3902f9a361 100644 (file)
--- a/src/common/config.h
+++ b/src/common/config.h
@@ -78,6 +78,9 @@ public:
   int get_val_from_conf_file(const std::vector <std::string> &sections,
                             const char *key, std::string &out) const;
 
+  // Do all metavariable substitutions
+  void expand_all_meta();
+
 private:
   // Private function for setting a default for a config option
   void set_val_from_default(const config_option *opt);
@@ -85,7 +88,7 @@ private:
   int set_val_impl(const char *val, const config_option *opt);
 
   // Do metavariable expansions
-  void conf_post_process_val(std::string &val) const;
+  bool expand_meta(std::string &val) const;
 
   // The configuration file we read, or NULL if we haven't read one.
   ConfFile *cf;

diff --git a/src/librados.cc b/src/librados.cc
index e1df8308e929e904b66dcb7cc31e9441636b4bba..2a0d7560e6be7b9b90ab34d64d2a8588e4ad9d08 100644 (file)
--- a/src/librados.cc
+++ b/src/librados.cc
@@ -2793,6 +2793,7 @@ extern "C" int rados_create(rados_t *pcluster, const char * const id)
     // configuration
     md_config_t *conf = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
     conf->parse_env(); // environment variables override
+    conf->expand_all_meta(); // future proofing
 
     ++rados_initialized;
   }
@@ -2860,6 +2861,7 @@ extern "C" int rados_conf_read_file(rados_t cluster, const char *path)
   if (ret)
     return ret;
   g_conf.parse_env(); // environment variables override
+  g_conf.expand_all_meta(); // handle metavariables in the config
   return 0;
 }