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, 94 insertions, 22 deletions

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")