Skip to content

Commit

Permalink
Rewrite loadjson to use the modern function API
Browse files Browse the repository at this point in the history
This also resolves a bug where JSON.parse returned a StringIO. To
properly catch this, the tests are rewritten to avoid mocking where
possible. Exceptions are with external URLs and where a failure is
expected.
  • Loading branch information
ekohl committed Apr 5, 2024
1 parent d871c4d commit 05139bb
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 184 deletions.
13 changes: 13 additions & 0 deletions lib/puppet/functions/loadjson.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

# @summary DEPRECATED. Use the namespaced function [`stdlib::loadjson`](#stdlibloadjson) instead.
Puppet::Functions.create_function(:loadjson, Puppet::Functions::InternalFunction) do
dispatch :deprecation_gen do
scope_param
repeated_param 'Any', :args
end
def deprecation_gen(scope, *args)
call_function('deprecation', 'loadjson', 'This function is deprecated, please use stdlib::loadjson instead.', false)
scope.call_function('stdlib::loadjson', args)
end
end
66 changes: 66 additions & 0 deletions lib/puppet/functions/stdlib/loadjson.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true

# @summary
# Load a JSON file containing an array, string, or hash, and return the data
# in the corresponding native data type.
#
# @example Example Usage:
# $myhash = loadjson('/etc/puppet/data/myhash.json')
# $myhash = loadjson('https://example.local/my_hash.json')
# $myhash = loadjson('https://username:password@example.local/my_hash.json')
# $myhash = loadjson('no-file.json', {'default' => 'value'})
Puppet::Functions.create_function(:'stdlib::loadjson') do
# @param path
# A file path or a URL.
# @param default
# The default value to be returned if the file was not found or could not
# be parsed.
#
# @return
# The data stored in the JSON file, the type depending on the type of data
# that was stored.
dispatch :loadjson do
param 'String[1]', :path
optional_param 'Data', :default
return_type 'Data'
end

def loadjson(path, default = nil)
if path.start_with?('http://', 'https://')
require 'uri'
require 'open-uri'
uri = URI.parse(path)
options = {}
if uri.user
options[:http_basic_authentication] = [uri.user, uri.password]
uri.user = nil
end

begin
content = uri.open(**options) { |f| load_json_source(f) }
rescue OpenURI::HTTPError => e
Puppet.warn("Can't load '#{url}' HTTP Error Code: '#{e.io.status[0]}'")
return default
end
elsif File.exist?(path)
content = File.open(path) { |f| load_json_source(f) }
else
Puppet.warn("Can't load '#{path}' File does not exist!")
return default
end

content || default
rescue StandardError => e
raise e unless default

default
end

def load_json_source(source)
if Puppet::Util::Package.versioncmp(Puppet.version, '8.0.0').negative?
PSON.load(source)
else
JSON.load(source)

