Fixing #2149 - Facts are passed as part of the catalog request

This removes the requirement of shared fact caching
on the servers, since the server responding to the catalog
request will receive the facts as part of the request.

The facts are serialized as a parameter to the request,
rather than each being set as a separate request parameter.

This hard-codes yaml as the serialization format for the
facts, because I couldn't get marshal to work and it's just not
as big a deal for such a small amount of data.

Signed-off-by: Luke Kanies <luke@madstop.com>

4 files changed, 56 insertions, 18 deletions

diff --git a/lib/puppet/application/puppetd.rb b/lib/puppet/application/puppetd.rb
index cacb84361..7a92db11d 100644
--- a/lib/puppet/application/puppetd.rb
+++ b/lib/puppet/application/puppetd.rb
@@ -222,7 +222,6 @@ Puppet::Application.new(:puppetd) do
         Puppet::Resource::Catalog.cache_class = :yaml
 
         Puppet::Node::Facts.terminus_class = :facter
-        Puppet::Node::Facts.cache_class = :rest
 
         # We need tomake the client either way, we just don't start it
         # if --no-client is set.
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index e6fdbee15..b74456de4 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -75,8 +75,6 @@ class Puppet::Configurer
         download_plugins()
 
         download_fact_plugins()
-
-        upload_facts()
     end
 
     # Get the remote catalog, yo.  Returns nil if no catalog can be found.
@@ -84,11 +82,16 @@ class Puppet::Configurer
         name = Facter.value("hostname")
         catalog_class = Puppet::Resource::Catalog
 
+        # This is a bit complicated.  We need the serialized and escaped facts,
+        # and we need to know which format they're encoded in.  Thus, we
+        # get a hash with both of these pieces of information.
+        fact_options = facts_for_uploading()
+
         # First try it with no cache, then with the cache.
         result = nil
         begin
             duration = thinmark do
-                result = catalog_class.find(name, :ignore_cache => true)
+                result = catalog_class.find(name, fact_options.merge(:ignore_cache => true))
             end
         rescue => detail
             puts detail.backtrace if Puppet[:trace]
@@ -98,7 +101,7 @@ class Puppet::Configurer
         unless result
             begin
                 duration = thinmark do
-                    result = catalog_class.find(name, :ignore_terminus => true)
+                    result = catalog_class.find(name, fact_options.merge(:ignore_terminus => true))
                 end
             rescue => detail
                 puts detail.backtrace if Puppet[:trace]
diff --git a/lib/puppet/configurer/fact_handler.rb b/lib/puppet/configurer/fact_handler.rb
index 8a6de5e9f..4135427c9 100644
--- a/lib/puppet/configurer/fact_handler.rb
+++ b/lib/puppet/configurer/fact_handler.rb
@@ -10,9 +10,7 @@ module Puppet::Configurer::FactHandler
         Puppet[:factsync]
     end
 
-    def upload_facts
-        # XXX down = Puppet[:downcasefacts]
-
+    def find_facts
         reload_facter()
 
         # This works because puppetd configures Facts to use 'facter' for
@@ -22,10 +20,22 @@ module Puppet::Configurer::FactHandler
             Puppet::Node::Facts.find(Puppet[:certname])
         rescue => detail
             puts detail.backtrace if Puppet[:trace]
-            Puppet.err("Could not retrieve local facts: %s" % detail)
+            raise Puppet::Error, "Could not retrieve local facts: %s" % detail
         end
     end
 
+    def facts_for_uploading
+        facts = find_facts
+        #format = facts.class.default_format
+
+        # Hard-code yaml, because I couldn't get marshal to work.
+        format = :yaml
+
+        text = facts.render(format)
+
+        return {:facts_format => format, :facts => URI.escape(text)}
+    end
+
     # Retrieve facts from the central server.
     def download_fact_plugins
         return unless download_fact_plugins?
diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
index 47635d88c..c9a216da1 100644
--- a/lib/puppet/indirector/catalog/compiler.rb
+++ b/lib/puppet/indirector/catalog/compiler.rb
@@ -12,17 +12,25 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code
 
     attr_accessor :code
 
+    def extract_facts_from_request(request)
+        return unless text_facts = request.options[:facts]
+        raise ArgumentError, "Facts but no fact format provided for %s" % request.name unless format = request.options[:facts_format]
+
+        # If the facts were encoded as yaml, then the param reconstitution system
+        # in Network::HTTP::Handler will automagically deserialize the value.
+        if text_facts.is_a?(Puppet::Node::Facts)
+            facts = text_facts
+        else
+            facts = Puppet::Node::Facts.convert_from(format, text_facts)
+        end
+        facts.save
+    end
+
     # Compile a node's catalog.
     def find(request)
-        unless node = request.options[:use_node]
-            # If the request is authenticated, then the 'node' info will
-            # be available; if not, then we use the passed-in key.  We rely
-            # on our authorization system to determine whether this is allowed.
-            name = request.node || request.key
-            unless node = find_node(name)
-                raise ArgumentError, "Could not find node '%s'; cannot compile" % name
-            end
-        end
+        extract_facts_from_request(request)
+
+        node = node_from_request(request)
 
         if catalog = compile(node)
             return catalog
@@ -102,6 +110,24 @@ class Puppet::Resource::Catalog::Compiler < Puppet::Indirector::Code
         node
     end
 
+    # Extract the node from the request, or use the request
+    # to find the node.
+    def node_from_request(request)
+        if node = request.options[:use_node]
+            return node
+        end
+
+        # If the request is authenticated, then the 'node' info will
+        # be available; if not, then we use the passed-in key.  We rely
+        # on our authorization system to determine whether this is allowed.
+        name = request.node || request.key
+        if node = find_node(name)
+            return node
+        end
+
+        raise ArgumentError, "Could not find node '%s'; cannot compile" % name
+    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