Allow cluster-detected proxy settings to be ignored in OpenShift

Change httpProxy, httpsProxy, and noProxy fields in DWOC's proxyConfig
field to be string pointers rather than strings. This allows
distinguishing between "not set" and "set to the empty string" in the
DevWorkspace Operator config, which is necessary to allow the admin to
"disable" automatically-detected proxy settings by setting them to the
empty string.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go

@@ -54,7 +54,8 @@ type RoutingConfig struct {
 	// On OpenShift, the operator automatically reads values from the "cluster" proxies.config.openshift.io
 	// object and this value only needs to be set to override those defaults. Values for httpProxy
 	// and httpsProxy override the cluster configuration directly. Entries for noProxy are merged
-	// with the noProxy values in the cluster configuration.
+	// with the noProxy values in the cluster configuration. To ignore automatically read values from the cluster,
+	// set values in fields to the empty string ("")
 	//
 	// Changes to the proxy configuration are detected by the DevWorkspace Operator and propagated to
 	// DevWorkspaces. However, changing the proxy configuration for the DevWorkspace Operator itself
@@ -154,13 +155,16 @@ type PersistentHomeConfig struct {
 }
 
 type Proxy struct {
-	// HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/
-	HttpProxy string `json:"httpProxy,omitempty"`
-	// HttpsProxy is the URL of the proxy for HTTPS requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/
-	HttpsProxy string `json:"httpsProxy,omitempty"`
+	// HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/. To ignore
+	// automatically detected proxy settings for the cluster, set this field to an empty string ("")
+	HttpProxy *string `json:"httpProxy,omitempty"`
+	// HttpsProxy is the URL of the proxy for HTTPS requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/. To ignore
+	// automatically detected proxy settings for the cluster, set this field to an empty string ("")
+	HttpsProxy *string `json:"httpsProxy,omitempty"`
 	// NoProxy is a comma-separated list of hostnames and/or CIDRs for which the proxy should not be used. Ignored
-	// when HttpProxy and HttpsProxy are unset
-	NoProxy string `json:"noProxy,omitempty"`
+	// when HttpProxy and HttpsProxy are unset. To ignore automatically detected proxy settings for the cluster, set this
+	// field to an empty string ("")
+	NoProxy *string `json:"noProxy,omitempty"`
 }
 
 type StorageSizes struct {

controllers/workspace/http.go

@@ -37,11 +37,17 @@ func setupHttpClients() {
 	globalConfig := config.GetGlobalConfig()
 
 	if globalConfig.Routing != nil && globalConfig.Routing.ProxyConfig != nil {
-		proxyConf := httpproxy.Config{
-			HTTPProxy:  globalConfig.Routing.ProxyConfig.HttpProxy,
-			HTTPSProxy: globalConfig.Routing.ProxyConfig.HttpsProxy,
-			NoProxy:    globalConfig.Routing.ProxyConfig.NoProxy,
+		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
+		}
+
 		proxyFunc := func(req *http.Request) (*url.URL, error) {
 			return proxyConf.ProxyFunc()(req.URL)
 		}

pkg/config/proxy/openshift.go

@@ -51,9 +51,9 @@ func GetClusterProxyConfig(nonCachedClient crclient.Client) (*controller.Proxy,
 	}
 
 	proxyConfig := &controller.Proxy{
-		HttpProxy:  proxy.Status.HTTPProxy,
-		HttpsProxy: proxy.Status.HTTPSProxy,
-		NoProxy:    proxy.Status.NoProxy,
+		HttpProxy:  &proxy.Status.HTTPProxy,
+		HttpsProxy: &proxy.Status.HTTPSProxy,
+		NoProxy:    &proxy.Status.NoProxy,
 	}
 
 	return proxyConfig, nil
@@ -63,31 +63,53 @@ func GetClusterProxyConfig(nonCachedClient crclient.Client) (*controller.Proxy,
 // operator configuration taking precedence. Accepts nil arguments. If both arguments are nil, returns nil.
 func MergeProxyConfigs(operatorConfig, clusterConfig *controller.Proxy) *controller.Proxy {
 	if clusterConfig == nil {
-		return operatorConfig
+		return removeEmptyStrings(operatorConfig)
 	}
 	if operatorConfig == nil {
-		return clusterConfig
+		return removeEmptyStrings(clusterConfig)
 	}
+
 	mergedProxy := &controller.Proxy{
 		HttpProxy:  operatorConfig.HttpProxy,
 		HttpsProxy: operatorConfig.HttpsProxy,
 		NoProxy:    operatorConfig.NoProxy,
 	}
 
-	if mergedProxy.HttpProxy == "" {
+	if mergedProxy.HttpProxy == nil {
 		mergedProxy.HttpProxy = clusterConfig.HttpProxy
 	}
-	if mergedProxy.HttpsProxy == "" {
+	if mergedProxy.HttpsProxy == nil {
 		mergedProxy.HttpsProxy = clusterConfig.HttpsProxy
 	}
-	if mergedProxy.NoProxy == "" {
+	if mergedProxy.NoProxy == nil {
 		mergedProxy.NoProxy = clusterConfig.NoProxy
-	} else {
+	} else if *mergedProxy.NoProxy != "" {
 		// Merge noProxy fields, joining with a comma
-		if clusterConfig.NoProxy != "" {
-			mergedProxy.NoProxy = fmt.Sprintf("%s,%s", clusterConfig.NoProxy, operatorConfig.NoProxy)
+		if clusterConfig.NoProxy != nil {
+			noProxy := fmt.Sprintf("%s,%s", *clusterConfig.NoProxy, *operatorConfig.NoProxy)
+			mergedProxy.NoProxy = &noProxy
 		}
 	}
 
-	return mergedProxy
+	return removeEmptyStrings(mergedProxy)
+}
+
+// removeEmptyStrings is a utility function for removing empty fields from a proxy configuration. This is required
+// to allow overriding
+func removeEmptyStrings(proxyConfig *controller.Proxy) *controller.Proxy {
+	if proxyConfig == nil {
+		return nil
+	}
+
+	updated := &controller.Proxy{}
+	if proxyConfig.HttpProxy != nil && *proxyConfig.HttpProxy != "" {
+		updated.HttpProxy = proxyConfig.HttpProxy
+	}
+	if proxyConfig.HttpsProxy != nil && *proxyConfig.HttpsProxy != "" {
+		updated.HttpsProxy = proxyConfig.HttpsProxy
+	}
+	if proxyConfig.NoProxy != nil && *proxyConfig.NoProxy != "" {
+		updated.NoProxy = proxyConfig.NoProxy
+	}
+	return updated
 }

pkg/library/env/workspaceenv.go

@@ -81,26 +81,27 @@ func GetProxyEnvVars(proxyConfig *v1alpha1.Proxy) []corev1.EnvVar {
 		return nil
 	}
 
-	if proxyConfig.HttpProxy == "" && proxyConfig.HttpsProxy == "" {
+	// If httpProxy and httpsProxy are both unset, ignore any value for noProxy``
+	if proxyConfig.HttpProxy == nil && proxyConfig.HttpsProxy == nil {
 		return nil
 	}
 
 	// Proxy env vars are defined by consensus rather than standard; most tools use the lower-snake-case version
 	// but some may only look at the upper-snake-case version, so we add both.
 	var env []v1.EnvVar
-	if proxyConfig.HttpProxy != "" {
-		env = append(env, v1.EnvVar{Name: "http_proxy", Value: proxyConfig.HttpProxy})
-		env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: proxyConfig.HttpProxy})
+	if proxyConfig.HttpProxy != nil {
+		env = append(env, v1.EnvVar{Name: "http_proxy", Value: *proxyConfig.HttpProxy})
+		env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: *proxyConfig.HttpProxy})
 	}
-	if proxyConfig.HttpsProxy != "" {
-		env = append(env, v1.EnvVar{Name: "https_proxy", Value: proxyConfig.HttpsProxy})
-		env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: proxyConfig.HttpsProxy})
+	if proxyConfig.HttpsProxy != nil {
+		env = append(env, v1.EnvVar{Name: "https_proxy", Value: *proxyConfig.HttpsProxy})
+		env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: *proxyConfig.HttpsProxy})
 	}
-	if proxyConfig.NoProxy != "" {
+	if proxyConfig.NoProxy != nil {
 		// Adding 'KUBERNETES_SERVICE_HOST' env var to the 'no_proxy / NO_PROXY' list. Hot Fix for https://issues.redhat.com/browse/CRW-2820
 		kubernetesServiceHost := os.Getenv("KUBERNETES_SERVICE_HOST")
-		env = append(env, v1.EnvVar{Name: "no_proxy", Value: proxyConfig.NoProxy + "," + kubernetesServiceHost})
-		env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: proxyConfig.NoProxy + "," + kubernetesServiceHost})
+		env = append(env, v1.EnvVar{Name: "no_proxy", Value: *proxyConfig.NoProxy + "," + kubernetesServiceHost})
+		env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: *proxyConfig.NoProxy + "," + kubernetesServiceHost})
 	}
 
 	return env