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

Fix panic in proxyDialer #36726

Merged
merged 2 commits into from
Mar 21, 2025
Merged

Fix panic in proxyDialer #36726

merged 2 commits into from
Mar 21, 2025

Conversation

hasan4791
Copy link
Contributor

Incase of error, the response from net/HTTP ReadResponse is always nil and no need to close the Body.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x40 pc=0x103e5bf00]

goroutine 1847 [running]:
github.com/hashicorp/terraform/internal/communicator/ssh.(*proxyDialer).Dial(0x140017fddc0, {0x103f1cf9a?, 0x1060351c0?}, {0x14000709600, 0xf})
	github.com/hashicorp/terraform/internal/communicator/ssh/http_proxy.go:103 +0x520
github.com/hashicorp/terraform/internal/communicator/ssh.newHttpProxyConn(0x14000ef30b0, {0x14000709600, 0xf})
	github.com/hashicorp/terraform/internal/communicator/ssh/http_proxy.go:148 +0xd0
github.com/hashicorp/terraform/internal/communicator/ssh.prepareSSHConfig.ConnectFunc.func1()
	github.com/hashicorp/terraform/internal/communicator/ssh/communicator.go:804 +0x48
github.com/hashicorp/terraform/internal/communicator/ssh.(*Communicator).Connect(0x1400101a500, {0x104af1be0, 0x14000dea5e8})
	github.com/hashicorp/terraform/internal/communicator/ssh/communicator.go:194 +0x600
github.com/hashicorp/terraform/internal/builtin/provisioners/remote-exec.runScripts.func1()
	github.com/hashicorp/terraform/internal/builtin/provisioners/remote-exec/resource_provisioner.go:238 +0x30
github.com/hashicorp/terraform/internal/communicator.Retry.func1()
	github.com/hashicorp/terraform/internal/communicator/communicator.go:115 +0x1a4
created by github.com/hashicorp/terraform/internal/communicator.Retry in goroutine 1587
	github.com/hashicorp/terraform/internal/communicator/communicator.go:98 +0xc8
make: *** [apply] Error 2

Target Release

1.12.x

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

Incase of error, the response from net/HTTP ReadResponse is always
nil and no need to close the Body.

Signed-off-by: T K Chandra Hasan <[email protected]>
@hasan4791 hasan4791 requested a review from a team as a code owner March 20, 2025 10:55
Copy link

hashicorp-cla-app bot commented Mar 20, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

Copy link
Contributor

Changelog Warning

Currently this PR would target a v1.12 release. Please add a changelog entry for in the .changes/v1.12 folder, or discuss which release you'd like to target with your reviewer. If you believe this change does not need a changelog entry, please add the 'no-changelog-needed' label.

@crw
Copy link
Contributor

crw commented Mar 20, 2025

Hi @hasan4791, was there an issue filed related to this fix? How did you run into this issue? Reproduction steps? Thanks.

@hasan4791
Copy link
Contributor Author

Hi @hasan4791, was there an issue filed related to this fix? How did you run into this issue? Reproduction steps? Thanks.

I was using proxy for the ssh connection and it looks like once the instance is created, it takes some time for the ssh connection to be established and while it retries, hitting this panic. If i run the apply after the ssh is up, no issues observed. If required I can create an issue as it was clearly a bug but needs a little change.

Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

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

Thanks @crw and @hasan4791, we can take this without any further examples. That line is simply incorrect and shouldn't have passed initial review.

@jbardin jbardin enabled auto-merge March 21, 2025 13:08
@jbardin jbardin merged commit 69e822e into hashicorp:main Mar 21, 2025
6 of 8 checks passed
@hasan4791
Copy link
Contributor Author

Thank you @jbardin

@hasan4791 hasan4791 deleted the chasan-working branch March 21, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants