The new configuration handler looks to be ready for usage. Now I just need to convert the interpreter to use SimpleNode objects, then continue with the Configuration object.

3 files changed, 301 insertions, 158 deletions

diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
index 7f81879ba..8c82d2e8f 100644
--- a/lib/puppet/network/handler/configuration.rb
+++ b/lib/puppet/network/handler/configuration.rb
@@ -8,216 +8,192 @@ require 'yaml'
 class Puppet::Network::Handler
     class Configuration < Handler
         desc "Puppet's configuration compilation interface.  Passed a node name
-            or other key, retrieves information about the node and returns a
-            compiled configuration."
+            or other key, retrieves information about the node (using the ``node_source``)
+            and returns a compiled configuration."
 
         include Puppet::Util
 
-        attr_accessor :ast, :local
-        attr_reader :ca
+        attr_accessor :local
 
         @interface = XMLRPC::Service::Interface.new("configuration") { |iface|
                 iface.add_method("string configuration(string)")
                 iface.add_method("string version()")
         }
 
-        # FIXME At some point, this should be autodocumenting.
-        def addfacts(facts)
-            # Add our server version to the fact list
-            facts["serverversion"] = Puppet.version.to_s
-
-            # And then add the server name and IP
-            {"servername" => "fqdn",
-                "serverip" => "ipaddress"
-            }.each do |var, fact|
-                if obj = Facter[fact]
-                    facts[var] = obj.value
-                else
-                    Puppet.warning "Could not retrieve fact %s" % fact
-                end
+        # Compile a node's configuration.
+        def configuration(key, client = nil, clientip = nil)
+            # Note that this is reasonable, because either their node source should actually
+            # know about the node, or they should be using the ``none`` node source, which
+            # will always return data.
+            unless node = node_handler.details(key)
+                raise Puppet::Error, "Could not find node '%s'" % key
             end
 
-            if facts["servername"].nil?
-                host = Facter.value(:hostname)
-                if domain = Facter.value(:domain)
-                    facts["servername"] = [host, domain].join(".")
-                else
-                    facts["servername"] = host
-                end
-            end
+            # Add any external data to the node.
+            add_node_data(node)
+
+            return translate(compile(node))
         end
 
-        # Manipulate the client name as appropriate.
-        def clientname(name, ip, facts)
-            # Always use the hostname from Facter.
-            client = facts["hostname"]
-            clientip = facts["ipaddress"]
-            if Puppet[:node_name] == 'cert'
-                if name
-                    client = name
-                end
-                if ip
-                    clientip = ip
-                end
+        def initialize(options = {})
+            if options[:Local]
+                @local = options[:Local]
+            else
+                @local = false
             end
 
-            return client, clientip
+            # Just store the options, rather than creating the interpreter
+            # immediately.  Mostly, this is so we can create the interpreter
+            # on-demand, which is easier for testing.
+            @options = options
         end
 
-        # Tell a client whether there's a fresh config for it
-        def freshness(client = nil, clientip = nil)
-            if Puppet.features.rails? and Puppet[:storeconfigs]
-                Puppet::Rails.connect
+        # Are we running locally, or are our clients networked?
+        def local?
+            self.local
+        end
 
-                host = Puppet::Rails::Host.find_or_create_by_name(client)
-                host.last_freshcheck = Time.now
-                if clientip and (! host.ip or host.ip == "" or host.ip == "NULL")
-                    host.ip = clientip
-                end
-                host.save
-            end
-            if defined? @interpreter
-                return @interpreter.parsedate
-            else
-                return 0
+        # Return the configuration version.
+        def version(client = nil, clientip = nil)
+            # If we can find the node, then store the fact that the node
+            # has checked in.
+            if node = node_handler.search(client)
+                update_node_freshness(client)
             end
+            interpreter.parsedate
         end
 
-        def initialize(hash = {})
-            args = {}
+        private
 
-            # Allow specification of a code snippet or of a file
-            if code = hash[:Code]
-                args[:Code] = code
-            else
-                args[:Manifest] = hash[:Manifest] || Puppet[:manifest]
+        # Add any extra data necessary to the node.
+        def add_node_data(node)
+            # Merge in our server-side facts, so they can be used during compilation.
+            node.fact_merge(@server_facts)
+
+            # Add any specified classes to the node's class list.
+            if classes = @options[:Classes]
+                classes.each do |klass|
+                    node.classes << klass
+                end
             end
