Fix #2218 - Ruby YAML bug prevents reloading catalog in puppetd

Because of ruby bug:
http://rubyforge.org/tracker/?group_id=426&atid=1698&func=detail&aid=8886
and
http://redmine.ruby-lang.org/issues/show/1331

YAML dump of hashes using ruby objects as keys is incorrect leading
to an error when deserializing the YAML in puppetd.

The error is easy to correct by a post-process fix-up of
the generated YAML, which transforms:
&id004 !ruby/object:Puppet::Relationship ?

to the correct:
? &id004 !ruby/object:Puppet::Relationship

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

5 files changed, 36 insertions, 34 deletions

diff --git a/lib/puppet/network/formats.rb b/lib/puppet/network/formats.rb
index 85e8ce6f8..51354b03d 100644
--- a/lib/puppet/network/formats.rb
+++ b/lib/puppet/network/formats.rb
@@ -12,18 +12,31 @@ Puppet::Network::FormatHandler.create(:yaml, :mime => "text/yaml") do
     end
 
     def render(instance)
-        instance.to_yaml
+        yaml = instance.to_yaml
+
+        yaml = fixup(yaml) unless yaml.nil?
+        yaml
     end
 
     # Yaml monkey-patches Array, so this works.
     def render_multiple(instances)
-        instances.to_yaml
+        yaml = instances.to_yaml
+
+        yaml = fixup(yaml) unless yaml.nil?
+        yaml
     end
 
     # Everything's supported
     def supported?(klass)
         true
     end
+
+    # fixup invalid yaml as per:
+    # http://redmine.ruby-lang.org/issues/show/1331
+    def fixup(yaml)
+        yaml.gsub!(/((?:&id\d+\s+)?!ruby\/object:.*?)\s*\?/) { "? #{$1}" }
+        yaml
+    end
 end
 
 
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 32fbc38d6..4f60f6e56 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -417,20 +417,6 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
         super
     end
 
-    def to_yaml_properties
-        result = instance_variables
-
-        # There's a ruby bug that hits us without this:
-        # http://rubyforge.org/tracker/?group_id=426&atid=1698&func=detail&aid=8886
-        # We need our resources to show up in as values in a hash
-        # before they show up as keys, because otherwise
-        # the loading fails.
-        result.delete "@resource_table"
-        result.unshift "@resource_table"
-
-        result
-    end
-
     private
 
     def cleanup
diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
index c5b0f4928..5d6f72943 100644
--- a/lib/puppet/simple_graph.rb
+++ b/lib/puppet/simple_graph.rb
@@ -360,20 +360,6 @@ class Puppet::SimpleGraph
         end
     end
 
-    def to_yaml_properties
-        result = instance_variables
-
-        # There's a ruby bug that hits us without this:
-        # http://rubyforge.org/tracker/?group_id=426&atid=1698&func=detail&aid=8886
-        # We need our resources to show up in as values in a hash
-        # before they show up as keys, because otherwise
-        # the loading fails.
-        result.delete "@edges"
-        result.unshift "@edges"
-
-        result
-    end
-
     # Just walk the tree and pass each edge.
     def walk(source, direction)
         # Use an iterative, breadth-first traversal of the graph. One could do
diff --git a/spec/unit/network/formats.rb b/spec/unit/network/formats.rb
index 0e21fefa7..b472cbf65 100755
--- a/spec/unit/network/formats.rb
+++ b/spec/unit/network/formats.rb
@@ -28,12 +28,29 @@ describe "Puppet Network Format" do
             @yaml.render(instance).should == "foo"
         end
 
+        it "should fixup generated yaml on render" do
+            instance = mock 'instance', :to_yaml => "foo"
+
+            @yaml.expects(:fixup).with("foo").returns "bar"
+
+            @yaml.render(instance).should == "bar"
+        end
+
         it "should render multiple instances by calling 'to_yaml' on the array" do
             instances = [mock('instance')]
             instances.expects(:to_yaml).returns "foo"
             @yaml.render_multiple(instances).should == "foo"
         end
 
+        it "should fixup generated yaml on render" do
+            instances = [mock('instance')]
+            instances.stubs(:to_yaml).returns "foo"
+
+            @yaml.expects(:fixup).with("foo").returns "bar"
+
+            @yaml.render(instances).should == "bar"
+        end
+
         it "should intern by calling 'YAML.load'" do
             text = "foo"
             YAML.expects(:load).with("foo").returns "bar"
@@ -45,6 +62,10 @@ describe "Puppet Network Format" do
             YAML.expects(:load).with("foo").returns "bar"
             @yaml.intern_multiple(String, text).should == "bar"
         end
+
+        it "should fixup incorrect yaml to correct" do
+            @yaml.fixup("&id004 !ruby/object:Puppet::Relationship ?").should == "? &id004 !ruby/object:Puppet::Relationship"
+        end
     end
 
     it "should include a marshal format" do
diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/catalog.rb
index d51f8fbd7..6a5922e2e 100755
--- a/spec/unit/resource/catalog.rb
+++ b/spec/unit/resource/catalog.rb
@@ -801,10 +801,6 @@ describe Puppet::Resource::Catalog, "when compiling" do
         it "should be able to be dumped to yaml" do
             YAML.dump(@catalog).should be_instance_of(String)
         end
-
-        it "should always have its resource table first in its yaml property list" do
-            @catalog.to_yaml_properties[0].should == "@resource_table"
-        end
     end
 
     describe "when converting from yaml" do