Updates windows-wcow runner to be GitHub-hosted vs self-hosted

[natalieparellano]

In theory this would allow us to maintain one less worker

[dfreilich]

Looks like some of the directory permissions also need to be adjusted for the tests

[natalieparellano]

ssh_dialer_test.go is failing with errors such as: ssh_dialer_test.go:400: Expected nil: the cp() function failed to read source file: open testdata\id_ed25519: Access is denied.

Following the code, I believe we may need to update the following file: https://github.com/buildpacks/pack/blob/main/internal/sshdialer/windows_test.go

As I'm not too familiar with this code, I'm unsure how to proceed.

[natalieparellano]

@matejvasek @jromero do you have any advice here?

[matejvasek]

No idea why it cannot be read.

[matejvasek]

I mean I don't see why it would behave differently on this runner.

[matejvasek]

@natalieparellano try

func fixupPrivateKeyMod(path string) {
	err := acl.Chmod(path, 0400)
	if err != nil {
		panic(err)
	}
}

[natalieparellano]

@matejvasek still seeing failure (though not as many?) here: https://github.com/buildpacks/pack/runs/7526815315?check_suite_focus=true#step:11:3891

RUN   TestCreateDialer/sshDialer/use_docker_system_dial-stdio/creates_a_dialer
    ssh_dialer_test.go:424:   any(
        + 	e`Get "http://docker/": command [ssh -i testdata\id_ed25519 -l testuser -p 50138 -- 127.0.0.1 docker system dial-stdio] has exited with exit status 1, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=usage:`...,
          )

[matejvasek]

@matejvasek still seeing failure (though not as many?) here: https://github.com/buildpacks/pack/runs/7526815315?check_suite_focus=true#step:11:3891

RUN   TestCreateDialer/sshDialer/use_docker_system_dial-stdio/creates_a_dialer
    ssh_dialer_test.go:424:   any(
        + 	e`Get "http://docker/": command [ssh -i testdata\id_ed25519 -l testuser -p 50138 -- 127.0.0.1 docker system dial-stdio] has exited with exit status 1, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=usage:`...,
          )

Is there more of the output?

[matejvasek]

@matejvasek still seeing failure (though not as many?) here: https://github.com/buildpacks/pack/runs/7526815315?check_suite_focus=true#step:11:3891

RUN   TestCreateDialer/sshDialer/use_docker_system_dial-stdio/creates_a_dialer
    ssh_dialer_test.go:424:   any(
        + 	e`Get "http://docker/": command [ssh -i testdata\id_ed25519 -l testuser -p 50138 -- 127.0.0.1 docker system dial-stdio] has exited with exit status 1, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=usage:`...,
          )

This may be too caused by access right -- if private key file is "too visible" ssh rejects to use it.

[matejvasek]

This may be too caused by access right -- if private key file is "too visible" ssh rejects to use it.

maybe not, the dial-stdio test uses ssh agent not direct file access.

[matejvasek]

This may be too caused by access right -- if private key file is "too visible" ssh rejects to use it.

maybe not, the dial-stdio test uses ssh agent not direct file access.

actually it's not using agent but file

[matejvasek]

but if it was access right there should be message about it in output

[matejvasek]

=== RUN   TestCreateDialer/sshDialer/use_docker_system_dial-stdio/creates_a_dialer
    ssh_dialer_test.go:428: dial error: Get "http://docker/": command [ssh -i testdata\id_ed25519 -l testuser -p 50044 -- 127.0.0.1 docker system dial-stdio] has exited with exit status 1, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=usage: ssh [-46AaCfGgKkMNnqsTtVvXxYy] [-B bind_interface]
                   [-b bind_address] [-c cipher_spec] [-D [bind_address:]port]
                   [-E log_file] [-e escape_char] [-F configfile] [-I pkcs11]
                   [-i identity_file] [-J [user@]host[:port]] [-L address]
                   [-l login_name] [-m mac_spec] [-O ctl_cmd] [-o option] [-p port]
                   [-Q query_option] [-R address] [-S ctl_path] [-W host:port]
          [-w local_tun[:remote_tun]] destination [command]
        'C:\Program' is not recognized as an internal or external command,
        operable program or batch file.
        

[matejvasek]

diff --git a/internal/sshdialer/ssh_dialer_test.go b/internal/sshdialer/ssh_dialer_test.go
index 9912f853..0c13a05a 100644
--- a/internal/sshdialer/ssh_dialer_test.go
+++ b/internal/sshdialer/ssh_dialer_test.go
@@ -948,7 +948,7 @@ SSH_BIN -o PasswordAuthentication=no -o ConnectTimeout=3 -o UserKnownHostsFile="
 `
        if runtime.GOOS == "windows" {
                sshScript = `@echo off
