From 2df2c22bd965fbaebc8af37d36d8b629c5c62f59 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:00:28 +0000 Subject: [PATCH 1/7] Add SkipStageEnvVarSet method --- test-structure/test_structure.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test-structure/test_structure.go b/test-structure/test_structure.go index cb126e577..18c2d8592 100644 --- a/test-structure/test_structure.go +++ b/test-structure/test_structure.go @@ -10,12 +10,15 @@ import ( "log" "github.com/gruntwork-io/terratest/files" "fmt" + "strings" ) +const SKIP_STAGE_ENV_VAR_PREFIX = "SKIP_" + // Execute the given test stage (e.g., setup, teardown, validation) if an environment variable of the name // `SKIP_` (e.g., SKIP_teardown) is not set. func RunTestStage(stageName string, logger *log.Logger, stage func()) { - envVarName := fmt.Sprintf("SKIP_%s", stageName) + envVarName := fmt.Sprintf("%s_%s", SKIP_STAGE_ENV_VAR_PREFIX, stageName) if os.Getenv(envVarName) == "" { logger.Printf("The '%s' environment variable is not set, so executing stage '%s'.", envVarName, stageName) stage() @@ -24,6 +27,18 @@ func RunTestStage(stageName string, logger *log.Logger, stage func()) { } } +// Returns true if an environment variable is set instructing Terratest to skip a test stage. This can be an easy way +// to tell if the tests are running in a local dev environment vs a CI server. +func SkipStageEnvVarSet() bool { + for _, environmentVariable := range os.Environ() { + if strings.HasPrefix(environmentVariable, SKIP_STAGE_ENV_VAR_PREFIX) { + return true + } + } + + return false +} + // Serialize and save TerratestOptions into the given folder. This allows you to create TerratestOptions during setup // and to reuse that TerratestOptions later during validation and teardown. func SaveTerratestOptions(t *testing.T, testFolder string, terratestOptions *terratest.TerratestOptions, logger *log.Logger) { From a3a2a3368c102457ec3c39c54710aee384abd2e0 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:27:04 +0000 Subject: [PATCH 2/7] Add proper timeout functionality for SSH --- ssh/ssh.go | 53 +++++++++++++++++++++++++++++++++++++++++++++---- ssh/ssh_test.go | 24 +++++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/ssh/ssh.go b/ssh/ssh.go index 45361d480..661e4fc1c 100644 --- a/ssh/ssh.go +++ b/ssh/ssh.go @@ -6,6 +6,7 @@ import ( "golang.org/x/crypto/ssh" "net" "time" + "fmt" ) type Host struct { @@ -156,7 +157,39 @@ func setupSshSession(sshSession *SshSession) error { func createSshClient(options *SshConnectionOptions) (*ssh.Client, error) { sshClientConfig := createSshClientConfig(options) - return ssh.Dial("tcp", options.ConnectionString(), sshClientConfig) + return DialWithTimeout("tcp", options.ConnectionString(), sshClientConfig) +} + +type DialResponse struct { + Client *ssh.Client + Err error +} + +// In theory, the ssh.Dial method should take into account the Timeout value in ssh.ClientConfig. In practice, it does +// not, and SSH connections can hang for up to 5 minutes or longer! Here, we implement a custom dial method with a +// timeout to prevent our tests from running for much longer than intended. This method will use the Timeout defined +// in the given config. If that Timeout is set to 0, this will just call the ssh.Dial method directly with no additional +// timeout logic. +// +// This is loosely based on: https://github.com/kubernetes/kubernetes/pull/23843 +func DialWithTimeout(network string, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { + if config.Timeout == 0 { + return ssh.Dial(network, addr, config) + } + + responseChannel := make(chan DialResponse, 1) + + go func() { + client, err := ssh.Dial(network, addr, config) + responseChannel <- DialResponse{Client: client, Err: err} + }() + + select { + case response := <- responseChannel: + return response.Client, response.Err + case <- time.After(config.Timeout): + return nil, SshConnectionTimeoutExceeded{Addr: addr, Timeout: config.Timeout} + } } func createSshClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig { @@ -164,9 +197,7 @@ func createSshClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig User: hostOptions.Username, Auth: hostOptions.AuthMethods, // Do not do a host key check, as Terratest is only used for testing, not prod - HostKeyCallback: func(hostname string, remote net.Addr, key ssh.PublicKey) error { - return nil - }, + HostKeyCallback: NoOpHostKeyCallback, // By default, Go does not impose a timeout, so a SSH connection attempt can hang for a LONG time. Timeout: 10 * time.Second, } @@ -174,6 +205,12 @@ func createSshClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig return clientConfig } +// An ssh.HostKeyCallback that does nothing. Only use this when you're sure you don't want to check the host key at all +// (e.g., only for testing and non-production use cases). +func NoOpHostKeyCallback(hostname string, remote net.Addr, key ssh.PublicKey) error { + return nil +} + func createAuthMethodsForHost(host Host) ([]ssh.AuthMethod, error) { signer, err := ssh.ParsePrivateKey([]byte(host.SshKeyPair.PrivateKey)) if err != nil { @@ -182,3 +219,11 @@ func createAuthMethodsForHost(host Host) ([]ssh.AuthMethod, error) { return []ssh.AuthMethod{ssh.PublicKeys(signer)}, nil } + +type SshConnectionTimeoutExceeded struct { + Addr string + Timeout time.Duration +} +func (err SshConnectionTimeoutExceeded) Error() string { + return fmt.Sprintf("SSH Connection Timeout of %s exceeded while trying to connect to %s", err.Timeout, err.Addr) +} \ No newline at end of file diff --git a/ssh/ssh_test.go b/ssh/ssh_test.go index e3b74a144..432991e99 100644 --- a/ssh/ssh_test.go +++ b/ssh/ssh_test.go @@ -9,6 +9,9 @@ import ( "log" "github.com/gruntwork-io/terratest/util" "time" + "golang.org/x/crypto/ssh" + "github.com/stretchr/testify/assert" + "github.com/gruntwork-io/terratest/test-util" ) const TERRAFORM_OUTPUT_PUBLIC_IP = "example_public_ip" @@ -144,4 +147,23 @@ func testSshToPrivateHost(terratestOptions *terratest.TerratestOptions, resource }) return err -} \ No newline at end of file +} + +func TestDialWithTimeoutNoResponse(t *testing.T) { + t.Parallel() + + start := time.Now() + + addr := "127.0.0.1:111" // Nothing should be listening at this address + timeout := 10 * time.Second + config := &ssh.ClientConfig{Timeout: timeout, HostKeyCallback: NoOpHostKeyCallback} + + client, err := DialWithTimeout("tcp", addr, config) + assert.Nil(t, client) + assert.Equal(t, SshConnectionTimeoutExceeded{Addr: addr, Timeout: timeout}, err, "Unexpected error: %v", err) + + end := time.Now() + timeElapsed := end.Sub(start) + + assert.True(t, timeElapsed.Seconds() - timeout.Seconds() > 0) +} From 5192578c03416e0586433d27ba6036de594b7592 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:31:08 +0000 Subject: [PATCH 3/7] Fix SKIP_ formatting --- test-structure/test_structure.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-structure/test_structure.go b/test-structure/test_structure.go index 18c2d8592..770cca9a3 100644 --- a/test-structure/test_structure.go +++ b/test-structure/test_structure.go @@ -18,7 +18,7 @@ const SKIP_STAGE_ENV_VAR_PREFIX = "SKIP_" // Execute the given test stage (e.g., setup, teardown, validation) if an environment variable of the name // `SKIP_` (e.g., SKIP_teardown) is not set. func RunTestStage(stageName string, logger *log.Logger, stage func()) { - envVarName := fmt.Sprintf("%s_%s", SKIP_STAGE_ENV_VAR_PREFIX, stageName) + envVarName := fmt.Sprintf("%s%s", SKIP_STAGE_ENV_VAR_PREFIX, stageName) if os.Getenv(envVarName) == "" { logger.Printf("The '%s' environment variable is not set, so executing stage '%s'.", envVarName, stageName) stage() From 2308d1021d9ce9e2b1399d9005bbf16206727151 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:36:30 +0000 Subject: [PATCH 4/7] Remove unused import --- ssh/ssh_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ssh/ssh_test.go b/ssh/ssh_test.go index 432991e99..283cdbede 100644 --- a/ssh/ssh_test.go +++ b/ssh/ssh_test.go @@ -11,7 +11,6 @@ import ( "time" "golang.org/x/crypto/ssh" "github.com/stretchr/testify/assert" - "github.com/gruntwork-io/terratest/test-util" ) const TERRAFORM_OUTPUT_PUBLIC_IP = "example_public_ip" From 4c8b37aa8932fbdb6f5314835bd340f523585b74 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:48:16 +0000 Subject: [PATCH 5/7] Remove DialWithTimeout --- ssh/ssh.go | 34 +--------------------------------- ssh/ssh_test.go | 21 --------------------- 2 files changed, 1 insertion(+), 54 deletions(-) diff --git a/ssh/ssh.go b/ssh/ssh.go index 661e4fc1c..ccd128dc0 100644 --- a/ssh/ssh.go +++ b/ssh/ssh.go @@ -157,39 +157,7 @@ func setupSshSession(sshSession *SshSession) error { func createSshClient(options *SshConnectionOptions) (*ssh.Client, error) { sshClientConfig := createSshClientConfig(options) - return DialWithTimeout("tcp", options.ConnectionString(), sshClientConfig) -} - -type DialResponse struct { - Client *ssh.Client - Err error -} - -// In theory, the ssh.Dial method should take into account the Timeout value in ssh.ClientConfig. In practice, it does -// not, and SSH connections can hang for up to 5 minutes or longer! Here, we implement a custom dial method with a -// timeout to prevent our tests from running for much longer than intended. This method will use the Timeout defined -// in the given config. If that Timeout is set to 0, this will just call the ssh.Dial method directly with no additional -// timeout logic. -// -// This is loosely based on: https://github.com/kubernetes/kubernetes/pull/23843 -func DialWithTimeout(network string, addr string, config *ssh.ClientConfig) (*ssh.Client, error) { - if config.Timeout == 0 { - return ssh.Dial(network, addr, config) - } - - responseChannel := make(chan DialResponse, 1) - - go func() { - client, err := ssh.Dial(network, addr, config) - responseChannel <- DialResponse{Client: client, Err: err} - }() - - select { - case response := <- responseChannel: - return response.Client, response.Err - case <- time.After(config.Timeout): - return nil, SshConnectionTimeoutExceeded{Addr: addr, Timeout: config.Timeout} - } + return ssh.Dial("tcp", options.ConnectionString(), sshClientConfig) } func createSshClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig { diff --git a/ssh/ssh_test.go b/ssh/ssh_test.go index 283cdbede..78fce1df6 100644 --- a/ssh/ssh_test.go +++ b/ssh/ssh_test.go @@ -9,8 +9,6 @@ import ( "log" "github.com/gruntwork-io/terratest/util" "time" - "golang.org/x/crypto/ssh" - "github.com/stretchr/testify/assert" ) const TERRAFORM_OUTPUT_PUBLIC_IP = "example_public_ip" @@ -147,22 +145,3 @@ func testSshToPrivateHost(terratestOptions *terratest.TerratestOptions, resource return err } - -func TestDialWithTimeoutNoResponse(t *testing.T) { - t.Parallel() - - start := time.Now() - - addr := "127.0.0.1:111" // Nothing should be listening at this address - timeout := 10 * time.Second - config := &ssh.ClientConfig{Timeout: timeout, HostKeyCallback: NoOpHostKeyCallback} - - client, err := DialWithTimeout("tcp", addr, config) - assert.Nil(t, client) - assert.Equal(t, SshConnectionTimeoutExceeded{Addr: addr, Timeout: timeout}, err, "Unexpected error: %v", err) - - end := time.Now() - timeElapsed := end.Sub(start) - - assert.True(t, timeElapsed.Seconds() - timeout.Seconds() > 0) -} From c169df6a7aa08b84fa3cf25b48dceb44073a5f25 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:49:47 +0000 Subject: [PATCH 6/7] Remove unused error --- ssh/ssh.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ssh/ssh.go b/ssh/ssh.go index ccd128dc0..001fe7410 100644 --- a/ssh/ssh.go +++ b/ssh/ssh.go @@ -186,12 +186,4 @@ func createAuthMethodsForHost(host Host) ([]ssh.AuthMethod, error) { } return []ssh.AuthMethod{ssh.PublicKeys(signer)}, nil -} - -type SshConnectionTimeoutExceeded struct { - Addr string - Timeout time.Duration -} -func (err SshConnectionTimeoutExceeded) Error() string { - return fmt.Sprintf("SSH Connection Timeout of %s exceeded while trying to connect to %s", err.Timeout, err.Addr) } \ No newline at end of file From 1eb024635b671098effcfd70c87196d79cc3ecc0 Mon Sep 17 00:00:00 2001 From: Yevgeniy Brikman Date: Wed, 21 Feb 2018 01:54:58 +0000 Subject: [PATCH 7/7] Remove unused import --- ssh/ssh.go | 1 - 1 file changed, 1 deletion(-) diff --git a/ssh/ssh.go b/ssh/ssh.go index 001fe7410..e642380e3 100644 --- a/ssh/ssh.go +++ b/ssh/ssh.go @@ -6,7 +6,6 @@ import ( "golang.org/x/crypto/ssh" "net" "time" - "fmt" ) type Host struct {