Share one HTTP client and stop promcheck following redirects

cmd/preflight/cmd_http.go

@@ -7,6 +7,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/vertti/preflight/pkg/httpcheck"
+	"github.com/vertti/preflight/pkg/httpclient"
 )
 
 var (
@@ -70,7 +71,7 @@ func runHTTPCheck(_ *cobra.Command, args []string) error {
 		Contains:        httpContains,
 		FollowRedirects: httpFollowRedirects,
 		JSONPath:        httpJSONPath,
-		Client:          &httpcheck.RealHTTPClient{Timeout: httpTimeout, Insecure: httpInsecure, FollowRedirects: httpFollowRedirects},
+		Client:          &httpclient.Real{Timeout: httpTimeout, Insecure: httpInsecure, FollowRedirects: httpFollowRedirects},
 	}
 
 	return runCheck(c)

cmd/preflight/cmd_prometheus.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/vertti/preflight/pkg/httpclient"
 	"github.com/vertti/preflight/pkg/promcheck"
 )
 
@@ -71,7 +72,7 @@ func runPrometheusCheck(cmd *cobra.Command, args []string) error {
 		RetryDelay: promRetryDelay,
 		Insecure:   promInsecure,
 		Headers:    headers,
-		Client:     &promcheck.RealHTTPClient{Timeout: promTimeout, Insecure: promInsecure},
+		Client:     &httpclient.Real{Timeout: promTimeout, Insecure: promInsecure},
 	}
 
 	// Only set threshold pointers if flags were explicitly provided

integration_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/vertti/preflight/pkg/gitcheck"
 	"github.com/vertti/preflight/pkg/hashcheck"
 	"github.com/vertti/preflight/pkg/httpcheck"
+	"github.com/vertti/preflight/pkg/httpclient"
 	"github.com/vertti/preflight/pkg/jsoncheck"
 	"github.com/vertti/preflight/pkg/resourcecheck"
 	"github.com/vertti/preflight/pkg/syscheck"
