First whack at windows (target) support for taste-tester #144

jaymzh · 2020-08-22T07:04:24Z

I can't imagine we have any desire to have TT itself run on Windows,
however, one has to manage their windows system sometime, and that means
being able to test on those systems.

This adds support for the remote system by generating the right
powershell to send over ssh.

That means there are two requirements to using this:

You have ssh enabled on your Windows PC
You set the default shell to powershell instead of cmd

Both are easily accomplished with this tiny bit of Chef:

powershell_package 'ComputerManagementDsc' do
  action :install
end

dsc_resource 'install ssh-client' do
  resource :windowscapability
  module_name 'ComputerManagementDsc'
  property :name, 'OpenSSH.Client~~~~0.0.1.0'
  property :ensure, 'Present'
end

dsc_resource 'install ssh-server' do
  resource :windowscapability
  module_name 'ComputerManagementDsc'
  property :name, 'OpenSSH.Server~~~~0.0.1.0'
  property :ensure, 'Present'
end

dsc_resource 'start sshd' do
  resource :service
  property :name, 'sshd'
  property :startuptype, 'Automatic'
  property :state, 'Running'
  property :ensure, 'Present'
end

This also requires you to be on a version modern enough that symlinks
actually work - sorry, we're not re-inventing how taste-tester works
for old broken OSes. Getting Windws support here is ugly enough as it
is.

You may be wondering "but Windows has bash support now!"... and you'd be
sorta-right. You can enable WSL and Bash in modern Windows, but you end
up in a embedded linux environment. You can access the Windows
filesystem, but it's not a thing most people are going to want to do on
their windows systems. So, powershell it is.

This fully supports tunnels and non-tunnels. As far as I can tell,
everything works except "bundle mode" and "local transport", but I don't
think those are necessary here.

In order to not repeat the crazy trans-shell logic, I factored out
some code that was repeated (but badly, with bugs - now we always
specify -o StrictHostKeyChecking=no and friends, not just sometimes)
between ssh.rb and tunnel.rb into ssh_util.rb. I took this approach
because it was the least change, and since that's not directly related
to this PR, I wanted to minimize that. However, the long-term solution
here is to just roll tunnel.rb into ssh.rb. It already has a tunnel
option it ignores in the initializer.

jaymzh · 2020-08-22T07:20:42Z