+        end
 
-            if hash[:Local]
-                @local = hash[:Local]
+        # Compile the actual configuration.
+        def compile(node)
+            # Pick the benchmark level.
+            if local?
+                level = :none
             else
-                @local = false
+                level = :notice
             end
 
-            args[:Local] = @local
+            # Ask the interpreter to compile the configuration.
+            config = nil
+            benchmark(level, "Compiled configuration for %s" % node.name) do
+                begin
+                    config = interpreter.compile(node)
+                rescue Puppet::Error => detail
+                    Puppet.err detail
+                    raise XMLRPC::FaultException.new(
+                        1, detail.to_s
+                    )
+                end
+            end
 
-            if hash.include?(:CA) and hash[:CA]
-                @ca = Puppet::SSLCertificates::CA.new()
+            return config
+        end
+
+        # Create our interpreter object.
+        def create_interpreter(options)
+            args = {}
+
+            # Allow specification of a code snippet or of a file
+            if code = options[:Code]
+                args[:Code] = code
             else
-                @ca = nil
+                args[:Manifest] = options[:Manifest] || Puppet[:manifest]
             end
 
-            Puppet.debug("Creating interpreter")
+            args[:Local] = local?
 
-            if hash.include?(:UseNodes)
-                args[:UseNodes] = hash[:UseNodes]
+            if options.include?(:UseNodes)
+                args[:UseNodes] = options[:UseNodes]
             elsif @local
                 args[:UseNodes] = false
             end
 
             # This is only used by the cfengine module, or if --loadclasses was
             # specified in +puppet+.
-            if hash.include?(:Classes)
-                args[:Classes] = hash[:Classes]
+            if options.include?(:Classes)
+                args[:Classes] = options[:Classes]
             end
 
-            @interpreter = Puppet::Parser::Interpreter.new(args)
+            return Puppet::Parser::Interpreter.new(args)
         end
 
-        def getconfig(facts, format = "marshal", client = nil, clientip = nil)
-            if @local
-                # we don't need to do anything, since we should already
-                # have raw objects
-                Puppet.debug "Our client is local"
-            else
-                Puppet.debug "Our client is remote"
-
-                # XXX this should definitely be done in the protocol, somehow
-                case format
-                when "marshal":
-                    Puppet.warning "You should upgrade your client.  'Marshal' will not be supported much longer."
-                    begin
-                        facts = Marshal::load(CGI.unescape(facts))
-                    rescue => detail
-                        raise XMLRPC::FaultException.new(
-                            1, "Could not rebuild facts"
-                        )
-                    end
-                when "yaml":
-                    begin
-                        facts = YAML.load(CGI.unescape(facts))
-                    rescue => detail
-                        raise XMLRPC::FaultException.new(
-                            1, "Could not rebuild facts"
-                        )
-                    end
-                else
-                    raise XMLRPC::FaultException.new(
-                        1, "Unavailable config format %s" % format
-                    )
-                end
+        # Create/return our interpreter.
+        def interpreter
+            unless defined?(@interpreter) and @interpreter
+                @interpreter = create_interpreter(@options)
             end
+            @interpreter
+        end
 
-            client, clientip = clientname(client, clientip, facts)
-
-            # Add any server-side facts to our server.
-            addfacts(facts)
-
-            retobjects = nil
-
-            # This is hackish, but there's no "silence" option for benchmarks
-            # right now
-            if @local
-                #begin
-                    retobjects = @interpreter.run(client, facts)
-                #rescue Puppet::Error => detail
-                #    Puppet.err detail
-                #    raise XMLRPC::FaultException.new(
-                #        1, detail.to_s
-                #    )
-                #rescue => detail
-                #    Puppet.err detail.to_s
-                #    return ""
-                #end
-            else
-                benchmark(:notice, "Compiled configuration for %s" % client) do
-                    begin
-                        retobjects = @interpreter.run(client, facts)
-                    rescue Puppet::Error => detail
-                        Puppet.err detail
-                        raise XMLRPC::FaultException.new(
-                            1, detail.to_s
-                        )
-                    rescue => detail
-                        Puppet.err detail.to_s
-                        return ""
-                    end
+        # Create a node handler instance for looking up our nodes.
+        def node_handler
+            unless defined?(@node_handler)
+                @node_handler = Puppet::Network::Handler.handler(:node).new
+            end
+            @node_handler
+        end
+
+        # Initialize our server fact hash; we add these to each client, and they
+        # won't change while we're running, so it's safe to cache the values.
+        def set_server_facts
+            @server_facts = {}
+
+            # Add our server version to the fact list
+            @server_facts["serverversion"] = Puppet.version.to_s
+
+            # And then add the server name and IP
+            {"servername" => "fqdn",
+                "serverip" => "ipaddress"
+            }.each do |var, fact|
+                if value = Facter.value(fact)
+                    @server_facts[var] = value
+                else
+                    Puppet.warning "Could not retrieve fact %s" % fact
                 end
             end
 