@@ -377,7 +378,7 @@ func TestIntegration_HTTP(t *testing.T) {
 
 	c := httpcheck.Check{
 		URL:    server.URL,
-		Client: &httpcheck.RealHTTPClient{Timeout: 5 * time.Second},
+		Client: &httpclient.Real{Timeout: 5 * time.Second},
 	}
 
 	result := c.Run()

pkg/httpcheck/check.go

@@ -2,7 +2,6 @@ package httpcheck
 
 import (
 	"bytes"
-	"crypto/tls"
 	"fmt"
 	"io"
 	"net/http"
@@ -12,43 +11,10 @@ import (
 	"time"
 
 	"github.com/vertti/preflight/pkg/check"
+	"github.com/vertti/preflight/pkg/httpclient"
 	"github.com/vertti/preflight/pkg/jsonpath"
 )
 
-// HTTPClient abstracts HTTP requests for testability.
-type HTTPClient interface {
-	Do(req *http.Request) (*http.Response, error)
-}
-
-// RealHTTPClient uses the real net/http package.
-type RealHTTPClient struct {
-	Timeout         time.Duration
-	Insecure        bool
-	FollowRedirects bool
-}
-
-// Do executes an HTTP request.
-func (c *RealHTTPClient) Do(req *http.Request) (*http.Response, error) {
-	transport := &http.Transport{}
-	if c.Insecure {
-		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // intentional for --insecure flag
-	}
-
-	client := &http.Client{
-		Timeout:   c.Timeout,
-		Transport: transport,
-	}
-
-	// Disable automatic redirects unless explicitly enabled
-	if !c.FollowRedirects {
-		client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
-			return http.ErrUseLastResponse
-		}
-	}
-
-	return client.Do(req)
-}
-
 // FileReader abstracts file reading for testability.
 type FileReader interface {
 	ReadFile(path string) ([]byte, error)
@@ -77,7 +43,7 @@ type Check struct {
 	Contains        string            // response body must contain this string
 	FollowRedirects bool              // follow HTTP redirects (3xx)
 	JSONPath        string            // JSON path to check (format: "path=expectedValue" or just "path")
-	Client          HTTPClient        // injected for testing
+	Client          httpclient.Client // injected for testing
 	FileReader      FileReader        // injected for testing
 }
 
@@ -117,7 +83,7 @@ func (c *Check) Run() check.Result {
 	// Initialize client if not injected
 	client := c.Client
 	if client == nil {
-		client = &RealHTTPClient{Timeout: timeout, Insecure: c.Insecure, FollowRedirects: c.FollowRedirects}
+		client = &httpclient.Real{Timeout: timeout, Insecure: c.Insecure, FollowRedirects: c.FollowRedirects}
 	}
 
 	// Resolve request body

pkg/httpcheck/check_test.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"io"
 	"net/http"
-	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
@@ -276,81 +275,6 @@ func TestHTTPCheckJSONPathRetry(t *testing.T) {
 	})
 }
 
-func TestRealHTTPClient(t *testing.T) {
-	t.Run("basic request", func(t *testing.T) {
-		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-			_, _ = w.Write([]byte("OK"))
-		}))
-		defer ts.Close()
-
-		client := &RealHTTPClient{Timeout: 5 * time.Second}
-		req, err := http.NewRequest(http.MethodGet, ts.URL, http.NoBody)
-		require.NoError(t, err)
-
-		resp, err := client.Do(req)
-		require.NoError(t, err)
-		defer func() { _ = resp.Body.Close() }()
-		assert.Equal(t, 200, resp.StatusCode)
-	})
-
-	t.Run("insecure TLS", func(t *testing.T) {
-		ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusOK)
-		}))
-		defer ts.Close()
-
-		client := &RealHTTPClient{Timeout: 5 * time.Second, Insecure: true}
-		req, err := http.NewRequest(http.MethodGet, ts.URL, http.NoBody)
-		require.NoError(t, err)
-
-		resp, err := client.Do(req)
-		require.NoError(t, err)
-		defer func() { _ = resp.Body.Close() }()
-		assert.Equal(t, 200, resp.StatusCode)
-	})
-
-	t.Run("redirects disabled", func(t *testing.T) {
-		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if r.URL.Path == "/redirect" {
-				http.Redirect(w, r, "/target", http.StatusFound)
-				return
-			}
-			w.WriteHeader(http.StatusOK)
-		}))
-		defer ts.Close()
-
-		client := &RealHTTPClient{Timeout: 5 * time.Second, FollowRedirects: false}
-		req, err := http.NewRequest(http.MethodGet, ts.URL+"/redirect", http.NoBody)
-		require.NoError(t, err)
-
-		resp, err := client.Do(req)
-		require.NoError(t, err)
-		defer func() { _ = resp.Body.Close() }()
-		assert.Equal(t, 302, resp.StatusCode)
-	})
-
-	t.Run("redirects enabled", func(t *testing.T) {
-		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			if r.URL.Path == "/redirect" {
-				http.Redirect(w, r, "/target", http.StatusFound)
-				return
-			}
-			w.WriteHeader(http.StatusOK)
-		}))
-		defer ts.Close()
-
-		client := &RealHTTPClient{Timeout: 5 * time.Second, FollowRedirects: true}
-		req, err := http.NewRequest(http.MethodGet, ts.URL+"/redirect", http.NoBody)
-		require.NoError(t, err)
-
-		resp, err := client.Do(req)
-		require.NoError(t, err)
-		defer func() { _ = resp.Body.Close() }()
-		assert.Equal(t, 200, resp.StatusCode)
-	})
-}
-
 func TestRealFileReader(t *testing.T) {
 	tmpFile := filepath.Join(t.TempDir(), "testfile")
 	content := []byte("test content")

pkg/httpclient/client.go

@@ -0,0 +1,49 @@
+// Package httpclient provides the HTTP client shared by the checks that make
+// requests. It exists because httpcheck and promcheck each had their own copy:
+// the copies drifted, promcheck's never gained redirect protection, and it
+// forwarded custom auth headers to whatever host a 3xx named.
+package httpclient
+
+import (
+	"crypto/tls"
+	"net/http"
+	"time"
+)
+
+// Client abstracts HTTP requests for testability.
+type Client interface {
+	Do(req *http.Request) (*http.Response, error)
+}
+
+// Real is a Client backed by net/http.
+type Real struct {
+	Timeout         time.Duration
+	Insecure        bool
+	FollowRedirects bool
+}
+
+// Do executes an HTTP request.
+//
+// Redirects are not followed unless FollowRedirects is set. Go strips
+// Authorization and Cookie when a redirect crosses hosts, but not custom
+// headers — and tenancy headers like X-Scope-OrgID are custom. Following a
+// redirect would also let the destination decide the check's verdict.
+func (c *Real) Do(req *http.Request) (*http.Response, error) {
+	transport := &http.Transport{}
+	if c.Insecure {
+		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint:gosec // intentional for --insecure flag
+	}
+
+	client := &http.Client{
+		Timeout:   c.Timeout,
+		Transport: transport,
+	}
+
+	if !c.FollowRedirects {
+		client.CheckRedirect = func(*http.Request, []*http.Request) error {
+			return http.ErrUseLastResponse
+		}
+	}
+
+	return client.Do(req)
+}

pkg/httpclient/client_test.go

@@ -0,0 +1,86 @@
+package httpclient
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestReal(t *testing.T) {
+	t.Run("basic request", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte("OK"))
+		}))
+		defer ts.Close()
+
+		client := &Real{Timeout: 5 * time.Second}
+		req, err := http.NewRequest(http.MethodGet, ts.URL, http.NoBody)
+		require.NoError(t, err)
+
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer func() { _ = resp.Body.Close() }()
+		assert.Equal(t, 200, resp.StatusCode)
+	})
+
+	t.Run("insecure TLS", func(t *testing.T) {
+		ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer ts.Close()
+
+		client := &Real{Timeout: 5 * time.Second, Insecure: true}
+		req, err := http.NewRequest(http.MethodGet, ts.URL, http.NoBody)
+		require.NoError(t, err)
+
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer func() { _ = resp.Body.Close() }()
+		assert.Equal(t, 200, resp.StatusCode)
+	})
+
+	t.Run("redirects disabled", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.Path == "/redirect" {
+				http.Redirect(w, r, "/target", http.StatusFound)
+				return
+			}
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer ts.Close()
+
+		client := &Real{Timeout: 5 * time.Second, FollowRedirects: false}
+		req, err := http.NewRequest(http.MethodGet, ts.URL+"/redirect", http.NoBody)
+		require.NoError(t, err)
+
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer func() { _ = resp.Body.Close() }()
+		assert.Equal(t, 302, resp.StatusCode)
+	})
+
+	t.Run("redirects enabled", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.URL.Path == "/redirect" {
+				http.Redirect(w, r, "/target", http.StatusFound)
+				return
+			}
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer ts.Close()
+
+		client := &Real{Timeout: 5 * time.Second, FollowRedirects: true}
+		req, err := http.NewRequest(http.MethodGet, ts.URL+"/redirect", http.NoBody)
+		require.NoError(t, err)
+
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer func() { _ = resp.Body.Close() }()
+		assert.Equal(t, 200, resp.StatusCode)
+	})
+}