Merge pull request #7349 from chef/2k8r2_powershell

Support windows_feature_powershell on Windows 2008 R2

5 files changed, 70 insertions, 31 deletions

diff --git a/lib/chef/platform/query_helpers.rb b/lib/chef/platform/query_helpers.rb
index b49010efc0..448885dfbc 100644
--- a/lib/chef/platform/query_helpers.rb
+++ b/lib/chef/platform/query_helpers.rb
@@ -20,6 +20,12 @@ class Chef
   class Platform
 
     class << self
+      # a simple helper to determine if we're on a windows release pre-2012 / 8
+      # @return [Boolean] Is the system older than Windows 8 / 2012
+      def older_than_win_2012_or_8?
+        node["platform_version"].to_f < 6.2
+      end
+
       def windows?
         ChefConfig.windows?
       end
diff --git a/lib/chef/resource/windows_feature_dism.rb b/lib/chef/resource/windows_feature_dism.rb
index ebd52c4db7..fc6f25fb3f 100644
--- a/lib/chef/resource/windows_feature_dism.rb
+++ b/lib/chef/resource/windows_feature_dism.rb
@@ -17,6 +17,7 @@
 #
 
 require "chef/resource"
+require "chef/platform/query_helpers"
 
 class Chef
   class Resource
@@ -30,7 +31,7 @@ class Chef
 
       property :feature_name, [Array, String],
                description: "The name of the feature/role(s) to install if it differs from the resource name.",
-               coerce: proc { |x| to_lowercase_array(x) },
+               coerce: proc { |x| to_formatted_array(x) },
                name_property: true
 
       property :source, String,
@@ -44,12 +45,12 @@ class Chef
                description: "Specifies a timeout (in seconds) for feature install.",
                default: 600
 
-      def to_lowercase_array(x)
+      # @return [Array] lowercase the array unless we're on < Windows 2012
+      def to_formatted_array(x)
         x = x.split(/\s*,\s*/) if x.is_a?(String) # split multiple forms of a comma separated list
 
-        # dism on windows < 2012 is case sensitive so only downcase when on 2012+
-        # @todo when we're really ready to remove support for Windows 2008 R2 this check can go away
-        node["platform_version"].to_f < 6.2 ? x : x.map(&:downcase)
+        # feature installs on windows < 2012 are case sensitive so only downcase when on 2012+
+        Chef::Platform.older_than_win_2012_or_8? ? x : x.map(&:downcase)
       end
 
       action :install do
@@ -200,7 +201,7 @@ class Chef
           # dism on windows 2012+ isn't case sensitive so it's best to compare
           # lowercase lists so the user input doesn't need to be case sensitive
           # @todo when we're ready to remove windows 2008R2 the gating here can go away
-          feature_details.downcase! unless node["platform_version"].to_f < 6.2
+          feature_details.downcase! unless Chef::Platform.older_than_win_2012_or_8?
           node.override["dism_features_cache"][feature_type] << feature_details
         end
 
@@ -208,7 +209,7 @@ class Chef
         # @return [void]
         def fail_if_removed
           return if new_resource.source # if someone provides a source then all is well
-          if node["platform_version"].to_f > 6.2
+          if node["platform_version"].to_f > 6.2 # 2012R2 or later
             return if registry_key_exists?('HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\Servicing') && registry_value_exists?('HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\Servicing', name: "LocalSourcePath") # if source is defined in the registry, still fine
           end
           removed = new_resource.feature_name & node["dism_features_cache"]["removed"]
@@ -218,7 +219,7 @@ class Chef
         # Fail unless we're on windows 8+ / 2012+ where deleting a feature is supported
         # @return [void]
         def raise_if_delete_unsupported
-          raise Chef::Exceptions::UnsupportedAction, "#{self} :delete action not support on Windows releases before Windows 8/2012. Cannot continue!" unless node["platform_version"].to_f >= 6.2
+          raise Chef::Exceptions::UnsupportedAction, "#{self} :delete action not supported on Windows releases before Windows 8/2012. Cannot continue!" if Chef::Platform.older_than_win_2012_or_8?
         end
       end
     end
diff --git a/lib/chef/resource/windows_feature_powershell.rb b/lib/chef/resource/windows_feature_powershell.rb
index 82713e3438..daee0e9d34 100644
--- a/lib/chef/resource/windows_feature_powershell.rb
+++ b/lib/chef/resource/windows_feature_powershell.rb
@@ -19,6 +19,7 @@
 require "chef/mixin/powershell_out"
 require "chef/json_compat"
 require "chef/resource"
+require "chef/platform/query_helpers"
 
 class Chef
   class Resource
@@ -35,7 +36,7 @@ class Chef
 
       property :feature_name, [Array, String],
                description: "The name of the feature/role(s) to install if it differs from the resource name.",
-               coerce: proc { |x| to_lowercase_array(x) },
+               coerce: proc { |x| to_formatted_array(x) },
                name_property: true
 
       property :source, String,
@@ -54,9 +55,13 @@ class Chef
                description: "",
                default: false
 
