From 1c2b469acde4a62d1a600045e6bada0c1caabe63 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Mon, 28 Sep 2020 10:37:08 +0200 Subject: [PATCH 01/18] add retry channel to ssm driver --- builder/amazon/common/ssm_driver.go | 50 +++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 563fa7eab..9c9c75c5d 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "os/exec" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -31,9 +32,10 @@ type SSMDriverConfig struct { type SSMDriver struct { SSMDriverConfig - session *ssm.StartSessionOutput - sessionParams ssm.StartSessionInput - pluginCmdFunc func(context.Context) error + session *ssm.StartSessionOutput + sessionParams ssm.StartSessionInput + pluginCmdFunc func(context.Context) error + retryConnection chan bool } func NewSSMDriver(config SSMDriverConfig) *SSMDriver { @@ -60,6 +62,29 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu return nil, fmt.Errorf("error encountered in starting session for instance %q: %s", aws.StringValue(input.Target), err) } + d.retryConnection = make(chan bool, 1) + // Starts go routine that will keep listening to a retry channel and retry the session creation when needed. + // The log loop will add data to the retry channel whenever a retryable error happens to session. + // TODO @sylviamoss add max retry attempts + // TODO @sylviamoss zero retry times + go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { + retryTimes := 0 + for { + select { + case <-ctx.Done(): + return + case <-driver.retryConnection: + if retryTimes <= 11 { + retryTimes++ + _, err := driver.StartSession(ctx, input) + if err != nil { + return + } + } + } + } + }(ctx, d, input) + d.session = output d.sessionParams = input @@ -118,6 +143,9 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { if output != "" { log.Printf("[ERROR] %s: %s", prefix, output) + if isRetryableError(output) { + d.retryConnection <- true + } } case output, ok := <-stdoutCh: if !ok { @@ -150,8 +178,24 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { return nil } +func isRetryableError(output string) bool { + retryableError := []string{ + "Unable to connect to specified port", + } + + for _, err := range retryableError { + if strings.Contains(output, err) { + return true + } + } + return false +} + // StopSession terminates an active Session Manager session func (d *SSMDriver) StopSession() error { + if d.retryConnection != nil { + close(d.retryConnection) + } if d.session == nil || d.session.SessionId == nil { return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", From b2c7897f583347a353f9a710ed1a6c1f8ed7d7e9 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Mon, 28 Sep 2020 15:17:59 +0200 Subject: [PATCH 02/18] add WaitGroup to avoid data race --- builder/amazon/common/ssm_driver.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 9c9c75c5d..f00ccf7aa 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -7,6 +7,7 @@ import ( "log" "os/exec" "strings" + "sync" "time" "github.com/aws/aws-sdk-go/aws" @@ -32,10 +33,12 @@ type SSMDriverConfig struct { type SSMDriver struct { SSMDriverConfig - session *ssm.StartSessionOutput - sessionParams ssm.StartSessionInput - pluginCmdFunc func(context.Context) error + session *ssm.StartSessionOutput + sessionParams ssm.StartSessionInput + pluginCmdFunc func(context.Context) error + retryConnection chan bool + wg sync.WaitGroup } func NewSSMDriver(config SSMDriverConfig) *SSMDriver { @@ -48,6 +51,8 @@ func NewSSMDriver(config SSMDriverConfig) *SSMDriver { // not wish to manage the session manually calling StopSession on a instance of this driver will terminate the active session // created from calling StartSession. func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { + d.wg.Add(1) + defer d.wg.Done() log.Printf("Starting PortForwarding session to instance %q", aws.StringValue(input.Target)) var output *ssm.StartSessionOutput @@ -76,6 +81,7 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu case <-driver.retryConnection: if retryTimes <= 11 { retryTimes++ + d.wg.Wait() _, err := driver.StartSession(ctx, input) if err != nil { return @@ -193,6 +199,9 @@ func isRetryableError(output string) bool { // StopSession terminates an active Session Manager session func (d *SSMDriver) StopSession() error { + d.wg.Add(1) + defer d.wg.Done() + if d.retryConnection != nil { close(d.retryConnection) } From 8e3f3e514c70081f648fa0ab7fbd9197eb439abe Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Mon, 28 Sep 2020 15:32:59 +0200 Subject: [PATCH 03/18] improve logs --- builder/amazon/common/ssm_driver.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index f00ccf7aa..2627ce0a8 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -70,22 +70,17 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu d.retryConnection = make(chan bool, 1) // Starts go routine that will keep listening to a retry channel and retry the session creation when needed. // The log loop will add data to the retry channel whenever a retryable error happens to session. - // TODO @sylviamoss add max retry attempts - // TODO @sylviamoss zero retry times go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { - retryTimes := 0 for { select { case <-ctx.Done(): return case <-driver.retryConnection: - if retryTimes <= 11 { - retryTimes++ - d.wg.Wait() - _, err := driver.StartSession(ctx, input) - if err != nil { - return - } + d.wg.Wait() + log.Printf("[DEBUG] Retrying to establish SSM connection") + _, err := driver.StartSession(ctx, input) + if err != nil { + return } } } @@ -148,9 +143,11 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { } if output != "" { - log.Printf("[ERROR] %s: %s", prefix, output) if isRetryableError(output) { + log.Printf("[ERROR] Retryable error - %s: %s", prefix, output) d.retryConnection <- true + } else { + log.Printf("[ERROR] %s: %s", prefix, output) } } case output, ok := <-stdoutCh: @@ -186,9 +183,8 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { func isRetryableError(output string) bool { retryableError := []string{ - "Unable to connect to specified port", + "connection refused", } - for _, err := range retryableError { if strings.Contains(output, err) { return true From 1f62249097ddc1a2820e36c060155c811a6fd0a7 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Tue, 29 Sep 2020 12:31:10 +0200 Subject: [PATCH 04/18] add retry terminated session chan --- builder/amazon/common/ssm_driver.go | 66 ++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 2627ce0a8..4eb62c931 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -37,8 +37,9 @@ type SSMDriver struct { sessionParams ssm.StartSessionInput pluginCmdFunc func(context.Context) error - retryConnection chan bool - wg sync.WaitGroup + retryConnection chan bool + retryAfterTermination chan bool + wg sync.WaitGroup } func NewSSMDriver(config SSMDriverConfig) *SSMDriver { @@ -55,33 +56,40 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu defer d.wg.Done() log.Printf("Starting PortForwarding session to instance %q", aws.StringValue(input.Target)) - var output *ssm.StartSessionOutput - err := retry.Config{ - ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, - RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, - }.Run(ctx, func(ctx context.Context) (err error) { - output, err = d.SvcClient.StartSessionWithContext(ctx, &input) - return err - }) + output, err := d.StartSessionWithContext(ctx, input) if err != nil { return nil, fmt.Errorf("error encountered in starting session for instance %q: %s", aws.StringValue(input.Target), err) } d.retryConnection = make(chan bool, 1) + d.retryAfterTermination = make(chan bool, 1) // Starts go routine that will keep listening to a retry channel and retry the session creation when needed. - // The log loop will add data to the retry channel whenever a retryable error happens to session. + // The log polling process will add data to the retry channel whenever a retryable error happens to session. go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { for { select { case <-ctx.Done(): return - case <-driver.retryConnection: - d.wg.Wait() - log.Printf("[DEBUG] Retrying to establish SSM connection") - _, err := driver.StartSession(ctx, input) - if err != nil { + case r := <-driver.retryAfterTermination: + if r { + d.wg.Wait() + log.Printf("[DEBUG] Restablishing SSM connection") + _, _ = driver.StartSession(ctx, input) + // Close channels and end goroutine. Another goroutine will start + // and the channels wil be reopened. + close(driver.retryConnection) + close(driver.retryAfterTermination) return } + case r := <-driver.retryConnection: + if r { + d.wg.Wait() + log.Printf("[DEBUG] Retrying to establish SSM connection") + _, err := driver.StartSessionWithContext(ctx, input) + if err != nil { + return + } + } } } }(ctx, d, input) @@ -100,6 +108,20 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu return d.session, nil } +func (d *SSMDriver) StartSessionWithContext(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { + d.wg.Add(1) + defer d.wg.Done() + var output *ssm.StartSessionOutput + err := retry.Config{ + ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) (err error) { + output, err = d.SvcClient.StartSessionWithContext(ctx, &input) + return err + }) + return output, err +} + func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { args, err := d.Args() if err != nil { @@ -163,6 +185,7 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { if stdoutCh == nil && stderrCh == nil { log.Printf("[DEBUG] %s: %s", prefix, "active session has been terminated; stopping all log polling processes.") + d.retryAfterTermination <- true return } } @@ -198,10 +221,6 @@ func (d *SSMDriver) StopSession() error { d.wg.Add(1) defer d.wg.Done() - if d.retryConnection != nil { - close(d.retryConnection) - } - if d.session == nil || d.session.SessionId == nil { return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", aws.StringValue(d.sessionParams.Target)) @@ -211,6 +230,13 @@ func (d *SSMDriver) StopSession() error { if err != nil { err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", aws.StringValue(d.session.SessionId), err) } + + if d.retryConnection != nil { + close(d.retryConnection) + } + if d.retryAfterTermination != nil { + close(d.retryAfterTermination) + } return err } From aa73cc7d7ef774970313b66b4fb45e44c6fa96f0 Mon Sep 17 00:00:00 2001 From: sylviamoss Date: Tue, 29 Sep 2020 16:15:47 +0200 Subject: [PATCH 05/18] add close chan to avoid unwanted retries --- builder/amazon/common/ssm_driver.go | 30 +++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 4eb62c931..86c5d0d16 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -39,6 +39,7 @@ type SSMDriver struct { retryConnection chan bool retryAfterTermination chan bool + closeRetryPolling chan bool wg sync.WaitGroup } @@ -63,25 +64,28 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu d.retryConnection = make(chan bool, 1) d.retryAfterTermination = make(chan bool, 1) - // Starts go routine that will keep listening to a retry channel and retry the session creation when needed. - // The log polling process will add data to the retry channel whenever a retryable error happens to session. + d.closeRetryPolling = make(chan bool, 1) + // Starts go routine that will keep listening to channels and retry the session creation/connection when needed. + // The log polling process will add data to the channels whenever a retryable error happens to the session or if it's terminated. go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { for { select { case <-ctx.Done(): return + case ok := <-d.closeRetryPolling: + if ok { + return + } case r := <-driver.retryAfterTermination: if r { d.wg.Wait() log.Printf("[DEBUG] Restablishing SSM connection") _, _ = driver.StartSession(ctx, input) - // Close channels and end goroutine. Another goroutine will start - // and the channels wil be reopened. - close(driver.retryConnection) - close(driver.retryAfterTermination) + // End this routine. Another routine will start. return } case r := <-driver.retryConnection: + // Tunnel is still open an we want to try to reconnect if r { d.wg.Wait() log.Printf("[DEBUG] Retrying to establish SSM connection") @@ -111,6 +115,7 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu func (d *SSMDriver) StartSessionWithContext(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { d.wg.Add(1) defer d.wg.Done() + var output *ssm.StartSessionOutput err := retry.Config{ ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, @@ -119,6 +124,7 @@ func (d *SSMDriver) StartSessionWithContext(ctx context.Context, input ssm.Start output, err = d.SvcClient.StartSessionWithContext(ctx, &input) return err }) + return output, err } @@ -221,6 +227,9 @@ func (d *SSMDriver) StopSession() error { d.wg.Add(1) defer d.wg.Done() + // Stop retry polling process to avoid unwanted retries at this point + d.closeRetryPolling <- true + if d.session == nil || d.session.SessionId == nil { return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", aws.StringValue(d.sessionParams.Target)) @@ -231,12 +240,9 @@ func (d *SSMDriver) StopSession() error { err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", aws.StringValue(d.session.SessionId), err) } - if d.retryConnection != nil { - close(d.retryConnection) - } - if d.retryAfterTermination != nil { - close(d.retryAfterTermination) - } + close(d.closeRetryPolling) + close(d.retryAfterTermination) + close(d.retryConnection) return err } From eb11009e2a6571dc684133c080fd798deaa7e4e8 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Wed, 7 Oct 2020 11:26:27 -0400 Subject: [PATCH 06/18] Check for closed channels as opposed to using a separate closeRetry channel --- builder/amazon/common/ssm_driver.go | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 86c5d0d16..cba93740a 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -39,7 +39,6 @@ type SSMDriver struct { retryConnection chan bool retryAfterTermination chan bool - closeRetryPolling chan bool wg sync.WaitGroup } @@ -64,7 +63,6 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu d.retryConnection = make(chan bool, 1) d.retryAfterTermination = make(chan bool, 1) - d.closeRetryPolling = make(chan bool, 1) // Starts go routine that will keep listening to channels and retry the session creation/connection when needed. // The log polling process will add data to the channels whenever a retryable error happens to the session or if it's terminated. go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { @@ -72,11 +70,10 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu select { case <-ctx.Done(): return - case ok := <-d.closeRetryPolling: - if ok { + case r, ok := <-driver.retryAfterTermination: + if !ok { return } - case r := <-driver.retryAfterTermination: if r { d.wg.Wait() log.Printf("[DEBUG] Restablishing SSM connection") @@ -84,7 +81,11 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu // End this routine. Another routine will start. return } - case r := <-driver.retryConnection: + case r, ok := <-driver.retryConnection: + if !ok { + return + } + // Tunnel is still open an we want to try to reconnect if r { d.wg.Wait() @@ -174,9 +175,9 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { if isRetryableError(output) { log.Printf("[ERROR] Retryable error - %s: %s", prefix, output) d.retryConnection <- true - } else { - log.Printf("[ERROR] %s: %s", prefix, output) + continue } + log.Printf("[ERROR] %s: %s", prefix, output) } case output, ok := <-stdoutCh: if !ok { @@ -227,9 +228,6 @@ func (d *SSMDriver) StopSession() error { d.wg.Add(1) defer d.wg.Done() - // Stop retry polling process to avoid unwanted retries at this point - d.closeRetryPolling <- true - if d.session == nil || d.session.SessionId == nil { return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", aws.StringValue(d.sessionParams.Target)) @@ -239,10 +237,10 @@ func (d *SSMDriver) StopSession() error { if err != nil { err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", aws.StringValue(d.session.SessionId), err) } - - close(d.closeRetryPolling) + // Stop retry polling process to avoid unwanted retries at this point close(d.retryAfterTermination) close(d.retryConnection) + return err } From dff9cde775585804029e0841764c52bf2527aabd Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Wed, 7 Oct 2020 11:27:23 -0400 Subject: [PATCH 07/18] Remove waitgroups --- builder/amazon/common/ssm_driver.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index cba93740a..2314cf3f9 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -7,7 +7,6 @@ import ( "log" "os/exec" "strings" - "sync" "time" "github.com/aws/aws-sdk-go/aws" @@ -39,7 +38,6 @@ type SSMDriver struct { retryConnection chan bool retryAfterTermination chan bool - wg sync.WaitGroup } func NewSSMDriver(config SSMDriverConfig) *SSMDriver { @@ -52,8 +50,6 @@ func NewSSMDriver(config SSMDriverConfig) *SSMDriver { // not wish to manage the session manually calling StopSession on a instance of this driver will terminate the active session // created from calling StartSession. func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { - d.wg.Add(1) - defer d.wg.Done() log.Printf("Starting PortForwarding session to instance %q", aws.StringValue(input.Target)) output, err := d.StartSessionWithContext(ctx, input) @@ -75,7 +71,6 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu return } if r { - d.wg.Wait() log.Printf("[DEBUG] Restablishing SSM connection") _, _ = driver.StartSession(ctx, input) // End this routine. Another routine will start. @@ -88,7 +83,6 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu // Tunnel is still open an we want to try to reconnect if r { - d.wg.Wait() log.Printf("[DEBUG] Retrying to establish SSM connection") _, err := driver.StartSessionWithContext(ctx, input) if err != nil { @@ -114,9 +108,6 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu } func (d *SSMDriver) StartSessionWithContext(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { - d.wg.Add(1) - defer d.wg.Done() - var output *ssm.StartSessionOutput err := retry.Config{ ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, @@ -225,9 +216,6 @@ func isRetryableError(output string) bool { // StopSession terminates an active Session Manager session func (d *SSMDriver) StopSession() error { - d.wg.Add(1) - defer d.wg.Done() - if d.session == nil || d.session.SessionId == nil { return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", aws.StringValue(d.sessionParams.Target)) From 646b973bd370c501fd075e9b1c208c34deb8600d Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 27 Oct 2020 16:05:02 -0400 Subject: [PATCH 08/18] Remove logic to retry a connection that reuses an existing SSM Session After testing it was found that once an session is terminated via an instance restart, console termination, or SSM agent restart. Any active session will essentially be terminated and unusable. So knowing that it is always best to start a new session and let the old one timeout get terminated. --- builder/amazon/common/ssm_driver.go | 71 +++++++++++------------------ 1 file changed, 26 insertions(+), 45 deletions(-) diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 2314cf3f9..9b95b6fe3 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -6,7 +6,6 @@ import ( "fmt" "log" "os/exec" - "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -36,7 +35,6 @@ type SSMDriver struct { sessionParams ssm.StartSessionInput pluginCmdFunc func(context.Context) error - retryConnection chan bool retryAfterTermination chan bool } @@ -57,7 +55,6 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu return nil, fmt.Errorf("error encountered in starting session for instance %q: %s", aws.StringValue(input.Target), err) } - d.retryConnection = make(chan bool, 1) d.retryAfterTermination = make(chan bool, 1) // Starts go routine that will keep listening to channels and retry the session creation/connection when needed. // The log polling process will add data to the channels whenever a retryable error happens to the session or if it's terminated. @@ -71,24 +68,11 @@ func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInpu return } if r { - log.Printf("[DEBUG] Restablishing SSM connection") + log.Printf("[DEBUG] Attempting to restablishing SSM connection") _, _ = driver.StartSession(ctx, input) // End this routine. Another routine will start. return } - case r, ok := <-driver.retryConnection: - if !ok { - return - } - - // Tunnel is still open an we want to try to reconnect - if r { - log.Printf("[DEBUG] Retrying to establish SSM connection") - _, err := driver.StartSessionWithContext(ctx, input) - if err != nil { - return - } - } } } }(ctx, d, input) @@ -156,34 +140,40 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { select { case <-ctx.Done(): return - case output, ok := <-stderrCh: + case errorOut, ok := <-stderrCh: if !ok { stderrCh = nil break } - if output != "" { - if isRetryableError(output) { - log.Printf("[ERROR] Retryable error - %s: %s", prefix, output) - d.retryConnection <- true - continue - } - log.Printf("[ERROR] %s: %s", prefix, output) + if errorOut == "" { + continue } + + log.Printf("[ERROR] %s: %s", prefix, errorOut) case output, ok := <-stdoutCh: if !ok { stdoutCh = nil break } - if output != "" { - log.Printf("[DEBUG] %s: %s", prefix, output) + if output == "" { + continue } + log.Printf("[DEBUG] %s: %s", prefix, output) } if stdoutCh == nil && stderrCh == nil { + // It's possible that a remote SSM session was terminated prematurely due to a system reboot, or a termination + // on the AWS console, or that the SSMAgent service was stopped on the instance. This will try to reconnect + // until the build eventually fails or hits the ssh_timeout threshold. log.Printf("[DEBUG] %s: %s", prefix, "active session has been terminated; stopping all log polling processes.") - d.retryAfterTermination <- true + + // A call to StopSession will nullify any active sessions so only retry if StopSession has not be called, + // since StopSession will also close the retryAfterTermination channel + if d.session != nil { + d.retryAfterTermination <- true + } return } } @@ -202,18 +192,6 @@ func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { return nil } -func isRetryableError(output string) bool { - retryableError := []string{ - "connection refused", - } - for _, err := range retryableError { - if strings.Contains(output, err) { - return true - } - } - return false -} - // StopSession terminates an active Session Manager session func (d *SSMDriver) StopSession() error { if d.session == nil || d.session.SessionId == nil { @@ -221,13 +199,16 @@ func (d *SSMDriver) StopSession() error { aws.StringValue(d.sessionParams.Target)) } - _, err := d.SvcClient.TerminateSession(&ssm.TerminateSessionInput{SessionId: d.session.SessionId}) - if err != nil { - err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", aws.StringValue(d.session.SessionId), err) - } + // Store session id before we nullify any active sessions + sid := aws.StringValue(d.session.SessionId) + // Stop retry polling process to avoid unwanted retries at this point + d.session = nil close(d.retryAfterTermination) - close(d.retryConnection) + _, err := d.SvcClient.TerminateSession(&ssm.TerminateSessionInput{SessionId: aws.String(sid)}) + if err != nil { + err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", sid, err) + } return err } From 8e355d0fe718596a56016e2bb03d1b1f9ccc8ee4 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 11:48:43 +0100 Subject: [PATCH 09/18] Move ssm code to its own ssm package and make it singlethreaded --- builder/amazon/common/ssm/driver.go | 111 +++++++++ builder/amazon/common/ssm_driver.go | 211 ------------------ builder/amazon/common/ssm_mock_funcs.go | 1 + .../amazon/common/step_create_ssm_tunnel.go | 34 ++- builder/amazon/error/utils.go | 18 ++ go.mod | 2 - go.sum | 11 - 7 files changed, 152 insertions(+), 236 deletions(-) create mode 100644 builder/amazon/common/ssm/driver.go create mode 100644 builder/amazon/error/utils.go diff --git a/builder/amazon/common/ssm/driver.go b/builder/amazon/common/ssm/driver.go new file mode 100644 index 000000000..79e662eb3 --- /dev/null +++ b/builder/amazon/common/ssm/driver.go @@ -0,0 +1,111 @@ +package ssm + +import ( + "context" + "encoding/json" + "fmt" + "log" + "os/exec" + "strings" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/ssm" + "github.com/aws/aws-sdk-go/service/ssm/ssmiface" + "github.com/hashicorp/packer/common/retry" + "github.com/hashicorp/packer/helper/builder/localexec" + "github.com/hashicorp/packer/packer" +) + +type Session struct { + SvcClient ssmiface.SSMAPI + Region string + Input ssm.StartSessionInput +} + +// Returns true if the error matches all these conditions: +// * err is of type awserr.Error +// * Error.Code() matches code +// * Error.Message() contains message +func isAWSErr(err error, code string, message string) bool { + if err, ok := err.(awserr.Error); ok { + return err.Code() == code && strings.Contains(err.Message(), message) + } + return false +} + +// getCommand return a valid ordered set of arguments to pass to the driver command. +func (s Session) getCommand(ctx context.Context) ([]string, string, error) { + var session *ssm.StartSessionOutput + err := retry.Config{ + ShouldRetry: func(err error) bool { return isAWSErr(err, "TargetNotConnected", "") }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) (err error) { + session, err = s.SvcClient.StartSessionWithContext(ctx, &s.Input) + return err + }) + + if err != nil { + return nil, "", err + } + + if session == nil { + return nil, "", fmt.Errorf("an active Amazon SSM Session is required before trying to open a session tunnel") + } + + // AWS session-manager-plugin requires a valid session be passed in JSON. + sessionDetails, err := json.Marshal(session) + if err != nil { + return nil, *session.SessionId, fmt.Errorf("error encountered in reading session details %s", err) + } + + // AWS session-manager-plugin requires the parameters used in the session to be passed in JSON as well. + sessionParameters, err := json.Marshal(s.Input) + if err != nil { + return nil, "", fmt.Errorf("error encountered in reading session parameter details %s", err) + } + + // Args must be in this order + args := []string{ + string(sessionDetails), + s.Region, + "StartSession", + "", // ProfileName + string(sessionParameters), + *session.StreamUrl, + } + return args, *session.SessionId, nil +} + +// Start an interactive Systems Manager session with a remote instance via the +// AWS session-manager-plugin. To terminate the session you must cancell the +// context. If you do not wish to terminate the session manually: calling +// StopSession on a instance of this driver will terminate the active session +// created from calling StartSession. +func (s Session) Start(ctx context.Context, ui packer.Ui) error { + for ctx.Err() == nil { + log.Printf("ssm: Starting PortForwarding session to instance %q", *s.Input.Target) + args, sessionID, err := s.getCommand(ctx) + if sessionID != "" { + defer func() { + _, err := s.SvcClient.TerminateSession(&ssm.TerminateSessionInput{SessionId: aws.String(sessionID)}) + if err != nil { + err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", sessionID, err) + } + }() + } + if err != nil { + return err + } + + cmd := exec.CommandContext(ctx, "session-manager-plugin", args...) + + ui.Message(fmt.Sprintf("Starting portForwarding session %q.", sessionID)) + err = localexec.RunAndStream(cmd, ui, nil) + if err != nil { + ui.Error(err.Error()) + } + } + return nil +} diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go index 9b95b6fe3..2a8dfbd53 100644 --- a/builder/amazon/common/ssm_driver.go +++ b/builder/amazon/common/ssm_driver.go @@ -2,17 +2,9 @@ package common import ( "context" - "encoding/json" - "fmt" - "log" - "os/exec" - "time" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/aws-sdk-go/service/ssm/ssmiface" - "github.com/hashicorp/packer/common/retry" - "github.com/mitchellh/iochan" ) const ( @@ -34,212 +26,9 @@ type SSMDriver struct { session *ssm.StartSessionOutput sessionParams ssm.StartSessionInput pluginCmdFunc func(context.Context) error - - retryAfterTermination chan bool } func NewSSMDriver(config SSMDriverConfig) *SSMDriver { d := SSMDriver{SSMDriverConfig: config} return &d } - -// StartSession starts an interactive Systems Manager session with a remote instance via the AWS session-manager-plugin -// This ssm.StartSessionOutput returned by this function can be used for terminating the session manually. If you do -// not wish to manage the session manually calling StopSession on a instance of this driver will terminate the active session -// created from calling StartSession. -func (d *SSMDriver) StartSession(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { - log.Printf("Starting PortForwarding session to instance %q", aws.StringValue(input.Target)) - - output, err := d.StartSessionWithContext(ctx, input) - if err != nil { - return nil, fmt.Errorf("error encountered in starting session for instance %q: %s", aws.StringValue(input.Target), err) - } - - d.retryAfterTermination = make(chan bool, 1) - // Starts go routine that will keep listening to channels and retry the session creation/connection when needed. - // The log polling process will add data to the channels whenever a retryable error happens to the session or if it's terminated. - go func(ctx context.Context, driver *SSMDriver, input ssm.StartSessionInput) { - for { - select { - case <-ctx.Done(): - return - case r, ok := <-driver.retryAfterTermination: - if !ok { - return - } - if r { - log.Printf("[DEBUG] Attempting to restablishing SSM connection") - _, _ = driver.StartSession(ctx, input) - // End this routine. Another routine will start. - return - } - } - } - }(ctx, d, input) - - d.session = output - d.sessionParams = input - - if d.pluginCmdFunc == nil { - d.pluginCmdFunc = d.openTunnelForSession - } - - if err := d.pluginCmdFunc(ctx); err != nil { - return nil, fmt.Errorf("error encountered in starting session for instance %q: %s", aws.StringValue(input.Target), err) - } - - return d.session, nil -} - -func (d *SSMDriver) StartSessionWithContext(ctx context.Context, input ssm.StartSessionInput) (*ssm.StartSessionOutput, error) { - var output *ssm.StartSessionOutput - err := retry.Config{ - ShouldRetry: func(err error) bool { return IsAWSErr(err, "TargetNotConnected", "") }, - RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, - }.Run(ctx, func(ctx context.Context) (err error) { - output, err = d.SvcClient.StartSessionWithContext(ctx, &input) - return err - }) - - return output, err -} - -func (d *SSMDriver) openTunnelForSession(ctx context.Context) error { - args, err := d.Args() - if err != nil { - return fmt.Errorf("error encountered validating session details: %s", err) - } - - cmd := exec.CommandContext(ctx, sessionManagerPluginName, args...) - - // Let's build up our logging - stdout, err := cmd.StdoutPipe() - if err != nil { - return err - } - stderr, err := cmd.StderrPipe() - if err != nil { - return err - } - - // Create the channels we'll use for data - stdoutCh := iochan.DelimReader(stdout, '\n') - stderrCh := iochan.DelimReader(stderr, '\n') - - /* Loop and get all our output - This particular logger will continue to run through an entire Packer run. - The decision to continue logging is due to the fact that session-manager-plugin - doesn't give a good way of knowing if the command failed or was successful other - than looking at the logs. Seeing as the plugin is updated frequently and that the - log information is a bit sparse this logger will indefinitely relying on other - steps to fail if the tunnel is unable to be created. If successful then the user - will get more information on the tunnel connection when running in a debug mode. - */ - go func(ctx context.Context, prefix string) { - for { - select { - case <-ctx.Done(): - return - case errorOut, ok := <-stderrCh: - if !ok { - stderrCh = nil - break - } - - if errorOut == "" { - continue - } - - log.Printf("[ERROR] %s: %s", prefix, errorOut) - case output, ok := <-stdoutCh: - if !ok { - stdoutCh = nil - break - } - - if output == "" { - continue - } - log.Printf("[DEBUG] %s: %s", prefix, output) - } - - if stdoutCh == nil && stderrCh == nil { - // It's possible that a remote SSM session was terminated prematurely due to a system reboot, or a termination - // on the AWS console, or that the SSMAgent service was stopped on the instance. This will try to reconnect - // until the build eventually fails or hits the ssh_timeout threshold. - log.Printf("[DEBUG] %s: %s", prefix, "active session has been terminated; stopping all log polling processes.") - - // A call to StopSession will nullify any active sessions so only retry if StopSession has not be called, - // since StopSession will also close the retryAfterTermination channel - if d.session != nil { - d.retryAfterTermination <- true - } - return - } - } - }(ctx, sessionManagerPluginName) - - log.Printf("[DEBUG %s] opening session tunnel to instance %q for session %q", sessionManagerPluginName, - aws.StringValue(d.sessionParams.Target), - aws.StringValue(d.session.SessionId), - ) - - if err := cmd.Start(); err != nil { - err = fmt.Errorf("error encountered when calling %s: %s\n", sessionManagerPluginName, err) - return err - } - - return nil -} - -// StopSession terminates an active Session Manager session -func (d *SSMDriver) StopSession() error { - if d.session == nil || d.session.SessionId == nil { - return fmt.Errorf("Unable to find a valid session to instance %q; skipping the termination step", - aws.StringValue(d.sessionParams.Target)) - } - - // Store session id before we nullify any active sessions - sid := aws.StringValue(d.session.SessionId) - - // Stop retry polling process to avoid unwanted retries at this point - d.session = nil - close(d.retryAfterTermination) - _, err := d.SvcClient.TerminateSession(&ssm.TerminateSessionInput{SessionId: aws.String(sid)}) - if err != nil { - err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", sid, err) - } - - return err -} - -// Args validates the driver inputs before returning an ordered set of arguments to pass to the driver command. -func (d *SSMDriver) Args() ([]string, error) { - if d.session == nil { - return nil, fmt.Errorf("an active Amazon SSM Session is required before trying to open a session tunnel") - } - - // AWS session-manager-plugin requires a valid session be passed in JSON. - sessionDetails, err := json.Marshal(d.session) - if err != nil { - return nil, fmt.Errorf("error encountered in reading session details %s", err) - } - - // AWS session-manager-plugin requires the parameters used in the session to be passed in JSON as well. - sessionParameters, err := json.Marshal(d.sessionParams) - if err != nil { - return nil, fmt.Errorf("error encountered in reading session parameter details %s", err) - } - - // Args must be in this order - args := []string{ - string(sessionDetails), - d.Region, - sessionCommand, - d.ProfileName, - string(sessionParameters), - d.SvcEndpoint, - } - - return args, nil -} diff --git a/builder/amazon/common/ssm_mock_funcs.go b/builder/amazon/common/ssm_mock_funcs.go index 1d358baaa..8ffb06a25 100644 --- a/builder/amazon/common/ssm_mock_funcs.go +++ b/builder/amazon/common/ssm_mock_funcs.go @@ -21,6 +21,7 @@ func (svc *MockSSMSvc) StartSessionWithContext(ctx aws.Context, input *ssm.Start svc.StartSessionCalled = true return MockStartSessionOutput(), svc.StartSessionError } + func (svc *MockSSMSvc) TerminateSession(input *ssm.TerminateSessionInput) (*ssm.TerminateSessionOutput, error) { svc.TerminateSessionCalled = true return new(ssm.TerminateSessionOutput), svc.TerminateSessionError diff --git a/builder/amazon/common/step_create_ssm_tunnel.go b/builder/amazon/common/step_create_ssm_tunnel.go index c29a4dc5b..4149acf4c 100644 --- a/builder/amazon/common/step_create_ssm_tunnel.go +++ b/builder/amazon/common/step_create_ssm_tunnel.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ssm" + pssm "github.com/hashicorp/packer/builder/amazon/common/ssm" "github.com/hashicorp/packer/common/net" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -24,6 +25,7 @@ type StepCreateSSMTunnel struct { instanceId string PauseBeforeSSM time.Duration driver *SSMDriver + stopSSMCommand func() } // Run executes the Packer build step that creates a session tunnel. @@ -73,30 +75,38 @@ func (s *StepCreateSSMTunnel) Run(ctx context.Context, state multistep.StateBag) driver := SSMDriver{SSMDriverConfig: cfg} s.driver = &driver } + state.Put("sessionPort", s.LocalPortNumber) input := s.BuildTunnelInputForInstance(s.instanceId) - _, err := s.driver.StartSession(ctx, input) - if err != nil { - err = fmt.Errorf("error encountered in establishing a tunnel %s", err) - ui.Error(err.Error()) - state.Put("error", err) - return multistep.ActionHalt - } - ui.Message(fmt.Sprintf("PortForwarding session %q has been started", s.instanceId)) - state.Put("sessionPort", s.LocalPortNumber) + ssmCtx, ssmCancel := context.WithCancel(ctx) + s.stopSSMCommand = ssmCancel + + go func() { + err := pssm.Session{ + SvcClient: s.driver.SvcClient, + Input: input, + Region: s.driver.Region, + }.Start(ssmCtx, ui) + + if err != nil { + err = fmt.Errorf("error encountered in establishing a tunnel %s", err) + ui.Error(err.Error()) + state.Put("error", err) + } + }() + return multistep.ActionContinue } // Cleanup terminates an active session on AWS, which in turn terminates the associated tunnel process running on the local machine. func (s *StepCreateSSMTunnel) Cleanup(state multistep.StateBag) { - ui := state.Get("ui").(packer.Ui) if !s.SSMAgentEnabled { return } - if err := s.driver.StopSession(); err != nil { - ui.Error(err.Error()) + if s.stopSSMCommand != nil { + s.stopSSMCommand() } } diff --git a/builder/amazon/error/utils.go b/builder/amazon/error/utils.go new file mode 100644 index 000000000..dcf98a825 --- /dev/null +++ b/builder/amazon/error/utils.go @@ -0,0 +1,18 @@ +package error + +import ( + "strings" + + "github.com/aws/aws-sdk-go/aws/awserr" +) + +// Returns true if the err matches all these conditions: +// * err is of type awserr.Error +// * Error.Code() matches code +// * Error.Message() contains message +func Matches(err error, code string, message string) bool { + if err, ok := err.(awserr.Error); ok { + return err.Code() == code && strings.Contains(err.Message(), message) + } + return false +} diff --git a/go.mod b/go.mod index 2ef26cf09..c53e9c9d8 100644 --- a/go.mod +++ b/go.mod @@ -93,7 +93,6 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/mitchellh/go-testing-interface v1.0.3 // indirect github.com/mitchellh/go-vnc v0.0.0-20150629162542-723ed9867aed - github.com/mitchellh/gox v1.0.1 // indirect github.com/mitchellh/iochan v1.0.0 github.com/mitchellh/mapstructure v1.2.3 github.com/mitchellh/panicwrap v1.0.0 @@ -104,7 +103,6 @@ require ( github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d // indirect github.com/olekukonko/tablewriter v0.0.0-20180105111133-96aac992fc8b github.com/oracle/oci-go-sdk v18.0.0+incompatible - github.com/outscale/osc-go v0.0.1 // indirect github.com/outscale/osc-sdk-go/osc v0.0.0-20200722135656-d654809d0699 github.com/packer-community/winrmcp v0.0.0-20180921204643-0fd363d6159a github.com/pierrec/lz4 v2.0.5+incompatible diff --git a/go.sum b/go.sum index b4afda482..8a908afec 100644 --- a/go.sum +++ b/go.sum @@ -126,7 +126,6 @@ github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM= -github.com/aws/aws-sdk-go v1.16.22/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.26.3/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/aws/aws-sdk-go v1.30.8 h1:4BHbh8K3qKmcnAgToZ2LShldRF9inoqIBccpCLNCy3I= github.com/aws/aws-sdk-go v1.30.8/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= @@ -377,7 +376,6 @@ github.com/hashicorp/go-uuid v1.0.1 h1:fv1ep09latC32wFoVwnqcnKJGnMSdBanPczbHAYm1 github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE= github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= -github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.1.0 h1:bPIoEKD27tNdebFGGxxYwcL4nepeY4j1QP23PFRGzg0= github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.2.0 h1:3vNe/fWF5CBgRIguda1meWhsZHy3m8gCJ5wx+dIzX/E= @@ -512,8 +510,6 @@ github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZX github.com/mitchellh/go-wordwrap v1.0.0 h1:6GlHJ/LTGMrIJbwgdqdl2eEH8o+Exx/0m8ir9Gns0u4= github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo= github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS42BGNg= -github.com/mitchellh/gox v1.0.1 h1:x0jD3dcHk9a9xPSDN6YEL4xL6Qz0dvNYm8yZqui5chI= -github.com/mitchellh/gox v1.0.1/go.mod h1:ED6BioOGXMswlXa2zxfh/xdd5QhwYliBFn9V18Ap4z4= github.com/mitchellh/iochan v1.0.0 h1:C+X3KsSTLFVBr/tK1eYN/vs4rJcvsiLU338UhYPJWeY= github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY= github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= @@ -538,9 +534,6 @@ github.com/olekukonko/tablewriter v0.0.0-20180105111133-96aac992fc8b/go.mod h1:v github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/oracle/oci-go-sdk v18.0.0+incompatible h1:FLV4KixsVfF3rwyVTMI6Ryp/Q+OSb9sR5TawbfjFLN4= github.com/oracle/oci-go-sdk v18.0.0+incompatible/go.mod h1:VQb79nF8Z2cwLkLS35ukwStZIg5F66tcBccjip/j888= -github.com/outscale/osc-go v0.0.1 h1:hvBtORyu7sWSKW1norGlfIP8C7c2aegI2Vkq75SRPCE= -github.com/outscale/osc-go v0.0.1/go.mod h1:hJLmXzqU/t07qQYh90I0TqZzu9s85Zs6FMrxk3ukiFM= -github.com/outscale/osc-sdk-go v1.2.0 h1:1HKr6OMLLVW4w6KQuiQwYZjhNaVz9mNzy/W3KW+zgnA= github.com/outscale/osc-sdk-go/osc v0.0.0-20200722135656-d654809d0699 h1:SHe9i7h5cHe+cB77fQ6lsEgIwKg3ckNU90P03CjGMnI= github.com/outscale/osc-sdk-go/osc v0.0.0-20200722135656-d654809d0699/go.mod h1:5AqqNH1X8zCHescKVlpSHRzrat1KCKDXqZoQPe8fY3A= github.com/packer-community/winrmcp v0.0.0-20180921204643-0fd363d6159a h1:A3QMuteviunoaY/8ex+RKFqwhcZJ/Cf3fCW3IwL2wx4= @@ -636,12 +629,8 @@ github.com/vmware/govmomi v0.23.1/go.mod h1:Y+Wq4lst78L85Ge/F8+ORXIWiKYqaro1vhAu github.com/vmware/vmw-guestinfo v0.0.0-20170707015358-25eff159a728/go.mod h1:x9oS4Wk2s2u4tS29nEaDLdzvuHdB19CvSGJjPgkZJNk= github.com/xanzy/go-cloudstack v0.0.0-20190526095453-42f262b63ed0 h1:NJrcIkdzq0C3I8ypAZwFE9RHtGbfp+mJvqIcoFATZuk= github.com/xanzy/go-cloudstack v0.0.0-20190526095453-42f262b63ed0/go.mod h1:sBh287mCRwCz6zyXHMmw7sSZGPohVpnx+o+OY4M+i3A= -github.com/yandex-cloud/go-genproto v0.0.0-20200608085315-d6e7ef5ceb97 h1:DoqSUxQkBLislVgA1qkM0u7g04It4VRMidyLBH/O/as= -github.com/yandex-cloud/go-genproto v0.0.0-20200608085315-d6e7ef5ceb97/go.mod h1:HEUYX/p8966tMUHHT+TsS0hF/Ca/NYwqprC5WXSDMfE= github.com/yandex-cloud/go-genproto v0.0.0-20200915125933-33de72a328bd h1:o4pvS7D4OErKOM6y+/q6IfOa65OaentKbEDh1ABirE8= github.com/yandex-cloud/go-genproto v0.0.0-20200915125933-33de72a328bd/go.mod h1:HEUYX/p8966tMUHHT+TsS0hF/Ca/NYwqprC5WXSDMfE= -github.com/yandex-cloud/go-sdk v0.0.0-20200610100221-ae86895efb97 h1:8KwSw9xtQBeyeX1EpOlOjRc0JaHlh8B8GglKA6iXt08= -github.com/yandex-cloud/go-sdk v0.0.0-20200610100221-ae86895efb97/go.mod h1:3p2xVpQrHyPxV4UCKnKozt9n+g1LRENOQ33CH8rqLnY= github.com/yandex-cloud/go-sdk v0.0.0-20200921111412-ef15ded2014c h1:LJrgyICodRAgtBvOO2eCbhDDIoaJgeLa1tGQecqW9ac= github.com/yandex-cloud/go-sdk v0.0.0-20200921111412-ef15ded2014c/go.mod h1:Zn/U9YKH0w8n83ezLps5eB6Jftc4gSoZWxVR8hgXgoY= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= From b058de072ad4392878c6f6c19b43c6ee5b44d905 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 11:57:14 +0100 Subject: [PATCH 10/18] move packer/builder/amazon/common.IsAWSErr to builder/amazon/common/awserrors.Matches to avoid cyclic dependency issues --- builder/amazon/common/access_config.go | 3 ++- .../{error => common/awserrors}/utils.go | 2 +- builder/amazon/common/helper_funcs.go | 18 +++--------------- builder/amazon/common/ssm/driver.go | 16 ++-------------- builder/amazon/common/step_create_tags.go | 3 ++- builder/amazon/common/step_pre_validate.go | 5 +++-- .../amazon/common/step_run_source_instance.go | 9 +++++---- .../amazon/common/step_run_spot_instance.go | 3 ++- .../amazon/common/step_stop_ebs_instance.go | 3 ++- builder/amazon/ebs/step_create_ami.go | 3 ++- 10 files changed, 24 insertions(+), 41 deletions(-) rename builder/amazon/{error => common/awserrors}/utils.go (95%) diff --git a/builder/amazon/common/access_config.go b/builder/amazon/common/access_config.go index 6ebd372d3..cd6aafb44 100644 --- a/builder/amazon/common/access_config.go +++ b/builder/amazon/common/access_config.go @@ -17,6 +17,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2/ec2iface" awsbase "github.com/hashicorp/aws-sdk-go-base" cleanhttp "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/template/interpolate" vaultapi "github.com/hashicorp/vault/api" ) @@ -271,7 +272,7 @@ func (c *AccessConfig) Session() (*session.Session, error) { cp, err := c.session.Config.Credentials.Get() - if IsAWSErr(err, "NoCredentialProviders", "") { + if awserrors.Matches(err, "NoCredentialProviders", "") { return nil, c.NewNoValidCredentialSourcesError(err) } diff --git a/builder/amazon/error/utils.go b/builder/amazon/common/awserrors/utils.go similarity index 95% rename from builder/amazon/error/utils.go rename to builder/amazon/common/awserrors/utils.go index dcf98a825..676ef9b4a 100644 --- a/builder/amazon/error/utils.go +++ b/builder/amazon/common/awserrors/utils.go @@ -1,4 +1,4 @@ -package error +package awserrors import ( "strings" diff --git a/builder/amazon/common/helper_funcs.go b/builder/amazon/common/helper_funcs.go index 5f75e2568..12dfe715e 100644 --- a/builder/amazon/common/helper_funcs.go +++ b/builder/amazon/common/helper_funcs.go @@ -4,12 +4,11 @@ import ( "context" "fmt" "log" - "strings" "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" ) @@ -31,7 +30,7 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - return IsAWSErr(err, "UnauthorizedOperation", "") + return awserrors.Matches(err, "UnauthorizedOperation", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { @@ -54,7 +53,7 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - return IsAWSErr(err, "UnauthorizedOperation", "") + return awserrors.Matches(err, "UnauthorizedOperation", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { @@ -74,14 +73,3 @@ func DestroyAMIs(imageids []*string, ec2conn *ec2.EC2) error { } return nil } - -// Returns true if the error matches all these conditions: -// * err is of type awserr.Error -// * Error.Code() matches code -// * Error.Message() contains message -func IsAWSErr(err error, code string, message string) bool { - if err, ok := err.(awserr.Error); ok { - return err.Code() == code && strings.Contains(err.Message(), message) - } - return false -} diff --git a/builder/amazon/common/ssm/driver.go b/builder/amazon/common/ssm/driver.go index 79e662eb3..5c8ad9819 100644 --- a/builder/amazon/common/ssm/driver.go +++ b/builder/amazon/common/ssm/driver.go @@ -6,13 +6,12 @@ import ( "fmt" "log" "os/exec" - "strings" "time" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/aws-sdk-go/service/ssm/ssmiface" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/builder/localexec" "github.com/hashicorp/packer/packer" @@ -24,22 +23,11 @@ type Session struct { Input ssm.StartSessionInput } -// Returns true if the error matches all these conditions: -// * err is of type awserr.Error -// * Error.Code() matches code -// * Error.Message() contains message -func isAWSErr(err error, code string, message string) bool { - if err, ok := err.(awserr.Error); ok { - return err.Code() == code && strings.Contains(err.Message(), message) - } - return false -} - // getCommand return a valid ordered set of arguments to pass to the driver command. func (s Session) getCommand(ctx context.Context) ([]string, string, error) { var session *ssm.StartSessionOutput err := retry.Config{ - ShouldRetry: func(err error) bool { return isAWSErr(err, "TargetNotConnected", "") }, + ShouldRetry: func(err error) bool { return awserrors.Matches(err, "TargetNotConnected", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) (err error) { session, err = s.SvcClient.StartSessionWithContext(ctx, &s.Input) diff --git a/builder/amazon/common/step_create_tags.go b/builder/amazon/common/step_create_tags.go index 15ed105d0..4805b1255 100644 --- a/builder/amazon/common/step_create_tags.go +++ b/builder/amazon/common/step_create_tags.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -91,7 +92,7 @@ func (s *StepCreateTags) Run(ctx context.Context, state multistep.StateBag) mult // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if IsAWSErr(err, "InvalidAMIID.NotFound", "") || IsAWSErr(err, "InvalidSnapshot.NotFound", "") { + if awserrors.Matches(err, "InvalidAMIID.NotFound", "") || awserrors.Matches(err, "InvalidSnapshot.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_pre_validate.go b/builder/amazon/common/step_pre_validate.go index b86e04f61..94ea7af57 100644 --- a/builder/amazon/common/step_pre_validate.go +++ b/builder/amazon/common/step_pre_validate.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -39,7 +40,7 @@ func (s *StepPreValidate) Run(ctx context.Context, state multistep.StateBag) mul err := retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - if IsAWSErr(err, "AuthFailure", "") { + if awserrors.Matches(err, "AuthFailure", "") { log.Printf("Waiting for Vault-generated AWS credentials" + " to pass authentication... trying again.") return true @@ -131,7 +132,7 @@ func (s *StepPreValidate) checkVpc(conn ec2iface.EC2API) error { } res, err := conn.DescribeVpcs(&ec2.DescribeVpcsInput{VpcIds: []*string{aws.String(s.VpcId)}}) - if IsAWSErr(err, "InvalidVpcID.NotFound", "") || err != nil { + if awserrors.Matches(err, "InvalidVpcID.NotFound", "") || err != nil { return fmt.Errorf("Error retrieving VPC information for vpc_id %s: %s", s.VpcId, err) } diff --git a/builder/amazon/common/step_run_source_instance.go b/builder/amazon/common/step_run_source_instance.go index 71e42c261..ed228a128 100644 --- a/builder/amazon/common/step_run_source_instance.go +++ b/builder/amazon/common/step_run_source_instance.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/communicator" "github.com/hashicorp/packer/helper/multistep" @@ -204,7 +205,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - if IsAWSErr(err, "InvalidParameterValue", "iamInstanceProfile") { + if awserrors.Matches(err, "InvalidParameterValue", "iamInstanceProfile") { return true } return false @@ -215,7 +216,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa return err }) - if IsAWSErr(err, "VPCIdNotSpecified", "No default VPC for this user") && subnetId == "" { + if awserrors.Matches(err, "VPCIdNotSpecified", "No default VPC for this user") && subnetId == "" { err := fmt.Errorf("Error launching source instance: a valid Subnet Id was not specified") state.Put("error", err) ui.Error(err.Error()) @@ -253,7 +254,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa var r *ec2.DescribeInstancesOutput err = retry.Config{Tries: 11, ShouldRetry: func(err error) bool { - if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { + if awserrors.Matches(err, "InvalidInstanceID.NotFound", "") { return true } return false @@ -298,7 +299,7 @@ func (s *StepRunSourceInstance) Run(ctx context.Context, state multistep.StateBa ec2Tags.Report(ui) // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { + if awserrors.Matches(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_run_spot_instance.go b/builder/amazon/common/step_run_spot_instance.go index fae1dc4bc..75ec401e9 100644 --- a/builder/amazon/common/step_run_spot_instance.go +++ b/builder/amazon/common/step_run_spot_instance.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/random" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/communicator" @@ -396,7 +397,7 @@ func (s *StepRunSpotInstance) Run(ctx context.Context, state multistep.StateBag) // Retry creating tags for about 2.5 minutes err = retry.Config{Tries: 11, ShouldRetry: func(error) bool { - if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { + if awserrors.Matches(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/common/step_stop_ebs_instance.go b/builder/amazon/common/step_stop_ebs_instance.go index f02f15bfc..f3ed58473 100644 --- a/builder/amazon/common/step_stop_ebs_instance.go +++ b/builder/amazon/common/step_stop_ebs_instance.go @@ -6,6 +6,7 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" @@ -42,7 +43,7 @@ func (s *StepStopEBSBackedInstance) Run(ctx context.Context, state multistep.Sta // Work around this by retrying a few times, up to about 5 minutes. err := retry.Config{Tries: 6, ShouldRetry: func(error) bool { - if IsAWSErr(err, "InvalidInstanceID.NotFound", "") { + if awserrors.Matches(err, "InvalidInstanceID.NotFound", "") { return true } return false diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index 74a3a419a..5cbbc5ca8 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" + "github.com/hashicorp/packer/builder/amazon/common/awserrors" "github.com/hashicorp/packer/common/random" "github.com/hashicorp/packer/common/retry" "github.com/hashicorp/packer/helper/multistep" @@ -61,7 +62,7 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi err = retry.Config{ Tries: 0, ShouldRetry: func(err error) bool { - if awscommon.IsAWSErr(err, "InvalidParameterValue", "Instance is not in state") { + if awserrors.Matches(err, "InvalidParameterValue", "Instance is not in state") { return true } return false From f329cb5b93288fae310189821db779b2c589ef18 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 12:18:41 +0100 Subject: [PATCH 11/18] simplify code --- builder/amazon/common/ssm_driver.go | 34 ------------------- .../amazon/common/step_create_ssm_tunnel.go | 20 +++-------- 2 files changed, 4 insertions(+), 50 deletions(-) delete mode 100644 builder/amazon/common/ssm_driver.go diff --git a/builder/amazon/common/ssm_driver.go b/builder/amazon/common/ssm_driver.go deleted file mode 100644 index 2a8dfbd53..000000000 --- a/builder/amazon/common/ssm_driver.go +++ /dev/null @@ -1,34 +0,0 @@ -package common - -import ( - "context" - - "github.com/aws/aws-sdk-go/service/ssm" - "github.com/aws/aws-sdk-go/service/ssm/ssmiface" -) - -const ( - sessionManagerPluginName string = "session-manager-plugin" - - //sessionCommand is the AWS-SDK equivalent to the command you would specify to `aws ssm ...` - sessionCommand string = "StartSession" -) - -type SSMDriverConfig struct { - SvcClient ssmiface.SSMAPI - Region string - ProfileName string - SvcEndpoint string -} - -type SSMDriver struct { - SSMDriverConfig - session *ssm.StartSessionOutput - sessionParams ssm.StartSessionInput - pluginCmdFunc func(context.Context) error -} - -func NewSSMDriver(config SSMDriverConfig) *SSMDriver { - d := SSMDriver{SSMDriverConfig: config} - return &d -} diff --git a/builder/amazon/common/step_create_ssm_tunnel.go b/builder/amazon/common/step_create_ssm_tunnel.go index 4149acf4c..1066a8200 100644 --- a/builder/amazon/common/step_create_ssm_tunnel.go +++ b/builder/amazon/common/step_create_ssm_tunnel.go @@ -24,7 +24,6 @@ type StepCreateSSMTunnel struct { SSMAgentEnabled bool instanceId string PauseBeforeSSM time.Duration - driver *SSMDriver stopSSMCommand func() } @@ -65,16 +64,6 @@ func (s *StepCreateSSMTunnel) Run(ctx context.Context, state multistep.StateBag) } s.instanceId = aws.StringValue(instance.InstanceId) - if s.driver == nil { - ssmconn := ssm.New(s.AWSSession) - cfg := SSMDriverConfig{ - SvcClient: ssmconn, - Region: s.Region, - SvcEndpoint: ssmconn.Endpoint, - } - driver := SSMDriver{SSMDriverConfig: cfg} - s.driver = &driver - } state.Put("sessionPort", s.LocalPortNumber) input := s.BuildTunnelInputForInstance(s.instanceId) @@ -83,16 +72,15 @@ func (s *StepCreateSSMTunnel) Run(ctx context.Context, state multistep.StateBag) s.stopSSMCommand = ssmCancel go func() { + ssmconn := ssm.New(s.AWSSession) err := pssm.Session{ - SvcClient: s.driver.SvcClient, + SvcClient: ssmconn, Input: input, - Region: s.driver.Region, + Region: s.Region, }.Start(ssmCtx, ui) if err != nil { - err = fmt.Errorf("error encountered in establishing a tunnel %s", err) - ui.Error(err.Error()) - state.Put("error", err) + ui.Error(fmt.Sprintf("ssm error: %s", err)) } }() From aef3d242136795d3085d163e274b9830b31d5f19 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 12:31:01 +0100 Subject: [PATCH 12/18] Update step_create_ssm_tunnel.go --- builder/amazon/common/step_create_ssm_tunnel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/amazon/common/step_create_ssm_tunnel.go b/builder/amazon/common/step_create_ssm_tunnel.go index 1066a8200..d2ddefb27 100644 --- a/builder/amazon/common/step_create_ssm_tunnel.go +++ b/builder/amazon/common/step_create_ssm_tunnel.go @@ -37,7 +37,7 @@ func (s *StepCreateSSMTunnel) Run(ctx context.Context, state multistep.StateBag) // Wait for the remote port to become available if s.PauseBeforeSSM > 0 { - ui.Say(fmt.Sprintf("Waiting %s for establishing the SSM session...", s.PauseBeforeSSM)) + ui.Say(fmt.Sprintf("Waiting %s before establishing the SSM session...", s.PauseBeforeSSM)) select { case <-time.After(s.PauseBeforeSSM): break From a4bd7449551b6f6a0d21d1891910bd98a6ebff1f Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 13:11:07 +0100 Subject: [PATCH 13/18] simplify things a bit more --- builder/amazon/common/ssm/driver.go | 30 +++++++++++++++---- .../amazon/common/step_create_ssm_tunnel.go | 29 ++++-------------- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/builder/amazon/common/ssm/driver.go b/builder/amazon/common/ssm/driver.go index 5c8ad9819..12d31f3f8 100644 --- a/builder/amazon/common/ssm/driver.go +++ b/builder/amazon/common/ssm/driver.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "os/exec" + "strconv" "time" "github.com/aws/aws-sdk-go/aws" @@ -18,19 +19,36 @@ import ( ) type Session struct { - SvcClient ssmiface.SSMAPI - Region string - Input ssm.StartSessionInput + SvcClient ssmiface.SSMAPI + Region string + InstanceID string + LocalPort, RemotePort int +} + +func (s Session) buildTunnelInput() *ssm.StartSessionInput { + portNumber, localPortNumber := strconv.Itoa(s.RemotePort), strconv.Itoa(s.LocalPort) + params := map[string][]*string{ + "portNumber": []*string{aws.String(portNumber)}, + "localPortNumber": []*string{aws.String(localPortNumber)}, + } + + return &ssm.StartSessionInput{ + DocumentName: aws.String("AWS-StartPortForwardingSession"), + Parameters: params, + Target: aws.String(s.InstanceID), + } } // getCommand return a valid ordered set of arguments to pass to the driver command. func (s Session) getCommand(ctx context.Context) ([]string, string, error) { + input := s.buildTunnelInput() + var session *ssm.StartSessionOutput err := retry.Config{ ShouldRetry: func(err error) bool { return awserrors.Matches(err, "TargetNotConnected", "") }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 60 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) (err error) { - session, err = s.SvcClient.StartSessionWithContext(ctx, &s.Input) + session, err = s.SvcClient.StartSessionWithContext(ctx, input) return err }) @@ -49,7 +67,7 @@ func (s Session) getCommand(ctx context.Context) ([]string, string, error) { } // AWS session-manager-plugin requires the parameters used in the session to be passed in JSON as well. - sessionParameters, err := json.Marshal(s.Input) + sessionParameters, err := json.Marshal(input) if err != nil { return nil, "", fmt.Errorf("error encountered in reading session parameter details %s", err) } @@ -73,7 +91,7 @@ func (s Session) getCommand(ctx context.Context) ([]string, string, error) { // created from calling StartSession. func (s Session) Start(ctx context.Context, ui packer.Ui) error { for ctx.Err() == nil { - log.Printf("ssm: Starting PortForwarding session to instance %q", *s.Input.Target) + log.Printf("ssm: Starting PortForwarding session to instance %s", s.InstanceID) args, sessionID, err := s.getCommand(ctx) if sessionID != "" { defer func() { diff --git a/builder/amazon/common/step_create_ssm_tunnel.go b/builder/amazon/common/step_create_ssm_tunnel.go index d2ddefb27..aad0fab4f 100644 --- a/builder/amazon/common/step_create_ssm_tunnel.go +++ b/builder/amazon/common/step_create_ssm_tunnel.go @@ -3,7 +3,6 @@ package common import ( "context" "fmt" - "strconv" "time" "github.com/aws/aws-sdk-go/aws" @@ -22,7 +21,6 @@ type StepCreateSSMTunnel struct { LocalPortNumber int RemotePortNumber int SSMAgentEnabled bool - instanceId string PauseBeforeSSM time.Duration stopSSMCommand func() } @@ -62,21 +60,20 @@ func (s *StepCreateSSMTunnel) Run(ctx context.Context, state multistep.StateBag) state.Put("error", err) return multistep.ActionHalt } - s.instanceId = aws.StringValue(instance.InstanceId) state.Put("sessionPort", s.LocalPortNumber) - input := s.BuildTunnelInputForInstance(s.instanceId) - ssmCtx, ssmCancel := context.WithCancel(ctx) s.stopSSMCommand = ssmCancel go func() { ssmconn := ssm.New(s.AWSSession) err := pssm.Session{ - SvcClient: ssmconn, - Input: input, - Region: s.Region, + SvcClient: ssmconn, + InstanceID: aws.StringValue(instance.InstanceId), + RemotePort: s.RemotePortNumber, + LocalPort: s.LocalPortNumber, + Region: s.Region, }.Start(ssmCtx, ui) if err != nil { @@ -127,19 +124,3 @@ func (s *StepCreateSSMTunnel) ConfigureLocalHostPort(ctx context.Context) error return nil } - -func (s *StepCreateSSMTunnel) BuildTunnelInputForInstance(instance string) ssm.StartSessionInput { - dst, src := strconv.Itoa(s.RemotePortNumber), strconv.Itoa(s.LocalPortNumber) - params := map[string][]*string{ - "portNumber": []*string{aws.String(dst)}, - "localPortNumber": []*string{aws.String(src)}, - } - - input := ssm.StartSessionInput{ - DocumentName: aws.String("AWS-StartPortForwardingSession"), - Parameters: params, - Target: aws.String(instance), - } - - return input -} From c6e2dd5538d391cfb6aad01a6f92e05e54f8c73e Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 13:21:24 +0100 Subject: [PATCH 14/18] remove unit test file for now, I think that an acceptance test will be easier here --- builder/amazon/common/ssm_driver_test.go | 188 ------------------ .../common/step_create_ssm_tunnel_test.go | 139 ------------- 2 files changed, 327 deletions(-) delete mode 100644 builder/amazon/common/ssm_driver_test.go delete mode 100644 builder/amazon/common/step_create_ssm_tunnel_test.go diff --git a/builder/amazon/common/ssm_driver_test.go b/builder/amazon/common/ssm_driver_test.go deleted file mode 100644 index c28844b4d..000000000 --- a/builder/amazon/common/ssm_driver_test.go +++ /dev/null @@ -1,188 +0,0 @@ -package common - -import ( - "context" - "fmt" - "reflect" - "strings" - "testing" -) - -func NewSSMDriverWithMockSvc(svc *MockSSMSvc) *SSMDriver { - config := SSMDriverConfig{ - SvcClient: svc, - Region: "east", - ProfileName: "default", - SvcEndpoint: "example.com", - } - - driver := SSMDriver{ - SSMDriverConfig: config, - pluginCmdFunc: func(ctx context.Context) error { return nil }, - } - - return &driver -} -func TestSSMDriver_StartSession(t *testing.T) { - mockSvc := MockSSMSvc{} - driver := NewSSMDriverWithMockSvc(&mockSvc) - - if driver.SvcClient == nil { - t.Fatalf("SvcClient for driver should not be nil") - } - - session, err := driver.StartSession(context.TODO(), MockStartSessionInput("fakeinstance")) - if err != nil { - t.Fatalf("calling StartSession should not error but got %v", err) - } - - if !mockSvc.StartSessionCalled { - t.Fatalf("expected test to call ssm mocks but didn't") - } - - if session == nil { - t.Errorf("expected session to be set after a successful call to StartSession") - } - - if !reflect.DeepEqual(session, MockStartSessionOutput()) { - t.Errorf("expected session to be %v but got %v", MockStartSessionOutput(), session) - } -} - -func TestSSMDriver_StartSessionWithError(t *testing.T) { - mockSvc := MockSSMSvc{StartSessionError: fmt.Errorf("bogus error")} - driver := NewSSMDriverWithMockSvc(&mockSvc) - - if driver.SvcClient == nil { - t.Fatalf("SvcClient for driver should not be nil") - } - - session, err := driver.StartSession(context.TODO(), MockStartSessionInput("fakeinstance")) - if err == nil { - t.Fatalf("StartSession should have thrown an error but didn't") - } - - if !mockSvc.StartSessionCalled { - t.Errorf("expected test to call StartSession mock but didn't") - } - - if session != nil { - t.Errorf("expected session to be nil after a bad StartSession call, but got %v", session) - } -} - -func TestSSMDriver_StopSession(t *testing.T) { - mockSvc := MockSSMSvc{} - driver := NewSSMDriverWithMockSvc(&mockSvc) - - if driver.SvcClient == nil { - t.Fatalf("SvcClient for driver should not be nil") - } - - // Calling StopSession before StartSession should fail - err := driver.StopSession() - if err == nil { - t.Fatalf("calling StopSession() on a driver that has no started session should fail") - } - - if driver.session != nil { - t.Errorf("expected session to be default to nil") - } - - if mockSvc.TerminateSessionCalled { - t.Fatalf("a call to TerminateSession should not occur when there is no valid SSM session") - } - - // Lets try calling start session, then stopping to see what happens. - session, err := driver.StartSession(context.TODO(), MockStartSessionInput("fakeinstance")) - if err != nil { - t.Fatalf("calling StartSession should not error but got %v", err) - } - - if !mockSvc.StartSessionCalled { - t.Fatalf("expected test to call StartSession mock but didn't") - } - - if session == nil || driver.session != session { - t.Errorf("expected session to be set after a successful call to StartSession") - } - - if !reflect.DeepEqual(session, MockStartSessionOutput()) { - t.Errorf("expected session to be %v but got %v", MockStartSessionOutput(), session) - } - - err = driver.StopSession() - if err != nil { - t.Errorf("calling StopSession() on a driver on a started session should not fail") - } - - if !mockSvc.TerminateSessionCalled { - t.Fatalf("expected test to call StopSession mock but didn't") - } - -} - -func TestSSMDriver_Args(t *testing.T) { - tt := []struct { - Name string - ProfileName string - SkipStartSession bool - ErrorExpected bool - }{ - { - Name: "NilSession", - SkipStartSession: true, - ErrorExpected: true, - }, - { - Name: "NonNilSession", - ErrorExpected: false, - }, - { - Name: "SessionWithProfileName", - ProfileName: "default", - ErrorExpected: false, - }, - } - - for _, tc := range tt { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - mockSvc := MockSSMSvc{} - driver := NewSSMDriverWithMockSvc(&mockSvc) - driver.ProfileName = tc.ProfileName - - if driver.SvcClient == nil { - t.Fatalf("svcclient for driver should not be nil") - } - - if !tc.SkipStartSession { - _, err := driver.StartSession(context.TODO(), MockStartSessionInput("fakeinstance")) - if err != nil { - t.Fatalf("got an error when calling StartSession %v", err) - } - } - - args, err := driver.Args() - if tc.ErrorExpected && err == nil { - t.Fatalf("Driver.Args with a %q should have failed but instead no error was returned", tc.Name) - } - - if tc.ErrorExpected { - return - } - - if err != nil { - t.Fatalf("got an error when it should've worked %v", err) - } - - // validate launch script - expectedArgString := fmt.Sprintf(`{"SessionId":"packerid","StreamUrl":"http://packer.io","TokenValue":"packer-token"} east StartSession %s {"DocumentName":"AWS-StartPortForwardingSession","Parameters":{"localPortNumber":["8001"],"portNumber":["22"]},"Target":"fakeinstance"} example.com`, tc.ProfileName) - argString := strings.Join(args, " ") - if argString != expectedArgString { - t.Errorf("Expected launch script to be %q but got %q", expectedArgString, argString) - } - - }) - } -} diff --git a/builder/amazon/common/step_create_ssm_tunnel_test.go b/builder/amazon/common/step_create_ssm_tunnel_test.go deleted file mode 100644 index 531f24765..000000000 --- a/builder/amazon/common/step_create_ssm_tunnel_test.go +++ /dev/null @@ -1,139 +0,0 @@ -package common - -import ( - "context" - "reflect" - "testing" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" - "github.com/hashicorp/packer/packer" -) - -func TestStepCreateSSMTunnel_Run(t *testing.T) { - mockSvc := MockSSMSvc{} - config := SSMDriverConfig{ - SvcClient: &mockSvc, - SvcEndpoint: "example.com", - } - - mockDriver := NewSSMDriver(config) - mockDriver.pluginCmdFunc = MockPluginCmdFunc - - state := testState() - state.Put("ui", &packer.NoopUi{}) - state.Put("instance", &ec2.Instance{InstanceId: aws.String("i-something")}) - - step := StepCreateSSMTunnel{ - driver: mockDriver, - } - - step.Run(context.Background(), state) - - err := state.Get("error") - if err != nil { - err = err.(error) - t.Fatalf("the call to Run failed with an error when it should've executed: %v", err) - } - - if mockSvc.StartSessionCalled { - t.Errorf("StartSession should not be called when SSMAgentEnabled is false") - } - - // Run when SSMAgentEnabled is true - step.SSMAgentEnabled = true - step.Run(context.Background(), state) - - err = state.Get("error") - if err != nil { - err = err.(error) - t.Fatalf("the call to Run failed with an error when it should've executed: %v", err) - } - - if !mockSvc.StartSessionCalled { - t.Errorf("calling run with the correct inputs should call StartSession") - } - - step.Cleanup(state) - if !mockSvc.TerminateSessionCalled { - t.Errorf("calling cleanup on a successful run should call TerminateSession") - } -} - -func TestStepCreateSSMTunnel_Cleanup(t *testing.T) { - mockSvc := MockSSMSvc{} - config := SSMDriverConfig{ - SvcClient: &mockSvc, - SvcEndpoint: "example.com", - } - - mockDriver := NewSSMDriver(config) - mockDriver.pluginCmdFunc = MockPluginCmdFunc - - step := StepCreateSSMTunnel{ - SSMAgentEnabled: true, - driver: mockDriver, - } - - state := testState() - state.Put("ui", &packer.NoopUi{}) - state.Put("instance", &ec2.Instance{InstanceId: aws.String("i-something")}) - - step.Cleanup(state) - - if mockSvc.TerminateSessionCalled { - t.Fatalf("calling cleanup on a non started session should not call TerminateSession") - } - -} - -func TestStepCreateSSMTunnel_BuildTunnelInputForInstance(t *testing.T) { - step := StepCreateSSMTunnel{ - Region: "region", - LocalPortNumber: 8001, - RemotePortNumber: 22, - SSMAgentEnabled: true, - } - - input := step.BuildTunnelInputForInstance("i-something") - - target := aws.StringValue(input.Target) - if target != "i-something" { - t.Errorf("input should contain instance id as target but it got %q", target) - } - - params := map[string][]*string{ - "portNumber": []*string{aws.String("22")}, - "localPortNumber": []*string{aws.String("8001")}, - } - if !reflect.DeepEqual(input.Parameters, params) { - t.Errorf("input should contain the expected port parameters but it got %v", input.Parameters) - } - -} - -func TestStepCreateSSMTunnel_ConfigureLocalHostPort(t *testing.T) { - tt := []struct { - Name string - Step StepCreateSSMTunnel - PortCheck func(int) bool - }{ - {"WithLocalPortNumber", StepCreateSSMTunnel{LocalPortNumber: 9001}, func(port int) bool { return port == 9001 }}, - {"WithNoLocalPortNumber", StepCreateSSMTunnel{}, func(port int) bool { return port >= 8000 && port <= 9000 }}, - } - - for _, tc := range tt { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - step := tc.Step - if err := step.ConfigureLocalHostPort(context.TODO()); err != nil { - t.Errorf("failed to configure a port on localhost") - } - - if !tc.PortCheck(step.LocalPortNumber) { - t.Errorf("failed to configure a port on localhost") - } - }) - } - -} From 5d06a6e6dfc626c17f670008cde80f1584e9feb8 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 13:26:09 +0100 Subject: [PATCH 15/18] rename file correctly --- builder/amazon/common/ssm/{driver.go => session.go} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename builder/amazon/common/ssm/{driver.go => session.go} (96%) diff --git a/builder/amazon/common/ssm/driver.go b/builder/amazon/common/ssm/session.go similarity index 96% rename from builder/amazon/common/ssm/driver.go rename to builder/amazon/common/ssm/session.go index 12d31f3f8..2c90233e8 100644 --- a/builder/amazon/common/ssm/driver.go +++ b/builder/amazon/common/ssm/session.go @@ -97,7 +97,7 @@ func (s Session) Start(ctx context.Context, ui packer.Ui) error { defer func() { _, err := s.SvcClient.TerminateSession(&ssm.TerminateSessionInput{SessionId: aws.String(sessionID)}) if err != nil { - err = fmt.Errorf("Error terminating SSM Session %q. Please terminate the session manually: %s", sessionID, err) + ui.Error(fmt.Sprintf("Error terminating SSM Session %q. Please terminate the session manually: %s", sessionID, err)) } }() } From 6c45f044679ff4c4892f2faa7f646b808005cb27 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 13:37:44 +0100 Subject: [PATCH 16/18] Delete ssm_mock_funcs.go --- builder/amazon/common/ssm_mock_funcs.go | 57 ------------------------- 1 file changed, 57 deletions(-) delete mode 100644 builder/amazon/common/ssm_mock_funcs.go diff --git a/builder/amazon/common/ssm_mock_funcs.go b/builder/amazon/common/ssm_mock_funcs.go deleted file mode 100644 index 8ffb06a25..000000000 --- a/builder/amazon/common/ssm_mock_funcs.go +++ /dev/null @@ -1,57 +0,0 @@ -package common - -import ( - "context" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/request" - "github.com/aws/aws-sdk-go/service/ssm" - "github.com/aws/aws-sdk-go/service/ssm/ssmiface" -) - -type MockSSMSvc struct { - ssmiface.SSMAPI - StartSessionError error - TerminateSessionError error - StartSessionCalled bool - TerminateSessionCalled bool -} - -func (svc *MockSSMSvc) StartSessionWithContext(ctx aws.Context, input *ssm.StartSessionInput, options ...request.Option) (*ssm.StartSessionOutput, error) { - svc.StartSessionCalled = true - return MockStartSessionOutput(), svc.StartSessionError -} - -func (svc *MockSSMSvc) TerminateSession(input *ssm.TerminateSessionInput) (*ssm.TerminateSessionOutput, error) { - svc.TerminateSessionCalled = true - return new(ssm.TerminateSessionOutput), svc.TerminateSessionError -} - -func MockPluginCmdFunc(ctx context.Context) error { - return nil -} - -func MockStartSessionOutput() *ssm.StartSessionOutput { - id, url, token := "packerid", "http://packer.io", "packer-token" - output := ssm.StartSessionOutput{ - SessionId: &id, - StreamUrl: &url, - TokenValue: &token, - } - return &output -} - -func MockStartSessionInput(instance string) ssm.StartSessionInput { - params := map[string][]*string{ - "portNumber": []*string{aws.String("22")}, - "localPortNumber": []*string{aws.String("8001")}, - } - - input := ssm.StartSessionInput{ - DocumentName: aws.String("AWS-StartPortForwardingSession"), - Parameters: params, - Target: aws.String(instance), - } - - return input -} From aae1992649ba36a01f24726da165624b9ede0cb8 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Thu, 29 Oct 2020 13:38:03 +0100 Subject: [PATCH 17/18] remove default PauseBeforeSSM, this will have to be set manually --- builder/amazon/common/run_config.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index 932e416ad..3e57d3a57 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -548,10 +548,6 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { msg := fmt.Errorf(`no iam_instance_profile defined; session_manager connectivity requires a valid instance profile with AmazonSSMManagedInstanceCore permissions. Alternatively a temporary_iam_instance_profile_policy_document can be used.`) errs = append(errs, msg) } - - if c.PauseBeforeSSM == 0 { - c.PauseBeforeSSM = 10 * time.Second - } } if c.Comm.SSHKeyPairName != "" { From 01d5e5ca763c9db281a0d274e84b58b3c860a90f Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Thu, 29 Oct 2020 09:39:19 -0400 Subject: [PATCH 18/18] test/amazon/ebs: Add acceptance test for Session Manager Interface connectivity Test Results ``` ... 2020/10/29 09:35:39 ui: test: Starting session with SessionId: wilken-00bcfae4d314f54e7 2020/10/29 09:35:40 [DEBUG] TCP connection to SSH ip/port failed: dial tcp [::1]:8047: connect: connection refused 2020/10/29 09:35:40 ui: test: Port 8047 opened for sessionId wilken-00bcfae4d314f54e7. 2020/10/29 09:35:45 [INFO] Attempting SSH connection to localhost:8047... 2020/10/29 09:35:45 [DEBUG] reconnecting to TCP connection for SSH 2020/10/29 09:35:45 ui: test: Connection accepted for session wilken-00bcfae4d314f54e7. 2020/10/29 09:35:45 [DEBUG] handshaking with SSH 2020/10/29 09:35:45 [DEBUG] SSH handshake err: ssh: handshake failed: ssh: invalid packet length, packet too large 2020/10/29 09:35:52 [INFO] Attempting SSH connection to localhost:8047... 2020/10/29 09:35:52 [DEBUG] reconnecting to TCP connection for SSH 2020/10/29 09:35:52 [DEBUG] handshaking with SSH 2020/10/29 09:35:52 [DEBUG] handshake complete! 2020/10/29 09:35:52 [DEBUG] Opening new ssh session 2020/10/29 09:35:53 [INFO] agent forwarding enabled 2020/10/29 09:35:53 ui: ==> test: Connected to SSH! 2020/10/29 09:35:53 Running the provision hook 2020/10/29 09:35:53 ui: ==> test: Stopping the source instance... 2020/10/29 09:35:53 ui: test: Stopping instance 2020/10/29 09:35:54 ui: ==> test: Waiting for the instance to stop... 2020/10/29 09:36:25 ui: ==> test: Creating AMI packer-ssm-test-1603978447 from instance i-0853cb6186a3406d5 2020/10/29 09:36:25 ui: test: AMI: ami-0868a41bbb2df77b3 2020/10/29 09:36:25 ui: ==> test: Waiting for AMI to become ready... 2020/10/29 09:37:59 ui: ==> test: Terminating the source AWS instance... 2020/10/29 09:37:59 ui error: ==> test: Bad exit status: -1 2020/10/29 09:38:15 ui: ==> test: Cleaning up any extra volumes... 2020/10/29 09:38:15 ui: ==> test: No volumes to clean up, skipping 2020/10/29 09:38:15 ui: ==> test: Deleting temporary security group... 2020/10/29 09:38:16 ui: ==> test: Deleting temporary keypair... 2020/10/29 09:38:16 Deregistering image ID (ami-0868a41bbb2df77b3) from region (us-east-1) 2020/10/29 09:38:17 Deregistered AMI id: ami-0868a41bbb2df77b3 2020/10/29 09:38:17 Deleted snapshot: snap-09602f15994bc9f51 --- PASS: TestBuilderAcc_SessionManagerInterface (249.87s) PASS ``` --- builder/amazon/ebs/builder_acc_test.go | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/builder/amazon/ebs/builder_acc_test.go b/builder/amazon/ebs/builder_acc_test.go index d03b83ddb..cf569a353 100644 --- a/builder/amazon/ebs/builder_acc_test.go +++ b/builder/amazon/ebs/builder_acc_test.go @@ -246,6 +246,14 @@ func checkBootEncrypted() builderT.TestCheckFunc { } } +func TestBuilderAcc_SessionManagerInterface(t *testing.T) { + builderT.Test(t, builderT.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Builder: &Builder{}, + Template: testBuilderAccSessionManagerInterface, + }) +} + func testAccPreCheck(t *testing.T) { } @@ -350,6 +358,31 @@ const testBuilderAccEncrypted = ` } ` +const testBuilderAccSessionManagerInterface = ` +{ + "builders": [{ + "type": "test", + "region": "us-east-1", + "instance_type": "m3.medium", + "source_ami_filter": { + "filters": { + "virtualization-type": "hvm", + "name": "ubuntu/images/*ubuntu-xenial-16.04-amd64-server-*", + "root-device-type": "ebs" + }, + "owners": [ + "099720109477" + ], + "most_recent": true + }, + "ssh_username": "ubuntu", + "ssh_interface": "session_manager", + "iam_instance_profile": "SSMInstanceProfile", + "ami_name": "packer-ssm-test-{{timestamp}}" + }] +} +` + func buildForceDeregisterConfig(val, name string) string { return fmt.Sprintf(testBuilderAccForceDeregister, val, name) }