Improve input validation on windows_package

- Make sure the correct installer_type symbol is passed and provide a useful error message when a bogus value is provided instead of just saying it needs to be a symbol and hoping a user eventually stumbles onto the docs
- Add descriptions fields to all the properties and update them since the ones on the docs site right now aren't the best
- Coerce the provided checksum value to lowercase. Right now if it's uppercase, but valid the resource will fail and the message provided is not useful. See https://github.com/chef/chef/issues/6682

Signed-off-by: Tim Smith <tsmith@chef.io>

2 files changed, 40 insertions, 16 deletions

diff --git a/lib/chef/resource/windows_package.rb b/lib/chef/resource/windows_package.rb
index 5592e537b4..784b68542e 100644
--- a/lib/chef/resource/windows_package.rb
+++ b/lib/chef/resource/windows_package.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Bryan McLellan <btm@loftninjas.org>
-# Copyright:: Copyright 2014-2017, Chef Software Inc.
+# Copyright:: Copyright 2014-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -20,6 +20,7 @@ require_relative "../mixin/uris"
 require_relative "package"
 require_relative "../provider/package/windows"
 require_relative "../win32/error" if RUBY_PLATFORM =~ /mswin|mingw|windows/
+require_relative "../dist"
 
 class Chef
   class Resource
@@ -30,7 +31,7 @@ class Chef
       provides(:windows_package) { true }
       provides :package, os: "windows"
 
-      description "Use the windows_package resource to manage Microsoft Installer Package (MSI) packages for the Microsoft Windows platform."
+      description "Use the windows_package resource to manage packages on the Microsoft Windows platform. The windows_package resource supports these installer formats:\n\n Microsoft Installer Package (MSI)\n Nullsoft Scriptable Install System (NSIS)\n Inno Setup (inno)\n Wise\n InstallShield\n Custom installers such as installing a non-.msi file that embeds an .msi-based installer\n"
       introduced "11.12"
 
       allowed_actions :install, :remove
@@ -41,21 +42,39 @@ class Chef
       end
 
       # windows can't take array options yet
-      property :options, String
+      property :options, String,
+        description: "One (or more) additional options that are passed to the command."
 
       # Unique to this resource
-      property :installer_type, Symbol
-      property :timeout, [ String, Integer ], default: 600
+      property :installer_type, Symbol,
+        equal_to: %i{custom inno installshield msi nsis wise},
+        description: "A symbol that specifies the type of package. Possible values: :custom (such as installing a non-.msi file that embeds an .msi-based installer), :inno (Inno Setup), :installshield (InstallShield), :msi (Microsoft Installer Package (MSI)), :nsis (Nullsoft Scriptable Install System (NSIS)), :wise (Wise)."
+
+      property :timeout, [ String, Integer ], default: 600,
+        default_description: "600 (seconds)",
+        description: "The amount of time (in seconds) to wait before timing out."
+
       # In the past we accepted return code 127 for an unknown reason and 42 because of a bug
-      property :returns, [ String, Integer, Array ], default: [ 0 ], desired_state: false
+      property :returns, [ String, Integer, Array ], default: [ 0 ],
+        desired_state: false,
+        description: "A comma-delimited list of return codes that indicate the success or failure of the package command that was run."
+
       property :source, String,
         coerce: (proc do |s|
           unless s.nil?
             uri_scheme?(s) ? s : Chef::Util::PathHelper.canonical_path(s, false)
           end
-        end)
-      property :checksum, String, desired_state: false
-      property :remote_file_attributes, Hash, desired_state: false
+        end),
+        default_description: "The resource block's name", # this property is basically a name_property but not really so we need to spell it out
+        description: "The path to a package in the local file system. The location of the package may be at a URL. \n"
+
+      property :checksum, String,
+        desired_state: false, coerce: (proc { |c| c.downcase }),
+        description: "The SHA-256 checksum of the file. Use to prevent a file from being re-downloaded. When the local file matches the checksum, Chef Infra Client does not download it. Use when a URL is specified by the source property."
+
+      property :remote_file_attributes, Hash,
+        desired_state: false,
+        description: "A package at a remote location define as a Hash of properties that modifies the properties of the remote_file resource."
     end
   end
 end
diff --git a/spec/unit/resource/windows_package_spec.rb b/spec/unit/resource/windows_package_spec.rb
index 7a513b020a..edf871f7fc 100644
--- a/spec/unit/resource/windows_package_spec.rb
+++ b/spec/unit/resource/windows_package_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Bryan McLellan <btm@loftninjas.org>
-# Copyright:: Copyright 2014-2016, Chef Software, Inc.
+# Copyright:: Copyright 2014-2019, Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -51,9 +51,14 @@ describe Chef::Resource::WindowsPackage, "initialize" do
     expect { resource.action :upgrade }.not_to raise_error
   end
 
-  it "supports setting installer_type as a symbol" do
-    resource.installer_type(:msi)
-    expect(resource.installer_type).to eql(:msi)
+  it "supports setting installer_type to :custom :inno :installshield :msi :nsis or :wise only" do
+    expect { resource.installer_type :custom }.not_to raise_error
+    expect { resource.installer_type :inno }.not_to raise_error
+    expect { resource.installer_type :installshield }.not_to raise_error
+    expect { resource.installer_type :msi }.not_to raise_error
+    expect { resource.installer_type :nsis }.not_to raise_error
+    expect { resource.installer_type :wise }.not_to raise_error
+    expect { resource.installer_type 'msi' }.to raise_error(Chef::Exceptions::ValidationFailed)
   end
 
   # String, Integer
@@ -72,7 +77,7 @@ describe Chef::Resource::WindowsPackage, "initialize" do
     end
   end
 
-  it "coverts a source to an absolute path" do
+  it "converts a source to an absolute path" do
     allow(::File).to receive(:absolute_path).and_return("c:\\files\\frost.msi")
     resource.source("frost.msi")
     expect(resource.source).to eql "c:\\files\\frost.msi"
@@ -89,8 +94,8 @@ describe Chef::Resource::WindowsPackage, "initialize" do
     expect(resource.source).to include("solitaire.msi")
   end
 
-  it "supports the checksum property" do
-    resource.checksum("somechecksum")
+  it "lowercases values provided in the checksum property" do
+    resource.checksum("SOMECHECKSUM")
     expect(resource.checksum).to eq("somechecksum")
   end