fix: identical endpoint name conflicts

adr/2026-10-07-discoverable-endpoint-index.md

@@ -0,0 +1,56 @@
+# Index discoverable endpoints for admission conflict checks
+
+**Status**: Accepted
+**Date**: 2026-10-07
+**Deciders**: Not recorded
+
+## Context
+
+Admission checks for a workspace with discoverable endpoints currently list every DevWorkspace in its namespace, then scan the returned specs for colliding Service names. This copies and examines unrelated workspaces. The existing `discoverableEndpointNames` helper already extracts normalized, unique endpoint names when the discoverable attribute is boolean or string `true` and exposure is not `none`.
+
+## Decision
+
+Register a controller-runtime field index on DevWorkspace objects under `controller.devfile.io/discoverable-endpoint`. Its extractor reuses `discoverableEndpointNames`. Register the index before webhook handlers are registered and before the manager starts.
+
+For each incoming normalized discoverable endpoint name, admission performs one namespace-scoped List using that exact indexed name. Keep the existing update gating, cached-client copy behavior, self-UID exclusion, error handling, and defensive endpoint scan of returned candidates. The scan remains the final admission check for a matching candidate.
+
+The index covers endpoints visible in `.spec.template`; contributed endpoints remain protected by the controller's Service synchronization ownership check. Admission remains eventually consistent, so concurrent requests can both pass; the controller's fresh Service ownership check remains authoritative. No API, RBAC, dependency, or code-generation changes are required.
+
+## Considered Alternatives
+
+### Alternative 1: Keep listing and scanning the whole namespace
+
+This is simpler, but continues copying every workspace and scanning unrelated specs for each admission check.
+
+**Rejected because**: The field index targets the matching workspaces through the existing cache.
+
+### Alternative 2: Disable cache object copying
+
+This would avoid copies for all listed workspaces but exposes shared cached objects to mutation and races.
+
+**Rejected because**: Limiting query results with the index avoids copying unrelated workspaces while retaining the client's normal safety behavior.
+
+### Alternative 3: Add a custom cache or external endpoint index owner
+
+This would add another mechanism and lifecycle to maintain.
+
+**Rejected because**: A native controller-runtime field index provides the needed lookup within the existing manager and cache.
+
+## Consequences
+
+### Positive
+
+Only workspaces matching each incoming discoverable endpoint name are copied and scanned.
+
+### Negative
+
+The cache uses memory for the index and CPU to extract and update indexed names. Requests with multiple incoming names issue one List per name.
+
+### Neutral
+
+The lookup remains a best-effort admission check over cached state. Existing update gating, conflict responses, and the controller's Service conflict guard retain their roles.
+
+## References
+
+- [Endpoint validation](../webhook/workspace/handler/validate.go)
+- [Webhook configuration](../webhook/workspace/config.go)

adr/2026-10-08-discoverable-service-certificates.md

@@ -0,0 +1,59 @@
+# Mount per-service certificates for discoverable endpoints
+
+**Status**: Accepted
+**Date**: 2026-10-08
+**Deciders**: Not recorded
+
+## Context
+
+With TLS enabled, cluster routing creates an aggregate workspace Service and one Service per discoverable endpoint. OpenShift issues a serving certificate for each annotated Service. Mounting all certificates at `/var/serving-cert/` overlaps mounts; clients need a distinct certificate location for each discoverable Service.
+
+## Decision
+
+Keep the aggregate Service certificate mounted at `/var/serving-cert/`. Mount each discoverable Service's certificate at `/var/serving-cert/<service-name>/`. The solver annotates each Service with its own name, and the Secret reference continues to use that actual Service name.
+
+Keep `devworkspace-serving-cert-<service-name>` when the full generated volume name fits within 63 characters; otherwise use `devworkspace-cert-` plus the hex encoding of the first 10 SHA-256 bytes of the Service name. Distinct prefixes prevent a digest name from deterministically colliding with one generated for a short Service name. Hashing avoids truncation collisions; the 80-bit digest can still collide and is not guaranteed unique. Service and Secret names remain unchanged.
+
+## Considered Alternatives
+
+### Alternative 1: Share the aggregate certificate mount
+
+Expose the aggregate certificate to workloads and use it for discoverable Service connections.
+
+**Rejected because**: OpenShift binds each serving certificate to its Service's internal DNS name. Reusing the aggregate certificate would leave the discoverable Service DNS names uncovered. [OpenShift certificate documentation](https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html/security_and_compliance/configuring-certificates)
+
+### Alternative 2: Truncate long volume names or reject long Service names
+
+Shorten generated names to fit Kubernetes' 63-character volume-name limit, or disallow endpoint names that produce longer values.
+
+**Rejected because**: Truncation can merge names from distinct Services, while rejecting them would disallow otherwise valid Service names.
+
+### Alternative 3: Hash every volume name
+
+Use a fixed-length digest-based name for all Services.
+
+**Rejected because**: It removes readable names for ordinary cases without avoiding any additional limit.
+
+## Consequences
+
+### Positive
+
+- Each discoverable Service has a distinct certificate mount path alongside the unchanged aggregate mount.
+- Long valid Service names can be represented by Kubernetes-compatible volume names without truncation.
+
+### Negative
+
+- Each discoverable Service adds a pod volume and mount.
+- The 80-bit digest has a theoretical collision risk for distinct long Service names.
+
+### Neutral
+
+- Workloads use the actual Service name to identify the corresponding Secret; only the generated volume name may differ for long names.
+- The reserved-name guard prevents a discoverable Service from taking the aggregate Service name, and duplicate normalized Service names are rejected.
+
+## References
+
+- [Cluster solver](../controllers/controller/devworkspacerouting/solvers/cluster_solver.go)
+- [Cluster solver certificate mount tests](../controllers/controller/devworkspacerouting/solvers/cluster_solver_test.go)
+- [Serving certificate volume naming](../pkg/common/naming.go)
+- [Discoverable Service name guard](../controllers/controller/devworkspacerouting/solvers/common.go)

