diff options
| -rw-r--r-- | lib/puppet/file_serving/fileset.rb | 20 | ||||
| -rw-r--r-- | lib/puppet/type/file.rb | 52 | ||||
| -rwxr-xr-x | spec/unit/file_serving/fileset.rb | 52 | ||||
| -rwxr-xr-x | spec/unit/type/file.rb | 26 | ||||
| -rwxr-xr-x | test/ral/type/file.rb | 2 |
5 files changed, 107 insertions, 45 deletions
diff --git a/lib/puppet/file_serving/fileset.rb b/lib/puppet/file_serving/fileset.rb index 61ca897e1..2076d0c4d 100644 --- a/lib/puppet/file_serving/fileset.rb +++ b/lib/puppet/file_serving/fileset.rb @@ -9,7 +9,7 @@ require 'puppet/file_serving/metadata' # Operate recursively on a path, returning a set of file paths. class Puppet::FileServing::Fileset attr_reader :path, :ignore, :links - attr_accessor :recurse + attr_accessor :recurse, :recurselimit # Produce a hash of files, with merged so that earlier files # with the same postfix win. E.g., /dir1/subfile beats /dir2/subfile. @@ -67,6 +67,7 @@ class Puppet::FileServing::Fileset @ignore = [] @links = :manage @recurse = false + @recurselimit = 0 # infinite recursion if options.is_a?(Puppet::Indirector::Request) initialize_from_request(options) @@ -75,6 +76,7 @@ class Puppet::FileServing::Fileset end raise ArgumentError.new("Fileset paths must exist") unless stat = stat(path) + raise ArgumentError.new("Fileset recurse parameter must not be a number anymore, please use recurselimit") if @recurse.is_a?(Integer) end def links=(links) @@ -87,17 +89,8 @@ class Puppet::FileServing::Fileset # Should we recurse further? This is basically a single # place for all of the logic around recursion. def recurse?(depth) - # If recurse is true, just return true - return true if self.recurse == true - - # Return false if the value is false or zero. - return false if [false, 0].include?(self.recurse) - - # Return true if our current depth is less than the allowed recursion depth. - return true if self.recurse.is_a?(Fixnum) and depth <= self.recurse - - # Else, return false. - return false + # recurse if told to, and infinite recursion or current depth not at the limit + self.recurse and (self.recurselimit == 0 or depth <= self.recurselimit) end def initialize_from_hash(options) @@ -112,7 +105,7 @@ class Puppet::FileServing::Fileset end def initialize_from_request(request) - [:links, :ignore, :recurse].each do |param| + [:links, :ignore, :recurse, :recurselimit].each do |param| if request.options.include?(param) # use 'include?' so the values can be false value = request.options[param] elsif request.options.include?(param.to_s) @@ -121,6 +114,7 @@ class Puppet::FileServing::Fileset next if value.nil? value = true if value == "true" value = false if value == "false" + value = Integer(value) if value.is_a?(String) and value =~ /^\d+$/ send(param.to_s + "=", value) end end diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 7d37fe865..05a4b37f9 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -115,7 +115,7 @@ module Puppet desc "Whether and how deeply to do recursive management." - newvalues(:true, :false, :inf, /^[0-9]+$/) + newvalues(:true, :false, :inf, :remote, /^[0-9]+$/) # Replace the validation so that we allow numbers in # addition to string representations of them. @@ -123,16 +123,39 @@ module Puppet munge do |value| newval = super(value) case newval - when :true, :inf; true - when :false; false - when Integer, Fixnum, Bignum; value - when /^\d+$/; Integer(value) + when :true, :inf: true + when :false: false + when :remote: :remote + when Integer, Fixnum, Bignum: + self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit" + resource[:recurselimit] = value + true + when /^\d+$/: + self.warning "Setting recursion depth with the recurse parameter is now deprecated, please use recurselimit" + resource[:recurselimit] = Integer(value) + true else raise ArgumentError, "Invalid recurse value %s" % value.inspect end end end + newparam(:recurselimit) do + desc "How deeply to do recursive management." + + newvalues(/^[0-9]+$/) + + munge do |value| + newval = super(value) + case newval + when Integer, Fixnum, Bignum: value + when /^\d+$/: Integer(value) + else + raise ArgumentError, "Invalid recurselimit value %s" % value.inspect + end + end + end + newparam(:replace, :boolean => true) do desc "Whether or not to replace a file that is sourced but exists. This is useful for using file sources @@ -247,6 +270,14 @@ module Puppet if count > 1 self.fail "You cannot specify more than one of %s" % CREATORS.collect { |p| p.to_s}.join(", ") end + + if !self[:source] and self[:recurse] == :remote + self.fail "You cannot specify a remote recursion without a source" + end + + if !self[:recurse] and self[:recurselimit] + self.warning "Possible error: recurselimit is set but not recurse, no recursion will happen" + end end def self.[](path) @@ -479,7 +510,7 @@ module Puppet options = @original_parameters.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? } # These should never be passed to our children. - [:parent, :ensure, :recurse, :target, :alias, :source].each do |param| + [:parent, :ensure, :recurse, :recurselimit, :target, :alias, :source].each do |param| options.delete(param) if options.include?(param) end @@ -517,7 +548,10 @@ module Puppet # be used to copy remote files, manage local files, and/or make links # to map to another directory. def recurse - children = recurse_local + children = {} + if self[:recurse] != :remote + children = recurse_local + end if self[:target] recurse_link(children) @@ -534,7 +568,7 @@ module Puppet val = @parameters[:recurse].value - if val and (val == true or val > 0) + if val and (val == true or val == :remote) return true else return false @@ -624,7 +658,7 @@ module Puppet end def perform_recursion(path) - Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore]) + Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => (self[:recurse] == :remote ? true : self[:recurse]), :recurselimit => self[:recurselimit], :ignore => self[:ignore]) end # Remove the old backup. diff --git a/spec/unit/file_serving/fileset.rb b/spec/unit/file_serving/fileset.rb index dfba9c1a1..6269d345a 100755 --- a/spec/unit/file_serving/fileset.rb +++ b/spec/unit/file_serving/fileset.rb @@ -24,6 +24,12 @@ describe Puppet::FileServing::Fileset, " when initializing" do set.recurse.should be_true end + it "should accept a 'recurselimit' option" do + File.expects(:lstat).with("/some/file").returns stub("stat") + set = Puppet::FileServing::Fileset.new("/some/file", :recurselimit => 3) + set.recurselimit.should == 3 + end + it "should accept an 'ignore' option" do File.expects(:lstat).with("/some/file").returns stub("stat") set = Puppet::FileServing::Fileset.new("/some/file", :ignore => ".svn") @@ -45,6 +51,11 @@ describe Puppet::FileServing::Fileset, " when initializing" do Puppet::FileServing::Fileset.new("/some/file").recurse.should == false end + it "should default to 0 (infinite) for recurselimit" do + File.expects(:lstat).with("/some/file").returns stub("stat") + Puppet::FileServing::Fileset.new("/some/file").recurselimit.should == 0 + end + it "should default to an empty ignore list" do File.expects(:lstat).with("/some/file").returns stub("stat") Puppet::FileServing::Fileset.new("/some/file").ignore.should == [] @@ -64,22 +75,27 @@ describe Puppet::FileServing::Fileset, " when initializing" do describe "using an indirector request" do before do File.stubs(:lstat).returns stub("stat") - @values = {:links => :manage, :ignore => %w{a b}, :recurse => true} + @values = {:links => :manage, :ignore => %w{a b}, :recurse => true, :recurselimit => 1234} @request = Puppet::Indirector::Request.new(:file_serving, :find, "foo") end - [:recurse, :ignore, :links].each do |option| - it "should pass :recurse, :ignore, and :links settings on to the fileset if present" do + [:recurse, :recurselimit, :ignore, :links].each do |option| + it "should pass :recurse, :recurselimit, :ignore, and :links settings on to the fileset if present" do @request.stubs(:options).returns(option => @values[option]) Puppet::FileServing::Fileset.new("/my/file", @request).send(option).should == @values[option] end - it "should pass :recurse, :ignore, and :links settings on to the fileset if present with the keys stored as strings" do + it "should pass :recurse, :recurselimit, :ignore, and :links settings on to the fileset if present with the keys stored as strings" do @request.stubs(:options).returns(option.to_s => @values[option]) Puppet::FileServing::Fileset.new("/my/file", @request).send(option).should == @values[option] end end + it "should convert the integer as a string to their integer counterpart when setting options" do + @request.stubs(:options).returns(:recurselimit => "1234") + Puppet::FileServing::Fileset.new("/my/file", @request).recurselimit.should == 1234 + end + it "should convert the string 'true' to the boolean true when setting options" do @request.stubs(:options).returns(:recurse => "true") Puppet::FileServing::Fileset.new("/my/file", @request).recurse.should == true @@ -99,8 +115,9 @@ describe Puppet::FileServing::Fileset, " when determining whether to recurse" do @fileset = Puppet::FileServing::Fileset.new(@path) end - it "should always recurse if :recurse is set to 'true'" do + it "should always recurse if :recurse is set to 'true' and with infinite recursion" do @fileset.recurse = true + @fileset.recurselimit = 0 @fileset.recurse?(0).should be_true end @@ -109,25 +126,23 @@ describe Puppet::FileServing::Fileset, " when determining whether to recurse" do @fileset.recurse?(-1).should be_false end - it "should recurse if :recurse is set to an integer and the current depth is less than that integer" do - @fileset.recurse = 1 + it "should recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is less than that integer" do + @fileset.recurse = true + @fileset.recurselimit = 1 @fileset.recurse?(0).should be_true end - it "should recurse if :recurse is set to an integer and the current depth is equal to that integer" do - @fileset.recurse = 1 + it "should recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is equal to that integer" do + @fileset.recurse = true + @fileset.recurselimit = 1 @fileset.recurse?(1).should be_true end - it "should not recurse if :recurse is set to an integer and the current depth is greater than that integer" do - @fileset.recurse = 1 + it "should not recurse if :recurse is set to true, :recurselimit is set to an integer and the current depth is greater than that integer" do + @fileset.recurse = true + @fileset.recurselimit = 1 @fileset.recurse?(2).should be_false end - - it "should not recurse if :recurse is set to 0" do - @fileset.recurse = 0 - @fileset.recurse?(-1).should be_false - end end describe Puppet::FileServing::Fileset, " when recursing" do @@ -173,9 +188,10 @@ describe Puppet::FileServing::Fileset, " when recursing" do # It seems like I should stub :recurse? here, or that I shouldn't stub the # examples above, but... - it "should recurse to the level set if :recurse is set to an integer" do + it "should recurse to the level set if :recurselimit is set to an integer" do mock_dir_structure(@path) - @fileset.recurse = 1 + @fileset.recurse = true + @fileset.recurselimit = 1 @fileset.files.should == %w{. one two .svn CVS} end diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb index 2be388f8e..0187cc41d 100755 --- a/spec/unit/type/file.rb +++ b/spec/unit/type/file.rb @@ -71,7 +71,7 @@ describe Puppet::Type.type(:file) do end describe "when validating attributes" do - %w{path backup recurse source replace force ignore links purge sourceselect}.each do |attr| + %w{path backup recurse recurselimit source replace force ignore links purge sourceselect}.each do |attr| it "should have a '#{attr}' parameter" do Puppet::Type.type(:file).attrtype(attr.intern).should == :param end @@ -233,8 +233,20 @@ describe Puppet::Type.type(:file) do end it "should pass its recursion value to the search" do - @file[:recurse] = 10 - Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == 10 } + @file[:recurse] = true + Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true } + @file.perform_recursion(@file[:path]) + end + + it "should pass true if recursion is remote" do + @file[:recurse] = :remote + Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurse] == true } + @file.perform_recursion(@file[:path]) + end + + it "should pass its recursion limit value to the search" do + @file[:recurselimit] = 10 + Puppet::FileServing::Metadata.expects(:search).with { |key, options| options[:recurselimit] == 10 } @file.perform_recursion(@file[:path]) end @@ -575,11 +587,17 @@ describe Puppet::Type.type(:file) do end end - it "should use recurse_local" do + it "should use recurse_local if recurse is not remote" do @file.expects(:recurse_local).returns({}) @file.recurse end + it "should not use recurse_local if recurse remote" do + @file[:recurse] = :remote + @file.expects(:recurse_local).never + @file.recurse + end + it "should return the generated resources as an array sorted by file path" do one = stub 'one', :[] => "/one" two = stub 'two', :[] => "/one/two" diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb index 08fdab821..1e7f2862d 100755 --- a/test/ral/type/file.rb +++ b/test/ral/type/file.rb @@ -392,7 +392,7 @@ class TestFile < Test::Unit::TestCase # Make sure we default to false assert(! file.recurse?, "Recurse defaulted to true") - [true, "true", 10, "inf"].each do |value| + [true, "true", 10, "inf", "remote"].each do |value| file[:recurse] = value assert(file.recurse?, "%s did not cause recursion" % value) end |