From a75674b8ee64e9117e2f58f0bda8a6ed49e2dfe0 Mon Sep 17 00:00:00 2001 From: Michael Harp Date: Sun, 4 Oct 2026 07:01:59 -0400 Subject: [PATCH] Port Puppet::Util::Json to the json 3 API json 3.0.0 makes every option a keyword argument, raises on unknown options, and removes several deprecated APIs. Puppet::Util::Json relied on all three, so every agent run on a Ruby with json 3 installed failed with "wrong number of arguments (given 2, expected 1)" as soon as it deserialized anything (file metadata, catalogs, facts, module metadata). Changes to lib: * Util::Json.load passes parser options to JSON.parse as keywords. * Util::Json.dump replaces the removed JSON::PRETTY_STATE_PROTOTYPE with the equivalent generator options and drops the obsolete :quirks_mode option that the network formats still pass. Both methods now dup the options hash instead of mutating the caller's. * The epp and parser faces use JSON.pretty_generate instead of the removed JSON.pretty_unparse. * Pal's JsonCatalogEncoder#encode_resource goes through Util::Json.dump. It passed :pretty straight to Hash#to_json, which json 2 silently ignored and json 3 rejects. As a side effect it now honours the pretty option like #encode already did. Changes to spec, so the suite also passes with json 3: * Specs for load options and dump behaviour. * The schema matcher parses the JSON before handing it to json-schema, which as of 6.2.1 still calls JSON.parse with a positional hash. * The loaders spec fixture produced metadata.json with a duplicate "dependencies" key, which json 3 rejects by default. * Drop :create_additions from JSON.parse calls in the catalog spec; it was already the default and the option no longer exists. The Gemfile pin added in #655 is lifted to json < 4 so CI resolves json 3 and exercises this port. Everything still works with the json 2.x that ships with Ruby 3.2 through 4.0. Fixes #654 Closes #664 Co-Authored-By: Claude Fable 5.1 Signed-off-by: Michael Harp (cherry picked from commit 8530418b9785d396e11226ad1e4d0aa4b07d020c) --- Gemfile | 2 +- lib/puppet/face/epp.rb | 2 +- lib/puppet/face/parser.rb | 2 +- lib/puppet/pal/json_catalog_encoder.rb | 2 +- lib/puppet/util/json.rb | 18 +++++-- spec/lib/matchers/json.rb | 5 ++ spec/unit/pops/loaders/loaders_spec.rb | 5 +- spec/unit/puppet_pal_catalog_spec.rb | 4 +- spec/unit/resource/catalog_spec.rb | 4 +- spec/unit/util/json_spec.rb | 65 ++++++++++++++++++++++++++ 10 files changed, 95 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index 8c586e5441..6ad23ddd24 100644 --- a/Gemfile +++ b/Gemfile @@ -77,7 +77,7 @@ group(:packaging) do gem 'vanagon', *location_for(ENV['VANAGON_LOCATION'] || 'https://github.com/openvoxproject/vanagon#main') end gem 'artifactory' - gem 'json', '< 3' + gem 'json', '< 4' gem 'octokit' end diff --git a/lib/puppet/face/epp.rb b/lib/puppet/face/epp.rb index 8f7fe4ba4e..dd0eb40718 100644 --- a/lib/puppet/face/epp.rb +++ b/lib/puppet/face/epp.rb @@ -384,7 +384,7 @@ def dump_parse(source, filename, options, show_filename = true) pn = Puppet::Pops::Model::PNTransformer.transform(parse_result) case fmt when 'json' - options[:pretty] ? JSON.pretty_unparse(pn.to_data) : JSON.dump(pn.to_data) + options[:pretty] ? JSON.pretty_generate(pn.to_data) : JSON.dump(pn.to_data) else pn.format(options[:pretty] ? Puppet::Pops::PN::Indent.new(' ') : nil, output) end diff --git a/lib/puppet/face/parser.rb b/lib/puppet/face/parser.rb index 12f5999588..d574473ead 100644 --- a/lib/puppet/face/parser.rb +++ b/lib/puppet/face/parser.rb @@ -194,7 +194,7 @@ def dump_parse(source, filename, options, show_filename = true) pn = Puppet::Pops::Model::PNTransformer.transform(parse_result) case fmt when 'json' - options[:pretty] ? JSON.pretty_unparse(pn.to_data) : JSON.dump(pn.to_data) + options[:pretty] ? JSON.pretty_generate(pn.to_data) : JSON.dump(pn.to_data) else pn.format(options[:pretty] ? Puppet::Pops::PN::Indent.new(' ') : nil, output) end diff --git a/lib/puppet/pal/json_catalog_encoder.rb b/lib/puppet/pal/json_catalog_encoder.rb index cce47e2a95..3cd3efd471 100644 --- a/lib/puppet/pal/json_catalog_encoder.rb +++ b/lib/puppet/pal/json_catalog_encoder.rb @@ -58,7 +58,7 @@ def encode_resource(type, title) r = possibly_filtered_catalog.resource(type, title) return nil if r.nil? - r.to_data_hash.to_json(:pretty => pretty) + Puppet::Util::Json.dump(r.to_data_hash, :pretty => pretty) end # Applies a filter for virtual resources and returns filtered catalog diff --git a/lib/puppet/util/json.rb b/lib/puppet/util/json.rb index 43632d3008..ac0ee19444 100644 --- a/lib/puppet/util/json.rb +++ b/lib/puppet/util/json.rb @@ -18,6 +18,14 @@ def self.build(original_exception, data) require 'json' + # Generator options equivalent to the `JSON::PRETTY_STATE_PROTOTYPE` + # that json 2.x provided and json 3 removed. + PRETTY_OPTIONS = { :indent => ' ', :space => ' ', :object_nl => "\n", :array_nl => "\n" }.freeze + + # Generator options that json 3 rejects as unknown. `quirks_mode` has been + # a no-op since json 2.0, when quirks mode became the only mode. + OBSOLETE_DUMP_OPTIONS = [:quirks_mode].freeze + # Load the content from a file as JSON if # contents are in valid format. This method does not # raise error but returns `nil` when invalid file is @@ -41,16 +49,20 @@ def self.load_file(filename, options = {}) def self.load(string, options = {}) string = string.read if string.respond_to?(:read) + options = options.dup options[:symbolize_names] = true if options.delete(:symbolize_keys) - ::JSON.parse(string, options) + # json 3 only accepts parser options as keyword arguments + ::JSON.parse(string, **options) rescue JSON::ParserError => e raise Puppet::Util::Json::ParseError.build(e, string) end def self.dump(object, options = {}) # Options is a state when we're being called recursively - if !options.is_a?(JSON::State) && options.delete(:pretty) - options.merge!(::JSON::PRETTY_STATE_PROTOTYPE.to_h) + unless options.is_a?(JSON::State) + options = options.dup + OBSOLETE_DUMP_OPTIONS.each { |key| options.delete(key) } + options.merge!(PRETTY_OPTIONS) if options.delete(:pretty) end object.to_json(options) end diff --git a/spec/lib/matchers/json.rb b/spec/lib/matchers/json.rb index 3d08b939e4..d4e48ca41d 100644 --- a/spec/lib/matchers/json.rb +++ b/spec/lib/matchers/json.rb @@ -112,6 +112,11 @@ def initialize(schema) end def matches?(json) + # Parse here rather than letting json-schema do it, since json-schema + # (as of 6.2.1) still calls JSON.parse with a positional options hash, + # which json 3 rejects. + json = Puppet::Util::Json.load(json) if json.is_a?(String) + JSON::Validator.validate!(JSON_META_SCHEMA, @schema) JSON::Validator.validate!(@schema, json) end diff --git a/spec/unit/pops/loaders/loaders_spec.rb b/spec/unit/pops/loaders/loaders_spec.rb index cc174ad49c..32448c0035 100644 --- a/spec/unit/pops/loaders/loaders_spec.rb +++ b/spec/unit/pops/loaders/loaders_spec.rb @@ -678,7 +678,6 @@ class tstt { "name": "example/%1$s", "version": "0.0.2", "source": "git@github.com/example/example-%1$s.git", - "dependencies": [], "author": "Bob the Builder", "license": "Apache-2.0"%2$s } @@ -732,7 +731,7 @@ class tstt { 'y.pp' => 'type C::D::Y = Integer' } }, - 'metadata.json' => sprintf(metadata_json, 'c', '') + 'metadata.json' => sprintf(metadata_json, 'c', ', "dependencies": []') }, 'd' => { 'types' => { @@ -746,7 +745,7 @@ class tstt { }] PUPPET }, - 'metadata.json' => sprintf(metadata_json, 'd', '') + 'metadata.json' => sprintf(metadata_json, 'd', ', "dependencies": []') } } } diff --git a/spec/unit/puppet_pal_catalog_spec.rb b/spec/unit/puppet_pal_catalog_spec.rb index e897789c9f..a4f7a16aee 100644 --- a/spec/unit/puppet_pal_catalog_spec.rb +++ b/spec/unit/puppet_pal_catalog_spec.rb @@ -208,7 +208,7 @@ c.with_json_encoding() {|encoder| encoder.encode_resource('notify', 'test') } } end - expect(result).to match(/"message":"yay"/) + expect(result).to match(/"message": ?"yay"/) end it 'encodes values as rich data when needed' do @@ -218,7 +218,7 @@ c.with_json_encoding(pretty: true) {|encoder| encoder.encode_resource('notify', 'test') } } end - expect(result).to match(/"__ptype":"Regexp"/) + expect(result).to match(/"__ptype": ?"Regexp"/) end end end diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb index 7abca7e7b5..c15b9c92a5 100644 --- a/spec/unit/resource/catalog_spec.rb +++ b/spec/unit/resource/catalog_spec.rb @@ -1087,7 +1087,7 @@ class {'multi_param_class': @catalog.add_resource(two) # TODO this should really guarantee sort order - expect(JSON.parse(@catalog.to_json,:create_additions => false)['resources'].sort).to eq(["one_resource", "two_resource"].sort) + expect(JSON.parse(@catalog.to_json)['resources'].sort).to eq(["one_resource", "two_resource"].sort) end it "should convert its edges to a JSON-encoded array and store it as the 'edges' data" do @@ -1101,7 +1101,7 @@ class {'multi_param_class': expect(@catalog.edges_between(one, two )[0]).to receive(:to_data_hash).and_return("one_two_json") expect(@catalog.edges_between(two, three)[0]).to receive(:to_data_hash).and_return("two_three_json") - expect(JSON.parse(@catalog.to_json,:create_additions => false)['edges'].sort).to eq(%w{one_two_json two_three_json}.sort) + expect(JSON.parse(@catalog.to_json)['edges'].sort).to eq(%w{one_two_json two_three_json}.sort) end end diff --git a/spec/unit/util/json_spec.rb b/spec/unit/util/json_spec.rb index b630c7583b..58d1aa5332 100644 --- a/spec/unit/util/json_spec.rb +++ b/spec/unit/util/json_spec.rb @@ -62,6 +62,71 @@ } JSON end + + it 'passes options to the parser' do + expect(Puppet::Util::Json.load('{"a": 1}', symbolize_names: true)).to eq(a: 1) + end + + it 'translates symbolize_keys to symbolize_names' do + expect(Puppet::Util::Json.load('{"a": 1}', symbolize_keys: true)).to eq(a: 1) + end + + it 'does not modify the options it is given' do + options = { symbolize_keys: true } + Puppet::Util::Json.load('{"a": 1}', options) + + expect(options).to eq(symbolize_keys: true) + end + + it 'loads from an IO' do + expect(Puppet::Util::Json.load(StringIO.new('{"a": 1}'))).to eq('a' => 1) + end + end + + context "#dump" do + let(:data) { { 'a' => [1, { 'b' => nil }] } } + + it 'dumps compact JSON by default' do + expect(Puppet::Util::Json.dump(data)).to eq('{"a":[1,{"b":null}]}') + end + + it 'pretty prints when pretty is set' do + expect(Puppet::Util::Json.dump(data, pretty: true)).to eq(<<~JSON.chomp) + { + "a": [ + 1, + { + "b": null + } + ] + } + JSON + end + + it 'dumps a scalar' do + expect(Puppet::Util::Json.dump(42, pretty: true)).to eq('42') + end + + it 'ignores the obsolete quirks_mode option' do + expect(Puppet::Util::Json.dump(data, pretty: true, quirks_mode: true)).to eq(JSON.pretty_generate(data)) + end + + it 'passes generator options through' do + expect(Puppet::Util::Json.dump(data, space: ' ')).to eq('{"a": [1,{"b": null}]}') + end + + it 'accepts a JSON::State when called recursively' do + state = JSON::State.new(indent: ' ', object_nl: "\n") + + expect(Puppet::Util::Json.dump(data, state)).to eq(data.to_json(state)) + end + + it 'does not modify the options it is given' do + options = { pretty: true, quirks_mode: true } + Puppet::Util::Json.dump(data, options) + + expect(options).to eq(pretty: true, quirks_mode: true) + end end context "load_file_if_valid" do