+1

On May 1, 2009, at 5:08 PM, Brice Figureau wrote:

>
> 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 <[email protected]>
> ---
> lib/puppet/network/formats.rb  |   17 +++++++++++++++--
> lib/puppet/resource/catalog.rb |   14 --------------
> lib/puppet/simple_graph.rb     |   14 --------------
> spec/unit/network/formats.rb   |   21 +++++++++++++++++++++
> spec/unit/resource/catalog.rb  |    4 ----
> 5 files changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/lib/puppet/network/formats.rb b/lib/puppet/network/ 
> formats.rb
> index 85e8ce6..51354b0 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 32fbc38..4f60f6e 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 c5b0f49..5d6f729 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 0e21fef..b472cbf 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 d51f8fb..6a5922e 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
> -- 
> 1.6.0.2
>
>
> >


-- 
Sometimes I think we're alone. Sometimes I think we're not. In
either case, the thought is staggering. --R. Buckminster Fuller
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to