Merge pull request #2313 from opscode/sersut/win-res-lookup-fix

Make sure windows_service and windows_package resources are found with the new dynamic provider resolver.
Conflicts:
	spec/unit/resource/windows_package_spec.rb

11 files changed, 70 insertions, 58 deletions

diff --git a/lib/chef/config.rb b/lib/chef/config.rb
index d033a2981b..dc04026acb 100644
--- a/lib/chef/config.rb
+++ b/lib/chef/config.rb
@@ -458,7 +458,7 @@ class Chef
     default :disable_event_loggers, false
     default :event_loggers do
       evt_loggers = []
-      if Chef::Platform::windows?
+      if Chef::Platform::windows? and not Chef::Platform::windows_server_2003?
         evt_loggers << :win_evt
       end
       evt_loggers
diff --git a/lib/chef/event_loggers/windows_eventlog.rb b/lib/chef/event_loggers/windows_eventlog.rb
index e3bbbfa1e6..b8d279594b 100644
--- a/lib/chef/event_loggers/windows_eventlog.rb
+++ b/lib/chef/event_loggers/windows_eventlog.rb
@@ -19,7 +19,7 @@
 require 'chef/event_loggers/base'
 require 'chef/platform/query_helpers'
 
-if Chef::Platform::windows?
+if Chef::Platform::windows? and not Chef::Platform::windows_server_2003?
   [:INFINITE, :WAIT_FAILED, :FORMAT_MESSAGE_IGNORE_INSERTS, :ERROR_INSUFFICIENT_BUFFER].each do |c|
     # These are redefined in 'win32/eventlog'
     Windows::Constants.send(:remove_const, c)
