Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New interop between Chef and PowerShell 4.0 (or higher) #6941

Merged
merged 11 commits into from
Mar 9, 2018

Conversation

stuartpreston
Copy link

@stuartpreston stuartpreston commented Mar 4, 2018

Description

This change provides a new way of working with PowerShell on Windows-based systems that support .NET Framework 4.0 and PowerShell 4.0 or higher (This is the out of box configuration included with Windows Server 2012 R2).

  • The primary benefit of this approach is PowerShell is accessed through the Ruby process and not via shell_out.
  • The output from PowerShell that is usually formatted specially for console but in the new case is coerced to JSON and available as a Hash.
  • Files containing potentially sensitive data are not written to disk, they are sent via COM to the PowerShell engine.
  • Errors that occurred during script block execution can be trapped and handled

What's included:

  • Chef::Powershell class (require "chef/powershell")
  • powershell_exec mixin
  • Binary inclusion of COM interop component required in the Windows package.
  • Registry keys required for operation added to the Windows installer package.

Example mixin usage

  outcome = powershell_exec("Get-WindowsFeature | where { $_.Installed -eq $true }")
  if outcome.error?
    outcome.errors.each do |e|
      Chef::Log.error e
    end
  else
    Chef::Log.info "Result: #{outcome.result}"
  end

NB: powershell_exec has been included in the Universal DSL mixin list, alongside the existing powershell_out

It should be noted a full install of Chef Client is required to test this due to the components being registered by Windows installer.

Check List

Stuart Preston added 3 commits March 3, 2018 16:49
@stuartpreston stuartpreston added Status: Incomplete A pull request that is not ready to be merged as noted by the author. Chef14 labels Mar 4, 2018
@stuartpreston stuartpreston requested a review from a team March 4, 2018 18:51
# @param script [String] script to run
# @return [Object] output
def initialize(*command_args)
raise "This class can only be used on the Windows platform." unless RUBY_PLATFORM =~ /mswin|mingw32|windows/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just say what the helper is. Sometimes you're deep in Chef errors and it's hard to see what spit the actual error out so you can fix your code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep you're right, I was struggling to find where we consistently apply this elsewhere, any recommendations?

# @return [Chef::PowerShell] output
def powershell_exec(*command_args)
script = command_args.first
options = command_args.last.is_a?(Hash) ? command_args.last : nil
Copy link
Contributor

@lamont-granquist lamont-granquist Mar 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be powershell_exec(script) for now. I don't think options are being used at all here now and can safely be introduced later when there's actually options to use (and **options should get used rather than the last.is_a?(Hash) stuff which is probably some ruby 1.8 kludginess in shell_out which we've not yet dared to do away with). The array style passing of arguments on windows is also busted (i tried adding windows support once to bake the poise_shell_out helper directly into mixlib-shellout, but its complicated and brittle -- https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was following powershell_out's style of doing things here. I'll revert back to taking the single argument for now :)

@stuartpreston stuartpreston added Status: Ready for Merge and removed Status: Incomplete A pull request that is not ready to be merged as noted by the author. labels Mar 8, 2018
@stuartpreston
Copy link
Author

Ready for merge, the Travis failures with Chef Zero seem unrelated...


## New Windows PowerShell mixin `powershell_exec`

Since our supported Windows platforms can all run .NET Framework 4.0 and PowerShell 4.0 we have taken time to add a new interop that will allow faster and safer interactions with the system PowerShell. You will be able to use the `powershell_exec` mixin in most places where you would have previously used `powershell_out`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a benchmark here so we could explain how fancy this new mixin is

@lock
Copy link

lock bot commented May 8, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants