8 files changed, 150 insertions, 40 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
diff --git a/spec/unit/application/puppetd.rb b/spec/unit/application/puppetd.rb
index d93a52761..dcd86185c 100755
--- a/spec/unit/application/puppetd.rb
+++ b/spec/unit/application/puppetd.rb
@@ -20,7 +20,6 @@ describe "puppetd" do
         Puppet::Node.stubs(:terminus_class=)
         Puppet::Node.stubs(:cache_class=)
         Puppet::Node::Facts.stubs(:terminus_class=)
-        Puppet::Node::Facts.stubs(:cache_class=)
     end
 
     it "should ask Puppet::Application to parse Puppet configuration file" do
@@ -164,7 +163,6 @@ describe "puppetd" do
             Puppet::Resource::Catalog.stubs(:terminus_class=)
             Puppet::Resource::Catalog.stubs(:cache_class=)
             Puppet::Node::Facts.stubs(:terminus_class=)
-            Puppet::Node::Facts.stubs(:cache_class=)
             @host = stub_everything 'host'
             Puppet::SSL::Host.stubs(:new).returns(@host)
             Puppet.stubs(:settraps)
@@ -297,12 +295,6 @@ describe "puppetd" do
             @puppetd.run_setup
         end
 
-        it "should tell the facts cache facts through REST" do
-            Puppet::Node::Facts.expects(:cache_class=).with(:rest)
-
-            @puppetd.run_setup
-        end
-
         it "should create an agent" do
             Puppet::Agent.stubs(:new).with(Puppet::Configurer)
 
diff --git a/spec/unit/configurer.rb b/spec/unit/configurer.rb
index 067283a03..712cbf715 100755
--- a/spec/unit/configurer.rb
+++ b/spec/unit/configurer.rb
@@ -25,6 +25,7 @@ describe Puppet::Configurer, "when executing a catalog run" do
     before do
         Puppet.settings.stubs(:use).returns(true)
         @agent = Puppet::Configurer.new
+        @agent.stubs(:facts_for_uploading).returns({})
     end
 
     it "should prepare for the run" do
@@ -70,6 +71,7 @@ describe Puppet::Configurer, "when retrieving a catalog" do
     before do
         Puppet.settings.stubs(:use).returns(true)
         @agent = Puppet::Configurer.new
+        @agent.stubs(:facts_for_uploading).returns({})
 
         @catalog = Puppet::Resource::Catalog.new
 
@@ -90,6 +92,13 @@ describe Puppet::Configurer, "when retrieving a catalog" do
         @agent.retrieve_catalog
     end
 
+    it "should pass the prepared facts and the facts format as arguments when retrieving the catalog" do
+        @agent.expects(:facts_for_uploading).returns(:facts => "myfacts", :facts_format => :foo)
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:facts] == "myfacts" and options[:facts_format] == :foo }.returns @catalog
+
+        @agent.retrieve_catalog
+    end
+
     it "should default to returning a catalog retrieved directly from the server, skipping the cache" do
         Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:ignore_cache] == true }.returns @catalog
 
@@ -177,7 +186,6 @@ describe Puppet::Configurer, "when preparing for a run" do
         Puppet.settings.stubs(:use).returns(true)
         @agent = Puppet::Configurer.new
         @agent.stubs(:dostorage)
-        @agent.stubs(:upload_facts)
         @facts = {"one" => "two", "three" => "four"}
     end
 
@@ -200,10 +208,4 @@ describe Puppet::Configurer, "when preparing for a run" do
 
         @agent.prepare
     end
-
-    it "should upload facts to use for catalog retrieval" do
-        @agent.stubs(:dostorage)
-        @agent.expects(:upload_facts)
-        @agent.prepare
-    end
 end
diff --git a/spec/unit/configurer/fact_handler.rb b/spec/unit/configurer/fact_handler.rb
index 43f6e4f60..504eaaf04 100755
--- a/spec/unit/configurer/fact_handler.rb
+++ b/spec/unit/configurer/fact_handler.rb
@@ -52,29 +52,59 @@ describe Puppet::Configurer::FactHandler do
         @facthandler.download_fact_plugins
     end
 
-    it "should have a method for uploading facts" do
-        @facthandler.should respond_to(:upload_facts)
+    it "should have a method for retrieving facts" do
+        @facthandler.should respond_to(:find_facts)
     end
 
