diff options
author | Luke Kanies <luke@madstop.com> | 2008-10-28 17:50:20 -0500 |
---|---|---|
committer | Luke Kanies <luke@madstop.com> | 2008-11-04 16:20:45 -0600 |
commit | 6f7ccff8bb764dffd1d41d5391dd79f7bd4a387c (patch) | |
tree | 4ed9cf36fcf35b4edeaef5644257148139e4dafd | |
parent | a4d4444ba216cb3627e03bb45e2eb92424740a44 (diff) | |
download | puppet-6f7ccff8bb764dffd1d41d5391dd79f7bd4a387c.tar.gz puppet-6f7ccff8bb764dffd1d41d5391dd79f7bd4a387c.tar.xz puppet-6f7ccff8bb764dffd1d41d5391dd79f7bd4a387c.zip |
Fixing #1641 - file recursion now only passes original parameters to child resources.
Previously, parameter values provided by remote sources or default values
were all passed to children, which provided strange state maintenance.
Signed-off-by: Luke Kanies <luke@madstop.com>
-rw-r--r-- | lib/puppet/type/file.rb | 18 | ||||
-rwxr-xr-x | spec/unit/type/file.rb | 45 |
2 files changed, 39 insertions, 24 deletions
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 922550bd8..6038aa50b 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -441,7 +441,7 @@ module Puppet def initialize(hash) # Store a copy of the arguments for later. - tmphash = hash.to_hash + @original_arguments = hash.to_hash # Used for caching clients @clients = {} @@ -461,9 +461,19 @@ module Puppet def newchild(path) full_path = File.join(self[:path], path) - # the right-side hash wins in the merge. - options = to_hash.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? } - [:parent, :recurse, :target].each do |param| + # Add some new values to our original arguments -- these are the ones + # set at initialization. We specifically want to exclude any param + # values set by the :source property or any default values. + # LAK:NOTE This is kind of silly, because the whole point here is that + # the values set at initialization should live as long as the resource + # but values set by default or by :source should only live for the transaction + # or so. Unfortunately, we don't have a straightforward way to manage + # the different lifetimes of this data, so we kludge it like this. + # The right-side hash wins in the merge. + options = @original_arguments.merge(:path => full_path, :implicit => true).reject { |param, value| value.nil? } + + # These should never be passed to our children. + [:parent, :ensure, :recurse, :target].each do |param| options.delete(param) if options.include?(param) end diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb index a476d3cd5..533f19556 100755 --- a/spec/unit/type/file.rb +++ b/spec/unit/type/file.rb @@ -564,38 +564,43 @@ describe Puppet::Type.type(:file) do describe "and making a new child resource" do it "should create an implicit resource using the provided relative path joined with the file's path" do - path = File.join(@file[:path], "my/path") - Puppet::Type.type(:file).expects(:create).with { |options| options[:implicit] == true and options[:path] == path } - @file.newchild("my/path") - end - - it "should copy most of the parent resource's 'should' values to the new resource" do - @file.expects(:to_hash).returns :foo => "bar", :fee => "fum" - Puppet::Type.type(:file).expects(:create).with { |options| options[:foo] == "bar" and options[:fee] == "fum" } - @file.newchild("my/path") + @file.newchild("my/path").should be_implicit end it "should not copy the parent resource's parent" do - @file.expects(:to_hash).returns :parent => "foo" Puppet::Type.type(:file).expects(:create).with { |options| ! options.include?(:parent) } @file.newchild("my/path") end - it "should not copy the parent resource's recurse value" do - @file.expects(:to_hash).returns :recurse => true - Puppet::Type.type(:file).expects(:create).with { |options| ! options.include?(:recurse) } - @file.newchild("my/path") + {:recurse => true, :target => "/foo/bar", :ensure => :present}.each do |param, value| + it "should not pass on #{param} to the sub resource" do + @file[param] = value + + @file.class.expects(:create).with { |params| params[param].nil? } + + @file.newchild("sub/file") + end + end + + it "should copy all of the parent resource's 'should' values that were set at initialization" do + file = @file.class.create(:path => "/foo/bar", :owner => "root", :group => "wheel") + file.class.expects(:create).with { |options| options[:owner] == "root" and options[:group] == "wheel" } + file.newchild("my/path") end - it "should not copy the parent resource's target value" do - @file.expects(:to_hash).returns :target => "foo" - Puppet::Type.type(:file).expects(:create).with { |options| ! options.include?(:target) } + it "should not copy default values to the new child" do + @file.class.expects(:create).with { |params| params[:backup].nil? } @file.newchild("my/path") end - it "should not copy any nil values from the parent" do - @file.expects(:to_hash).returns :ensure => nil - Puppet::Type.type(:file).expects(:create).with { |options| ! options.include?(:ensure) } + it "should not copy values to the child which were set by the source" do + @file[:source] = "/foo/bar" + metadata = stub 'metadata', :owner => "root", :group => "root", :mode => 0755, :ftype => "file", :checksum => "{md5}whatever" + @file.property(:source).stubs(:metadata).returns metadata + + @file.property(:source).copy_source_values + + @file.class.expects(:create).with { |params| params[:group].nil? } @file.newchild("my/path") end end |