-            if @local
-                return retobjects
-            else
-                str = nil
-                case format
-                when "marshal":
-                    str = Marshal::dump(retobjects)
-                when "yaml":
-                    str = retobjects.to_yaml(:UseBlock => true)
+            if @server_facts["servername"].nil?
+                host = Facter.value(:hostname)
+                if domain = Facter.value(:domain)
+                    @server_facts["servername"] = [host, domain].join(".")
                 else
-                    raise XMLRPC::FaultException.new(
-                        1, "Unavailable config format %s" % format
-                    )
+                    @server_facts["servername"] = host
                 end
-                return CGI.escape(str)
             end
         end
 
-        def local?
-            if defined? @local and @local
-                return true
+        # Translate our configuration appropriately for sending back to a client.
+        def translate(config)
+            if local?
+                config
             else
-                return false
+                CGI.escape(config.to_yaml(:UseBlock => true))
+            end
+        end
+
+        # Mark that the node has checked in. FIXME this needs to be moved into
+        # the SimpleNode class, or somewhere that's got abstract backends.
+        def update_node_freshness(node)
+            if Puppet.features.rails? and Puppet[:storeconfigs]
+                Puppet::Rails.connect
+
+                host = Puppet::Rails::Host.find_or_create_by_name(node.name)
+                host.last_freshcheck = Time.now
+                host.save
             end
         end
     end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index d1d14977c..5a10f5344 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -201,7 +201,7 @@ module Util
             raise Puppet::DevError, "Failed to provide level to :benchmark"
         end
 
-        unless object.respond_to? level
+        unless level == :none or object.respond_to? level
             raise Puppet::DevError, "Benchmarked object does not respond to %s" % level
         end
 
