Move get_file and save_file to ConfigurationManagementMixin

In moving, the methods become settings_for_resource_yaml and
add_settings_for_resource_yaml, respectively.  The old interfaces are
still left intact temporarily until the UI is changed to use the new
methods.

Also preserved is the special activation that is currently done for
ntp_reload, as introduced in 939524fa, which avoids
repeated activation for every worker, instead preferring to do it once
at the server level.

https://bugzilla.redhat.com/show_bug.cgi?id=1082155

app/models/miq_server/configuration_management.rb

@@ -13,11 +13,28 @@ def get_config(type = "vmdb")
   def set_config(config)
     config = config.config if config.respond_to?(:config)
     add_settings_for_resource(config)
-    ntp_reload_queue
   end
 
   def reload_settings
-    Vmdb::Settings.reload! if is_local?
+    return if is_remote?
+
+    Vmdb::Settings.reload!
+    activate_settings_for_appliance
+  end
+
+  # The purpose of this method is to do special activation of things
+  #   that can only happen once per server.  Normally, the
+  #   VMDB::Config::Activator would be used, however the activations
+  #   will end up occurring once per worker on the entire server, which
+  #   can be detrimental.
+  #
+  #   As an example, ntp_reload works by telling systemctl to restart
+  #   chronyd.  However, if this occurs on every worker, you end up with
+  #   dozens of calls to `systemctl restart chronyd` simultaneously.
+  #   Instead, this method will allow it to only happen once on
+  #   the reload of settings in evmserverd.
+  private def activate_settings_for_appliance
+    ntp_reload_queue
   end
 
   def servers_for_settings_reload

app/models/mixins/configuration_management_mixin.rb

@@ -9,11 +9,20 @@ def settings_for_resource
     Vmdb::Settings.for_resource(self)
   end
 
+  def settings_for_resource_yaml
+    Vmdb::Settings.for_resource_yaml(self)
+  end
+
   def add_settings_for_resource(settings)
     Vmdb::Settings.save!(self, settings)
     immediately_reload_settings
   end
 
+  def add_settings_for_resource_yaml(contents)
+    Vmdb::Settings.save_yaml!(self, contents)
+    immediately_reload_settings
+  end
+
   def remove_settings_path_for_resource(*keys)
     Vmdb::Settings.destroy!(self, keys)
     immediately_reload_settings

lib/vmdb/config.rb

@@ -58,22 +58,15 @@ def save(resource = MiqServer.my_server)
 
     # NOTE: Used by Configuration -> Advanced
     def self.get_file(resource = MiqServer.my_server)
-      Vmdb::Settings.encrypt_passwords!(resource.settings_for_resource.to_hash).to_yaml
+      resource.settings_for_resource_yaml
     end
 
     # NOTE: Used by Configuration -> Advanced
     def self.save_file(contents, resource = MiqServer.my_server)
-      config = new("vmdb")
-
-      begin
-        config.config = Vmdb::Settings.decrypt_passwords!(YAML.load(contents))
-        config.validate
-      rescue StandardError, Psych::SyntaxError => err
-        config.errors = [[:contents, "File contents are malformed, '#{err.message}'"]]
-      end
-
-      return config.errors unless config.errors.blank?
-      config.save(resource)
+      resource.add_settings_for_resource_yaml(contents)
+    rescue Vmdb::Settings::ConfigurationInvalid => err
+      err.errors
+    else
       true
     end
 

lib/vmdb/settings.rb

@@ -8,7 +8,15 @@ module Vmdb
   class Settings
     extend Vmdb::SettingsWalker::ClassMethods
 
-    class ConfigurationInvalid < StandardError; end
+    class ConfigurationInvalid < StandardError
+      attr_accessor :errors
+
+      def initialize(errors)
+        @errors = errors
+        message = errors.map { |k, v| "#{k}: #{v}" }.join("; ")
+        super(message)
+      end
+    end
 
     PASSWORD_FIELDS = Vmdb::SettingsWalker::PASSWORD_FIELDS
     DUMP_LOG_FILE   = Rails.root.join("log/last_settings.txt").freeze
@@ -56,10 +64,7 @@ def self.save!(resource, hash)
       new_settings = build_without_local(resource).load!.merge!(hash).to_hash
 
       valid, errors = validate(new_settings)
-      unless valid
-        message = errors.map { |k, v| "#{k}: #{v}" }.join("; ")
-        raise ConfigurationInvalid, message
-      end
+      raise ConfigurationInvalid.new(errors) unless valid # rubocop:disable Style/RaiseArgs
 
       hash_for_parent = parent_settings_without_local(resource).load!.to_hash
       diff = HashDiffer.diff(hash_for_parent, new_settings)
@@ -68,6 +73,18 @@ def self.save!(resource, hash)
       apply_settings_changes(resource, deltas)
     end
 