adr/2026-10-08-routing-service-ownership.md

@@ -0,0 +1,62 @@
+# Guard routing Service ownership during synchronization
+
+**Status**: Accepted
+**Date**: 2026-10-08
+**Deciders**: Not recorded
+
+## Context
+
+DevWorkspaceRouting Services share namespace names, including discoverable endpoints. Admission reads cached workspace state, so concurrent requests can both pass; the cache can also hide a Service missing its workspace ID label. Treating that miss as proof a name is free risks overwriting another routing's Service.
+
+## Decision
+
+Identify routing Services by the discoverable annotation or DevWorkspaceRouting controller owner. When configured, read them through the non-caching client (controller setup supplies it) and use this fresh result for ownership. If the desired Service has a controller owner, its UID must match the existing controller owner's UID; that UID remains authoritative when the workspace ID label is stale or missing, allowing synchronization to repair the label. Without a desired controller owner, require a nonempty workspace ID label matching the existing Service. Reject a foreign same-name Service as a permanent conflict.
+
+If Service creation returns AlreadyExists, retry instead of using the generic update fallback. Updates carry the existing UID, resourceVersion, and ClusterIP; cleanup deletes only a routing-controlled Service with UID and resourceVersion preconditions. These guards reject writes against a changed or replaced Service; update conflicts and missing objects trigger another reconcile. The read and ownership decision are not atomic, and this does not reserve names across reconcilers.
+
+## Considered Alternatives
+
+### Alternative 1: Trust admission conflict checks
+
+Use cached admission as the sole protection for discoverable endpoint names.
+
+**Rejected because**: Stale cache state lets concurrent requests both pass before a Service exists.
+
+### Alternative 2: Treat workspace ID labels as ownership proof
+
+Require the existing Service label to match the desired workspace ID.
+
+**Rejected because**: A label match does not prove matching controller ownership, and labels can drift. A valid routing owner can repair a stale label without losing its Service.
+
+### Alternative 3: Keep the generic AlreadyExists update fallback
+
+Use generic update after Service creation reports AlreadyExists.
+
+**Rejected because**: The fallback lacks a fresh object for ownership validation and identity preservation.
+
+## Consequences
+
+### Positive
+
+- The fresh check protects foreign Services hidden by cache filtering.
+- Owned Services with stale labels can be repaired while preserving identity and ClusterIP.
+
+### Negative
+
+- Fresh reads add API calls and latency; changed Services can make guarded writes conflict and require another reconcile.
+
+### Neutral
+
+- Admission remains advisory; the controller check is authoritative. Concurrent creates rely on Kubernetes name uniqueness and retry.
+
+## References
+
+- [Service synchronization](../pkg/provision/sync/sync.go)
+- [Service update fields](../pkg/provision/sync/update.go)
+- [Service conflict error](../pkg/provision/sync/service.go)
+- [Routing Service cleanup](../controllers/controller/devworkspacerouting/sync_services.go)
+- [Routing controller client setup](../controllers/controller/devworkspacerouting/devworkspacerouting_controller.go)
+- [Ownership and cache-miss tests](../pkg/provision/sync/service_test.go)
+- [Routing cleanup and conflict tests](../controllers/controller/devworkspacerouting/sync_services_test.go)
+- [Label repair tests](../controllers/controller/devworkspacerouting/workspace_name_test.go)
+- [Admission index decision](2026-10-07-discoverable-endpoint-index.md)

