diff options
| -rw-r--r-- | CHANGELOG | 6 | ||||
| -rw-r--r-- | lib/puppet/indirector/catalog/compiler.rb | 2 | ||||
| -rw-r--r-- | lib/puppet/indirector/ldap.rb | 12 | ||||
| -rw-r--r-- | lib/puppet/indirector/node/ldap.rb | 122 | ||||
| -rw-r--r-- | lib/puppet/node.rb | 118 | ||||
| -rwxr-xr-x | spec/unit/indirector/catalog/compiler.rb | 22 | ||||
| -rwxr-xr-x | spec/unit/indirector/node/ldap.rb | 22 | ||||
| -rwxr-xr-x | spec/unit/node.rb | 76 |
8 files changed, 164 insertions, 216 deletions
@@ -1,3 +1,9 @@ + Removed support for the 'node_name' setting in LDAP and external node lookups. + Also removed support for 'default' nodes in external nodes. + LDAP nodes now use the certificate name, the short name, and 'default', + but external nodes just use the certificate name and any custom terminus + types will use just the certificate name. + Adding a ResourceTemplate class for using templates directly within resources (i.e., client-side templates). This would really only be used for composite resources that pass the results of the diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb index 2b5e8d912..455a92cc7 100644 --- a/lib/puppet/indirector/catalog/compiler.rb +++ b/lib/puppet/indirector/catalog/compiler.rb @@ -89,7 +89,7 @@ class Puppet::Node::Catalog::Compiler < Puppet::Indirector::Code # key = client #end - return nil unless node = Puppet::Node.find_by_any_name(key) + return nil unless node = Puppet::Node.find(key) # Add any external data to the node. add_node_data(node) diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb index 07ad38933..695d38a95 100644 --- a/lib/puppet/indirector/ldap.rb +++ b/lib/puppet/indirector/ldap.rb @@ -1,14 +1,16 @@ require 'puppet/indirector/terminus' class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus - # Perform our ldap search and process the result. - def find(request) + # We split this apart so it's easy to call multiple times with different names. + def entry2hash(name) # We have to use 'yield' here because the LDAP::Entry objects # get destroyed outside the scope of the search, strangely. - ldapsearch(request.key) { |entry| return process(request.key, entry) } + ldapsearch(name) { |entry| return process(name, entry) } + end - # Return nil if we haven't found something. - return nil + # Perform our ldap search and process the result. + def find(request) + return entry2hash(request.key) || nil end # Process the found entry. We assume that we don't just want the diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb index bc58908fd..4ed053eff 100644 --- a/lib/puppet/indirector/node/ldap.rb +++ b/lib/puppet/indirector/node/ldap.rb @@ -3,7 +3,9 @@ require 'puppet/indirector/ldap' class Puppet::Node::Ldap < Puppet::Indirector::Ldap desc "Search in LDAP for node configuration information. See - the `LdapNodes`:trac: page for more information." + the `LdapNodes`:trac: page for more information. This will first + search for whatever the certificate name is, then (if that name + contains a '.') for the short name, then 'default'." # The attributes that Puppet class information is stored in. def class_attributes @@ -13,57 +15,23 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap # Look for our node in ldap. def find(request) - return nil unless information = super - - name = request.key - - node = Puppet::Node.new(name) - - information[:stacked_parameters] = {} - - parent_info = nil - parent = information[:parent] - parents = [name] - while parent - if parents.include?(parent) - raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent - end - parents << parent - - ldapsearch(parent) { |entry| parent_info = process(parent, entry) } - - unless parent_info - raise Puppet::Error.new("Could not find parent node '%s'" % parent) - end - information[:classes] += parent_info[:classes] - parent_info[:stacked].each do |value| - param = value.split('=', 2) - information[:stacked_parameters][param[0]] = param[1] - end - parent_info[:parameters].each do |param, value| - # Specifically test for whether it's set, so false values are handled - # correctly. - information[:parameters][param] = value unless information[:parameters].include?(param) - end - - information[:environment] ||= parent_info[:environment] - - parent = parent_info[:parent] + names = [request.key] + if request.key.include?(".") # we assume it's an fqdn + names << request.key.sub(/\..+/, '') end + names << "default" - information[:stacked].each do |value| - param = value.split('=', 2) - information[:stacked_parameters][param[0]] = param[1] + information = nil + names.each do |name| + break if information = entry2hash(name) end + return nil unless information - information[:stacked_parameters].each do |param, value| - information[:parameters][param] = value unless information[:parameters].include?(param) - end + name = request.key - node.classes = information[:classes].uniq unless information[:classes].empty? - node.parameters = information[:parameters] unless information[:parameters].empty? - node.environment = information[:environment] if information[:environment] - node.fact_merge + node = Puppet::Node.new(name) + + add_to_node(node, information) return node end @@ -155,4 +123,64 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap end filter end + + private + + # Add our hash of ldap information to the node instance. + def add_to_node(node, information) + information[:stacked_parameters] = {} + + parent_info = nil + parent = information[:parent] + parents = [node.name] + while parent + if parents.include?(parent) + raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent + end + parents << parent + parent = find_and_merge_parent(parent, information) + end + + if information[:stacked] + information[:stacked].each do |value| + param = value.split('=', 2) + information[:stacked_parameters][param[0]] = param[1] + end + end + + if information[:stacked_parameters] + information[:stacked_parameters].each do |param, value| + information[:parameters][param] = value unless information[:parameters].include?(param) + end + end + + node.classes = information[:classes].uniq unless information[:classes].nil? or information[:classes].empty? + node.parameters = information[:parameters] unless information[:parameters].nil? or information[:parameters].empty? + node.environment = information[:environment] if information[:environment] + node.fact_merge + end + + # Find information for our parent and merge it into the current info. + def find_and_merge_parent(parent, information) + parent_info = nil + ldapsearch(parent) { |entry| parent_info = process(parent, entry) } + + unless parent_info + raise Puppet::Error.new("Could not find parent node '%s'" % parent) + end + information[:classes] += parent_info[:classes] + parent_info[:stacked].each do |value| + param = value.split('=', 2) + information[:stacked_parameters][param[0]] = param[1] + end + parent_info[:parameters].each do |param, value| + # Specifically test for whether it's set, so false values are handled + # correctly. + information[:parameters][param] = value unless information[:parameters].include?(param) + end + + information[:environment] ||= parent_info[:environment] + + parent_info[:parent] + end end diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb index 576e2265d..14d0f6ac7 100644 --- a/lib/puppet/node.rb +++ b/lib/puppet/node.rb @@ -13,88 +13,7 @@ class Puppet::Node indirects :node, :terminus_setting => :node_terminus, :doc => "Where to find node information. A node is composed of its name, its facts, and its environment." - # Retrieve a node from the node source, with some additional munging - # thrown in for kicks. - def self.find_by_any_name(key) - return nil unless key - - node = nil - names = node_names(key) - names.each do |name| - name = name.to_s if name.is_a?(Symbol) - break if node = find(name) - end - - # If they made it this far, we haven't found anything, so look for a - # default node. - unless node or names.include?("default") - if node = find("default") - Puppet.notice "Using default node for %s" % key - end - end - - return nil unless node - - node.names = names - - # This is critical, because it forces our node's name to always - # be the key, which is nearly always the node's certificate. - # This is how the node instance is linked to the Facts instance, - # so it quite matters. - node.name = key - return node - end - - private - - # Look up the node facts so we can generate the node names to use. - def self.node_facts(key) - if facts = Puppet::Node::Facts.find(key) - facts.values - else - {} - end - end - - # Calculate the list of node names we should use for looking - # up our node. - def self.node_names(key, facts = nil) - facts ||= node_facts(key) - names = [] - - # First, get the fqdn - unless fqdn = facts["fqdn"] - if domain = facts["domain"] - fqdn = facts["hostname"] + "." + facts["domain"] - end - end - - # Now that we (might) have the fqdn, add each piece to the name - # list to search, in order of longest to shortest. - if fqdn - list = fqdn.split(".") - tmp = [] - list.each_with_index do |short, i| - tmp << list[0..i].join(".") - end - names += tmp.reverse - end - - # And make sure the key is first, since that's the most - # likely usage. - # The key is usually the Certificate CN, but it can be - # set to the 'facter' hostname instead. - if Puppet[:node_name] == 'cert' - names.unshift key - else - names.unshift facts["hostname"] - end - names.uniq - end - - public - - attr_accessor :name, :classes, :parameters, :source, :ipaddress, :names + attr_accessor :name, :classes, :parameters, :source, :ipaddress attr_reader :time # Set the environment, making sure that it's valid. @@ -165,4 +84,39 @@ class Puppet::Node @parameters[name] = value unless @parameters.include?(name) end end + + # Calculate the list of names we might use for looking + # up our node. This is only used for AST nodes. + def names + names = [] + + # First, get the fqdn + unless fqdn = parameters["fqdn"] + if domain = parameters["domain"] + fqdn = parameters["hostname"] + "." + parameters["domain"] + end + end + + # Now that we (might) have the fqdn, add each piece to the name + # list to search, in order of longest to shortest. + if fqdn + list = fqdn.split(".") + tmp = [] + list.each_with_index do |short, i| + tmp << list[0..i].join(".") + end + names += tmp.reverse + end + + # And make sure the node name is first, since that's the most + # likely usage. + # The name is usually the Certificate CN, but it can be + # set to the 'facter' hostname instead. + if Puppet[:node_name] == 'cert' + names.unshift name + else + names.unshift parameters["hostname"] + end + names.uniq + end end diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb index 083a9ced5..cf7186a5a 100755 --- a/spec/unit/indirector/catalog/compiler.rb +++ b/spec/unit/indirector/catalog/compiler.rb @@ -23,8 +23,8 @@ describe Puppet::Node::Catalog::Compiler do node1 = stub 'node1', :merge => nil node2 = stub 'node2', :merge => nil compiler.stubs(:compile) - Puppet::Node.stubs(:find_by_any_name).with('node1').returns(node1) - Puppet::Node.stubs(:find_by_any_name).with('node2').returns(node2) + Puppet::Node.stubs(:find).with('node1').returns(node1) + Puppet::Node.stubs(:find).with('node2').returns(node2) compiler.find(stub('request', :key => 'node1', :options => {})) compiler.find(stub('node2request', :key => 'node2', :options => {})) @@ -69,7 +69,7 @@ describe Puppet::Node::Catalog::Compiler, " when finding nodes" do it "should look node information up via the Node class with the provided key" do @node.stubs :merge - Puppet::Node.expects(:find_by_any_name).with(@name).returns(@node) + Puppet::Node.expects(:find).with(@name).returns(@node) @compiler.find(@request) end end @@ -77,7 +77,6 @@ end describe Puppet::Node::Catalog::Compiler, " after finding nodes" do before do Puppet.expects(:version).returns(1) - Puppet.settings.stubs(:value).with(:node_name).returns("cert") Facter.expects(:value).with('fqdn').returns("my.server.com") Facter.expects(:value).with('ipaddress').returns("my.ip.address") @compiler = Puppet::Node::Catalog::Compiler.new @@ -85,7 +84,7 @@ describe Puppet::Node::Catalog::Compiler, " after finding nodes" do @node = mock 'node' @request = stub 'request', :key => @name, :options => {} @compiler.stubs(:compile) - Puppet::Node.stubs(:find_by_any_name).with(@name).returns(@node) + Puppet::Node.stubs(:find).with(@name).returns(@node) end it "should add the server's Puppet version to the node's parameters as 'serverversion'" do @@ -102,13 +101,6 @@ describe Puppet::Node::Catalog::Compiler, " after finding nodes" do @node.expects(:merge).with { |args| args["serverip"] == "my.ip.address" } @compiler.find(@request) end - - # LAK:TODO This is going to be difficult, because this whole process is so - # far removed from the actual connection that the certificate information - # will be quite hard to come by, dum by, gum by. - it "should search for the name using the client certificate's DN if the :node_name setting is set to 'cert'" do - pending "Probably will end up in the REST work" - end end describe Puppet::Node::Catalog::Compiler, " when creating catalogs" do @@ -122,18 +114,18 @@ describe Puppet::Node::Catalog::Compiler, " when creating catalogs" do @node = Puppet::Node.new @name @node.stubs(:merge) @request = stub 'request', :key => @name, :options => {} - Puppet::Node.stubs(:find_by_any_name).with(@name).returns(@node) + Puppet::Node.stubs(:find).with(@name).returns(@node) end it "should directly use provided nodes" do - Puppet::Node.expects(:find_by_any_name).never + Puppet::Node.expects(:find).never @compiler.interpreter.expects(:compile).with(@node) @request.stubs(:options).returns(:node => @node) @compiler.find(@request) end it "should fail if no node is passed and none can be found" do - Puppet::Node.stubs(:find_by_any_name).with(@name).returns(nil) + Puppet::Node.stubs(:find).with(@name).returns(nil) proc { @compiler.find(@request) }.should raise_error(ArgumentError) end diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb index 878039c7c..24b2dd759 100755 --- a/spec/unit/indirector/node/ldap.rb +++ b/spec/unit/indirector/node/ldap.rb @@ -21,14 +21,32 @@ describe Puppet::Node::Ldap do @searcher.stubs(:search_base).returns(:yay) @searcher.stubs(:search_filter).returns(:filter) - @node = mock 'node' + @name = "mynode.domain.com" + @node = stub 'node', :name => @name @node.stubs(:fact_merge) - @name = "mynode" Puppet::Node.stubs(:new).with(@name).returns(@node) @request = stub 'request', :key => @name end + it "should search first for the provided key" do + @searcher.expects(:entry2hash).with("mynode.domain.com").returns({}) + @searcher.find(@request) + end + + it "should search for the short version of the provided key if the key looks like a hostname and no results are found for the key itself" do + @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil) + @searcher.expects(:entry2hash).with("mynode").returns({}) + @searcher.find(@request) + end + + it "should search for default information if no information can be found for the key" do + @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil) + @searcher.expects(:entry2hash).with("mynode").returns(nil) + @searcher.expects(:entry2hash).with("default").returns({}) + @searcher.find(@request) + end + it "should return nil if no results are found in ldap" do @connection.stubs(:search) @searcher.find(@request).should be_nil diff --git a/spec/unit/node.rb b/spec/unit/node.rb index 6603f6b58..c6d2e61bf 100755 --- a/spec/unit/node.rb +++ b/spec/unit/node.rb @@ -54,11 +54,6 @@ describe Puppet::Node, "when initializing" do Puppet.settings.stubs(:value).with(:environments).returns("myenv") proc { Puppet::Node.new("testing", :environment => "other") }.should raise_error(ArgumentError) end - - it "should accept names passed in" do - @node = Puppet::Node.new("testing", :names => ["myenv"]) - @node.names.should == ["myenv"] - end end describe Puppet::Node, "when returning the environment" do @@ -136,42 +131,33 @@ describe Puppet::Node, "when indirecting" do end end -describe Puppet::Node do - # LAK:NOTE This is used to keep track of when a given node has connected, - # so we can report on nodes that do not appear to connecting to the - # central server. - it "should provide a method for noting that the node has connected" -end - describe Puppet::Node, "when generating the list of names to search through" do before do - @facts = Puppet::Node::Facts.new("foo", "hostname" => "yay", "domain" => "domain.com") - @node = Puppet::Node.new("foo") - - Puppet::Node.stubs(:node_facts).returns @facts.values + @node = Puppet::Node.new("foo.domain.com") + @node.parameters = {"hostname" => "yay", "domain" => "domain.com"} end it "should return an array of names" do - Puppet::Node.node_names("foo").should be_instance_of(Array) + @node.names.should be_instance_of(Array) end it "should have the node's fqdn as the second name" do - Puppet::Node.node_names("foo.domain.com")[1].should == "yay.domain.com" + @node.names[1].should == "yay.domain.com" end it "should set the fqdn to the node's 'fqdn' fact if it is available" do - @facts.values["fqdn"] = "boo.domain.com" - Puppet::Node.node_names("foo")[1].should == "boo.domain.com" + @node.parameters["fqdn"] = "boo.domain.com" + @node.names[1].should == "boo.domain.com" end it "should set the fqdn to the node's hostname and domain if no fqdn is available" do - Puppet::Node.node_names("foo")[1].should == "yay.domain.com" + @node.names[1].should == "yay.domain.com" end it "should contain an entry for each name available by stripping a segment of the fqdn" do - @facts.values["fqdn"] = "foo.deep.sub.domain.com" - Puppet::Node.node_names("foo")[2].should == "foo.deep.sub.domain" - Puppet::Node.node_names("foo")[3].should == "foo.deep.sub" + @node.parameters["fqdn"] = "foo.deep.sub.domain.com" + @node.names[2].should == "foo.deep.sub.domain" + @node.names[3].should == "foo.deep.sub" end describe "and :node_name is set to 'cert'" do @@ -180,7 +166,7 @@ describe Puppet::Node, "when generating the list of names to search through" do end it "should use the passed-in key as the first value" do - Puppet::Node.node_names("foo")[0].should == "foo" + @node.names[0].should == "foo.domain.com" end end @@ -190,45 +176,7 @@ describe Puppet::Node, "when generating the list of names to search through" do end it "should use the node's 'hostname' fact as the first value" do - Puppet::Node.node_names("foo")[0].should == "yay" + @node.names[0].should == "yay" end end end - -describe Puppet::Node, "when searching for nodes" do - before do - @facts = Puppet::Node::Facts.new("foo", "hostname" => "yay", "domain" => "domain.com") - @node = Puppet::Node.new("foo") - Puppet::Node::Facts.stubs(:find).with("foo").returns(@facts) - end - - it "should use the 'node_names' method to get its list of names to search" do - Puppet::Node.expects(:node_names).with{ |*args| args[0] == "foo" }.returns %w{a b} - Puppet::Node.stubs(:find) - Puppet::Node.find_by_any_name("foo") - end - - it "should return the first node found using the generated list of names" do - Puppet::Node.expects(:node_names).returns %w{a b} - Puppet::Node.expects(:find).with("a").returns(nil) - Puppet::Node.expects(:find).with("b").returns(@node) - Puppet::Node.find_by_any_name("foo").should equal(@node) - end - - it "should attempt to find a default node if no names are found" do - names = [] - Puppet::Node.stubs(:find).with do |name| - names << name - end.returns(nil) - Puppet::Node.find_by_any_name("foo") - names[-1].should == "default" - end - - it "should set the node name to the provided key" do - Puppet::Node.stubs(:node_names).returns %w{a b} - Puppet::Node.stubs(:find).returns @node - - @node.expects(:name=).with("foo") - Puppet::Node.find_by_any_name("foo") - end -end |