Jira (PUP-10210) puppet fails with JSON error when a fact value equals Infinity

47 views
Skip to first unread message

Thomas Kishel (JIRA)

unread,
Dec 26, 2019, 2:05:04 PM12/26/19
to puppe...@googlegroups.com
Thomas Kishel created an issue
 
Puppet / Bug PUP-10210
puppet fails with JSON error when a fact value equals Infinity
Issue Type: Bug Bug
Affects Versions: PUP 6.10.1
Assignee: Unassigned
Created: 2019/12/26 11:04 AM
Priority: Normal Normal
Reporter: Thomas Kishel

Puppet Version: 6.10 and possibly earlier

A fact converted to / interpreted as (Ruby) Infinity generates a JSON error when executing puppet agent or puppet facts The same error does not occur when executing facter -p

Desired Behavior:

No JSON error via puppet

Actual Behavior:

 Given:

puppet config print maxwaitforcert
unlimited

Create a custom fact for maxwaitforcert:

Facter.add('maxwaitforcert') do
  setcode do
    Puppet.settings['maxwaitforcert']
  end
end

[root@pe-201921-master ~]# puppet facts
Error: 862: Infinity not allowed in JSON
Error: Try 'puppet help facts find' for usage

[root@pe-201921-master ~]# puppet agent -t
Info: Using configured environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Error: Failed to apply catalog: Could not render to json: 862: Infinity not allowed in JSON

Versus:

[root@pe-201921-master ~]# facter -p maxwaitforcert
inf

 

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Thomas Kishel (JIRA)

unread,
Dec 26, 2019, 2:47:03 PM12/26/19
to puppe...@googlegroups.com

Thomas Kishel (JIRA)

unread,
Dec 27, 2019, 12:13:03 PM12/27/19
to puppe...@googlegroups.com

