config: prevent SIGHUP from changing non-liveupdatable parameters

Before this change, it was possible to change non-liveupdatable config
parameter without process restart. This erroneous behavior not only
contradicts the documentation but is potentially dangerous, as various
components theoretically might not be prepared for a change of
configuration parameter value without a restart. The issue came from
a fact that liveupdatability verification check was skipped for default
configuration parameters (those without its initial values
in configuration file during process start).

This change:
 - Introduce _initialization_completed member in config_file
 - Set _initialization_completed=true when config file is processed on
   server start
 - Verify config_file's initialization status during config update - if
   config_file was initialized, prevent from further changes of
   non-liveupdatable parameters

Fixes scylladb/scylladb#5382

utils/config_file.cc

@@ -226,7 +226,7 @@ sstring utils::hyphenate(const std::string_view& v) {
 }
 
 utils::config_file::config_file(std::initializer_list<cfg_ref> cfgs)
-    : _cfgs(cfgs)
+    : _cfgs(cfgs), _initialization_completed(false)
 {}
 
 void utils::config_file::add(cfg_ref cfg, std::unique_ptr<any_value> value) {
@@ -331,7 +331,7 @@ void utils::config_file::read_from_yaml(const char* yaml, error_handler h) {
         }
         // Still, a syntax error is an error warning, not a fail
         try {
-            cfg.set_value(node.second);
+            cfg.set_value(node.second, this->_initialization_completed ? config_source::SettingsFile : config_source::None);
         } catch (std::exception& e) {
             h(label, e.what(), cfg.status());
         } catch (...) {
@@ -365,6 +365,11 @@ future<> utils::config_file::read_from_file(file f, error_handler h) {
         return do_with(make_file_input_stream(f), [this, s, h](input_stream<char>& in) {
             return in.read_exactly(s).then([this, h](temporary_buffer<char> buf) {
                read_from_yaml(sstring(buf.begin(), buf.end()), h);
+                if (!_initialization_completed) {
+                    // Boolean value set on only one shard, but broadcast_to_all_shards().get() called later
+                    // in main.cc will apply the required memory barriers anyway.
+                    _initialization_completed = true;
+                }
             });
         });
     });

utils/config_file.hh

@@ -147,7 +147,7 @@ public:
         }
         bool matches(std::string_view name) const;
         virtual void add_command_line_option(bpo::options_description_easy_init&) = 0;
-        virtual void set_value(const YAML::Node&) = 0;
+        virtual void set_value(const YAML::Node&, config_source) = 0;
         virtual bool set_value(sstring, config_source) = 0;
         virtual future<bool> set_value_on_all_shards(sstring, config_source) = 0;
         virtual value_status status() const noexcept = 0;
@@ -255,11 +255,12 @@ public:
         }
 
         void add_command_line_option(bpo::options_description_easy_init&) override;
-        void set_value(const YAML::Node&) override;
+        void set_value(const YAML::Node&, config_source) override;
         bool set_value(sstring, config_source) override;
         // For setting a single value on all shards,
         // without having to call broadcast_to_all_shards
         // that broadcasts all values to all shards.
+
         future<bool> set_value_on_all_shards(sstring, config_source) override;
     };
 
@@ -317,6 +318,7 @@ private:
 
     configs
         _cfgs;
+    bool _initialization_completed;
 };
 
 template <typename T>

utils/config_file_impl.hh

@@ -205,9 +205,8 @@ void utils::config_file::named_value<T>::add_command_line_option(boost::program_
 }
 
 template<typename T>
-void utils::config_file::named_value<T>::set_value(const YAML::Node& node) {
-    if (_source == config_source::SettingsFile && _liveness != liveness::LiveUpdate) {
-        // FIXME: warn if different?
+void utils::config_file::named_value<T>::set_value(const YAML::Node& node, config_source previous_src) {
+    if (previous_src == config_source::SettingsFile && _liveness != liveness::LiveUpdate) {
         return;
     }
     (*this)(node.as<T>());