(#6) Always convert source to a URI

We always convert the source to a URI (even for local files) so we can
easily use the URI class to check the scheme. While this is not very
useful right now, it can be used to handle some URLs (like `puppet://`)
differently in the future.

lib/puppet/provider/rpmkey/rpm.rb

@@ -30,9 +30,22 @@ def self.prefetch(resources)
     end
   end
 
+  def path(source)
+    uri = URI.parse(URI.escape(source))
+    case uri.scheme
+    when 'file'
+      # no need to download a local file. Just use the filename
+      uri_to_path(uri)
+    else
+      # we don't know how to handle other types (e.g. http or https)
+      # so we trust rpm how to handle these
+      source
+    end
+  end
+
   def create
     raise Puppet::Error, "Cannot add key without a source" unless @resource[:source]
-    rpm('--import', @resource[:source])
+    rpm '--import', path(@resource[:source])
   end
 
   def exists?

lib/puppet/type/rpmkey.rb

@@ -1,3 +1,5 @@
+require 'uri'
+
 Puppet::Type.newtype(:rpmkey) do
 
   @doc = "Define public GPG keys that should be part of the rpm
@@ -30,11 +32,42 @@
   ensurable
 
   newparam(:source) do
-    desc "The source of the public key if the key is not already imported."
+    desc "The source of the public key. This can be a local file or
+        any URL that `rpm` supports (e.g. `http`). You can also specify
+        a `puppet://` URL in which case the keyfile will be downloaded
+        from the puppet master prior to importing it."
+
+    validate do |source|
+      # the value must be either a filename or a valid URL
+      unless Puppet::Util.absolute_path?(source)
+        begin
+          uri = URI.parse(URI.escape(source))
+        rescue => detail
+          raise Puppet::Error, "Could not understand source #{source}: #{detail}"
+        end
+
+        raise Puppet::Error, "Cannot use relative URLs '#{source}'" unless uri.absolute?
+        raise Puppet::Error, "Cannot use opaque URLs '#{source}'" unless uri.hierarchical?
+      end
+    end
+
+    SEPARATOR_REGEX = [Regexp.escape(File::SEPARATOR.to_s), Regexp.escape(File::ALT_SEPARATOR.to_s)].join
+
+    # make sure we always pass a valid URI to the provider
+    munge do |source|
+      source = source.sub(/[#{SEPARATOR_REGEX}]+$/, '')
+      if Puppet::Util.absolute_path?(source)
+        URI.unescape(Puppet::Util.path_to_uri(source).to_s)
+      else
+        source
+      end
+    end
   end
 
   autorequire(:file) do
-    self[:source] if self[:source] =~ /^\//
+    if source = self[:source] and uri = URI.parse(URI.escape(source)) and uri.scheme == 'file'
+      uri_to_path(uri)
+    end
   end
 
 end

spec/unit/provider/rpmkey/rpm_spec.rb

@@ -51,7 +51,17 @@
   end
 
   describe "#create" do
-    it "should import the key" do
+    it "should import a local file by filename" do
+      provider = described_class.new(Puppet::Type.type(:rpmkey).new(
+        :name   => 'DB42A60E',
+        :source => '/etc/pki/some key',
+        :ensure => :present
+      ))
+      provider.expects(:rpm).with('--import', '/etc/pki/some key')
+      provider.create
+    end
+
+    it "should import a http link by url" do
       provider = described_class.new(Puppet::Type.type(:rpmkey).new(
         :name   => 'DB42A60E',
         :source => 'http://example.com/foo',

spec/unit/type/rpmkey_spec.rb

@@ -64,10 +64,15 @@
 
     describe "for source" do
       it "should support a local filename" do
-        expect { described_class.new(:name => 'DB42A60E', :source => '/tmp/foo', :ensure => :present) }.to_not raise_error
+        expect(described_class.new(:name => 'DB42A60E', :source => '/etc/pki/some key', :ensure => :present)[:source]).to eq('file:/etc/pki/some key')
       end
-      it "should support a http link" do
-        expect { described_class.new(:name => 'DB42A60E', :source => 'http://example.com/foo', :ensure => :present) }.to_not raise_error
+
+      it "should support a file url" do
+        expect(described_class.new(:name => 'DB42A60E', :source => 'file:/tmp/some key', :ensure => :present)[:source]).to eq('file:/tmp/some key')
+      end
+
+      it "should support a http url" do
+        expect(described_class.new(:name => 'DB42A60E', :source => 'http://example.com/some key', :ensure => :present)[:source]).to eq('http://example.com/some key')
       end
     end
 
@@ -76,9 +81,9 @@
         catalog = Puppet::Resource::Catalog.new
       }
       it "should autorequire a local file" do
-        file = Puppet::Type.type(:file).new(:name => '/tmp/foo', :content => 'bar' )
+        file = Puppet::Type.type(:file).new(:name => '/tmp/some key', :content => 'bar' )
         catalog.add_resource file
-        key = described_class.new(:name => 'DB42A60E', :source => '/tmp/foo', :ensure => :present)
+        key = described_class.new(:name => 'DB42A60E', :source => '/tmp/some key', :ensure => :present)
         catalog.add_resource key
         expect(key.autorequire.size).to eq(1)
       end