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

Error making requests to Cloudflare #388

Closed
1 task done
bootswithdefer opened this issue Feb 26, 2024 · 8 comments · Fixed by #390
Closed
1 task done

Error making requests to Cloudflare #388

bootswithdefer opened this issue Feb 26, 2024 · 8 comments · Fixed by #390
Assignees
Labels
Milestone

Comments

@bootswithdefer
Copy link

bootswithdefer commented Feb 26, 2024

Terraform CLI and Provider Versions

Terraform v1.5.6
on linux_amd64

  • provider registry.terraform.io/hashicorp/http v3.4.1

Terraform Configuration

data "http" "example" {
  url = "https://www.cloudflare.com/ips-v4"
}

Expected Behavior

No errors

Actual Behavior

data.http.example: Reading...
╷
│ Error: Error making request
│ 
│   with data.http.example,
│   on main.tf line 1, in data "http" "example":
│    1: data "http" "example" {
│ 
│ Error making request: GET https://www.cloudflare.com/ips-v4 giving up after 1 attempt(s)

Steps to Reproduce

  1. terraform apply

How much impact is this issue causing?

Medium

Logs

https://gist.github.com/bootswithdefer/0ac09cc56826691aec9427e33a7739e1

Additional Information

When querying the Cloudflare URL the provider errors, however if I disable the http2client (GODEBUG http2debug=1,http2client=0) then the provider is successful. This just started today as near as I can tell, we have been querying Cloudflare like this for years.

I tried a simple golang http client program and it can query Cloudflare with no issues. I tried building the provider locally with golang 1.21.6 and it also errors like the distributed provider.

The gist contains two outputs, one with GODEBUG http2debug=1,http2client=1 and the second with GODEBUG http2debug=1,http2client=0

Code of Conduct

  • I agree to follow this project's Code of Conduct
@acdha
Copy link

acdha commented Feb 26, 2024

I just encountered this using the same URL for the same reason, with identical behaviour using Terraform Terraform v1.7.4 on darwin_arm64 and linux_amd64 both with provider registry.terraform.io/hashicorp/http v3.4.1. I also tested with a different Go-based HTTP client w/o issue and all other tools (curl, httpie, browsers, etc.) do not have this problem.

I deployed the workaround of setting GODEBUG to http2client=0 in our CI configuration which resolved this. I'm sure it's slightly slower but for two requests that's moot.

@samm-git
Copy link

It is enough to remove www. and it will start working.

@acdha
Copy link

acdha commented Feb 27, 2024

It is enough to remove www. and it will start working.

That's really interesting because without www. it redirects to the original URL:

https://redbot.org/?uri=https://cloudflare.com/ips-v4

Location: https://www.cloudflare.com/ips-v4

@acdha
Copy link

acdha commented Feb 27, 2024

Some regression testing shows that the last working version of the HTTP provider was 3.2.1 but 3.3.0 was a fairly large release with some significant changes like going from Go 1.18 to 1.19:

v3.2.1...v3.3.0

@bflad bflad self-assigned this Feb 29, 2024
@bflad
Copy link
Contributor

bflad commented Feb 29, 2024

Hi everyone 👋 Thank you for the reports and sorry for the frustrating experience. The data source should be doing a better job returning more error information than just the failed HTTP method and URL.

As far as I can tell, the actual error occurring is a 500 Internal Server Error for these requests. Through a process of elimination, it appears related to the fact that the current data source implementation always sends an empty string request body, instead of only sending a request body when one is configured. While the receiving server should probably be ignoring that request body as it always was previously or sending a different 4xx error, the fix here (to be verified still) will likely be a small code change and a new provider release.

I will keep this issue updated with more details throughout the process. Thank you.

@bflad bflad added this to the v3.4.2 milestone Feb 29, 2024
bflad added a commit that referenced this issue Feb 29, 2024
Reference: #388

Previously the HTTP request would unexpectedly always contain a body for all requests. Certain HTTP server implementations are sensitive to this data existing if it is not expected. Requests now only contain a request body if the `request_body` attribute is explicitly set. To exactly preserve the previous behavior, set `request_body = ""`.

Added new acceptance testing for `request_body` handling. Go net/http server implementations seem to ignore this problematic situation (or it was not easy to determine), so also verified with a real world configuration using a temporary acceptance test as well as manually running Terraform using a development build containing these changes:

```
// Reference: #388
func TestDataSource_RequestBody_Null(t *testing.T) {
	t.Parallel()

	resource.Test(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		Steps: []resource.TestStep{
			{
				Config: `
					data "http" "test" {
						url = "https://www.cloudflare.com/ips-v4"
					}`,
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("data.http.test", "status_code", "200"),
				),
			},
		},
	})
}
```

This real world acceptance test is not added as the server may change its implementation in the future.
bflad added a commit that referenced this issue Feb 29, 2024
…red (#390)

Reference: #388

Previously the HTTP request would unexpectedly always contain a body for all requests. Certain HTTP server implementations are sensitive to this data existing if it is not expected. Requests now only contain a request body if the `request_body` attribute is explicitly set. To exactly preserve the previous behavior, set `request_body = ""`.

Added new acceptance testing for `request_body` handling. Go net/http server implementations seem to ignore this problematic situation (or it was not easy to determine), so also verified with a real world configuration using a temporary acceptance test as well as manually running Terraform using a development build containing these changes:

```
// Reference: #388
func TestDataSource_RequestBody_Null(t *testing.T) {
	t.Parallel()

	resource.Test(t, resource.TestCase{
		ProtoV5ProviderFactories: protoV5ProviderFactories(),
		Steps: []resource.TestStep{
			{
				Config: `
					data "http" "test" {
						url = "https://www.cloudflare.com/ips-v4"
					}`,
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("data.http.test", "status_code", "200"),
				),
			},
		},
	})
}
```

This real world acceptance test is not added as the server may change its implementation in the future.
@bflad
Copy link
Contributor

bflad commented Feb 29, 2024

v3.4.2 has been released with a fix for this and should become available in the public Terraform Registry shortly.

@bootswithdefer
Copy link
Author

3.4.2 resolved the issue for me.

Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants