From ff8a409c290ed1b82fff24b89f6bd3e43145eff3 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Fri, 10 Dec 2021 18:33:52 -0500 Subject: [PATCH 1/9] Read cluster proxy on OpenShift and propagate settings to workspaces On OpenShift, read the cluster proxy configuration and propagate the proxy configuration values to workspaces as env vars. Signed-off-by: Angel Misevski --- main.go | 3 ++ pkg/config/proxy/openshift.go | 62 ++++++++++++++++++++++++++++ pkg/config/sync.go | 9 ++++ pkg/provision/workspace/commonenv.go | 35 +++++++++++++++- 4 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 pkg/config/proxy/openshift.go diff --git a/main.go b/main.go index 2d1c0210a..fb447e034 100644 --- a/main.go +++ b/main.go @@ -36,6 +36,7 @@ import ( controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" workspacecontroller "github.com/devfile/devworkspace-operator/controllers/workspace" + configv1 "github.com/openshift/api/config/v1" oauthv1 "github.com/openshift/api/oauth/v1" routev1 "github.com/openshift/api/route/v1" securityv1 "github.com/openshift/api/security/v1" @@ -79,6 +80,8 @@ func init() { // Enable controller to manage SCCs in OpenShift; permissions to do this are not requested // by default and must be added by a cluster-admin. utilruntime.Must(securityv1.Install(scheme)) + // Enable controller to read cluster-wide proxy on OpenShift + utilruntime.Must(configv1.AddToScheme(scheme)) } // +kubebuilder:scaffold:scheme diff --git a/pkg/config/proxy/openshift.go b/pkg/config/proxy/openshift.go new file mode 100644 index 000000000..1ceae11f0 --- /dev/null +++ b/pkg/config/proxy/openshift.go @@ -0,0 +1,62 @@ +// Copyright (c) 2019-2021 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package proxy + +import ( + "context" + + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + "github.com/go-logr/logr" + configv1 "github.com/openshift/api/config/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + crclient "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + openshiftClusterProxyName = "cluster" +) + +type Proxy struct { + // HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + HttpProxy string + // HttpsProxy is the URL of the proxy for HTTPS requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + HttpsProxy string + // 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 +} + +func GetOpenShiftClusterProxyConfig(nonCachedClient crclient.Client, log logr.Logger) (*Proxy, error) { + if !infrastructure.IsOpenShift() { + return nil, nil + } + proxy := &configv1.Proxy{} + err := nonCachedClient.Get(context.Background(), types.NamespacedName{Name: openshiftClusterProxyName}, proxy) + if err != nil { + if k8sErrors.IsNotFound(err) { + return nil, nil + } + return nil, err + } + + proxyConfig := &Proxy{ + HttpProxy: proxy.Status.HTTPProxy, + HttpsProxy: proxy.Status.HTTPSProxy, + NoProxy: proxy.Status.NoProxy, + } + log.Info("Read proxy configuration", "config", proxyConfig) + + return proxyConfig, nil +} diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 82f05e5ad..625f9d817 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -21,6 +21,7 @@ import ( "strings" "sync" + "github.com/devfile/devworkspace-operator/pkg/config/proxy" routeV1 "github.com/openshift/api/route/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -44,6 +45,9 @@ var ( configMutex sync.Mutex configNamespace string log = ctrl.Log.WithName("operator-configuration") + + // Proxy stores the current proxy configuration, if one is defined. This is a parsed form for easier access + Proxy *proxy.Proxy ) func SetConfigForTesting(config *controller.OperatorConfiguration) { @@ -83,6 +87,11 @@ func SetupControllerConfig(client crclient.Client) error { internalConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix updatePublicConfig() } + clusterProxy, err := proxy.GetOpenShiftClusterProxyConfig(client, log) + if err != nil { + return err + } + Proxy = clusterProxy return nil } diff --git a/pkg/provision/workspace/commonenv.go b/pkg/provision/workspace/commonenv.go index be7718ab6..986524184 100644 --- a/pkg/provision/workspace/commonenv.go +++ b/pkg/provision/workspace/commonenv.go @@ -20,13 +20,14 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" ) func CommonEnvironmentVariables(workspaceName, workspaceId, namespace, creator string) []corev1.EnvVar { - return []corev1.EnvVar{ + envvars := []corev1.EnvVar{ { Name: constants.DevWorkspaceNamespace, Value: namespace, @@ -48,6 +49,38 @@ func CommonEnvironmentVariables(workspaceName, workspaceId, namespace, creator s Value: config.Workspace.IdleTimeout, }, } + + envvars = append(envvars, getProxyEnvVars()...) + + return envvars +} + +func getProxyEnvVars() []corev1.EnvVar { + if config.Proxy == nil { + return nil + } + + if config.Proxy.HttpProxy == "" && config.Proxy.HttpsProxy == "" { + 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 config.Proxy.HttpProxy != "" { + env = append(env, v1.EnvVar{Name: "http_proxy", Value: config.Proxy.HttpProxy}) + env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: config.Proxy.HttpProxy}) + } + if config.Proxy.HttpsProxy != "" { + env = append(env, v1.EnvVar{Name: "https_proxy", Value: config.Proxy.HttpsProxy}) + env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: config.Proxy.HttpsProxy}) + } + if config.Proxy.NoProxy != "" { + env = append(env, v1.EnvVar{Name: "no_proxy", Value: config.Proxy.NoProxy}) + env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: config.Proxy.NoProxy}) + } + + return env } func collectWorkspaceEnv(flattenedDW *dw.DevWorkspaceTemplateSpec) ([]corev1.EnvVar, error) { From 5c926e025595faeed7ed37ada891350553842024 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Mon, 13 Dec 2021 18:04:27 -0500 Subject: [PATCH 2/9] Add RBAC for reading the cluster proxy on OpenShift Signed-off-by: Angel Misevski --- controllers/workspace/devworkspace_controller.go | 1 + .../devworkspace-operator.clusterserviceversion.yaml | 8 ++++++++ deploy/deployment/kubernetes/combined.yaml | 8 ++++++++ .../objects/devworkspace-controller-role.ClusterRole.yaml | 8 ++++++++ deploy/deployment/openshift/combined.yaml | 8 ++++++++ .../objects/devworkspace-controller-role.ClusterRole.yaml | 8 ++++++++ deploy/templates/components/rbac/role.yaml | 8 ++++++++ 7 files changed, 49 insertions(+) diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 9ade093fb..f1bb7af99 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -88,6 +88,7 @@ type DevWorkspaceReconciler struct { // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings;clusterroles;clusterrolebindings,verbs=get;list;watch;create;update // +kubebuilder:rbac:groups=oauth.openshift.io,resources=oauthclients,verbs=get;list;watch;create;update;patch;delete;deletecollection // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors,verbs=get;create +// +kubebuilder:rbac:groups=config.openshift.io,resources=proxies,verbs=get,resourceNames=cluster // +kubebuilder:rbac:groups=apps,resourceNames=devworkspace-controller,resources=deployments/finalizers,verbs=update /////// Required permissions for workspace ServiceAccount // +kubebuilder:rbac:groups="",resources=pods/exec,verbs=create diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index bfae0755e..9feb578a2 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -199,6 +199,14 @@ spec: - patch - update - watch + - apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index b77a756f4..af7fa3b6b 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -19827,6 +19827,14 @@ rules: - patch - update - watch +- apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index cb9ddd9a6..a292a209e 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -126,6 +126,14 @@ rules: - patch - update - watch +- apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 0d855bb57..28fe89825 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -19827,6 +19827,14 @@ rules: - patch - update - watch +- apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index cb9ddd9a6..a292a209e 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -126,6 +126,14 @@ rules: - patch - update - watch +- apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index 0871040a6..c9b8621a0 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -125,6 +125,14 @@ rules: - patch - update - watch +- apiGroups: + - config.openshift.io + resourceNames: + - cluster + resources: + - proxies + verbs: + - get - apiGroups: - controller.devfile.io resources: From c28b686dafc5fd41dad06911c2651fe14be09a5e Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 14 Dec 2021 12:13:10 -0500 Subject: [PATCH 3/9] Move proxy to config CR, allow operator config to override values Move the definition of the proxy configuration into the DevWorkspaceOperatorConfigs CRD and enable values there to override cluster proxy settings. Signed-off-by: Angel Misevski --- .../devworkspaceoperatorconfig_types.go | 18 ++++++ pkg/config/proxy/openshift.go | 59 ++++++++++++++----- pkg/config/sync.go | 9 ++- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 311675288..d21d756db 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -42,6 +42,24 @@ type RoutingConfig struct { // On OpenShift, the DevWorkspace Operator will attempt to determine the appropriate // value automatically. Must be specified on Kubernetes. ClusterHostSuffix string `json:"clusterHostSuffix,omitempty"` + // ProxyConfig defines the proxy settings that should be used for all DevWorkspaces. + // These values are propagated to workspace containers as environment variables. + // + // 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. + ProxyConfig *Proxy `json:"proxyConfig,omitempty"` +} + +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"` + // 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"` } type WorkspaceConfig struct { diff --git a/pkg/config/proxy/openshift.go b/pkg/config/proxy/openshift.go index 1ceae11f0..8e213f7af 100644 --- a/pkg/config/proxy/openshift.go +++ b/pkg/config/proxy/openshift.go @@ -15,9 +15,10 @@ package proxy import ( "context" + "fmt" + controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/infrastructure" - "github.com/go-logr/logr" configv1 "github.com/openshift/api/config/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" @@ -28,17 +29,10 @@ const ( openshiftClusterProxyName = "cluster" ) -type Proxy struct { - // HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ - HttpProxy string - // HttpsProxy is the URL of the proxy for HTTPS requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ - HttpsProxy string - // 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 -} - -func GetOpenShiftClusterProxyConfig(nonCachedClient crclient.Client, log logr.Logger) (*Proxy, error) { +// GetClusterProxyConfig reads a proxy configuration from the "cluster" proxies.config.openshift.io on +// OpenShift. If running in a non-OpenShift cluster, returns (nil, nil). If the cluster proxy is empty, returns +// (nil, nil) +func GetClusterProxyConfig(nonCachedClient crclient.Client) (*controller.Proxy, error) { if !infrastructure.IsOpenShift() { return nil, nil } @@ -46,17 +40,54 @@ func GetOpenShiftClusterProxyConfig(nonCachedClient crclient.Client, log logr.Lo err := nonCachedClient.Get(context.Background(), types.NamespacedName{Name: openshiftClusterProxyName}, proxy) if err != nil { if k8sErrors.IsNotFound(err) { + // Should never happen as OpenShift cluster proxy is always present return nil, nil } return nil, err } - proxyConfig := &Proxy{ + if proxy.Status.HTTPProxy == "" && proxy.Status.HTTPSProxy == "" && proxy.Status.NoProxy == "" { + return nil, nil + } + + proxyConfig := &controller.Proxy{ HttpProxy: proxy.Status.HTTPProxy, HttpsProxy: proxy.Status.HTTPSProxy, NoProxy: proxy.Status.NoProxy, } - log.Info("Read proxy configuration", "config", proxyConfig) return proxyConfig, nil } + +// MergeProxyConfigs merges proxy configurations from the operator and the cluster and merges them, with the +// 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 + } + if operatorConfig == nil { + return clusterConfig + } + mergedProxy := &controller.Proxy{ + HttpProxy: operatorConfig.HttpProxy, + HttpsProxy: operatorConfig.HttpsProxy, + NoProxy: operatorConfig.NoProxy, + } + + if mergedProxy.HttpProxy == "" { + mergedProxy.HttpProxy = clusterConfig.HttpProxy + } + if mergedProxy.HttpsProxy == "" { + mergedProxy.HttpsProxy = clusterConfig.HttpsProxy + } + if mergedProxy.NoProxy == "" { + mergedProxy.NoProxy = clusterConfig.NoProxy + } else { + // Merge noProxy fields, joining with a comma + if clusterConfig.NoProxy != "" { + mergedProxy.NoProxy = fmt.Sprintf("%s,%s", clusterConfig.NoProxy, operatorConfig.NoProxy) + } + } + + return mergedProxy +} diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 625f9d817..34d7425ca 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -47,7 +47,7 @@ var ( log = ctrl.Log.WithName("operator-configuration") // Proxy stores the current proxy configuration, if one is defined. This is a parsed form for easier access - Proxy *proxy.Proxy + Proxy *controller.Proxy ) func SetConfigForTesting(config *controller.OperatorConfiguration) { @@ -87,11 +87,14 @@ func SetupControllerConfig(client crclient.Client) error { internalConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix updatePublicConfig() } - clusterProxy, err := proxy.GetOpenShiftClusterProxyConfig(client, log) + + clusterProxy, err := proxy.GetClusterProxyConfig(client) if err != nil { return err } - Proxy = clusterProxy + Proxy = proxy.MergeProxyConfigs(internalConfig.Routing.ProxyConfig, clusterProxy) + log.Info("Resolved proxy configuration", "proxy", Proxy) + return nil } From cd864aab36a7b13987e320fb440faa765d00cafe Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 14 Dec 2021 13:57:15 -0500 Subject: [PATCH 4/9] Regenerate files to include proxy settings in config CR Signed-off-by: Angel Misevski --- .../v1alpha1/zz_generated.deepcopy.go | 22 +++++++++++++++- ...evfile.io_devworkspaceoperatorconfigs.yaml | 13 ++++++++++ deploy/deployment/kubernetes/combined.yaml | 25 +++++++++++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 25 +++++++++++++++++++ deploy/deployment/openshift/combined.yaml | 25 +++++++++++++++++++ ...r.devfile.io.CustomResourceDefinition.yaml | 25 +++++++++++++++++++ ...evfile.io_devworkspaceoperatorconfigs.yaml | 25 +++++++++++++++++++ 7 files changed, 159 insertions(+), 1 deletion(-) diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index 5cef25d28..84ff330f5 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -349,7 +349,7 @@ func (in *OperatorConfiguration) DeepCopyInto(out *OperatorConfiguration) { if in.Routing != nil { in, out := &in.Routing, &out.Routing *out = new(RoutingConfig) - **out = **in + (*in).DeepCopyInto(*out) } if in.Workspace != nil { in, out := &in.Workspace, &out.Workspace @@ -442,9 +442,29 @@ func (in *PodAdditions) DeepCopy() *PodAdditions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Proxy) DeepCopyInto(out *Proxy) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Proxy. +func (in *Proxy) DeepCopy() *Proxy { + if in == nil { + return nil + } + out := new(Proxy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RoutingConfig) DeepCopyInto(out *RoutingConfig) { *out = *in + if in.ProxyConfig != nil { + in, out := &in.ProxyConfig, &out.ProxyConfig + *out = new(Proxy) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutingConfig. diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index b1d2ba42d..4f02b8448 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -42,6 +42,19 @@ spec: defaultRoutingClass: description: DefaultRoutingClass specifies the routingClass to be used when a DevWorkspace specifies an empty `.spec.routingClass`. Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should be used for all DevWorkspaces. These values are propagated to workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how DevWorkspaces are managed diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index af7fa3b6b..8bb6ed913 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -55,6 +55,31 @@ spec: Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should + be used for all DevWorkspaces. These values are propagated to + workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, + in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS + requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how diff --git a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 4a2d0d493..83f25ca69 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -55,6 +55,31 @@ spec: Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should + be used for all DevWorkspaces. These values are propagated to + workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, + in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS + requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 28fe89825..29b3ce26a 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -55,6 +55,31 @@ spec: Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should + be used for all DevWorkspaces. These values are propagated to + workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, + in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS + requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how diff --git a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 4a2d0d493..83f25ca69 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -55,6 +55,31 @@ spec: Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should + be used for all DevWorkspaces. These values are propagated to + workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, + in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS + requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 696cb60b9..2db44a3aa 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -54,6 +54,31 @@ spec: Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string + proxyConfig: + description: "ProxyConfig defines the proxy settings that should + be used for all DevWorkspaces. These values are propagated to + workspace containers as environment variables. \n 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." + properties: + httpProxy: + description: HttpProxy is the URL of the proxy for HTTP requests, + in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + httpsProxy: + description: HttpsProxy is the URL of the proxy for HTTPS + requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ + type: string + noProxy: + description: 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 + type: string + type: object type: object workspace: description: Workspace defines configuration options related to how From 065ffc975c649b1d6650fd7b906f67ee37b23144 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 14 Dec 2021 12:26:12 -0500 Subject: [PATCH 5/9] Update Proxy configuration when controller config is updated Signed-off-by: Angel Misevski --- pkg/config/sync.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 34d7425ca..95bf6e066 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -46,8 +46,11 @@ var ( configNamespace string log = ctrl.Log.WithName("operator-configuration") - // Proxy stores the current proxy configuration, if one is defined. This is a parsed form for easier access + // Proxy stores the current proxy configuration, if one is defined. Will be nil if no proxy settings are defined. Proxy *controller.Proxy + // Store the cluster proxy config if it is available to allow controller proxy to be updated while + // controller is running. + clusterProxyConfig *controller.Proxy ) func SetConfigForTesting(config *controller.OperatorConfiguration) { @@ -63,21 +66,23 @@ func SetupControllerConfig(client crclient.Client) error { return fmt.Errorf("internal controller configuration is already set up") } internalConfig = &controller.OperatorConfiguration{} + namespace, err := infrastructure.GetNamespace() if err != nil { return err } configNamespace = namespace + config, err := getClusterConfig(configNamespace, client) if err != nil { return err } if config == nil { internalConfig = DefaultConfig.DeepCopy() - updatePublicConfig() } else { syncConfigFrom(config) } + defaultRoutingSuffix, err := discoverRouteSuffix(client) if err != nil { return err @@ -85,16 +90,15 @@ func SetupControllerConfig(client crclient.Client) error { DefaultConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix if internalConfig.Routing.ClusterHostSuffix == "" { internalConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix - updatePublicConfig() } clusterProxy, err := proxy.GetClusterProxyConfig(client) if err != nil { return err } - Proxy = proxy.MergeProxyConfigs(internalConfig.Routing.ProxyConfig, clusterProxy) - log.Info("Resolved proxy configuration", "proxy", Proxy) + clusterProxyConfig = clusterProxy + updatePublicConfig() return nil } @@ -138,6 +142,15 @@ func updatePublicConfig() { Routing = internalConfig.Routing.DeepCopy() Workspace = internalConfig.Workspace.DeepCopy() log.Info(fmt.Sprintf("Updated config to [%s]", formatCurrentConfig())) + + if internalConfig.Routing == nil { + Proxy = clusterProxyConfig + } else { + Proxy = proxy.MergeProxyConfigs(internalConfig.Routing.ProxyConfig, clusterProxyConfig) + } + if Proxy != nil { + log.Info("Resolved proxy configuration", "proxy", Proxy) + } } // discoverRouteSuffix attempts to determine a clusterHostSuffix that is compatible with the current cluster. @@ -202,6 +215,12 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { if from.Routing.ClusterHostSuffix != "" { to.Routing.ClusterHostSuffix = from.Routing.ClusterHostSuffix } + if from.Routing.ProxyConfig != nil { + if to.Routing.ProxyConfig == nil { + to.Routing.ProxyConfig = &controller.Proxy{} + } + to.Routing.ProxyConfig = proxy.MergeProxyConfigs(from.Routing.ProxyConfig, DefaultConfig.Routing.ProxyConfig) + } } if from.Workspace != nil { if to.Workspace == nil { From 186facde6c8a61b568bac3b1c1d2da70e2cb5358 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 14 Dec 2021 14:28:29 -0500 Subject: [PATCH 6/9] Rework proxy configuration to just reuse config structs Now that Proxy is included in the DevWorkspaceOperatorConfig CRD, we can just store proxy config in the same way as all other config settings, with the cluster config providing the default values. Signed-off-by: Angel Misevski --- pkg/config/sync.go | 38 ++++++++++------------------ pkg/provision/workspace/commonenv.go | 22 ++++++++-------- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 95bf6e066..5df98db89 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -45,12 +45,6 @@ var ( configMutex sync.Mutex configNamespace string log = ctrl.Log.WithName("operator-configuration") - - // Proxy stores the current proxy configuration, if one is defined. Will be nil if no proxy settings are defined. - Proxy *controller.Proxy - // Store the cluster proxy config if it is available to allow controller proxy to be updated while - // controller is running. - clusterProxyConfig *controller.Proxy ) func SetConfigForTesting(config *controller.OperatorConfiguration) { @@ -88,15 +82,14 @@ func SetupControllerConfig(client crclient.Client) error { return err } DefaultConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix - if internalConfig.Routing.ClusterHostSuffix == "" { - internalConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix - } clusterProxy, err := proxy.GetClusterProxyConfig(client) if err != nil { return err } - clusterProxyConfig = clusterProxy + DefaultConfig.Routing.ProxyConfig = clusterProxy + + mergeConfig(DefaultConfig, internalConfig) updatePublicConfig() return nil @@ -141,16 +134,7 @@ func restoreDefaultConfig() { func updatePublicConfig() { Routing = internalConfig.Routing.DeepCopy() Workspace = internalConfig.Workspace.DeepCopy() - log.Info(fmt.Sprintf("Updated config to [%s]", formatCurrentConfig())) - - if internalConfig.Routing == nil { - Proxy = clusterProxyConfig - } else { - Proxy = proxy.MergeProxyConfigs(internalConfig.Routing.ProxyConfig, clusterProxyConfig) - } - if Proxy != nil { - log.Info("Resolved proxy configuration", "proxy", Proxy) - } + logCurrentConfig() } // discoverRouteSuffix attempts to determine a clusterHostSuffix that is compatible with the current cluster. @@ -247,10 +231,10 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { } } -// formatCurrentConfig formats the current operator configuration as a plain string -func formatCurrentConfig() string { +// logCurrentConfig formats the current operator configuration as a plain string +func logCurrentConfig() { if internalConfig == nil { - return "" + return } var config []string if Routing != nil { @@ -283,7 +267,11 @@ func formatCurrentConfig() string { config = append(config, "enableExperimentalFeatures=true") } if len(config) == 0 { - return "(default config)" + log.Info("(default config)") + } else { + log.Info("Updated config to [%s]", strings.Join(config, ",")) + } + if internalConfig.Routing.ProxyConfig != nil { + log.Info("Resolved proxy configuration", "proxy", internalConfig.Routing.ProxyConfig) } - return strings.Join(config, ", ") } diff --git a/pkg/provision/workspace/commonenv.go b/pkg/provision/workspace/commonenv.go index 986524184..e26c29696 100644 --- a/pkg/provision/workspace/commonenv.go +++ b/pkg/provision/workspace/commonenv.go @@ -56,28 +56,28 @@ func CommonEnvironmentVariables(workspaceName, workspaceId, namespace, creator s } func getProxyEnvVars() []corev1.EnvVar { - if config.Proxy == nil { + if config.Routing.ProxyConfig == nil { return nil } - if config.Proxy.HttpProxy == "" && config.Proxy.HttpsProxy == "" { + if config.Routing.ProxyConfig.HttpProxy == "" && config.Routing.ProxyConfig.HttpsProxy == "" { 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 config.Proxy.HttpProxy != "" { - env = append(env, v1.EnvVar{Name: "http_proxy", Value: config.Proxy.HttpProxy}) - env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: config.Proxy.HttpProxy}) + if config.Routing.ProxyConfig.HttpProxy != "" { + env = append(env, v1.EnvVar{Name: "http_proxy", Value: config.Routing.ProxyConfig.HttpProxy}) + env = append(env, v1.EnvVar{Name: "HTTP_PROXY", Value: config.Routing.ProxyConfig.HttpProxy}) } - if config.Proxy.HttpsProxy != "" { - env = append(env, v1.EnvVar{Name: "https_proxy", Value: config.Proxy.HttpsProxy}) - env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: config.Proxy.HttpsProxy}) + if config.Routing.ProxyConfig.HttpsProxy != "" { + env = append(env, v1.EnvVar{Name: "https_proxy", Value: config.Routing.ProxyConfig.HttpsProxy}) + env = append(env, v1.EnvVar{Name: "HTTPS_PROXY", Value: config.Routing.ProxyConfig.HttpsProxy}) } - if config.Proxy.NoProxy != "" { - env = append(env, v1.EnvVar{Name: "no_proxy", Value: config.Proxy.NoProxy}) - env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: config.Proxy.NoProxy}) + if config.Routing.ProxyConfig.NoProxy != "" { + env = append(env, v1.EnvVar{Name: "no_proxy", Value: config.Routing.ProxyConfig.NoProxy}) + env = append(env, v1.EnvVar{Name: "NO_PROXY", Value: config.Routing.ProxyConfig.NoProxy}) } return env From 9212d25b731fd6436a34192e545651fa7c0edf0f Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Tue, 14 Dec 2021 19:10:32 -0500 Subject: [PATCH 7/9] Use proxy configuration in DevWorkspace Operator itself Signed-off-by: Angel Misevski --- .../devworkspaceoperatorconfig_types.go | 4 ++ .../workspace/devworkspace_controller.go | 10 ++-- controllers/workspace/http.go | 56 +++++++++++++++++++ controllers/workspace/status.go | 11 +--- go.mod | 1 + 5 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 controllers/workspace/http.go diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index d21d756db..3758b0cf7 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -49,6 +49,10 @@ type RoutingConfig struct { // 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. + // + // 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 + // requires restarting the controller deployment. ProxyConfig *Proxy `json:"proxyConfig,omitempty"` } diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index f1bb7af99..3088b88fa 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -18,13 +18,10 @@ package controllers import ( "context" "fmt" - "net/http" "strings" "time" devfilevalidation "github.com/devfile/api/v2/pkg/validation" - "github.com/devfile/devworkspace-operator/pkg/provision/sync" - "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/controllers/workspace/metrics" @@ -38,6 +35,7 @@ import ( "github.com/devfile/devworkspace-operator/pkg/library/projects" "github.com/devfile/devworkspace-operator/pkg/provision/metadata" "github.com/devfile/devworkspace-operator/pkg/provision/storage" + "github.com/devfile/devworkspace-operator/pkg/provision/sync" wsprovision "github.com/devfile/devworkspace-operator/pkg/provision/workspace" "github.com/devfile/devworkspace-operator/pkg/timing" @@ -232,13 +230,13 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } timing.SetTime(timingInfo, timing.ComponentsCreated) - // TODO#185 : Temporarily do devfile flattening in main reconcile loop; this should be moved to a subcontroller. flattenHelpers := flatten.ResolverTools{ WorkspaceNamespace: workspace.Namespace, Context: ctx, K8sClient: r.Client, - HttpClient: http.DefaultClient, + HttpClient: httpClient, } + flattenedWorkspace, warnings, err := flatten.ResolveDevWorkspace(&workspace.Spec.Template, flattenHelpers) if err != nil { return r.failWorkspace(workspace, fmt.Sprintf("Error processing devfile: %s", err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus) @@ -612,6 +610,8 @@ func dwRelatedPodsHandler() handler.EventHandler { } func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error { + setupHttpClients() + maxConcurrentReconciles, err := config.GetMaxConcurrentReconciles() if err != nil { return err diff --git a/controllers/workspace/http.go b/controllers/workspace/http.go new file mode 100644 index 000000000..1a6a963f1 --- /dev/null +++ b/controllers/workspace/http.go @@ -0,0 +1,56 @@ +// Copyright (c) 2019-2021 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + "crypto/tls" + "net/http" + "net/url" + + "github.com/devfile/devworkspace-operator/pkg/config" + "golang.org/x/net/http/httpproxy" +) + +var ( + httpClient *http.Client + healthCheckHttpClient *http.Client +) + +func setupHttpClients() { + transport := http.DefaultTransport.(*http.Transport).Clone() + healthCheckTransport := http.DefaultTransport.(*http.Transport).Clone() + healthCheckTransport.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + + if config.Routing != nil && config.Routing.ProxyConfig != nil { + proxyConf := httpproxy.Config{ + HTTPProxy: config.Routing.ProxyConfig.HttpProxy, + HTTPSProxy: config.Routing.ProxyConfig.HttpsProxy, + NoProxy: config.Routing.ProxyConfig.NoProxy, + } + proxyFunc := func(req *http.Request) (*url.URL, error) { + return proxyConf.ProxyFunc()(req.URL) + } + transport.Proxy = proxyFunc + healthCheckTransport.Proxy = proxyFunc + } + + httpClient = &http.Client{ + Transport: transport, + } + healthCheckHttpClient = &http.Client{ + Transport: healthCheckTransport, + } +} diff --git a/controllers/workspace/status.go b/controllers/workspace/status.go index 7d6afccf8..191512cd9 100644 --- a/controllers/workspace/status.go +++ b/controllers/workspace/status.go @@ -17,9 +17,7 @@ package controllers import ( "context" - "crypto/tls" "fmt" - "net/http" "net/url" "sort" "time" @@ -62,13 +60,6 @@ type currentStatus struct { // This variable makes it easier to test conditions. var clock kubeclock.Clock = &kubeclock.RealClock{} -// healthHttpClient is supposed to be used for performing health checks of workspace endpoints -var healthHttpClient = &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, -} - // updateWorkspaceStatus updates the current workspace's status field with conditions and phase from the passed in status. // Parameters for result and error are returned unmodified, unless error is nil and another error is encountered while // updating the status. @@ -171,7 +162,7 @@ func checkServerStatus(workspace *dw.DevWorkspace) (ok bool, err error) { } healthz.Path = healthz.Path + "healthz" - resp, err := healthHttpClient.Get(healthz.String()) + resp, err := healthCheckHttpClient.Get(healthz.String()) if err != nil { return false, err } diff --git a/go.mod b/go.mod index e4dec9ca3..6f9c5d472 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/redhat-cop/operator-utils v1.1.4 github.com/stretchr/testify v1.7.0 golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 + golang.org/x/net v0.0.0-20210428140749-89ef3d95e781 k8s.io/api v0.21.3 k8s.io/apiextensions-apiserver v0.21.3 k8s.io/apimachinery v0.21.3 From c022b3ae0c5fbc16b6726bc541da2116e2914824 Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Wed, 15 Dec 2021 20:31:14 -0500 Subject: [PATCH 8/9] Regenerate files to refresh docs Signed-off-by: Angel Misevski --- .../controller.devfile.io_devworkspaceoperatorconfigs.yaml | 2 +- deploy/deployment/kubernetes/combined.yaml | 5 ++++- ...nfigs.controller.devfile.io.CustomResourceDefinition.yaml | 5 ++++- deploy/deployment/openshift/combined.yaml | 5 ++++- ...nfigs.controller.devfile.io.CustomResourceDefinition.yaml | 5 ++++- .../controller.devfile.io_devworkspaceoperatorconfigs.yaml | 5 ++++- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 4f02b8448..e155da804 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -43,7 +43,7 @@ spec: description: DefaultRoutingClass specifies the routingClass to be used when a DevWorkspace specifies an empty `.spec.routingClass`. Supported routingClasses can be defined in other controllers. If not specified, the default value of "basic" is used. type: string proxyConfig: - description: "ProxyConfig defines the proxy settings that should be used for all DevWorkspaces. These values are propagated to workspace containers as environment variables. \n 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." + description: "ProxyConfig defines the proxy settings that should be used for all DevWorkspaces. These values are propagated to workspace containers as environment variables. \n 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. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, in the format http://USERNAME:PASSWORD@SERVER:PORT/ diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 8bb6ed913..e2c3ff133 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -64,7 +64,10 @@ spec: 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." + configuration. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, diff --git a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 83f25ca69..4c4f8dc5c 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -64,7 +64,10 @@ spec: 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." + configuration. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 29b3ce26a..e971acdc9 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -64,7 +64,10 @@ spec: 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." + configuration. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, diff --git a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 83f25ca69..4c4f8dc5c 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -64,7 +64,10 @@ spec: 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." + configuration. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 2db44a3aa..db789caac 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -63,7 +63,10 @@ spec: 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." + configuration. \n 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 requires restarting the controller deployment." properties: httpProxy: description: HttpProxy is the URL of the proxy for HTTP requests, From fa5c3c99492a73ae9dcc3f40e7bcb3ee99004b1d Mon Sep 17 00:00:00 2001 From: Angel Misevski Date: Thu, 23 Dec 2021 00:30:03 -0500 Subject: [PATCH 9/9] Fixup config syncing and tests Signed-off-by: Angel Misevski --- pkg/config/common_test.go | 2 ++ pkg/config/sync.go | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/config/common_test.go b/pkg/config/common_test.go index cb18834bd..799e61dd9 100644 --- a/pkg/config/common_test.go +++ b/pkg/config/common_test.go @@ -17,6 +17,7 @@ import ( "testing" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + configv1 "github.com/openshift/api/config/v1" routev1 "github.com/openshift/api/route/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -39,6 +40,7 @@ func init() { utilruntime.Must(v1alpha1.AddToScheme(scheme)) utilruntime.Must(dw.AddToScheme(scheme)) utilruntime.Must(routev1.Install(scheme)) + utilruntime.Must(configv1.Install(scheme)) } func setupForTest(t *testing.T) { diff --git a/pkg/config/sync.go b/pkg/config/sync.go index 5df98db89..e84555a08 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -82,14 +82,18 @@ func SetupControllerConfig(client crclient.Client) error { return err } DefaultConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix + if internalConfig.Routing.ClusterHostSuffix == "" { + internalConfig.Routing.ClusterHostSuffix = defaultRoutingSuffix + } clusterProxy, err := proxy.GetClusterProxyConfig(client) if err != nil { return err } DefaultConfig.Routing.ProxyConfig = clusterProxy - - mergeConfig(DefaultConfig, internalConfig) + if internalConfig.Routing.ProxyConfig == nil { + internalConfig.Routing.ProxyConfig = clusterProxy + } updatePublicConfig() return nil