From 3459e43b287e5954d82a7090e0804bc5355eda64 Mon Sep 17 00:00:00 2001
From: Michael T Lombardi <michael.t.lombardi@outlook.com>
Date: Thu, 24 Oct 2019 15:59:53 -0500
Subject: [PATCH 1/3] (FM-8643) Replace requires with feature confine

Prior to this commit the providers loaded the ruby-pwsh library
via a requires statement. When the powershell module is installed
without the dependency module, pwshlib, this requires statement
blows up as the library cannot be found.

This commit adds a ruby-pwsh feature and leverages it for the
providers.
---
 lib/puppet/feature/ruby_pwsh.rb        |  3 +++
 lib/puppet/provider/exec/powershell.rb | 10 ++++++++--
 lib/puppet/provider/exec/pwsh.rb       |  5 ++++-
 3 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 lib/puppet/feature/ruby_pwsh.rb

diff --git a/lib/puppet/feature/ruby_pwsh.rb b/lib/puppet/feature/ruby_pwsh.rb
new file mode 100644
index 00000000..71256417
--- /dev/null
+++ b/lib/puppet/feature/ruby_pwsh.rb
@@ -0,0 +1,3 @@
+require 'puppet/util/feature'
+
+Puppet.features.add(:ruby_pwsh, :libs => ["ruby-pwsh"])
diff --git a/lib/puppet/provider/exec/powershell.rb b/lib/puppet/provider/exec/powershell.rb
index 6eeb2b4a..0f32c770 100644
--- a/lib/puppet/provider/exec/powershell.rb
+++ b/lib/puppet/provider/exec/powershell.rb
@@ -1,10 +1,12 @@
 require 'puppet/provider/exec'
-require 'ruby-pwsh'
 
 Puppet::Type.type(:exec).provide :powershell, :parent => Puppet::Provider::Exec do
   confine :operatingsystem => :windows
+  confine :feature => :ruby_pwsh
 
-  commands :powershell => Pwsh::Manager.powershell_path
+  include Pwsh if Puppet.features.ruby_pwsh?
+
+  commands :powershell => defined?(Pwsh::Manager.powershell_path) ? Pwsh::Manager.powershell_path : nil
 
   desc <<-EOT
     Executes Powershell commands. One of the `onlyif`, `unless`, or `creates`
@@ -42,6 +44,10 @@
   have .NET Framework 3.5 installed.
   UPGRADE
 
+  def self.powershell_command
+    Pwsh::Manager.powershell_path.defined? ? Pwsh::Manager.powershell_path : nil
+  end
+
   def self.upgrade_message
     Puppet.warning POWERSHELL_MODULE_UPGRADE_MSG if !@upgrade_warning_issued
     @upgrade_warning_issued = true
diff --git a/lib/puppet/provider/exec/pwsh.rb b/lib/puppet/provider/exec/pwsh.rb
index 395f5b6a..6f7d1757 100644
--- a/lib/puppet/provider/exec/pwsh.rb
+++ b/lib/puppet/provider/exec/pwsh.rb
@@ -1,7 +1,10 @@
 require 'puppet/provider/exec'
-require 'ruby-pwsh'
 
 Puppet::Type.type(:exec).provide :pwsh, :parent => Puppet::Provider::Exec do
+  confine :feature => :ruby_pwsh
+
+  include Pwsh if Puppet.features.ruby_pwsh?
+
   desc <<-EOT
     Executes PowerShell Core commands. One of the `onlyif`, `unless`, or `creates`
     parameters should be specified to ensure the command is idempotent.

From 8e449d20350122af11dc9c8233a1f86e734c44a8 Mon Sep 17 00:00:00 2001
From: Michael T Lombardi <michael.t.lombardi@outlook.com>
Date: Tue, 29 Oct 2019 15:49:42 -0500
Subject: [PATCH 2/3] (MAINT) Alternate implementation

