🌱 OPRUN-4573: tests: add unit and e2e tests for HTTPS_PROXY support

Makefile

@@ -255,9 +255,10 @@ $(eval $(call install-sh,standard,operator-controller-standard.yaml))
 test: manifests generate fmt lint test-unit test-e2e test-regression #HELP Run all tests.
 
 E2E_TIMEOUT ?= 10m
+GODOG_ARGS ?=
 .PHONY: e2e
 e2e: #EXHELP Run the e2e tests.
-	go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT)
+	go test -count=1 -v ./test/e2e/features_test.go -timeout=$(E2E_TIMEOUT) $(if $(GODOG_ARGS),-args $(GODOG_ARGS))
 
 E2E_REGISTRY_NAME := docker-registry
 E2E_REGISTRY_NAMESPACE := operator-controller-e2e

internal/shared/util/http/httputil.go

@@ -20,6 +20,10 @@ func BuildHTTPClient(cpw *CertPoolWatcher) (*http.Client, error) {
 	}
 	tlsTransport := &http.Transport{
 		TLSClientConfig: tlsConfig,
+		// Proxy must be set explicitly; a nil Proxy field means "no proxy" and
+		// ignores HTTPS_PROXY/NO_PROXY env vars.  Only http.DefaultTransport sets
+		// this by default; custom transports must opt in.
+		Proxy: http.ProxyFromEnvironment,
 	}
 	httpClient.Transport = tlsTransport
 

internal/shared/util/http/httputil_test.go