-SSH_BIN -o PasswordAuthentication=no -o ConnectTimeout=3 -o UserKnownHostsFile=%USERPROFILE%\.ssh\known_hosts %*
+"SSH_BIN" -o PasswordAuthentication=no -o ConnectTimeout=3 -o UserKnownHostsFile=%USERPROFILE%\.ssh\known_hosts %*
 `
        }
        sshScript = strings.ReplaceAll(sshScript, "SSH_BIN", sshAbsPath)

maybe just maybe

[matejvasek]

or maybe %USERPROFILE%\.ssh\known_hosts needs to be quoted

[matejvasek]

quotes are not helping...

[matejvasek]

@natalieparellano any idea what could cause:

is valid, and Docker 18.09 or later is installed on the remote host: stderr=usage: ssh [-46AaCfGgKkMNnqsTtVvXxYy] [-B bind_interface]
2022-07-26T20:46:24.9289157Z                    [-b bind_address] [-c cipher_spec] [-D [bind_address:]port]
2022-07-26T20:46:24.9298806Z                    [-E log_file] [-e escape_char] [-F configfile] [-I pkcs11]
2022-07-26T20:46:24.9300506Z                    [-i identity_file] [-J [user@]host[:port]] [-L address]
2022-07-26T20:46:24.9302009Z                    [-l login_name] [-m mac_spec] [-O ctl_cmd] [-o option] [-p port]
2022-07-26T20:46:24.9302819Z                    [-Q query_option] [-R address] [-S ctl_path] [-W host:port]
2022-07-26T20:46:24.9303538Z                    [-w local_tun[:remote_tun]] destination [command]
2022-07-26T20:46:24.9304113Z         'C:\Program' is not recognized as an internal or external command,
2022-07-26T20:46:24.9304552Z         operable program or batch file.

[matejvasek]

Isn't the test running under some weird user?

[natalieparellano]

@matejvasek thanks for looking into this. I am not sure who the user in this case - I will check.

[matejvasek]

@natalieparellano
try this out:

diff --git a/internal/sshdialer/ssh_dialer_test.go b/internal/sshdialer/ssh_dialer_test.go
index 3872ef54..2bb40d5d 100644
--- a/internal/sshdialer/ssh_dialer_test.go
+++ b/internal/sshdialer/ssh_dialer_test.go
@@ -932,17 +932,9 @@ func (b badAgent) Signers() ([]ssh.Signer, error) {
 func withFixedUpSSHCLI(t *testing.T) func() {
        t.Helper()
 
-       which := "which"
-       if runtime.GOOS == "windows" {
-               which = "where"
-       }
-
-       out, err := exec.Command(which, "ssh").CombinedOutput()
+       sshAbsPath, err := exec.LookPath("ssh")
        th.AssertNil(t, err)
 
-       sshAbsPath := string(out)
-       sshAbsPath = strings.Trim(sshAbsPath, "\r\n")
-
        sshScript := `#!/bin/sh
 "SSH_BIN" -o PasswordAuthentication=no -o ConnectTimeout=3 -o UserKnownHostsFile="$HOME/.ssh/known_hosts" $@
 `

[matejvasek]

@natalieparellano try

func fixupPrivateKeyMod(path string) {
	err := acl.Chmod(path, 0400)
	if err != nil {
		panic(err)
	}
}

Seems this is not correct, it makes key "too visible". We need to revert this, which gets us back.

[matejvasek]

diff --git a/internal/sshdialer/windows_test.go b/internal/sshdialer/windows_test.go
index 304549d9..29ba7319 100644
--- a/internal/sshdialer/windows_test.go
+++ b/internal/sshdialer/windows_test.go
@@ -10,6 +10,7 @@ import (
        "strings"
 
        "github.com/hectane/go-acl"
+       "golang.org/x/sys/windows"
        "gopkg.in/natefinch/npipe.v2"
 )
 
@@ -18,11 +19,17 @@ func fixupPrivateKeyMod(path string) {
        if err != nil {
                panic(err)
        }
+
+       sid, err := windows.StringToSid(usr.Uid)
+       if err != nil {
+               panic(err)
+       }
+
        mode := uint32(0400)
        err = acl.Apply(path,
                true,
                false,
-               acl.GrantName(((mode&0700)<<23)|((mode&0200)<<9), usr.Name))
+               acl.GrantSid(((mode&0700)<<23)|((mode&0200)<<9), sid))
 
        // See https://github.com/hectane/go-acl/issues/1
        if err != nil && err.Error() != "The operation completed successfully." {

[matejvasek]

The name is empty hence the patch above using sid.

[matejvasek]

The sid it not working either 😢

[matejvasek]

maybe Username instead Name would work?

[matejvasek]

Username seems to work.

[matejvasek]

Beside Name -> Username , this is needed.

[codecov]

Codecov Report

Merging #1491 (4fc0140) into main (7ff6918) will increase coverage by 3.84%.
The diff coverage is n/a.

❗ Current head 4fc0140 differs from pull request most recent head de39959. Consider uploading reports for the commit de39959 to get more accurate results

@@            Coverage Diff             @@
##             main    #1491      +/-   ##
==========================================
+ Coverage   77.54%   81.37%   +3.84%     
==========================================
  Files         151      152       +1     
  Lines        9859     9864       +5     
==========================================
+ Hits         7644     8026     +382     
+ Misses       1760     1361     -399     
- Partials      455      477      +22     

Flag	Coverage Δ
os_linux	80.10% <ø> (?)
os_macos	77.58% <ø> (+0.05%)	⬆️
os_windows	80.49% <ø> (?)
unit	81.37% <ø> (+3.84%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[natalieparellano]

Thank you so much @matejvasek !!!