Caused by:

    :maxwaitforcert => {
      :default  => "unlimited",
      :type     => :ttl,

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/defaults.rb#L1811 

With:

https://github.com/puppetlabs/puppet/blob/master/lib/puppet/settings/ttl_setting.rb

Could be addressed by:

  def sanitize_fact(fact)
    if fact.is_a? Hash then
      ret = {}
      fact.each_pair { |k,v| ret[sanitize_fact k]=sanitize_fact v }
      ret
    elsif fact.is_a? Array then
      fact.collect { |i| sanitize_fact i }
    elsif fact.is_a? Numeric
      fact.infinite? ? 'unlimited' : fact
    elsif fact.is_a? String \
      or fact.is_a? TrueClass \
      or fact.is_a? FalseClass
      fact
    else

Since Ruby 2.4, both Integers and Floats support infinite? so suggesting the above instead of modeling the following from ttl_setting.rb ...

      fact == Float::INFINITY ? 'unlimited' : fact

Jorie Tappa (JIRA)

unread,
Jan 6, 2020, 1:18:05 PM1/6/20
to puppe...@googlegroups.com
Jorie Tappa updated an issue
 
Change By: Jorie Tappa
*Puppet Version: 6.10 and possibly earlier*

A ANY fact converted to / interpreted as (Ruby) {{Infinity}} generates a JSON error when executing {{puppet agent}} or {{puppet facts}} The same error does not occur when executing {{facter -p}}

*Desired Behavior:*


No JSON error via {{puppet}}

*Actual Behavior:*

 Given:

{code}
puppet config print maxwaitforcert
unlimited
{code}

Create a custom fact for {{maxwaitforcert}}:

{code}
Facter.add('maxwaitforcert') do
  setcode do
    Puppet.settings['maxwaitforcert']
  end
end
{code}

{code}
[root@pe-201921-master ~]# puppet facts
Error: 862: Infinity not allowed in JSON
Error: Try 'puppet help facts find' for usage
{code}

{code}
[root@pe-201921-master ~]# puppet agent -t
Info: Using configured environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Retrieving locales
Info: Loading facts
Error: Failed to apply catalog: Could not render to json: 862: Infinity not allowed in JSON
{code}

Versus:

{code}
[root@pe-201921-master ~]# facter -p maxwaitforcert
inf
{code}

 

Jorie Tappa (JIRA)

unread,
Jan 6, 2020, 1:21:04 PM1/6/20
to puppe...@googlegroups.com
Jorie Tappa updated an issue
Change By: Jorie Tappa
Acceptance Criteria: Puppet needs to be able to serialize infinite data/rich data

Jorie Tappa (JIRA)

unread,
Jan 6, 2020, 1:23:03 PM1/6/20
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-10210
 
Re: puppet fails with JSON error when a fact value equals Infinity

ping Henrik Lindberg can pcore handle numeric values like this, and if so can you please help Coremunity to get this type of behavior working here as well?

Thomas Kishel (JIRA)

unread,
Jan 6, 2020, 1:25:03 PM1/6/20
to puppe...@googlegroups.com

Henrik Lindberg (JIRA)

unread,
Jan 6, 2020, 5:17:03 PM1/6/20
to puppe...@googlegroups.com

Jorie Tappa Pcore does not have the notion of Infinity and it does not exists in the Puppet Language as a concept. If a Ruby Float::INFINITY is returned to the puppet language it will consider it to be a Float data type value and it will print as Float[Infinity]. That is however not 100% correct since Infinity isn't a recognized data type.

The only way to get a constant representing Infinity would be to write a function that returns Ruby Float::INFINITY and then that the type of that using the function type(). You now have a data type that represents Infinity. The same cannot be done statically - i.e. you cannot define a parameter of a function to be of that data type.

Since there are issues with representation of Infinity in JSON (as well as in other formats), it would be "best" if this became an integral part of Pcore since it could have the special provisions for it when serializing/deserializing. Ping Thomas Hallgren. I am however concerned that we may be opening a bit of Pandora's box here.

If we do add an Infinity data type it may be best if this is a subtype of Float as we have a very precise definition of what an Integer is in terms of max/min value. Worth considering is also that PDB then needs to be able to handle this data type.

Another approach would be to define Infinite as an Object data type. Then it is a matter of handling Float::INFINITY as being instances of that type - which is not trivial.

The conservative (and safe) solution would be to not allow Ruby Float::INFINITY as a Float, and force all such values that are to be processed by Puppet to be transformed to some supported data type. (That is what I think it means with the suggested approach above - the infinite numeric value is turned into the string ”unlimited".

Thomas Hallgren (JIRA)

unread,
Jan 7, 2020, 4:13:03 AM1/7/20
to puppe...@googlegroups.com

Float::INFINITY is of type Float, both in Ruby and in most other languages. I don't see anything wrong in treating it as a Float in Pcore too. Further more, many serialization formats, including YAML, can handle both NaN and infinity. The way I see it, this is only a problem when serializing using JSON and as such, it doesn't motivate a new data type. The best solution is to improve the Pcore rich-data serialization specifically for JSON so that the values NaN, infinity, and negative infinity are serialized using:

{ "__ptype": "Float", "__pvalue": "NaN"},{ "__ptype": "Float", "__pvalue": "infinity"}, { "__ptype": "Float", "__pvalue": "-infinity"}

i.e. no new data type, just force use of strings on the Float data type.

Henrik Lindberg (JIRA)

unread,
Jan 7, 2020, 6:45:03 AM1/7/20
to puppe...@googlegroups.com

Thomas Hallgren yes, that works. We should allow a way to match infinity with a type as well - right now you cannot do that - perhaps Float["infinity"]? Which I guess would also work if you want to create an instance.

Thomas Hallgren (JIRA)

unread,
Jan 7, 2020, 7:53:04 AM1/7/20
to puppe...@googlegroups.com

I agree. Float["infinity"], Float["-infinity"], and Float["NaN"] should be made to match their corresponding float values.

Rob Braden (JIRA)

unread,
Jan 13, 2020, 12:52:04 PM1/13/20
to puppe...@googlegroups.com

Mihai Buzgau (JIRA)

unread,
Jan 15, 2020, 4:48:04 AM1/15/20
to puppe...@googlegroups.com

Mihai Buzgau (JIRA)

unread,
Jan 15, 2020, 4:51:04 AM1/15/20
to puppe...@googlegroups.com

Mihai Buzgau (JIRA)

unread,
Jan 15, 2020, 4:51:04 AM1/15/20
to puppe...@googlegroups.com

Mihai Buzgau (JIRA)

unread,
Jan 15, 2020, 5:27:04 AM1/15/20
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: PR NW - Triage 2020-01-22

Dorin Pleava (JIRA)

unread,
Jan 15, 2020, 8:42:04 AM1/15/20
to puppe...@googlegroups.com

Mihai Buzgau (JIRA)

unread,
Jan 22, 2020, 4:39:09 AM1/22/20
to puppe...@googlegroups.com
Mihai Buzgau updated an issue
Change By: Mihai Buzgau
Sprint: NW - 2020-01-22 , NW - 2020-02-05

Dorin Pleava (JIRA)

unread,
Jan 23, 2020, 8:24:04 AM1/23/20
to puppe...@googlegroups.com
Dorin Pleava commented on Bug PUP-10210
 
Re: puppet fails with JSON error when a fact value equals Infinity

Hi all, I am working on a PR on this, and I have some questions regarding it.

If I understand correctly, in a .pp file, Float["infinity"], Float["-infinity"], and Float["NaN"] should match their corresponding float values(Ruby types Float::INFINITY, Float::Infinity and Float::NaN).

My problems start when I treat a custom fact that returns 1.0/0.0 as Float::INFINITY and as string "Infinity".

After some debugging, if the fact is treated as Float::INFINITY, the JSON becomes

{ "my_custom_fact" : Infinity }

which is not a valid JSON, but MultiJson can parse such a json with :allow_nan flag set to true, but I think this will require changes everywhere the standard Json library fails. The classifier on the master node fails as it also needs to parse invalid JSON and I assume Content-Type' => 'application/json' POST requests will also fail.

If facts that return Float::Infinity are turned into string "Infinite", then in a .pp file a simple if $::maxwaitforcert > 100 comparison will fail because of incompatible types. Treating every "Infinity" string in a .pp like a Float::Infinity seems like it could also have many holes.

I also tried to treat a fact that returns Float::Infinite like Float::MAX and this seems to cause no problems but it may be inconsistent depending on the platform, and reported fact returns something like 1.7976931348623157e+308.

I may be missing something, but can't find good way to treat this type of facts. Anyone got any ideas on what to try next?

Henrik Lindberg (JIRA)

unread,
Jan 23, 2020, 8:41:04 AM1/23/20
to puppe...@googlegroups.com

It is simply impossible to encode infinity/NaN with standard JSON - there is no way around that.

The real solution is to use the Rich Data encoding in Puppet. That would produce JSON with those float values specially encoded as shown in the Thomas Hallgren's comment above (https://tickets.puppetlabs.com/browse/PUP-10210?focusedCommentId=710348&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-710348).

Unfortunately, making facts use Rich Data encoding throughout may be quite a bit of work - it needs to roundtrip with PDB for example.
This means that fixing the type system to recognize these values won't really help unless that is also done.

Without actually fixing it "the right way" (doing all of the above), the options seems to be to use a hash instead of just a float value, and make that hash have an extra entry if the value is Infinity or NaN - for example using a boolean.

{ "value" => 1.0 } # a valid 1.0
{ "value" => 1.0 "valid" => true} # a valid 1.0
{"value" => 0.0, "valid" => false} # invalid NaN or Infinity

That is naturally then specific to this fact and not a general solution.

Another approach is naturally to treat the value as Variant[String, Float] and string return values would be "Infinity", and "NaN", but then that needs to be tested for explicitly as well...

Josh Cooper (JIRA)

unread,
Jan 23, 2020, 12:23:05 PM1/23/20
to puppe...@googlegroups.com

Josh Cooper (JIRA)

unread,
Jan 23, 2020, 12:30:04 PM1/23/20
to puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10210

One way to fix this problem without needing to "boil the ocean" is to change the default maxwaitforcert setting to 2,147,483,647 (2^32 - 1) and places where we check to see if the value is "unlimited". This value will round-trip through JSON correctly and doesn't differ from "unlimited" in any meaningful way. AFAIK this is the only puppet setting whose default value is "unlimited" though there are other ttl type settings that could be set to unlimited and cause a similar problem environment_timeout and statettl.

Justin Stoller (JIRA)

unread,
Jan 27, 2020, 2:18:04 PM1/27/20
to puppe...@googlegroups.com

I don't know if this is too conservative but the fact w/in the description could be changed to:

Puppet.settings.setting(:maxwaitforcert).print(Puppet.settings[:maxwaitforcert])

Which would "properly" print the setting as "unlimited". This would require more work w/in a manifest for comparing, but is backwards compatible across different agent versions while allowing the value to be round tripped back into the settings system (don't know if that valuable).

Henrik Lindberg (JIRA)

unread,
Jan 27, 2020, 4:24:04 PM1/27/20
to puppe...@googlegroups.com

Justin Stoller I think that is a sensible and pragmatic solution as it makes the value symmetric (i.e. it roundtrips). Any other special value would have to be compared against anyway - only small benefit of using a numeric value (a supported Infinity or a defined MAX_INT) would be that it is directly comparable. The cost for proper Infinity support is however quite high. So... what you suggest makes a lot of sense.

Dorin Pleava (JIRA)

unread,
Jan 31, 2020, 6:06:04 AM1/31/20
to puppe...@googlegroups.com
Dorin Pleava commented on Bug PUP-10210

Read again the ticket, and I think both this PR and @justinstoller solutions are a workaround for puppet ttl settings, but I also think Justin's solution is more useful as you can choose the desired output of the fact.
Still, the main issue is that if a fact returns Float::INFINITY, puppet will fail. This will still happen, and the best solution is what @hlindberg proposed in PUP-478.
I'm OK with closing this PR and using Justin's solution.

Reply all
Reply to author
Forward
0 new messages