fix: sanitize automount volume names to be DNS-1123 compliant

adr/2026-08-26-sanitize-automount-volume-names.md

@@ -0,0 +1,95 @@
+# Sanitize automount volume names to be DNS-1123 compliant
+
+**Status**: Proposed
+**Date**: 2026-08-26
+**Deciders**: DevWorkspace Operator maintainers
+**Related Issue**: CRW-9800
+
+## Context
+
+When a Secret, ConfigMap, or PVC is auto-mounted into a workspace (via the
+`controller.devfile.io/mount-to-devworkspace` label), DWO derives a pod volume
+name from the object's name. Previously the object name was used verbatim
+(`AutoMountSecretVolumeName`, `AutoMountConfigMapVolumeName`,
+`AutoMountPVCVolumeName` all returned their input unchanged).
+
+Kubernetes object names and volume names have *different* validation rules. A
+Secret named `test.pullsecret` is perfectly legal, but a pod volume name must be
+a DNS-1123 label (lowercase alphanumeric plus `-`, must start/end alphanumeric,
+≤63 chars). A dot is invalid in a volume name. As a result, auto-mounting a
+secret whose name contained a dot (or other invalid character) produced an
+invalid Deployment, and the workspace failed to start.
+
+The object name itself is valid and must be preserved — the volume's
+`secretName`/`configMap.name`/`claimName` still has to reference the real
+object. Only DWO's *derivation* of the volume name was wrong.
+
+## Decision
+
+Sanitize the derived volume name to a DNS-1123 label via a shared
+`sanitizeVolumeName` helper in `pkg/common/naming.go`, used by all three
+`AutoMount*VolumeName` functions. Sanitization lowercases the name, replaces
+runs of invalid characters with `-`, trims leading/trailing `-`, and truncates
+to 63 characters (trimming any trailing `-` left by truncation).
+
+The volume's reference to the underlying object (`secretName`, `configMap.name`,
+`claimName`) continues to use the original, unmodified object name.
+
+## Considered Alternatives
+
+### Alternative 1: Reject invalid object names at admission (webhook validation)
+
+Add a validating webhook that denies a workspace (or the labeled object) when an
+auto-mount source has a name that cannot form a valid volume name.
+
+**Rejected because**:
+- The object name is legal Kubernetes; rejecting it pushes a DWO-internal
+  limitation onto the user, who did nothing wrong.
+- Auto-mounted objects are matched by label and can be created independently of
+  (and after) the workspace, so there is no single admission point that cleanly
+  owns this validation.
+- It is a worse user experience: the workspace fails instead of just working.
+
+### Alternative 2: Keep names verbatim, only truncate for length
+
+The pre-existing behavior already tolerated long names implicitly; only add
+length handling.
+
+**Rejected because**:
+- It does not fix the reported bug — invalid *characters* (dots, underscores,
+  etc.), not just length, are the failure in CRW-9800.
+
+## Consequences
+
+### Positive
+
+1. Auto-mounting objects with names that are legal in Kubernetes but invalid as
+   volume names now works transparently.
+2. Length handling (≤63 chars) is now correct as a side effect, replacing the
+   previous reliance on never adding characters to the name.
+
+### Negative
+
+1. Sanitization is not injective: two distinct object names can map to the same
+   volume name (e.g. `test.pullsecret` and `test-pullsecret`). This is an
+   accepted trade-off. Because `checkAutomountVolumesForCollision` previously only
+   detected DevWorkspace-vs-automount name collisions and mount-path collisions —
+   not two *automounted* objects resolving to the same name — this change also
+   extends that check to catch the new case, so it surfaces a clear error rather
+   than producing an invalid pod spec that the API server rejects. Previously
+   these names were distinct; the collision case is new but rare and fails loudly.
+
+### Neutral
+
+1. The old comment on `AutoMount*VolumeName` explaining why prefixes were not
+   added (to avoid exceeding 63 chars) was removed, as length is now handled
+   explicitly by `sanitizeVolumeName`.
+
+## References
+
+- `pkg/common/naming.go` — `sanitizeVolumeName` and the `AutoMount*VolumeName` functions
+- `pkg/common/naming_test.go` — unit tests for sanitization
+- `pkg/provision/automount/testdata/testSanitizesInvalidVolumeNames.yaml` — fixture-based integration test
+- `pkg/provision/automount/testdata/errorDuplicateVolumeNameAfterSanitization.yaml` — fixture for the collision case
+- `test/e2e/pkg/tests/automount_volume_sanitization_tests.go` — end-to-end test
+- `pkg/provision/automount/common.go` — `checkAutomountVolumesForCollision` (extended to detect automount-vs-automount name collisions)

pkg/common/naming.go

@@ -1,5 +1,5 @@
 //