Check failure on line 63 in lib/puppet/functions/stdlib/loadjson.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Security/JSONLoad: Prefer `JSON.parse` over `JSON.load`. (https://ruby-doc.org/stdlib-2.7.0/libdoc/json/rdoc/JSON.html#method-i-load)

Check failure on line 63 in lib/puppet/functions/stdlib/loadjson.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Security/JSONLoad: Prefer `JSON.parse` over `JSON.load`. (https://ruby-doc.org/stdlib-2.7.0/libdoc/json/rdoc/JSON.html#method-i-load)
end
end
end
75 changes: 0 additions & 75 deletions lib/puppet/parser/functions/loadjson.rb

This file was deleted.

160 changes: 51 additions & 109 deletions spec/functions/loadjson_spec.rb
Original file line number Diff line number Diff line change
@@ -1,178 +1,120 @@
# frozen_string_literal: true

require 'spec_helper'
require 'open-uri'
require 'stringio'

describe 'loadjson' do
describe 'stdlib::loadjson' do
it { is_expected.not_to be_nil }
it { is_expected.to run.with_params.and_raise_error(ArgumentError, %r{wrong number of arguments}i) }
it { is_expected.to run.with_params.and_raise_error(ArgumentError, "'stdlib::loadjson' expects between 1 and 2 arguments, got none") }

describe 'when calling with valid arguments' do
before :each do
# In Puppet 7, there are two prior calls to File.read prior to the responses we want to mock
allow(File).to receive(:read).with(anything, anything).and_call_original
allow(File).to receive(:read).with(%r{/(stdlib|test)/metadata.json}, encoding: 'utf-8').and_return('{"name": "puppetlabs-stdlib"}')
allow(File).to receive(:read).with(%r{/(stdlib|test)/metadata.json}).and_return('{"name": "puppetlabs-stdlib"}')
# Additional modules used by litmus which are identified while running these dues to being in fixtures
allow(File).to receive(:read).with(%r{/(provision|puppet_agent|facts)/metadata.json}, encoding: 'utf-8')
end

context 'when a non-existing file is specified' do
let(:filename) do
if Puppet::Util::Platform.windows?
'C:/tmp/doesnotexist'
else
'/tmp/doesnotexist'
end
file = Tempfile.create
file.close
File.unlink(file.path)
file.path
end

before(:each) do
allow(File).to receive(:exist?).and_call_original
allow(File).to receive(:exist?).with(filename).and_return(false).once
if Puppet::PUPPETVERSION[0].to_i < 8
allow(PSON).to receive(:load).never # rubocop:disable RSpec/ReceiveNever Switching to not_to receive breaks testing in this case
else
allow(JSON).to receive(:parse).never # rubocop:disable RSpec/ReceiveNever
end
end

it { is_expected.to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') }
it { is_expected.to run.with_params(filename, 'đẽƒằưļŧ' => '٧ẵłựέ').and_return('đẽƒằưļŧ' => '٧ẵłựέ') }
it { is_expected.to run.with_params(filename, 'デフォルト' => '値').and_return('デフォルト' => '値') }
it { is_expected.to run.with_params(filename, {'default' => 'value'}).and_return({'default' => 'value'}) }

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 28 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)
it { is_expected.to run.with_params(filename, {'đẽƒằưļŧ' => '٧ẵłựέ'}).and_return({'đẽƒằưļŧ' => '٧ẵłựέ'}) }

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 29 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside } missing. (https://rubystyle.guide#spaces-braces)
it { is_expected.to run.with_params(filename, {'デフォルト' => '値'}).and_return({'デフォルト' => '値'}) }

Check failure on line 30 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 7.24, Ruby Ver: 2.7)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)

Check failure on line 30 in spec/functions/loadjson_spec.rb

View workflow job for this annotation

GitHub Actions / Spec / Spec tests (Puppet: ~> 8.0, Ruby Ver: 3.2)

Layout/SpaceInsideHashLiteralBraces: Space inside { missing. (https://rubystyle.guide#spaces-braces)
end

context 'when an existing file is specified' do
let(:filename) do
if Puppet::Util::Platform.windows?
'C:/tmp/doesexist'
else
'/tmp/doesexist'
end
end
let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } }
let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' }

before(:each) do
allow(File).to receive(:exist?).and_call_original
allow(File).to receive(:exist?).with(filename).and_return(true).once
allow(File).to receive(:read).with(filename).and_return(json).once
allow(File).to receive(:read).with(filename).and_return(json).once
if Puppet::PUPPETVERSION[0].to_i < 8
allow(PSON).to receive(:load).with(json).and_return(data).once
else
allow(JSON).to receive(:parse).with(json).and_return(data).once
it do
Tempfile.new do |file|
file.write(json)
file.flush

is_expected.to run.with_params(file.path).and_return(data)
end
end

it { is_expected.to run.with_params(filename).and_return(data) }
end

context 'when the file could not be parsed' do
let(:filename) do
if Puppet::Util::Platform.windows?
'C:/tmp/doesexist'
else
'/tmp/doesexist'
end
end
let(:json) { '{"key":"value"}' }

before(:each) do
allow(File).to receive(:exist?).and_call_original
allow(File).to receive(:exist?).with(filename).and_return(true).once
allow(File).to receive(:read).with(filename).and_return(json).once
if Puppet::PUPPETVERSION[0].to_i < 8
allow(PSON).to receive(:load).with(json).once.and_raise StandardError, 'Something terrible have happened!'
else
allow(JSON).to receive(:parse).with(json).once.and_raise StandardError, 'Something terrible have happened!'
it do
Tempfile.new do |file|
file.write(json)
file.flush

is_expected.to run.with_params(filename, {'default' => 'value'}).and_return({'default' => 'value'})
end
end

it { is_expected.to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') }
end

context 'when an existing URL is specified' do
context 'when an existing URL with username and password is specified' do
let(:filename) do
'https://example.local/myhash.json'
'https://user1:pass1@example.local/myhash.json'
end
let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } }
let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' }