This commit includes an alternate to relying on the
confine function which is not available in modules
build on the resource api.
---
 .rubocop_todo.yml                          |  5 +-
 lib/puppet/feature/ruby_pwsh.rb            |  3 -
 lib/puppet/provider/exec/powershell.rb     | 20 +++---
 lib/puppet/provider/exec/pwsh.rb           |  5 +-
 lib/puppet_x/powershell/util.rb            | 19 +++++
 spec/unit/provider/exec/powershell_spec.rb | 84 +++-------------------
 spec/unit/provider/exec/pwsh_spec.rb       |  1 +
 7 files changed, 41 insertions(+), 96 deletions(-)
 delete mode 100644 lib/puppet/feature/ruby_pwsh.rb
 create mode 100644 lib/puppet_x/powershell/util.rb

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index b456bed0..e34073ce 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-10-18 11:01:56 -0500 using RuboCop version 0.49.1.
+# on 2019-10-29 15:58:53 -0500 using RuboCop version 0.49.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -13,7 +13,8 @@ GetText/DecorateFunctionMessage:
     - 'lib/puppet/provider/exec/pwsh.rb'
     - 'spec/spec_helper_acceptance.rb'
 
-# Offense count: 1
+# Offense count: 2
 GetText/DecorateString:
   Exclude:
     - 'lib/puppet/provider/exec/powershell.rb'
+    - 'lib/puppet_x/powershell/util.rb'
diff --git a/lib/puppet/feature/ruby_pwsh.rb b/lib/puppet/feature/ruby_pwsh.rb
deleted file mode 100644
index 71256417..00000000
--- a/lib/puppet/feature/ruby_pwsh.rb
+++ /dev/null
@@ -1,3 +0,0 @@
-require 'puppet/util/feature'
-
-Puppet.features.add(:ruby_pwsh, :libs => ["ruby-pwsh"])
diff --git a/lib/puppet/provider/exec/powershell.rb b/lib/puppet/provider/exec/powershell.rb
index 0f32c770..4592b67f 100644
--- a/lib/puppet/provider/exec/powershell.rb
+++ b/lib/puppet/provider/exec/powershell.rb
@@ -1,12 +1,8 @@
 require 'puppet/provider/exec'
+require 'puppet_x/powershell/util'
 
 Puppet::Type.type(:exec).provide :powershell, :parent => Puppet::Provider::Exec do
   confine :operatingsystem => :windows
-  confine :feature => :ruby_pwsh
-
-  include Pwsh if Puppet.features.ruby_pwsh?
-
-  commands :powershell => defined?(Pwsh::Manager.powershell_path) ? Pwsh::Manager.powershell_path : nil
 
   desc <<-EOT
     Executes Powershell commands. One of the `onlyif`, `unless`, or `creates`
@@ -44,23 +40,25 @@
   have .NET Framework 3.5 installed.
   UPGRADE
 
-  def self.powershell_command
-    Pwsh::Manager.powershell_path.defined? ? Pwsh::Manager.powershell_path : nil
+  def get_powershell_command
+    defined?(Pwsh::Manager.powershell_path) ? Pwsh::Manager.powershell_path : nil
   end
 
-  def self.upgrade_message
+  def upgrade_message
     Puppet.warning POWERSHELL_MODULE_UPGRADE_MSG if !@upgrade_warning_issued
     @upgrade_warning_issued = true
   end
 
   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(@powershell_command, Pwsh::Manager.powershell_args, debug: debug_output)
   end
 
   def run(command, check = false)
+    PuppetX::PowerShell::Util.load_lib unless PuppetX::PowerShell::Util.lib_loaded?
+    @powershell_command ||= get_powershell_command
     unless Pwsh::Manager.windows_powershell_supported?
-      self.class.upgrade_message
+      upgrade_message
       write_script(command) do |native_path|
         # Ideally, we could keep a handle open on the temp file in this
         # process (to prevent TOCTOU attacks), and execute powershell
@@ -70,7 +68,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(@powershell_command)}\" #{legacy_args} -Command - < \"#{native_path}\"\"", check)
       end
     else
       return execute_resource(command, resource)