controllers/controller/devworkspacerouting/devworkspacerouting_controller.go

@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"time"
 
+	dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+
 	"github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/solvers"
 	maputils "github.com/devfile/devworkspace-operator/internal/map"
 	"github.com/devfile/devworkspace-operator/pkg/config"
@@ -34,7 +36,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -54,8 +58,9 @@ const devWorkspaceRoutingFinalizer = "devworkspacerouting.controller.devfile.io"
 // DevWorkspaceRoutingReconciler reconciles a DevWorkspaceRouting object
 type DevWorkspaceRoutingReconciler struct {
 	client.Client
-	Log    logr.Logger
-	Scheme *runtime.Scheme
+	NonCachingClient client.Client
+	Log              logr.Logger
+	Scheme           *runtime.Scheme
 	// SolverGetter will be used to get solvers for a particular devWorkspaceRouting
 	SolverGetter solvers.RoutingSolverGetter
 	// Enable additional debug logging
@@ -97,6 +102,7 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
 	solver, err := r.SolverGetter.GetSolver(r.Client, instance.Spec.RoutingClass)
 	if err != nil {
 		if errors.Is(err, solvers.RoutingNotSupported) {
+			reqLogger.Info("Routing class not supported by this controller, skipping reconciliation", "routingClass", instance.Spec.RoutingClass)
 			return reconcile.Result{}, nil
 		}
 		return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Invalid routingClass for DevWorkspace: %s", err))
@@ -126,9 +132,11 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
 	}
 
 	workspaceMeta := solvers.DevWorkspaceMetadata{
-		DevWorkspaceId: instance.Spec.DevWorkspaceId,
-		Namespace:      instance.Namespace,
-		PodSelector:    instance.Spec.PodSelector,
+		DevWorkspaceId:         instance.Spec.DevWorkspaceId,
+		DevWorkspaceName:       workspaceName(instance),
+		DevWorkspaceRoutingUID: instance.UID,
+		Namespace:              instance.Namespace,
+		PodSelector:            instance.Spec.PodSelector,
 	}
 
 	restrictedAccess, setRestrictedAccess := instance.Annotations[constants.DevWorkspaceRestrictedAccessAnnotation]
@@ -150,6 +158,18 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
 			return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Unable to provision networking for DevWorkspace: %s", invalid))
 		}
 
+		var conflict *solvers.ServiceConflictError
+		if errors.As(err, &conflict) {
+			reqLogger.Error(conflict, "Routing controller detected a service conflict", "endpointName", conflict.EndpointName, "workspaceName", conflict.WorkspaceName)
+			return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Unable to provision networking for DevWorkspace: %s", conflict))
+		}
+
+		var duplicate *solvers.DuplicateEndpointError
+		if errors.As(err, &duplicate) {
+			reqLogger.Error(duplicate, "Routing controller detected a duplicate endpoint name", "endpointName", duplicate.EndpointName)
+			return reconcile.Result{}, r.markRoutingFailed(instance, fmt.Sprintf("Unable to provision networking for DevWorkspace: %s", duplicate))
+		}
+
 		// generic error, just fail the reconciliation
 		return reconcile.Result{}, err
 	}
@@ -241,6 +261,15 @@ func (r *DevWorkspaceRoutingReconciler) Reconcile(ctx context.Context, req ctrl.
 	return reconcile.Result{}, r.reconcileStatus(instance, &routingObjects, exposedEndpoints, endpointsAreReady, "")
 }
 
