Skip to content

Commit a4b9afd

Browse files
authored
fix: initialization HTTP clients with merged DevWorkspaceOperatorConfig
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
1 parent 32c3d0a commit a4b9afd

4 files changed

Lines changed: 447 additions & 67 deletions

File tree

controllers/workspace/devworkspace_controller.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,6 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
144144
reqLogger = reqLogger.WithValues(constants.DevWorkspaceIDLoggerKey, workspace.Status.DevWorkspaceId)
145145
reqLogger.Info("Reconciling Workspace", "resolvedConfig", configString)
146146

147-
// Inject ca certificates to the http client, if the certificates configmap is created and defined in the config.
148-
InjectCertificates(r.Client, r.Log)
149-
150147
// Check if the DevWorkspaceRouting instance is marked to be deleted, which is
151148
// indicated by the deletion timestamp being set.
152149
if workspace.GetDeletionTimestamp() != nil {
@@ -260,6 +257,8 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
260257
return reconcile.Result{Requeue: true}, err
261258
}
262259

260+
httpClient := httpClientsFactory.GetHttpClient(ctx, config.Routing)
261+
263262
flattenHelpers := flatten.ResolverTools{
264263
WorkspaceNamespace: workspace.Namespace,
265264
Context: ctx,
@@ -788,7 +787,10 @@ func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace *
788787
}
789788

