fixup! fixup! fixup! fixup! fix: identical endpoint name conflicts

Address CodeRabbitAI review feedback:
- Return (*ServiceConflictError, error) from validateEndpoints so
  infrastructure errors use admission.Errored instead of admission.Denied
- Fix import ordering in common_test.go and validate_test.go (3 groups)
- Add .claude/settings.local.json to .gitignore

Assisted-by: Claude Code (claude-opus-4-6)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>

Author: akurinnoy

.gitignore

@@ -6,6 +6,7 @@ deploy/current
 deploy/deployment/olm
 generated
 .vscode
+.claude/settings.local.json
 __debug_bin
 
 # Ignore cached kustomize for makefile

controllers/controller/devworkspacerouting/solvers/common_test.go

@@ -16,14 +16,15 @@ package solvers
 import (
 	"testing"
 
-	controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
-	"github.com/devfile/devworkspace-operator/pkg/constants"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+
+	controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
+	"github.com/devfile/devworkspace-operator/pkg/constants"
 )
 
 func TestGetDiscoverableServicesForEndpoints(t *testing.T) {

webhook/workspace/handler/validate.go

@@ -78,9 +78,12 @@ func (h *WebhookHandler) ValidateDevfile(ctx context.Context, req admission.Requ
 		}
 	}
 
