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

Add a ps command to vagrant that drops the user into a remote powershell shell #4400

Merged
merged 6 commits into from Nov 18, 2015

Conversation

Projects
None yet
6 participants
@mwrock
Copy link
Contributor

mwrock commented Aug 27, 2014

This adds a new vagrant command:

vagrant ps

Example:

C:\dev\vagrant\win8.1x64> vagrant ps
    default: Creating powershell session to 192.168.1.14:5985
    default: Username: vagrant
[192.168.1.14]: PS C:\Users\vagrant\Documents>

The command is very similar to the ssh command but drops the user into a remote powershell session. The command is currently only supported on windows hosts with a guest running via the winrm communicator. The command, similar to vagrant ssh also supports a --command argument that can execute any powershell command over winrm.

Would be great to add @sneal 's repl to add support for linux hosts.

note: this PR is a squashed and final version of #4330

Fixes #5725

@mitchellh

This comment has been minimized.

Copy link
Member

mitchellh commented Aug 29, 2014

This looks really great. I'm going to review, but I won't be able to merge until we're working on 1.7.0 (shortly).

@mwrock

This comment has been minimized.

Copy link
Contributor Author

mwrock commented Aug 29, 2014

Sounds good. Thanks!

@mkuzmin mkuzmin referenced this pull request Sep 5, 2014

Closed

"vagrant console" command #4466

@sethvargo sethvargo changed the title adds a ps command to vagrant that drops the user into a remote powershell shell Add a ps command to vagrant that drops the user into a remote powershell shell Dec 15, 2014

@sethvargo

This comment has been minimized.

Copy link
Contributor

sethvargo commented May 30, 2015

Hi @mwrock

Sorry for the delay on this one. Could you please rebase this? It would also be great to get @sneal's thoughts and review here 😄

@sethvargo

This comment has been minimized.

Copy link
Contributor

sethvargo commented May 31, 2015

Fixes #5725

options = {}

opts = OptionParser.new do |o|
o.banner = "Usage: vagrant ps [-- extra ps args]"

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

I think this is actually:

vagrant ps [options] -- [powershell args]

right?

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

I'm trying to use the same convention as vagrant ssh which uses this same OptionParser banner language. This basically does the same thing: the extra ps args are simply passed through to the powershell.exe command.

o.separator "Options:"
o.separator ""

o.on("-c", "--command COMMAND", "Execute a powershell command directly") do |c|

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

I think the description here is "the powershell command to execute"

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

Yeah I'm again sticking to the same language used in the vagrant ssh command and the functionality is exactly the same. If a --command is supplied, it is executed on the vagrant box, otherwise one is dropped into an interactive powershell session.

raise Vagrant::Errors::VMNotCreatedError
end

if machine.config.vm.communicator != :winrm #|| !machine.provider.capability?(:winrm_info)

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

commented

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

good catch. missed this in the clean up.

end

if !options[:command].nil?
out_code = machine.communicate.execute options[:command]

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

Explicit parenthesis here would make this a bit easier to read 😄

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

added

if out_code == 0
machine.ui.detail("Command: #{options[:command]} executed succesfully with output code #{out_code}.")
end
break

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

What is the point of the break here? Shouldn't there only be one vm?

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

oops. meant next ...fixed

@mwrock mwrock force-pushed the mwrock:ps-cmd branch from ffb8a01 to 327050d May 31, 2015

# Extra arguments if we have any
ps_info[:extra_args] = options[:extra_args]

result = ready_ps_remoting_for machine, ps_info

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

( ) please 😄

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

done

@env.host.capability(:ps_client, ps_info)
ensure
if !result["PreviousTrustedHosts"].nil?
reset_ps_remoting_for machine, ps_info

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

( )

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

fixed

class Plugin < Vagrant.plugin("2")
name "ps command"
description <<-DESC
The ps command opens a remote powershell session to the

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

I think Microsoft likes when you use PowerShell

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

they so crazy. fixed.

DESC

command("ps") do
require File.expand_path("../command", __FILE__)

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

Let's use require_relative instead - it's faster, especially on Windows Ruby

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

ah. good to know. done.

def self.init!
return if defined?(@_init)
I18n.load_path << File.expand_path(
"templates/locales/command_ps.yml", Vagrant.source_root)

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

Let's keep this on one line, even if it overflows a tiny bit 😄

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

fixed.


logger.debug("Starting remote powershell with command:\n#{command}")
command = command.chars.to_a.join("\x00").chomp
command << "\x00" unless command[-1].eql? "\x00"

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

Don't we need an \x04 to signal EOT or no? Sorry - I'm not super familiar with Windows.

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

