Prefer ipv4 for default_gateway over ipv6 on windows

- Minor fix to select default_interface based on the least metric value
- Fix to prefer default_fateway in IPV4 Format
- DRY-UP to get interface_code
- Commented as per YARD format
- Added test cases
- Fixes: MSYS-875

Signed-off-by: Nimesh <nimesh.patni@msystechnologies.com>

2 files changed, 204 insertions, 5 deletions

diff --git a/lib/ohai/plugins/windows/network.rb b/lib/ohai/plugins/windows/network.rb
index 88c3d1a5..ad1b74eb 100644
--- a/lib/ohai/plugins/windows/network.rb
+++ b/lib/ohai/plugins/windows/network.rb
@@ -53,6 +53,65 @@ Ohai.plugin(:Network) do
     data
   end
 
+  # Returns interface code for an interface
+  # 
+  # Interface Index (if present, Index otherwise) will be converted in hexadecimal format
+  #
+  # @param int_idx [String or nil] the interface index of interface
+  # @param idx [String] the index of interface
+  #
+  # @return [String]
+  #
+  # @example Interface Code when interface index is present
+  #   plugin.interface_code("1", "1") #=> "ox1"
+  # @example Interface Code when interface index is not present
+  #   plugin.interface_code(nil, "2") #=> "ox2"
+  #
+  def interface_code(int_idx, idx)
+    sprintf("0x%x", (int_idx || idx)).downcase
+  end
+
+  # Returns IPV4 address from list of addresses containing IPV4 and IPV6 formats
+  # 
+  # @param addresses [Array<String>] List of addresses
+  #
+  # @return [String]
+  #
+  # @example When list contains both IPV4 and IPV6 formats
+  #   plugin.prefer_ipv4([IPV4, IPV6]) #=> "IPV4"
+  # @example When list contains only IPV6 format
+  #   plugin.prefer_ipv4([IPV6]) #=> "IPV6"
+  #
+  def prefer_ipv4(addresses)
+    return nil unless addresses.is_a?(Array)
+    addresses.find { |ip| IPAddress.valid_ipv4?(ip) } ||
+      addresses.find { |ip| IPAddress.valid_ipv6?(ip) }
+  end
+
+  # Selects default interface and returns its information
+  #
+  # @note Interface with least metric value should be prefered as default_route
+  #
+  # @param configuration [Mash] Configuration of interfaces as iface_config
+  #   [<interface_index> => {<interface_configurations>}]
+  #
+  # @return [Hash<:index, :interface_index, :default_ip_gateway, :ip_connection_metric>]
+  #
+  def favored_default_route(configuration)
+    return nil unless configuration.is_a?(Hash)
+    config = configuration.dup
+
+    config.inject([]) do |arr, (k, v)|
+      if v["default_ip_gateway"]
+        arr << { index: v["index"],
+                 interface_index: v["interface_index"],
+                 default_ip_gateway: prefer_ipv4(v["default_ip_gateway"]),
+                 ip_connection_metric: v["ip_connection_metric"] }
+      end
+      arr
+    end.min_by { |r| r[:ip_connection_metric] }
+  end
+
   collect_data(:windows) do
 
     require "wmi-lite/wmi"
@@ -70,6 +129,7 @@ Ohai.plugin(:Network) do
       iface_config[i] = Mash.new unless iface_config[i]
       iface_config[i][:ip_address] ||= []
       iface_config[i][:ip_address] << adapter["IPAddress"]
+
       adapter.wmi_ole_object.properties_.each do |p|
         if iface_config[i][p.name.wmi_underscore.to_sym].nil?
           iface_config[i][p.name.wmi_underscore.to_sym] = adapter[p.name.downcase]
@@ -89,7 +149,7 @@ Ohai.plugin(:Network) do
 
     iface_instance.each_key do |i|
       if iface_instance[i][:name] && iface_config[i] && iface_config[i][:ip_address][0]
-        cint = sprintf("0x%x", (iface_instance[i][:interface_index] || iface_instance[i][:index]) ).downcase
+        cint = interface_code(iface_instance[i][:interface_index], iface_instance[i][:index])
         iface[cint] = Mash.new
         iface[cint][:configuration] = iface_config[i]
         iface[cint][:instance] = iface_instance[i]
@@ -97,6 +157,7 @@ Ohai.plugin(:Network) do
         iface[cint][:counters] = Mash.new
         iface[cint][:addresses] = Mash.new
         iface[cint][:configuration][:ip_address] = iface[cint][:configuration][:ip_address].flatten
+
         iface[cint][:configuration][:ip_address].each_index do |ip_index|
           ip = iface[cint][:configuration][:ip_address][ip_index]
           ip_and_subnet = ip.dup
@@ -127,13 +188,14 @@ Ohai.plugin(:Network) do
         iface[cint][:type] = iface[cint][:instance][:adapter_type] if iface[cint][:instance][:adapter_type]
         iface[cint][:arp] = {}
         iface[cint][:encapsulation] = windows_encaps_lookup(iface[cint][:instance][:adapter_type]) if iface[cint][:instance][:adapter_type]
-        if !iface[cint][:configuration][:default_ip_gateway].nil? && iface[cint][:configuration][:default_ip_gateway].size > 0
-          network[:default_gateway] = iface[cint][:configuration][:default_ip_gateway].first
-          network[:default_interface] = cint
-        end
       end
     end
 
