(FACT-2553) Quote special string in YAML format

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --exclude-limit 1000`
-# on 2020-05-06 13:07:31 +0300 using RuboCop version 0.74.0.
+# on 2020-05-07 16:13:14 +0300 using RuboCop version 0.74.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
@@ -110,7 +110,7 @@ RSpec/LeakyConstantDeclaration:
     - 'spec/facter/resolvers/macosx/mountpoints_resolver_spec.rb'
     - 'spec/facter/resolvers/utils/windows/network_utils_spec.rb'
 
-# Offense count: 114
+# Offense count: 108
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
@@ -146,7 +146,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/windows/dmi/product/name_spec.rb'
     - 'spec/facter/facts/windows/dmi/product/serial_number_spec.rb'
     - 'spec/facter/facts/windows/dmi/product/uuid_spec.rb'
-    - 'spec/facter/facts/windows/fips_enabled_spec.rb'
     - 'spec/facter/facts/windows/identity/user_spec.rb'
     - 'spec/facter/facts/windows/ipaddress6_interfaces_spec.rb'
     - 'spec/facter/facts/windows/ipaddress_interfaces_spec.rb'
@@ -176,7 +175,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/windows/ruby/sitedir_spec.rb'
     - 'spec/facter/facts/windows/scope6_interfaces_spec.rb'
     - 'spec/facter/resolvers/utils/aix/odm_query_spec.rb'
-    - 'spec/framework/core/fact_loaders/external_fact_loader_spec.rb'
 
 # Offense count: 26
 RSpec/SubjectStub:
@@ -189,7 +187,7 @@ RSpec/SubjectStub:
     - 'spec/custom_facts/util/fact_spec.rb'
     - 'spec/custom_facts/util/resolution_spec.rb'
 
-# Offense count: 162
+# Offense count: 132
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Exclude:
@@ -217,7 +215,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/facts/windows/networking/dhcp_spec.rb'
     - 'spec/facter/facts/windows/networking/interfaces_spec.rb'
     - 'spec/facter/facts/windows/networking/mtu_spec.rb'
-    - 'spec/facter/facts/windows/networking/primary_spec.rb'
     - 'spec/facter/facts/windows/processors/models_spec.rb'
     - 'spec/facter/facts/windows/virtualization/is_virtual_spec.rb'
     - 'spec/facter/facts/windows/virtualization/virtual_spec.rb'
@@ -236,11 +233,9 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/resolvers/windows/identity_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/kernel_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/memory_resolver_spec.rb'
-    - 'spec/facter/resolvers/windows/networking_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/processors_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/system32_resolver_spec.rb'
     - 'spec/facter/resolvers/windows/uptime_resolver_spec.rb'
-    - 'spec/facter/resolvers/windows/virtualization_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'

lib/facts/windows/networking/primary.rb

@@ -7,7 +7,7 @@ class Primary
         FACT_NAME = 'networking.primary'
 
         def call_the_resolver
-          fact_value = Facter::Resolvers::Networking.resolve(:primary)
+          fact_value = Facter::Resolvers::Networking.resolve(:primary_interface)
 
           Facter::ResolvedFact.new(FACT_NAME, fact_value)
         end

lib/framework/formatters/yaml_fact_formatter.rb

@@ -6,13 +6,59 @@ def initialize
       @log = Log.new(self)
     end
 
-    def format(fact_hash)
-      yaml_pretty = YAML.dump(JSON.parse(JsonFactFormatter.new.format(fact_hash)))
+    def format(resolved_facts)
+      user_queries = resolved_facts.uniq(&:user_query).map(&:user_query)
+
+      facts_to_display = if user_queries.count == 1 && user_queries.first.empty?
+                           FormatterHelper.retrieve_fact_collection(resolved_facts)
+                         else
+                           FormatterHelper.retrieve_facts_to_display_for_user_query(user_queries, resolved_facts)
+                         end
+
+      facts_to_display = Psych.parse_stream(facts_to_display.to_yaml)
+      facts_to_display.children[0].tag_directives = []
+      yaml_pretty = quote_special_strings(facts_to_display)
 
-      @log.debug('Replace single backslash with double backslashes')
-      yaml_pretty.gsub!(/\\/, '\&\&')
       @log.debug('Replace ---  from yaml beginning, to keep it compatible with C facter')
       yaml_pretty.gsub(/^---[\r\n]+/, '')
     end
+
+    private
+
+    def quote_special_strings(fact_hash)
+      require 'psych'
+
+      fact_hash.grep(Psych::Nodes::Scalar).each do |node|
+        next unless needs_quote?(node.value)
+
+        node.plain  = false
+        node.quoted = true
+        node.style  = Psych::Nodes::Scalar::DOUBLE_QUOTED
+      end
+
+      fact_hash = unquote_keys(fact_hash)
+      fact_hash.yaml
+    end
+
+    def unquote_keys(fact_hash)
+      fact_hash.grep(Psych::Nodes::Mapping).each do |node|
+        node.children.each_slice(2) do |k, _|
+          k.plain  = true
+          k.quoted = false
+          k.style  = Psych::Nodes::Scalar::ANY
+        end
+      end
+      fact_hash
+    end
+
+    def needs_quote?(value)
+      return false if value =~ /true|false/
+      return false if value[/^[0-9]+$/]
+      return true if value =~ /y|Y|yes|Yes|YES|n|N|no|No|NO|True|TRUE|False|FALSE|on|On|ON|off|Off|OFF|:/
+      return false if value[/[a-zA-Z]/]
+      return false if value[/[0-9]+\.[0-9]+\./]
+
+      true
+    end
   end
 end

lib/resolvers/windows/ffi/network_utils.rb

@@ -42,7 +42,7 @@ def build_binding(addr, mask_length)
              else
                IPAddr.new('255.255.255.255').mask(mask_length)
              end
-      { address: addr, netmask: mask, network: ip.mask(mask_length) }
+      { address: addr, netmask: mask.to_s, network: ip.mask(mask_length).to_s }
     end
 
     def get_scope(sockaddr)

lib/resolvers/windows/networking_resolver.rb

@@ -27,6 +27,7 @@ def read_network_information(fact_name)
 
           iterate_list(adapter_addresses)
           set_interfaces_other_facts if @fact_list[:interfaces]
+          @fact_list[:primary_interface] = @fact_list[:primary_interface].to_s
           @fact_list[fact_name]
         end
 
@@ -112,8 +113,8 @@ def add_ip_data(addr, unicast, sock_addr, bindings)
           result = find_bindings(sock_addr, unicast, addr)
           return unless result
 
-          bindings[:ipv6] << result if result[:network].ipv6?
-          bindings[:ipv4] << result if result[:network].ipv4?
+          bindings[:ipv6] << result if sock_addr[:sa_family] == NetworkingFFI::AF_INET6
+          bindings[:ipv4] << result if sock_addr[:sa_family] == NetworkingFFI::AF_INET
         end
 
         def find_bindings(sock_addr, unicast, addr)
@@ -126,7 +127,7 @@ def find_primary_interface(sock_addr, name, addr)
           if !@fact_list[:primary_interface] &&
              ([NetworkingFFI::AF_INET, NetworkingFFI::AF_INET6].include?(sock_addr[:sa_family]) &&
                  !NetworkUtils.ignored_ip_address(addr))
-            @fact_list[:primary] = name
+            @fact_list[:primary_interface] = name
           end
         end
 
@@ -138,7 +139,7 @@ def set_interfaces_other_facts
             end
             if value[:bindings6]
               binding = find_valid_binding(value[:bindings6])
-              populate_interface(binding, value)
+              populate_interface(binding, value, true)
             end
             set_networking_other_facts(value, interface_name)
           end
@@ -151,10 +152,10 @@ def find_valid_binding(bindings)
           bindings.empty? ? nil : bindings.first
         end
 
-        def populate_interface(bind, interface)
+        def populate_interface(bind, interface, ipv6 = false)
           return if !bind || bind.empty?
 
-          if bind[:network].ipv6?
+          if ipv6
             interface[:ip6] = bind[:address]
             interface[:netmask6] = bind[:netmask]
             interface[:network6] = bind[:network]
@@ -167,7 +168,7 @@ def populate_interface(bind, interface)
         end
 
         def set_networking_other_facts(value, interface_name)
-          return unless @fact_list[:primary] == interface_name
+          return unless @fact_list[:primary_interface] == interface_name
 
           %i[mtu dhcp mac ip ip6 scope6 netmask netmask6 network network6].each do |key|
             @fact_list[key] = value[key]

lib/resolvers/windows/virtualization_resolver.rb

@@ -11,6 +11,9 @@ class << self
         # Virtual
         # Is_Virtual
 
+        MODEL_HASH = { 'VirtualBox' => 'virtualbox', 'VMware' => 'vmware', 'KVM' => 'kvm',
+                       'Bochs' => 'bochs', 'Google' => 'gce', 'OpenStack' => 'openstack' }.freeze
+
         private
 
         def post_resolve(fact_name)
@@ -31,9 +34,7 @@ def read_fact_from_computer_system(fact_name)
         end
 
         def determine_hypervisor_by_model(comp)
-          model_hash = { 'VirtualBox' => 'virtualbox', 'VMware' => 'vmware', 'KVM' => 'kvm',
-                         'Bochs' => 'bochs', 'Google' => 'gce', 'OpenStack' => 'openstack' }
-          model_hash[model_hash.keys.find { |key| comp.Model =~ /^#{key}/ }]
+          MODEL_HASH[MODEL_HASH.keys.find { |key| comp.Model =~ /^#{key}/ }]
         end
 
         def determine_hypervisor_by_manufacturer(comp)
@@ -57,7 +58,7 @@ def build_fact_list(comp)
           hypervisor = determine_hypervisor_by_model(comp) || determine_hypervisor_by_manufacturer(comp)
 
           @fact_list[:virtual] = hypervisor
-          @fact_list[:is_virtual] = (!hypervisor.include?('physical')).to_s
+          @fact_list[:is_virtual] = hypervisor.include?('physical') ? false : true
         end
       end
     end

spec/facter/facts/windows/fips_enabled_spec.rb

@@ -11,8 +11,8 @@
     end
 
     it 'calls Facter::Windows::Resolvers::Fips' do
-      expect(Facter::Resolvers::Windows::Fips).to receive(:resolve).with(:fips_enabled)
       fact.call_the_resolver
+      expect(Facter::Resolvers::Windows::Fips).to have_received(:resolve).with(:fips_enabled)
     end
 
     it 'returns true if fips enabled' do

spec/facter/facts/windows/networking/primary_spec.rb

@@ -2,13 +2,22 @@
 
 describe Facts::Windows::Networking::Primary do
   describe '#call_the_resolver' do
-    it 'returns a fact' do
-      expected_fact = double(Facter::ResolvedFact, name: 'networking.primary', value: 'value')
-      allow(Facter::Resolvers::Networking).to receive(:resolve).with(:primary).and_return('value')
-      allow(Facter::ResolvedFact).to receive(:new).with('networking.primary', 'value').and_return(expected_fact)
+    subject(:fact) { Facts::Windows::Networking::Primary.new }
 
-      fact = Facts::Windows::Networking::Primary.new
-      expect(fact.call_the_resolver).to eq(expected_fact)
+    let(:value) { 'Ethernet0' }
+
+    before do
+      allow(Facter::Resolvers::Networking).to receive(:resolve).with(:primary_interface).and_return(value)
+    end
+
+    it 'calls Facter::Windows::Resolvers::Fips' do
+      fact.call_the_resolver
+      expect(Facter::Resolvers::Networking).to have_received(:resolve).with(:primary_interface)
+    end
+
+    it 'returns true if fips enabled' do
+      expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \
+        have_attributes(name: 'networking.primary', value: value)
     end
   end
 end

spec/facter/resolvers/utils/windows/network_utils_spec.rb

@@ -3,9 +3,9 @@
 describe NetworkUtils do
   describe '#address_to_strig' do
     let(:logger) { instance_spy(Facter::Log) }
-    let(:addr) { double('SocketAddress') }
-    let(:size) { double(FFI::MemoryPointer) }
-    let(:buffer) { double(FFI::MemoryPointer) }
+    let(:addr) { instance_spy('SocketAddress') }
+    let(:size) { instance_spy(FFI::MemoryPointer) }
+    let(:buffer) { instance_spy(FFI::MemoryPointer) }
     let(:length) { 32 }
 
     before do
@@ -31,7 +31,7 @@
     end
 
     context 'when error code is zero' do
-      let(:address) { double(FFI::MemoryPointer) }
+      let(:address) { instance_spy(FFI::MemoryPointer) }
       let(:error) { 0 }
 
       it 'returns an address' do
@@ -40,7 +40,7 @@
     end
 
     context 'when error code is not zero' do
-      let(:address) { double(FFI::MemoryPointer) }
+      let(:address) { instance_spy(FFI::MemoryPointer) }
       let(:error) { 1 }
 
       it 'returns nil and logs debug message' do
@@ -90,8 +90,8 @@
 
   describe '#build_binding' do
     context 'when input is ipv4 address' do
-      let(:netmask) { IPAddr.new('255.255.240.0/255.255.240.0') }
-      let(:network) { IPAddr.new('10.16.112.0/255.255.240.0') }
+      let(:netmask) { IPAddr.new('255.255.240.0/255.255.240.0').to_s }
+      let(:network) { IPAddr.new('10.16.112.0/255.255.240.0').to_s }
       let(:addr) { '10.16.121.248' }
 
       it 'returns ipv4 binding' do
@@ -100,8 +100,12 @@
     end
 
     context 'when input is ipv6 address' do
-      let(:network) { IPAddr.new('fe80:0000:0000:0000:0000:0000:0000:0000/ffff:ffff:ffff:ffff:0000:0000:0000:0000') }
-      let(:netmask) { IPAddr.new('ffff:ffff:ffff:ffff:0000:0000:0000:0000/ffff:ffff:ffff:ffff:0000:0000:0000:0000') }
+      let(:network) do
+        IPAddr.new('fe80:0000:0000:0000:0000:0000:0000:0000/ffff:ffff:ffff:ffff:0000:0000:0000:0000').to_s
+      end
+      let(:netmask) do
+        IPAddr.new('ffff:ffff:ffff:ffff:0000:0000:0000:0000/ffff:ffff:ffff:ffff:0000:0000:0000:0000').to_s
+      end
       let(:addr) { 'fe80::dc20:a2b9:5253:9b46' }
 
       it 'returns ipv6 binding' do