diff --git a/app/models/miq_server/configuration_management.rb b/app/models/miq_server/configuration_management.rb index 59d24025bc2..c5289643b3f 100644 --- a/app/models/miq_server/configuration_management.rb +++ b/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 diff --git a/app/models/mixins/configuration_management_mixin.rb b/app/models/mixins/configuration_management_mixin.rb index 64fb8e5aa62..d4dd24b8dcf 100644 --- a/app/models/mixins/configuration_management_mixin.rb +++ b/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 diff --git a/lib/vmdb/config.rb b/lib/vmdb/config.rb index d519c20513f..f90620680d2 100644 --- a/lib/vmdb/config.rb +++ b/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 diff --git a/lib/vmdb/settings.rb b/lib/vmdb/settings.rb index 69d45add5f1..0a5ea0b0b14 100644 --- a/lib/vmdb/settings.rb +++ b/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 diff --git a/spec/lib/vmdb/config_spec.rb b/spec/lib/vmdb/config_spec.rb index fe6edb4d7f0..00366462e4b 100644 --- a/spec/lib/vmdb/config_spec.rb +++ b/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") diff --git a/spec/lib/vmdb/settings_spec.rb b/spec/lib/vmdb/settings_spec.rb index a52ef1ee9e9..a11ea9e4218 100644 --- a/spec/lib/vmdb/settings_spec.rb +++ b/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"),