Fix bugs in handling 'source' in msu_package and cab_package

The two issues fixed here:

1) cab_package did not require source, but didn't set it from the resource name so cab_package 'xyz.cab' would just fail since source was nil
2) msu_package would coerce the resource name and not package_name so if you overwrote the package name you would also have to set the source, although you wouldn't realize that until it failed oddly

So the solution here is to set the source value to package_name by default and that gets coerced into a path as was previously done. This adds tests for this behavior and the coerce behavior itself to both resources.

This fixes issue #6674

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

4 files changed, 46 insertions, 11 deletions

diff --git a/lib/chef/resource/cab_package.rb b/lib/chef/resource/cab_package.rb
index 73e639814d..fcf3f02b86 100644
--- a/lib/chef/resource/cab_package.rb
+++ b/lib/chef/resource/cab_package.rb
@@ -34,7 +34,8 @@ class Chef
                   unless s.nil?
                     uri_scheme?(s) ? s : Chef::Util::PathHelper.canonical_path(s, false)
                   end
-                end)
+                end),
+                default: lazy { |r| r.package_name }
     end
   end
 end
diff --git a/lib/chef/resource/msu_package.rb b/lib/chef/resource/msu_package.rb
index 65959c5621..93c8fbbe21 100644
--- a/lib/chef/resource/msu_package.rb
+++ b/lib/chef/resource/msu_package.rb
@@ -31,12 +31,12 @@ class Chef
       default_action :install
 
       property :source, String,
-                name_property: true,
                 coerce: (proc do |s|
                   unless s.nil?
                     uri_scheme?(s) ? s : Chef::Util::PathHelper.canonical_path(s, false)
                   end
-                end)
+                end),
+                default: lazy { |r| r.package_name }
       property :checksum, String, desired_state: false
     end
   end
diff --git a/spec/unit/resource/cab_package_spec.rb b/spec/unit/resource/cab_package_spec.rb
index aa4890f171..167cf6e86e 100644
--- a/spec/unit/resource/cab_package_spec.rb
+++ b/spec/unit/resource/cab_package_spec.rb
@@ -32,7 +32,30 @@ describe Chef::Resource::CabPackage do
     expect(resource.resource_name).to eql(:cab_package)
   end
 
-  it "coerce its name to a package_name" do
+  it "sets the default action as :install" do
+    expect(resource.action).to eql([:install])
+  end
+
+  it "coerces name property to package_name property" do
     expect(resource.package_name).to eql("test_pkg")
   end
+
+  it "coerces name property to a source property if source not provided" do
+    expect(resource.source).to end_with("test_pkg")
+  end
+
+  it "coerces name property to a source property if source not provided and package_name is" do
+    resource.package_name("package.cab")
+    expect(resource.source).to end_with("package.cab")
+  end
+
+  it "does not coerce the source property if it looks like a path" do
+    resource.source("/foo/bar/package.cab")
+    expect(resource.source).to eq("/foo/bar/package.cab")
+  end
+
+  it "coerces source property if it does not looks like a path" do
+    resource.source("package.cab")
+    expect(resource.source).not_to eq("package.cab")
+  end
 end
diff --git a/spec/unit/resource/msu_package_spec.rb b/spec/unit/resource/msu_package_spec.rb
index 66aacbc020..e32fab897d 100644
--- a/spec/unit/resource/msu_package_spec.rb
+++ b/spec/unit/resource/msu_package_spec.rb
@@ -31,19 +31,30 @@ describe Chef::Resource::MsuPackage do
     expect(resource.resource_name).to eql(:msu_package)
   end
 
-  it "sets the source as its name and then coerces it to a path" do
+  it "sets the default action as :install" do
+    expect(resource.action).to eql([:install])
+  end
+
+  it "coerces name property to package_name property" do
+    expect(resource.package_name).to eql("test_pkg")
+  end
+
+  it "coerces name property to a source property if source not provided" do
     expect(resource.source).to end_with("test_pkg")
   end
 
-  it "sets the default action as :install" do
-    expect(resource.action).to eql([:install])
+  it "coerces name property to a source property if source not provided and package_name is" do
+    resource.package_name("package.msu")
+    expect(resource.source).to end_with("package.msu")
   end
 
-  it "raises error if invalid action is given" do
-    expect { resource.action :abc }.to raise_error(Chef::Exceptions::ValidationFailed)
+  it "does not coerce the source property if it looks like a path" do
+    resource.source("/foo/bar/package.msu")
+    expect(resource.source).to eq("/foo/bar/package.msu")
   end
 
-  it "coerce its name to a package_name" do
-    expect(resource.package_name).to eql("test_pkg")
+  it "coerces source property if it does not looks like a path" do
+    resource.source("package.msu")
+    expect(resource.source).not_to eq("package.msu")
   end
 end