When we are checking if a node exists before creating a new one we were also trying to match with regex node names, finding matches where in fact there is no equality.
Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/parser/loaded_code.rb | 9 +++++-- lib/puppet/parser/parser_support.rb | 8 +++++- spec/unit/parser/loaded_code.rb | 37 +++++++++++++++++++++++++--------- spec/unit/parser/parser.rb | 4 +- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/lib/puppet/parser/loaded_code.rb b/lib/puppet/parser/loaded_code.rb index 065fcb9..80a7497 100644 --- a/lib/puppet/parser/loaded_code.rb +++ b/lib/puppet/parser/loaded_code.rb @@ -22,7 +22,7 @@ class Puppet::Parser::LoadedCode @nodes[name] = code end - def node(name) + def node_matching(name) name = check_name(name) if node = @nodes[name] @@ -30,12 +30,15 @@ class Puppet::Parser::LoadedCode end @node_list.each do |nodename| - n = @nodes[nodename] - return n if nodename.regex? and nodename.match(name) + return @nodes[nodename] if nodename.regex? and nodename.match(name) end nil end + def node_named(name) + @nodes[check_name(name)] + end + def nodes? @nodes.length > 0 end diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index dfc14e0..41aa9b3 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -98,12 +98,16 @@ class Puppet::Parser::Parser end end - [:hostclass, :definition, :node, :nodes?].each do |method| + [:hostclass, :definition, :nodes?].each do |method| define_method(method) do |*args| @loaded_code.send(method, *args) end end + def node(name) + @loaded_code.node_matching(name) + end + def find_hostclass(namespace, name) find_or_load(namespace, name, :hostclass) end @@ -376,7 +380,7 @@ class Puppet::Parser::Parser doc = lexer.getcomment names.collect do |name| name = AST::HostName.new :value => name unless name.is_a?(AST::HostName) - if other = @loaded_code.node(name) + if other = @loaded_code.node_named(name) error("Node %s is already defined at %s:%s; cannot redefine" % [other.name, other.file, other.line]) end name = name.to_s if name.is_a?(Symbol) diff --git a/spec/unit/parser/loaded_code.rb b/spec/unit/parser/loaded_code.rb index ca4b0aa..0972a22 100644 --- a/spec/unit/parser/loaded_code.rb +++ b/spec/unit/parser/loaded_code.rb @@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/parser/loaded_code' describe Puppet::Parser::LoadedCode do - %w{hostclass node definition}.each do |data| + { "hostclass" => "hostclass", "node" => "node_matching", "definition" => "definition" }.each do |data, method| it "should have a method for adding a #{data}" do Puppet::Parser::LoadedCode.new.should respond_to("add_" + data) end @@ -13,17 +13,17 @@ describe Puppet::Parser::LoadedCode do it "should be able to retrieve #{data} by name" do loader = Puppet::Parser::LoadedCode.new loader.send("add_" + data, "foo", "bar") - loader.send(data, "foo").should == "bar" + loader.send(method, "foo").should == "bar" end it "should retrieve #{data} insensitive to case" do loader = Puppet::Parser::LoadedCode.new loader.send("add_" + data, "Foo", "bar") - loader.send(data, "fOo").should == "bar" + loader.send(method, "fOo").should == "bar" end it "should return nil when asked for a #{data} that has not been added" do - Puppet::Parser::LoadedCode.new.send(data, "foo").should be_nil + Puppet::Parser::LoadedCode.new.send(method, "foo").should be_nil end it "should be able to retrieve all #{data}s" do @@ -140,7 +140,7 @@ describe Puppet::Parser::LoadedCode do it "should create an HostName if nodename is a string" do Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo") - @loader.node("foo") + @loader.node_matching("foo") end it "should not create an HostName if nodename is an HostName" do @@ -148,7 +148,7 @@ describe Puppet::Parser::LoadedCode do Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo").never - @loader.node(name) + @loader.node_matching(name) end it "should be able to find node by HostName" do @@ -156,7 +156,24 @@ describe Puppet::Parser::LoadedCode do nameout = Puppet::Parser::AST::HostName.new(:value => "foo") @loader.add_node(namein, "bar") - @loader.node(nameout) == "bar" + @loader.node_matching(nameout).should == "bar" + end + + it "should be able to find node by HostName strict equality" do + namein = Puppet::Parser::AST::HostName.new(:value => "foo") + nameout = Puppet::Parser::AST::HostName.new(:value => "foo") + + @loader.add_node(namein, "bar") + @loader.node_named(nameout).should == "bar" + end + + it "should not use node name matching when finding with strict node HostName" do + name1 = Puppet::Parser::AST::HostName.new(:value => "foo") + name2 = Puppet::Parser::AST::HostName.new(:value => Puppet::Parser::AST::Regex.new(:value => /foo/)) + + @loader.add_node(name1, "bar") + @loader.add_node(name2, "baz") + @loader.node_named(name1).should == "bar" end it "should return the first matching regex nodename" do @@ -168,7 +185,7 @@ describe Puppet::Parser::LoadedCode do @loader.add_node(@nodename1, @node1) @loader.add_node(@nodename2, @node2) - @loader.node("test").should == @node1 + @loader.node_matching("test").should == @node1 end it "should not scan non-regex node" do @@ -180,7 +197,7 @@ describe Puppet::Parser::LoadedCode do @loader.add_node(@nodename1,@node1) @loader.add_node(@nodename2,@node2) - @loader.node("test") + @loader.node_matching("test") end it "should prefer non-regex nodes to regex nodes" do @@ -192,7 +209,7 @@ describe Puppet::Parser::LoadedCode do @loader.add_node(@nodename1,@node1) @loader.add_node(@nodename2,@node2) - @loader.node(@nodename1) + @loader.node_matching(@nodename1) end end end diff --git a/spec/unit/parser/parser.rb b/spec/unit/parser/parser.rb index 9127599..d993c1e 100755 --- a/spec/unit/parser/parser.rb +++ b/spec/unit/parser/parser.rb @@ -253,7 +253,7 @@ describe Puppet::Parser do end it "should raise an error if the node already exists" do - @loaded_code.stubs(:node).with(@nodename).returns(@node) + @loaded_code.stubs(:node_named).with(@nodename).returns(@node) lambda { @parser.newnode(@nodename) }.should raise_error end @@ -291,7 +291,7 @@ describe Puppet::Parser do describe "when retrieving a specific node" do it "should delegate to the loaded_code node" do - @loaded_code.expects(:node).with("node") + @loaded_code.expects(:node_matching).with("node") @parser.node("node") end -- 1.6.4 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