This entire encoding block was pretty much a copy/paste from the winrm gem as it stood at the time of this PR. This has changed quite a bit since. I am going to replace all of this with:

command = ::WinRM::PowershellScript.new(command).encoded
logger.debug("Starting remote powershell with command:\n#{command}")
command = command.chars.to_a.join("\x00").chomp
command << "\x00" unless command[-1].eql? "\x00"
if(defined?(command.encode))

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

I don't think this does what you think it does 😄

If you are trying to see if the command responds to encode, you should use .respond_to? instead.

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

I am also not sure I understand the encoding pieces here - could you explain a bit more?

It's also worth noting that the .encode command can fail.

en:
vagrant_ps:
detecting: |-
Detecting if a remote powershell connection can be made with the guest...

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

PowerShell to make the Microsofters happy 😄

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

fixed

ps_remoting_undetected: |-
Unable to establish a remote powershell connection with the guest.
Check if the firewall rules on the guest allow connections to the
windows remote management service.

This comment has been minimized.

@sethvargo

sethvargo May 31, 2015

Contributor

Windows

This comment has been minimized.

@mwrock

mwrock Jun 5, 2015

Author Contributor

fixed

@sethvargo

This comment has been minimized.

Copy link
Contributor

sethvargo commented May 31, 2015

Hi @mwrock

Thank you for the PR and I apologize for the delay in review. We have been a bit busy 😄. I am not super familiar with the Windows-side of things, but I left a few Ruby/Vagrant-related comments. Please let me know if you have any questions. I think this also may need a rebase 😄.

@mwrock

This comment has been minimized.

Copy link
Contributor Author

mwrock commented May 31, 2015

This is great. I had pretty much forgotten about this PR. I just rebased and I'll review and act on the comments this week. Thanks a bunch for looking at it!!

@mwrock mwrock force-pushed the mwrock:ps-cmd branch from b431f64 to 7408770 Jun 5, 2015

@mwrock

This comment has been minimized.

Copy link
Contributor Author

mwrock commented Jun 5, 2015

ok @sethvargo I have finished my edits, made an improvement to the -c switch making sure to return its output to the ui.detail and have done a fresh rebase.

This should be good to go.

args << "-password" << ps_info[:password]
result = Vagrant::Util::PowerShell.execute(script_path, *args)
if result.exit_code != 0
raise Errors::PowershellError,

This comment has been minimized.

@sneal

sneal Jun 6, 2015

Collaborator

Is PowershellError available in this context?

This comment has been minimized.

@mwrock

mwrock Jun 6, 2015

Author Contributor

Good catch! The PowershellError was actually completely misconfigured. This is now fixed.

vagrant_ps:
detecting: |-
Detecting if a remote PowerShell connection can be made with the guest...
reseting: |-

This comment has been minimized.

@sneal

sneal Jun 6, 2015

Collaborator

spelling, should be resetting

This comment has been minimized.

@mwrock

mwrock Jun 6, 2015

Author Contributor

thanks. fixed.

@sneal

This comment has been minimized.

Copy link
Collaborator

sneal commented Jun 6, 2015

LGTM, I wish I could easily try this out 😢 I'd like to add dropping to a rwinrm command outside Windows after this is merged.

@mwrock

This comment has been minimized.

Copy link
Contributor Author

mwrock commented Jun 6, 2015

Yeah that would be cool. Eventually it may be cool to just have one console commabnd that forwards you to the right shell: powershell automatically based on your host/guest: (windows/windows), rwinrm (linux(h)/windows(g)) or ssh (linux guest).

@bheuvel

This comment has been minimized.

Copy link
Contributor

bheuvel commented Sep 24, 2015

Any update on this PR? Am very happy with this functionality; cherry-picked this in my own branch/Gemfile to use it.

Know more Windows admins would love this as a standard feature in Vagrant.

@bobziuchkovski

This comment has been minimized.

Copy link

bobziuchkovski commented Sep 27, 2015

I'm also interested in this functionality. Any chance of merging this @sethvargo ?

@sethvargo sethvargo added this to the 1.8 milestone Nov 9, 2015

@mitchellh

This comment has been minimized.

Copy link
Member

mitchellh commented Nov 18, 2015

I trust you @mwrock and Seth reviewed this. The time is now, the place is here!

mitchellh added a commit that referenced this pull request Nov 18, 2015

Merge pull request #4400 from mwrock/ps-cmd
Add a ps command to vagrant that drops the user into a remote powershell shell

@mitchellh mitchellh merged commit 8bbf6f5 into hashicorp:master Nov 18, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.