Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-7381

Consistently use encode_with for YAML serialization

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: PUP 5.0.0
    • Component/s: None
    • Template:
    • Acceptance Criteria:
      Hide
      • Puppet consistently uses `encode_with` (as part of PsychSupport) instead of relying on the deprecated to_yaml_properties method.
      • Puppet's `to_yaml_properties` and `to_data_hash` methods serialize the same information in the same way, and that they continue to read data generated by older puppet versions.
      Show
      Puppet consistently uses `encode_with` (as part of PsychSupport) instead of relying on the deprecated to_yaml_properties method. Puppet's `to_yaml_properties` and `to_data_hash` methods serialize the same information in the same way, and that they continue to read data generated by older puppet versions.
    • Team:
      Agent
    • Story Points:
      3
    • Sprint:
      PDE 2017-05-31, Agent 2017-06-14
    • Release Notes:
      Bug Fix
    • Release Notes Summary:
      YAML produced from Puppet could sometimes not be read by the consumer due to different Ruby versions in the producer and consumer. This fix ensures that the YAML format is consistent regardless of Ruby version.
    • QA Risk Assessment:
      No Action
    • QA Risk Assessment Reason:
      covered by our existing automation

      Description

      Puppet serializes objects to YAML using both to_yaml_properties:

      lib/puppet/graph/simple_graph.rb
      lib/puppet/parser/yaml_trimmer.rb
      lib/puppet/resource/status.rb
      lib/puppet/resource.rb
      lib/puppet/transaction/event.rb
      lib/puppet/transaction/report.rb
      

      and encode_with (provided by PsychSupport):

      lib/puppet/indirector/request.rb
      lib/puppet/node/facts.rb
      lib/puppet/node.rb
      lib/puppet/resource/status.rb
      lib/puppet/util/log.rb
      

      This is bad for a few reasons:

      1. Relying on YAML#to_yaml_properties has been deprecated since ruby 1.9.3
      2. Note that Puppet::Resource::Status defines both to_yaml_properties and uses encode_with. As a result the serialization behavior depends on the ruby version and YAML engines (Syck vs Psych) and the 2 serialization methods could serialize different instance variables.
      3. Ruby 2.1 changed how Array subclasses are serialized, and Ruby 2.3 does the same for
      Hash subclasses. Note the ivars and the hash-with-ivars tag:

      --- !ruby/array:MyArrayObject
      internal:
      - 1
      ivars:
        :@foo: 2
      

      --- !ruby/hash-with-ivars:MyHashObject
      elements:
        1: 2
      ivars:
        :@bar: 3
      

      This means YAML data written by an agent running ruby 2.3 may not be readable by puppetserver using JRuby 1.7 (which is compatible with MRI ruby 1.9.3).

      Puppet should consistently use PsychSupport and its encode_with methods. Similarly, we should be using init_with and leveraging the from_data_hash methods to deserialize instances. This ensures that objects explicitly state which data to serialize, and ensures that object initializers and setters are called during deserialization, instead of using bypassing that with instance_variable_set.

      See discussion in https://github.com/puppetlabs/puppet/pull/4214#issuecomment-138691121

      /cc Henrik Lindberg, Geoff Nichols, Ethan Brown

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              thomas.hallgren Thomas Hallgren
              Reporter:
              josh Josh Cooper
              QA Contact:
              Eric Delaney Eric Delaney
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Zendesk Support