From c118a50f909f9944c8c54032b72bbb84862de0b2 Mon Sep 17 00:00:00 2001 From: Jesse Hathaway Date: Tue, 30 May 2023 16:46:52 -0500 Subject: [PATCH] re-add support for loading aliases in yaml files Ruby 3.1 includes Psych 4.0 which no longer loads aliases by default. Since aliases are part of the yaml spec, it seems sensible to re-add support. We need to read the file first to be compatible with ruby2.7 which does not support the aliases option on YAML.load_file. Use temp files in rspec tests to avoid needing to mock read. --- lib/puppet/parser/functions/loadyaml.rb | 7 ++- spec/functions/loadyaml_spec.rb | 79 +++++++++++++++++++++---- 2 files changed, 73 insertions(+), 13 deletions(-) diff --git a/lib/puppet/parser/functions/loadyaml.rb b/lib/puppet/parser/functions/loadyaml.rb index bb2a7b00a..22bceb72b 100644 --- a/lib/puppet/parser/functions/loadyaml.rb +++ b/lib/puppet/parser/functions/loadyaml.rb @@ -49,9 +49,12 @@ module Puppet::Parser::Functions warning("Can't load '#{url}' HTTP Error Code: '#{res.status[0]}'") args[1] end - YAML.safe_load(contents) || args[1] + YAML.safe_load(contents, aliases: true) || args[1] elsif File.exist?(args[0]) - YAML.load_file(args[0]) || args[1] + # Read the file first rather than calling YAML.load_file as ruby2.7 + # doesn't support the aliases option on YAML.load_file + contents = File.read(args[0]) + YAML.safe_load(contents, aliases: true) || args[1] else warning("Can't load '#{args[0]}' File does not exist!") args[1] diff --git a/spec/functions/loadyaml_spec.rb b/spec/functions/loadyaml_spec.rb index 99e87d8a9..0eff7100e 100644 --- a/spec/functions/loadyaml_spec.rb +++ b/spec/functions/loadyaml_spec.rb @@ -12,30 +12,48 @@ it "'default' => 'value'" do allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(false).once - expect(YAML).not_to receive(:load_file) + expect(YAML).not_to receive(:safe_load) expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') end end context 'when an existing file is specified' do - let(:filename) { '/tmp/doesexist' } + let(:tempfile) { Tempfile.new } + let(:filename) { tempfile.path } let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } } + let(:yaml) do + <<~YAML + key: 'value' + ķęŷ: 'νậŀųề' + キー: '値' + YAML + end it "returns 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値'" do + tempfile.write(yaml) + tempfile.rewind allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(true).once - expect(YAML).to receive(:load_file).with(filename).and_return(data).once + expect(YAML).to receive(:safe_load).and_call_original expect(subject).to run.with_params(filename).and_return(data) end end - context 'when the file could not be parsed' do - let(:filename) { '/tmp/doesexist' } + context 'when the file could not be parsed, with default specified' do + let(:tempfile) { Tempfile.new } + let(:filename) { tempfile.path } + let(:yaml) do + <<~YAML + ,,,, + YAML + end - it 'filename /tmp/doesexist' do + it 'is expected to return the default value' do + tempfile.write(yaml) + tempfile.rewind allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(true).once - allow(YAML).to receive(:load_file).with(filename).once.and_raise(StandardError, 'Something terrible have happened!') + allow(YAML).to receive(:safe_load).with(yaml, aliases: true).once.and_raise(StandardError, 'Something terrible have happened!') expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') end end @@ -48,7 +66,7 @@ it { expect(OpenURI).to receive(:open_uri).with(filename, basic_auth).and_return(yaml) - expect(YAML).to receive(:safe_load).with(yaml).and_return(data).once + expect(YAML).to receive(:safe_load).with(yaml, aliases: true).and_return(data).once expect(subject).to run.with_params(filename).and_return(data) } end @@ -62,7 +80,7 @@ it { expect(OpenURI).to receive(:open_uri).with(url_no_auth, basic_auth).and_return(yaml) - expect(YAML).to receive(:safe_load).with(yaml).and_return(data).once + expect(YAML).to receive(:safe_load).with(yaml, aliases: true).and_return(data).once expect(subject).to run.with_params(filename).and_return(data) } end @@ -76,7 +94,7 @@ it { expect(OpenURI).to receive(:open_uri).with(url_no_auth, basic_auth).and_return(yaml) - expect(YAML).to receive(:safe_load).with(yaml).and_return(data).once + expect(YAML).to receive(:safe_load).with(yaml, aliases: true).and_return(data).once expect(subject).to run.with_params(filename).and_return(data) } end @@ -88,7 +106,7 @@ it { expect(OpenURI).to receive(:open_uri).with(filename, basic_auth).and_return(yaml) - expect(YAML).to receive(:safe_load).with(yaml).and_raise StandardError, 'Cannot parse data' + expect(YAML).to receive(:safe_load).with(yaml, aliases: true).and_raise StandardError, 'Cannot parse data' expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') } end @@ -103,4 +121,43 @@ expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') } end + + context 'when the file contains aliases' do + let(:tempfile) { Tempfile.new } + let(:filename) { tempfile.path } + let(:yaml) do + <<~YAML + some_numbers: &nums + - one + - two + more_numbers: *nums + YAML + end + let(:data) { { 'some_numbers' => ['one', 'two'], 'more_numbers' => ['one', 'two'] } } + + it 'parses the aliases' do + tempfile.write(yaml) + tempfile.rewind + expect(subject).to run.with_params(filename).and_return(data) + end + end + + context 'when a URL returns yaml with aliases' do + let(:filename) { 'https://example.local/myhash.yaml' } + let(:basic_auth) { { http_basic_authentication: ['', ''] } } + let(:yaml) do + <<~YAML + some_numbers: &nums + - one + - two + more_numbers: *nums + YAML + end + let(:data) { { 'some_numbers' => ['one', 'two'], 'more_numbers' => ['one', 'two'] } } + + it { + expect(OpenURI).to receive(:open_uri).with(filename, basic_auth).and_return(yaml) + expect(subject).to run.with_params(filename).and_return(data) + } + end end