(MODULES-10539) Remove commands idiom from PowerShell provider

Prior to this commit the PowerShell exec provider used the commands idiom to
load the PowerShell executable path for use. Once the module was refactored
to rely on the ruby-pwsh gem via the puppetlabs/pwshlib module, the code for
determining the path was updated to depend on a helper function. However,
the commands method is (apparently?) evaluated during catalogue compilation,
causing the Puppet run to explode unexpectedly if the dependent module is
not available.

This commit removes the commands idiom and relies on the feature confine which
should evaluate during a Puppet run and, when that feature is not found
(because puppetlabs/pwshlib is not installed) it will report that the provider
is not functional rather than blow up the entire Puppet run.

Author: michaeltlombardi

lib/puppet/provider/exec/powershell.rb

@@ -4,16 +4,6 @@
   confine :operatingsystem => :windows
   confine :feature => :pwshlib
 
-  def self.powershell_path
-    begin
-      require 'ruby-pwsh'
-      Pwsh::Manager.powershell_path
-    rescue
-      nil
-    end
-  end
-  commands :powershell => self.powershell_path
-
   desc <<-EOT
     Executes Powershell commands. One of the `onlyif`, `unless`, or `creates`
     parameters should be specified to ensure the command is idempotent.
@@ -57,7 +47,7 @@ def self.upgrade_message
 
   def ps_manager
     debug_output = Puppet::Util::Log.level == :debug
-    Pwsh::Manager.instance(command(:powershell), Pwsh::Manager.powershell_args, debug: debug_output)
+    Pwsh::Manager.instance(Pwsh::Manager.powershell_path, Pwsh::Manager.powershell_args, debug: debug_output)
   end
 
   def run(command, check = false)
@@ -72,7 +62,7 @@ def run(command, check = false)
         # we redirect powershell's stdin to read from the file. Current
         # versions of Windows use per-user temp directories with strong
         # permissions, but I'd rather not make (poor) assumptions.
-        return super("cmd.exe /c \"\"#{native_path(command(:powershell))}\" #{legacy_args} -Command - < \"#{native_path}\"\"", check)
+        return super("cmd.exe /c \"\"#{native_path(Pwsh::Manager.powershell_path)}\" #{legacy_args} -Command - < \"#{native_path}\"\"", check)
       end
     else
       return execute_resource(command, resource)

spec/acceptance/exec_powershell_spec.rb

@@ -465,4 +465,31 @@
     PS1
     it_should_behave_like 'standard exec', pimport
   end
+
+  # TODO: For some reason, Puppet still sees the dependent module as available during
+  # a localhost run, but not when testing against a remote target.
+  context 'without pwshlib available', unless: (ENV['TARGET_HOST'] == 'localhost') do
+    before(:all) do
+      remove_pwshlib
+    end
+    after(:all) do
+      install_pwshlib
+    end
+  
+    let(:manifest) {
+      <<-MANIFEST
+        exec{'TestPowershell':
+          command   => 'Get-Process > c:/process.txt',
+          unless    => 'if(!(test-path "c:/process.txt")){exit 1}',
+          provider  => powershell,
+        }
+      MANIFEST
+    }
+  
+    it "Errors predictably" do
+      apply_manifest(manifest, expect_failures: true) do |result|
+        expect(result.stderr).to match(/Provider powershell is not functional on this host/)
+      end
+    end
+  end
 end

spec/acceptance/exec_pwsh_spec.rb

@@ -517,5 +517,32 @@ def platform_string(windows, posix)
       PS1
       it_should_behave_like 'standard exec', pimport
     end
+
+    # TODO: For some reason, Puppet still sees the dependent module as available during
+    # a localhost run, but not when testing against a remote target.
+    describe 'without pwshlib available', unless: (ENV['TARGET_HOST'] == 'localhost') do
+      before(:all) do
+        remove_pwshlib
+      end
+      after(:all) do
+        install_pwshlib
+      end
+    
+      let(:manifest) {
+        <<-MANIFEST
+          exec{'TestPowershell':
+            command   => 'Get-Process > /process.txt',
+            unless    => 'if(!(test-path "/process.txt")){exit 1}',
+            provider  => pwsh,
+          }
+        MANIFEST
+      }
+    
+      it "Errors predictably" do
+        apply_manifest(manifest, expect_failures: true) do |result|
+          expect(result.stderr).to match(/Provider pwsh is not functional on this host/)
+        end
+      end
+    end
   end
 end

spec/spec_helper_acceptance_local.rb

@@ -55,13 +55,32 @@ def interpolate_powershell(command)
   "powershell.exe -NoProfile -EncodedCommand #{encoded_command}"
 end
 
+def relative_folder(relative_path)
+  expanded_path = File.expand_path(File.join(File.dirname(__FILE__), relative_path))
+  Dir.open(expanded_path) if File.exist?(expanded_path)
+end
+
+def remove_pwshlib
+  uninstall_command = 'puppet module uninstall puppetlabs/pwshlib --force'
+  uninstall_command += " --modulepath #{relative_folder('fixtures/modules').path}" if ENV['TARGET_HOST'] == 'localhost'
+  Helper.instance.run_shell(uninstall_command, expect_failures: true) do |result|
+    raise "Failed to uninstall puppetlabs/pwshlib" unless result.stderr =~ /Module 'puppetlabs-pwshlib' is not installed/ || result.exit_code == 0
+  end
+end
+
+def install_pwshlib
+  install_command = 'puppet module install puppetlabs/pwshlib'
+  install_command += " --modulepath #{relative_folder('fixtures/modules').path}" if ENV['TARGET_HOST'] == 'localhost'
+  Helper.instance.run_shell(install_command)
+end
+
 def localhost_windows?
   os[:family] == 'windows' && ENV['TARGET_HOST'] == 'localhost'
 end
 
 RSpec.configure do |c|
   c.before :suite do
-    Helper.instance.run_shell('puppet module install puppetlabs/pwshlib')
+    install_pwshlib
     cleanup_files
   end
 end

spec/unit/provider/exec/powershell_spec.rb

@@ -38,6 +38,7 @@
   describe "#run" do
     context "stubbed calls" do
       before :each do
+        require 'ruby-pwsh'
         Pwsh::Manager.stubs(:windows_powershell_supported?).returns(false)
         Puppet::Provider::Exec.any_instance.stubs(:run)
       end

spec/unit/provider/exec/pwsh_spec.rb

@@ -1,6 +1,7 @@
 #! /usr/bin/env ruby
 require 'spec_helper'
 require 'puppet/util'
+require 'ruby-pwsh'
 
 describe Puppet::Type.type(:exec).provider(:pwsh) do
   # Override the run value so we can test the super call