Merge pull request #11665 from chef/more_windows_printer

windows_printer: Install drivers, allow skipping port creation, and load state properly

3 files changed, 93 insertions, 45 deletions

diff --git a/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb b/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
index 9b7f5c91d8..64a05c05c9 100644
--- a/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
+++ b/kitchen-tests/cookbooks/end_to_end/recipes/_windows_printer.rb
@@ -22,3 +22,17 @@ end
 windows_printer_port "10.4.64.37" do
   action :delete
 end
+
+# create a printer that will also create the port
+windows_printer "HP LaserJet 6th Floor" do
+  ipv4_address "10.4.64.40"
+  driver_name "Generic / Text Only"
+end
+
+# create a printer that uses an existing port
+windows_printer "HP LaserJet 5th Floor" do
+  ipv4_address "10.4.64.41"
+  driver_name "Generic / Text Only"
+  port_name "My awesome port"
+  create_port false
+end
diff --git a/lib/chef/resource/windows_printer.rb b/lib/chef/resource/windows_printer.rb
index b1d6e29374..24d746b648 100644
--- a/lib/chef/resource/windows_printer.rb
+++ b/lib/chef/resource/windows_printer.rb
@@ -22,6 +22,10 @@ require_relative "../resource"
 
 class Chef
   class Resource
+    # @todo
+    # 1. Allow updating the printer properties
+    # 2. Fail with a warning if the port can't be found and create_port is false
+    # 3. Fail with helpful messaging if the printer driver can't be installed
     class WindowsPrinter < Chef::Resource
       unified_mode true
 
@@ -29,7 +33,7 @@ class Chef
 
       provides(:windows_printer) { true }
 
-      description "Use the **windows_printer** resource to setup Windows printers. Note that this doesn't currently install a printer driver. You must already have the driver installed on the system."
+      description "Use the **windows_printer** resource to setup Windows printers. This resource will automatically install the driver specified in the `driver_name` property and will automatically create a printer port using either the `ipv4_address` property or the `port_name property."
       introduced "14.0"
       examples <<~DOC
       **Create a printer**:
