fix: preserve default proxy settings (#1656)

* chore: don't reset default proxy settings
* chore: consmetic changes

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Author: tolusha

controllers/workspace/http.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/config"
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/go-logr/logr"
@@ -44,7 +45,7 @@ type HttpClientsFactory interface {
 	// GetHealthCheckHttpClient returns an HTTP client that skips TLS verification.
 	// This client MUST only be used for workspace health/readiness checks, not for
 	// fetching external content or making security-sensitive requests.
-	GetHealthCheckHttpClient(*controller.RoutingConfig) *http.Client
+	GetHealthCheckHttpClient() *http.Client
 }
 
 // DefaultHttpClientsFactory is a thread-safe, caching implementation of HttpClientsFactory.
@@ -59,13 +60,11 @@ type DefaultHttpClientsFactory struct {
 
 	mu sync.RWMutex
 
-	httpClientProxyConfig  *controller.Proxy
 	httpClientConfigmapRef *controller.ConfigmapReference
 	httpClientCertsVersion string
 
-	healthCheckHttpClientProxyConfig *controller.Proxy
-
 	systemCertPool *x509.CertPool
+	proxyFunc      func(*http.Request) (*url.URL, error)
 }
 
 func SetupHttpClientsFactory(k8s client.Client, logger logr.Logger) error {
@@ -74,10 +73,26 @@ func SetupHttpClientsFactory(k8s client.Client, logger logr.Logger) error {
 		return fmt.Errorf("failed to load system cert pool: %w", err)
 	}
 
+	proxyFunc := getProxyFunc()
+
+	healthCheckHttpClientTransport := http.DefaultTransport.(*http.Transport).Clone()
+	if proxyFunc != nil {
+		// Preserve the default proxy (from env vars) when no explicit proxy is configured.
+		healthCheckHttpClientTransport.Proxy = proxyFunc
+	}
+	healthCheckHttpClientTransport.TLSClientConfig = &tls.Config{
+		InsecureSkipVerify: true,
+	}
+
 	httpClientsFactory = &DefaultHttpClientsFactory{
 		k8s:            k8s,
 		logger:         logger,
 		systemCertPool: systemCertPool,
+		proxyFunc:      proxyFunc,
+		healthCheckHttpClient: &http.Client{
+			Transport: healthCheckHttpClientTransport,
+			Timeout:   500 * time.Millisecond,
+		},
 	}
 
 	return nil
@@ -97,13 +112,7 @@ func (h *DefaultHttpClientsFactory) GetHttpClient(ctx context.Context, routingCo
 	defer h.mu.Unlock()
 
 	if h.shouldCreateHttpClient(routingConfig, certsCM) {
-		h.httpClient = h.createHttpClient(routingConfig, certsCM)
-
-		if routingConfig == nil {
-			h.httpClientProxyConfig = nil
-		} else {
-			h.httpClientProxyConfig = routingConfig.ProxyConfig.DeepCopy()
-		}
+		h.httpClient = h.createHttpClient(certsCM)
 
 		if certsCM == nil {
 			h.httpClientCertsVersion = ""
@@ -120,9 +129,12 @@ func (h *DefaultHttpClientsFactory) GetHttpClient(ctx context.Context, routingCo
 	return h.httpClient
 }
 
-func (h *DefaultHttpClientsFactory) createHttpClient(routingConfig *controller.RoutingConfig, certsCM *corev1.ConfigMap) *http.Client {
+func (h *DefaultHttpClientsFactory) createHttpClient(certsCM *corev1.ConfigMap) *http.Client {
 	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.Proxy = h.getProxyFunc(routingConfig)
+	if h.proxyFunc != nil {
+		// Preserve the default proxy (from env vars) when no explicit proxy is configured.
+		transport.Proxy = h.proxyFunc
+	}
 	transport.TLSClientConfig = &tls.Config{
 		RootCAs: h.getCaCertPool(certsCM),
 	}
@@ -140,7 +152,6 @@ func (h *DefaultHttpClientsFactory) shouldCreateHttpClient(routingConfig *contro
 
 	var certsVersion string
 	var configmapRef *controller.ConfigmapReference
-	var proxyConfig *controller.Proxy
 
 	if certsCM != nil {
 		certsVersion = certsCM.ResourceVersion
@@ -150,88 +161,46 @@ func (h *DefaultHttpClientsFactory) shouldCreateHttpClient(routingConfig *contro
 		}
 	}
 
-	if routingConfig != nil {
-		proxyConfig = routingConfig.ProxyConfig
-	}
-
 	return certsVersion != h.httpClientCertsVersion ||
-		!reflect.DeepEqual(configmapRef, h.httpClientConfigmapRef) ||
-		!reflect.DeepEqual(proxyConfig, h.httpClientProxyConfig)
+		!reflect.DeepEqual(configmapRef, h.httpClientConfigmapRef)
 }
 
-func (h *DefaultHttpClientsFactory) GetHealthCheckHttpClient(routingConfig *controller.RoutingConfig) *http.Client {
+func (h *DefaultHttpClientsFactory) GetHealthCheckHttpClient() *http.Client {
 	h.mu.RLock()
-	if !h.shouldCreateHealthCheckHttpClient(routingConfig) {
-		defer h.mu.RUnlock()
-		return h.healthCheckHttpClient
-	}
-	h.mu.RUnlock()
-
-	h.mu.Lock()
-	defer h.mu.Unlock()
-
-	if h.shouldCreateHealthCheckHttpClient(routingConfig) {
-		h.healthCheckHttpClient = h.createHealthCheckHttpClient(routingConfig)
-
-		if routingConfig == nil {
-			h.healthCheckHttpClientProxyConfig = nil
-		} else {
-			h.healthCheckHttpClientProxyConfig = routingConfig.ProxyConfig.DeepCopy()
-		}
-	}
+	defer h.mu.RUnlock()
 
 	return h.healthCheckHttpClient
 }
 
-func (h *DefaultHttpClientsFactory) shouldCreateHealthCheckHttpClient(routingConfig *controller.RoutingConfig) bool {
-	if h.healthCheckHttpClient == nil {
-		return true
-	}
-
-	var proxyConfig *controller.Proxy
-
-	if routingConfig != nil {
-		proxyConfig = routingConfig.ProxyConfig
-	}
-
-	return !reflect.DeepEqual(proxyConfig, h.healthCheckHttpClientProxyConfig)
-}
-
-func (h *DefaultHttpClientsFactory) createHealthCheckHttpClient(routingConfig *controller.RoutingConfig) *http.Client {
-	transport := http.DefaultTransport.(*http.Transport).Clone()
-	transport.Proxy = h.getProxyFunc(routingConfig)
-	transport.TLSClientConfig = &tls.Config{
-		InsecureSkipVerify: true,
-	}
-
-	return &http.Client{
-		Transport: transport,
-		Timeout:   500 * time.Millisecond,
-	}
-}
-
-// getProxyFunc returns a proxy function based on the proxy settings in routingConfig.
+// getProxyFunc returns a proxy function based on the global operator configuration.
 // Returns nil if no proxy is configured; a nil proxy func causes the HTTP transport to
 // use the default proxy settings from environment variables.
-func (h *DefaultHttpClientsFactory) getProxyFunc(routingConfig *controller.RoutingConfig) func(*http.Request) (*url.URL, error) {
-	if routingConfig == nil || routingConfig.ProxyConfig == nil {
-		return nil
-	}
+func getProxyFunc() func(*http.Request) (*url.URL, error) {
+	globalConfig := config.GetGlobalConfig()
 
-	proxyConfig := httpproxy.Config{}
-	if routingConfig.ProxyConfig.HttpProxy != nil {
-		proxyConfig.HTTPProxy = *routingConfig.ProxyConfig.HttpProxy
-	}
-	if routingConfig.ProxyConfig.HttpsProxy != nil {
-		proxyConfig.HTTPSProxy = *routingConfig.ProxyConfig.HttpsProxy
-	}
-	if routingConfig.ProxyConfig.NoProxy != nil {
-		proxyConfig.NoProxy = *routingConfig.ProxyConfig.NoProxy
-	}
+	if globalConfig.Routing != nil && globalConfig.Routing.ProxyConfig != nil {
+		proxyConf := httpproxy.Config{}
+		if globalConfig.Routing.ProxyConfig.HttpProxy != nil {
+			proxyConf.HTTPProxy = *globalConfig.Routing.ProxyConfig.HttpProxy
+		}
+		if globalConfig.Routing.ProxyConfig.HttpsProxy != nil {
+			proxyConf.HTTPSProxy = *globalConfig.Routing.ProxyConfig.HttpsProxy
+		}
+		if globalConfig.Routing.ProxyConfig.NoProxy != nil {
+			proxyConf.NoProxy = *globalConfig.Routing.ProxyConfig.NoProxy
+		}
+
+		if proxyConf.HTTPProxy == "" && proxyConf.HTTPSProxy == "" {
+			return nil
+		}
 
-	return func(req *http.Request) (*url.URL, error) {
-		return proxyConfig.ProxyFunc()(req.URL)
+		proxyFn := proxyConf.ProxyFunc()
+		return func(req *http.Request) (*url.URL, error) {
+			return proxyFn(req.URL)
+		}
 	}
+
+	return nil
 }
 
 // getCaCertPool returns a CA cert pool that includes system certs and any additional certs from the ConfigMap.