diff --git a/lib/puppet/provider/openstack.rb b/lib/puppet/provider/openstack.rb index 5c261e3d..9a403fe3 100644 --- a/lib/puppet/provider/openstack.rb +++ b/lib/puppet/provider/openstack.rb @@ -78,6 +78,23 @@ class Puppet::Provider::Openstack < Puppet::Provider rc end + # Copy of Puppet::Util::withenv but that filters out + # env variables starting with OS_ from the existing + # environment. + # + # @param hash [Hash] Hash of environment variables + def self.os_withenv(hash) + saved = ENV.to_hash + begin + cleaned_env = ENV.to_hash.reject { |k, _| k.start_with?('OS_') } + ENV.replace(cleaned_env) + ENV.merge!(hash.transform_keys(&:to_s)) + yield + ensure + ENV.replace(saved) + end + end + # Returns an array of hashes, where the keys are the downcased CSV headers # with underscores instead of spaces # @@ -87,7 +104,7 @@ class Puppet::Provider::Openstack < Puppet::Provider env = credentials ? credentials.to_env : {} no_retry = options[:no_retry_exception_msgs] - Puppet::Util.withenv(env) do + os_withenv(env) do rv = nil end_time = current_time + request_timeout start_time = current_time diff --git a/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml b/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml new file mode 100644 index 00000000..e381ca63 --- /dev/null +++ b/releasenotes/notes/os-withenv-3ca466fde75f6441.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``Puppet::Provider::Openstack.request`` now filters out all external + ``OS_*`` environment variables to prevent collisions with environment + variables in the shell where Puppet is running. diff --git a/spec/unit/provider/openstack_spec.rb b/spec/unit/provider/openstack_spec.rb index 81b636aa..37aa005c 100644 --- a/spec/unit/provider/openstack_spec.rb +++ b/spec/unit/provider/openstack_spec.rb @@ -80,7 +80,7 @@ name="test" end it 'uses provided credentials' do - expect(Puppet::Util).to receive(:withenv).with(credentials.to_env) + expect(provider.class).to receive(:os_withenv).with(credentials.to_env) Puppet::Provider::Openstack.request('project', 'list', ['--long'], credentials) end @@ -210,4 +210,45 @@ name="test" end end end + + describe '#os_withenv' do + around do |example| + @original_env = ENV.to_hash + example.run + ENV.replace(@original_env) + end + + it 'removes environment variables starting with OS_' do + ENV['OS_FOO'] = 'should be removed' + ENV['OTHER'] = 'should stay' + + Puppet::Provider::Openstack.os_withenv({}) do + expect(ENV.key?('OS_FOO')).to be false + expect(ENV['OTHER']).to eq('should stay') + end + end + + it 'merges the given environment variables' do + Puppet::Provider::Openstack.os_withenv({'MY_VAR' => '123'}) do + expect(ENV['MY_VAR']).to eq('123') + end + end + + it 'restores the original environment after the block' do + original = ENV.to_hash + Puppet::Provider::Openstack.os_withenv({'TEMP_VAR' => 'test'}) do + ENV['INSIDE_BLOCK'] = 'yep' + end + + expect(ENV.key?('TEMP_VAR')).to be false + expect(ENV.key?('INSIDE_BLOCK')).to be false + expect(ENV.to_hash).to eq(original) + end + + it 'handles string and symbol keys in the input hash' do + Puppet::Provider::Openstack.os_withenv({:FOO => 'bar'}) do + expect(ENV['FOO']).to eq('bar') + end + end + end end