Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-4275) Customize ACE YAML serialization #97

Conversation

Iristyle
Copy link
Contributor

@Iristyle Iristyle commented Apr 12, 2017

  • Ruby 2.3 has changed the behavior of YAML serialization so that
    Hash derived instances will serialize all of their instance
    variables per:

    ruby/ruby@fe0414b

    Psych used to produce an empty serialization of the ACE type in
    Ruby 2.1.9 like:

   --- !ruby/hash:Puppet::Type::Acl::Ace {}

In Ruby 2.3, it will produce something like this:

   --- !ruby/hash-with-ivars:Puppet::Type::Acl::Ace
     elements: {}
     ivars:
       :@provider:
       :@identity: Administrators
       :@hash:
       :@id: S-32-12-0
       :@mask: '2023422'
       :@rights:
         - :full
       :@perm_type: :allow
       :@child_types: :all
       :@affects: :all
       :@is_inherited: false

The issue that arises is that @Provider is a Puppet provider
instance that cannot be YAML serialized and will crash in 2 key
situations during a Puppet run:

* When the last_run_report.yaml is persisted
* When corrective changes transaction reports are written
  • The solution is to implement the encode_with method on the ACE type
    to control what gets serialized. Since ACE already has a to_hash
    method, let it do the heavy lifting to produce the bare Hash
    structure for Psych to serialize.

    Note that nil is passed as the "tag" used by Pysch, which means
    that the key / value pairs are emitted as a bare YAML hash instead
    of the custom type:

    --- !ruby/object:Puppet::Type::Acl::Ace
  • This should also more correctly fix a longstanding PE console issue
    described in PCP-542 where the master is unable to deserialize
    YAML strings like the following because the Ruby classes are not
    loaded on the master:
   !ruby/hash:Puppet::Type::Acl::Ace {}
  • Also see PUP-7381 for other YAML serialization work proceeding
    within Puppe to address Ruby 2.3 related behavioral changes.

 - Ruby 2.3 has changed the behavior of YAML serialization so that
   Hash derived instances will serialize all of their instance
   variables per:

   ruby/ruby@fe0414b

   Psych used to produce an empty serialization of the ACE type in
   Ruby 2.1.9 like:

   --- !ruby/hash:Puppet::Type::Acl::Ace {}

   In Ruby 2.3, it will produce something like this:

   --- !ruby/hash-with-ivars:Puppet::Type::Acl::Ace
     elements: {}
     ivars:
       :@Provider:
       :@identity: Administrators
       :@hash:
       :@id: S-32-12-0
       :@Mask: '2023422'
       :@rights:
         - :full
       :@perm_type: :allow
       :@child_types: :all
       :@affects: :all
       :@is_inherited: false

  The issue that arises is that @Provider is a Puppet provider
  instance that cannot be YAML serialized and will crash in 2 key
  situations during a Puppet run:

    * When the last_run_report.yaml is persisted
    * When corrective changes transaction reports are written

 - The solution is to implement the encode_with method on the ACE type
   to control what gets serialized. Since ACE already has a to_hash
   method, let it do the heavy lifting to produce the bare Hash
   structure for Psych to serialize.

   Note that nil is passed as the "tag" used by Pysch, which means
   that the key / value pairs are emitted as a bare YAML hash instead
   of the custom type:

    --- !ruby/object:Puppet::Type::Acl::Ace

 - This should also more correctly fix a longstanding PE console issue
   described in PCP-542 where the master is unable to deserialize
   YAML strings like the following because the Ruby classes are not
   loaded on the master:

   !ruby/hash:Puppet::Type::Acl::Ace {}

 - Also see PUP-7381 for other YAML serialization work proceeding
   within Puppe to address Ruby 2.3 related behavioral changes.
@joshcooper
Copy link
Contributor

@glennsarti what's the earliest branch this should be targeted at?

@Iristyle
Copy link
Contributor Author

This change will be backwards compatible with older Puppets, so the course of action depends on release plans I think. This could plausibly be a 1.1.3 or 1.2.0 that maintains the same compatibility that it currently does.

There is another ticket that needs to be tackled for Puppet 5 that will break backwards compatibility - https://tickets.puppetlabs.com/browse/MODULES-3093. It will require a major version bump to ACL 2.0 and will require a minimum Puppet version of 4.4 I believe.

@glennsarti
Copy link
Contributor

@joshcooper I'm not the best person to answer the targetting question.

@glennsarti
Copy link
Contributor

From conversation. This should target master

@glennsarti glennsarti merged commit f919c4a into puppetlabs:master Apr 14, 2017
@Iristyle Iristyle deleted the ticket/master/MODULES-4275-ruby-23-Hash-serialization-issue branch April 14, 2017 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants