Merge branch 'kpaulisse-puppet-version-check-environment' into kpaulisse-octocatalog-diff-0-5-4

Author: Kevin Paulisse

.version

@@ -1 +1 @@
-0.5.3
+0.5.4

doc/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 | Version | Date | Description / Changes |
 | ------- | ---- | ----------- |
+| 0.5.4   | 2016-11-03 | https://github.com/github/octocatalog-diff/pull/16: environment running `puppet --version` |
 | 0.5.3   | 2016-10-31 | https://github.com/github/octocatalog-diff/pull/10: facts terminus optimization |
 | 0.5.2   | - | Unreleased internal version |
 | 0.5.1   | 2016-10-20 | Initial release |

lib/octocatalog-diff/util/puppetversion.rb

@@ -15,9 +15,16 @@ def self.puppet_version(puppet)
         raise ArgumentError, 'Puppet binary was not supplied' if puppet.nil?
         raise Errno::ENOENT, "Puppet binary #{puppet} doesn't exist" unless File.file?(puppet)
         cmdline = [Shellwords.escape(puppet), '--version'].join(' ')
-        output, _code = Open3.capture2e(cmdline)
-        return Regexp.last_match(1) if output =~ /^([\d\.]+)\s*$/
-        raise "Unable to determine Puppet version: #{output}"
+
+        # This is the environment provided to the puppet command.
+        env = {
+          'HOME' => ENV['HOME'],
+          'PATH' => ENV['PATH'],
+          'PWD' => File.dirname(puppet)
+        }
+        out, err, _status = Open3.capture3(env, cmdline, unsetenv_others: true, chdir: env['PWD'])
+        return Regexp.last_match(1) if out =~ /^([\d\.]+)\s*$/
+        raise "Unable to determine Puppet version: #{out} #{err}"
       end
     end
   end

spec/octocatalog-diff/tests/catalog/computed_spec.rb

@@ -239,92 +239,85 @@
     end
 
     describe '#puppet_version' do
-      before(:all) do
-        @temp_puppet = Tempfile.new('puppet')
-        @temp_puppet.write "#!/bin/bash\n"
-        @temp_puppet.write "if [ \"$FAKE_PUPPET_VERSION\" == \"oops\" ]; then echo \"oops\" >&2; exit 1; fi\n"
-        @temp_puppet.write "echo $FAKE_PUPPET_VERSION\n"
-        @temp_puppet.close
-        FileUtils.chmod 0o755, @temp_puppet.path
-      end
+      context 'with working Puppet version' do
+        before(:all) do
+          @temp_puppet = Tempfile.new('puppet')
+          @temp_puppet.write "#!/bin/bash\n"
+          @temp_puppet.write "echo '3.8.7'\n"
+          @temp_puppet.close
+          FileUtils.chmod 0o755, @temp_puppet.path
+        end
 
-      after(:all) do
-        @temp_puppet.unlink
-      end
+        after(:all) do
+          @temp_puppet.unlink
+        end
 
-      after(:each) do
-        ENV.delete('FAKE_PUPPET_VERSION')
+        it 'should return a puppet version upon success' do
+          opts = {
+            basedir: '/',
+            node: 'foonode',
+            puppet_binary: @temp_puppet.path,
+            puppet_command: @temp_puppet.path,
+            fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
+            branch: '.'
+          }
+          catalog = OctocatalogDiff::Catalog::Computed.new(opts)
+          catalog.build
+          expect(catalog.puppet_version).to eq('3.8.7')
+        end
       end
 
-      it 'should return a puppet version upon success' do
-        ENV['FAKE_PUPPET_VERSION'] = '3.8.7'
-        opts = {
-          basedir: '/',
-          node: 'foonode',
-          pass_env_vars: ['FAKE_PUPPET_VERSION'],
-          puppet_binary: @temp_puppet.path,
-          puppet_command: @temp_puppet.path,
-          fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
-          branch: '.'
-        }
-        catalog = OctocatalogDiff::Catalog::Computed.new(opts)
-        catalog.build
-        expect(catalog.puppet_version).to eq('3.8.7')
-      end
+      context 'with failing Puppet version' do
+        before(:all) do
+          @temp_puppet = Tempfile.new('puppet')
+          @temp_puppet.write "#!/bin/bash\n"
+          @temp_puppet.write "echo 1>&2 'something failed horribly'\n"
+          @temp_puppet.write "exit 1\n"
+          @temp_puppet.close
+          FileUtils.chmod 0o755, @temp_puppet.path
+        end
 
-      it 'should raise an error if the version number is empty' do
-        opts = {
-          basedir: '/',
-          node: 'foonode',
-          puppet_binary: @temp_puppet.path,
-          puppet_command: @temp_puppet.path,
-          fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
-          branch: '.'
-        }
-        expect { OctocatalogDiff::Catalog::Computed.new(opts).build }
-          .to raise_error(RuntimeError, /Unable to determine Puppet version/)
-      end
+        after(:all) do
+          @temp_puppet.unlink
+        end
 
-      it 'should raise an error if the puppet binary is nil' do
-        opts = {
-          basedir: '/',
-          node: 'foonode',
-          puppet_command: @temp_puppet.path,
-          fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
-          branch: '.'
-        }
-        expect { OctocatalogDiff::Catalog::Computed.new(opts).build }.to raise_error(ArgumentError)
+        it 'should raise an error if the version number is empty' do
+          opts = {
+            basedir: '/',
+            node: 'foonode',
+            puppet_binary: @temp_puppet.path,
+            puppet_command: @temp_puppet.path,
+            fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
+            branch: '.'
+          }
+          expect { OctocatalogDiff::Catalog::Computed.new(opts).build }
+            .to raise_error(RuntimeError, /Unable to determine Puppet version/)
+        end
       end
+    end
 
-      it 'should raise an error if the puppet binary does not exist' do
-        ENV['FAKE_PUPPET_VERSION'] = 'chicken'
-        opts = {
-          basedir: '/',
-          node: 'foonode',
-          pass_env_vars: ['FAKE_PUPPET_VERSION'],
-          puppet_binary: '/alsdfklafjasfkljafjafkjsdflaksfjasdfjadsfjadsf',
-          puppet_command: @temp_puppet.path,
-          fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
-          branch: '.'
-        }
-        expect { OctocatalogDiff::Catalog::Computed.new(opts).build }
-          .to raise_error(Errno::ENOENT)
-      end
+    it 'should raise an error if the puppet binary is nil' do
+      opts = {
+        basedir: '/',
+        node: 'foonode',
+        puppet_command: '/alsdfklafjasfkljafjafkjsdflaksfjasdfjadsfjadsf',
+        fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
+        branch: '.'
+      }
+      expect { OctocatalogDiff::Catalog::Computed.new(opts).build }.to raise_error(ArgumentError)
+    end
 
-      it 'should raise an error if the puppet binary returns a nonzero exit code' do
-        ENV['FAKE_PUPPET_VERSION'] = 'oops'
-        opts = {
-          basedir: '/',
-          node: 'foonode',
-          pass_env_vars: ['FAKE_PUPPET_VERSION'],
-          puppet_binary: @temp_puppet.path,
-          puppet_command: @temp_puppet.path,
-          fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
-          branch: '.'
-        }
-        expect { OctocatalogDiff::Catalog::Computed.new(opts).build }
-          .to raise_error(RuntimeError, /Unable to determine Puppet version/)
-      end
+    it 'should raise an error if the puppet binary does not exist' do
+      opts = {
+        basedir: '/',
+        node: 'foonode',
+        puppet_binary: '/alsdfklafjasfkljafjafkjsdflaksfjasdfjadsfjadsf',
+        puppet_command: '/alsdfklafjasfkljafjafkjsdflaksfjasdfjadsfjadsf',
+        fact_file: OctocatalogDiff::Spec.fixture_path('facts/valid-facts.yaml'),
+        branch: '.'
+      }
+      expect { OctocatalogDiff::Catalog::Computed.new(opts).build }
+        .to raise_error(Errno::ENOENT)
     end
   end