Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

builder: call Comm.Prepare before resetting host #91

Merged
merged 2 commits into from
Sep 23, 2022

Conversation

lbajolet-hashicorp
Copy link
Contributor

A regression was introduced in the plugin in commit f65be3c, where the logic for detecting a missing host, and falling back to localhost used to work with SSH only, and was generalised through the `Host()' method to work with WinRM too.

This logic however was flawed, as it required an explicit Type for the communicator to work. This is the case for winrm, but ssh being the default didn't need to, leading to the case where we resetted the host to 127.0.0.1.

To prevent this, we ensure the Type is properly set, which is done in the c.Comm.Prepare function, from the packer SDK. This in turn makes Host() report the right host for implicit ssh connections, and not an empty one as previously would happen.

Closes: hashicorp/packer#11961

A regression was introduced in the plugin in commit f65be3c, where the
logic for detecting a missing host, and falling back to localhost used
to work with SSH only, and was generalised through the `Host()' method
to work with WinRM too.

This logic however was flawed, as it required an explicit Type for the
communicator to work. This is the case for winrm, but ssh being the
default didn't need to, leading to the case where we resetted the host
to 127.0.0.1.

To prevent this, we ensure the Type is properly set, which is done in
the `c.Comm.Prepare` function, from the packer SDK.
This in turn makes `Host()` report the right host for implicit ssh
connections, and not an empty one as previously would happen.
Copy link
Member

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! LGTM

@lbajolet-hashicorp lbajolet-hashicorp merged commit e180626 into main Sep 23, 2022
@lbajolet-hashicorp lbajolet-hashicorp deleted the fix_ssh_reset_host_bug branch September 23, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Packer 1.8.2 and newer disregards ssh_host parameter (checked with qemu builder)
2 participants