it {
expect(OpenURI).to receive(:open_uri).with(filename, {}).and_return(json)
if Puppet::PUPPETVERSION[0].to_i < 8
expect(PSON).to receive(:load).with(json).and_return(data).once
else
expect(JSON).to receive(:parse).with(json).and_return(data).once
end
expect(subject).to run.with_params(filename).and_return(data)
}
end
it do
uri = URI.parse(filename)
allow(URI).to receive(:parse).and_call_original
expect(URI).to receive(:parse).with(filename).and_return(uri)
expect(uri).to receive(:open).with(http_basic_authentication: ['user1', 'pass1']).and_yield(StringIO.new('{"key":"value"}'))

context 'when an existing URL (with username and password) is specified' do
let(:filename) do
'https://user1:pass1@example.local/myhash.json'
is_expected.to run.with_params(filename).and_return({'key' => 'value'})
expect(uri.user).to be_nil
end
let(:url_no_auth) { 'https://example.local/myhash.json' }
let(:basic_auth) { { http_basic_authentication: ['user1', 'pass1'] } }
let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } }
let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' }

it {
expect(OpenURI).to receive(:open_uri).with(url_no_auth, basic_auth).and_return(json)
if Puppet::PUPPETVERSION[0].to_i < 8
expect(PSON).to receive(:load).with(json).and_return(data).once
else
expect(JSON).to receive(:parse).with(json).and_return(data).once
end
expect(subject).to run.with_params(filename).and_return(data)
}
end

context 'when an existing URL (with username) is specified' do
context 'when an existing URL is specified' do
let(:filename) do
'https://user1@example.local/myhash.json'
'https://example.com/myhash.json'
end
let(:url_no_auth) { 'https://example.local/myhash.json' }
let(:basic_auth) { { http_basic_authentication: ['user1', ''] } }
let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } }
let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' }
let(:json) { '{"key":"value", "ķęŷ":"νậŀųề", "キー":"値" }' }

it {
expect(OpenURI).to receive(:open_uri).with(url_no_auth, basic_auth).and_return(json)
if Puppet::PUPPETVERSION[0].to_i < 8
expect(PSON).to receive(:load).with(json).and_return(data).once
else
expect(JSON).to receive(:parse).with(json).and_return(data).once
end
uri = URI.parse(filename)
allow(URI).to receive(:parse).and_call_original
expect(URI).to receive(:parse).with(filename).and_return(uri)
expect(uri).to receive(:open).with(no_args).and_yield(StringIO.new(json))
expect(subject).to run.with_params(filename).and_return(data)
}
end

context 'when the URL output could not be parsed, with default specified' do
let(:filename) do
'https://example.local/myhash.json'
'https://example.com/myhash.json'
end
let(:json) { ',;{"key":"value"}' }

it {
expect(OpenURI).to receive(:open_uri).with(filename, {}).and_return(json)
if Puppet::PUPPETVERSION[0].to_i < 8
expect(PSON).to receive(:load).with(json).once.and_raise StandardError, 'Something terrible have happened!'
else
expect(JSON).to receive(:parse).with(json).once.and_raise StandardError, 'Something terrible have happened!'
end
expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value')
uri = URI.parse(filename)
allow(URI).to receive(:parse).and_call_original
expect(URI).to receive(:parse).with(filename).and_return(uri)
expect(uri).to receive(:open).with(no_args).and_yield(StringIO.new(json))
expect(subject).to run.with_params(filename, {'default' => 'value'}).and_return({'default' => 'value'})
}
end

context 'when the URL does not exist, with default specified' do
let(:filename) do
'https://example.local/myhash.json'
'https://example.com/myhash.json'
end

it {
expect(OpenURI).to receive(:open_uri).with(filename, {}).and_raise OpenURI::HTTPError, '404 File not Found'
expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value')
uri = URI.parse(filename)
allow(URI).to receive(:parse).and_call_original
expect(URI).to receive(:parse).with(filename).and_return(uri)
expect(uri).to receive(:open).with(no_args).and_raise(OpenURI::HTTPError, '404 File not Found')
expect(subject).to run.with_params(filename, {'default' => 'value'}).and_return({'default' => 'value'})
}
end
end
Expand Down

0 comments on commit 05139bb

Please sign in to comment.