diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 429346bd92..aefca6db6d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,12 +1,12 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 1000` -# on 2020-08-13 15:12:10 +0300 using RuboCop version 0.81.0. +# on 2020-08-26 01:26:00 +0300 using RuboCop version 0.81.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 81 +# Offense count: 76 # Configuration parameters: CustomTransform, IgnoreMethods, SpecSuffixOnly. RSpec/FilePath: Exclude: @@ -45,13 +45,10 @@ RSpec/FilePath: - 'spec/facter/resolvers/macosx/dmi_resolver_spec.rb' - 'spec/facter/resolvers/macosx/utils/system_profile_executor_spec.rb' - 'spec/facter/resolvers/memory_resolver_spec.rb' - - 'spec/facter/resolvers/mountpoints_resolver_spec.rb' - 'spec/facter/resolvers/processors_resolver_spec.rb' - 'spec/facter/resolvers/redhat_release_resolver_spec.rb' - 'spec/facter/resolvers/selinux_resolver_spec.rb' - 'spec/facter/resolvers/solaris/solaris_release_resolver_spec.rb' - - 'spec/facter/resolvers/solaris/solaris_zone_name_spec.rb' - - 'spec/facter/resolvers/solaris/zone_resolver_spec.rb' - 'spec/facter/resolvers/suse_relese_resolver_spec.rb' - 'spec/facter/resolvers/system_profile_resolver_spec.rb' - 'spec/facter/resolvers/utils/aix/odm_query_spec.rb' @@ -64,13 +61,11 @@ RSpec/FilePath: - 'spec/facter/resolvers/windows/kernel_resolver_spec.rb' - 'spec/facter/resolvers/windows/memory_resolver_spec.rb' - 'spec/facter/resolvers/windows/netkvm_resolver_spec.rb' - - 'spec/facter/resolvers/windows/networking_resolver_spec.rb' - 'spec/facter/resolvers/windows/processors_resolver_spec.rb' - 'spec/facter/resolvers/windows/product_release_resolver_spec.rb' - 'spec/facter/resolvers/windows/system32_resolver_spec.rb' - 'spec/facter/resolvers/windows/virtualization_resolver_spec.rb' - 'spec/facter/resolvers/windows/win_os_description_resolver_spec.rb' - - 'spec/facter/resolvers/zpool_resolver_spec.rb' - 'spec/framework/config/config_reader_spec.rb' - 'spec/framework/config/fact_groups_spec.rb' - 'spec/framework/core/fact/external/external_fact_manager_spec.rb' @@ -176,7 +171,7 @@ RSpec/SubjectStub: - 'spec/custom_facts/util/fact_spec.rb' - 'spec/custom_facts/util/resolution_spec.rb' -# Offense count: 130 +# Offense count: 123 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. RSpec/VerifiedDoubles: Exclude: @@ -225,7 +220,6 @@ RSpec/VerifiedDoubles: - 'spec/facter/resolvers/windows/uptime_resolver_spec.rb' - 'spec/facter/resolvers/windows/win_os_description_resolver_spec.rb' - 'spec/framework/core/fact_loaders/external_fact_loader_spec.rb' - - 'spec/framework/core/fact_loaders/fact_loader_spec.rb' - 'spec/framework/core/fact_manager_spec.rb' - 'spec/framework/core/session_cache_spec.rb' - 'spec/framework/formatters/hocon_fact_formatter_spec.rb' diff --git a/lib/facter/framework/core/fact_filter.rb b/lib/facter/framework/core/fact_filter.rb index bb77510755..f2f258db8b 100644 --- a/lib/facter/framework/core/fact_filter.rb +++ b/lib/facter/framework/core/fact_filter.rb @@ -7,6 +7,8 @@ module Facter class FactFilter def filter_facts!(searched_facts) filter_legacy_facts!(searched_facts) + filter_blocked_legacy_facts!(searched_facts) + searched_facts.each do |fact| fact.value = if fact.filter_tokens.any? && fact.value.respond_to?(:dig) fact.value.dig(*fact.filter_tokens) @@ -18,6 +20,17 @@ def filter_facts!(searched_facts) private + # This will filter out the legacy facts that should be blocked. Because some legacy facts are just aliases + # to the core ones, even if they are blocked, facter will resolved them but they won't be displayed. + + def filter_blocked_legacy_facts!(facts) + blocked_facts = Options[:blocked_facts] || [] + + facts.reject! do |fact| + blocked_facts.select { |blocked_fact| fact.name.match(/^#{blocked_fact}/) && fact.type == :legacy }.any? + end + end + def filter_legacy_facts!(resolved_facts) return unless !Options[:show_legacy] && Options[:user_query].empty? diff --git a/lib/facter/framework/core/fact_loaders/fact_loader.rb b/lib/facter/framework/core/fact_loaders/fact_loader.rb index 292264a9de..16195b4f09 100644 --- a/lib/facter/framework/core/fact_loaders/fact_loader.rb +++ b/lib/facter/framework/core/fact_loaders/fact_loader.rb @@ -64,11 +64,34 @@ def load_external_facts(options) def block_facts(facts, options) blocked_facts = options[:blocked_facts] || [] + reject_list_core, reject_list_legacy = construct_reject_lists(blocked_facts, facts) + facts = facts.reject do |fact| - blocked_facts.select { |blocked_fact| fact.name.match(/^#{blocked_fact}/) }.any? + reject_list_core.include?(fact) || reject_list_core.find do |fact_to_block| + fact_to_block.klass == fact.klass + end || reject_list_legacy.include?(fact) end facts end + + def construct_reject_lists(blocked_facts, facts) + reject_list_core = [] + reject_list_legacy = [] + + blocked_facts.each do |blocked| + facts.each do |fact| + next unless fact.name =~ /^#{blocked}/ + + if fact.type == :core + reject_list_core << fact + else + reject_list_legacy << fact + end + end + end + + [reject_list_core, reject_list_legacy] + end end end diff --git a/spec/facter/fact_filter_spec.rb b/spec/facter/fact_filter_spec.rb index 7e59d6465a..0c60487325 100644 --- a/spec/facter/fact_filter_spec.rb +++ b/spec/facter/fact_filter_spec.rb @@ -48,6 +48,25 @@ expect(resolved_fact.value).to eq('value_1') end end + + context 'with legacy fact that should be blocked' do + let(:fact_value) { 'value_1' } + let(:resolved_fact) { Facter::ResolvedFact.new('my_fact', fact_value, :legacy) } + + before do + resolved_fact.user_query = '' + resolved_fact.filter_tokens = [] + allow(Facter::Options).to receive(:[]).with(:blocked_facts).and_return(['my_fact']) + allow(Facter::Options).to receive(:[]).with(:show_legacy).and_return(true) + allow(Facter::Options).to receive(:[]).with(:user_query).and_return('') + end + + it 'filters blocked legacy facts' do + fact_filter_input = [resolved_fact] + Facter::FactFilter.new.filter_facts!(fact_filter_input) + expect(fact_filter_input).to eq([]) + end + end end it 'filters value inside fact when value is array' do diff --git a/spec/framework/core/fact_loaders/fact_loader_spec.rb b/spec/framework/core/fact_loaders/fact_loader_spec.rb index c3d08cc7a5..81a6a7a156 100644 --- a/spec/framework/core/fact_loaders/fact_loader_spec.rb +++ b/spec/framework/core/fact_loaders/fact_loader_spec.rb @@ -2,17 +2,21 @@ describe Facter::FactLoader do describe '#load' do - let(:internal_fact_loader_double) { double(Facter::InternalFactLoader) } - let(:external_fact_loader_double) { double(Facter::ExternalFactLoader) } + let(:internal_fact_loader_double) { instance_spy(Facter::InternalFactLoader) } + let(:external_fact_loader_double) { instance_spy(Facter::ExternalFactLoader) } - let(:ubuntu_os_name) { double(Facts::Linux::Os::Name) } - let(:networking_class) { double(Facts::Windows::NetworkInterfaces) } + let(:ubuntu_os_name) { instance_spy(Facts::Linux::Os::Name) } + let(:networking_class) { instance_spy(Facts::Windows::NetworkInterfaces) } - let(:loaded_fact_os_name) { double(Facter::LoadedFact, name: 'os.name', klass: ubuntu_os_name, type: :core) } + let(:loaded_fact_os_name) { instance_spy(Facter::LoadedFact, name: 'os.name', klass: ubuntu_os_name, type: :core) } + let(:loaded_fact_os_name_legacy) do + instance_spy(Facter::LoadedFact, name: 'operatingsystem', klass: ubuntu_os_name, + type: :legacy) + end let(:loaded_fact_networking) do - double(Facter::LoadedFact, name: 'network_.*', klass: networking_class, type: :legacy) + instance_spy(Facter::LoadedFact, name: 'network_.*', klass: networking_class, type: :legacy) end - let(:loaded_fact_custom_fact) { double(Facter::LoadedFact, name: 'custom_fact', klass: nil, type: :custom) } + let(:loaded_fact_custom_fact) { instance_spy(Facter::LoadedFact, name: 'custom_fact', klass: nil, type: :custom) } before do Singleton.__init__(Facter::FactLoader) @@ -50,7 +54,7 @@ it 'blocks one internal fact' do options = { blocked_facts: ['os.name'] } - facts_to_load = [loaded_fact_os_name] + facts_to_load = [loaded_fact_os_name, loaded_fact_os_name_legacy] allow(internal_fact_loader_double).to receive(:core_facts).and_return(facts_to_load) allow(external_fact_loader_double).to receive(:custom_facts).and_return([]) @@ -60,6 +64,19 @@ expect(loaded_facts.size).to eq(0) end + it 'blocks one legacy fact' do + options = { blocked_facts: ['operatingsystem'] } + + facts_to_load = [loaded_fact_os_name, loaded_fact_os_name_legacy] + + allow(internal_fact_loader_double).to receive(:core_facts).and_return(facts_to_load) + allow(external_fact_loader_double).to receive(:custom_facts).and_return([]) + allow(external_fact_loader_double).to receive(:external_facts).and_return([]) + + loaded_facts = Facter::FactLoader.instance.load(options) + expect(loaded_facts.size).to eq(1) + end + context 'when blocking custom facts' do before do facts_to_load = [loaded_fact_custom_fact]