feat: Allow external DWOC to be merged with internal DWOC

Allow specifying an external DWOC that gets merged with the global DWOC for use by a workspace.
During the merge, the fields which are set in the external DWOC will overwrite those existing in the internal/global DWOC,
resulting in a merged DWOC to be used by the workspace. The internal/global DWOC will remain unchanged after the merge.

The external DWOC's name and namespace are specified with the `controller.devfile.io/devworkspace-config` DevWorkspace attribute,
which has the following structure:

attributes:
  controller.devfile.io/devworkspace-config:
    name: <string>
    namespace: <string>

Part of eclipse-che/che#21405

Signed-off-by: Andrew Obuchowicz <[email protected]>

Author: AObuchow

controllers/workspace/devworkspace_controller.go

@@ -123,7 +123,11 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return reconcile.Result{}, err
 	}
 
-	config := wkspConfig.GetGlobalConfig()
+	config, err := wkspConfig.ResolveConfigForWorkspace(rawWorkspace, clusterAPI.Client)
+	if err != nil {
+		reqLogger.Error(err, "Error applying external DevWorkspace-Operator configuration")
+		config = wkspConfig.GetGlobalConfig()
+	}
 	configString := wkspConfig.GetCurrentConfigString()
 	workspace := &common.DevWorkspaceWithConfig{}
 	workspace.DevWorkspace = rawWorkspace

pkg/config/common_test.go

@@ -31,7 +31,11 @@ import (
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 )
 
-const testNamespace = "test-namespace"
+const (
+	testNamespace           = "test-namespace"
+	externalConfigName      = "external-config-name"
+	externalConfigNamespace = "external-config-namespace"
+)
 
 var (
 	scheme   = runtime.NewScheme()
@@ -68,3 +72,13 @@ func buildConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceO
 		Config: config,
 	}
 }
+
+func buildExternalConfig(config *v1alpha1.OperatorConfiguration) *v1alpha1.DevWorkspaceOperatorConfig {
+	return &v1alpha1.DevWorkspaceOperatorConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      externalConfigName,
+			Namespace: externalConfigNamespace,
+		},
+		Config: config,
+	}
+}

pkg/config/sync.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"sync"
 
+	dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/devworkspace-operator/pkg/config/proxy"
 	routeV1 "github.com/openshift/api/route/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