@@ -56,7 +56,7 @@ class Chef
 
       def run_start(version)
         @eventlog.report_event(
-          :event_type => EventLog::INFO_TYPE, 
+          :event_type => EventLog::INFO_TYPE,
           :source => SOURCE,
           :event_id => RUN_START_EVENT_ID,
           :data => [version]
@@ -66,7 +66,7 @@ class Chef
       def run_started(run_status)
         @run_status = run_status
         @eventlog.report_event(
-          :event_type => EventLog::INFO_TYPE, 
+          :event_type => EventLog::INFO_TYPE,
           :source => SOURCE,
           :event_id => RUN_STARTED_EVENT_ID,
           :data => [run_status.run_id]
@@ -75,7 +75,7 @@ class Chef
 
       def run_completed(node)
         @eventlog.report_event(
-          :event_type => EventLog::INFO_TYPE, 
+          :event_type => EventLog::INFO_TYPE,
           :source => SOURCE,
           :event_id => RUN_COMPLETED_EVENT_ID,
           :data => [@run_status.run_id, @run_status.elapsed_time.to_s]
@@ -88,13 +88,13 @@ class Chef
       #Exception backtrace: %5
       def run_failed(e)
         @eventlog.report_event(
-          :event_type => EventLog::ERROR_TYPE, 
-          :source => SOURCE, 
+          :event_type => EventLog::ERROR_TYPE,
+          :source => SOURCE,
           :event_id => RUN_FAILED_EVENT_ID,
-          :data => [@run_status.run_id, 
-                    @run_status.elapsed_time.to_s, 
-                    e.class.name, 
-                    e.message, 
+          :data => [@run_status.run_id,
+                    @run_status.elapsed_time.to_s,
+                    e.class.name,
+                    e.message,
                     e.backtrace.join("\n")]
         )
       end
diff --git a/lib/chef/provider/group/pw.rb b/lib/chef/provider/group/pw.rb
index c39c20da67..7a66ab4d69 100644
--- a/lib/chef/provider/group/pw.rb
+++ b/lib/chef/provider/group/pw.rb
@@ -40,20 +40,16 @@ class Chef
           command = "pw groupadd"
           command << set_options
 
-          # pw group[add|mod] -M is used to set the full membership list on a
-          # new or existing group. Because pw groupadd does not support the -m
-          # and -d options used by manage_group, we treat group creation as a
-          # special case and use -M.
-          Chef::Log.debug("#{@new_resource} setting group members: #{@new_resource.members.join(',')}")
-          member_options = [" -M #{@new_resource.members.join(',')}"]
-
-          if member_options.empty?
-            run_command(:command => command)
-          else
-            member_options.each do |option|
-              run_command(:command => command + option)
-            end
+          unless @new_resource.members.empty?
+            # pw group[add|mod] -M is used to set the full membership list on a
+            # new or existing group. Because pw groupadd does not support the -m
+            # and -d options used by manage_group, we treat group creation as a
+            # special case and use -M.
+            Chef::Log.debug("#{@new_resource} setting group members: #{@new_resource.members.join(',')}")
+            command += " -M #{@new_resource.members.join(',')}"
           end
+
+          run_command(:command => command)
         end
 
         # Manage the group when it already exists
diff --git a/lib/chef/provider/package/windows.rb b/lib/chef/provider/package/windows.rb
index 25be5b822c..143d82f111 100644
--- a/lib/chef/provider/package/windows.rb
+++ b/lib/chef/provider/package/windows.rb
@@ -25,8 +25,11 @@ class Chef
     class Package
       class Windows < Chef::Provider::Package
 
-        # Depending on the installer, we may need to examine installer_type or 
-        # source attributes, or search for text strings in the installer file 
+        provides :package, os: "windows"
+        provides :windows_package, os: "windows"
+
+        # Depending on the installer, we may need to examine installer_type or
+        # source attributes, or search for text strings in the installer file
         # binary to determine the installer type for the user. Since the file
         # must be on disk to do so, we have to make this choice in the provider.
         require 'chef/provider/package/windows/msi.rb'
diff --git a/lib/chef/provider/service/windows.rb b/lib/chef/provider/service/windows.rb
index 4b1d2079ec..d4c272354e 100644
--- a/lib/chef/provider/service/windows.rb
+++ b/lib/chef/provider/service/windows.rb
@@ -26,6 +26,7 @@ end
 class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   provides :service, os: "windows"
+  provides :windows_service, os: "windows"
 
   include Chef::Mixin::ShellOut
 
diff --git a/lib/chef/resource/windows_package.rb b/lib/chef/resource/windows_package.rb
index c563ba5fdc..7b3361402c 100644
--- a/lib/chef/resource/windows_package.rb
+++ b/lib/chef/resource/windows_package.rb
@@ -24,7 +24,8 @@ class Chef
   class Resource
     class WindowsPackage < Chef::Resource::Package
 
-      provides :package, platform: "windows"
+      provides :package, os: "windows"
+      provides :windows_package, os: "windows"
 
       def initialize(name, run_context=nil)
         super
diff --git a/lib/chef/resource/windows_service.rb b/lib/chef/resource/windows_service.rb
index 49495117ee..2aec4d6304 100644
--- a/lib/chef/resource/windows_service.rb
+++ b/lib/chef/resource/windows_service.rb
@@ -25,7 +25,8 @@ class Chef
       # Until #1773 is resolved, you need to manually specify the windows_service resource
       # to use action :configure_startup and attribute startup_type
 
-      provides :service, platform: "windows"
+      provides :service, os: "windows"
+      provides :windows_service, os: "windows"
 
       identity_attr :service_name
 
diff --git a/spec/functional/dsl/reboot_pending_spec.rb b/spec/functional/dsl/reboot_pending_spec.rb
index 114754ccba..125c952a55 100644
--- a/spec/functional/dsl/reboot_pending_spec.rb
+++ b/spec/functional/dsl/reboot_pending_spec.rb
@@ -30,11 +30,11 @@ describe Chef::DSL::RebootPending, :windows_only do
     ohai
   end
 
-  def registry_safe?
-    !registry.value_exists?('HKLM\SYSTEM\CurrentControlSet\Control\Session Manager', { :name => 'PendingFileRenameOperations' }) ||
-    !registry.key_exists?('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired') ||
-    !registry.key_exists?('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootRequired') ||
-    !registry.key_exists?('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile')
+  def registry_unsafe?
+    registry.value_exists?('HKLM\SYSTEM\CurrentControlSet\Control\Session Manager', { :name => 'PendingFileRenameOperations' }) ||
+    registry.key_exists?('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired')
+    registry.key_exists?('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootRequired') ||
+    registry.key_exists?('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile')
   end
 
   let(:node) { Chef::Node.new }
@@ -48,22 +48,22 @@ describe Chef::DSL::RebootPending, :windows_only do
 
     describe "when there is nothing to indicate a reboot is pending" do
       it "should return false" do
-        pending "Found existing registry keys" unless registry_safe?
+        pending "Found existing registry keys" if registry_unsafe?
         expect(recipe.reboot_pending?).to be_false
       end
     end
 
     describe 'HKLM\SYSTEM\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations' do
       it "returns true if the registry value exists" do
-        pending "Found existing registry keys" unless registry_safe?
-        registry.set_value('HKLM\SYSTEM\CurrentControlSet\Control\Session Manager', 
+        pending "Found existing registry keys" if registry_unsafe?
+        registry.set_value('HKLM\SYSTEM\CurrentControlSet\Control\Session Manager',
             { :name => 'PendingFileRenameOperations', :type => :multi_string, :data => ['\??\C:\foo.txt|\??\C:\bar.txt'] })
 
         expect(recipe.reboot_pending?).to be_true
       end
 
       after do
-        if registry_safe?
+        unless registry_unsafe?
           registry.delete_value('HKLM\SYSTEM\CurrentControlSet\Control\Session Manager', { :name => 'PendingFileRenameOperations' })
         end
       end
@@ -71,14 +71,14 @@ describe Chef::DSL::RebootPending, :windows_only do
 
     describe 'HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired' do
       it "returns true if the registry key exists" do
-        pending "Found existing registry keys" unless registry_safe?
+        pending "Found existing registry keys" if registry_unsafe?
         registry.create_key('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired', false)
 
         expect(recipe.reboot_pending?).to be_true
       end
 
       after do
-        if registry_safe?
+        unless registry_unsafe?
           registry.delete_key('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\WindowsUpdate\Auto Update\RebootRequired', false)
         end
       end
@@ -87,14 +87,14 @@ describe Chef::DSL::RebootPending, :windows_only do
     describe 'HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootRequired' do
       it "returns true if the registry key exists" do
         pending "Permissions are limited to 'TrustedInstaller' by default"
-        pending "Found existing registry keys" unless registry_safe?
+        pending "Found existing registry keys" if registry_unsafe?
         registry.create_key('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootRequired', false)
 
         expect(recipe.reboot_pending?).to be_true
       end
 
       after do
-        if registry_safe?
+        unless registry_unsafe?
           registry.delete_key('HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootRequired', false)
         end
       end
@@ -102,7 +102,7 @@ describe Chef::DSL::RebootPending, :windows_only do
 
     describe 'HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile\Flags' do
       it "returns true if the registry key exists" do
-        pending "Found existing registry keys" unless registry_safe?
+        pending "Found existing registry keys" if registry_unsafe?
         registry.create_key('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile', true)
         registry.set_value('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile',
                     { :name => 'Flags', :type => :dword, :data => 3 })
@@ -111,7 +111,7 @@ describe Chef::DSL::RebootPending, :windows_only do
       end
 
       after do
-        if registry_safe?
+        unless registry_unsafe?
           registry.delete_value('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile', { :name => 'Flags' })
           registry.delete_key('HKLM\SOFTWARE\Microsoft\Updates\UpdateExeVolatile', false)
         end
diff --git a/spec/functional/event_loggers/windows_eventlog_spec.rb b/spec/functional/event_loggers/windows_eventlog_spec.rb
index 9da9f60fa9..c50bc7e00f 100644
--- a/spec/functional/event_loggers/windows_eventlog_spec.rb
+++ b/spec/functional/event_loggers/windows_eventlog_spec.rb
@@ -19,12 +19,12 @@
 require 'spec_helper'
 require 'securerandom'
 require 'chef/event_loggers/windows_eventlog'
-if Chef::Platform.windows?
+if Chef::Platform.windows? and not Chef::Platform::windows_server_2003?
   require 'win32/eventlog'
   include Win32
 end
 
-describe Chef::EventLoggers::WindowsEventLogger, :windows_only do
+describe Chef::EventLoggers::WindowsEventLogger, :windows_only, :not_supported_on_win2k3 do
   let(:run_id)       { SecureRandom.uuid }
   let(:version)      { SecureRandom.uuid }
   let(:elapsed_time) { SecureRandom.random_number(100) }
@@ -43,14 +43,14 @@ describe Chef::EventLoggers::WindowsEventLogger, :windows_only do
   it 'writes run_start event with event_id 10000 and contains version' do
     logger.run_start(version)
 
-    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10000 && 
+    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10000 &&
                                                e.string_inserts[0].include?(version)}).to be_true
   end
 
   it 'writes run_started event with event_id 10001 and contains the run_id' do
     logger.run_started(run_status)
 
-    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10001 && 
+    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10001 &&
                                                e.string_inserts[0].include?(run_id)}).to be_true
   end
 
@@ -58,7 +58,7 @@ describe Chef::EventLoggers::WindowsEventLogger, :windows_only do
     logger.run_started(run_status)
     logger.run_completed(node)
 
-    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10002 && 
+    expect(event_log.read(flags, offset).any? { |e| e.source == 'Chef' && e.event_id == 10002 &&
                                                 e.string_inserts[0].include?(run_id) &&
                                                 e.string_inserts[1].include?(elapsed_time.to_s)
     }).to be_true
diff --git a/spec/unit/resource/windows_package_spec.rb b/spec/unit/resource/windows_package_spec.rb
index 3c45548ece..fed029f9fe 100644
--- a/spec/unit/resource/windows_package_spec.rb
+++ b/spec/unit/resource/windows_package_spec.rb
@@ -18,7 +18,18 @@
 
 require 'spec_helper'
 
-describe Chef::Resource::WindowsPackage, "initialize", :windows_only do
+describe Chef::Resource::WindowsPackage, "initialize" do
+  before(:each) do
+    stub_const("File::ALT_SEPARATOR", "\\")
+  end
+
+  static_provider_resolution(
+    resource: Chef::Resource::WindowsPackage,
+    provider: Chef::Provider::Package::Windows,
+    os: "windows",
+    name: :windows_package,
+    action: :start
+  )
 
   let(:resource) { Chef::Resource::WindowsPackage.new("solitaire.msi") }
 
@@ -30,10 +41,6 @@ describe Chef::Resource::WindowsPackage, "initialize", :windows_only do
     expect(resource.resource_name).to eql(:windows_package)
   end
 
-  it "sets the provider to Chef::Provider::Package::Windows" do
-    expect(resource.provider).to eql(Chef::Provider::Package::Windows)
-  end
-
   it "supports setting installer_type" do
     resource.installer_type("msi")
     expect(resource.installer_type).to eql("msi")
diff --git a/spec/unit/resource/windows_service_spec.rb b/spec/unit/resource/windows_service_spec.rb
index fd7bedec9f..45a295c24e 100644
--- a/spec/unit/resource/windows_service_spec.rb
+++ b/spec/unit/resource/windows_service_spec.rb
@@ -18,7 +18,14 @@
 
 require 'spec_helper'
 
-describe Chef::Resource::WindowsService, "initialize", :windows_only do
+describe Chef::Resource::WindowsService, "initialize" do
+  static_provider_resolution(
+    resource: Chef::Resource::WindowsService,
+    provider: Chef::Provider::Service::Windows,
+    os: "windows",
+    name: :windows_service,
+    action: :start
+  )
 
   let(:resource) { Chef::Resource::WindowsService.new("BITS") }
 
@@ -30,10 +37,6 @@ describe Chef::Resource::WindowsService, "initialize", :windows_only do
     expect(resource.resource_name).to eql(:windows_service)
   end
 
-  it "sets the provider to Chef::Provider::Service::Windows" do
-    expect(resource.provider).to eql(Chef::Provider::Service::Windows)
-  end
-
   it "supports setting startup_type" do
     resource.startup_type(:manual)
     expect(resource.startup_type).to eql(:manual)