+    if (route = favored_default_route(iface_config))
+      network[:default_gateway] = route[:default_ip_gateway]
+      network[:default_interface] = interface_code(route[:interface_index], route[:index])
+    end
+
     cint = nil
     so = shell_out("arp -a")
     if so.exitstatus == 0
diff --git a/spec/unit/plugins/windows/network_spec.rb b/spec/unit/plugins/windows/network_spec.rb
new file mode 100644
index 00000000..9202b513
--- /dev/null
+++ b/spec/unit/plugins/windows/network_spec.rb
@@ -0,0 +1,137 @@
+#
+#  Author:: Nimesh Pathi <nimesh.patni@msystechnologies.com>
+#  Copyright:: Copyright (c) 2018 Chef Software, Inc.
+#  License:: Apache License, Version 2.0
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+#
+
+require_relative "../../../spec_helper.rb"
+require "ipaddress"
+
+describe Ohai::System, "Windows Network Plugin" do
+  let(:plugin) { get_plugin("windows/network") }
+
+  before(:each) do
+    allow(plugin).to receive(:collect_os).and_return(:windows)
+  end
+
+  describe "#interface_code" do
+    let(:interface_idx) { 1 }
+    let(:index) { 2 }
+    context "when interface index is given" do
+      it "Returns a valid string having hexadecimal interface_index" do
+        index = nil
+        expect(plugin.interface_code(interface_idx, index)).to eq("0x1")
+      end
+    end
+    context "when interface index is not given" do
+      it "Returns a valid string having hexadecimal index" do
+        interface_idx = nil
+        expect(plugin.interface_code(interface_idx, index)).to eq("0x2")
+      end
+    end
+  end
+
+  describe "#prefer_ipv4" do
+    let(:inet4) { "192.168.1.1" }
+    let(:inet6) { "fe80::2fe:c8ff:fef5:c88f" }
+    context "When Array is not passed" do
+      it "Returns nil" do
+        expect(plugin.prefer_ipv4("Invalid")).to be_nil
+      end
+    end
+    context "When no address is passed in Array" do
+      it "Returns nil" do
+        expect(plugin.prefer_ipv4([])).to be_nil
+      end
+    end
+    context "Preferred chances of IPV4 address" do
+      it "Returns the address when only IPV4 address is passed" do
+        expect(plugin.prefer_ipv4([inet4])).to eq(inet4)
+      end
+      it "Returns the address when IPV6 is also present at latter place" do
+        expect(plugin.prefer_ipv4([inet4, inet6])).to eq(inet4)
+      end
+      it "Returns the address when IPV6 is also present at former place" do
+        expect(plugin.prefer_ipv4([inet6, inet4])).to eq(inet4)
+      end
+    end
+    context "Preferred chances of IPV6 address" do
+      it "Returns the address when only IPV6 address is passed" do
+        expect(plugin.prefer_ipv4([inet4])).to eq(inet4)
+      end
+      it "Does not return the address if IPV4 is also present at former place" do
+        expect(plugin.prefer_ipv4([inet4, inet6])).not_to eq(inet6)
+      end
+      it "Does not return the address if IPV4 is also present at latter place" do
+        expect(plugin.prefer_ipv4([inet6, inet4])).not_to eq(inet6)
+      end
+    end
+  end
+
+  describe "#favored_default_route" do
+    let(:interface1) do
+      { "index" => 1,
+        "interface_index" => 1,
+        "ip_connection_metric" => 10,
+        "default_ip_gateway" => ["fe80::2fe:c8ff:fef5:c88f", "192.168.1.1"] }
+    end
+    let(:iface_config) { { 1 => interface1 } }
+    context "When a hash is not passed" do
+      it "Returns nil" do
+        expect(plugin.favored_default_route("Invalid")).to be_nil
+      end
+    end
+    context "When no interface is passed in Hash" do
+      it "Returns nil" do
+        expect(plugin.favored_default_route({})).to be_nil
+      end
+    end
+    context "When an interface configuration is passed" do
+      context "without default_ip_gateway" do
+        it "Returns nil" do
+          interface1["default_ip_gateway"] = nil
+          expect(plugin.favored_default_route(iface_config)).to be_nil
+        end
+      end
+      context "with default_ip_gateway" do
+        it "Returns a hash with details" do
+          expect(plugin.favored_default_route(iface_config)).to be_a(Hash)
+          expect(plugin.favored_default_route(iface_config)).not_to be_empty
+        end
+        it "Returns the default_gateway in IPV4 format" do
+          expect(plugin.favored_default_route(iface_config)).to include(default_ip_gateway: "192.168.1.1")
+        end
+      end
+    end
+    context "When multiple interfaces are passed" do
+      let(:interface2) do
+        { "index" => 2,
+          "interface_index" => 3,
+          "ip_connection_metric" => 20,
+          "default_ip_gateway" => ["192.168.1.2"] }
+      end
+      let(:iface_config) do
+        { 1 => interface1,
+          2 => interface2 }
+      end
+      it "Returns the default route as least metric interface" do
+        expect(plugin.favored_default_route(iface_config)).to include(interface_index: 1)
+      end
+      it "Returns its default_gateway in IPV4 format" do
+        expect(plugin.favored_default_route(iface_config)).to include(default_ip_gateway: "192.168.1.1")
+      end
+    end
+  end
+end