Taking a first stab at moving configuration compiling

into the indirection system.  There are still quite
a few unanswered questions, the two most notable
being embodied in unimplemented tests in the Configuration
Code terminus.

This also requires changing the behaviour in a few places.
In particular, 'puppet' and the 'module_puppet' cfengine
module need to store a Node object in memory with the appropriate
classes, since that's now the only way to communicate with
the compiler.  That integration work has not yet been done,
partially because the old configuration handler (which the
soon-to-be-deprecated master handler now uses) still exists.

8 files changed, 391 insertions, 10 deletions

diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index bd2487e33..6ff2de1b4 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -16,7 +16,7 @@ module Puppet::Indirector
     # evaluated at parse time, which is before the user has had a chance
     # to override it.
     def indirects(indirection)
-        raise(ArgumentError, "Already handling indirection for %s; cannot also handle %s" % [@indirection.name, indirection]) if defined?(@indirection) and indirection
+        raise(ArgumentError, "Already handling indirection for %s; cannot also handle %s" % [@indirection.name, indirection]) if defined?(@indirection) and @indirection
         # populate this class with the various new methods
         extend ClassMethods
         include InstanceMethods
diff --git a/lib/puppet/indirector/code.rb b/lib/puppet/indirector/code.rb
new file mode 100644
index 000000000..0c0ee146b
--- /dev/null
+++ b/lib/puppet/indirector/code.rb
@@ -0,0 +1,6 @@
+require 'puppet/indirector/terminus'
+
+# Do nothing, requiring that the back-end terminus do all
+# of the work.
+class Puppet::Indirector::Code < Puppet::Indirector::Terminus
+end
diff --git a/lib/puppet/indirector/code/configuration.rb b/lib/puppet/indirector/code/configuration.rb
new file mode 100644
index 000000000..6d0317204
--- /dev/null
+++ b/lib/puppet/indirector/code/configuration.rb
@@ -0,0 +1,171 @@
+require 'puppet/node'
+require 'puppet/node/configuration'
+require 'puppet/indirector/code'
+require 'puppet/parser/interpreter'
+require 'yaml'
+
+class Puppet::Indirector::Code::Configuration < Puppet::Indirector::Code
+    desc "Puppet's configuration compilation interface.  Passed a node name
+        or other key, retrieves information about the node (using the ``node_source``)
+        and returns a compiled configuration."
+
+    include Puppet::Util
+
+    attr_accessor :code
+
+    # Compile a node's configuration.
+    def find(key, client = nil, clientip = nil)
+        # If we want to use the cert name as our key
+        if Puppet[:node_name] == 'cert' and client
+            key = client
+        end
+
+        # 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 = Puppet::Node.search(key)
+            raise Puppet::Error, "Could not find node '%s'" % key
+        end
+
+        # Add any external data to the node.
+        add_node_data(node)
+
+        configuration = compile(node)
+
+        return configuration
+    end
+
+    def initialize
+        set_server_facts
+    end
+
+    # Create/return our interpreter.
+    def interpreter
+        unless defined?(@interpreter) and @interpreter
+            @interpreter = create_interpreter
+        end
+        @interpreter
+    end
+
+    # Return the configuration version.
+    def version(client = nil, clientip = nil)
+        if client and node = Puppet::Node.search(client)
+            update_node_check(node)
+            return interpreter.configuration_version(node)
+        else
+            # Just return something that will always result in a recompile, because
+            # this is local.
+            return (Time.now + 1000).to_i
+        end
+    end
+
+    private
+
+    # 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.merge(@server_facts)
+    end
+
+    # Compile the actual configuration.
+    def compile(node)
+        # Ask the interpreter to compile the configuration.
+        str = "Compiled configuration for %s" % node.name
+        if node.environment
+            str += " in environment %s" % node.environment
+        end
+        config = nil
+
+        # LAK:FIXME This should log at :none when our client is
+        # local, since we don't want 'puppet' (vs. puppetmasterd) to
+        # log compile times.
+        benchmark(:notice, "Compiled configuration for %s" % node.name) do
+            begin
+                config = interpreter.compile(node)
+            rescue Puppet::Error => detail
+                if Puppet[:trace]
+                    puts detail.backtrace
+                end
+                unless local?
+                    Puppet.err detail.to_s
+                end
+                raise XMLRPC::FaultException.new(
+                    1, detail.to_s
+                )
+            end
+        end
+
+        return config
+    end
+
+    # Create our interpreter object.
+    def create_interpreter
+        args = {}
+
+        # Allow specification of a code snippet or of a file
+        if self.code
+            args[:Code] = self.code
+        end
+
+        # LAK:FIXME This needs to be handled somehow.
+        #if options.include?(:UseNodes)
+        #    args[:UseNodes] = options[:UseNodes]
+        #elsif @local
+        #    args[:UseNodes] = false
+        #end
+
+        return Puppet::Parser::Interpreter.new(args)
+    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 @server_facts["servername"].nil?
+            host = Facter.value(:hostname)
+            if domain = Facter.value(:domain)
+                @server_facts["servername"] = [host, domain].join(".")
+            else
+                @server_facts["servername"] = host
+            end
+        end
+    end
+
+    # Translate our configuration appropriately for sending back to a client.
+    # LAK:FIXME This method should probably be part of the protocol, but it
+    # shouldn't be here.
+    def translate(config)
+        if local?
+            config
+        else
+            CGI.escape(config.to_yaml(:UseBlock => true))
+        end
+    end
+
+    # Mark that the node has checked in. LAK:FIXME this needs to be moved into
+    # the Node class, or somewhere that's got abstract backends.
+    def update_node_check(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/node/configuration.rb b/lib/puppet/node/configuration.rb
index 0ae03a651..53f63d003 100644
--- a/lib/puppet/node/configuration.rb
+++ b/lib/puppet/node/configuration.rb
@@ -1,3 +1,4 @@
+require 'puppet/indirector'
 require 'puppet/external/gratr/digraph'
 
 # This class models a node configuration.  It is the thing
@@ -5,6 +6,9 @@ require 'puppet/external/gratr/digraph'
 # of the information in the configuration, including the resources
 # and the relationships between them.
 class Puppet::Node::Configuration < Puppet::PGraph
+    extend Puppet::Indirector
+    indirects :configuration
+
     # The host name this is a configuration for.
     attr_accessor :name
 
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index f2af13dc2..1478cd8a5 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -45,15 +45,17 @@ class Puppet::Util::Settings
     end
 
     # A simplified equality operator.
-    def ==(other)
-        self.each { |myname, myobj|
-            unless other[myname] == value(myname)
-                return false
-            end
-        }
-
-        return true
-    end
+    # LAK: For some reason, this causes mocha to not be able to mock
+    # the 'value' method, and it's not used anywhere.
+#    def ==(other)
+#        self.each { |myname, myobj|
+#            unless other[myname] == value(myname)
+#                return false
+#            end
+#        }
+#
+#        return true
+#    end
 
     # Generate the list of valid arguments, in a format that GetoptLong can
     # understand, and add them to the passed option list.
diff --git a/spec/unit/indirector/code.rb b/spec/unit/indirector/code.rb
new file mode 100755
index 000000000..f34dcf402
--- /dev/null
+++ b/spec/unit/indirector/code.rb
@@ -0,0 +1,33 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+require 'puppet/indirector/code'
+
+describe Puppet::Indirector::Code do
+    before do
+        Puppet::Indirector::Terminus.stubs(:register_terminus_class)
+        @model = mock 'model'
+        @indirection = stub 'indirection', :name => :mystuff, :register_terminus_type => nil, :model => @model
+        Puppet::Indirector::Indirection.stubs(:instance).returns(@indirection)
+
+        @code_class = Class.new(Puppet::Indirector::Code) do
+            def self.to_s
+                "Testing"
+            end
+        end
+
+        @searcher = @code_class.new
+    end
+
+    it "should not have a find() method defined" do
+        @searcher.should_not respond_to(:find)
+    end
+
+    it "should not have a save() method defined" do
+        @searcher.should_not respond_to(:save)
+    end
+
+    it "should not have a destroy() method defined" do
+        @searcher.should_not respond_to(:destroy)
+    end
+end
diff --git a/spec/unit/indirector/code/configuration.rb b/spec/unit/indirector/code/configuration.rb
new file mode 100755
index 000000000..8652f342d
--- /dev/null
+++ b/spec/unit/indirector/code/configuration.rb
@@ -0,0 +1,158 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2007-9-23.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/indirector/code/configuration'
+
+describe Puppet::Indirector::Code::Configuration do
+    # LAK:TODO I have no idea how to do this, or even if it should be in this class or test or what.
+    # This is used for determining if the client should recompile its configuration, so it's not sufficient
+    # to recompile and compare versions.
+    #   It might be that the right solution is to require configuration caching, and then compare the cached
+    # configuration version to the current version, via some querying mechanism (i.e., the client asks for just
+    # the configuration's 'up-to-date' attribute, rather than the whole configuration).
+    it "should provide a mechanism for determining if the client's configuration is up to date"
+end
+
+describe Puppet::Indirector::Code::Configuration do
+    before do
+        Puppet.expects(:version).returns(1)
+        Facter.expects(:value).with('fqdn').returns("my.server.com")
+        Facter.expects(:value).with('ipaddress').returns("my.ip.address")
+    end
+
+    it "should gather data about itself" do
+        Puppet::Indirector::Code::Configuration.new
+    end
+
+    it "should cache the server metadata and reuse it" do
+        compiler = Puppet::Indirector::Code::Configuration.new
+        node1 = stub 'node1', :merge => nil
+        node2 = stub 'node2', :merge => nil
+        compiler.stubs(:compile)
+        Puppet::Node.stubs(:search).with('node1').returns(node1)
+        Puppet::Node.stubs(:search).with('node2').returns(node2)
+
+        compiler.find('node1')
+        compiler.find('node2')
+    end
+end
+
+describe Puppet::Indirector::Code::Configuration, " when creating the interpreter" do
+    before do
+        @compiler = Puppet::Indirector::Code::Configuration.new
+    end
+
+    it "should not create the interpreter until it is asked for the first time" do
+        interp = mock 'interp'
+        Puppet::Parser::Interpreter.expects(:new).with({}).returns(interp)
+        @compiler.interpreter.should equal(interp)
+    end
+
+    it "should use the same interpreter for all compiles" do
+        interp = mock 'interp'
+        Puppet::Parser::Interpreter.expects(:new).with({}).returns(interp)
+        @compiler.interpreter.should equal(interp)
+        @compiler.interpreter.should equal(interp)
+    end
+
+    it "should provide a mechanism for setting the code to pass to the interpreter" do
+        @compiler.should respond_to(:code=)
+    end
+
+    it "should pass any specified code on to the interpreter when it is being initialized" do
+        code = "some code"
+        @compiler.code = code
+        interp = mock 'interp'
+        Puppet::Parser::Interpreter.expects(:new).with(:Code => code).returns(interp)
+        @compiler.send(:interpreter).should equal(interp)
+    end
+
+end
+
+describe Puppet::Indirector::Code::Configuration, " when finding nodes" do
+    before do
+        @compiler = Puppet::Indirector::Code::Configuration.new
+        @name = "me"
+        @node = mock 'node'
+        @compiler.stubs(:compile)
+    end
+
+    it "should look node information up via the Node class with the provided key" do
+        @node.stubs :merge 
+        Puppet::Node.expects(:search).with(@name).returns(@node)
+        @compiler.find(@name)
+    end
+
+    it "should fail if it cannot find the node" do
+        @node.stubs :merge 
+        Puppet::Node.expects(:search).with(@name).returns(nil)
+        proc { @compiler.find(@name) }.should raise_error(Puppet::Error)
+    end
+end
+
+describe Puppet::Indirector::Code::Configuration, " 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::Indirector::Code::Configuration.new
+        @name = "me"
+        @node = mock 'node'
+        @compiler.stubs(:compile)
+        Puppet::Node.stubs(:search).with(@name).returns(@node)
+    end
+
+    it "should add the server's Puppet version to the node's parameters as 'serverversion'" do
+        @node.expects(:merge).with { |args| args["serverversion"] == "1" }
+        @compiler.find(@name)
+    end
+
+    it "should add the server's fqdn to the node's parameters as 'servername'" do
+        @node.expects(:merge).with { |args| args["servername"] == "my.server.com" }
+        @compiler.find(@name)
+    end
+
+    it "should add the server's IP address to the node's parameters as 'serverip'" do
+        @node.expects(:merge).with { |args| args["serverip"] == "my.ip.address" }
+        @compiler.find(@name)
+    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'"
+end
+
+describe Puppet::Indirector::Code::Configuration, " when creating configurations" do
+    before do
+        @compiler = Puppet::Indirector::Code::Configuration.new
+        @name = "me"
+        @node = stub 'node', :merge => nil, :name => @name, :environment => "yay"
+        Puppet::Node.stubs(:search).with(@name).returns(@node)
+    end
+
+    it "should pass the found node to the interpreter for compiling" do
+        config = mock 'config'
+        @compiler.interpreter.expects(:compile).with(@node)
+        @compiler.find(@name)
+    end
+
+    it "should return the results of compiling as the configuration" do
+        config = mock 'config'
+        @compiler.interpreter.expects(:compile).with(@node).returns(:configuration)
+        @compiler.find(@name).should == :configuration
+    end
+
+    it "should benchmark the compile process" do
+        @compiler.expects(:benchmark).with do |level, message|
+            level == :notice and message =~ /^Compiled configuration/
+        end
+        @compiler.interpreter.stubs(:compile).with(@node)
+        @compiler.find(@name)
+    end
+end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 3146f6e7e..fe5d2be8b 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -126,3 +126,10 @@ describe Puppet::Node, " when indirecting" do
         Puppet::Indirector::Indirection.clear_cache
     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