diff --git a/test/network/handler/configuration.rb b/test/network/handler/configuration.rb
new file mode 100755
index 000000000..7a505f5eb
--- /dev/null
+++ b/test/network/handler/configuration.rb
@@ -0,0 +1,167 @@
+#!/usr/bin/env ruby
+
+$:.unshift("../../lib") if __FILE__ =~ /\.rb$/
+
+require 'puppettest'
+require 'puppet/network/handler/configuration'
+
+class TestHandlerConfiguration < Test::Unit::TestCase
+	include PuppetTest
+
+    Config = Puppet::Network::Handler.handler(:configuration)
+
+    # Check all of the setup stuff.
+    def test_initialize
+        config = nil
+        assert_nothing_raised("Could not create local config") do
+            config = Config.new(:Local => true)
+        end
+
+        assert(config.local?, "Config is not considered local after being started that way")
+    end
+
+    # Make sure we create the node handler when necessary.
+    def test_node_handler
+        config = Config.new
+        handler = nil
+        assert_nothing_raised("Could not create node handler") do
+            handler = config.send(:node_handler)
+        end
+        assert_instance_of(Puppet::Network::Handler.handler(:node), handler, "Did not create node handler")
+
+        # Now make sure we get the same object back again
+        assert_equal(handler.object_id, config.send(:node_handler).object_id, "Did not cache node handler")
+    end
+
+    # Test creation/returning of the interpreter
+    def test_interpreter
+        config = Config.new
+
+        # First test the defaults
+        args = {}
+        config.instance_variable_set("@options", args)
+        config.expects(:create_interpreter).with(args).returns(:interp)
+        assert_equal(:interp, config.send(:interpreter), "Did not return the interpreter")
+
+        # Now run it again and make sure we get the same thing
+        assert_equal(:interp, config.send(:interpreter), "Did not cache the interpreter")
+    end
+
+    def test_create_interpreter
+        config = Config.new(:Local => false)
+        args = {}
+
+        # Try it first with defaults.
+        Puppet::Parser::Interpreter.expects(:new).with(:Local => config.local?, :Manifest => Puppet[:manifest]).returns(:interp)
+        assert_equal(:interp, config.send(:create_interpreter, args), "Did not return the interpreter")
+
+        # Now reset it and make sure a specified manifest passes through
+        file = tempfile
+        args[:Manifest] = file
+        Puppet::Parser::Interpreter.expects(:new).with(:Local => config.local?, :Manifest => file).returns(:interp)
+        assert_equal(:interp, config.send(:create_interpreter, args), "Did not return the interpreter")
+
+        # And make sure the code does, too
+        args.delete(:Manifest)
+        args[:Code] = "yay"
+        Puppet::Parser::Interpreter.expects(:new).with(:Local => config.local?, :Code => "yay").returns(:interp)
+        assert_equal(:interp, config.send(:create_interpreter, args), "Did not return the interpreter")
+    end
+
+    # Make sure node objects get appropriate data added to them.
+    def test_add_node_data
+        # First with no classes
+        config = Config.new
+
+        fakenode = Object.new
+        # Set the server facts to something
+        config.instance_variable_set("@server_facts", :facts)
+        fakenode.expects(:fact_merge).with(:facts)
+        config.send(:add_node_data, fakenode)
+
+        # Now try it with classes.
+        config.instance_variable_set("@options", {:Classes => %w{a b}})
+        list = []
+        fakenode = Object.new
+        fakenode.expects(:fact_merge).with(:facts)
+        fakenode.expects(:classes).returns(list).times(2)
+        config.send(:add_node_data, fakenode)
+        assert_equal(%w{a b}, list, "Did not add classes to node")
+    end
+
+    def test_compile
+        config = Config.new
+
+        # First do a local
+        node = Object.new
+        node.expects(:name).returns(:mynode)
+
+        interp = Object.new
+        interp.expects(:compile).with(node).returns(:config)
+        config.expects(:interpreter).returns(interp)
+
+        Puppet.expects(:notice) # The log message from benchmarking
+
+        assert_equal(:config, config.send(:compile, node), "Did not return config")
+        
+        # Now try it non-local
+        config = Config.new(:Local => true)
+
+        node = Object.new
+        node.expects(:name).returns(:mynode)
+
+        interp = Object.new
+        interp.expects(:compile).with(node).returns(:config)
+        config.expects(:interpreter).returns(interp)
+
+        assert_equal(:config, config.send(:compile, node), "Did not return config")
+    end
+
+    def test_set_server_facts
+        config = Config.new
+        assert_nothing_raised("Could not call :set_server_facts") do
+            config.send(:set_server_facts)
+        end
+        facts = config.instance_variable_get("@server_facts")
+        %w{servername serverversion serverip}.each do |fact|
+            assert(facts.include?(fact), "Config did not set %s fact" % fact)
+        end
+    end
+
+    def test_translate
+        # First do a local config
+        config = Config.new(:Local => true)
+        assert_equal(:plain, config.send(:translate, :plain), "Attempted to translate local config")
+
+        # Now a non-local
+        config = Config.new(:Local => false)
+        obj = Object.new
+        yamld = Object.new
+        obj.expects(:to_yaml).with(:UseBlock => true).returns(yamld)
+        CGI.expects(:escape).with(yamld).returns(:translated)
+        assert_equal(:translated, config.send(:translate, obj), "Did not return translated config")
+    end
+
+    # Check that we're storing the node freshness into the rails db.  Hackilicious.
+    def test_update_node_freshness
+        # This is stupid.
+        config = Config.new
+        node = Object.new
+        node.expects(:name).returns(:hostname)
+        now = Object.new
+        Time.expects(:now).returns(now)
+        host = Object.new
+        host.expects(:last_freshcheck=).with(now)
+        host.expects(:save)
+
+        # Only test the case where rails is there
+        Puppet[:storeconfigs] = true
+        Puppet.features.expects(:rails?).returns(true)
+        Puppet::Rails.expects(:connect)
+        Puppet::Rails::Host.expects(:find_or_create_by_name).with(:hostname).returns(host)
+
+        config.send(:update_node_freshness, node)
+
+
+    end
+end