+// Generated by Codex.
+func workspaceName(routing *controllerv1alpha1.DevWorkspaceRouting) string {
+	owner := metav1.GetControllerOf(routing)
+	if owner == nil || owner.Kind != "DevWorkspace" || schema.FromAPIVersionAndKind(owner.APIVersion, owner.Kind).Group != dwv2.SchemeGroupVersion.Group {
+		return ""
+	}
+	return owner.Name
+}
+
 // setFinalizer ensures a finalizer is set on a devWorkspaceRouting instance; no-op if finalizer is already present.
 func (r *DevWorkspaceRoutingReconciler) setFinalizer(reqLogger logr.Logger, solver solvers.RoutingSolver, m *controllerv1alpha1.DevWorkspaceRouting) error {
 	if !solver.FinalizerRequired(m) || contains(m.GetFinalizers(), devWorkspaceRoutingFinalizer) {
@@ -335,6 +364,14 @@ func remove(list []string, s string) []string {
 }
 
 func (r *DevWorkspaceRoutingReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	if r.NonCachingClient == nil {
+		freshClient, err := client.New(mgr.GetConfig(), client.Options{Scheme: mgr.GetScheme()})
+		if err != nil {
+			return err
+		}
+		r.NonCachingClient = freshClient
+	}
+
 	maxConcurrentReconciles, err := config.GetMaxConcurrentReconciles()
 	if err != nil {
 		return err

controllers/controller/devworkspacerouting/devworkspacerouting_controller_test.go

@@ -14,22 +14,86 @@
 package devworkspacerouting_test
 
 import (
+	"context"
 	"fmt"
+	"testing"
 
-	controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
-	"github.com/devfile/devworkspace-operator/pkg/common"
-	"github.com/devfile/devworkspace-operator/pkg/config"
-	"github.com/devfile/devworkspace-operator/pkg/constants"
-	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	routeV1 "github.com/openshift/api/route/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting"
+	"github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/solvers"
+	"github.com/devfile/devworkspace-operator/pkg/common"
+	"github.com/devfile/devworkspace-operator/pkg/config"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
+	"github.com/devfile/devworkspace-operator/pkg/infrastructure"
 )
 
+func TestReconcileMarksDuplicateEndpointRoutingFailed(t *testing.T) {
+	infrastructure.InitializeForTesting(infrastructure.Kubernetes)
+	g := NewWithT(t)
+	scheme := runtime.NewScheme()
+	g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
+	g.Expect(controllerv1alpha1.AddToScheme(scheme)).To(Succeed())
+	routing := &controllerv1alpha1.DevWorkspaceRouting{
+		ObjectMeta: metav1.ObjectMeta{Name: "duplicate-endpoints", Namespace: "test-namespace"},
+		Spec: controllerv1alpha1.DevWorkspaceRoutingSpec{
+			DevWorkspaceId: "workspace-id",
+			RoutingClass:   controllerv1alpha1.DevWorkspaceRoutingCluster,
+			PodSelector:    map[string]string{constants.DevWorkspaceIDLabel: "workspace-id"},
+			Endpoints: map[string]controllerv1alpha1.EndpointList{
+				"machine1": {{
+					Name:       "my--endpoint",
+					TargetPort: 5000,
+					Exposure:   controllerv1alpha1.InternalEndpointExposure,
+					Attributes: controllerv1alpha1.Attributes{}.
+						PutBoolean(string(controllerv1alpha1.DiscoverableAttribute), true),
+				}},
+				"machine2": {{
+					Name:       "my-endpoint",
+					TargetPort: 6000,
+					Exposure:   controllerv1alpha1.InternalEndpointExposure,
+					Attributes: controllerv1alpha1.Attributes{}.
+						PutBoolean(string(controllerv1alpha1.DiscoverableAttribute), true),
+				}},
+			},
+		},
+	}
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
+		WithStatusSubresource(&controllerv1alpha1.DevWorkspaceRouting{}).WithObjects(routing).Build()
+	reconciler := &devworkspacerouting.DevWorkspaceRoutingReconciler{
+		Client:       fakeClient,
+		Log:          ctrl.Log,
+		Scheme:       scheme,
+		SolverGetter: &solvers.SolverGetter{},
+	}
+	testCtx := context.Background()
+	key := client.ObjectKeyFromObject(routing)
+	result, err := reconciler.Reconcile(testCtx, ctrl.Request{NamespacedName: key})
+	g.Expect(err).To(Succeed())
+	g.Expect(result).To(Equal(ctrl.Result{}))
+	stored := &controllerv1alpha1.DevWorkspaceRouting{}
+	g.Expect(fakeClient.Get(testCtx, key, stored)).To(Succeed())
+	g.Expect(stored.Status.Phase).To(Equal(controllerv1alpha1.RoutingFailed))
+	g.Expect(stored.Status.Message).To(ContainSubstring("is declared by more than one component"))
+	// Container-map iteration determines which raw alias is reported.
+	g.Expect(stored.Status.Message).To(Or(ContainSubstring("'my--endpoint'"), ContainSubstring("'my-endpoint'")))
+	services := &corev1.ServiceList{}
+	g.Expect(fakeClient.List(testCtx, services)).To(Succeed())
+	g.Expect(services.Items).To(BeEmpty())
+}
+
 var _ = Describe("DevWorkspaceRouting Controller", func() {
 	Context("Basic DevWorkspaceRouting Tests", func() {
 		It("Gets Ready Status on OpenShift", func() {