@@ -50,6 +54,23 @@ class Chef
         action :delete
       end
       ```
+
+      **Create a printer port and a printer that uses that port (new in 17.3)**
+
+      ```ruby
+      windows_printer_port '10.4.64.39' do
+        port_name 'My awesome printer port'
+        snmp_enabled true
+        port_protocol 2
+      end
+
+      windows_printer 'HP LaserJet 5th Floor' do
+        driver_name 'HP LaserJet 4100 Series PCL6'
+        port_name 'My awesome printer port'
+        ipv4_address '10.4.64.38'
+        create_port false
+      end
+      ```
       DOC
 
       property :device_id, String,
@@ -84,25 +105,74 @@ class Chef
             proc { |v| v.match(Resolv::IPv4::Regex) },
         }
 
-      PRINTERS_REG_KEY = 'HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Print\Printers\\'.freeze unless defined?(PRINTERS_REG_KEY)
+      property :create_port, [TrueClass, FalseClass],
+        description: "Create a printer port for the printer. Set this to false and specify the `port_name` property if using the `windows_printer_port` resource to create the port instead.",
+        introduced: "17.3",
+        default: true, desired_state: false
+
+      property :port_name, String,
+        description: "The port name.",
+        default: lazy { |x| "IP_#{x.ipv4_address}" },
+        introduced: "17.3",
+        default_description: "The resource block name or the ipv4_address prepended with IP_."
 
-      # @todo Set @current_resource printer properties from registry
       load_current_value do |new_resource|
-        name new_resource.name
+        printer_data = powershell_exec(%Q{Get-WmiObject -Class Win32_Printer -Filter "Name='#{new_resource.device_id}'"}).result
+
+        if printer_data.empty?
+          current_value_does_not_exist!
+        else
+          device_id new_resource.device_id
+          comment printer_data["Comment"]
+          default printer_data["Default"]
+          location printer_data["Location"]
+          shared printer_data["Shared"]
+          share_name printer_data["ShareName"]
+          port_name printer_data["PortName"]
+
+          driver_data = powershell_exec(%Q{Get-PrinterDriver -Name="#{new_resource.driver_name}"}).result
+          unless driver_data.empty?
+            driver_name new_resource.driver_name
+          end
+        end
       end
 
       action :create, description: "Create a new printer and printer port, if one doesn't already." do
-        if printer_exists?
+        if current_resource
           Chef::Log.info "#{@new_resource} already exists - nothing to do."
         else
-          converge_by("Create #{@new_resource}") do
-            create_printer
+          # Create the printer port first unless the property is set to false
+          if new_resource.create_port
+            windows_printer_port new_resource.port_name do
+              ipv4_address new_resource.ipv4_address
+              port_name new_resource.port_name
+            end
+          end
+
+          converge_by("install driver #{new_resource.driver_name}") do
+            powershell_exec!("Add-PrinterDriver -Name '#{new_resource.driver_name}'")
+          end
+
+          converge_by("create #{@new_resource.device_id}") do
+            powershell_exec! <<-EOH
+            Set-WmiInstance -class Win32_Printer `
+              -EnableAllPrivileges `
+              -Argument @{ DeviceID   = "#{new_resource.device_id}";
+                          Comment    = "#{new_resource.comment}";
+                          Default    = "$#{new_resource.default}";
+                          DriverName = "#{new_resource.driver_name}";
+                          Location   = "#{new_resource.location}";
+                          PortName   = "#{new_resource.port_name}";
+                          Shared     = "$#{new_resource.shared}";
+                          ShareName  = "#{new_resource.share_name}";
+                        }
+            EOH
           end
         end
       end
 
       action :delete, description: "Delete an existing printer. Note that this resource does not delete the associated printer port." do
-        if printer_exists?
+        if current_resource
           converge_by("Delete #{new_resource.device_id}") do
             powershell_exec!("Remove-Printer -Name '#{new_resource.device_id}'")
           end
@@ -110,42 +180,6 @@ class Chef
           Chef::Log.info "#{new_resource.device_id} doesn't exist - can't delete."
         end
       end
-
-      action_class do
-        # does the printer exist
-        #
-        # @param [String] name the name of the printer
-        # @return [Boolean]
-        def printer_exists?
-          printer_reg_key = PRINTERS_REG_KEY + new_resource.name
-          logger.trace "Checking to see if this reg key exists: '#{printer_reg_key}'"
-          registry_key_exists?(printer_reg_key)
-        end
-
-        # creates the printer port and then the printer
-        def create_printer
-          # Create the printer port first
-          windows_printer_port new_resource.ipv4_address
-
-          port_name = "IP_#{new_resource.ipv4_address}"
-
-          declare_resource(:powershell_script, "Creating printer: #{new_resource.device_id}") do
-            code <<-EOH
-              Set-WmiInstance -class Win32_Printer `
-                -EnableAllPrivileges `
-                -Argument @{ DeviceID   = "#{new_resource.device_id}";
-                            Comment    = "#{new_resource.comment}";
-                            Default    = "$#{new_resource.default}";
-                            DriverName = "#{new_resource.driver_name}";
-                            Location   = "#{new_resource.location}";
-                            PortName   = "#{port_name}";
-                            Shared     = "$#{new_resource.shared}";
-                            ShareName  = "#{new_resource.share_name}";
-                          }
-            EOH
-          end
-        end
-      end
     end
   end
 end
diff --git a/lib/chef/resource/windows_printer_port.rb b/lib/chef/resource/windows_printer_port.rb
index b109528b5d..636ae9dd54 100644
--- a/lib/chef/resource/windows_printer_port.rb
+++ b/lib/chef/resource/windows_printer_port.rb
@@ -137,7 +137,7 @@ class Chef
 
       action :delete, description: "Delete an existing printer port." do
         if current_resource
-          converge_by("Delete #{new_resource.port_name}") do
+          converge_by("delete port #{new_resource.port_name}") do
             powershell_exec!("Remove-PrinterPort -Name #{new_resource.port_name}")
           end
         else