Apply suggestions from code review

Co-authored-by: Megan Marsh <[email protected]>

builder/amazon/common/run_config.go

@@ -410,7 +410,7 @@ type RunConfig struct {
 
 	// Which port to connect the local end of the session tunnel to. If
 	// left blank, Packer will choose a port for you from available ports.
-	// This option is on used when `ssh_interface` is set `session_manager`.
+	// This option is only used when `ssh_interface` is set `session_manager`.
 	SessionManagerPort int `mapstructure:"session_manager_port"`
 }
 
@@ -462,12 +462,12 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error {
 	// Connectivity via Session Manager has a few requirements
 	if c.SSHInterface == "session_manager" {
 		if c.Comm.Type == "winrm" {
-			msg := fmt.Errorf(`connectivity via %q is not currently supported with the %q communicator; please use "ssh"`, c.SSHInterface, c.Comm.Type)
+			msg := fmt.Errorf(`session_manager connectivity is not supported with the "winrm" communicator; please use "ssh"`)
 			errs = append(errs, msg)
 		}
 
 		if c.IamInstanceProfile == "" && c.TemporaryIamInstanceProfilePolicyDocument == nil {
-			msg := fmt.Errorf(`no iam_instance_profile defined; when using %q a valid instance profile with AmazonSSMManagedInstanceCore permissions is required. Alternatively a temporary_iam_instance_profile_policy_document can be used.`, c.SSHInterface)
+			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)
 		}
 	}

builder/amazon/common/ssm_driver.go

@@ -54,7 +54,15 @@ func (d *SSMDriver) StartSession(ctx context.Context) error {
 	stdoutCh := iochan.DelimReader(stdout, '\n')
 	stderrCh := iochan.DelimReader(stderr, '\n')
 
-	// Loop and get all our output
+	/* 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 {
@@ -81,6 +89,7 @@ func (d *SSMDriver) StartSession(ctx context.Context) error {
 	return nil
 }
 
+// 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")

builder/amazon/ebs/builder.go

@@ -264,8 +264,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			SSMAgentEnabled:  b.config.SSMAgentEnabled(),
 		},
 		&communicator.StepConnect{
-			// StepConnect is provided settings for WinRM and SSH, but
-			// the communicator will ultimately determine which port to use.
 			Config: &b.config.RunConfig.Comm,
 			Host: awscommon.SSHHost(
 				ec2conn,
@@ -276,10 +274,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				b.config.SSHInterface,
 				b.config.Comm.Port(),
 			),
-			WinRMPort: awscommon.Port(
-				b.config.SSHInterface,
-				b.config.Comm.Port(),
-			),
 			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},

builder/amazon/ebssurrogate/builder.go

@@ -297,10 +297,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				b.config.SSHInterface,
 				b.config.Comm.Port(),
 			),
-			WinRMPort: awscommon.Port(
-				b.config.SSHInterface,
-				b.config.Comm.Port(),
-			),
 			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},

builder/amazon/ebsvolume/builder.go

@@ -274,10 +274,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				b.config.SSHInterface,
 				b.config.Comm.Port(),
 			),
-			WinRMPort: awscommon.Port(
-				b.config.SSHInterface,
-				b.config.Comm.Port(),
-			),
 			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},

builder/amazon/instance/builder.go

@@ -358,10 +358,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				b.config.SSHInterface,
 				b.config.Comm.Port(),
 			),
-			WinRMPort: awscommon.Port(
-				b.config.SSHInterface,
-				b.config.Comm.Port(),
-			),
 			SSHConfig: b.config.RunConfig.Comm.SSHConfigFunc(),
 		},
 		&common.StepProvision{},

website/pages/partials/builder/amazon/common/RunConfig-not-required.mdx

@@ -320,5 +320,5 @@
     
 -   `session_manager_port` (int) - Which port to connect the local end of the session tunnel to. If
     left blank, Packer will choose a port for you from available ports.
-    This option is on used when `ssh_interface` is set `session_manager`.
+    This option is only used when `ssh_interface` is set `session_manager`.
     

website/pages/partials/builders/aws-sesson-manager.mdx

@@ -4,7 +4,7 @@ Support for the AWS Systems Manager session manager capability lets users manage
 
 To use the session manager as the connection interface for the SSH communicator you need to add the following configuration options to the Amazon builder options:
 
- * `ssh_interface`: The ssh interface must be set to "session_manager", when using this option the builder will no to create an SSM tunnel to the configured `ssh_port` (defaults to 22) on the remote host.
+ * `ssh_interface`: The ssh interface must be set to "session_manager".  When using this option the builder will create an SSM tunnel to the configured `ssh_port` (defaults to 22) on the remote host.
  * `iam_instance_profile`: A valid instance profile granting Systems Manger permissions to manage the remote instance is required in order for the aws ssm-agent to start and stop session connections. See below for more details on IAM instance profile for Systems Manager(#iam-instance-profile-for-systems-manager).
 
 #### Optional