Control: tags -1 - moreinfo + patch pending Control: severity -1 serious On 13:43 Mon 11 Mar , Kienan Stewart wrote: > Hi Apollon, > > On Sat, Mar 09, 2019 at 11:24:27PM +0200, Apollon Oikonomopoulos wrote: > > Control: tags -1 - unreproducible > > > > Hi, > > > > > > Thanks for the patch, it should do the trick. However, before applying > > it I want to be 100% sure that we know what's happening and why. > > > > So, I managed to reproduce this by simply installing ruby-multi-json, > > which allows Puppet to use different json backends. Can you verify that > > ruby-multi-json is installed, presumably on the master? > > > > ruby-multi-json (1.12.1-1) is installed. > > Checking with 'apt-cache policy rdepends ruby-multi-json', it's like because > we use r10k
OK, thanks for confirming this and thanks for the detailed report, it's very helpful! I'm bumping this bug to RC, as I don't think Puppet report storage should break merely by installing an unrelated Ruby package. Additionally this is a regression from Puppet 4.x. So, here's what's happening: - Puppet 5 switched from a custom wire format (PSON) to JSON for transmitting facts and reports. - There is no issue when using the ruby-json JSON library to parse reports. - Some JSON libraries (Oj and JrJackson) de-serialize floats with many decimal digits - such as the timing metrics found in a Puppet agent report - as BigDecimal. - When BigDecimal's are serialized again to JSON, they are serialized as Strings (and not floats), causing PuppetDB's schema validation to fail. - This path is only triggered when ruby-oj and ruby-multi-json are installed, enabling Puppet to use Oj via multi-json. (JrJackson is Jruby-only, and there are provisions upstream handling BigDecimal conversion in this case). Your patch fixes the issue, but it does so right before the report is transmitted to PuppetDB. I think it's best to instruct Oj to never deserialize floats as BigDecimals and avoid having to do any conversions in the first place. Additionally, this will guard all other report processors (e.g. store) which might want to handle metrics against similar issues. Can you test the attached patch and confirm that it works? Regards, Apollon
From: Apollon Oikonomopoulos <apoi...@debian.org> Date: Sun, 10 Mar 2019 01:22:29 +0200 Subject: Avoid BigDecimals when loading JSON using Oj This is already done for JrJackson and needs to be done for Oj as well to avoid sending malformed reports to PuppetDB. Bug-Debian: https://bugs.debian.org/923976 --- lib/puppet/util/json.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/puppet/util/json.rb b/lib/puppet/util/json.rb index 6baea59..3475990 100644 --- a/lib/puppet/util/json.rb +++ b/lib/puppet/util/json.rb @@ -32,13 +32,15 @@ module Puppet::Util def self.load(string, options = {}) if defined? MultiJson begin - # This ensures that JrJackson will parse very large or very small + # This ensures that JrJackson and Oj will parse very large or very small # numbers as floats rather than BigDecimals, which are serialized as # strings by the built-in JSON gem and therefore can cause schema errors, # for example, when we are rendering reports to JSON using `to_pson` in # PuppetDB. if MultiJson.adapter.name == "MultiJson::Adapters::JrJackson" options[:use_bigdecimal] = false + elsif MultiJson.adapter.name == "MultiJson::Adapters::Oj" + options[:bigdecimal_load] = :float end MultiJson.load(string, options)
