[PATCH/puppet 1/1] Fixes #2740. pkdmg provider no longer deletes dmgs with a local source

[Nigel Kersten]

Signed-off-by: Nigel Kersten <nig...@google.com>
---
lib/puppet/provider/package/pkgdmg.rb | 6 ++----
spec/unit/provider/package/pkgdmg.rb | 28 ++++++++++++++++++++++++++--
2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb
index 8a916d5..210c3b7 100644
--- a/lib/puppet/provider/package/pkgdmg.rb
+++ b/lib/puppet/provider/package/pkgdmg.rb
@@ -79,8 +79,7 @@ Puppet::Type.type(:package).provide :pkgdmg, :parent => Puppet::Provider::Packag
curl "-o", cached_source, "-C", "-", "-k", "-s", "--url", source
Puppet.debug "Success: curl transfered [#{name}]"
rescue Puppet::ExecutionFailure
- Puppet.debug "curl did not transfer [#{name}]. Falling back to slower open-uri transfer methods."
- cached_source = source
+ raise Puppet::Error.new("Unable to download #{source} disk image to #{cached_source}")
end
end

@@ -109,8 +108,7 @@ Puppet::Type.type(:package).provide :pkgdmg, :parent => Puppet::Provider::Packag
end
end
ensure
- # JJM Remove the file if open-uri didn't already do so.
- File.unlink(cached_source) if File.exist?(cached_source)
+ File.unlink(cached_source) if cached_source != source and File.exist?(cached_source)
end
end

diff --git a/spec/unit/provider/package/pkgdmg.rb b/spec/unit/provider/package/pkgdmg.rb
index d437574..57c0214 100755
--- a/spec/unit/provider/package/pkgdmg.rb
+++ b/spec/unit/provider/package/pkgdmg.rb
@@ -45,6 +45,11 @@ describe provider do
@provider.stubs(:[]).with(:hdiutil).returns ""
lambda { @provider.install }.should raise_error(Puppet::Error)
end
+
+ it "a non-existent source URI is specified" do
+ @provider.stubs(:[]).with(:source).returns "gopher://yourmom/foo.dmg"
+ lambda { @provider.install }.should raise_error(Puppet::Error)
+ end
end

# These tests shouldn't be this messy. The pkgdmg provider needs work...
@@ -54,6 +59,7 @@ describe provider do
fh.stubs(:path).yields "/tmp/foo"
@resource.stubs(:[]).with(:source).returns "foo.dmg"
File.stubs(:open).yields fh
+ Dir.stubs(:entries).returns ["foo.pkg"]
end

it "should call hdiutil to mount and eject the disk image" do
@@ -62,12 +68,30 @@ describe provider do
@provider.class.expects(:hdiutil).with("mount", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", "/tmp", nil).returns @fakehdiutilplist
@provider.install
end
-
+
it "should call installpkg if a pkg/mpkg is found on the dmg" do
- Dir.stubs(:entries).returns ["foo.pkg"]
@provider.class.stubs(:hdiutil).returns @fakehdiutilplist
@provider.class.expects(:installpkg).with("#{@fakemountpoint}/foo.pkg", @resource[:name], "foo.dmg").returns ""
@provider.install
end
+
+ it "should not delete the dmg if the source is local" do
+ @resouce.stubs(:source).returns "/local/filesystem/foo.dmg"
+ @provider.class.stubs(:hdiutil).returns @fakehdiutilplist
+ @provider.class.stubs(:installpkg)
+ File.stubs(:exist?).returns true
+ File.expects(:unlink).never
+ @provider.install
+ end
+
+ it "should delete the cached dmg if the source is remote" do
+ @resource.stubs(:[]).with(:source).returns "http://blah/foo.dmg"
+ @provider.class.stubs(:hdiutil).returns @fakehdiutilplist
+ @provider.class.stubs(:curl)
+ @provider.class.stubs(:installpkg)
+ File.stubs(:exist?).returns true
+ File.expects(:unlink).with("/tmp/#{@resource[:name]}").once
+ @provider.install
+ end
end
end
--
1.6.5.2

[Ian Ward Comfort]

I'm as whimsical as the next guy, but might it be preferable to use a
URL whose invalidity is RFC-mandated? E.g.

gopher://domain.invalid/foo.dmg

> --
>
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
> .

--
Ian Ward Comfort <icom...@rescomp.stanford.edu>
Systems Team Lead, Student Computing, Stanford University

[Nigel Kersten]

Yes. I'll change that and rebase when submitting if approved.

nigel