Another intermediate commit. The node and fact classes are now functional and are used instead of the network handlers, which have been removed. There are some failing tests as a result, but I want to get this code committed before I massage the rest of the system to make it work again.

3 files changed, 61 insertions, 29 deletions

diff --git a/lib/puppet/indirector/node/external.rb b/lib/puppet/indirector/node/external.rb
index ed2a8893e..13cd265fb 100644
--- a/lib/puppet/indirector/node/external.rb
+++ b/lib/puppet/indirector/node/external.rb
@@ -1,18 +1,66 @@
+require 'puppet/node/facts'
+
 Puppet::Indirector.register_terminus :node, :external do
     desc "Call an external program to get node information."
 
     include Puppet::Util
+
+    # Proxy the execution, so it's easier to test.
+    def execute(command)
+        Puppet::Util.execute(command)
+    end
+
     # Look for external node definitions.
     def get(name)
-        return nil unless Puppet[:external_nodes] != "none"
+        unless Puppet[:external_nodes] != "none"
+            raise ArgumentError, "You must set the 'external_nodes' parameter to use the external node source"
+        end
   
+        unless Puppet[:external_nodes][0] == File::SEPARATOR[0]
+            raise ArgumentError, "You must set the 'external_nodes' parameter to a fully qualified command"
+        end
+
+        # Run the command.
+        unless output = query(name)
+            return nil
+        end
+
+        # Translate the output to ruby.
+        result = translate(name, output)
+
+        return create_node(name, result)
+    end
+
+    private
+
+    # Turn our outputted objects into a Puppet::Node instance.
+    def create_node(name, result)
+        node = Puppet::Node.new(name)
+        set = false
+        [:parameters, :classes].each do |param|
+            if value = result[param]
+                node.send(param.to_s + "=", value)
+                set = true
+            end
+        end
+
+        if set
+            node.fact_merge
+            return node
+        else
+            return nil
+        end
+    end
+
+    # Call the external command and see if it returns our output.
+    def query(name)
         # This is a very cheap way to do this, since it will break on
         # commands that have spaces in the arguments.  But it's good
         # enough for most cases.
         external_node_command = Puppet[:external_nodes].split
         external_node_command << name
         begin
-            output = Puppet::Util.execute(external_node_command)
+            output = execute(external_node_command)
         rescue Puppet::ExecutionFailure => detail
             if $?.exitstatus == 1
                 return nil
@@ -25,31 +73,17 @@ Puppet::Indirector.register_terminus :node, :external do
         if output =~ /\A\s*\Z/ # all whitespace
             Puppet.debug "Empty response for %s from external node source" % name
             return nil
+        else
+            return output
         end
+    end
 
+    # Translate the yaml string into Ruby objects.
+    def translate(name, output)
         begin
-            result = YAML.load(output).inject({}) { |hash, data| hash[symbolize(data[0])] = data[1]; hash }
+            YAML.load(output).inject({}) { |hash, data| hash[symbolize(data[0])] = data[1]; hash }
         rescue => detail
             raise Puppet::Error, "Could not load external node results for %s: %s" % [name, detail]
         end
-
-        node = Puppe::Node.new(name)
-        set = false
-        [:parameters, :classes].each do |param|
-            if value = result[param]
-                node.send(param.to_s + "=", value)
-                set = true
-            end
-        end
-
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
-
-        if set
-            return node
-        else
-            return nil
-        end
     end
 end
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 77be04126..fb60cad31 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -18,10 +18,8 @@ Puppet::Indirector.register_terminus :node, :ldap do
             end
         end
 
-        node = Puppe::Node.new(name, :classes => classes, :source => "ldap", :parameters => parameters)
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
+        node = Puppet::Node.new(name, :classes => classes, :source => "ldap", :parameters => parameters)
+        node.fact_merge
         return node
     end
 
diff --git a/lib/puppet/indirector/node/none.rb b/lib/puppet/indirector/node/none.rb
index 7143033d9..2b326968e 100644
--- a/lib/puppet/indirector/node/none.rb
+++ b/lib/puppet/indirector/node/none.rb
@@ -1,3 +1,5 @@
+require 'puppet/node/facts'
+
 Puppet::Indirector.register_terminus :node, :none do
     desc "Always return an empty node object.  This is the node source you should
         use when you don't have some other, functional source you want to use,
@@ -6,9 +8,7 @@ Puppet::Indirector.register_terminus :node, :none do
     # Just return an empty node.
     def get(name)
         node = Puppet::Node.new(name)
-        if facts = Puppet::Node.facts(name)
-            node.fact_merge(facts)
-        end
+        node.fact_merge
         node
     end
 end