@@ -0,0 +1,198 @@
+package http_test
+
+import (
+	"context"
+	"encoding/pem"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
+)
+
+// startRecordingProxy starts a plain-HTTP CONNECT proxy that tunnels HTTPS
+// connections and records the target host of each CONNECT request.
+func startRecordingProxy(proxied chan<- string) *httptest.Server {
+	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodConnect {
+			http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
+			return
+		}
+		// Non-blocking: if there are unexpected extra CONNECT requests (retries,
+		// parallel connections) we record the first one and drop the rest rather
+		// than blocking the proxy handler goroutine.
+		select {
+		case proxied <- r.Host:
+		default:
+		}
+
+		dst, err := net.Dial("tcp", r.Host)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadGateway)
+			return
+		}
+		defer dst.Close()
+
+		hj, ok := w.(http.Hijacker)
+		if !ok {
+			http.Error(w, "hijacking not supported", http.StatusInternalServerError)
+			return
+		}
+		conn, bufrw, err := hj.Hijack()
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		defer conn.Close()
+
+		if _, err = conn.Write([]byte("HTTP/1.1 200 Connection established\r\n\r\n")); err != nil {
+			return
+		}
+
+		done := make(chan struct{}, 2)
+		tunnel := func(dst io.Writer, src io.Reader) {
+			defer func() { done <- struct{}{} }()
+			_, _ = io.Copy(dst, src)
+		}
+		// Use bufrw (not conn) as the client→dst source: Hijack may have
+		// buffered bytes (e.g. the TLS ClientHello) that arrived together with
+		// the CONNECT headers; reading from conn directly would lose them.
+		go tunnel(dst, bufrw)
+		go tunnel(conn, dst)
+		<-done
+		<-done // wait for both directions before closing connections
+	}))
+}
+
+// certPoolWatcherForTLSServer creates a CertPoolWatcher that trusts the given
+// TLS test server's certificate.
+func certPoolWatcherForTLSServer(t *testing.T, server *httptest.Server) *httputil.CertPoolWatcher {
+	t.Helper()
+
+	dir := t.TempDir()
+	certPath := filepath.Join(dir, "server.pem")
+
+	certDER := server.TLS.Certificates[0].Certificate[0]
+	f, err := os.Create(certPath)
+	require.NoError(t, err)
+	require.NoError(t, pem.Encode(f, &pem.Block{Type: "CERTIFICATE", Bytes: certDER}))
+	require.NoError(t, f.Close())
+
+	cpw, err := httputil.NewCertPoolWatcher(dir, log.FromContext(context.Background()))
+	require.NoError(t, err)
+	require.NotNil(t, cpw)
+	t.Cleanup(cpw.Done)
+	require.NoError(t, cpw.Start(context.Background()))
+	return cpw
+}
+
+// TestBuildHTTPClientTransportUsesProxyFromEnvironment verifies that the
+// transport returned by BuildHTTPClient has Proxy set to http.ProxyFromEnvironment
+// so that HTTPS_PROXY and NO_PROXY env vars are honoured at runtime.
+func TestBuildHTTPClientTransportUsesProxyFromEnvironment(t *testing.T) {
+	// Use system certs (empty dir) — we only need a valid CertPoolWatcher.
+	cpw, err := httputil.NewCertPoolWatcher("", log.FromContext(context.Background()))
+	require.NoError(t, err)
+	t.Cleanup(cpw.Done)
+	require.NoError(t, cpw.Start(context.Background()))
+
+	client, err := httputil.BuildHTTPClient(cpw)
+	require.NoError(t, err)
+
+	transport, ok := client.Transport.(*http.Transport)
+	require.True(t, ok)
+	require.NotNil(t, transport.Proxy,
+		"BuildHTTPClient must set transport.Proxy so that HTTPS_PROXY env vars are respected; "+
+			"a nil Proxy field means no proxy regardless of environment")
+}
+
+// TestBuildHTTPClientProxyTunnelsConnections verifies end-to-end that the
+// HTTP client produced by BuildHTTPClient correctly tunnels HTTPS connections
+// through an HTTP CONNECT proxy.
+//
+// The test overrides transport.Proxy with http.ProxyURL rather than relying on
+// HTTPS_PROXY: httptest servers bind to 127.0.0.1, which http.ProxyFromEnvironment
+// silently excludes from proxying, and env-var changes within the same process
+// are unreliable due to sync.Once caching.  Using http.ProxyURL directly exercises
+// the same tunnelling code path that HTTPS_PROXY triggers in production.
+func TestBuildHTTPClientProxyTunnelsConnections(t *testing.T) {
+	targetServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer targetServer.Close()
+
+	proxied := make(chan string, 1)
+	proxyServer := startRecordingProxy(proxied)
+	defer proxyServer.Close()
+
+	proxyURL, err := url.Parse(proxyServer.URL)
+	require.NoError(t, err)
+
+	cpw := certPoolWatcherForTLSServer(t, targetServer)
+	client, err := httputil.BuildHTTPClient(cpw)
+	require.NoError(t, err)
+
+	// Point the transport directly at our test proxy, bypassing the loopback
+	// exclusion and env-var caching of http.ProxyFromEnvironment.
+	transport, ok := client.Transport.(*http.Transport)
+	require.True(t, ok)
+	transport.Proxy = http.ProxyURL(proxyURL)
+
+	resp, err := client.Get(targetServer.URL)
+	require.NoError(t, err)
+	resp.Body.Close()
+
+	select {
+	case host := <-proxied:
+		require.Equal(t, targetServer.Listener.Addr().String(), host,
+			"proxy must have received a CONNECT request for the target server address")
+	case <-time.After(5 * time.Second):
+		t.Fatal("HTTPS connection to target server did not go through the proxy")
+	}
+}
+
+// TestBuildHTTPClientProxyBlocksWhenRejected verifies that when the proxy
+// rejects the CONNECT tunnel, the client request fails rather than silently
+// falling back to a direct connection.
+func TestBuildHTTPClientProxyBlocksWhenRejected(t *testing.T) {
+	targetServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer targetServer.Close()
+
+	// A proxy that returns 403 Forbidden for every CONNECT request.
+	rejectingProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodConnect {
+			http.Error(w, "proxy access denied", http.StatusForbidden)
+			return
+		}
+		http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed)
+	}))
+	defer rejectingProxy.Close()
+
+	proxyURL, err := url.Parse(rejectingProxy.URL)
+	require.NoError(t, err)
+
+	cpw := certPoolWatcherForTLSServer(t, targetServer)
+	client, err := httputil.BuildHTTPClient(cpw)
+	require.NoError(t, err)
+
+	transport, ok := client.Transport.(*http.Transport)
+	require.True(t, ok)
+	transport.Proxy = http.ProxyURL(proxyURL)
+
+	resp, err := client.Get(targetServer.URL)
+	if resp != nil {
+		resp.Body.Close()
+	}
+	require.Error(t, err, "request should fail when the proxy rejects the CONNECT tunnel")
+}

