The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Feature #21427

Deprecate YAML for network data transmission

Added by Anonymous almost 3 years ago. Updated over 2 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:-
Target version:3.3.0
Affected Puppet version: Branch:https://github.com/puppetlabs/puppet/pull/1874
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com


Description

YAML serialization has ended up being slower than P/JSON and opens the system up to a number of security flaws. It is time to leave YAML behind on the network layer. We’ll continue to keep it for storing some information on disk.


Related issues

Related to Puppet - Bug #3883: Transaction reports and contained instances have no json ... Accepted 05/26/2010
Related to Puppet - Bug #19393: Unsafe YAML deserialization Closed
Related to Puppet - Bug #17053: Broken handling of facts that look like dates Needs Decision
Related to Puppet - Bug #15730: No parity between pson and yaml returned from REST API fo... Investigating 07/30/2012
Related to Puppet - Bug #15512: puppet resource_type search '*' - differences between yam... Needs More Information 07/13/2012
Related to Puppet - Bug #21337: Security fix using safe_yaml caused a drastic performance... Closed

History

#1 Updated by Anonymous almost 3 years ago

#2 Updated by Anonymous almost 3 years ago

Instead of using YAML for network communication, PSON or JSON should be used. This can be done by setting an HTTP header of “Accept: pson” to get responses in PSON format. When sending requests that have a serialized body send a “Content-Type: pson” header and serialize in PSON format. If you are using puppet as a library, then this should be done already.

#3 Updated by Anonymous almost 3 years ago

Part these changes is that the agent will no longer send data in YAML over the network. The master will still accept it, but will issue a deprecation message. A side effect of this, is that the master needs to be able to understand some things differently and only a newer master will have that updated protocol. The agent will only be able to talk to the new master (so the 3.3.0 agent will require a master >= 3.3.0), but the master will still be able to communicate with older agents, it will just issue deprecation warnings.

#4 Updated by Luke Kanies almost 3 years ago

I can’t seem to add related issues any more. Here’s one:

https://projects.puppetlabs.com/issues/3883

#5 Updated by Anonymous almost 3 years ago

Some clarification as to what will not work under older masters. We had to change out the “ignore” parameter is transmitted to the master. On webrick setups this will unfortunately result in silently not ignoring things because and older master will not see all of the ignore items. On a rack setup, the master will blow up because it doesn’t understand how to deal with arrays of parameter values.

#6 Updated by Anonymous almost 3 years ago

  • Status changed from Accepted to Merged - Pending Release
  • Branch set to https://github.com/puppetlabs/puppet/pull/1734

#7 Updated by eric sorenson over 2 years ago

The changes for this ticket cause errors submitting reports when a 3.3 agent talks to a 3.2 master. Because this change was part of a security fix, it’s not a great idea to make the compatibility mode selectable by a setting; however, there’s a legitimate concern (which Moses eloquently expressed in PP-281) that releasing such a change in a 3.3 release is a violation of semantic versioning guarantees. Talking through this in #puppet-dev and around the office, seems like there’s two real options, each with their own pros and cons. (Reverting the change because we can’t figure out how to release it doesn’t make any sense)

Release as 4.0

  • Pro: strict interpretation of semver supports this “Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API.” The major number bump will convey meaning to people that they need to be careful with upgrading.
  • Counter-Pro: Puppet’s internal network communications do not constitute public API. People who follow master-first upgrade instructions will not see this. People who do not follow upgrade instructions will not have their infrastructure stop working (as they would if catalog requests failed to work).
  • Con: If this is the only backwards-incompatible change that’s in Puppet 4, we’ll release Puppet 5 before the end of the year, which seems arbitrary and poorly-planned.
  • Counter-Con: It “seems” that way because it would actually be that way, but it’s better (in a utilitarian sense: better fulfilling the values of honesty and transparency) to take the lumps than pretend otherwise.

Release as 3.3.0

  • Pro: In Andy’s words, “this is analogous to a C program that requires glibc 2.18 in a new version where before it could work with 2.17 is required to bump the major version.”
  • Counter-Pro: that analogy is only true if the program and library are distinct to one another, rather than a single unit.
  • Con: People who directly use Puppetlabs repos for provisioning will see errors as they did when we released 3.0, except without a version-number indication of why.
  • Counter-con: Incrementing the version number will not actually solve the problem for those people; if that’s the primary reason it’s only a CYA move not a substantive change to help them.

#8 Updated by Anonymous over 2 years ago

Here is a possible solution: add a setting for YAML serialization format that can take wither “yaml” or “pson”. If it is “yaml” puppet issues a deprecation warning. In the next major version (4.0 at this point) that setting is removed and it will only send “pson”.

#9 Updated by Anonymous over 2 years ago

  • Status changed from Merged - Pending Release to Code Insufficient
  • Assignee changed from Patrick Carlisle to Anonymous

This needs to be updated to use the setting (defaulting to pson, but having yaml selectable) for 3.3.0.

#10 Updated by Anonymous over 2 years ago

  • Status changed from Code Insufficient to Merged - Pending Release
  • Branch changed from https://github.com/puppetlabs/puppet/pull/1734 to https://github.com/puppetlabs/puppet/pull/1869

New PR that adds in the ability to get a newer agent to talk to an older master. The format that the report is sent in is controlled with report_serialization_format=pson|yaml. https://github.com/puppetlabs/puppet/pull/1869

#11 Updated by Josh Cooper over 2 years ago

Merged into stable in 90c0c3846

#12 Updated by Anonymous over 2 years ago

  • Branch changed from https://github.com/puppetlabs/puppet/pull/1869 to https://github.com/puppetlabs/puppet/pull/1874

Turns out that there are some parts of the code that expect symbols and some parts that expect anything :( This showed up as a problem on ruby 1.9.3 where it couldn’t submit reports because it didn’t understand the string "pson" as a format.

PR https://github.com/puppetlabs/puppet/pull/1874 fixes this.

#14 Updated by Anonymous over 2 years ago

  • Status changed from Merged - Pending Release to Closed

Released in 3.3.0

#15 Updated by Anonymous over 2 years ago

Released in 3.3.0

Also available in: Atom PDF