diff --git a/lib/puppet/provider/exec/pwsh.rb b/lib/puppet/provider/exec/pwsh.rb
index 6f7d1757..4840945d 100644
--- a/lib/puppet/provider/exec/pwsh.rb
+++ b/lib/puppet/provider/exec/pwsh.rb
@@ -1,10 +1,6 @@
 require 'puppet/provider/exec'
 
 Puppet::Type.type(:exec).provide :pwsh, :parent => Puppet::Provider::Exec do
-  confine :feature => :ruby_pwsh
-
-  include Pwsh if Puppet.features.ruby_pwsh?
-
   desc <<-EOT
     Executes PowerShell Core commands. One of the `onlyif`, `unless`, or `creates`
     parameters should be specified to ensure the command is idempotent.
@@ -19,6 +15,7 @@
   EOT
 
   def run(command, check = false)
+    PuppetX::PowerShell::Util.load_lib unless PuppetX::PowerShell::Util.lib_loaded?
     @pwsh ||= get_pwsh_command
     self.fail 'pwsh could not be found' if @pwsh.nil?
     if Pwsh::Manager.pwsh_supported?
diff --git a/lib/puppet_x/powershell/util.rb b/lib/puppet_x/powershell/util.rb
new file mode 100644
index 00000000..211729cb
--- /dev/null
+++ b/lib/puppet_x/powershell/util.rb
@@ -0,0 +1,19 @@
+module PuppetX
+  module PowerShell
+    module Util
+      def lib_loaded?
+        defined?(Pwsh::Manager)
+      end
+      module_function :lib_loaded?
+    
+      def load_lib
+        begin
+          require 'ruby-pwsh'
+        rescue LoadError
+          Puppet.error 'Could not load the "ruby-pwsh" library; is puppetlabs-pwshlib installed?'
+        end
+      end
+      module_function :load_lib
+    end
+  end
+end
\ No newline at end of file
diff --git a/spec/unit/provider/exec/powershell_spec.rb b/spec/unit/provider/exec/powershell_spec.rb
index 41d67622..9e6c3cef 100644
--- a/spec/unit/provider/exec/powershell_spec.rb
+++ b/spec/unit/provider/exec/powershell_spec.rb
@@ -2,6 +2,7 @@
 require 'spec_helper'
 require 'puppet/util'
 require 'fileutils'
+require 'ruby-pwsh'
 
 describe Puppet::Type.type(:exec).provider(:powershell) do
 
@@ -48,6 +49,12 @@
         provider.run_spec_override(command)
       end
 
+      it "should raise an upgrade message warning" do
+        Puppet::Type::Exec::ProviderPowershell.any_instance.expects(:upgrade_message).once
+
+        provider.run_spec_override(command)
+      end
+
       context "on windows", :if => Puppet.features.microsoft_windows? do
         it "should call cmd.exe /c" do
           Puppet::Type::Exec::ProviderPowershell.any_instance.expects(:run)
@@ -84,6 +91,7 @@
 
     context "actual runs" do
       context "on Windows", :if => Puppet.features.microsoft_windows? do
+
         it "returns the output and status" do
           output, status = provider.run(command)
 
@@ -156,80 +164,4 @@
       end
     end
   end
