Merge pull request #9102 from chef/windows_package

Improve input validation on windows_package

3 files changed, 41 insertions, 17 deletions

diff --git a/lib/chef/resource/windows_package.rb b/lib/chef/resource/windows_package.rb
index 5592e537b4..3402cb0792 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::Dist::PRODUCT} does not download it. Use when a URL is specified by the source property."
+
+      property :remote_file_attributes, Hash,
+        desired_state: false,
+        description: "If the source package to install is at a remote location this property allows you to define a hash of properties and their value which will be used by the underlying remote_file resource, which fetches the source."
     end
   end
 end
diff --git a/spec/unit/resource/windows_package_spec.rb b/spec/unit/resource/windows_package_spec.rb
index 7a513b020a..a0f746393d 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
 
diff --git a/tasks/docs.rb b/tasks/docs.rb
index 643eaee7a6..c347445c2e 100755
--- a/tasks/docs.rb
+++ b/tasks/docs.rb
@@ -26,7 +26,7 @@ namespace :docs_site do
       if default.is_a?(String)
 
         # .inspect wraps the value in quotes which we want for strings, but not sentences or symbols as strings
-        return default.inspect unless default[0] == ":" || default.end_with?('.')
+        return default.inspect unless default[0] == ":" || default.end_with?(".")
       end
       default
     end