-    it "should reload Facter and find local facts when asked to upload facts" do
+    it "should use the Facts class with the :certname to find the facts" do
+        Puppet.settings.expects(:value).with(:certname).returns "foo"
+        Puppet::Node::Facts.expects(:find).with("foo").returns "myfacts"
+        @facthandler.stubs(:reload_facter)
+        @facthandler.find_facts.should == "myfacts"
+    end
+
+    it "should reload Facter and find local facts when asked to find facts" do
         @facthandler.expects(:reload_facter)
 
         Puppet.settings.expects(:value).with(:certname).returns "myhost"
         Puppet::Node::Facts.expects(:find).with("myhost")
 
-        @facthandler.upload_facts
+        @facthandler.find_facts
     end
 
-    it "should not fail if uploading facts fails" do
+    it "should fail if finding facts fails" do
         @facthandler.stubs(:reload_facter)
 
         Puppet.settings.stubs(:value).with(:trace).returns false
         Puppet.settings.stubs(:value).with(:certname).returns "myhost"
         Puppet::Node::Facts.expects(:find).raises RuntimeError
 
-        Puppet.expects(:err)
+        lambda { @facthandler.find_facts }.should raise_error(Puppet::Error)
+    end
+
+    it "should have a method to prepare the facts for uploading" do
+        @facthandler.should respond_to(:facts_for_uploading)
+    end
+
+    # I couldn't get marshal to work for this, only yaml, so we hard-code yaml.
+    it "should serialize and URI escape the fact values for uploading" do
+        facts = stub 'facts'
+        facts.expects(:render).returns "my text"
+        text = URI.escape("my text")
+
+        @facthandler.expects(:find_facts).returns facts
+
+        @facthandler.facts_for_uploading.should == {:facts_format => :yaml, :facts => text}
+    end
+
+    it "should hard-code yaml as the serialization" do
+        facts = stub 'facts'
+        facts.expects(:render).with(:yaml).returns "my text"
+        text = URI.escape("my text")
+
+        @facthandler.expects(:find_facts).returns facts
 
-        lambda { @facthandler.upload_facts }.should_not raise_error
+        @facthandler.facts_for_uploading
     end
 
     describe "when reloading Facter" do
diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb
index 5518169b1..9dac76f0e 100755
--- a/spec/unit/indirector/catalog/compiler.rb
+++ b/spec/unit/indirector/catalog/compiler.rb
@@ -110,6 +110,12 @@ describe Puppet::Resource::Catalog::Compiler do
             @compiler.find(@request)
         end
 
+        it "should extract and save any facts from the request" do
+            @compiler.expects(:extract_facts_from_request).with(@request)
+            @compiler.interpreter.stubs(:compile)
+            @compiler.find(@request)
+        end
+
         it "should return the results of compiling as the catalog" do
             Puppet::Node.stubs(:find).returns(@node)
             config = mock 'config'
@@ -130,6 +136,48 @@ describe Puppet::Resource::Catalog::Compiler do
         end
     end
 
+    describe "when extracting facts from the request" do
+        before do
+            @compiler = Puppet::Resource::Catalog::Compiler.new
+            @request = stub 'request', :options => {}
+
+            @facts = stub 'facts', :save => nil
+        end
+
+        it "should do nothing if no facts are provided" do
+            Puppet::Node::Facts.expects(:convert_from).never
+            @request.options[:facts] = nil
+
+            @compiler.extract_facts_from_request(@request)
+        end
+
+        it "should use the Facts class to deserialize the provided facts" do
+            @request.options[:facts_format] = "foo"
+            @request.options[:facts] = "bar"
+            Puppet::Node::Facts.expects(:convert_from).returns @facts
+
+            @compiler.extract_facts_from_request(@request)
+        end
+
+        it "should use the provided fact format" do
+            @request.options[:facts_format] = "foo"
+            @request.options[:facts] = "bar"
+            Puppet::Node::Facts.expects(:convert_from).with { |format, text| format == "foo" }.returns @facts
+
+            @compiler.extract_facts_from_request(@request)
+        end
+
+        it "should convert the facts into a fact instance and save it" do
+            @request.options[:facts_format] = "foo"
+            @request.options[:facts] = "bar"
+            Puppet::Node::Facts.expects(:convert_from).returns @facts
+
+            @facts.expects(:save)
+
+            @compiler.extract_facts_from_request(@request)
+        end
+    end
+
     describe "when finding nodes" do
         before do
             Facter.stubs(:value).returns("whatever")