Oh one thing this does not yet do is handle run as (Windows' closest cousin to sudo). This assumes you're in the Administrator group and have all the relevant rights and privileges. Support for run as is doable, but I don't want to roll that into this PR. I'll probably get around to another PR for it at some point.

davide125

This seems overall reasonable, but I'd like someone with powershell/windows experience to review it as well. cc @svmastersamurai @HeyItsGilbert

davide125 · 2020-08-22T15:44:16Z

bin/taste-tester

-    rescue StandardError
-      logger.error("Invalid date: #{time}")
-      exit 1
+      # can make this an implicit rescue after we dtop 2.4


s/dtop/drop/ ?

bin/taste-tester

lib/taste_tester/host.rb

davide125 · 2020-08-22T15:47:40Z

lib/taste_tester/host.rb

+        "$ttconfig = \"#{ttconfig}\"",
+        "$realconfig = \"#{realconfig}\"",
+
+        # TODO use real tmpfiles


yeah, you should be able to use New-TemporaryFile here: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/new-temporaryfile

davide125 · 2020-08-22T15:49:31Z

lib/taste_tester/ssh_util.rb

+          # *and* powershell, so in order to preserve the quotes, it gets
+          # pretty ugly.
+          #
+          # The tldr here is that in shell you can't escapte quotes you're


s/escapte/escape/

lib/taste_tester/ssh_util.rb

davide125 · 2020-08-22T15:50:43Z

lib/taste_tester/ssh_util.rb

+          #
+          # We're doing the same thing here. What we want on the other side of
+          # the ssh is:
+          #   Text.Encoding]::Utf8.GetString([Convert]::FromBase64String('...'))


I think you dropped a leading [ and meant [Text.Encoding]::Utf8.GetString([Convert]::FromBase64String('...')) here?

davide125 · 2020-08-22T15:51:02Z

lib/taste_tester/ssh_util.rb

+          # single quotes too. For simplicity lets call those two functions
+          # above GetString() and Base64(). So we'll start with:
+          #    ssh host 'GetString(Base64('
+          # We've closed that strong, now we add the single quote we want there,


s/strong/string/

davide125 · 2020-08-22T15:51:16Z

lib/taste_tester/ssh_util.rb

+          #        string 1           string2     string3
+          cmd += 'powershell.exe -c -; exit $LASTEXITCODE\''
+          #       ----------------------------------------^
+          #       continued strong3


s/strong3/string3/

I can't imagine we have any desire to have TT itself run on Windows, however, one has to manage their windows system sometime, and that means being able to test on those systems. This adds support for the remote system by generating the right powershell to send over ssh. That means there are two requirements to using this: 1. You have ssh enabled on your Windows PC 2. You set the default shell to powershell instead of cmd Both are easily accomplished with this tiny bit of Chef: ```ruby powershell_package 'ComputerManagementDsc' do action :install end dsc_resource 'install ssh-client' do resource :windowscapability module_name 'ComputerManagementDsc' property :name, 'OpenSSH.Client~~~~0.0.1.0' property :ensure, 'Present' end dsc_resource 'install ssh-server' do resource :windowscapability module_name 'ComputerManagementDsc' property :name, 'OpenSSH.Server~~~~0.0.1.0' property :ensure, 'Present' end dsc_resource 'start sshd' do resource :service property :name, 'sshd' property :startuptype, 'Automatic' property :state, 'Running' property :ensure, 'Present' end ``` This also requires you to be on a version modern enough that symlinks actually work - sorry, we're not re-inventing how taste-tester works for old broken OSes. Getting Windws support here is ugly enough as it is. You may be wondering "but Windows has bash support now!"... and you'd be sorta-right. You can enable WSL and Bash in modern Windows, but you end up in a embedded linux environment. You can access the Windows filesystem, but it's not a thing most people are going to want to do on their windows systems. So, powershell it is. This fully supports tunnels and non-tunnels. As far as I can tell, everything works except "bundle mode" and "local transport", but I don't think those are necessary here. In order to not repeat the *crazy* trans-shell logic, I factored out some code that was repeated (but badly, with bugs - now we *always* specify `-o StrictHostKeyChecking=no` and friends, not just sometimes) between ssh.rb and tunnel.rb into ssh_util.rb. I took this approach because it was the least change, and since that's not directly related to this PR, I wanted to minimize that. However, the long-term solution here is to just roll tunnel.rb into ssh.rb. It already has a `tunnel` option it ignores in the initializer. Signed-off-by: Phil Dibowitz <[email protected]>

HeyItsGilbert

Just nits. Welcome to Powershell! 😂

HeyItsGilbert · 2020-08-24T16:34:41Z

lib/taste_tester/host.rb

+    # Sources must be 'registered' with the Eventlog, so check if we have
+    # registered and register if necessary
+    def create_eventlog_if_needed_cmd
+      get_src = 'Get-EventLog -LogName Application -source taste-tester 2>$null'


This is probably fine. The PowerShell way would be to Pipe this to Out-Null

I just follows the docs on MS's site, they did this. But sure, I'm find with whatever. :)

HeyItsGilbert · 2020-08-24T16:45:42Z

lib/taste_tester/host.rb

+
+        # Remove our tmp file before we write to it or certutil crashes...
+        'if (Test-Path $tmp) { rm $tmp }',
+        'certutil -decode $tmp64 $tmp',


Shelling out to certutil feels wrong. .Net methods are available in PowerShell by default. Maybe something like: [Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($tmp))

So like [Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($b64)) | Out-File Encoding ASCII $tmp -Force

maybe?

Whoops. Missed you were writing ASCII. That works, one minor tweak (assuming the $b64 text is ASCII): [Text.Encoding]::ASCII.GetString([Convert]::FromBase64String($b64)) | Out-File Encoding ASCII $tmp -Force

HeyItsGilbert · 2020-08-24T16:47:14Z

lib/taste_tester/host.rb

+        '$b64 | Out-File -Encoding ASCII $tmp64 -Force',
+
+        # Remove our tmp file before we write to it or certutil crashes...
+        'if (Test-Path $tmp) { rm $tmp }',


rm is an alias for Remove-Item which you might want to add a -Force to. Not sure if we should worry about file locks or if we want it to fail on purpose. If the command failed, would everything stop or would it keep going?

-Force STILL crashes if the file isn't there... hence the test.

HeyItsGilbert · 2020-08-24T16:52:01Z

lib/taste_tester/ssh_util.rb

+
+      def build_ssh_cmd(ssh, command_list)
+        if TasteTester::Config.windows_target
+          # Powershell has no `&&`. So originally we looked into joining the


PowerShell commands have the notion of ErrorAction. You could set this globally with $ErrorActionPreference = Stop or individually by adding $ErrorAction = Stop to individual cmdlets.

yeah I played with that a bit but we also want to catch the "testing by someone else" differently, and I found that I was losing that... I'm sure it can be made ti work...

HeyItsGilbert · 2020-08-24T16:56:29Z

lib/taste_tester/tunnel.rb


+      while (1 -eq 1) {


while ($true)

For some reason that didn't work for me.

HeyItsGilbert · 2020-08-24T16:58:24Z

lib/taste_tester/tunnel.rb

+      # For the record, if you want to play with this, you do so with:
+      #   (gwmi win32_process | ? processid -eq $PID).parentprocessid
+      #
+      # Also note that backtick is a line-continuation marker in powershell.


This is almost always a no-no since it makes it super hard to read. I'll recommend a splat below.

haha I searched for SO LONG on how to break lines in powershell and the only answer I got was "don't"... and then I FINALLY found backtick.

HeyItsGilbert · 2020-08-24T17:00:17Z

lib/taste_tester/tunnel.rb

+      while (1 -eq 1) {
+        if (-Not (Test-Path $ts)) {
+          # if we are here, we know we've created our source
+          Write-EventLog -LogName "Application" -Source "taste-tester" `


$splat = @{ LogName = "Application" Source = "taste-tester" EventID = 5 EntryType = "Information" Message = "Ending tunnel: timestamp file disappeared" } Write-EventLog @splat

facebook-github-bot added the CLA Signed label Aug 22, 2020

jaymzh force-pushed the windoze branch from 13130b3 to bec4172 Compare August 22, 2020 07:06

jaymzh requested review from NaomiReeves, davide125, mikedodge04 and oxo42 August 22, 2020 07:06

jaymzh force-pushed the windoze branch from bec4172 to badba60 Compare August 22, 2020 07:14

jaymzh force-pushed the windoze branch from badba60 to 5472f74 Compare August 22, 2020 07:35

davide125 reviewed Aug 22, 2020

View reviewed changes

jaymzh force-pushed the windoze branch 2 times, most recently from 5a51af7 to e795255 Compare August 24, 2020 12:35

davide125 mentioned this pull request Aug 24, 2020

Drop undocumented awk dependency #147

Open

davide125 approved these changes Aug 24, 2020

View reviewed changes

jaymzh force-pushed the windoze branch from e795255 to 722c7e0 Compare August 24, 2020 15:38

jaymzh merged commit 72302b2 into facebook:master Aug 24, 2020

HeyItsGilbert reviewed Aug 24, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First whack at windows (target) support for taste-tester #144

First whack at windows (target) support for taste-tester #144

jaymzh commented Aug 22, 2020 •

edited

Loading

jaymzh commented Aug 22, 2020 •

edited

Loading

davide125 left a comment

davide125 Aug 22, 2020

davide125 Aug 22, 2020

davide125 Aug 22, 2020

davide125 Aug 22, 2020

davide125 Aug 22, 2020

davide125 Aug 22, 2020

HeyItsGilbert left a comment

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

jaymzh Aug 24, 2020

HeyItsGilbert Aug 24, 2020

First whack at windows (target) support for taste-tester #144

First whack at windows (target) support for taste-tester #144

Conversation

jaymzh commented Aug 22, 2020 • edited Loading

jaymzh commented Aug 22, 2020 • edited Loading

davide125 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HeyItsGilbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaymzh commented Aug 22, 2020 •

edited

Loading

jaymzh commented Aug 22, 2020 •

edited

Loading