From 43749c48f0cacdfa75ac5b8d60321ce3bfb12047 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 6 May 2026 11:45:11 +0200 Subject: [PATCH 1/3] feat: mount ConfigMaps/Secrets/PVCs only to a specific DevWorkspace Signed-off-by: Anatolii Bazko --- .../workspace/devworkspace_controller.go | 2 +- controllers/workspace/eventhandlers.go | 26 +- docs/additional-configuration.adoc | 66 ++- pkg/constants/metadata.go | 22 + pkg/provision/automount/common.go | 101 +++- .../automount/common_persistenthome_test.go | 2 +- pkg/provision/automount/common_test.go | 253 +++++++++- pkg/provision/automount/configmap.go | 19 +- pkg/provision/automount/gitconfig.go | 76 ++- pkg/provision/automount/gitconfig_test.go | 477 +++++++++++++++++- pkg/provision/automount/pvcs.go | 15 +- pkg/provision/automount/secret.go | 19 +- .../testBothIncludeAndExcludeAnnotations.yaml | 55 ++ .../testExcludeAnnotationDoesNotMatch.yaml | 73 +++ .../testExcludeAnnotationMatches.yaml | 52 ++ .../testIncludeAnnotationDoesNotMatch.yaml | 52 ++ .../testIncludeAnnotationMatches.yaml | 73 +++ 17 files changed, 1296 insertions(+), 87 deletions(-) create mode 100644 pkg/provision/automount/testdata/testBothIncludeAndExcludeAnnotations.yaml create mode 100644 pkg/provision/automount/testdata/testExcludeAnnotationDoesNotMatch.yaml create mode 100644 pkg/provision/automount/testdata/testExcludeAnnotationMatches.yaml create mode 100644 pkg/provision/automount/testdata/testIncludeAnnotationDoesNotMatch.yaml create mode 100644 pkg/provision/automount/testdata/testIncludeAnnotationMatches.yaml diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 240f17266..f53af369e 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -465,7 +465,7 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } } - err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, home.PersistUserHomeEnabled(workspace), workspaceDeployment) + err = automount.ProvisionAutoMountResourcesInto(devfilePodAdditions, clusterAPI, workspace.Namespace, workspace.Name, home.PersistUserHomeEnabled(workspace), workspaceDeployment) if shouldReturn, reconcileResult, reconcileErr := r.checkDWError(workspace, err, "Failed to process automount resources", metrics.ReasonBadRequest, reqLogger, &reconcileStatus); shouldReturn { return reconcileResult, reconcileErr } diff --git a/controllers/workspace/eventhandlers.go b/controllers/workspace/eventhandlers.go index c1329c526..f32c1dc61 100644 --- a/controllers/workspace/eventhandlers.go +++ b/controllers/workspace/eventhandlers.go @@ -20,6 +20,7 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" wkspConfig "github.com/devfile/devworkspace-operator/pkg/config" "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/provision/automount" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -118,22 +119,29 @@ func (r *DevWorkspaceReconciler) dwPVCHandler(ctx context.Context, obj client.Ob return reconciles } -func (r *DevWorkspaceReconciler) runningWorkspacesHandler(ctx context.Context, obj client.Object) []reconcile.Request { +func (r *DevWorkspaceReconciler) runningWorkspacesHandler(_ context.Context, obj client.Object) []reconcile.Request { dwList := &dw.DevWorkspaceList{} if err := r.Client.List(context.Background(), dwList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { return []reconcile.Request{} } var reconciles []reconcile.Request for _, workspace := range dwList.Items { - // Queue reconciles for any started workspaces to make sure they pick up new object - if workspace.Spec.Started { - reconciles = append(reconciles, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: workspace.GetName(), - Namespace: workspace.GetNamespace(), - }, - }) + if !workspace.Spec.Started { + continue } + + // In context of performance. + // Don't trigger reconcile if automount resource doesn't match the target workspace. + if !automount.MatchesWorkspaceTarget(obj, workspace.GetName()) { + continue + } + + reconciles = append(reconciles, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: workspace.GetName(), + Namespace: workspace.GetNamespace(), + }, + }) } return reconciles } diff --git a/docs/additional-configuration.adoc b/docs/additional-configuration.adoc index b26330986..d0222a138 100644 --- a/docs/additional-configuration.adoc +++ b/docs/additional-configuration.adoc @@ -168,7 +168,58 @@ When "file" is used, the configmap is mounted as a directory within the workspac * `controller.devfile.io/mount-on-start`: when set to `"true"`, the resource will only be mounted when a workspace starts. If the resource is created while a workspace is already running, it will not be automatically mounted until the workspace is restarted. This prevents unwanted workspace restarts caused by newly created automount resources. This annotation can be applied to configmaps, secrets, and persistent volume claims. + -For git credential secrets (labelled with `controller.devfile.io/git-credential`) and git TLS configmaps (labelled with `controller.devfile.io/git-tls-credential`), the annotation is evaluated across all git credential resources as a group, not individually. Since all git credentials are merged into a single mounted secret, if at least one git credential secret (or TLS configmap) lacks the `controller.devfile.io/mount-on-start` annotation, all git credentials (or TLS configmaps) will be mounted, including those marked with `mount-on-start`. +For git credential secrets (labeled with `controller.devfile.io/git-credential`) +and git TLS configmaps (labelled with `controller.devfile.io/git-tls-credential`), +this annotation behave differently: ++ +-- +** The `mount-on-start` annotation is evaluated across all git credential resources as a group, not individually. Since all git credentials are merged into a single mounted secret, if at least one git credential secret (or TLS configmap) lacks the `mount-on-start` annotation, all git credentials (or TLS configmaps) will be mounted, including those marked with `mount-on-start`. +-- + +* `controller.devfile.io/mount-to-devworkspace-include`: configure which DevWorkspaces the +resource should be mounted to. The value is a comma-separated list of patterns matched against +the DevWorkspace name. The resource is only mounted to workspaces whose name matches at +least one pattern. This annotation can be applied to ConfigMaps, Secrets and Persistent Volume Claims. ++ +* `controller.devfile.io/mount-to-devworkspace-exclude`: configure which DevWorkspaces the +resource should NOT be mounted to. The value is a comma-separated list of patterns +matched against the DevWorkspace name. The resource is not mounted to workspaces whose name +matches any pattern. This annotation can be applied to ConfigMaps, Secrets and Persistent Volume Claims. ++ +Supported patterns for both annotations: ++ +-- +** `name` -- exact match +** `name*` -- prefix match +** `*name` -- suffix match +** `\*name*` -- contains match +** `*` -- matches all workspaces +-- ++ +Both annotations can be used at the same time on the same resource. When both are present, the resource is mounted only to workspaces that match the include pattern AND do not match the exclude pattern. +If neither annotation is present, the resource is mounted to all workspaces (default behavior). ++ +[source,yaml] +---- +metadata: + annotations: + controller.devfile.io/mount-to-devworkspace-include: "my-workspace, staging-*" +---- ++ +[source,yaml] +---- +metadata: + annotations: + controller.devfile.io/mount-to-devworkspace-exclude: "temp-*, *-test" +---- ++ +[source,yaml] +---- +metadata: + annotations: + controller.devfile.io/mount-to-devworkspace-include: "staging-*" + controller.devfile.io/mount-to-devworkspace-exclude: "staging-legacy" +---- + [source,yaml] ---- @@ -201,6 +252,19 @@ data: This will mount a file `/tmp/.git-credentials/credentials` in all workspace containers, and construct a git config to use this file as a credentials store. +### Using a base gitconfig +If an automount configmap or secret (labeled with `controller.devfile.io/automount: true`) +is mounted as `subpath` and contains a key that resolves to `/etc/gitconfig`, +its contents are used as the base git configuration for the workspace. +This base gitconfig is merged with the generated git credentials configuration. + +For example, a configmap with `controller.devfile.io/mount-path: /etc` and a key named +`gitconfig` will be detected and used as the base git configuration. + +The `controller.devfile.io/mount-to-devworkspace-include` and +`controller.devfile.io/mount-to-devworkspace-exclude` annotations are respected, +allowing the base gitconfig to be targeted to specific workspaces. + ## Configuring DevWorkspaces to use SSH keys for Git operations Git SSH keys can be configured for DevWorkspaces by mounting secrets to workspaces. diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 65c803029..d2c9ba14f 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -49,6 +49,28 @@ const ( // DevWorkspaceMountLabel is the label key to store if a configmap, secret, or PVC should be mounted to the devworkspace DevWorkspaceMountLabel = "controller.devfile.io/mount-to-devworkspace" + // DevWorkspaceMountIncludeAnnotation is an annotation to configure which DevWorkspaces an automount + // resource should be mounted to. The value is a comma-separated list of patterns matched against + // the DevWorkspace name. The resource is only mounted to workspaces whose name matches at least one pattern. + // Supported patterns: + // - exact match `name` + // - prefix match `name*` + // - suffix match `*name`, + // - contains match `*name*` + // - matches all workspaces `*` + DevWorkspaceMountIncludeAnnotation = "controller.devfile.io/mount-to-devworkspace-include" + + // DevWorkspaceMountExcludeAnnotation is an annotation to configure which DevWorkspaces an automount + // resource should NOT be mounted to. The value is a comma-separated list of patterns matched against + // the DevWorkspace name. The resource is not mounted to workspaces whose name matches any pattern. + // Supported patterns: + // - exact match `name` + // - prefix match `name*` + // - suffix match `*name`, + // - contains match `*name*` + // - matches all workspaces `*` + DevWorkspaceMountExcludeAnnotation = "controller.devfile.io/mount-to-devworkspace-exclude" + // DevWorkspaceMountPathAnnotation is the annotation key to store the mount path for the secret or configmap. // If no mount path is provided, configmaps will be mounted at /etc/config/, secrets will // be mounted at /etc/secret/, and persistent volume claims will be mounted to /tmp/ diff --git a/pkg/provision/automount/common.go b/pkg/provision/automount/common.go index 2818e7423..b4ca69523 100644 --- a/pkg/provision/automount/common.go +++ b/pkg/provision/automount/common.go @@ -46,11 +46,12 @@ type Resources struct { func ProvisionAutoMountResourcesInto( podAdditions *v1alpha1.PodAdditions, api sync.ClusterAPI, - namespace string, + workspaceNamespace string, + workspaceName string, persistentHome bool, workspaceDeployment *appsv1.Deployment, ) error { - resources, err := getAutomountResources(api, namespace, workspaceDeployment) + resources, err := getAutomountResources(api, workspaceNamespace, workspaceName, workspaceDeployment) if err != nil { return err @@ -85,20 +86,21 @@ func ProvisionAutoMountResourcesInto( func getAutomountResources( api sync.ClusterAPI, - namespace string, + workspaceNamespace string, + workspaceName string, workspaceDeployment *appsv1.Deployment, ) (*Resources, error) { - gitCMAutoMountResources, err := ProvisionGitConfiguration(api, namespace, workspaceDeployment) + gitCMAutoMountResources, err := ProvisionGitConfiguration(api, workspaceNamespace, workspaceName, workspaceDeployment) if err != nil { return nil, err } - cmAutoMountResources, err := getDevWorkspaceConfigmaps(namespace, api, workspaceDeployment) + cmAutoMountResources, err := getDevWorkspaceConfigmaps(workspaceNamespace, workspaceName, api, workspaceDeployment) if err != nil { return nil, err } - secretAutoMountResources, err := getDevWorkspaceSecrets(namespace, api, workspaceDeployment) + secretAutoMountResources, err := getDevWorkspaceSecrets(workspaceNamespace, workspaceName, api, workspaceDeployment) if err != nil { return nil, err } @@ -115,7 +117,7 @@ func getAutomountResources( } dropItemsFieldFromVolumes(mergedResources.Volumes) - pvcAutoMountResources, err := getAutoMountPVCs(namespace, api, workspaceDeployment) + pvcAutoMountResources, err := getAutoMountPVCs(workspaceNamespace, workspaceName, api, workspaceDeployment) if err != nil { return nil, err } @@ -214,12 +216,17 @@ func flattenAutomountResources(resources []Resources) Resources { return flattened } -// findGitconfigAutomount searches a namespace for a automount resource (configmap or secret) that contains +// findGitconfigAutomount searches a namespace for an automount resource (configmap or secret) that contains // a system-wide gitconfig (i.e. the mountpath is `/etc/gitconfig`). Only objects with mount type "subpath" -// are considered. If a suitable object is found, the contents of the gitconfig defined there is returned. -func findGitconfigAutomount(api sync.ClusterAPI, namespace string) (gitconfig *string, err error) { +// are considered. Resources that do not match the workspace's include/exclude annotations are skipped. +// If a suitable object is found, the contents of the gitconfig defined there is returned. +func findGitconfigAutomount( + api sync.ClusterAPI, + workspaceNamespace string, + workspaceName string, +) (gitconfig *string, err error) { configmapList := &corev1.ConfigMapList{} - if err := api.Client.List(api.Ctx, configmapList, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{ + if err := api.Client.List(api.Ctx, configmapList, k8sclient.InNamespace(workspaceNamespace), k8sclient.MatchingLabels{ constants.DevWorkspaceMountLabel: "true", }); err != nil { return nil, err @@ -228,6 +235,13 @@ func findGitconfigAutomount(api sync.ClusterAPI, namespace string) (gitconfig *s if cm.Annotations[constants.DevWorkspaceMountAsAnnotation] != constants.DevWorkspaceMountAsSubpath { continue } + + // Filter resources by workspace name + if !MatchesWorkspaceTarget(&cm, workspaceName) { + log.V(1).Info("skip ConfigMap, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) + continue + } + mountPath := cm.Annotations[constants.DevWorkspaceMountPathAnnotation] for key, value := range cm.Data { if path.Join(mountPath, key) == "/etc/gitconfig" { @@ -240,7 +254,7 @@ func findGitconfigAutomount(api sync.ClusterAPI, namespace string) (gitconfig *s } secretList := &corev1.SecretList{} - if err := api.Client.List(api.Ctx, secretList, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{ + if err := api.Client.List(api.Ctx, secretList, k8sclient.InNamespace(workspaceNamespace), k8sclient.MatchingLabels{ constants.DevWorkspaceMountLabel: "true", }); err != nil { return nil, err @@ -249,6 +263,13 @@ func findGitconfigAutomount(api sync.ClusterAPI, namespace string) (gitconfig *s if secret.Annotations[constants.DevWorkspaceMountAsAnnotation] != constants.DevWorkspaceMountAsSubpath { continue } + + // Filter resources by workspace name + if !MatchesWorkspaceTarget(&secret, workspaceName) { + log.V(1).Info("skip Secret, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + continue + } + mountPath := secret.Annotations[constants.DevWorkspaceMountPathAnnotation] for key, value := range secret.Data { if path.Join(mountPath, key) == "/etc/gitconfig" { @@ -370,10 +391,10 @@ func isMountOnStart(obj k8sclient.Object) bool { return obj.GetAnnotations()[constants.MountOnStartAttribute] == "true" } -// isAllowedToMount checks whether an automount resource can be added to the workspace pod. +// canMountWithoutRestart checks whether an automount resource can be added to the workspace pod. // Resources marked with mount-on-start are only allowed when // the workspace is not yet running or when they are already present in the current deployment. -func isAllowedToMount( +func canMountWithoutRestart( obj k8sclient.Object, automountResource Resources, workspaceDeployment *appsv1.Deployment, @@ -437,3 +458,55 @@ func isEnvFromSourceExistsInDeployment(automountResource Resources, workspaceDep return false } + +// MatchesWorkspaceTarget checks whether a resource should be mounted to a given workspace +// based on include/exclude annotations. Both annotations can be used together: the resource +// is mounted when it matches the include pattern (or no include is set) and does not match the exclude pattern. +func MatchesWorkspaceTarget( + obj k8sclient.Object, + workspaceName string, +) bool { + annotations := obj.GetAnnotations() + + includePatterns := strings.TrimSpace(annotations[constants.DevWorkspaceMountIncludeAnnotation]) + excludePatterns := strings.TrimSpace(annotations[constants.DevWorkspaceMountExcludeAnnotation]) + + included := includePatterns == "" || matchesAnyPattern(includePatterns, workspaceName) + excluded := excludePatterns != "" && matchesAnyPattern(excludePatterns, workspaceName) + + return included && !excluded +} + +func matchesAnyPattern(patternsStr string, workspaceName string) bool { + patterns := strings.Split(patternsStr, ",") + for _, pattern := range patterns { + pattern = strings.TrimSpace(pattern) + if pattern == "" { + continue + } + + if pattern == "*" { + return true + } + + startsWithWildcard := strings.HasPrefix(pattern, "*") + endsWithWildcard := strings.HasSuffix(pattern, "*") + + var matched bool + switch { + case startsWithWildcard && endsWithWildcard: + matched = strings.Contains(workspaceName, pattern[1:len(pattern)-1]) + case endsWithWildcard: + matched = strings.HasPrefix(workspaceName, pattern[:len(pattern)-1]) + case startsWithWildcard: + matched = strings.HasSuffix(workspaceName, pattern[1:]) + default: + matched = workspaceName == pattern + } + + if matched { + return true + } + } + return false +} diff --git a/pkg/provision/automount/common_persistenthome_test.go b/pkg/provision/automount/common_persistenthome_test.go index 549037962..91ed7c814 100644 --- a/pkg/provision/automount/common_persistenthome_test.go +++ b/pkg/provision/automount/common_persistenthome_test.go @@ -56,7 +56,7 @@ func TestProvisionAutomountResourcesIntoPersistentHomeEnabled(t *testing.T) { Client: fake.NewClientBuilder().WithObjects(tt.Input.allObjects...).Build(), } - err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, true, nil) + err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, "test-workspace", true, nil) if !assert.NoError(t, err, "Unexpected error") { return diff --git a/pkg/provision/automount/common_test.go b/pkg/provision/automount/common_test.go index fa7d5a644..c85d5b20a 100644 --- a/pkg/provision/automount/common_test.go +++ b/pkg/provision/automount/common_test.go @@ -34,6 +34,7 @@ import ( "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" "github.com/devfile/devworkspace-operator/pkg/common" + "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) @@ -128,7 +129,7 @@ func TestProvisionAutomountResourcesInto(t *testing.T) { } // Note: this test does not allow for returning AutoMountError with isFatal: false (i.e. no retrying) // and so is not suitable for testing automount features that provision cluster resources (yet) - err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, false, nil) + err := ProvisionAutoMountResourcesInto(podAdditions, testAPI, testNamespace, "test-workspace", false, nil) if tt.Output.ErrRegexp != nil { if !assert.Error(t, err, "Expected an error but got none") { return @@ -422,7 +423,7 @@ func TestShouldNotMountSecretWithMountOnStartIfWorkspaceStarted(t *testing.T) { }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, emptyDeployment()) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, emptyDeployment()) assert.NoError(t, err) assert.Empty(t, testPodAdditions.Volumes) assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts) @@ -440,7 +441,7 @@ func TestMountSecretWithMountOnStartIfWorkspaceNotStarted(t *testing.T) { }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, nil) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, nil) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1) @@ -476,7 +477,7 @@ func TestMountOnStartSecretAsEnvAllowedWhenEnvFromExistsInDeployment(t *testing. }, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, deployment) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, deployment) assert.NoError(t, err) assert.Len(t, testPodAdditions.Containers[0].EnvFrom, 1) assert.Equal(t, common.AutoMountSecretVolumeName("test-secret"), testPodAdditions.Containers[0].EnvFrom[0].SecretRef.Name) @@ -505,7 +506,7 @@ func TestMountOnStartSecretAsFileAllowedWhenVolumeExistsInDeployment(t *testing. }, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, deployment) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, deployment) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Equal(t, common.AutoMountSecretVolumeName("test-secret"), testPodAdditions.Volumes[0].Name) @@ -523,7 +524,7 @@ func TestShouldNotMountConfigMapWithMountOnStartIfWorkspaceStarted(t *testing.T) }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, emptyDeployment()) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, emptyDeployment()) assert.NoError(t, err) assert.Empty(t, testPodAdditions.Volumes) assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts) @@ -541,7 +542,7 @@ func TestMountConfigMapWithMountOnStartIfWorkspaceNotStarted(t *testing.T) { }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, nil) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, nil) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1) @@ -577,7 +578,7 @@ func TestMountOnStartConfigMapAsEnvAllowedWhenEnvFromExistsInDeployment(t *testi }, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, deployment) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, deployment) assert.NoError(t, err) assert.Len(t, testPodAdditions.Containers[0].EnvFrom, 1) assert.Equal(t, common.AutoMountConfigMapVolumeName("test-cm"), testPodAdditions.Containers[0].EnvFrom[0].ConfigMapRef.Name) @@ -606,7 +607,7 @@ func TestMountOnStartConfigMapAsFileAllowedWhenVolumeExistsInDeployment(t *testi }, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, deployment) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, deployment) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Equal(t, common.AutoMountConfigMapVolumeName("test-cm"), testPodAdditions.Volumes[0].Name) @@ -624,7 +625,7 @@ func TestShouldNotMountPVCWithMountOnStartIfWorkspaceStarted(t *testing.T) { }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, emptyDeployment()) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, emptyDeployment()) assert.NoError(t, err) assert.Empty(t, testPodAdditions.Volumes) assert.Empty(t, testPodAdditions.Containers[0].VolumeMounts) @@ -642,7 +643,7 @@ func TestMountPVCWithMountOnStartIfWorkspaceNotStarted(t *testing.T) { }}, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, nil) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, nil) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Len(t, testPodAdditions.Containers[0].VolumeMounts, 1) @@ -672,7 +673,7 @@ func TestMountOnStartPVCAllowedWhenVolumeExistsInDeployment(t *testing.T) { }, } - err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, false, deployment) + err := ProvisionAutoMountResourcesInto(testPodAdditions, testAPI, testNamespace, "test-workspace", false, deployment) assert.NoError(t, err) assert.Len(t, testPodAdditions.Volumes, 1) assert.Equal(t, common.AutoMountPVCVolumeName("test-pvc"), testPodAdditions.Volumes[0].Name) @@ -787,3 +788,231 @@ func mountOnStartPVC() *corev1.PersistentVolumeClaim { }, } } + +func TestMatchesWorkspaceTarget(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + workspaceName string + expected bool + }{ + // No annotations / empty annotations + { + name: "No annotations mounts to all workspaces", + annotations: nil, + workspaceName: "any-workspace", + expected: true, + }, + { + name: "Empty include annotation mounts to all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: ""}, + workspaceName: "any-workspace", + expected: true, + }, + { + name: "Empty exclude annotation mounts to all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: ""}, + workspaceName: "any-workspace", + expected: true, + }, + { + name: "Whitespace-only include annotation mounts to all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: " "}, + workspaceName: "any-workspace", + expected: true, + }, + { + name: "Whitespace-only exclude annotation mounts to all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: " "}, + workspaceName: "any-workspace", + expected: true, + }, + // Exact match + { + name: "Include exact match", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "my-workspace"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include exact match does not match different name", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "other-workspace"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude exact match", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "my-workspace"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude exact match does not match different name", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "other-workspace"}, + workspaceName: "my-workspace", + expected: true, + }, + // Prefix match (pattern*) + { + name: "Include prefix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "my-*"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include prefix pattern does not match", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "other-*"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude prefix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "my-*"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude prefix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "test-*"}, + workspaceName: "my-workspace", + expected: true, + }, + // Suffix match (*pattern) + { + name: "Include suffix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "*workspace"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include suffix pattern does not match", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "*other"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude suffix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "*workspace"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude suffix pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "*test"}, + workspaceName: "my-workspace", + expected: true, + }, + // Contains match (*pattern*) + { + name: "Include contains pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "*work*"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include contains pattern does not match", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "*xyz*"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude contains pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "*work*"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude contains pattern matches", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "*test*"}, + workspaceName: "my-workspace", + expected: true, + }, + // Wildcard (*) + { + name: "Include wildcard matches all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "*"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Exclude wildcard excludes all workspaces", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "*"}, + workspaceName: "my-workspace", + expected: false, + }, + // Comma-separated patterns + { + name: "Include with comma-separated list matches second pattern", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "other-workspace, my-workspace"}, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include with comma-separated list matches none", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: "foo, bar"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Exclude with comma-separated list matches second pattern", + annotations: map[string]string{constants.DevWorkspaceMountExcludeAnnotation: "other, my-*"}, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Include with comma-separated list and trailing whitespace", + annotations: map[string]string{constants.DevWorkspaceMountIncludeAnnotation: " my-workspace , other "}, + workspaceName: "my-workspace", + expected: true, + }, + // Both include and exclude annotations + { + name: "Include matches and exclude matches results in not mounted", + annotations: map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "my-workspace", + constants.DevWorkspaceMountExcludeAnnotation: "my-workspace", + }, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Include matches and exclude does not match results in mounted", + annotations: map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "my-workspace", + constants.DevWorkspaceMountExcludeAnnotation: "other-workspace", + }, + workspaceName: "my-workspace", + expected: true, + }, + { + name: "Include does not match and exclude does not match results in not mounted", + annotations: map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + constants.DevWorkspaceMountExcludeAnnotation: "another-workspace", + }, + workspaceName: "my-workspace", + expected: false, + }, + { + name: "Include does not match and exclude matches results in not mounted", + annotations: map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + constants.DevWorkspaceMountExcludeAnnotation: "my-workspace", + }, + workspaceName: "my-workspace", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-obj", + Annotations: tt.annotations, + }, + } + assert.Equal(t, tt.expected, MatchesWorkspaceTarget(obj, tt.workspaceName)) + }) + } +} diff --git a/pkg/provision/automount/configmap.go b/pkg/provision/automount/configmap.go index 933bd7638..5c9628f8b 100644 --- a/pkg/provision/automount/configmap.go +++ b/pkg/provision/automount/configmap.go @@ -30,9 +30,14 @@ import ( "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) -func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI, workspaceDeployment *appsv1.Deployment) (*Resources, error) { +func getDevWorkspaceConfigmaps( + workspaceNamespace string, + workspaceName string, + api sync.ClusterAPI, + workspaceDeployment *appsv1.Deployment, +) (*Resources, error) { configmaps := &corev1.ConfigMapList{} - if err := api.Client.List(api.Ctx, configmaps, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{ + if err := api.Client.List(api.Ctx, configmaps, k8sclient.InNamespace(workspaceNamespace), k8sclient.MatchingLabels{ constants.DevWorkspaceMountLabel: "true", }); err != nil { return nil, err @@ -42,6 +47,12 @@ func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI, workspaceD var allAutoMountResources []Resources for _, configmap := range configmaps.Items { + // Filter resources by workspace name + if !MatchesWorkspaceTarget(&configmap, workspaceName) { + log.V(1).Info("Skip ConfigMap mount, workspace does not match include/exclude annotations", "namespace", configmap.Namespace, "name", configmap.Name, "workspace", workspaceName) + continue + } + mountAs := configmap.Annotations[constants.DevWorkspaceMountAsAnnotation] mountPath := configmap.Annotations[constants.DevWorkspaceMountPathAnnotation] if mountPath == "" { @@ -56,8 +67,8 @@ func getDevWorkspaceConfigmaps(namespace string, api sync.ClusterAPI, workspaceD } automountCM := getAutomountConfigmap(mountPath, mountAs, accessMode, &configmap) - if !isAllowedToMount(&configmap, automountCM, workspaceDeployment) { - log.V(1).Info("Not allowed to mount ConfigMap", "namespace", configmap.Namespace, "name", configmap.Name) + if !canMountWithoutRestart(&configmap, automountCM, workspaceDeployment) { + log.V(1).Info("Skipping ConfigMap mount: resource requires workspace restart to be mounted", "namespace", configmap.Namespace, "name", configmap.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/gitconfig.go b/pkg/provision/automount/gitconfig.go index 0a4c90fd1..a4d4d25a0 100644 --- a/pkg/provision/automount/gitconfig.go +++ b/pkg/provision/automount/gitconfig.go @@ -31,31 +31,32 @@ const mergedGitCredentialsMountPath = "/.git-credentials/" // ProvisionGitConfiguration takes care of mounting git credentials and a gitconfig into a devworkspace. func ProvisionGitConfiguration( api sync.ClusterAPI, - namespace string, + workspaceNamespace string, + workspaceName string, workspaceDeployment *appsv1.Deployment, ) (*Resources, error) { - credentialsSecrets, tlsConfigMaps, err := getGitResources(api, namespace, workspaceDeployment) + credentialsSecrets, tlsConfigMaps, err := getGitResources(api, workspaceNamespace, workspaceName, workspaceDeployment) if err != nil { return nil, err } - baseGitConfig, err := findGitconfigAutomount(api, namespace) + baseGitConfig, err := findGitconfigAutomount(api, workspaceNamespace, workspaceName) if err != nil { return nil, err } if len(credentialsSecrets) == 0 && len(tlsConfigMaps) == 0 && baseGitConfig == nil { // Remove any existing git configuration - err := cleanupGitConfig(api, namespace) + err := cleanupGitConfig(api, workspaceNamespace) return nil, err } - mergedCredentialsSecret, err := mergeGitCredentials(namespace, credentialsSecrets) + mergedCredentialsSecret, err := mergeGitCredentials(workspaceNamespace, credentialsSecrets) if err != nil { return nil, &dwerrors.FailError{Message: "Failed to collect git credentials secrets", Err: err} } - gitConfigMap, err := constructGitConfig(namespace, mergedGitCredentialsMountPath, tlsConfigMaps, baseGitConfig) + gitConfigMap, err := constructGitConfig(workspaceNamespace, mergedGitCredentialsMountPath, tlsConfigMaps, baseGitConfig) if err != nil { return nil, &dwerrors.FailError{Message: "Failed to prepare git config for workspace", Err: err} } @@ -78,7 +79,8 @@ func ProvisionGitConfiguration( func getGitResources( api sync.ClusterAPI, - namespace string, + workspaceNamespace string, + workspaceName string, workspaceDeployment *appsv1.Deployment, ) (credentialSecrets []corev1.Secret, tlsConfigMaps []corev1.ConfigMap, err error) { credentialsLabelSelector := k8sclient.MatchingLabels{ @@ -89,34 +91,56 @@ func getGitResources( } secretList := &corev1.SecretList{} - if err := api.Client.List(api.Ctx, secretList, k8sclient.InNamespace(namespace), credentialsLabelSelector); err != nil { + if err := api.Client.List(api.Ctx, secretList, k8sclient.InNamespace(workspaceNamespace), credentialsLabelSelector); err != nil { return nil, nil, err } + + // Filter resources by workspace name var secrets []corev1.Secret - if isGitCredentialsAllowedToMount(secretList.Items, workspaceDeployment) { - if len(secretList.Items) > 0 { - secrets = secretList.Items + for _, secret := range secretList.Items { + if !MatchesWorkspaceTarget(&secret, workspaceName) { + log.V(1).Info("Skip Git credentials Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + } + + secrets = append(secrets, secret) + } + + if len(secrets) > 0 { + if canGitCredentialsMountWithoutRestart(secrets, workspaceDeployment) { + sortSecrets(secrets) + } else { + // Cleanup slice, there are no Git credentials Secrets to mount + secrets = nil + log.V(1).Info("Skipping all Git credentials Secrets mount: resource requires workspace restart to be mounted", "namespace", workspaceNamespace, "workspace", workspaceName) } - } else { - log.V(1).Info("Not allowed to mount Git credentials secret", "namespace", namespace) } - sortSecrets(secrets) configmapList := &corev1.ConfigMapList{} - if err := api.Client.List(api.Ctx, configmapList, k8sclient.InNamespace(namespace), tlsLabelSelector); err != nil { + if err := api.Client.List(api.Ctx, configmapList, k8sclient.InNamespace(workspaceNamespace), tlsLabelSelector); err != nil { return nil, nil, err } + + // Filter resources by workspace name var configmaps []corev1.ConfigMap + for _, cm := range configmapList.Items { + if !MatchesWorkspaceTarget(&cm, workspaceName) { + log.V(1).Info("skip Git ConfigMap mount, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) + } + + configmaps = append(configmaps, cm) + } + // When git credentials are present, the gitconfig ConfigMap must be created // regardless of mount-on-start annotations. - if len(secrets) > 0 || isGitConfigsAllowedToMount(configmapList.Items, workspaceDeployment) { - if len(configmapList.Items) > 0 { - configmaps = configmapList.Items + if len(configmaps) > 0 { + if len(secrets) > 0 || canGitConfigsMountWithoutRestart(configmaps, workspaceDeployment) { + sortConfigmaps(configmaps) + } else { + // Cleanup slice, there are no gitconfig to mount + configmaps = nil + log.V(1).Info("Skipping all Git ConfigMaps mount: resource requires workspace restart to be mounted", "namespace", workspaceNamespace, "workspace", workspaceName) } - } else { - log.V(1).Info("Not allowed to mount Git configs", "namespace", namespace) } - sortConfigmaps(configmaps) return secrets, configmaps, nil } @@ -161,17 +185,17 @@ func cleanupGitConfig(api sync.ClusterAPI, namespace string) error { return nil } -func isGitCredentialsAllowedToMount(secrets []corev1.Secret, workspaceDeployment *appsv1.Deployment) bool { +func canGitCredentialsMountWithoutRestart(secrets []corev1.Secret, workspaceDeployment *appsv1.Deployment) bool { volumeName := common.AutoMountSecretVolumeName(constants.GitCredentialsMergedSecretName) - return isGitObjectsAllowedToMount(secrets, volumeName, workspaceDeployment) + return canGitObjectsMountWithoutRestart(secrets, volumeName, workspaceDeployment) } -func isGitConfigsAllowedToMount(configMaps []corev1.ConfigMap, workspaceDeployment *appsv1.Deployment) bool { +func canGitConfigsMountWithoutRestart(configMaps []corev1.ConfigMap, workspaceDeployment *appsv1.Deployment) bool { volumeName := common.AutoMountConfigMapVolumeName(constants.GitCredentialsConfigMapName) - return isGitObjectsAllowedToMount(configMaps, volumeName, workspaceDeployment) + return canGitObjectsMountWithoutRestart(configMaps, volumeName, workspaceDeployment) } -func isGitObjectsAllowedToMount[T any](objs []T, volumeName string, workspaceDeployment *appsv1.Deployment) bool { +func canGitObjectsMountWithoutRestart[T any](objs []T, volumeName string, workspaceDeployment *appsv1.Deployment) bool { // No deployment exists yet — workspace is not running, no restart risk if workspaceDeployment == nil { return true diff --git a/pkg/provision/automount/gitconfig_test.go b/pkg/provision/automount/gitconfig_test.go index 1a90c10dd..9e1796468 100644 --- a/pkg/provision/automount/gitconfig_test.go +++ b/pkg/provision/automount/gitconfig_test.go @@ -58,7 +58,7 @@ func TestUserCredentialsAreMountedWithOneCredential(t *testing.T) { // ProvisionGitConfiguration has to be called multiple times since it stops after creating each configmap/secret ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -84,7 +84,7 @@ func TestUserCredentialsAreOnlyMountedOnceWithMultipleCredentials(t *testing.T) // ProvisionGitConfiguration has to be called multiple times since it stops after creating each configmap/secret ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -104,7 +104,7 @@ func TestGitConfigIsFullyMounted(t *testing.T) { // ProvisionGitConfiguration has to be called multiple times since it stops after creating each configmap/secret ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -240,7 +240,7 @@ func TestShouldNotMountGitCredentialSecretWithMountOnStartIfWorkspaceStarted(t * var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, emptyDeployment()) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", emptyDeployment()) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -265,7 +265,7 @@ func TestMountGitCredentialSecretWithMountOnStartIfWorkspaceNotStarted(t *testin var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -305,7 +305,7 @@ func TestMountGitCredentialSecretWithMountOnStartWhenVolumeExistsInDeployment(t var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, deployment) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", deployment) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -337,7 +337,7 @@ func TestMountGitCredentialSecretWithMixedMountOnStart(t *testing.T) { var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, emptyDeployment()) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", emptyDeployment()) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -364,7 +364,7 @@ func TestShouldNotMountGitConfigWithMountOnStartIfWorkspaceStarted(t *testing.T) var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, emptyDeployment()) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", emptyDeployment()) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -388,7 +388,7 @@ func TestMountGitConfigWithMountOnStartIfWorkspaceNotStarted(t *testing.T) { var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -425,7 +425,7 @@ func TestMountGitConfigWithMountOnStartWhenVolumeExistsInDeployment(t *testing.T var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, deployment) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", deployment) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -454,7 +454,7 @@ func TestMountGitConfigWithMixedMountOnStart(t *testing.T) { var resources *Resources ok := assert.Eventually(t, func() bool { var err error - resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, nil) + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) t.Log(err) return err == nil }, 100*time.Millisecond, 10*time.Millisecond) @@ -467,6 +467,446 @@ func TestMountGitConfigWithMixedMountOnStart(t *testing.T) { } } +func TestMountGitCredentialSecretWithIncludeAnnotationMatching(t *testing.T) { + mountPath := "/sample/test" + testSecret := buildSecretWithAnnotations("test-secret", mountPath, map[string][]byte{ + gitCredentialsSecretKey: []byte("my_credentials"), + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Len(t, resources.Volumes, 2) + assert.Len(t, resources.VolumeMounts, 2) + + assertGitCredentialsSecret("my_credentials", clusterAPI, t) + assertGitCredentialsConfigMap(clusterAPI, t) + } +} + +func TestSkipGitCredentialSecretWithIncludeAnnotationNotMatching(t *testing.T) { + mountPath := "/sample/test" + testSecret := buildSecretWithAnnotations("test-secret", mountPath, map[string][]byte{ + gitCredentialsSecretKey: []byte("my_credentials"), + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestSkipGitCredentialSecretWithExcludeAnnotationMatching(t *testing.T) { + mountPath := "/sample/test" + testSecret := buildSecretWithAnnotations("test-secret", mountPath, map[string][]byte{ + gitCredentialsSecretKey: []byte("my_credentials"), + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestMountGitCredentialSecretWithExcludeAnnotationNotMatching(t *testing.T) { + mountPath := "/sample/test" + testSecret := buildSecretWithAnnotations("test-secret", mountPath, map[string][]byte{ + gitCredentialsSecretKey: []byte("my_credentials"), + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testSecret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Len(t, resources.Volumes, 2) + assert.Len(t, resources.VolumeMounts, 2) + + assertGitCredentialsSecret("my_credentials", clusterAPI, t) + assertGitCredentialsConfigMap(clusterAPI, t) + } +} + +func TestMountGitTLSConfigMapWithIncludeAnnotationMatching(t *testing.T) { + mountPath := "/sample/test" + testConfigMap := buildConfigWithAnnotations("test-cm", mountPath, defaultData, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Len(t, resources.Volumes, 2) + assert.Len(t, resources.VolumeMounts, 2) + + assertGitCredentialsConfigMapWithSslCAInfo(mountPath, clusterAPI, t) + } +} + +func TestSkipGitTLSConfigMapWithIncludeAnnotationNotMatching(t *testing.T) { + mountPath := "/sample/test" + testConfigMap := buildConfigWithAnnotations("test-cm", mountPath, defaultData, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestSkipGitTLSConfigMapWithExcludeAnnotationMatching(t *testing.T) { + mountPath := "/sample/test" + testConfigMap := buildConfigWithAnnotations("test-cm", mountPath, defaultData, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestMountGitTLSConfigMapWithExcludeAnnotationNotMatching(t *testing.T) { + mountPath := "/sample/test" + testConfigMap := buildConfigWithAnnotations("test-cm", mountPath, defaultData, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&testConfigMap).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Len(t, resources.Volumes, 2) + assert.Len(t, resources.VolumeMounts, 2) + + assertGitCredentialsConfigMapWithSslCAInfo(mountPath, clusterAPI, t) + } +} + +func TestFindGitconfigAutomountConfigMapIncludeMatches(t *testing.T) { + gitconfigContent := "[user]\n\tname = Test User" + cm := buildAutomountGitConfigMap("gitconfig-cm", "/etc", map[string]string{ + "gitconfig": gitconfigContent, + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&cm).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.NotNil(t, resources) + assertGitConfigContains(gitconfigContent, clusterAPI, t) + } +} + +func TestFindGitconfigAutomountConfigMapIncludeDoesNotMatch(t *testing.T) { + cm := buildAutomountGitConfigMap("gitconfig-cm", "/etc", map[string]string{ + "gitconfig": "[user]\n\tname = Test User", + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&cm).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestFindGitconfigAutomountConfigMapExcludeMatches(t *testing.T) { + cm := buildAutomountGitConfigMap("gitconfig-cm", "/etc", map[string]string{ + "gitconfig": "[user]\n\tname = Test User", + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&cm).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestFindGitconfigAutomountConfigMapExcludeDoesNotMatch(t *testing.T) { + gitconfigContent := "[user]\n\tname = Test User" + cm := buildAutomountGitConfigMap("gitconfig-cm", "/etc", map[string]string{ + "gitconfig": gitconfigContent, + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&cm).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.NotNil(t, resources) + assertGitConfigContains(gitconfigContent, clusterAPI, t) + } +} + +func TestFindGitconfigAutomountSecretIncludeMatches(t *testing.T) { + gitconfigContent := "[user]\n\tname = Test User" + secret := buildAutomountGitSecret("gitconfig-secret", "/etc", map[string][]byte{ + "gitconfig": []byte(gitconfigContent), + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&secret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.NotNil(t, resources) + assertGitConfigContains(gitconfigContent, clusterAPI, t) + } +} + +func TestFindGitconfigAutomountSecretIncludeDoesNotMatch(t *testing.T) { + secret := buildAutomountGitSecret("gitconfig-secret", "/etc", map[string][]byte{ + "gitconfig": []byte("[user]\n\tname = Test User"), + }, map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&secret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestFindGitconfigAutomountSecretExcludeMatches(t *testing.T) { + secret := buildAutomountGitSecret("gitconfig-secret", "/etc", map[string][]byte{ + "gitconfig": []byte("[user]\n\tname = Test User"), + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "test-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&secret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.Nil(t, resources) + } +} + +func TestFindGitconfigAutomountSecretExcludeDoesNotMatch(t *testing.T) { + gitconfigContent := "[user]\n\tname = Test User" + secret := buildAutomountGitSecret("gitconfig-secret", "/etc", map[string][]byte{ + "gitconfig": []byte(gitconfigContent), + }, map[string]string{ + constants.DevWorkspaceMountExcludeAnnotation: "other-workspace", + }) + + clusterAPI := sync.ClusterAPI{ + Client: fake.NewClientBuilder().WithObjects(&secret).Build(), + Logger: zap.New(), + } + + var resources *Resources + ok := assert.Eventually(t, func() bool { + var err error + resources, err = ProvisionGitConfiguration(clusterAPI, testNamespace, "test-workspace", nil) + t.Log(err) + return err == nil + }, 100*time.Millisecond, 10*time.Millisecond) + + if ok { + assert.NotNil(t, resources) + assertGitConfigContains(gitconfigContent, clusterAPI, t) + } +} + +func buildAutomountGitConfigMap(name, mountPath string, data map[string]string, extraAnnotations map[string]string) corev1.ConfigMap { + extraAnnotations[constants.DevWorkspaceMountAsAnnotation] = constants.DevWorkspaceMountAsSubpath + cm := buildConfigWithAnnotations(name, mountPath, data, extraAnnotations) + cm.Labels = map[string]string{ + constants.DevWorkspaceMountLabel: "true", + } + return cm +} + +func buildAutomountGitSecret(name, mountPath string, data map[string][]byte, extraAnnotations map[string]string) corev1.Secret { + extraAnnotations[constants.DevWorkspaceMountAsAnnotation] = constants.DevWorkspaceMountAsSubpath + secret := buildSecretWithAnnotations(name, mountPath, data, extraAnnotations) + secret.Labels = map[string]string{ + constants.DevWorkspaceMountLabel: "true", + } + return secret +} + func buildConfig(name string, mountPath string, data map[string]string) corev1.ConfigMap { return corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ @@ -515,6 +955,21 @@ func buildConfigWithAnnotations(name string, mountPath string, data map[string]s return cm } +func assertGitConfigContains(expectedContent string, clusterAPI sync.ClusterAPI, t *testing.T) { + cm := &corev1.ConfigMap{} + err := clusterAPI.Client.Get( + context.Background(), + types.NamespacedName{ + Name: constants.GitCredentialsConfigMapName, + Namespace: testNamespace, + }, + cm, + ) + + assert.NoError(t, err) + assert.Contains(t, cm.Data[gitConfigName], expectedContent) +} + func assertGitCredentialsSecret(expectedCredentials string, clusterAPI sync.ClusterAPI, t *testing.T) { secret := &corev1.Secret{} err := clusterAPI.Client.Get( diff --git a/pkg/provision/automount/pvcs.go b/pkg/provision/automount/pvcs.go index 140ccd3fa..9936513a2 100644 --- a/pkg/provision/automount/pvcs.go +++ b/pkg/provision/automount/pvcs.go @@ -68,12 +68,13 @@ func parseMountPathAnnotation(annotation string, pvcName string) ([]mountPathEnt } func getAutoMountPVCs( - namespace string, + workspaceNamespace string, + workspaceName string, api sync.ClusterAPI, workspaceDeployment *appsv1.Deployment, ) (*Resources, error) { pvcs := &corev1.PersistentVolumeClaimList{} - if err := api.Client.List(api.Ctx, pvcs, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{ + if err := api.Client.List(api.Ctx, pvcs, k8sclient.InNamespace(workspaceNamespace), k8sclient.MatchingLabels{ constants.DevWorkspaceMountLabel: "true", }); err != nil { return nil, err @@ -84,6 +85,12 @@ func getAutoMountPVCs( var allAutoMountResources []Resources for _, pvc := range pvcs.Items { + // Filter resources by workspace name + if !MatchesWorkspaceTarget(&pvc, workspaceName) { + log.V(1).Info("skip PVC mount, workspace does not match include/exclude annotations", "namespace", pvc.Namespace, "name", pvc.Name, "workspace", workspaceName) + continue + } + mountReadOnly := pvc.Annotations[constants.DevWorkspaceMountReadyOnlyAnnotation] == "true" volume := corev1.Volume{ @@ -115,8 +122,8 @@ func getAutoMountPVCs( VolumeMounts: volumeMounts, } - if !isAllowedToMount(&pvc, automountPVC, workspaceDeployment) { - log.V(1).Info("Not allowed to mount PVC", "namespace", pvc.Namespace, "name", pvc.Name) + if !canMountWithoutRestart(&pvc, automountPVC, workspaceDeployment) { + log.V(1).Info("Skipping PVC mount: resource requires workspace restart to be mounted", "namespace", pvc.Namespace, "name", pvc.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/secret.go b/pkg/provision/automount/secret.go index 7863b0e61..d0c6507b0 100644 --- a/pkg/provision/automount/secret.go +++ b/pkg/provision/automount/secret.go @@ -30,9 +30,14 @@ import ( "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) -func getDevWorkspaceSecrets(namespace string, api sync.ClusterAPI, workspaceDeployment *appsv1.Deployment) (*Resources, error) { +func getDevWorkspaceSecrets( + workspaceNamespace string, + workspaceName string, + api sync.ClusterAPI, + workspaceDeployment *appsv1.Deployment, +) (*Resources, error) { secrets := &corev1.SecretList{} - if err := api.Client.List(api.Ctx, secrets, k8sclient.InNamespace(namespace), k8sclient.MatchingLabels{ + if err := api.Client.List(api.Ctx, secrets, k8sclient.InNamespace(workspaceNamespace), k8sclient.MatchingLabels{ constants.DevWorkspaceMountLabel: "true", }); err != nil { return nil, err @@ -40,6 +45,12 @@ func getDevWorkspaceSecrets(namespace string, api sync.ClusterAPI, workspaceDepl sortSecrets(secrets.Items) var allAutoMountResources []Resources for _, secret := range secrets.Items { + // Filter resources by workspace name + if !MatchesWorkspaceTarget(&secret, workspaceName) { + log.V(1).Info("skip Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + continue + } + mountAs := secret.Annotations[constants.DevWorkspaceMountAsAnnotation] mountPath := secret.Annotations[constants.DevWorkspaceMountPathAnnotation] if mountPath == "" { @@ -54,8 +65,8 @@ func getDevWorkspaceSecrets(namespace string, api sync.ClusterAPI, workspaceDepl } automountSecret := getAutomountSecret(mountPath, mountAs, accessMode, &secret) - if !isAllowedToMount(&secret, automountSecret, workspaceDeployment) { - log.V(1).Info("Not allowed to mount Secret", "namespace", secret.Namespace, "name", secret.Name) + if !canMountWithoutRestart(&secret, automountSecret, workspaceDeployment) { + log.V(1).Info("Skipping Secret mount: resource requires workspace restart to be mounted", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/testdata/testBothIncludeAndExcludeAnnotations.yaml b/pkg/provision/automount/testdata/testBothIncludeAndExcludeAnnotations.yaml new file mode 100644 index 000000000..714d975f4 --- /dev/null +++ b/pkg/provision/automount/testdata/testBothIncludeAndExcludeAnnotations.yaml @@ -0,0 +1,55 @@ +name: Does not provision configmap, secret, or PVC when both include and exclude annotations are present + +input: + configmaps: + - + apiVersion: v1 + kind: ConfigMap + metadata: + name: conflicting-configmap + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-configmap: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/conflicting + controller.devfile.io/mount-to-devworkspace-include: "test-workspace" + controller.devfile.io/mount-to-devworkspace-exclude: "test-workspace" + data: + config-key: "config-value" + secrets: + - + apiVersion: v1 + kind: Secret + metadata: + name: conflicting-secret + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-secret: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/conflicting-secret + controller.devfile.io/mount-to-devworkspace-include: "test-workspace" + controller.devfile.io/mount-to-devworkspace-exclude: "test-workspace" + stringData: + secret-key: "secret-value" + pvcs: + - + apiVersion: v1 + kind: PersistentVolumeClaim + metadata: + name: conflicting-pvc + labels: + controller.devfile.io/mount-to-devworkspace: "true" + annotations: + controller.devfile.io/mount-path: /tmp/conflicting-pvc + controller.devfile.io/mount-to-devworkspace-include: "test-workspace" + controller.devfile.io/mount-to-devworkspace-exclude: "test-workspace" + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +output: {} diff --git a/pkg/provision/automount/testdata/testExcludeAnnotationDoesNotMatch.yaml b/pkg/provision/automount/testdata/testExcludeAnnotationDoesNotMatch.yaml new file mode 100644 index 000000000..acfb3fb85 --- /dev/null +++ b/pkg/provision/automount/testdata/testExcludeAnnotationDoesNotMatch.yaml @@ -0,0 +1,73 @@ +name: Provisions configmap, secret, and PVC when exclude annotation does not match workspace name + +input: + configmaps: + - + apiVersion: v1 + kind: ConfigMap + metadata: + name: not-excluded-configmap + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-configmap: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/not-excluded-cm + controller.devfile.io/mount-to-devworkspace-exclude: "staging-*, prod-*" + data: + config-key: "config-value" + secrets: + - + apiVersion: v1 + kind: Secret + metadata: + name: not-excluded-secret + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-secret: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/not-excluded + controller.devfile.io/mount-to-devworkspace-exclude: "staging-*, prod-*" + stringData: + secret-key: "secret-value" + pvcs: + - + apiVersion: v1 + kind: PersistentVolumeClaim + metadata: + name: not-excluded-pvc + labels: + controller.devfile.io/mount-to-devworkspace: "true" + annotations: + controller.devfile.io/mount-path: /tmp/not-excluded-pvc + controller.devfile.io/mount-to-devworkspace-exclude: "staging-*, prod-*" + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +output: + volumes: + - name: not-excluded-configmap + configmap: + name: not-excluded-configmap + defaultMode: 0640 + - name: not-excluded-secret + secret: + secretName: not-excluded-secret + defaultMode: 0640 + - name: not-excluded-pvc + persistentVolumeClaim: + claimName: not-excluded-pvc + volumeMounts: + - name: not-excluded-configmap + readOnly: true + mountPath: /tmp/not-excluded-cm + - name: not-excluded-secret + readOnly: true + mountPath: /tmp/not-excluded + - name: not-excluded-pvc + mountPath: /tmp/not-excluded-pvc diff --git a/pkg/provision/automount/testdata/testExcludeAnnotationMatches.yaml b/pkg/provision/automount/testdata/testExcludeAnnotationMatches.yaml new file mode 100644 index 000000000..9890eace3 --- /dev/null +++ b/pkg/provision/automount/testdata/testExcludeAnnotationMatches.yaml @@ -0,0 +1,52 @@ +name: Does not provision configmap, secret, or PVC when exclude annotation matches workspace name + +input: + configmaps: + - + apiVersion: v1 + kind: ConfigMap + metadata: + name: excluded-configmap + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-configmap: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/excluded-cm + controller.devfile.io/mount-to-devworkspace-exclude: "test-*" + data: + config-key: "config-value" + secrets: + - + apiVersion: v1 + kind: Secret + metadata: + name: excluded-secret + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-secret: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/excluded + controller.devfile.io/mount-to-devworkspace-exclude: "test-*" + stringData: + secret-key: "secret-value" + pvcs: + - + apiVersion: v1 + kind: PersistentVolumeClaim + metadata: + name: excluded-pvc + labels: + controller.devfile.io/mount-to-devworkspace: "true" + annotations: + controller.devfile.io/mount-path: /tmp/excluded-pvc + controller.devfile.io/mount-to-devworkspace-exclude: "test-*" + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +output: {} diff --git a/pkg/provision/automount/testdata/testIncludeAnnotationDoesNotMatch.yaml b/pkg/provision/automount/testdata/testIncludeAnnotationDoesNotMatch.yaml new file mode 100644 index 000000000..859e0066f --- /dev/null +++ b/pkg/provision/automount/testdata/testIncludeAnnotationDoesNotMatch.yaml @@ -0,0 +1,52 @@ +name: Does not provision configmap, secret, or PVC when include annotation does not match workspace name + +input: + configmaps: + - + apiVersion: v1 + kind: ConfigMap + metadata: + name: non-targeted-configmap + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-configmap: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/non-targeted + controller.devfile.io/mount-to-devworkspace-include: "other-ws, staging-*" + data: + config-key: "config-value" + secrets: + - + apiVersion: v1 + kind: Secret + metadata: + name: non-targeted-secret + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-secret: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/non-targeted-secret + controller.devfile.io/mount-to-devworkspace-include: "other-ws, staging-*" + stringData: + secret-key: "secret-value" + pvcs: + - + apiVersion: v1 + kind: PersistentVolumeClaim + metadata: + name: non-targeted-pvc + labels: + controller.devfile.io/mount-to-devworkspace: "true" + annotations: + controller.devfile.io/mount-path: /tmp/non-targeted-pvc + controller.devfile.io/mount-to-devworkspace-include: "other-ws, staging-*" + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +output: {} diff --git a/pkg/provision/automount/testdata/testIncludeAnnotationMatches.yaml b/pkg/provision/automount/testdata/testIncludeAnnotationMatches.yaml new file mode 100644 index 000000000..b2390bb66 --- /dev/null +++ b/pkg/provision/automount/testdata/testIncludeAnnotationMatches.yaml @@ -0,0 +1,73 @@ +name: Provisions configmap, secret, and PVC when include annotation matches workspace name + +input: + configmaps: + - + apiVersion: v1 + kind: ConfigMap + metadata: + name: targeted-configmap + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-configmap: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/targeted + controller.devfile.io/mount-to-devworkspace-include: "test-workspace, other-ws" + data: + config-key: "config-value" + secrets: + - + apiVersion: v1 + kind: Secret + metadata: + name: targeted-secret + labels: + controller.devfile.io/mount-to-devworkspace: "true" + controller.devfile.io/watch-secret: 'true' + annotations: + controller.devfile.io/mount-as: file + controller.devfile.io/mount-path: /tmp/targeted-secret + controller.devfile.io/mount-to-devworkspace-include: "test-workspace, other-ws" + stringData: + secret-key: "secret-value" + pvcs: + - + apiVersion: v1 + kind: PersistentVolumeClaim + metadata: + name: targeted-pvc + labels: + controller.devfile.io/mount-to-devworkspace: "true" + annotations: + controller.devfile.io/mount-path: /tmp/targeted-pvc + controller.devfile.io/mount-to-devworkspace-include: "test-workspace, other-ws" + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi + +output: + volumes: + - name: targeted-configmap + configmap: + name: targeted-configmap + defaultMode: 0640 + - name: targeted-secret + secret: + secretName: targeted-secret + defaultMode: 0640 + - name: targeted-pvc + persistentVolumeClaim: + claimName: targeted-pvc + volumeMounts: + - name: targeted-configmap + readOnly: true + mountPath: /tmp/targeted + - name: targeted-secret + readOnly: true + mountPath: /tmp/targeted-secret + - name: targeted-pvc + mountPath: /tmp/targeted-pvc From e73f0b0363fab19291890d733d45725f206e2aa6 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 6 May 2026 12:01:28 +0200 Subject: [PATCH 2/3] fixup Signed-off-by: Anatolii Bazko --- pkg/provision/automount/gitconfig.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/provision/automount/gitconfig.go b/pkg/provision/automount/gitconfig.go index a4d4d25a0..c82d949ee 100644 --- a/pkg/provision/automount/gitconfig.go +++ b/pkg/provision/automount/gitconfig.go @@ -100,6 +100,7 @@ func getGitResources( for _, secret := range secretList.Items { if !MatchesWorkspaceTarget(&secret, workspaceName) { log.V(1).Info("Skip Git credentials Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + continue } secrets = append(secrets, secret) @@ -125,6 +126,7 @@ func getGitResources( for _, cm := range configmapList.Items { if !MatchesWorkspaceTarget(&cm, workspaceName) { log.V(1).Info("skip Git ConfigMap mount, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) + continue } configmaps = append(configmaps, cm) From 76a4732ee3fa5d92ad603bc3eb20d4bc4ba00935 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 6 May 2026 14:45:31 +0200 Subject: [PATCH 3/3] fixup Signed-off-by: Anatolii Bazko --- controllers/workspace/eventhandlers.go | 5 +++-- docs/additional-configuration.adoc | 10 +++++----- pkg/constants/metadata.go | 4 ++-- pkg/provision/automount/common.go | 4 ++-- pkg/provision/automount/common_test.go | 10 ++++++++++ pkg/provision/automount/configmap.go | 2 +- pkg/provision/automount/gitconfig.go | 4 ++-- pkg/provision/automount/pvcs.go | 2 +- pkg/provision/automount/secret.go | 2 +- 9 files changed, 27 insertions(+), 16 deletions(-) diff --git a/controllers/workspace/eventhandlers.go b/controllers/workspace/eventhandlers.go index f32c1dc61..c2cf98a6f 100644 --- a/controllers/workspace/eventhandlers.go +++ b/controllers/workspace/eventhandlers.go @@ -126,12 +126,13 @@ func (r *DevWorkspaceReconciler) runningWorkspacesHandler(_ context.Context, obj } var reconciles []reconcile.Request for _, workspace := range dwList.Items { + // Queue reconciles ONLY for any started workspaces to make sure they pick up new object if !workspace.Spec.Started { continue } - // In context of performance. - // Don't trigger reconcile if automount resource doesn't match the target workspace. + // Skip workspaces that don't match the resource's include/exclude annotations + // to avoid unnecessary reconciles in multi-workspace clusters. if !automount.MatchesWorkspaceTarget(obj, workspace.GetName()) { continue } diff --git a/docs/additional-configuration.adoc b/docs/additional-configuration.adoc index d0222a138..db2c68195 100644 --- a/docs/additional-configuration.adoc +++ b/docs/additional-configuration.adoc @@ -170,11 +170,11 @@ When "file" is used, the configmap is mounted as a directory within the workspac + For git credential secrets (labeled with `controller.devfile.io/git-credential`) and git TLS configmaps (labelled with `controller.devfile.io/git-tls-credential`), -this annotation behave differently: -+ --- -** The `mount-on-start` annotation is evaluated across all git credential resources as a group, not individually. Since all git credentials are merged into a single mounted secret, if at least one git credential secret (or TLS configmap) lacks the `mount-on-start` annotation, all git credentials (or TLS configmaps) will be mounted, including those marked with `mount-on-start`. --- +this annotation behave differently. The `controller.devfile.io/mount-on-start` annotation is evaluated across +all git credential resources as a group, not individually. Since all git credentials are +merged into a single mounted secret, if at least one git credential secret (or TLS configmap) +lacks the `controller.devfile.io/mount-on-start` annotation, all git credentials +(or TLS configmaps) will be mounted, including those marked with `controller.devfile.io/mount-on-start`. * `controller.devfile.io/mount-to-devworkspace-include`: configure which DevWorkspaces the resource should be mounted to. The value is a comma-separated list of patterns matched against diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index d2c9ba14f..33f1c062f 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -55,7 +55,7 @@ const ( // Supported patterns: // - exact match `name` // - prefix match `name*` - // - suffix match `*name`, + // - suffix match `*name` // - contains match `*name*` // - matches all workspaces `*` DevWorkspaceMountIncludeAnnotation = "controller.devfile.io/mount-to-devworkspace-include" @@ -66,7 +66,7 @@ const ( // Supported patterns: // - exact match `name` // - prefix match `name*` - // - suffix match `*name`, + // - suffix match `*name` // - contains match `*name*` // - matches all workspaces `*` DevWorkspaceMountExcludeAnnotation = "controller.devfile.io/mount-to-devworkspace-exclude" diff --git a/pkg/provision/automount/common.go b/pkg/provision/automount/common.go index b4ca69523..8cae61ae3 100644 --- a/pkg/provision/automount/common.go +++ b/pkg/provision/automount/common.go @@ -238,7 +238,7 @@ func findGitconfigAutomount( // Filter resources by workspace name if !MatchesWorkspaceTarget(&cm, workspaceName) { - log.V(1).Info("skip ConfigMap, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) + log.V(1).Info("Skipping ConfigMap, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) continue } @@ -266,7 +266,7 @@ func findGitconfigAutomount( // Filter resources by workspace name if !MatchesWorkspaceTarget(&secret, workspaceName) { - log.V(1).Info("skip Secret, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + log.V(1).Info("Skipping Secret, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/common_test.go b/pkg/provision/automount/common_test.go index c85d5b20a..c19b0c1ca 100644 --- a/pkg/provision/automount/common_test.go +++ b/pkg/provision/automount/common_test.go @@ -1002,6 +1002,16 @@ func TestMatchesWorkspaceTarget(t *testing.T) { workspaceName: "my-workspace", expected: false, }, + // Edge cases + { + name: "Should mount to all except my-workspace", + annotations: map[string]string{ + constants.DevWorkspaceMountIncludeAnnotation: "*", + constants.DevWorkspaceMountExcludeAnnotation: "my-workspace", + }, + workspaceName: "workspace-123.dev", + expected: true, + }, } for _, tt := range tests { diff --git a/pkg/provision/automount/configmap.go b/pkg/provision/automount/configmap.go index 5c9628f8b..a1e067c96 100644 --- a/pkg/provision/automount/configmap.go +++ b/pkg/provision/automount/configmap.go @@ -49,7 +49,7 @@ func getDevWorkspaceConfigmaps( for _, configmap := range configmaps.Items { // Filter resources by workspace name if !MatchesWorkspaceTarget(&configmap, workspaceName) { - log.V(1).Info("Skip ConfigMap mount, workspace does not match include/exclude annotations", "namespace", configmap.Namespace, "name", configmap.Name, "workspace", workspaceName) + log.V(1).Info("Skipping ConfigMap mount, workspace does not match include/exclude annotations", "namespace", configmap.Namespace, "name", configmap.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/gitconfig.go b/pkg/provision/automount/gitconfig.go index c82d949ee..5f0ebe843 100644 --- a/pkg/provision/automount/gitconfig.go +++ b/pkg/provision/automount/gitconfig.go @@ -99,7 +99,7 @@ func getGitResources( var secrets []corev1.Secret for _, secret := range secretList.Items { if !MatchesWorkspaceTarget(&secret, workspaceName) { - log.V(1).Info("Skip Git credentials Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + log.V(1).Info("Skipping Git credentials Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) continue } @@ -125,7 +125,7 @@ func getGitResources( var configmaps []corev1.ConfigMap for _, cm := range configmapList.Items { if !MatchesWorkspaceTarget(&cm, workspaceName) { - log.V(1).Info("skip Git ConfigMap mount, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) + log.V(1).Info("Skipping Git ConfigMap mount, workspace does not match include/exclude annotations", "namespace", cm.Namespace, "name", cm.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/pvcs.go b/pkg/provision/automount/pvcs.go index 9936513a2..9910290a0 100644 --- a/pkg/provision/automount/pvcs.go +++ b/pkg/provision/automount/pvcs.go @@ -87,7 +87,7 @@ func getAutoMountPVCs( for _, pvc := range pvcs.Items { // Filter resources by workspace name if !MatchesWorkspaceTarget(&pvc, workspaceName) { - log.V(1).Info("skip PVC mount, workspace does not match include/exclude annotations", "namespace", pvc.Namespace, "name", pvc.Name, "workspace", workspaceName) + log.V(1).Info("Skipping PVC mount, workspace does not match include/exclude annotations", "namespace", pvc.Namespace, "name", pvc.Name, "workspace", workspaceName) continue } diff --git a/pkg/provision/automount/secret.go b/pkg/provision/automount/secret.go index d0c6507b0..9dd86822e 100644 --- a/pkg/provision/automount/secret.go +++ b/pkg/provision/automount/secret.go @@ -47,7 +47,7 @@ func getDevWorkspaceSecrets( for _, secret := range secrets.Items { // Filter resources by workspace name if !MatchesWorkspaceTarget(&secret, workspaceName) { - log.V(1).Info("skip Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) + log.V(1).Info("Skipping Secret mount, workspace does not match include/exclude annotations", "namespace", secret.Namespace, "name", secret.Name, "workspace", workspaceName) continue }