@@ -30,6 +31,7 @@ import (
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	controller "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 )
 
@@ -49,6 +51,38 @@ func GetGlobalConfig() *controller.OperatorConfiguration {
 	return internalConfig.DeepCopy()
 }
 
+// ResolveConfigForWorkspace returns the resulting config from merging the global DevWorkspaceOperatorConfig with the
+// DevWorkspaceOperatorConfig specified by the optional workspace attribute `controller.devfile.io/devworkspace-config`.
+// If the `controller.devfile.io/devworkspace-config` is not set, the global DevWorkspaceOperatorConfig is returned.
+// If the `controller.devfile.io/devworkspace-config` attribute is incorrectly set, or the specified DevWorkspaceOperatorConfig
+// does not exist on the cluster, an error is returned.
+func ResolveConfigForWorkspace(workspace *dw.DevWorkspace, client crclient.Client) (*controller.OperatorConfiguration, error) {
+	if !workspace.Spec.Template.Attributes.Exists(constants.ExternalDevWorkspaceConfiguration) {
+		return GetGlobalConfig(), nil
+	}
+
+	namespacedName := types.NamespacedName{}
+	err := workspace.Spec.Template.Attributes.GetInto(constants.ExternalDevWorkspaceConfiguration, &namespacedName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read attribute %s in DevWorkspace attributes: %w", constants.ExternalDevWorkspaceConfiguration, err)
+	}
+
+	if namespacedName.Name == "" {
+		return nil, fmt.Errorf("'name' must be set for attribute %s in DevWorkspace attributes", constants.ExternalDevWorkspaceConfiguration)
+	}
+
+	if namespacedName.Namespace == "" {
+		return nil, fmt.Errorf("'namespace' must be set for attribute %s in DevWorkspace attributes", constants.ExternalDevWorkspaceConfiguration)
+	}
+
+	externalDWOC := &controller.DevWorkspaceOperatorConfig{}
+	err = client.Get(context.TODO(), namespacedName, externalDWOC)
+	if err != nil {
+		return nil, fmt.Errorf("could not fetch external DWOC with name %s in namespace %s: %w", namespacedName.Name, namespacedName.Namespace, err)
+	}
+	return getMergedConfig(externalDWOC.Config, internalConfig), nil
+}
+
 func GetConfigForTesting(customConfig *controller.OperatorConfiguration) *controller.OperatorConfiguration {
 	configMutex.Lock()
 	defer configMutex.Unlock()
@@ -121,6 +155,13 @@ func getClusterConfig(namespace string, client crclient.Client) (*controller.Dev
 	return clusterConfig, nil
 }
 
+func getMergedConfig(from, to *controller.OperatorConfiguration) *controller.OperatorConfiguration {
+	mergedConfig := to.DeepCopy()
+	fromCopy := from.DeepCopy()
+	mergeConfig(fromCopy, mergedConfig)
+	return mergedConfig
+}
+
 func syncConfigFrom(newConfig *controller.DevWorkspaceOperatorConfig) {
 	if newConfig == nil || newConfig.Name != OperatorConfigName || newConfig.Namespace != configNamespace {
 		return

pkg/config/sync_test.go

@@ -16,18 +16,24 @@
 package config
 
 import (
+	"context"
 	"fmt"
 	"testing"
 
+	dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	attributes "github.com/devfile/api/v2/pkg/attributes"
 	"github.com/google/go-cmp/cmp"
 	fuzz "github.com/google/gofuzz"
 	routev1 "github.com/openshift/api/route/v1"
 	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
 	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 )
 
@@ -83,6 +89,96 @@ func TestSetupControllerMergesClusterConfig(t *testing.T) {
 	assert.Equal(t, expectedConfig, internalConfig, fmt.Sprintf("Processed config should merge settings from cluster: %s", cmp.Diff(internalConfig, expectedConfig)))
 }
 
+func TestCatchesNonExistentExternalDWOC(t *testing.T) {
+	setupForTest(t)
+
+	workspace := &dw.DevWorkspace{}
+	attributes := attributes.Attributes{}
+	namespacedName := types.NamespacedName{
+		Name:      "external-config-name",
+		Namespace: "external-config-namespace",
+	}
+	attributes.Put(constants.ExternalDevWorkspaceConfiguration, namespacedName, nil)
+	workspace.Spec.Template.DevWorkspaceTemplateSpecContent = dw.DevWorkspaceTemplateSpecContent{
+		Attributes: attributes,
+	}
+	client := fake.NewClientBuilder().WithScheme(scheme).Build()
+
+	resolvedConfig, err := ResolveConfigForWorkspace(workspace, client)
+	if !assert.Error(t, err, "Error should be given if external DWOC specified in workspace spec does not exist") {
+		return
+	}
+	assert.Equal(t, resolvedConfig, internalConfig, "Internal/Global DWOC should be used as fallback if external DWOC could not be resolved")
+}
+
+func TestMergeExternalConfig(t *testing.T) {
+	setupForTest(t)
+
+	workspace := &dw.DevWorkspace{}
+	attributes := attributes.Attributes{}
+	namespacedName := types.NamespacedName{
+		Name:      externalConfigName,
+		Namespace: externalConfigNamespace,
+	}
+	attributes.Put(constants.ExternalDevWorkspaceConfiguration, namespacedName, nil)
+	workspace.Spec.Template.DevWorkspaceTemplateSpecContent = dw.DevWorkspaceTemplateSpecContent{
+		Attributes: attributes,
+	}
+
+	// External config is based off of the default/internal config, with just a few changes made
+	// So when the internal config is merged with the external one, they will become identical
+	externalConfig := buildExternalConfig(defaultConfig.DeepCopy())
+	externalConfig.Config.Workspace.ImagePullPolicy = "Always"
+	externalConfig.Config.Workspace.PVCName = "test-PVC-name"
+	storageSize := resource.MustParse("15Gi")
+	externalConfig.Config.Workspace.DefaultStorageSize = &v1alpha1.StorageSizes{
+		Common:       &storageSize,
+		PerWorkspace: &storageSize,
+	}
+
+	clusterConfig := buildConfig(defaultConfig.DeepCopy())
+	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(clusterConfig, externalConfig).Build()
+	err := SetupControllerConfig(client)
+	if !assert.NoError(t, err, "Should not return error") {
+		return
+	}
+
+	// Sanity check
+	if !cmp.Equal(clusterConfig.Config, internalConfig) {
+		t.Error("Internal config should be same as cluster config before starting:", cmp.Diff(clusterConfig.Config, internalConfig))
+	}
+
+	resolvedConfig, err := ResolveConfigForWorkspace(workspace, client)
+	if !assert.NoError(t, err, "Should not return error") {
+		return
+	}
+
+	// Compare the resolved config and external config
+	if !cmp.Equal(resolvedConfig, externalConfig.Config) {
+		t.Error("Resolved config and external config should match after merge:", cmp.Diff(resolvedConfig, externalConfig.Config))
+	}
+
+	// Ensure the internal config was not affected by merge
+	if !cmp.Equal(clusterConfig.Config, internalConfig) {
+		t.Error("Internal config should remain the same after merge:", cmp.Diff(clusterConfig.Config, internalConfig))
+	}
+
+	// Get the global config off cluster and ensure it hasn't changed
+	retrievedClusterConfig := &v1alpha1.DevWorkspaceOperatorConfig{}
+	namespacedName = types.NamespacedName{
+		Name:      OperatorConfigName,
+		Namespace: testNamespace,
+	}
+	err = client.Get(context.TODO(), namespacedName, retrievedClusterConfig)
+	if !assert.NoError(t, err, "Should not return error when fetching config from cluster") {
+		return
+	}
+
+	if !cmp.Equal(retrievedClusterConfig.Config, clusterConfig.Config) {
+		t.Error("Config on cluster and global config should match after merge; global config should not have been modified from merge:", cmp.Diff(retrievedClusterConfig, clusterConfig.Config))
+	}
+}
+
 func TestSetupControllerAlwaysSetsDefaultClusterRoutingSuffix(t *testing.T) {
 	setupForTest(t)
 	infrastructure.InitializeForTesting(infrastructure.OpenShiftv4)

pkg/constants/attributes.go

@@ -28,6 +28,21 @@ const (
 	//                    stopped.
 	DevWorkspaceStorageTypeAttribute = "controller.devfile.io/storage-type"
 
+	// ExternalDevWorkspaceConfiguration is an attribute that allows for specifying an (optional) external DevWorkspaceOperatorConfig
+	// which will merged with the internal/global DevWorkspaceOperatorConfig. The DevWorkspaceOperatorConfig resulting from the merge will be used for the workspace.
+	// The fields which are set in the external DevWorkspaceOperatorConfig will overwrite those existing in the
+	// internal/global DevWorkspaceOperatorConfig during the merge.
+	// The structure of the attribute value should contain two strings: name and namespace.
+	// 'name' specifies the metadata.name of the external operator configuration.
+	// 'namespace' specifies the metadata.namespace of the external operator configuration                                                                           .
+	// For example:
+	//
+	//   attributes:
+	//     controller.devfile.io/devworkspace-config:
+	//         name: external-dwoc-name
+	//         namespace: some-namespace
+	ExternalDevWorkspaceConfiguration = "controller.devfile.io/devworkspace-config"
+
 	// RuntimeClassNameAttribute is an attribute added to a DevWorkspace to specify a runtimeClassName for container
 	// components in the DevWorkspace (pod.spec.runtimeClassName). If empty, no runtimeClassName is added.
 	RuntimeClassNameAttribute = "controller.devfile.io/runtime-class"