-      def to_lowercase_array(x)
+      # Converts strings of features into an Array. Array objects are lowercased unless we're on < 8/2k12+.
+      # @return [Array] array of features
+      def to_formatted_array(x)
         x = x.split(/\s*,\s*/) if x.is_a?(String) # split multiple forms of a comma separated list
-        x.map(&:downcase)
+
+        # feature installs on windows < 8/2012 are case sensitive so only downcase when on 2012+
+        Chef::Platform.older_than_win_2012_or_8? ? x : x.map(&:downcase)
       end
 
       include Chef::Mixin::PowershellOut
@@ -73,8 +78,8 @@ class Chef
           converge_by("install Windows feature#{'s' if features_to_install.count > 1} #{features_to_install.join(',')}") do
             install_command = "#{install_feature_cmdlet} #{features_to_install.join(',')}"
             install_command << " -IncludeAllSubFeature"  if new_resource.all
-            if node["platform_version"].to_f < 6.2 && (new_resource.source || new_resource.management_tools)
-              Chef::Log.warn("The 'source' and 'management_tools' properties are not available on Windows 2012R2 or great. Skipping these properties!")
+            if Chef::Platform.older_than_win_2012_or_8? && (new_resource.source || new_resource.management_tools)
+              Chef::Log.warn("The 'source' and 'management_tools' properties are only available on Windows 8/2012 or greater. Skipping these properties!")
             else
               install_command << " -Source \"#{new_resource.source}\"" if new_resource.source
               install_command << " -IncludeManagementTools" if new_resource.management_tools
@@ -148,12 +153,16 @@ class Chef
           raise "The windows_feature_powershell resource requires PowerShell 3.0 or later. Please install PowerShell 3.0+ before running this resource." if powershell_version < 3
         end
 
+        # The appropriate cmdlet to install a windows feature based on windows release
+        # @return [String]
         def install_feature_cmdlet
-          node["platform_version"].to_f < 6.2 ? "Import-Module ServerManager; Add-WindowsFeature" : "Install-WindowsFeature"
+          Chef::Platform.older_than_win_2012_or_8? ? "Add-WindowsFeature" : "Install-WindowsFeature"
         end
 
+        # The appropriate cmdlet to remove a windows feature based on windows release
+        # @return [String]
         def remove_feature_cmdlet
-          node["platform_version"].to_f < 6.2 ? "Import-Module ServerManager; Remove-WindowsFeature" : "Uninstall-WindowsFeature"
+          Chef::Platform.older_than_win_2012_or_8? ? "Remove-WindowsFeature" : "Uninstall-WindowsFeature"
         end
 
         # @return [Array] features the user has requested to install which need installation
@@ -220,8 +229,9 @@ class Chef
         # fetch the list of available feature names and state in JSON and parse the JSON
         def parsed_feature_list
           # Grab raw feature information from dism command line
-          raw_list_of_features = if node["platform_version"].to_f < 6.2
-                                   powershell_out!("Import-Module ServerManager; Get-WindowsFeature | Select-Object -Property Name,InstallState | ConvertTo-Json -Compress", timeout: new_resource.timeout).stdout
+          # Windows < 2012 doesn't present a state value so we have to check if the feature is installed or not
+          raw_list_of_features = if Chef::Platform.older_than_win_2012_or_8? # make the older format look like the new format, warts and all
+                                   powershell_out!('Get-WindowsFeature | Select-Object -Property Name, @{Name=\"InstallState\"; Expression = {If ($_.Installed) { 1 } Else { 0 }}} | ConvertTo-Json -Compress', timeout: new_resource.timeout).stdout
                                  else
                                    powershell_out!("Get-WindowsFeature | Select-Object -Property Name,InstallState | ConvertTo-Json -Compress", timeout: new_resource.timeout).stdout
                                  end
@@ -232,14 +242,15 @@ class Chef
         # add the features values to the appropriate array
         # @return [void]
         def add_to_feature_mash(feature_type, feature_details)
-          node.override["powershell_features_cache"][feature_type] << feature_details.downcase # lowercase so we can compare properly
+          # add the lowercase feature name to the mash unless we're on < 2012 where they're case sensitive
+          node.override["powershell_features_cache"][feature_type] << (Chef::Platform.older_than_win_2012_or_8? ? feature_details : feature_details.downcase)
         end
 
         # Fail if any of the packages are in a removed state
         # @return [void]
         def fail_if_removed
           return if new_resource.source # if someone provides a source then all is well
-          if node["platform_version"].to_f > 6.2
+          if node["platform_version"].to_f > 6.2 # 2012R2 or later
             return if registry_key_exists?('HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\Servicing') && registry_value_exists?('HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\Servicing', name: "LocalSourcePath") # if source is defined in the registry, still fine
           end
           removed = new_resource.feature_name & node["powershell_features_cache"]["removed"]
@@ -248,7 +259,7 @@ class Chef
 
         # Fail unless we're on windows 8+ / 2012+ where deleting a feature is supported
         def raise_if_delete_unsupported
