Merge "Replace Puppet::Util::withenv with custom version"

lib/puppet/provider/openstack.rb

@@ -76,6 +76,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
   #
@@ -85,7 +102,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
       start_time = current_time
       end_time = start_time + request_timeout

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.

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
 
@@ -242,4 +242,45 @@ name="test"
       expect(Puppet::Provider::Openstack.parse_python_list(s)).to eq([1, 2, 3])
     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