Automatically migrate old configmap-based configuration to new CRD

main.go

@@ -191,5 +191,8 @@ func setupControllerConfig(mgr ctrl.Manager) error {
 	if err != nil {
 		return err
 	}
+	if err := config.MigrateConfigFromConfigMap(nonCachedClient); err != nil {
+		return err
+	}
 	return config.SetupControllerConfig(nonCachedClient)
 }

pkg/config/common_test.go

@@ -0,0 +1,65 @@
+//
+// Copyright (c) 2019-2021 Red Hat, Inc.
+// This program and the accompanying materials are made
+// available under the terms of the Eclipse Public License 2.0
+// which is available at https://www.eclipse.org/legal/epl-2.0/
+//
+// SPDX-License-Identifier: EPL-2.0
+//
+// Contributors:
+//   Red Hat, Inc. - initial API and implementation
+//
+
+package config
+
+import (
+	"os"
+	"testing"
+
+	dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	routev1 "github.com/openshift/api/route/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+
+	"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
+)
+
+const testNamespace = "test-namespace"
+
+var (
+	scheme   = runtime.NewScheme()
+	trueBool = true
+)
+
+func init() {
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(v1alpha1.AddToScheme(scheme))
+	utilruntime.Must(dw.AddToScheme(scheme))
+	utilruntime.Must(routev1.Install(scheme))
+}
+
+func setupForTest(t *testing.T) {
+	if err := os.Setenv("WATCH_NAMESPACE", testNamespace); err != nil {
+		t.Fatalf("failed to set up for test: %s", err)
+	}
+	infrastructure.InitializeForTesting(infrastructure.Kubernetes)
+	configNamespace = testNamespace
+	originalDefaultConfig := DefaultConfig.DeepCopy()
+	t.Cleanup(func() {
+		internalConfig = nil
+		DefaultConfig = originalDefaultConfig
+	})
+}
+
+func buildConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceOperatorConfig {
+	return &v1alpha1.DevWorkspaceOperatorConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      OperatorConfigName,
+			Namespace: testNamespace,
+		},
+		Config: config,
+	}
+}

pkg/config/configmap/config.go

@@ -18,21 +18,13 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strings"
 
-	"github.com/devfile/devworkspace-operator/pkg/constants"
-	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
-
-	"sigs.k8s.io/controller-runtime/pkg/event"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
-
-	routeV1 "github.com/openshift/api/route/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 )
 
 var ControllerCfg ControllerConfig
@@ -57,27 +49,31 @@ func (wc *ControllerConfig) update(configMap *corev1.ConfigMap) {
 	wc.configMap = configMap
 }
 