-	endpointErrors := h.validateEndpoints(ctx, wksp)
-	if endpointErrors != nil {
-		devfileErrors = append(devfileErrors, endpointErrors.Error())
+	conflict, err := h.validateEndpoints(ctx, wksp)
+	if err != nil {
+		return admission.Errored(http.StatusInternalServerError, err)
+	}
+	if conflict != nil {
+		devfileErrors = append(devfileErrors, conflict.Error())
 	}
 
 	if len(devfileErrors) > 0 {
@@ -90,7 +93,7 @@ func (h *WebhookHandler) ValidateDevfile(ctx context.Context, req admission.Requ
 	return admission.Allowed("No Devfile errors were found")
 }
 
-func (h *WebhookHandler) validateEndpoints(ctx context.Context, workspace *dwv2.DevWorkspace) error {
+func (h *WebhookHandler) validateEndpoints(ctx context.Context, workspace *dwv2.DevWorkspace) (*solvers.ServiceConflictError, error) {
 	discoverableEndpoints := map[string]bool{}
 	for _, component := range workspace.Spec.Template.Components {
 		if component.Container != nil {
@@ -103,12 +106,12 @@ func (h *WebhookHandler) validateEndpoints(ctx context.Context, workspace *dwv2.
 	}
 
 	if len(discoverableEndpoints) == 0 {
-		return nil
+		return nil, nil
 	}
 
 	workspaceList := &dwv2.DevWorkspaceList{}
 	if err := h.Client.List(ctx, workspaceList, client.InNamespace(workspace.Namespace)); err != nil {
-		return err
+		return nil, err
 	}
 
 	for _, otherWorkspace := range workspaceList.Items {
@@ -122,12 +125,12 @@ func (h *WebhookHandler) validateEndpoints(ctx context.Context, workspace *dwv2.
 						return &solvers.ServiceConflictError{
 							EndpointName:  endpoint.Name,
 							WorkspaceName: otherWorkspace.Name,
-						}
+						}, nil
 					}
 				}
 			}
 		}
 	}
 
-	return nil
+	return nil, nil
 }

webhook/workspace/handler/validate_test.go

@@ -19,15 +19,15 @@ import (
 	"path/filepath"
 	"testing"
 
-	dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
-	"github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/solvers"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/yaml"
+
+	dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 )
 
 func loadObjectFromFile(objName string, obj client.Object, filename string) error {
@@ -67,13 +67,11 @@ func TestValidateEndpoints(t *testing.T) {
 		// Test for conflict in same namespace
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(otherWorkspaceSameNS).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.Error(t, err, "Expected a conflict error for workspaces in the same namespace")
-
-		var conflictErr *solvers.ServiceConflictError
-		assert.ErrorAs(t, err, &conflictErr, "Error should be a ServiceConflictError")
-		assert.Equal(t, "test-endpoint", conflictErr.EndpointName, "Conflict should be on 'test-endpoint'")
-		assert.Equal(t, "workspace-2", conflictErr.WorkspaceName, "Conflict should reference 'workspace-2'")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err, "Did not expect an infrastructure error")
+		assert.NotNil(t, conflict, "Expected a conflict for workspaces in the same namespace")
+		assert.Equal(t, "test-endpoint", conflict.EndpointName, "Conflict should be on 'test-endpoint'")
+		assert.Equal(t, "workspace-2", conflict.WorkspaceName, "Conflict should reference 'workspace-2'")
 	})
 
 	t.Run("No conflict in different namespace", func(t *testing.T) {
@@ -86,8 +84,9 @@ func TestValidateEndpoints(t *testing.T) {
 		// Test no conflict in different namespace (workspace only queries its own namespace)
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(otherWorkspaceDiffNS).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.NoError(t, err, "Did not expect an error for workspaces in different namespaces")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err)
+		assert.Nil(t, conflict, "Did not expect a conflict for workspaces in different namespaces")
 	})
 
 	t.Run("No conflict when endpoint name is different", func(t *testing.T) {
@@ -98,8 +97,9 @@ func TestValidateEndpoints(t *testing.T) {
 
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(otherWorkspace).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.NoError(t, err, "Did not expect an error for different endpoint names")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err)
+		assert.Nil(t, conflict, "Did not expect a conflict for different endpoint names")
 	})
 
 	t.Run("Conflict detected even when workspace is being deleted", func(t *testing.T) {
@@ -114,13 +114,11 @@ func TestValidateEndpoints(t *testing.T) {
 
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(deletingWorkspace).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.Error(t, err, "Should detect conflict even with workspace being deleted")
-
-		var conflictErr *solvers.ServiceConflictError
-		assert.ErrorAs(t, err, &conflictErr, "Error should be a ServiceConflictError")
-		assert.Equal(t, "test-endpoint", conflictErr.EndpointName, "Conflict should be on 'test-endpoint'")
-		assert.Equal(t, "workspace-deleting", conflictErr.WorkspaceName, "Conflict should reference 'workspace-deleting'")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err, "Did not expect an infrastructure error")
+		assert.NotNil(t, conflict, "Should detect conflict even with workspace being deleted")
+		assert.Equal(t, "test-endpoint", conflict.EndpointName, "Conflict should be on 'test-endpoint'")
+		assert.Equal(t, "workspace-deleting", conflict.WorkspaceName, "Conflict should reference 'workspace-deleting'")
 	})
 
 	t.Run("No conflict when workspace has no discoverable endpoints", func(t *testing.T) {
@@ -132,8 +130,9 @@ func TestValidateEndpoints(t *testing.T) {
 
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(otherWorkspace).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.NoError(t, err, "Did not expect an error when workspace has no discoverable endpoints")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err)
+		assert.Nil(t, conflict, "Did not expect a conflict when workspace has no discoverable endpoints")
 	})
 
 	t.Run("No conflict when other workspace endpoint is not discoverable", func(t *testing.T) {
@@ -146,8 +145,9 @@ func TestValidateEndpoints(t *testing.T) {
 
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(otherWorkspace).Build()
 		handler := &WebhookHandler{Client: fakeClient}
-		err := handler.validateEndpoints(context.TODO(), workspace)
-		assert.NoError(t, err, "Should not conflict when other workspace's endpoint is not discoverable")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace)
+		assert.NoError(t, err)
+		assert.Nil(t, conflict, "Should not conflict when other workspace's endpoint is not discoverable")
 	})
 
 	t.Run("Multiple workspaces in different namespaces can have same endpoint", func(t *testing.T) {
@@ -166,7 +166,8 @@ func TestValidateEndpoints(t *testing.T) {
 		handler := &WebhookHandler{Client: fakeClient}
 
 		// Validating workspace1 should succeed (different namespaces)
-		err := handler.validateEndpoints(context.TODO(), workspace1)
-		assert.NoError(t, err, "Should allow same endpoint name in different namespaces")
+		conflict, err := handler.validateEndpoints(context.TODO(), workspace1)
+		assert.NoError(t, err)
+		assert.Nil(t, conflict, "Should allow same endpoint name in different namespaces")
 	})
 }