From dbba368ebef038850713b08defd881e8359c59cc Mon Sep 17 00:00:00 2001 From: Stefan Schulte Date: Mon, 30 Mar 2015 00:36:19 +0200 Subject: [PATCH] (#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 | 15 ++++++++++- lib/puppet/type/rpmkey.rb | 37 +++++++++++++++++++++++++-- spec/unit/provider/rpmkey/rpm_spec.rb | 12 ++++++++- spec/unit/type/rpmkey_spec.rb | 15 +++++++---- 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/lib/puppet/provider/rpmkey/rpm.rb b/lib/puppet/provider/rpmkey/rpm.rb index 6648bf7..e012c90 100644 --- a/lib/puppet/provider/rpmkey/rpm.rb +++ b/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? diff --git a/lib/puppet/type/rpmkey.rb b/lib/puppet/type/rpmkey.rb index cd8518a..4d5937d 100644 --- a/lib/puppet/type/rpmkey.rb +++ b/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 diff --git a/spec/unit/provider/rpmkey/rpm_spec.rb b/spec/unit/provider/rpmkey/rpm_spec.rb index 959daec..094d666 100644 --- a/spec/unit/provider/rpmkey/rpm_spec.rb +++ b/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', diff --git a/spec/unit/type/rpmkey_spec.rb b/spec/unit/type/rpmkey_spec.rb index 20b7bfb..2efe188 100644 --- a/spec/unit/type/rpmkey_spec.rb +++ b/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