-// Copyright (c) 2019-2025 Red Hat, Inc.
+// Copyright (c) 2019-2026 Red Hat, Inc.
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -116,19 +116,34 @@ func MetadataConfigMapName(workspaceId string) string {
 	return fmt.Sprintf("%s-metadata", workspaceId)
 }
 
-// We can't add prefixes to automount volume names, as adding any characters
-// can potentially push the name over the 63 character limit (if the original
-// object has a long name)
 func AutoMountConfigMapVolumeName(volumeName string) string {
-	return volumeName
+	return sanitizeVolumeName(volumeName)
 }
 
 func AutoMountSecretVolumeName(volumeName string) string {
-	return volumeName
+	return sanitizeVolumeName(volumeName)
 }
 
 func AutoMountPVCVolumeName(pvcName string) string {
-	return pvcName
+	return sanitizeVolumeName(pvcName)
+}
+
+// sanitizeVolumeName converts a name to be DNS-1123 label compliant for use as a Kubernetes volume name.
+// Volume names must:
+// - contain at most 63 characters
+// - contain only lowercase alphanumeric characters or '-'
+// - start with an alphanumeric character
+// - end with an alphanumeric character
+//
+// Generated by Claude
+func sanitizeVolumeName(name string) string {
+	sanitized := strings.ToLower(name)
+	sanitized = NonAlphaNumRegexp.ReplaceAllString(sanitized, "-")
+	sanitized = strings.Trim(sanitized, "-")
+	if len(sanitized) > 63 {
+		sanitized = strings.TrimSuffix(sanitized[:63], "-")
+	}
+	return sanitized
 }
 
 func AutoMountProjectedVolumeName(mountPath string) string {

pkg/common/naming_test.go

@@ -0,0 +1,80 @@
+//
+// Copyright (c) 2019-2026 Red Hat, Inc.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package common
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSanitizeVolumeName(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "replaces dots with hyphens",
+			input:    "test.pullsecret",
+			expected: "test-pullsecret",
+		},
+		{
+			name:     "replaces all invalid characters and lowercases",
+			input:    "Test.Secret_Name@example",
+			expected: "test-secret-name-example",
+		},
+		{
+			name:     "collapses consecutive invalid characters and trims edges",
+			input:    ".test...secret.",
+			expected: "test-secret",
+		},
+		{
+			// Hyphens are non-alphanumeric, so the [^a-z0-9]+ regex matches a run of
+			// literal hyphens and collapses it to a single '-'. This guarantees the
+			// sanitized name can never contain two or more consecutive hyphens.
+			name:     "collapses consecutive literal hyphens into a single hyphen",
+			input:    "test--.-secret",
+			expected: "test-secret",
+		},
+		{
+			name:     "leaves already valid names unchanged",
+			input:    "valid-secret-123",
+			expected: "valid-secret-123",
+		},
+		{
+			name:     "truncates to 63 characters",
+			input:    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-----bb",
+			expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-b",
+		},
+		{
+			name:     "truncates characters without a trailing hyphen, keeping a valid ending",
+			input:    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-----bb",
+			expected: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := sanitizeVolumeName(tt.input)
+			assert.Equal(t, tt.expected, result, "sanitizeVolumeName(%q) should match expected value", tt.input)
+
+			// Verify DNS-1123 label compliance
+			assert.LessOrEqual(t, len(result), 63, "Volume name should not exceed 63 characters")
+			assert.Regexp(t, "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$", result, "Volume name should be a valid DNS-1123 label")
+		})
+	}
+}

pkg/provision/automount/common.go

