Use func to obtain proxy from environment to avoid issues with cachin…

…g during testing (#197)
hashicorp · Nov 3, 2022 · 837d6b2 · 837d6b2
1 parent 0914057
commit 837d6b2
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 82 deletions.
diff --git a/internal/provider/data_source_http.go b/internal/provider/data_source_http.go
@@ -8,11 +8,13 @@ import (
 	"io/ioutil"
 	"mime"
 	"net/http"
+	"net/url"
 	"regexp"
 	"strings"
 
 	"github.com/hashicorp/terraform-plugin-framework-validators/schemavalidator"
 	"github.com/hashicorp/terraform-plugin-framework/path"
+	"golang.org/x/net/http/httpproxy"
 
 	"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
 	"github.com/hashicorp/terraform-plugin-framework/datasource"
@@ -151,7 +153,7 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		return
 	}
 
-	url := model.URL.ValueString()
+	requestURL := model.URL.ValueString()
 	method := model.Method.ValueString()
 	requestHeaders := model.RequestHeaders
 	requestBody := strings.NewReader(model.RequestBody.ValueString())
@@ -171,6 +173,11 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		return
 	}
 
+	// Prevent issues with tests caching the proxy configuration.
+	tr.Proxy = func(req *http.Request) (*url.URL, error) {
+		return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
+	}
+
 	tr.TLSClientConfig = &tls.Config{}
 
 	if !model.Insecure.IsNull() {
@@ -195,7 +202,7 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		Transport: tr,
 	}
 
-	request, err := http.NewRequestWithContext(ctx, method, url, requestBody)
+	request, err := http.NewRequestWithContext(ctx, method, requestURL, requestBody)
 	if err != nil {
 		resp.Diagnostics.AddError(
 			"Error creating request",
@@ -258,7 +265,7 @@ func (d *httpDataSource) Read(ctx context.Context, req datasource.ReadRequest, r
 		return
 	}
 
-	model.ID = types.StringValue(url)
+	model.ID = types.StringValue(requestURL)
 	model.ResponseHeaders = respHeadersState
 	model.ResponseBody = types.StringValue(responseBody)
 	model.Body = types.StringValue(responseBody)

diff --git a/internal/provider/data_source_http_test.go b/internal/provider/data_source_http_test.go
@@ -16,85 +16,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
 
-func TestDataSource_HTTPViaProxyWithEnv(t *testing.T) {
-	proxyRequests := 0
-	serverRequests := 0
-	pReqPtr := &proxyRequests
-	sReqPtr := &serverRequests
-
-	// Content-Type is set to text/plain otherwise the http data source issues a warning which
-	// causes Terraform 0.14 to not write any data to state.
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		*sReqPtr++
-		w.Header().Set("Content-Type", "text/plain")
-		w.WriteHeader(http.StatusOK)
-	}))
-
-	defer server.Close()
-
-	// Neither localhost nor the loopback interface (127.0.0.1) can be used for the
-	// address of the server as httpproxy/proxy.go will ignore these addresses. See
-	// https://cs.opensource.google/go/x/net/+/internal-branch.go1.19-vendor:http/httpproxy/proxy.go;l=181
-	// https://cs.opensource.google/go/x/net/+/internal-branch.go1.19-vendor:http/httpproxy/proxy.go;l=186
-	serverURLStr := strings.Replace(server.URL, "127.0.0.1", "server", -1)
-	serverURL, err := url.Parse(serverURLStr)
-	if err != nil {
-		t.Error(err)
-	}
-
-	// The URL is intercepted and modified so that requests received by the proxy are forwarded to
-	// the loopback interface (127.0.0.1).
-	proxy := func(u *url.URL) http.Handler {
-		pServerURLStr := strings.Replace(u.String(), "server", "127.0.0.1", -1)
-		pServerURL, err := url.Parse(pServerURLStr)
-		if err != nil {
-			t.Error(err)
-		}
-
-		p := httputil.NewSingleHostReverseProxy(pServerURL)
-
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			*pReqPtr++
-			p.ServeHTTP(w, r)
-		})
-	}(serverURL)
-
-	frontend := httptest.NewServer(proxy)
-	defer frontend.Close()
-
-	t.Setenv("HTTP_PROXY", frontend.URL)
-	t.Setenv("HTTPS_PROXY", frontend.URL)
-
-	resource.UnitTest(t, resource.TestCase{
-		ProtoV5ProviderFactories: protoV5ProviderFactories(),
-
-		Steps: []resource.TestStep{
-			{
-				Config: fmt.Sprintf(`
-					data "http" "http_test" {
-						url = "%s"
-						insecure = "true"
-					}
-				`, serverURLStr),
-				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("data.http.http_test", "status_code", "200"),
-					CheckServerAndProxyRequestCount(pReqPtr, sReqPtr),
-				),
-			},
-		},
-	})
-}
-
-func CheckServerAndProxyRequestCount(proxyRequestCount, serverRequestCount *int) resource.TestCheckFunc {
-	return func(_ *terraform.State) error {
-		if *proxyRequestCount != *serverRequestCount {
-			return fmt.Errorf("expected proxy and server request count to match: proxy was %d, while server was %d", *proxyRequestCount, *serverRequestCount)
-		}
-
-		return nil
-	}
-}
-
 func TestDataSource_200(t *testing.T) {
 	testHttpMock := setUpMockHttpServer(false)
 	defer testHttpMock.server.Close()
@@ -553,6 +474,75 @@ func TestDataSource_UnsupportedInsecureCaCert(t *testing.T) {
 	})
 }
 