790789
func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error {
791-
setupHttpClients(mgr.GetClient(), mgr.GetLogger())
790+
err := SetupHttpClientsFactory(mgr.GetClient(), mgr.GetLogger())
791+
if err != nil {
792+
return err
793+
}
792794

793795
maxConcurrentReconciles, err := wkspConfig.GetMaxConcurrentReconciles()
794796
if err != nil {

controllers/workspace/http.go

Lines changed: 215 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2019-2025 Red Hat, Inc.
1+
// Copyright (c) 2019-2026 Red Hat, Inc.
22
// Licensed under the Apache License, Version 2.0 (the "License");
33
// you may not use this file except in compliance with the License.
44
// You may obtain a copy of the License at
@@ -17,12 +17,14 @@ import (
1717
"context"
1818
"crypto/tls"
1919
"crypto/x509"
20+
"fmt"
2021
"net/http"
2122
"net/url"
23+
"reflect"
24+
"sync"
2225
"time"
2326

24-
"github.com/devfile/devworkspace-operator/pkg/config"
25-
27+
controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
2628
"k8s.io/apimachinery/pkg/types"
2729

2830
"github.com/go-logr/logr"
@@ -32,87 +34,242 @@ import (
3234
"golang.org/x/net/http/httpproxy"
3335
)
3436

35-
var (
37+
var httpClientsFactory HttpClientsFactory
38+
39+
type HttpClientsFactory interface {
40+
// GetHttpClient returns an HTTP client configured with proxy, TLS, and custom CA certificates
41+
// from routingConfig.
42+
GetHttpClient(context.Context, *controller.RoutingConfig) *http.Client
43+
44+
// GetHealthCheckHttpClient returns an HTTP client that skips TLS verification.
45+
// This client MUST only be used for workspace health/readiness checks, not for
46+
// fetching external content or making security-sensitive requests.
47+
GetHealthCheckHttpClient(*controller.RoutingConfig) *http.Client
48+
}
49+
50+
// DefaultHttpClientsFactory is a thread-safe, caching implementation of HttpClientsFactory.
51+
// It caches one HTTP client and one health-check client, rebuilding either only when the
52+
// relevant routing configuration (proxy settings, TLS certificates) changes.
53+
type DefaultHttpClientsFactory struct {
54+
k8s client.Client
55+
logger logr.Logger
56+
3657
httpClient *http.Client
3758
healthCheckHttpClient *http.Client
38-
)
3959

40-
func setupHttpClients(k8s client.Client, logger logr.Logger) {
41-
transport := http.DefaultTransport.(*http.Transport).Clone()
42-
healthCheckTransport := http.DefaultTransport.(*http.Transport).Clone()
43-
healthCheckTransport.TLSClientConfig = &tls.Config{
44-
InsecureSkipVerify: true,
60+
mu sync.RWMutex
61+
62+
httpClientProxyConfig *controller.Proxy
63+
httpClientConfigmapRef *controller.ConfigmapReference
64+
httpClientCertsVersion string
65+
66+
healthCheckHttpClientProxyConfig *controller.Proxy
67+
68+
systemCertPool *x509.CertPool
69+
}
70+
71+
func SetupHttpClientsFactory(k8s client.Client, logger logr.Logger) error {
72+
systemCertPool, err := x509.SystemCertPool()
73+
if err != nil {
74+
return fmt.Errorf("failed to load system cert pool: %w", err)
4575
}
4676

47-
globalConfig := config.GetGlobalConfig()
77+
httpClientsFactory = &DefaultHttpClientsFactory{
78+
k8s: k8s,
79+
logger: logger,
80+
systemCertPool: systemCertPool,
81+
}
4882

49-
if globalConfig.Routing != nil && globalConfig.Routing.ProxyConfig != nil {
50-
proxyConf := httpproxy.Config{}
51-
if globalConfig.Routing.ProxyConfig.HttpProxy != nil {
52-
proxyConf.HTTPProxy = *globalConfig.Routing.ProxyConfig.HttpProxy
53-
}
54-
if globalConfig.Routing.ProxyConfig.HttpsProxy != nil {
55-
proxyConf.HTTPSProxy = *globalConfig.Routing.ProxyConfig.HttpsProxy
56-
}
57-
if globalConfig.Routing.ProxyConfig.NoProxy != nil {
58-
proxyConf.NoProxy = *globalConfig.Routing.ProxyConfig.NoProxy
83+
return nil
84+
}
85+
86+
func (h *DefaultHttpClientsFactory) GetHttpClient(ctx context.Context, routingConfig *controller.RoutingConfig) *http.Client {
87+
certsCM := h.readCertificates(ctx, routingConfig)
88+
89+
h.mu.RLock()
90+
if !h.shouldCreateHttpClient(routingConfig, certsCM) {
91+
defer h.mu.RUnlock()
92+
return h.httpClient
93+
}
94+
h.mu.RUnlock()
95+
96+
h.mu.Lock()
97+
defer h.mu.Unlock()
98+
99+
if h.shouldCreateHttpClient(routingConfig, certsCM) {
100+
h.httpClient = h.createHttpClient(routingConfig, certsCM)
101+
102+
if routingConfig == nil {
103+
h.httpClientProxyConfig = nil
104+
} else {
105+
h.httpClientProxyConfig = routingConfig.ProxyConfig.DeepCopy()
59106
}
60107

61-
proxyFunc := func(req *http.Request) (*url.URL, error) {
62-
return proxyConf.ProxyFunc()(req.URL)
108+
if certsCM == nil {
109+
h.httpClientCertsVersion = ""
110+
h.httpClientConfigmapRef = nil
111+
} else {
112+
h.httpClientCertsVersion = certsCM.ResourceVersion
113+
h.httpClientConfigmapRef = &controller.ConfigmapReference{
114+
Name: certsCM.Name,
115+
Namespace: certsCM.Namespace,
116+
}
63117
}
64-
transport.Proxy = proxyFunc
65-
healthCheckTransport.Proxy = proxyFunc
66118
}
67119

68-
httpClient = &http.Client{
120+
return h.httpClient
121+
}
122+
123+
func (h *DefaultHttpClientsFactory) createHttpClient(routingConfig *controller.RoutingConfig, certsCM *corev1.ConfigMap) *http.Client {
124+
transport := http.DefaultTransport.(*http.Transport).Clone()
125+
transport.Proxy = h.getProxyFunc(routingConfig)
126+
transport.TLSClientConfig = &tls.Config{
127+
RootCAs: h.getCaCertPool(certsCM),
128+
}
129+
130+
return &http.Client{
69131
Transport: transport,
132+
Timeout: 10 * time.Second,
70133
}
71-
healthCheckHttpClient = &http.Client{
72-
Transport: healthCheckTransport,
73-
Timeout: 500 * time.Millisecond,
134+
}
135+
136+
func (h *DefaultHttpClientsFactory) shouldCreateHttpClient(routingConfig *controller.RoutingConfig, certsCM *corev1.ConfigMap) bool {
137+
if h.httpClient == nil {
138+
return true
139+
}
140+
141+
var certsVersion string
142+
var configmapRef *controller.ConfigmapReference
143+
var proxyConfig *controller.Proxy
144+
145+
if certsCM != nil {
146+
certsVersion = certsCM.ResourceVersion
147+
configmapRef = &controller.ConfigmapReference{
148+
Name: certsCM.Name,
149+
Namespace: certsCM.Namespace,
150+
}
74151
}
75-
InjectCertificates(k8s, logger)
152+
153+
if routingConfig != nil {
154+
proxyConfig = routingConfig.ProxyConfig
155+
}
156+
157+
return certsVersion != h.httpClientCertsVersion ||
158+
!reflect.DeepEqual(configmapRef, h.httpClientConfigmapRef) ||
159+
!reflect.DeepEqual(proxyConfig, h.httpClientProxyConfig)
76160
}
77161

78-
func InjectCertificates(k8s client.Client, logger logr.Logger) {
79-
if certs, ok := readCertificates(k8s, logger); ok {
80-
for _, certsPem := range certs {
81-
injectCertificates([]byte(certsPem), httpClient.Transport.(*http.Transport), logger)
162+
func (h *DefaultHttpClientsFactory) GetHealthCheckHttpClient(routingConfig *controller.RoutingConfig) *http.Client {
163+
h.mu.RLock()
164+
if !h.shouldCreateHealthCheckHttpClient(routingConfig) {
165+
defer h.mu.RUnlock()
166+
return h.healthCheckHttpClient
167+
}
168+
h.mu.RUnlock()
169+
170+
h.mu.Lock()
171+
defer h.mu.Unlock()
172+
173+
if h.shouldCreateHealthCheckHttpClient(routingConfig) {
174+
h.healthCheckHttpClient = h.createHealthCheckHttpClient(routingConfig)
175+
176+
if routingConfig == nil {
177+
h.healthCheckHttpClientProxyConfig = nil
178+
} else {
179+
h.healthCheckHttpClientProxyConfig = routingConfig.ProxyConfig.DeepCopy()
82180
}
83181
}
182+
183+
return h.healthCheckHttpClient
84184
}
85185

86-
func readCertificates(k8s client.Client, logger logr.Logger) (map[string]string, bool) {
87-
configmapRef := config.GetGlobalConfig().Routing.TLSCertificateConfigmapRef
88-
if configmapRef == nil {
89-
return nil, false
186+
func (h *DefaultHttpClientsFactory) shouldCreateHealthCheckHttpClient(routingConfig *controller.RoutingConfig) bool {
187+
if h.healthCheckHttpClient == nil {
188+
return true
90189
}
91-
configMap := &corev1.ConfigMap{}
92-
namespacedName := &types.NamespacedName{
93-
Name: configmapRef.Name,
94-
Namespace: configmapRef.Namespace,
190+
191+
var proxyConfig *controller.Proxy
192+
193+
if routingConfig != nil {
194+
proxyConfig = routingConfig.ProxyConfig
95195
}
96-
err := k8s.Get(context.Background(), *namespacedName, configMap)
97-
if err != nil {
98-
logger.Error(err, "Failed to read configmap with certificates")
99-
return nil, false
196+
197+
return !reflect.DeepEqual(proxyConfig, h.healthCheckHttpClientProxyConfig)
198+
}
199+
200+
func (h *DefaultHttpClientsFactory) createHealthCheckHttpClient(routingConfig *controller.RoutingConfig) *http.Client {
201+
transport := http.DefaultTransport.(*http.Transport).Clone()
202+
transport.Proxy = h.getProxyFunc(routingConfig)
203+
transport.TLSClientConfig = &tls.Config{
204+
InsecureSkipVerify: true,
205+
}
206+
207+
return &http.Client{
208+
Transport: transport,
209+
Timeout: 500 * time.Millisecond,
100210
}
101-
return configMap.Data, true
102211
}
103212

104-
func injectCertificates(certsPem []byte, transport *http.Transport, logger logr.Logger) {
105-
caCertPool := transport.TLSClientConfig.RootCAs
106-
if caCertPool == nil {
107-
systemCertPool, err := x509.SystemCertPool()
108-
if err != nil {
109-
logger.Error(err, "Failed to load system cert pool")
110-
caCertPool = x509.NewCertPool()
111-
} else {
112-
caCertPool = systemCertPool
213+
// getProxyFunc returns a proxy function based on the proxy settings in routingConfig.
214+
// Returns nil if no proxy is configured; a nil proxy func causes the HTTP transport to
215+
// use the default proxy settings from environment variables.
216+
func (h *DefaultHttpClientsFactory) getProxyFunc(routingConfig *controller.RoutingConfig) func(*http.Request) (*url.URL, error) {
217+
if routingConfig == nil || routingConfig.ProxyConfig == nil {
218+
return nil
219+
}
220+
221+
proxyConfig := httpproxy.Config{}
222+
if routingConfig.ProxyConfig.HttpProxy != nil {
223+
proxyConfig.HTTPProxy = *routingConfig.ProxyConfig.HttpProxy
224+
}
225+
if routingConfig.ProxyConfig.HttpsProxy != nil {
226+
proxyConfig.HTTPSProxy = *routingConfig.ProxyConfig.HttpsProxy
227+
}
228+
if routingConfig.ProxyConfig.NoProxy != nil {
229+
proxyConfig.NoProxy = *routingConfig.ProxyConfig.NoProxy
230+
}
231+
232+
return func(req *http.Request) (*url.URL, error) {
233+
return proxyConfig.ProxyFunc()(req.URL)
234+
}
235+
}
236+
237+
// getCaCertPool returns a CA cert pool that includes system certs and any additional certs from the ConfigMap.
238+
// A nil pool causes the HTTP client to use the system default root CAs.
239+
func (h *DefaultHttpClientsFactory) getCaCertPool(certsCM *corev1.ConfigMap) *x509.CertPool {
240+
if certsCM == nil || len(certsCM.Data) == 0 {
241+
return nil
242+
}
243+
244+
caCertPool := h.systemCertPool.Clone()
245+
246+
for _, certsPem := range certsCM.Data {
247+
if !caCertPool.AppendCertsFromPEM([]byte(certsPem)) {
248+
h.logger.Error(fmt.Errorf("failed to parse one or more certificates from ConfigMap"), "Could not append CA certificates to pool")
113249
}
114250
}
115-
if ok := caCertPool.AppendCertsFromPEM(certsPem); ok {
116-
transport.TLSClientConfig = &tls.Config{RootCAs: caCertPool}
251+
252+
return caCertPool
253+
}
254+
255+
func (h *DefaultHttpClientsFactory) readCertificates(ctx context.Context, routingConfig *controller.RoutingConfig) *corev1.ConfigMap {
256+
if routingConfig == nil || routingConfig.TLSCertificateConfigmapRef == nil {
257+
return nil
258+
}
259+
260+
configmapRef := routingConfig.TLSCertificateConfigmapRef
261+
262+
namespacedName := types.NamespacedName{
263+
Name: configmapRef.Name,
264+
Namespace: configmapRef.Namespace,
117265
}
266+
267+
configMap := &corev1.ConfigMap{}
268+
if err := h.k8s.Get(ctx, namespacedName, configMap); err != nil {
269+
// print and ignore the error, http clients will be created with host's root CA set.
270+
h.logger.Error(err, "Failed to read ConfigMap containing certificates", "namespace", configmapRef.Namespace, "name", configmapRef.Name)
271+
return nil
272+
}
273+
274+
return configMap
118275
}

0 commit comments

Comments
 (0)