+    def self.save_yaml!(resource, contents)
+      require 'yaml'
+      hash =
+        begin
+          decrypt_passwords!(YAML.load(contents))
+        rescue => err
+          raise ConfigurationInvalid.new(:contents => "File contents are malformed: #{err.message.inspect}")
+        end
+
+      save!(resource, hash)
+    end
+
     def self.destroy!(resource, keys)
       return if keys.blank?
       settings_path = File.join("/", keys.collect(&:to_s))
@@ -78,6 +95,11 @@ def self.for_resource(resource)
       build(resource).load!
     end
 
+    def self.for_resource_yaml(resource)
+      require 'yaml'
+      encrypt_passwords!(for_resource(resource).to_hash).to_yaml
+    end
+
     def self.template_settings
       build_template.load!
     end

spec/lib/vmdb/config_spec.rb

@@ -49,8 +49,9 @@
   end
 
   context ".save_file" do
+    let!(:resource) { FactoryGirl.create(:miq_server) }
+
     it "normal" do
-      resource = FactoryGirl.create(:miq_server)
       MiqRegion.seed
       data = {}
       data.store_path(:api, :token_ttl, "1.day")
@@ -62,7 +63,7 @@
     end
 
     it "catches syntax errors" do
-      errors = VMDB::Config.save_file("xxx")
+      errors = VMDB::Config.save_file("xxx", resource)
       expect(errors.length).to eq(1)
       expect(errors.first[0]).to eq(:contents)
       expect(errors.first[1]).to start_with("File contents are malformed")

spec/lib/vmdb/settings_spec.rb

@@ -267,6 +267,62 @@
     end
   end
 
+  describe "save_yaml!" do
+    let(:miq_server) { FactoryGirl.create(:miq_server) }
+
+    it "saves the settings" do
+      data = {:api => {:token_ttl => "1.day"}}.to_yaml
+      described_class.save_yaml!(miq_server, data)
+
+      miq_server.reload
+
+      expect(miq_server.settings_changes.count).to eq 1
+      expect(miq_server.settings_changes.first).to have_attributes(:key   => "/api/token_ttl",
+                                                                   :value => "1.day")
+    end
+
+    it "handles incoming unencrypted values" do
+      password  = "pa$$word"
+      encrypted = MiqPassword.encrypt(password)
+
+      data = {:authentication => {:bind_pwd => password}}.to_yaml
+      described_class.save_yaml!(miq_server, data)
+
+      miq_server.reload
+
+      expect(miq_server.settings_changes.count).to eq 1
+      expect(miq_server.settings_changes.first).to have_attributes(:key   => "/authentication/bind_pwd",
+                                                                   :value => encrypted)
+    end
+
+    it "handles incoming encrypted values" do
+      password  = "pa$$word"
+      encrypted = MiqPassword.encrypt(password)
+
+      data = {:authentication => {:bind_pwd => encrypted}}.to_yaml
+      described_class.save_yaml!(miq_server, data)
+
+      miq_server.reload
+
+      expect(miq_server.settings_changes.count).to eq 1
+      expect(miq_server.settings_changes.first).to have_attributes(:key   => "/authentication/bind_pwd",
+                                                                   :value => encrypted)
+    end
+
+    {
+      "syntax"     => "--- -", # invalid YAML
+      "non-syntax" => "xxx"    # valid YAML, but invalid config
+    }.each do |type, contents|
+      it "catches #{type} errors" do
+        expect { described_class.save_yaml!(miq_server, contents) }.to raise_error(described_class::ConfigurationInvalid) do |err|
+          expect(err.errors.size).to eq 1
+          expect(err.errors[:contents]).to start_with("File contents are malformed")
+          expect(err.message).to include("contents: File contents are malformed")
+        end
+      end
+    end
+  end
+
   shared_examples_for "password handling" do
     subject do
       described_class.send(method, Settings)
@@ -397,6 +453,33 @@
     end
   end
 
+  describe ".for_resource_yaml" do
+    it "fetches the yaml with changes" do
+      miq_server = FactoryGirl.create(:miq_server)
+      described_class.save!(miq_server, :api => {:token_ttl => "1.day"})
+
+      yaml = described_class.for_resource_yaml(miq_server)
+      expect(yaml).to_not include("Config::Options")
+
+      hash = YAML.load(yaml)
+      expect(hash).to be_kind_of Hash
+      expect(hash.fetch_path(:api, :token_ttl)).to eq "1.day"
+    end
+
+    it "ensures passwords are encrypted" do
+      password  = "pa$$word"
+      encrypted = MiqPassword.encrypt(password)
+
+      miq_server = FactoryGirl.create(:miq_server)
+      described_class.save!(miq_server, :authentication => {:bind_pwd => password})
+
+      yaml = described_class.for_resource_yaml(miq_server)
+
+      hash = YAML.load(yaml)
+      expect(hash.fetch_path(:authentication, :bind_pwd)).to eq encrypted
+    end
+  end
+
   it "with .local file" do
     stub_local_settings_file(
       Rails.root.join("config/settings/test.local.yml"),