Merge pull request #10274 from chef/resource-code-cleanup

powershell_script cleanup

5 files changed, 57 insertions, 59 deletions

diff --git a/lib/chef/provider/powershell_script.rb b/lib/chef/provider/powershell_script.rb
index 2b7c384246..49d800b6d2 100644
--- a/lib/chef/provider/powershell_script.rb
+++ b/lib/chef/provider/powershell_script.rb
@@ -31,21 +31,32 @@ class Chef
         super()
       end
 
-      def command
-        # Powershell.exe is always in "v1.0" folder (for backwards compatibility)
-        interpreter_path = Chef::Util::PathHelper.join(basepath, "WindowsPowerShell", "v1.0", interpreter)
+      # Set InputFormat to None as PowerShell will hang if STDIN is redirected
+      # http://connect.microsoft.com/PowerShell/feedback/details/572313/powershell-exe-can-hang-if-stdin-is-redirected
+      DEFAULT_FLAGS = "-NoLogo -NonInteractive -NoProfile -ExecutionPolicy Bypass -InputFormat None".freeze
 
+      def command
         # Must use -File rather than -Command to launch the script
         # file created by the base class that contains the script
         # code -- otherwise, powershell.exe does not propagate the
         # error status of a failed Windows process that ran at the
         # end of the script, it gets changed to '1'.
         #
-        "\"#{interpreter_path}\" #{new_resource.flags} -File \"#{script_file_path}\""
+        [
+          %Q{"#{interpreter_path}"},
+          DEFAULT_FLAGS,
+          new_resource.flags,
+          %Q{-File "#{script_file_path}"},
+        ].join(" ")
       end
 
       protected
 
+      def interpreter_path
+        # Powershell.exe is always in "v1.0" folder (for backwards compatibility)
+        Chef::Util::PathHelper.join(basepath, "WindowsPowerShell", "v1.0", interpreter)
+      end
+
       def code
         code = wrapper_script
         logger.trace("powershell_script provider called with script code:\n\n#{new_resource.code}\n")
@@ -71,7 +82,12 @@ class Chef
           # written to the file system at this point, which is required since
           # the intent is to execute the code just written to it.
           user_script_file.close
-          validation_command = "\"#{interpreter}\" #{new_resource.flags} -Command \". '#{user_script_file.path}'\""
+          validation_command = [
+            %Q{"#{interpreter_path}"},
+            DEFAULT_FLAGS,
+            new_resource.flags,
+            %Q{-Command ". '#{user_script_file.path}'"},
+          ].join(" ")
 
           # Note that other script providers like bash allow syntax errors
           # to be suppressed by setting 'returns' to a value that the
diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index cc4837c901..c274f9945d 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -509,7 +509,6 @@ class Chef
       def initialize(name, run_context = nil)
         super
         @command = name
-        @backup = 5
         @default_guard_interpreter = :execute
         @is_guard_interpreter = false
       end
diff --git a/lib/chef/resource/powershell_script.rb b/lib/chef/resource/powershell_script.rb
index a334f945e3..7699c0a976 100644
--- a/lib/chef/resource/powershell_script.rb
+++ b/lib/chef/resource/powershell_script.rb
@@ -25,19 +25,31 @@ class Chef
       provides :powershell_script, os: "windows"
 
       property :flags, String,