-          raise Chef::Exceptions::UnsupportedAction, "#{self} :delete action not support on Windows releases before Windows 8/2012. Cannot continue!" unless node["platform_version"].to_f >= 6.2
+          raise Chef::Exceptions::UnsupportedAction, "#{self} :delete action not supported on Windows releases before Windows 8/2012. Cannot continue!" if Chef::Platform.older_than_win_2012_or_8?
         end
       end
     end
diff --git a/spec/unit/resource/windows_feature_dism.rb b/spec/unit/resource/windows_feature_dism.rb
index 4627387ddb..87d99ecbaf 100644
--- a/spec/unit/resource/windows_feature_dism.rb
+++ b/spec/unit/resource/windows_feature_dism.rb
@@ -32,7 +32,7 @@ describe Chef::Resource::WindowsFeatureDism do
   end
 
   it "the feature_name property is the name_property" do
-    node.automatic[:platform_version] = "6.2"
+    node.automatic[:platform_version] = "6.2.9200"
     expect(resource.feature_name).to eql(%w{snmp dhcp})
   end
 
@@ -47,25 +47,25 @@ describe Chef::Resource::WindowsFeatureDism do
   end
 
   it "coerces comma separated lists of features to a lowercase array on 2012+" do
-    node.automatic[:platform_version] = "6.2"
+    node.automatic[:platform_version] = "6.2.9200"
     resource.feature_name "SNMP, DHCP"
     expect(resource.feature_name).to eql(%w{snmp dhcp})
   end
 
   it "coerces a single feature as a String to a lowercase array on 2012+" do
-    node.automatic[:platform_version] = "6.2"
+    node.automatic[:platform_version] = "6.2.9200"
     resource.feature_name "SNMP"
     expect(resource.feature_name).to eql(["snmp"])
   end
 
   it "coerces comma separated lists of features to an array, but preserves case on < 2012" do
-    node.automatic[:platform_version] = "6.1"
+    node.automatic[:platform_version] = "6.1.7601"
     resource.feature_name "SNMP, DHCP"
     expect(resource.feature_name).to eql(%w{SNMP DHCP})
   end
 
   it "coerces a single feature as a String to an array, but preserves case on < 2012" do
-    node.automatic[:platform_version] = "6.1"
+    node.automatic[:platform_version] = "6.1.7601"
     resource.feature_name "SNMP"
     expect(resource.feature_name).to eql(["SNMP"])
   end
diff --git a/spec/unit/resource/windows_feature_powershell.rb b/spec/unit/resource/windows_feature_powershell.rb
index a9b6f5f88f..3dc1604361 100644
--- a/spec/unit/resource/windows_feature_powershell.rb
+++ b/spec/unit/resource/windows_feature_powershell.rb
@@ -18,7 +18,10 @@
 require "spec_helper"
 
 describe Chef::Resource::WindowsFeaturePowershell do
-  let(:resource) { Chef::Resource::WindowsFeaturePowershell.new(%w{SNMP DHCP}) }
+  let(:node) { Chef::Node.new }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:resource) { Chef::Resource::WindowsFeaturePowershell.new(%w{SNMP DHCP}, run_context) }
 
   it "sets resource name as :windows_feature_powershell" do
     expect(resource.resource_name).to eql(:windows_feature_powershell)
@@ -28,24 +31,42 @@ describe Chef::Resource::WindowsFeaturePowershell do
     expect(resource.action).to eql([:install])
   end
 
+  it "the feature_name property is the name_property" do
+    node.automatic[:platform_version] = "6.2.9200"
+    expect(resource.feature_name).to eql(%w{snmp dhcp})
+  end
+
+  it "sets the default action as :install" do
+    expect(resource.action).to eql([:install])
+  end
+
   it "supports :delete, :install, :remove actions" do
     expect { resource.action :delete }.not_to raise_error
     expect { resource.action :install }.not_to raise_error
     expect { resource.action :remove }.not_to raise_error
   end
 
-  it "sets the feature_name property as its name_property" do
-    expect(resource.feature_name).to eql(%w{SNMP DHCP})
+  it "coerces comma separated lists of features to a lowercase array on 2012+" do
+    node.automatic[:platform_version] = "6.2.9200"
+    resource.feature_name "SNMP, DHCP"
+    expect(resource.feature_name).to eql(%w{snmp dhcp})
+  end
+
+  it "coerces a single feature as a String to a lowercase array on 2012+" do
+    node.automatic[:platform_version] = "6.2.9200"
+    resource.feature_name "SNMP"
+    expect(resource.feature_name).to eql(["snmp"])
   end
 
-  it "coerces comma separated lists of features to arrays" do
+  it "coerces comma separated lists of features to an array, but preserves case on < 2012" do
+    node.automatic[:platform_version] = "6.1.7601"
     resource.feature_name "SNMP, DHCP"
     expect(resource.feature_name).to eql(%w{SNMP DHCP})
   end
 
-  it "coerces a single feature as a String into an array" do
+  it "coerces a single feature as a String to an array, but preserves case on < 2012" do
+    node.automatic[:platform_version] = "6.1.7601"
     resource.feature_name "SNMP"
     expect(resource.feature_name).to eql(["SNMP"])
   end
-
 end