+// testProxiedURL is a hardcoded URL used in acceptance testing where it is
+// expected that a locally started HTTP proxy will handle the request.
+//
+// Neither localhost nor the loopback interface (127.0.0.1) can be used for the
+// address of the server as httpproxy/proxy.go will ignore these addresses.
+//
+// References:
+//   - https://cs.opensource.google/go/x/net/+/internal-branch.go1.19-vendor:http/httpproxy/proxy.go;l=181
+//   - https://cs.opensource.google/go/x/net/+/internal-branch.go1.19-vendor:http/httpproxy/proxy.go;l=186
+const testProxiedURL = "http://terraform-provider-http-test-proxy"
+
+func TestDataSource_HTTPViaProxyWithEnv(t *testing.T) {
+	proxyRequests := 0
+	serverRequests := 0
+
+	// Content-Type is set to text/plain otherwise the http data source issues a warning which
+	// causes Terraform 0.14 to not write any data to state.
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		serverRequests++
+		w.Header().Set("Content-Type", "text/plain")
+		w.WriteHeader(http.StatusOK)
+	}))
+
+	defer server.Close()
+
+	serverURL, err := url.Parse(server.URL)
+
+	if err != nil {
+		t.Fatalf("error parsing server URL: %s", err)
+	}
+
+	proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		proxyRequests++
+		httputil.NewSingleHostReverseProxy(serverURL).ServeHTTP(w, r)
+	}))
+	defer proxy.Close()
+
+	t.Setenv("HTTP_PROXY", proxy.URL)
+	t.Setenv("HTTPS_PROXY", proxy.URL)
+
+	resource.UnitTest(t, resource.TestCase{
+		ProtoV5ProviderFactories: protoV5ProviderFactories(),
+
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+					data "http" "http_test" {
+						url = "%s"
+					}
+				`, testProxiedURL),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("data.http.http_test", "status_code", "200"),
+					CheckServerAndProxyRequestCount(&proxyRequests, &serverRequests),
+				),
+			},
+		},
+	})
+}
+
+func CheckServerAndProxyRequestCount(proxyRequestCount, serverRequestCount *int) resource.TestCheckFunc {
+	return func(_ *terraform.State) error {
+		if *proxyRequestCount != *serverRequestCount {
+			return fmt.Errorf("expected proxy and server request count to match: proxy was %d, while server was %d", *proxyRequestCount, *serverRequestCount)
+		}
+
+		return nil
+	}
+}
+
 type TestHttpMock struct {
 	server *httptest.Server
 }