-        description: "A string that is passed to the Windows PowerShell command",
-        default: lazy { default_flags },
-        coerce: proc { |input|
-          if input == default_flags
-            # Means there was no input provided,
-            # and should use defaults in this case
-            input
-          else
-            # The last occurrence of a flag would override its
-            # previous one at the time of command execution.
-            [default_flags, input].join(" ")
+        description: "A string that is passed to the Windows PowerShell command"
+
+      property :convert_boolean_return, [true, false],
+        default: false,
+        description: <<~DESC
+          Return `0` if the last line of a command is evaluated to be true or to return `1` if the last line is evaluated to be false.
+
+          When the `guard_interpreter` common attribute is set to `:powershell_script`, a string command will be evaluated as if this value were set to `true`. This is because the behavior of this attribute is similar to the value of the `"$?"` expression common in UNIX interpreters. For example, this:
+
+          ```ruby
+          powershell_script 'make_safe_backup' do
+            guard_interpreter :powershell_script
+            code 'cp ~/data/nodes.json ~/data/nodes.bak'
+            not_if 'test-path ~/data/nodes.bak'
+          end
+          ```
+
+          is similar to:
+          ```ruby
+          bash 'make_safe_backup' do
+            code 'cp ~/data/nodes.json ~/data/nodes.bak'
+            not_if 'test -e ~/data/nodes.bak'
           end
-        }
+          ```
+        DESC
 
       description "Use the **powershell_script** resource to execute a script using the Windows PowerShell"\
                   " interpreter, much like how the script and script-based resources—bash, csh, perl, python,"\
@@ -52,15 +64,6 @@ class Chef
         super
         @interpreter = "powershell.exe"
         @default_guard_interpreter = resource_name
-        @convert_boolean_return = false
-      end
-
-      def convert_boolean_return(arg = nil)
-        set_or_return(
-          :convert_boolean_return,
-          arg,
-          kind_of: [ FalseClass, TrueClass ]
-        )
       end
 
       # Allow callers evaluating guards to request default
@@ -73,15 +76,6 @@ class Chef
       def self.get_default_attributes(opts)
         { convert_boolean_return: true }
       end
-
-      # Options that will be passed to Windows PowerShell command
-      #
-      # @returns [String]
-      def default_flags
-        # Set InputFormat to None as PowerShell will hang if STDIN is redirected
-        # http://connect.microsoft.com/PowerShell/feedback/details/572313/powershell-exe-can-hang-if-stdin-is-redirected
-        "-NoLogo -NonInteractive -NoProfile -ExecutionPolicy Bypass -InputFormat None"
-      end
     end
   end
 end
diff --git a/spec/unit/provider/powershell_script_spec.rb b/spec/unit/provider/powershell_script_spec.rb
index e0857a1ea4..a0f31c10f7 100644
--- a/spec/unit/provider/powershell_script_spec.rb
+++ b/spec/unit/provider/powershell_script_spec.rb
@@ -30,14 +30,20 @@ describe Chef::Provider::PowershellScript, "action_run" do
     Chef::Provider::PowershellScript.new(new_resource, run_context)
   end
 
-  context "when setting interpreter flags" do
+  describe "#command" do
     before(:each) do
-      allow(provider).to receive(:basepath).and_return("C:\\Windows\\system32")
+      allow(provider).to receive(:basepath).and_return("C:/Windows/system32")
     end
 
-    it "sets the -File flag as the last flag" do
-      flags = provider.command.split(" ").keep_if { |flag| flag =~ /^-/ }
-      expect(flags.pop).to eq("-File")
+    it "includes the user's flags after the default flags when building the command" do
+      new_resource.flags = "-InputFormat Fabulous"
+      provider.send(:script_file_path=, "C:/Temp/Script.ps1")
+
+      expected = <<~CMD.strip
+        "C:/Windows/system32/WindowsPowerShell/v1.0/powershell.exe" -NoLogo -NonInteractive -NoProfile -ExecutionPolicy Bypass -InputFormat None -InputFormat Fabulous -File "C:/Temp/Script.ps1"
+      CMD
+
+      expect(provider.command).to eq(expected)
     end
   end
 end
diff --git a/spec/unit/resource/powershell_script_spec.rb b/spec/unit/resource/powershell_script_spec.rb
index 6666c4749b..10e3530a5f 100644
--- a/spec/unit/resource/powershell_script_spec.rb
+++ b/spec/unit/resource/powershell_script_spec.rb
@@ -130,24 +130,7 @@ describe Chef::Resource::PowershellScript do
     let(:resource_instance_name ) { @resource.command }
     let(:resource_name) { :powershell_script }
     let(:interpreter_file_name) { "powershell.exe" }
-    before do
-      allow(@resource).to receive(:default_flags).and_return(nil)
-    end
-    it_behaves_like "a Windows script resource"
-  end
-
-  describe "#flags" do
-    let(:resource) { @resource }
 
-    it "appends user's flags to the defaults" do
-      flags = %q{-Lunch "tacos"}
-      resource.flags = flags
-
-      expect(resource.flags).to eq("#{resource.default_flags} #{flags}")
-    end
-
-    it "uses the defaults when user doesn't provide flags" do
-      expect(resource.flags).to eq(resource.default_flags)
-    end
+    it_behaves_like "a Windows script resource"
   end
 end