-func (wc *ControllerConfig) GetWorkspacePVCName() string {
-	return wc.GetPropertyOrDefault(workspacePVCName, defaultWorkspacePVCName)
+func (wc *ControllerConfig) GetWorkspacePVCName() *string {
+	return wc.GetProperty(workspacePVCName)
 }
 
-func (wc *ControllerConfig) GetDefaultRoutingClass() string {
-	return wc.GetPropertyOrDefault(routingClass, defaultRoutingClass)
+func (wc *ControllerConfig) GetDefaultRoutingClass() *string {
+	return wc.GetProperty(routingClass)
+}
+
+func (wc *ControllerConfig) GetClusterRoutingSuffix() *string {
+	return wc.GetProperty(routingSuffix)
 }
 
 //GetExperimentalFeaturesEnabled returns true if experimental features should be enabled.
 //DO NOT TURN ON IT IN THE PRODUCTION.
 //Experimental features are not well tested and may be totally removed without announcement.
-func (wc *ControllerConfig) GetExperimentalFeaturesEnabled() bool {
-	return wc.GetPropertyOrDefault(experimentalFeaturesEnabled, defaultExperimentalFeaturesEnabled) == "true"
+func (wc *ControllerConfig) GetExperimentalFeaturesEnabled() *string {
+	return wc.GetProperty(experimentalFeaturesEnabled)
 }
 
 func (wc *ControllerConfig) GetPVCStorageClassName() *string {
 	return wc.GetProperty(workspacePVCStorageClassName)
 }
 
-func (wc *ControllerConfig) GetSidecarPullPolicy() string {
-	return wc.GetPropertyOrDefault(sidecarPullPolicy, defaultSidecarPullPolicy)
+func (wc *ControllerConfig) GetSidecarPullPolicy() *string {
+	return wc.GetProperty(sidecarPullPolicy)
 }
 
 func (wc *ControllerConfig) GetProperty(name string) *string {
@@ -88,20 +84,12 @@ func (wc *ControllerConfig) GetProperty(name string) *string {
 	return nil
 }
 
-func (wc *ControllerConfig) GetPropertyOrDefault(name string, defaultValue string) string {
-	val, exists := wc.configMap.Data[name]
-	if exists {
-		return val
-	}
-	return defaultValue
-}
-
 func (wc *ControllerConfig) Validate() error {
 	return nil
 }
 
-func (wc *ControllerConfig) GetWorkspaceIdleTimeout() string {
-	return wc.GetPropertyOrDefault(devworkspaceIdleTimeout, defaultDevWorkspaceIdleTimeout)
+func (wc *ControllerConfig) GetWorkspaceIdleTimeout() *string {
+	return wc.GetProperty(devworkspaceIdleTimeout)
 }
 
 func syncConfigmapFromCluster(client client.Client, obj client.Object) {
@@ -113,141 +101,44 @@ func syncConfigmapFromCluster(client client.Client, obj client.Object) {
 		ControllerCfg.update(cm)
 		return
 	}
-
-	configMap := &corev1.ConfigMap{}
-	err := client.Get(context.TODO(), ConfigMapReference, configMap)
-	if err != nil {
-		log.Error(err, fmt.Sprintf("Cannot find the '%s' ConfigMap in namespace '%s'", ConfigMapReference.Name, ConfigMapReference.Namespace))
-	}
-	ControllerCfg.update(configMap)
 }
 
-func WatchControllerConfig(mgr manager.Manager) error {
-	customConfig := false
+func LoadControllerConfig(nonCachedClient client.Client) (found bool, err error) {
 	configMapName, found := os.LookupEnv(ConfigMapNameEnvVar)
 	if found && len(configMapName) > 0 {
 		ConfigMapReference.Name = configMapName
-		customConfig = true
 	}
 	configMapNamespace, found := os.LookupEnv(ConfigMapNamespaceEnvVar)
 	if found && len(configMapNamespace) > 0 {
 		ConfigMapReference.Namespace = configMapNamespace
-		customConfig = true
+	} else {
+		namespace, err := infrastructure.GetNamespace()
+		if err != nil {
+			return false, err
+		}
+		ConfigMapReference.Namespace = namespace
 	}
 
 	if ConfigMapReference.Namespace == "" {
-		return fmt.Errorf("you should set the namespace of the controller config map through the '%s' environment variable", ConfigMapNamespaceEnvVar)
+		return false, fmt.Errorf("you should set the namespace of the controller config map through the '%s' environment variable", ConfigMapNamespaceEnvVar)
 	}
 
 	configMap := &corev1.ConfigMap{}
-	nonCachedClient, err := client.New(mgr.GetConfig(), client.Options{
-		Scheme: mgr.GetScheme(),
-	})
-	if err != nil {
-		return err
-	}
 	log.Info(fmt.Sprintf("Searching for config map '%s' in namespace '%s'", ConfigMapReference.Name, ConfigMapReference.Namespace))
 	err = nonCachedClient.Get(context.TODO(), ConfigMapReference, configMap)
 	if err != nil {
 		if !k8sErrors.IsNotFound(err) {
-			return err
-		}
-		if customConfig {
-			return fmt.Errorf("cannot find the '%s' ConfigMap in namespace '%s'", ConfigMapReference.Name, ConfigMapReference.Namespace)
-		}
-
-		buildDefaultConfigMap(configMap)
-
-		err = nonCachedClient.Create(context.TODO(), configMap)
-		if err != nil {
-			return err
+			return false, err
 		}
-		log.Info(fmt.Sprintf("  => created config map '%s' in namespace '%s'", configMap.GetObjectMeta().GetName(), configMap.GetObjectMeta().GetNamespace()))
+		return false, nil
 	} else {
 		log.Info(fmt.Sprintf("  => found config map '%s' in namespace '%s'", configMap.GetObjectMeta().GetName(), configMap.GetObjectMeta().GetNamespace()))
 	}
 
 	if configMap.Data == nil {
 		configMap.Data = map[string]string{}
 	}
-	err = fillOpenShiftRouteSuffixIfNecessary(nonCachedClient, configMap)
-	if err != nil {
-		return err
-	}
-
 	syncConfigmapFromCluster(nonCachedClient, configMap)
 
-	return nil
-}
-
-func SetupConfigForTesting(cm *corev1.ConfigMap) {
-	ControllerCfg.update(cm)
-}
-
-func buildDefaultConfigMap(cm *corev1.ConfigMap) {
-	cm.Name = ConfigMapReference.Name
-	cm.Namespace = ConfigMapReference.Namespace
-	cm.Labels = constants.ControllerAppLabels()
-
-	cm.Data = map[string]string{}
-}
-
-func fillOpenShiftRouteSuffixIfNecessary(nonCachedClient client.Client, configMap *corev1.ConfigMap) error {
-	if !infrastructure.IsOpenShift() {
-		return nil
-	}
-
-	testRoute := &routeV1.Route{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: configMap.Namespace,
-			Name:      "devworkspace-controller-test-route",
-		},
-		Spec: routeV1.RouteSpec{
-			To: routeV1.RouteTargetReference{
-				Kind: "Service",
-				Name: "devworkspace-controller-test-route",
-			},
-		},
-	}
-
-	err := nonCachedClient.Create(context.TODO(), testRoute)
-	if err != nil {
-		return err
-	}
-	defer nonCachedClient.Delete(context.TODO(), testRoute)
-	host := testRoute.Spec.Host
-	if host != "" {
-		prefixToRemove := "devworkspace-controller-test-route-" + configMap.Namespace + "."
-		configMap.Data[RoutingSuffix] = strings.TrimPrefix(host, prefixToRemove)
-	}
-
-	err = nonCachedClient.Update(context.TODO(), configMap)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func ConfigMapPredicates(mgr manager.Manager) predicate.Predicate {
-	return predicate.Funcs{
-		UpdateFunc: func(evt event.UpdateEvent) bool {
-			if evt.ObjectNew.GetName() == ConfigMapReference.Name && evt.ObjectNew.GetNamespace() == ConfigMapReference.Namespace {
-				syncConfigmapFromCluster(mgr.GetClient(), evt.ObjectNew)
-			}
-			return false
-		},
-		CreateFunc: func(evt event.CreateEvent) bool {
-			if evt.Object.GetName() == ConfigMapReference.Name && evt.Object.GetNamespace() == ConfigMapReference.Namespace {
-				syncConfigmapFromCluster(mgr.GetClient(), evt.Object)
-			}
-			return false
-		},
-		DeleteFunc: func(evt event.DeleteEvent) bool {
-			return false
-		},
-		GenericFunc: func(evt event.GenericEvent) bool {
-			return false
-		},
-	}
+	return true, nil
 }

pkg/config/configmap/property.go

@@ -29,10 +29,10 @@ const (
 	routingClass        = "devworkspace.default_routing_class"
 	defaultRoutingClass = "basic"
 
-	// RoutingSuffix is the base domain for routes/ingresses created on the cluster. All
-	// routes/ingresses will be created with URL http(s)://<unique-to-workspace-part>.<RoutingSuffix>
+	// routingSuffix is the base domain for routes/ingresses created on the cluster. All
+	// routes/ingresses will be created with URL http(s)://<unique-to-workspace-part>.<routingSuffix>
 	// is supposed to be used by embedded routing solvers only
-	RoutingSuffix = "devworkspace.routing.cluster_host_suffix"
+	routingSuffix = "devworkspace.routing.cluster_host_suffix"
 
 	experimentalFeaturesEnabled        = "devworkspace.experimental_features_enabled"
 	defaultExperimentalFeaturesEnabled = "false"