@@ -126,9 +126,18 @@ func getAutomountResources(
 }
 
 func checkAutomountVolumesForCollision(podAdditions *v1alpha1.PodAdditions, automount *Resources) error {
-	// Get a map of automounted volume names to volume structs
+	// Get a map of automounted volume names to volume structs. Two automounted objects can resolve to the
+	// same (sanitized) volume name -- e.g. secrets 'test.pullsecret' and 'test-pullsecret' both sanitize to
+	// 'test-pullsecret' -- which would produce an invalid pod spec with duplicate volume names. Detect this
+	// here so it surfaces as a clear error instead of a Deployment rejected by the API server.
 	automountVolumeNames := map[string]corev1.Volume{}
 	for _, volume := range automount.Volumes {
+		if conflict, exists := automountVolumeNames[volume.Name]; exists {
+			return &dwerrors.FailError{
+				Message: fmt.Sprintf("auto-mounted volumes from %s and %s resolve to the same volume name '%s'",
+					formatVolumeDescription(volume), formatVolumeDescription(conflict), volume.Name),
+			}
+		}
 		automountVolumeNames[volume.Name] = volume
 	}
 

pkg/provision/automount/common_test.go

@@ -167,7 +167,11 @@ func TestProvisionAutomountResourcesInto(t *testing.T) {
 
 func TestCheckAutoMountVolumesForCollision(t *testing.T) {
 	type volumeDesc struct {
-		name       string
+		name string
+		// sourceName is the name of the underlying object (secret/configmap/pvc) referenced by the volume.
+		// It defaults to name when empty; set it separately to model two distinct objects whose (sanitized)
+		// volume names collide.
+		sourceName string
 		mountPath  string
 		volumeType mountedVolumeType
 	}
@@ -251,16 +255,38 @@ func TestCheckAutoMountVolumesForCollision(t *testing.T) {
 			},
 			errRegexp: "auto-mounted volumes from configmap 'testVolume2' and secret 'testVolume1' have the same mount path",
 		},
+		{
+			name: "Detects volume name collision between automounted volumes",
+			automountPodAdditions: []volumeDesc{
+				{
+					name:       "test-pullsecret",
+					sourceName: "test.pullsecret",
+					mountPath:  "/test/mount1",
+					volumeType: secretVolumeType,
+				},
+				{
+					name:       "test-pullsecret",
+					sourceName: "test-pullsecret",
+					mountPath:  "/test/mount2",
+					volumeType: secretVolumeType,
+				},
+			},
+			errRegexp: "auto-mounted volumes from secret 'test-pullsecret' and secret 'test.pullsecret' resolve to the same volume name 'test-pullsecret'",
+		},
 	}
 
 	convertDescToVolume := func(desc volumeDesc) (*corev1.Volume, *corev1.VolumeMount, *corev1.Container) {
+		sourceName := desc.sourceName
+		if sourceName == "" {
+			sourceName = desc.name
+		}
 		switch desc.volumeType {
 		case secretVolumeType:
 			volume := &corev1.Volume{
 				Name: desc.name,
 				VolumeSource: corev1.VolumeSource{
 					Secret: &corev1.SecretVolumeSource{
-						SecretName: desc.name,
+						SecretName: sourceName,
 					},
 				},
 			}
@@ -275,7 +301,7 @@ func TestCheckAutoMountVolumesForCollision(t *testing.T) {
 				VolumeSource: corev1.VolumeSource{
 					ConfigMap: &corev1.ConfigMapVolumeSource{
 						LocalObjectReference: corev1.LocalObjectReference{
-							Name: desc.name,
+							Name: sourceName,
 						},
 					},
 				},

pkg/provision/automount/testdata/errorDuplicateVolumeNameAfterSanitization.yaml

@@ -0,0 +1,36 @@
+# Two objects whose names differ only by characters that sanitization collapses
+# (a dot vs a hyphen) resolve to the same volume name. This must fail with a clear
+# error rather than producing an invalid pod spec with duplicate volume names.
+name: "Errors when two automounted objects resolve to the same volume name"
+
+input:
+  secrets:
+    -
+      apiVersion: v1
+      kind: Secret
+      metadata:
+        name: test.pullsecret
+        labels:
+          controller.devfile.io/mount-to-devworkspace: "true"
+          controller.devfile.io/watch-secret: "true"
+        annotations:
+          controller.devfile.io/mount-as: file
+      type: Opaque
+      data:
+        test_data: aGVsbG8K # "hello"
+    -
+      apiVersion: v1
+      kind: Secret
+      metadata:
+        name: test-pullsecret
+        labels:
+          controller.devfile.io/mount-to-devworkspace: "true"
+          controller.devfile.io/watch-secret: "true"
+        annotations:
+          controller.devfile.io/mount-as: file
+      type: Opaque
+      data:
+        test_data: aGVsbG8K # "hello"
+
+output:
+  errRegexp: "resolve to the same volume name 'test-pullsecret'"

pkg/provision/automount/testdata/testSanitizesInvalidVolumeNames.yaml

@@ -0,0 +1,54 @@
+# Volume names derived from automounted objects must be DNS-1123 label
+# compliant. Object names may legally contain characters (e.g. dots) that are
+# invalid in a volume name, so the derived volume name must be sanitized while
+# still referencing the original object by its real name.
+name: "Sanitizes invalid characters in automount volume names"
+
+input:
+  secrets:
+    -
+      apiVersion: v1
+      kind: Secret
+      metadata:
+        name: test.pullsecret
+        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/secret/file
+      type: Opaque
+      data:
+        test_data: aGVsbG8K # "hello"
+  configmaps:
+    -
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: test.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/configmap/file
+      data:
+        configmap-key: "hello"
+
+output:
+  volumes:
+  - name: test-pullsecret
+    secret:
+      secretName: test.pullsecret
+      defaultMode: 0640
+  - name: test-configmap
+    configmap:
+      name: test.configmap
+      defaultMode: 0640
+  volumeMounts:
+  - name: test-pullsecret
+    readOnly: true
+    mountPath: /tmp/secret/file
+  - name: test-configmap
+    readOnly: true
+    mountPath: /tmp/configmap/file