test/e2e/features/proxy.feature

@@ -0,0 +1,64 @@
+Feature: HTTPS proxy support for outbound catalog requests
+
+  OLM's operator-controller fetches catalog data from catalogd over HTTPS.
+  When HTTPS_PROXY is set in the operator-controller's environment, all
+  outbound HTTPS requests must be routed through the configured proxy.
+
+  Background:
+    Given OLM is available
+    And ClusterCatalog "test" serves bundles
+    And ServiceAccount "olm-sa" with needed permissions is available in test namespace
+
+  @HTTPProxy
+  Scenario: operator-controller respects HTTPS_PROXY when fetching catalog data
+    Given the "operator-controller" component is configured with HTTPS_PROXY "http://127.0.0.1:39999"
+    When ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    Then ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
+      """
+      proxyconnect
+      """
+
+  @HTTPProxy
+  Scenario: operator-controller sends catalog requests through a configured HTTPS proxy
+    # The recording proxy runs on the host and cannot route to in-cluster service
+    # addresses, so it responds 502 after recording the CONNECT.  This is
+    # intentional: the scenario only verifies that operator-controller respects
+    # HTTPS_PROXY and sends catalog fetches through the proxy, not that the full
+    # end-to-end request succeeds.
+    Given the "operator-controller" component is configured with HTTPS_PROXY pointing to a recording proxy
+    When ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    Then the recording proxy received a CONNECT request for the catalogd service

test/e2e/steps/hooks.go

@@ -27,6 +27,15 @@ type resource struct {
 	namespace string
 }
 
+// deploymentRestore records the original state of a deployment container so
+// it can be rolled back after a test that modifies deployment configuration.
+type deploymentRestore struct {
+	name          string // deployment name
+	namespace     string
+	containerName string
+	originalEnv   []string // "NAME=VALUE" pairs
+}
+
 type scenarioContext struct {
 	id                   string
 	namespace            string
@@ -39,7 +48,9 @@ type scenarioContext struct {
 	metricsResponse      map[string]string
 	leaderPods           map[string]string // component name -> leader pod name
 
-	extensionObjects []client.Object
+	extensionObjects   []client.Object
+	deploymentRestores []deploymentRestore
+	proxy              *recordingProxy
 }
 
 // GatherClusterExtensionObjects collects all resources related to the ClusterExtension container in
@@ -182,6 +193,23 @@ func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context
 			_ = p.Kill()
 		}
 	}
+
+	// Stop the in-process recording proxy if one was started.
+	if sc.proxy != nil {
+		sc.proxy.stop()
+	}
+
+	// Restore any deployments that were modified during the scenario.  This runs
+	// unconditionally (even on test failure) so that a misconfigured deployment
+	// does not bleed into subsequent scenarios.  configureDeploymentProxy
+	// deduplicates entries, so each deployment appears at most once.
+	for i := len(sc.deploymentRestores) - 1; i >= 0; i-- {
+		restore := sc.deploymentRestores[i]
+		if err := restoreDeployment(restore); err != nil {
+			logger.Info("Error restoring deployment", "deployment", restore.name, "namespace", restore.namespace, "error", err)
+		}
+	}
+
 	if err != nil {
 		return ctx, err
 	}