diff options
| -rw-r--r-- | CHANGELOG | 5 | ||||
| -rw-r--r-- | lib/puppet/parser/compiler.rb | 1 | ||||
| -rw-r--r-- | lib/puppet/parser/resource.rb | 37 | ||||
| -rwxr-xr-x | spec/unit/parser/resource.rb | 161 | ||||
| -rwxr-xr-x | test/language/resource.rb | 159 |
5 files changed, 178 insertions, 185 deletions
@@ -1,3 +1,8 @@ + Always duplicating resource defaults in the parser, so that + stacked metaparameter values do not result in all resources + that receive a given default also getting those stacked + values. + 0.24.2 Fixing #1062 by moving the yamldir setting to its own yaml section. This should keep the yamldir from being created diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 70cd6e11a..8fba41121 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -366,7 +366,6 @@ class Puppet::Parser::Compiler # Make sure all of our resources and such have done any last work # necessary. def finish - #@catalog.resources.each do |name| @catalog.vertices.each do |resource| # Add in any resource overrides. if overrides = resource_overrides(resource) diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb index b001e165b..4b48ff6cf 100644 --- a/lib/puppet/parser/resource.rb +++ b/lib/puppet/parser/resource.rb @@ -233,6 +233,20 @@ class Puppet::Parser::Resource @ref.to_s end + # Define a parameter in our resource. + def set_parameter(param, value = nil) + if value + param = Puppet::Parser::Resource::Param.new( + :name => param, :value => value, :source => self.source + ) + elsif ! param.is_a?(Puppet::Parser::Resource::Param) + raise ArgumentError, "Must pass a parameter or all necessary values" + end + + # And store it in our parameter hash. + @params[param.name] = param + end + def to_hash @params.inject({}) do |hash, ary| param = ary[1] @@ -331,7 +345,7 @@ class Puppet::Parser::Resource unless @params.include?(name) self.debug "Adding default for %s" % name - @params[name] = param + @params[name] = param.dup end end end @@ -370,11 +384,8 @@ class Puppet::Parser::Resource # The parameter is already set. Fail if they're not allowed to override it. unless param.source.child_of?(current.source) - if Puppet[:trace] - puts caller - end - msg = "Parameter '%s' is already set on %s" % - [param.name, self.to_s] + puts caller if Puppet[:trace] + msg = "Parameter '%s' is already set on %s" % [param.name, self.to_s] if current.source.to_s != "" msg += " by %s" % current.source end @@ -426,20 +437,6 @@ class Puppet::Parser::Resource end end - # Define a parameter in our resource. - def set_parameter(param, value = nil) - if value - param = Puppet::Parser::Resource::Param.new( - :name => param, :value => value, :source => self.source - ) - elsif ! param.is_a?(Puppet::Parser::Resource::Param) - raise ArgumentError, "Must pass a parameter or all necessary values" - end - - # And store it in our parameter hash. - @params[param.name] = param - end - # Make sure the resource's parameters are all valid for the type. def validate @params.each do |name, param| diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb index 9ce7b391b..1948a3c07 100755 --- a/spec/unit/parser/resource.rb +++ b/spec/unit/parser/resource.rb @@ -14,6 +14,36 @@ describe Puppet::Parser::Resource do @scope = @compiler.topscope end + def mkresource(args = {}) + args[:source] ||= "source" + args[:scope] ||= stub('scope', :source => mock('source')) + + {:type => "resource", :title => "testing", :source => "source", :scope => "scope"}.each do |param, value| + args[param] ||= value + end + + params = args[:params] || {:one => "yay", :three => "rah"} + if args[:params] == :none + args.delete(:params) + else + args[:params] = paramify(args[:source], params) + end + + Puppet::Parser::Resource.new(args) + end + + def param(name, value, source) + Puppet::Parser::Resource::Param.new(:name => name, :value => value, :source => source) + end + + def paramify(source, hash) + hash.collect do |name, value| + Puppet::Parser::Resource::Param.new( + :name => name, :value => value, :source => source + ) + end + end + it "should be isomorphic if it is builtin and models an isomorphic type" do Puppet::Type.type(:file).expects(:isomorphic?).returns(true) @resource = Puppet::Parser::Resource.new(:type => "file", :title => "whatever", :scope => @scope, :source => @source).isomorphic?.should be_true @@ -29,6 +59,30 @@ describe Puppet::Parser::Resource do @resource = Puppet::Parser::Resource.new(:type => "whatever", :title => "whatever", :scope => @scope, :source => @source).isomorphic?.should be_true end + it "should have a array-indexing method for retrieving parameter values" do + @resource = mkresource + @resource[:one].should == "yay" + end + + describe "when initializing" do + before do + @arguments = {:type => "resource", :title => "testing", :scope => stub('scope', :source => mock('source'))} + end + + [:type, :title, :scope].each do |name| + it "should fail unless #{name.to_s} is specified" do + try = @arguments.dup + try.delete(name) + lambda { Puppet::Parser::Resource.new(try) }.should raise_error(ArgumentError) + end + end + + it "should set the reference correctly" do + res = Puppet::Parser::Resource.new(@arguments) + res.ref.should == "Resource[testing]" + end + end + describe "when evaluating" do before do @type = Puppet::Parser::Resource @@ -60,11 +114,10 @@ describe Puppet::Parser::Resource do describe "when finishing" do before do - @definition = @parser.newdefine "mydefine" @class = @parser.newclass "myclass" @nodedef = @parser.newnode("mynode")[0] - @resource = Puppet::Parser::Resource.new(:type => "mydefine", :title => "whatever", :scope => @scope, :source => @source) + @resource = Puppet::Parser::Resource.new(:type => "file", :title => "whatever", :scope => @scope, :source => @source) end it "should do nothing if it has already been finished" do @@ -73,6 +126,32 @@ describe Puppet::Parser::Resource do @resource.finish end + it "should add all defaults available from the scope" do + @resource.scope.expects(:lookupdefaults).with(@resource.type).returns(:owner => param(:owner, "default", @resource.source)) + @resource.finish + + @resource[:owner].should == "default" + end + + it "should not replace existing parameters with defaults" do + @resource.set_parameter :owner, "oldvalue" + @resource.scope.expects(:lookupdefaults).with(@resource.type).returns(:owner => :replaced) + @resource.finish + + @resource[:owner].should == "oldvalue" + end + + it "should add a copy of each default, rather than the actual default parameter instance" do + newparam = param(:owner, "default", @resource.source) + other = newparam.dup + other.value = "other" + newparam.expects(:dup).returns(other) + @resource.scope.expects(:lookupdefaults).with(@resource.type).returns(:owner => newparam) + @resource.finish + + @resource[:owner].should == "other" + end + it "should copy metaparams from its scope" do @scope.setvar("noop", "true") @@ -82,7 +161,7 @@ describe Puppet::Parser::Resource do end it "should not copy metaparams that it already has" do - @resource.class.publicize_methods(:set_parameter) { @resource.set_parameter("noop", "false") } + @resource.set_parameter("noop", "false") @scope.setvar("noop", "true") @resource.class.publicize_methods(:add_metaparams) { @resource.add_metaparams } @@ -91,7 +170,7 @@ describe Puppet::Parser::Resource do end it "should stack relationship metaparams from its container if it already has them" do - @resource.class.publicize_methods(:set_parameter) { @resource.set_parameter("require", "resource") } + @resource.set_parameter("require", "resource") @scope.setvar("require", "container") @resource.class.publicize_methods(:add_metaparams) { @resource.add_metaparams } @@ -100,7 +179,7 @@ describe Puppet::Parser::Resource do end it "should flatten the array resulting from stacking relationship metaparams" do - @resource.class.publicize_methods(:set_parameter) { @resource.set_parameter("require", ["resource1", "resource2"]) } + @resource.set_parameter("require", ["resource1", "resource2"]) @scope.setvar("require", %w{container1 container2}) @resource.class.publicize_methods(:add_metaparams) { @resource.add_metaparams } @@ -167,4 +246,76 @@ describe Puppet::Parser::Resource do lambda { @resource.tag("good_tag") }.should_not raise_error(Puppet::ParseError) end end + + describe "when merging overrides" do + before do + @source = "source1" + @resource = mkresource :source => @source + @override = mkresource :source => @source + end + + it "should fail when the override was not created by a parent class" do + @override.source = "source2" + @override.source.expects(:child_of?).with("source1").returns(false) + assert_raise(Puppet::ParseError, "Allowed unrelated resources to override") do + @resource.merge(@override) + end + end + + it "should succeed when the override was created in the current scope" do + @resource.source = "source3" + @override.source = @resource.source + @override.source.expects(:child_of?).with("source3").never + params = {:a => :b, :c => :d} + @override.expects(:params).returns(params) + @resource.expects(:override_parameter).with(:b) + @resource.expects(:override_parameter).with(:d) + @resource.merge(@override) + end + + it "should succeed when a parent class created the override" do + @resource.source = "source3" + @override.source = "source4" + @override.source.expects(:child_of?).with("source3").returns(true) + params = {:a => :b, :c => :d} + @override.expects(:params).returns(params) + @resource.expects(:override_parameter).with(:b) + @resource.expects(:override_parameter).with(:d) + @resource.merge(@override) + end + + it "should add new parameters when the parameter is not set" do + @source.stubs(:child_of?).returns true + @override.set_parameter(:testing, "value") + @resource.merge(@override) + + @resource[:testing].should == "value" + end + + it "should replace existing parameter values" do + @source.stubs(:child_of?).returns true + @resource.set_parameter(:testing, "old") + @override.set_parameter(:testing, "value") + + @resource.merge(@override) + + @resource[:testing].should == "value" + end + + it "should add values to the parameter when the override was created with the '+>' syntax" do + @source.stubs(:child_of?).returns true + param = Puppet::Parser::Resource::Param.new(:name => :testing, :value => "testing", :source => @resource.source) + param.add = true + + @override.set_parameter(param) + + @resource.set_parameter(:testing, "other") + + @resource.merge(@override) + + @resource[:testing].should == %w{other testing} + end + + + end end diff --git a/test/language/resource.rb b/test/language/resource.rb index 608e7c995..b3eaf0390 100755 --- a/test/language/resource.rb +++ b/test/language/resource.rb @@ -23,95 +23,6 @@ class TestResource < PuppetTest::TestCase mocha_verify end - def test_initialize - args = {:type => "resource", :title => "testing", - :scope => mkscope} - # Check our arg requirements - args.each do |name, value| - try = args.dup - try.delete(name) - assert_raise(ArgumentError, "Did not fail when %s was missing" % name) do - Parser::Resource.new(try) - end - end - - res = nil - assert_nothing_raised do - res = Parser::Resource.new(args) - end - - ref = res.instance_variable_get("@ref") - assert_equal("Resource", ref.type, "did not set resource type") - assert_equal("testing", ref.title, "did not set resource title") - end - - def test_merge - res = mkresource - other = mkresource - - # First try the case where the resource is not allowed to override - res.source = "source1" - other.source = "source2" - other.source.expects(:child_of?).with("source1").returns(false) - assert_raise(Puppet::ParseError, "Allowed unrelated resources to override") do - res.merge(other) - end - - # Next try it when the sources are equal. - res.source = "source3" - other.source = res.source - other.source.expects(:child_of?).with("source3").never - params = {:a => :b, :c => :d} - other.expects(:params).returns(params) - res.expects(:override_parameter).with(:b) - res.expects(:override_parameter).with(:d) - res.merge(other) - - # And then parentage is involved - other = mkresource - res.source = "source3" - other.source = "source4" - other.source.expects(:child_of?).with("source3").returns(true) - params = {:a => :b, :c => :d} - other.expects(:params).returns(params) - res.expects(:override_parameter).with(:b) - res.expects(:override_parameter).with(:d) - res.merge(other) - end - - # the [] method - def test_array_accessors - res = mkresource - params = res.instance_variable_get("@params") - assert_nil(res[:missing], "Found a missing parameter somehow") - params[:something] = stub(:value => "yay") - assert_equal("yay", res[:something], "Did not correctly call value on the parameter") - - res.expects(:title).returns(:mytitle) - assert_equal(:mytitle, res[:title], "Did not call title when asked for it as a param") - end - - # Make sure any defaults stored in the scope get added to our resource. - def test_add_defaults - res = mkresource - params = res.instance_variable_get("@params") - params[:a] = :b - res.scope.expects(:lookupdefaults).with(res.type).returns(:a => :replaced, :c => :d) - res.expects(:debug) - - res.send(:add_defaults) - assert_equal(:d, params[:c], "Did not set default") - assert_equal(:b, params[:a], "Replaced parameter with default") - end - - def test_finish - res = mkresource - res.expects(:add_defaults) - res.expects(:add_metaparams) - res.expects(:validate) - res.finish - end - # Make sure we paramcheck our params def test_validate res = mkresource @@ -123,43 +34,6 @@ class TestResource < PuppetTest::TestCase res.send(:validate) end - def test_override_parameter - res = mkresource - params = res.instance_variable_get("@params") - - # There are three cases, with the second having two options: - - # No existing parameter. - param = stub(:name => "myparam") - res.send(:override_parameter, param) - assert_equal(param, params["myparam"], "Override was not added to param list") - - # An existing parameter that we can override. - source = stub(:child_of? => true) - # Start out without addition - params["param2"] = stub(:source => :whatever) - param = stub(:name => "param2", :source => source, :add => false) - res.send(:override_parameter, param) - assert_equal(param, params["param2"], "Override was not added to param list") - - # Try with addition. - params["param2"] = stub(:value => :a, :source => :whatever) - param = stub(:name => "param2", :source => source, :add => true, :value => :b) - param.expects(:value=).with([:a, :b]) - res.send(:override_parameter, param) - assert_equal(param, params["param2"], "Override was not added to param list") - - # And finally, make sure we throw an exception when the sources aren't related - source = stub(:child_of? => false) - params["param2"] = stub(:source => :whatever, :file => :f, :line => :l) - old = params["param2"] - param = stub(:name => "param2", :source => source, :file => :f, :line => :l) - assert_raise(Puppet::ParseError, "Did not fail when params conflicted") do - res.send(:override_parameter, param) - end - assert_equal(old, params["param2"], "Param was replaced irrespective of conflict") - end - def test_set_parameter res = mkresource params = res.instance_variable_get("@params") @@ -420,37 +294,4 @@ class TestResource < PuppetTest::TestCase assert(newres.exported?, "Exported defined resource generated non-exported resources") assert(newres.virtual?, "Exported defined resource generated non-virtual resources") end - - # Make sure tags behave appropriately. - def test_tags - scope_resource = stub 'scope_resource', :tags => %w{srone srtwo} - scope = stub 'scope', :resource => scope_resource - resource = Puppet::Parser::Resource.new(:type => "file", :title => "yay", :scope => scope, :source => mock('source')) - - # Make sure we get the type and title - %w{yay file}.each do |tag| - assert(resource.tags.include?(tag), "Did not tag resource with %s" % tag) - end - - # make sure we can only set legal tags - ["an invalid tag", "-anotherinvalid", "bad*tag"].each do |tag| - assert_raise(Puppet::ParseError, "Tag #{tag} was considered valid") do - resource.tag tag - end - end - - # make sure good tags make it through. - tags = %w{good-tag yaytag GoodTag another_tag a ab A} - tags.each do |tag| - assert_nothing_raised("Tag #{tag} was considered invalid") do - resource.tag tag - end - end - - # make sure we get each of them. - ptags = resource.tags - tags.each do |tag| - assert(ptags.include?(tag.downcase), "missing #{tag}") - end - end end |