Please review pull request #713: (#14173) Enforce that filebucket paths must be absolute opened by (daniel-pittman)
Description:
If you passed the filebucket a relative path, perhaps by accidentally quoting
'false', it would happily accept that and create a directory relative to the
current working directory to store content in.
This enforces that the path must be absolute, as well as failing cleanly if
multiple paths are passed.
Signed-off-by: Daniel Pittman [email protected]
- Opened: Wed Apr 25 20:17:33 UTC 2012
- Based on: puppetlabs:2.7.x (64348a0ea036fa6af20ef50b860bfda60c12c719)
- Requested merge: daniel-pittman:bug/2.7.x/14173-filebucket-resource-should-validate-path (603b368a46a77189bbc1c965f43cee3a0de1afb4)
Diff follows:
diff --git a/lib/puppet/type/filebucket.rb b/lib/puppet/type/filebucket.rb
index 6fe15ca..644231a 100755
--- a/lib/puppet/type/filebucket.rb
+++ b/lib/puppet/type/filebucket.rb
@@ -61,6 +61,18 @@ module Puppet
can be specified to set the remote server."
defaultto { Puppet[:clientbucketdir] }
+
+ validate do |value|
+ if value.is_a? Array
+ raise ArgumentError, "You can only have one filebucket path"
+ end
+
+ if value.is_a? String and not Puppet::Util.absolute_path?(value)
+ raise ArgumentError, "Filebucket paths must be absolute"
+ end
+
+ true
+ end
end
# Create a default filebucket.
diff --git a/spec/unit/type/filebucket_spec.rb b/spec/unit/type/filebucket_spec.rb
index 3c53111..0603dd5 100755
--- a/spec/unit/type/filebucket_spec.rb
+++ b/spec/unit/type/filebucket_spec.rb
@@ -35,16 +35,44 @@
bucket.bucket.should be_local
end
- it "not be local if path is false" do
- bucket = Puppet::Type.type(:filebucket).new :name => "main", :path => false
+ describe "path" do
+ include PuppetSpec::Files
- bucket.bucket.should_not be_local
- end
+ def bucket(hash)
+ Puppet::Type.type(:filebucket).new({:name => 'main'}.merge(hash))
+ end
- it "be local if both a path and a server are specified" do
- bucket = Puppet::Type.type(:filebucket).new :name => "main", :server => "puppet", :path => "/my/path"
+ it "should accept false as a value" do
+ expect { bucket(:path => false) }.not_to raise_error
+ end
- bucket.bucket.should be_local
+ it "should accept true as a value" do
+ expect { bucket(:path => true) }.not_to raise_error
+ end
+
+ it "should fail when given an array of values" do
+ expect { bucket(:path => ['one', 'two']) }.
+ to raise_error Puppet::Error, /only have one filebucket path/
+ end
+
+ %w{one ../one one/two}.each do |path|
+ it "should fail if given a relative path of #{path.inspect}" do
+ expect { bucket(:path => path) }.
+ to raise_error Puppet::Error, /Filebucket paths must be absolute/
+ end
+ end
+
+ it "should succeed if given an absolute path" do
+ expect { bucket(:path => make_absolute('/tmp/bucket')) }.not_to raise_error
+ end
+
+ it "not be local if path is false" do
+ bucket(:path => false).bucket.should_not be_local
+ end
+
+ it "be local if both a path and a server are specified" do
+ bucket(:server => "puppet", :path => "/my/path").bucket.should be_local
+ end
end
describe "when creating the filebucket" do
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
