From c63d5235934c26106c7cac667d3fec2db58ef0d2 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 22 Jan 2024 09:04:16 +0100 Subject: [PATCH] fixups Signed-off-by: Anatolii Bazko --- .../usernamespace/workspace_cm_syncer_test.go | 105 +++++++++++++++++- .../workspace_secret_syncer_test.go | 103 +++++++++++++++++ .../workspaces_config_controller.go | 15 +++ 3 files changed, 222 insertions(+), 1 deletion(-) diff --git a/controllers/usernamespace/workspace_cm_syncer_test.go b/controllers/usernamespace/workspace_cm_syncer_test.go index 59b77cff1b..c1616626f1 100644 --- a/controllers/usernamespace/workspace_cm_syncer_test.go +++ b/controllers/usernamespace/workspace_cm_syncer_test.go @@ -191,13 +191,116 @@ func TestSyncConfigMap(t *testing.T) { assert.Nil(t, err) assertSyncConfig(t, workspaceConfigReconciler, 0, v1ConfigMapGKV) - // Check that destination Secret in a user namespace is deleted + // Check that destination ConfigMap in a user namespace is deleted cm = &corev1.ConfigMap{} err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, cm) assert.NotNil(t, err) assert.True(t, errors.IsNotFound(err)) } +func TestSyncConfigMapShouldMergeLabelsAndAnnotationsOnUpdate(t *testing.T) { + deployContext := test.GetDeployContext(nil, []runtime.Object{ + &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: objectName, + Namespace: "eclipse-che", + Labels: map[string]string{ + "label": "label-value", + constants.KubernetesPartOfLabelKey: constants.CheEclipseOrg, + constants.KubernetesComponentLabelKey: constants.WorkspacesConfig, + }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + }, + Data: map[string]string{ + "a": "b", + }, + }}) + + workspaceConfigReconciler := NewWorkspacesConfigReconciler( + deployContext.ClusterAPI.Client, + deployContext.ClusterAPI.NonCachingClient, + deployContext.ClusterAPI.Scheme, + NewNamespaceCache(deployContext.ClusterAPI.NonCachingClient)) + + // Sync ConfigMap + err := workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1ConfigMapGKV) + + // Check ConfigMap in a user namespace is created + cm := &corev1.ConfigMap{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, cm) + assert.Nil(t, err) + assert.Equal(t, constants.WorkspacesConfig, cm.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, cm.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/watch-configmap"]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value", cm.Labels["label"]) + assert.Equal(t, "annotation-value", cm.Annotations["annotation"]) + + // Update labels and annotations on dst ConfigMap + cm = &corev1.ConfigMap{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, cm) + assert.Nil(t, err) + utils.AddMap(cm.Labels, map[string]string{"new-label": "new-label-value"}) + utils.AddMap(cm.Annotations, map[string]string{"new-annotation": "new-annotation-value"}) + err = workspaceConfigReconciler.client.Update(context.TODO(), cm) + assert.Nil(t, err) + + // Sync ConfigMap + err = workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1ConfigMapGKV) + + // Check that destination ConfigMap is not reverted + cm = &corev1.ConfigMap{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, cm) + assert.Nil(t, err) + assert.Equal(t, constants.WorkspacesConfig, cm.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, cm.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/watch-configmap"]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value", cm.Labels["label"]) + assert.Equal(t, "new-label-value", cm.Labels["new-label"]) + assert.Equal(t, "annotation-value", cm.Annotations["annotation"]) + assert.Equal(t, "new-annotation-value", cm.Annotations["new-annotation"]) + + // Update src ConfigMap + cm = &corev1.ConfigMap{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInCheNs, cm) + assert.Nil(t, err) + cm.Data["a"] = "c" + utils.AddMap(cm.Labels, map[string]string{"label": "label-value-2"}) + utils.AddMap(cm.Annotations, map[string]string{"annotation": "annotation-value-2"}) + err = workspaceConfigReconciler.client.Update(context.TODO(), cm) + assert.Nil(t, err) + + // Sync ConfigMap + err = workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1ConfigMapGKV) + + // Check that destination ConfigMap is updated but old labels and annotations are preserved + cm = &corev1.ConfigMap{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, cm) + assert.Nil(t, err) + assert.Equal(t, "c", cm.Data["a"]) + assert.Equal(t, constants.WorkspacesConfig, cm.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, cm.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/watch-configmap"]) + assert.Equal(t, "true", cm.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value-2", cm.Labels["label"]) + assert.Equal(t, "new-label-value", cm.Labels["new-label"]) + assert.Equal(t, "annotation-value-2", cm.Annotations["annotation"]) + assert.Equal(t, "new-annotation-value", cm.Annotations["new-annotation"]) +} + func assertSyncConfig(t *testing.T, workspaceConfigReconciler *WorkspacesConfigReconciler, expectedNumberOfRecords int, gkv schema.GroupVersionKind) { cm, err := workspaceConfigReconciler.getSyncConfig(context.TODO(), userNamespace) assert.Nil(t, err) diff --git a/controllers/usernamespace/workspace_secret_syncer_test.go b/controllers/usernamespace/workspace_secret_syncer_test.go index e7db9efa9d..cbe591a7bf 100644 --- a/controllers/usernamespace/workspace_secret_syncer_test.go +++ b/controllers/usernamespace/workspace_secret_syncer_test.go @@ -195,3 +195,106 @@ func TestSyncSecrets(t *testing.T) { assert.NotNil(t, err) assert.True(t, errors.IsNotFound(err)) } + +func TestSyncSecretShouldMergeLabelsAndAnnotationsOnUpdate(t *testing.T) { + deployContext := test.GetDeployContext(nil, []runtime.Object{ + &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: objectName, + Namespace: "eclipse-che", + Labels: map[string]string{ + "label": "label-value", + constants.KubernetesPartOfLabelKey: constants.CheEclipseOrg, + constants.KubernetesComponentLabelKey: constants.WorkspacesConfig, + }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + }, + StringData: map[string]string{ + "a": "b", + }, + }}) + + workspaceConfigReconciler := NewWorkspacesConfigReconciler( + deployContext.ClusterAPI.Client, + deployContext.ClusterAPI.NonCachingClient, + deployContext.ClusterAPI.Scheme, + NewNamespaceCache(deployContext.ClusterAPI.NonCachingClient)) + + // Sync Secret + err := workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1SecretGKV) + + // Check Secret in a user namespace is created + secret := &corev1.Secret{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, secret) + assert.Nil(t, err) + assert.Equal(t, constants.WorkspacesConfig, secret.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, secret.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/watch-secret"]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value", secret.Labels["label"]) + assert.Equal(t, "annotation-value", secret.Annotations["annotation"]) + + // Update labels and annotations on dst Secret + secret = &corev1.Secret{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, secret) + assert.Nil(t, err) + utils.AddMap(secret.Labels, map[string]string{"new-label": "new-label-value"}) + utils.AddMap(secret.Annotations, map[string]string{"new-annotation": "new-annotation-value"}) + err = workspaceConfigReconciler.client.Update(context.TODO(), secret) + assert.Nil(t, err) + + // Sync Secret + err = workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1SecretGKV) + + // Check that destination Secret is not reverted + secret = &corev1.Secret{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, secret) + assert.Nil(t, err) + assert.Equal(t, constants.WorkspacesConfig, secret.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, secret.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/watch-secret"]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value", secret.Labels["label"]) + assert.Equal(t, "new-label-value", secret.Labels["new-label"]) + assert.Equal(t, "annotation-value", secret.Annotations["annotation"]) + assert.Equal(t, "new-annotation-value", secret.Annotations["new-annotation"]) + + // Update src Secret + secret = &corev1.Secret{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInCheNs, secret) + assert.Nil(t, err) + secret.StringData["a"] = "c" + utils.AddMap(secret.Labels, map[string]string{"label": "label-value-2"}) + utils.AddMap(secret.Annotations, map[string]string{"annotation": "annotation-value-2"}) + err = workspaceConfigReconciler.client.Update(context.TODO(), secret) + assert.Nil(t, err) + + // Sync Secret + err = workspaceConfigReconciler.syncWorkspacesConfig(context.TODO(), userNamespace) + assert.Nil(t, err) + assertSyncConfig(t, workspaceConfigReconciler, 2, v1SecretGKV) + + // Check that destination Secret is updated but old labels and annotations are preserved + secret = &corev1.Secret{} + err = workspaceConfigReconciler.client.Get(context.TODO(), objectKeyInUserNs, secret) + assert.Nil(t, err) + assert.Equal(t, "c", secret.StringData["a"]) + assert.Equal(t, constants.WorkspacesConfig, secret.Labels[constants.KubernetesComponentLabelKey]) + assert.Equal(t, constants.CheEclipseOrg, secret.Labels[constants.KubernetesPartOfLabelKey]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/watch-secret"]) + assert.Equal(t, "true", secret.Labels["controller.devfile.io/mount-to-devworkspace"]) + assert.Equal(t, "label-value-2", secret.Labels["label"]) + assert.Equal(t, "new-label-value", secret.Labels["new-label"]) + assert.Equal(t, "annotation-value-2", secret.Annotations["annotation"]) + assert.Equal(t, "new-annotation-value", secret.Annotations["new-annotation"]) +} diff --git a/controllers/usernamespace/workspaces_config_controller.go b/controllers/usernamespace/workspaces_config_controller.go index 6555b95d75..dfdf1c345f 100644 --- a/controllers/usernamespace/workspaces_config_controller.go +++ b/controllers/usernamespace/workspaces_config_controller.go @@ -374,6 +374,11 @@ func (r *WorkspacesConfigReconciler) doSyncObjectToNamespace( return nil } else { if syncContext.syncer.isExistedObjChanged(newObj, existedObj) { + // preserve labels and annotations from existed object + newObj.SetLabels(preserveExistedMapValues(newObj.GetLabels(), existedObj.GetLabels())) + newObj.SetAnnotations(preserveExistedMapValues(newObj.GetAnnotations(), existedObj.GetAnnotations())) + + // set the correct resource version to update object newObj.SetResourceVersion(existedObj.GetResourceVersion()) if err := r.client.Update(syncContext.ctx, newObj); err != nil { return err @@ -496,3 +501,13 @@ func mergeWorkspaceConfigObjectLabels(srcLabels map[string]string, additionalLab return newLabels } + +func preserveExistedMapValues(newObjMap map[string]string, existedObjMap map[string]string) map[string]string { + preservedMap := utils.CloneMap(newObjMap) + for key, value := range existedObjMap { + if _, ok := preservedMap[key]; !ok { + preservedMap[key] = value + } + } + return preservedMap +}