-
-  describe 'when applying a catalog' do
-    let(:manifest) { <<-MANIFEST
-      exec { 'PS':
-        command   => 'exit 0',
-        provider  => powershell,
-      }
-    MANIFEST
-    }
-    let(:tmpdir) { Dir.mktmpdir('statetmp').encode!(Encoding::UTF_8) }
-
-    before :each do
-      skip('Not on Windows platform') unless Puppet.features.microsoft_windows?
-      # a statedir setting must now exist per the new transactionstore code
-      # introduced in Puppet 4.6 for corrective changes, as a new YAML file
-      # called transactionstore.yaml will be written under this path
-      # which defaults to c:\dev\null when not set on Windows
-      Puppet[:statedir] = tmpdir
-    end
-
-    after :each do
-      FileUtils.rm_rf(tmpdir)
-    end
-
-    def compile_to_catalog(string, node = Puppet::Node.new('foonode'))
-      Puppet[:code] = string
-
-      # see lib/puppet/indirector/catalog/compiler.rb#filter
-      Puppet::Parser::Compiler.compile(node).filter { |r| r.virtual? }
-    end
-
-    def compile_to_ral(manifest)
-      catalog = compile_to_catalog(manifest)
-      ral = catalog.to_ral
-      ral.finalize
-      ral
-    end
-
-    def apply_compiled_manifest(manifest)
-      catalog = compile_to_ral(manifest)
-
-      # ensure compilation works from Puppet 3.0.0 forward
-      args = [catalog, Puppet::Transaction::Report.new('apply')]
-      args << Puppet::Graph::SequentialPrioritizer.new if defined?(Puppet::Graph)
-      transaction = Puppet::Transaction.new(*args)
-      transaction.evaluate
-      transaction.report.finalize_report
-
-      transaction
-    end
-
-    it 'does not emit a warning message when PowerShellManager is usable in a Windows environment' do
-
-      Pwsh::Manager.stubs(:win32console_enabled?).returns(false)
-
-      expect(Pwsh::Manager.windows_powershell_supported?).to eq(true)
-
-      # given PowerShellManager is supported, never emit an upgrade message
-      Puppet::Type::Exec::ProviderPowershell.expects(:upgrade_message).never
-
-      apply_compiled_manifest(manifest)
-    end
-
-    it 'emits a warning message when PowerShellManager cannot be used in a Windows environment' do
-
-      # pretend we're Ruby 1.9.3 / Puppet 3.x x86
-      Pwsh::Manager.stubs(:win32console_enabled?).returns(true)
-
-      expect(Pwsh::Manager.windows_powershell_supported?).to eq(false)
-
-      # given PowerShellManager is NOT supported, emit an upgrade message
-      Puppet::Type::Exec::ProviderPowershell.expects(:upgrade_message).once
-
-      apply_compiled_manifest(manifest)
-    end
-  end
 end
diff --git a/spec/unit/provider/exec/pwsh_spec.rb b/spec/unit/provider/exec/pwsh_spec.rb
index 4a30720f..eb7f0a91 100644
--- a/spec/unit/provider/exec/pwsh_spec.rb
+++ b/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

From 008a5e9541b840fbf1c841fe2072146b56927078 Mon Sep 17 00:00:00 2001
From: Michael T Lombardi <michael.t.lombardi@outlook.com>
Date: Tue, 29 Oct 2019 17:26:10 -0500
Subject: [PATCH 3/3] (MAINT) Fail loudly if dependency missing

This commit causes the loading of the module to fail
loudly and clearly if the dependency module is not
found.
---
 .rubocop_todo.yml                          |  8 ++++----
 lib/puppet/provider/exec/powershell.rb     |  7 +++++--
 lib/puppet/provider/exec/pwsh.rb           |  6 +++++-
 lib/puppet_x/powershell/util.rb            | 19 -------------------
 spec/unit/provider/exec/powershell_spec.rb |  1 -
 spec/unit/provider/exec/pwsh_spec.rb       |  1 -
 6 files changed, 14 insertions(+), 28 deletions(-)
 delete mode 100644 lib/puppet_x/powershell/util.rb

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index e34073ce..befd5389 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -1,20 +1,20 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2019-10-29 15:58:53 -0500 using RuboCop version 0.49.1.
+# on 2019-10-29 17:24:12 -0500 using RuboCop version 0.49.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 4
+# Offense count: 6
 GetText/DecorateFunctionMessage:
   Exclude:
     - 'lib/puppet/provider/exec/powershell.rb'
     - 'lib/puppet/provider/exec/pwsh.rb'
     - 'spec/spec_helper_acceptance.rb'
 
-# Offense count: 2
+# Offense count: 3
 GetText/DecorateString:
   Exclude:
     - 'lib/puppet/provider/exec/powershell.rb'
-    - 'lib/puppet_x/powershell/util.rb'
+    - 'lib/puppet/provider/exec/pwsh.rb'
diff --git a/lib/puppet/provider/exec/powershell.rb b/lib/puppet/provider/exec/powershell.rb
index 4592b67f..0eae38e7 100644
--- a/lib/puppet/provider/exec/powershell.rb
+++ b/lib/puppet/provider/exec/powershell.rb
@@ -1,5 +1,9 @@
 require 'puppet/provider/exec'
-require 'puppet_x/powershell/util'
+begin
+  require 'ruby-pwsh'
+rescue LoadError
+  raise 'Could not load the "ruby-pwsh" library; is the module puppetlabs-pwshlib installed in this environment?'
+end
 
 Puppet::Type.type(:exec).provide :powershell, :parent => Puppet::Provider::Exec do
   confine :operatingsystem => :windows
@@ -55,7 +59,6 @@ def ps_manager
   end
 
   def run(command, check = false)
-    PuppetX::PowerShell::Util.load_lib unless PuppetX::PowerShell::Util.lib_loaded?
     @powershell_command ||= get_powershell_command
     unless Pwsh::Manager.windows_powershell_supported?
       upgrade_message
diff --git a/lib/puppet/provider/exec/pwsh.rb b/lib/puppet/provider/exec/pwsh.rb
index 4840945d..1156d690 100644
--- a/lib/puppet/provider/exec/pwsh.rb
+++ b/lib/puppet/provider/exec/pwsh.rb
@@ -1,4 +1,9 @@
 require 'puppet/provider/exec'
+begin
+  require 'ruby-pwsh'
+rescue LoadError
+  raise 'Could not load the "ruby-pwsh" library; is the module puppetlabs-pwshlib installed in this environment?'
+end
 
 Puppet::Type.type(:exec).provide :pwsh, :parent => Puppet::Provider::Exec do
   desc <<-EOT
@@ -15,7 +20,6 @@
   EOT
 
   def run(command, check = false)
-    PuppetX::PowerShell::Util.load_lib unless PuppetX::PowerShell::Util.lib_loaded?
     @pwsh ||= get_pwsh_command
     self.fail 'pwsh could not be found' if @pwsh.nil?
     if Pwsh::Manager.pwsh_supported?
diff --git a/lib/puppet_x/powershell/util.rb b/lib/puppet_x/powershell/util.rb
deleted file mode 100644
index 211729cb..00000000
--- a/lib/puppet_x/powershell/util.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-module PuppetX
-  module PowerShell
-    module Util
-      def lib_loaded?
-        defined?(Pwsh::Manager)
-      end
-      module_function :lib_loaded?
-    
-      def load_lib
-        begin
-          require 'ruby-pwsh'
-        rescue LoadError
-          Puppet.error 'Could not load the "ruby-pwsh" library; is puppetlabs-pwshlib installed?'
-        end
-      end
-      module_function :load_lib
-    end
-  end
-end
\ No newline at end of file
diff --git a/spec/unit/provider/exec/powershell_spec.rb b/spec/unit/provider/exec/powershell_spec.rb
index 9e6c3cef..1c431476 100644
--- a/spec/unit/provider/exec/powershell_spec.rb
+++ b/spec/unit/provider/exec/powershell_spec.rb
@@ -2,7 +2,6 @@
 require 'spec_helper'
 require 'puppet/util'
 require 'fileutils'
-require 'ruby-pwsh'
 
 describe Puppet::Type.type(:exec).provider(:powershell) do
 
diff --git a/spec/unit/provider/exec/pwsh_spec.rb b/spec/unit/provider/exec/pwsh_spec.rb
index eb7f0a91..4a30720f 100644
--- a/spec/unit/provider/exec/pwsh_spec.rb
+++ b/spec/unit/provider/exec/pwsh_spec.rb
@@ -1,7 +1,6 @@
 #! /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