diff --git a/acceptance/tests/custom_facts/cached_custom_fact.rb b/acceptance/tests/custom_facts/cached_custom_fact.rb index 0f7a452548..bf7a083859 100644 --- a/acceptance/tests/custom_facts/cached_custom_fact.rb +++ b/acceptance/tests/custom_facts/cached_custom_fact.rb @@ -73,7 +73,7 @@ step 'should read from the cached file for a custom fact that has been cached' do on(agent, facter("#{custom_fact_name} --debug", environment: env)) do |facter_result| - assert_match(/Loading cached custom facts from file ".+"|loading cached values for cached-custom-facts facts/, facter_result.stderr, + assert_match(/Loading cached custom facts from file ".+"|loading cached values for random_custom_fact facts/, facter_result.stderr, 'Expected debug message to state that cached custom facts are read from file') end end diff --git a/acceptance/tests/custom_facts/weighted_cached_custom_facts.rb b/acceptance/tests/custom_facts/weighted_cached_custom_facts.rb index dcd9a363ea..3cc04ee30d 100644 --- a/acceptance/tests/custom_facts/weighted_cached_custom_facts.rb +++ b/acceptance/tests/custom_facts/weighted_cached_custom_facts.rb @@ -85,7 +85,7 @@ step 'should read from the cached file for a custom fact that has been cached' do on(agent, facter("#{duplicate_custom_fact_name} --debug", environment: env)) do |facter_result| - assert_match(/Loading cached custom facts from file ".+"|loading cached values for cached-custom-facts facts/, facter_result.stderr, + assert_match(/Loading cached custom facts from file ".+"|loading cached values for random_custom_fact facts/, facter_result.stderr, 'Expected debug message to state that cached custom facts are read from file') end end diff --git a/lib/facter/custom_facts/util/directory_loader.rb b/lib/facter/custom_facts/util/directory_loader.rb index 22b34047cb..e363208c95 100644 --- a/lib/facter/custom_facts/util/directory_loader.rb +++ b/lib/facter/custom_facts/util/directory_loader.rb @@ -59,7 +59,7 @@ def load_directory_entries(_collection) basename = File.basename(file) next if file_blocked?(basename) - if facts.find { |f| f.name == basename } && cm.group_cached?(basename) + if facts.find { |f| f.name == basename } && cm.fact_cache_enabled?(basename) Facter.log_exception(Exception.new("Caching is enabled for group \"#{basename}\" while "\ 'there are at least two external facts files with the same filename')) else diff --git a/lib/facter/framework/config/fact_groups.rb b/lib/facter/framework/config/fact_groups.rb index efd06870a1..d65570def9 100644 --- a/lib/facter/framework/config/fact_groups.rb +++ b/lib/facter/framework/config/fact_groups.rb @@ -2,7 +2,7 @@ module Facter class FactGroups - attr_reader :groups, :block_list + attr_reader :groups, :block_list, :facts_ttls @groups_ttls = [] @@ -14,6 +14,7 @@ def initialize(group_list_path = nil) @groups ||= File.readable?(@groups_file_path) ? Hocon.load(@groups_file_path) : {} load_groups load_groups_from_options + load_facts_ttls end # Breakes down blocked groups in blocked facts @@ -31,6 +32,9 @@ def blocked_facts # Get the group name a fact is part of def get_fact_group(fact_name) + fact = get_fact(fact_name) + return fact[:group] if fact + @groups.detect { |k, v| break k if Array(v).find { |f| fact_name =~ /^#{f}.*/ } } end @@ -41,6 +45,15 @@ def get_group_ttls(group_name) ttls_to_seconds(ttls[group_name]) end + def get_fact(fact_name) + return @facts_ttls[fact_name] if @facts_ttls[fact_name] + + fact = @facts_ttls.select { |k, v| break v if fact_name =~ /^#{k}\..*/ } + return nil if fact == {} + + fact + end + private def load_groups_from_options @@ -55,10 +68,30 @@ def load_groups_from_options end end + def load_facts_ttls + @facts_ttls ||= {} + return if @groups_ttls == [] + + @groups_ttls.reduce(:merge).each do |group, ttls| + ttls = ttls_to_seconds(ttls) + if @groups[group] + @groups[group].each do |fact| + if @facts_ttls[fact] + @facts_ttls[fact] = { ttls: ttls, group: group } if @facts_ttls[fact][:ttls] < ttls + else + @facts_ttls[fact] = { ttls: ttls, group: group } + end + end + else + @facts_ttls[group] = { ttls: ttls, group: group } + end + end + end + def load_groups config = ConfigReader.init(Options[:config]) @block_list = config.block_list || {} - @groups_ttls = config.ttls || {} + @groups_ttls = config.ttls || [] @groups.merge!(config.fact_groups) if config.fact_groups end diff --git a/lib/facter/framework/core/cache_manager.rb b/lib/facter/framework/core/cache_manager.rb index 92170bc7a5..4654aa4a17 100644 --- a/lib/facter/framework/core/cache_manager.rb +++ b/lib/facter/framework/core/cache_manager.rb @@ -40,31 +40,42 @@ def cache_facts(resolved_facts) end end - def group_cached?(group_name) - cached = @fact_groups.get_group_ttls(group_name) ? true : false - delete_cache(group_name) unless cached + def fact_cache_enabled?(fact_name) + fact = @fact_groups.get_fact(fact_name) + cached = if fact + !fact[:ttls].nil? + else + false + end + + fact_group = @fact_groups.get_fact_group(fact_name) + delete_cache(fact_group) if fact_group && !cached cached end private def resolve_fact(searched_fact) - group_name = if searched_fact.file - searched_fact.name - else - @fact_groups.get_fact_group(searched_fact.name) - end + return unless fact_cache_enabled?(searched_fact.name) - return unless group_name + fact = if searched_fact.file + @fact_groups.get_fact(File.basename(searched_fact.file)) + else + @fact_groups.get_fact(searched_fact.name) + end - return unless group_cached?(group_name) + return unless fact - return unless check_ttls?(group_name) + return unless check_ttls?(fact[:group], fact[:ttls]) - data = read_group_json(group_name) + read_fact(searched_fact, fact[:group]) + end + + def read_fact(searched_fact, fact_group) + data = read_group_json(fact_group) return unless data - @log.debug("loading cached values for #{group_name} facts") + @log.debug("loading cached values for #{searched_fact.name} facts") create_facts(searched_fact, data) end @@ -93,7 +104,7 @@ def cache_fact(fact) end return if !group_name || fact.value.nil? - return unless group_cached?(group_name) + return unless fact_cache_enabled?(fact.name) @groups[group_name] ||= {} @groups[group_name][fact.name] = fact.value @@ -106,10 +117,12 @@ def write_cache end @groups.each do |group_name, data| - next unless check_ttls?(group_name) + next unless check_ttls?(group_name, @fact_groups.get_group_ttls(group_name)) - @log.debug("caching values for #{group_name} facts") cache_file_name = File.join(@cache_dir, group_name) + next if File.readable?(cache_file_name) + + @log.debug("caching values for #{group_name} facts") File.write(cache_file_name, JSON.pretty_generate(data)) end end @@ -128,8 +141,7 @@ def read_group_json(group_name) @groups[group_name] = data end - def check_ttls?(group_name) - ttls = @fact_groups.get_group_ttls(group_name) + def check_ttls?(group_name, ttls) return false unless ttls cache_file_name = File.join(@cache_dir, group_name) diff --git a/spec/facter/cache_manager_spec.rb b/spec/facter/cache_manager_spec.rb index 2b12c737ee..ee540348cb 100644 --- a/spec/facter/cache_manager_spec.rb +++ b/spec/facter/cache_manager_spec.rb @@ -25,6 +25,8 @@ let(:group_name) { 'operating system' } let(:cache_file_name) { File.join(cache_dir, group_name) } let(:fact_groups) { instance_spy(Facter::FactGroups) } + let(:os_fact) { { ttls: 60, group: 'operating system' } } + let(:external_fact) { { ttls: 60, group: 'ext_file.txt' } } before do allow(LegacyFacter::Util::Config).to receive(:facts_cache_dir).and_return(cache_dir) @@ -72,7 +74,10 @@ allow(File).to receive(:directory?).with(cache_dir).and_return(true) allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(fact_groups).to receive(:get_fact_group).with('my_custom_fact').and_return(nil) + allow(fact_groups).to receive(:get_fact_group).with('ext_file.txt').and_return(nil) allow(fact_groups).to receive(:get_group_ttls).with('ext_file.txt').and_return(nil) + allow(fact_groups).to receive(:get_fact).with('ext_file.txt').and_return(nil) + allow(fact_groups).to receive(:get_fact).with('my_custom_fact').and_return(nil) allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:mtime).with(cache_file_name).and_return(Time.now) allow(Facter::Util::FileHelper).to receive(:safe_read).with(cache_file_name).and_return(cached_core_fact) @@ -80,7 +85,7 @@ end it 'returns cached fact' do - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(fact_groups).to receive(:get_fact).with('os').and_return(os_fact) allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) @@ -91,7 +96,7 @@ end it 'returns searched fact' do - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(fact_groups).to receive(:get_fact).with('os').and_return(os_fact) allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) @@ -103,9 +108,10 @@ end it 'deletes cache file' do - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(fact_groups).to receive(:get_fact).with('os').and_return(nil) allow(File).to receive(:readable?).with(cache_file_name).and_return(true) allow(File).to receive(:delete).with(cache_file_name) + allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(File).to receive(:readable?).with(File.join(cache_dir, 'ext_file.txt')).and_return(false) cache_manager.resolve_facts(searched_facts) @@ -113,8 +119,9 @@ end it 'returns cached external facts' do - allow(fact_groups).to receive(:get_group_ttls).with('ext_file.txt').and_return(1000) - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(fact_groups).to receive(:get_fact).with('os').and_return(nil) + allow(fact_groups).to receive(:get_fact).with('my_custom_fact').and_return(nil) + allow(fact_groups).to receive(:get_fact).with('ext_file.txt').and_return(external_fact) allow(File).to receive(:readable?).with(cache_file_name).and_return(false) allow(Facter::Util::FileHelper).to receive(:safe_read).with(File.join(cache_dir, 'ext_file.txt')) .and_return(cached_external_fact) @@ -135,7 +142,7 @@ before do allow(File).to receive(:directory?).with(cache_dir).and_return(true) allow(File).to receive(:readable?).with(cache_file_name).and_return(false) - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(fact_groups).to receive(:get_fact).with('os').and_return(nil) allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(File).to receive(:write).with(cache_file_name, cached_core_fact) allow(Facter::Options).to receive(:[]).with(:cache).and_return(true) @@ -150,10 +157,10 @@ context 'with cache group' do before do allow(File).to receive(:directory?).with(cache_dir).and_return(true) + allow(fact_groups).to receive(:get_fact).with('os').and_return(os_fact) allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(fact_groups).to receive(:get_fact_group).with('my_custom_fact').and_return(nil) allow(fact_groups).to receive(:get_fact_group).with('my_external_fact').and_return(nil) - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) allow(File).to receive(:readable?).with(cache_file_name).and_return(false) allow(File).to receive(:write).with(cache_file_name, cached_core_fact) allow(Facter::Options).to receive(:[]).with(:cache).and_return(true) @@ -166,35 +173,36 @@ end end - describe '#group_cached?' do + describe '#fact_cache_enabled?' do context 'with ttls' do before do - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(1000) + allow(fact_groups).to receive(:get_fact).with('os').and_return(os_fact) allow(File).to receive(:readable?).with(cache_file_name).and_return(false) end it 'returns true' do - result = cache_manager.group_cached?(group_name) + result = cache_manager.fact_cache_enabled?('os') expect(result).to be true end end context 'without ttls' do before do - allow(fact_groups).to receive(:get_group_ttls).with(group_name).and_return(nil) + allow(fact_groups).to receive(:get_fact).with('os').and_return(nil) + allow(fact_groups).to receive(:get_fact_group).with('os').and_return(group_name) allow(Facter::Options).to receive(:[]).with(:cache).and_return(true) allow(File).to receive(:delete).with(cache_file_name) end it 'returns false' do allow(File).to receive(:readable?).with(cache_file_name).and_return(false) - result = cache_manager.group_cached?(group_name) + result = cache_manager.fact_cache_enabled?('os') expect(result).to be false end it 'deletes invalid cache file' do allow(File).to receive(:readable?).with(cache_file_name).and_return(true) - cache_manager.group_cached?(group_name) + cache_manager.fact_cache_enabled?('os') expect(File).to have_received(:delete).with(cache_file_name) end end