fix: prevent discoverable endpoint Service conflicts

[akurinnoy]

What does this PR do?

Prevents discoverable endpoints from overwriting Services owned by another workspace, including endpoints whose names normalize to the same Service name.

• Admission uses a native cache index to check eligible endpoint names within the request namespace. Boolean/string discoverability, exposure rules, namespace fallback, unchanged-name updates and removal-only updates are covered.
• Routing checks current Service ownership and protects updates and cleanup with object identity guards. Concurrent admissions can both pass before caches converge; reconciliation remains authoritative and preserves foreign Services. Duplicate-name diagnostics are deterministic.
• Cluster-TLS routing validates each certificate Secret against its synchronized Service before publishing mounts. Missing certificates keep routing pending; foreign Secrets stay unchanged and cannot become generated workspace mounts. Each discoverable Service has its own certificate mount.
• Failed routing can recover after a spec change through the new optional status.observedGeneration field. Failures remain terminal within a generation. Install updated CRDs before the updated controller so this field is retained.
• Reconciliation cancellation reaches solver lookups and Service synchronization. External Go callers and RoutingSolver implementations must update the context-bearing GetSpecObjects signature.

RBAC: the webhook ServiceAccount gains cluster-wide DevWorkspace get/list/watch through its existing ClusterRoleBinding. This permits reading full workspace specs, including literal environment values and repository URLs, across namespaces. Conflict decisions remain namespace-scoped. The index reuses these permissions and adds no further RBAC.

The ADRs record ownership, certificate naming/readiness, indexing, generation recovery and context decisions. Load-test tooling and detailed campaign reports are preserved on a local branch for a later separate PR. No load-testing files are included in this PR.

What issues does this PR fix or reference?

Fixes eclipse-che/che#23231

Separate follow-ups: the backup CronJob nil-Workspace configuration panic predates this PR; its production fix is out of scope and the e2e workaround is documented. A stricter public endpoint-alias case reproduced an existing normalized Route/raw-name URL lookup mismatch. Its expected behavior needs team discussion. Neither issue has been filed yet, and neither production fix is included here.

Is it tested? How?

Use a cluster running this PR's controller and webhook image, with its updated CRDs installed first. The steps below use oc and disposable namespaces; certificate checks require OpenShift service-ca. Run the command blocks in the same shell. Commands marked as expected denials should fail admission.

Same-namespace conflicts and normalized names

Save this as workspace.yaml:

apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: endpoint-test-1
spec:
  started: true
  routingClass: basic
  template:
    attributes:
      controller.devfile.io/storage-type: ephemeral
    components:
      - name: database
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql-db
              targetPort: 5432
              exposure: internal
              protocol: tcp
              attributes:
                discoverable: true
          env:
            - name: POSTGRESQL_USER
              value: user
            - name: POSTGRESQL_PASSWORD
              value: pass
            - name: POSTGRESQL_ROOT_PASSWORD
              value: root
            - name: POSTGRESQL_DATABASE
              value: db

Create the first workspace and wait for readiness before trying a second:

review_ns="dwo-endpoints-$(date +%s)"
review_ns_other="${review_ns}-other"
oc create namespace "$review_ns"
oc create namespace "$review_ns_other"
oc create -n "$review_ns" -f workspace.yaml
oc wait --for=condition=Ready dw/endpoint-test-1 -n "$review_ns" --timeout=300s
sed 's/name: endpoint-test-1/name: endpoint-test-2/' workspace.yaml | oc create -n "$review_ns" -f -

Expected: the second create is rejected with discoverable endpoint 'postgresql-db' is already in use by workspace 'endpoint-test-1'. Repeat with a differently spelled endpoint that normalizes to the same Service name:

sed -e 's/name: endpoint-test-1/name: endpoint-test-2/' -e 's/name: postgresql-db/name: postgresql--db/' workspace.yaml | oc create -n "$review_ns" -f -

Expected: the same conflict denial; neither attempt creates endpoint-test-2.

Namespace isolation and discoverability updates

The same endpoint must work in another namespace:

oc create -n "$review_ns_other" -f workspace.yaml
oc wait --for=condition=Ready dw/endpoint-test-1 -n "$review_ns_other" --timeout=300s
oc get svc postgresql-db -n "$review_ns"
oc get svc postgresql-db -n "$review_ns_other"

Create a stopped, non-discoverable workspace in the first namespace, then try enabling discoverability with both encodings:

sed -e 's/name: endpoint-test-1/name: endpoint-test-2/' -e 's/started: true/started: false/' -e 's/discoverable: true/discoverable: false/' workspace.yaml | oc create -n "$review_ns" -f -
oc patch dw endpoint-test-2 -n "$review_ns" --type=json -p='[{"op":"replace","path":"/spec/template/components/0/container/endpoints/0/attributes/discoverable","value":true}]'
oc patch dw endpoint-test-2 -n "$review_ns" --type=json -p='[{"op":"replace","path":"/spec/template/components/0/container/endpoints/0/attributes/discoverable","value":"true"}]'

Expected: creation succeeds; both updates are denied with the same conflict reason. The stopped workspace creates no workspace pod.

An unchanged endpoint-name set and a discoverability removal must remain admissible. These probes use server-side dry-run to preserve the first workspace:

oc patch dw endpoint-test-1 -n "$review_ns" --dry-run=server --type=json -p='[{"op":"replace","path":"/spec/template/components/0/container/env/1/value","value":"changed"}]'
oc patch dw endpoint-test-1 -n "$review_ns" --dry-run=server --type=json -p='[{"op":"remove","path":"/spec/template/components/0/container/endpoints/0/attributes/discoverable"}]'

Expected: both succeed without persisting changes.

Foreign Service protection and routing recovery

Use a standalone routing object to exercise reconciliation independently of admission and workspace startup. Create a foreign Service and save its contents:

oc create service clusterip review-foreign --tcp=8080:8080 -n "$review_ns"
oc get svc review-foreign -n "$review_ns" -o yaml > foreign-service-before.yaml
oc create -n "$review_ns" -f - <<'YAML'
apiVersion: controller.devfile.io/v1alpha1
kind: DevWorkspaceRouting
metadata:
  name: review-routing
spec:
  devworkspaceId: review-routing
  routingClass: cluster
  podSelector:
    devworkspace_id: review-routing
  endpoints:
    database:
      - name: review-foreign
        targetPort: 8080
        exposure: internal
        protocol: tcp
        attributes:
          discoverable: true
YAML
oc wait -n "$review_ns" dwr/review-routing --for=jsonpath='{.status.phase}'=Failed --timeout=120s
oc get dwr review-routing -n "$review_ns" -o yaml

Expected: routing fails with an ownership conflict and status.observedGeneration matches metadata.generation. Rename the endpoint on the same routing object:

oc patch dwr review-routing -n "$review_ns" --type=json -p='[{"op":"replace","path":"/spec/endpoints/database/0/name","value":"review-recovered"}]'
oc wait -n "$review_ns" dwr/review-routing --for=jsonpath='{.status.phase}'=Ready --timeout=120s
oc get dwr review-routing -n "$review_ns" -o yaml
oc get svc review-foreign -n "$review_ns" -o yaml > foreign-service-after.yaml
diff -u foreign-service-before.yaml foreign-service-after.yaml

Expected: the routing keeps its UID, advances generation, reaches Ready with matching observed generation, and creates review-recovered. The foreign Service remains identical. Cluster routing creates no public Route/Ingress.

Per-Service certificates and foreign Secrets (OpenShift)

Switch the recovered routing to TLS:

oc patch dwr review-routing -n "$review_ns" --type=merge -p='{"spec":{"routingClass":"cluster-tls"}}'
routing_generation=$(oc get dwr review-routing -n "$review_ns" -o jsonpath='{.metadata.generation}')
oc wait -n "$review_ns" dwr/review-routing --for=jsonpath='{.status.observedGeneration}'="$routing_generation" --timeout=120s
oc wait -n "$review_ns" dwr/review-routing --for=jsonpath='{.status.phase}'=Ready --timeout=120s
oc get dwr review-routing -n "$review_ns" -o yaml
oc get svc,secret -n "$review_ns"

Expected: after service-ca issues certificates, status.podAdditions contains the aggregate mount at /var/serving-cert/ and a distinct discoverable mount at /var/serving-cert/review-recovered/. Each certificate Secret's originating Service name/UID annotations and Service owner reference match its Service. Volumes project only tls.crt and tls.key.

Check that an unrelated Secret cannot become a workspace mount:

oc create secret generic review-unrelated -n "$review_ns" --from-literal=marker=keep-me
oc get secret review-unrelated -n "$review_ns" -o yaml > foreign-secret-before.yaml
oc patch dwr review-routing -n "$review_ns" --type=json -p='[{"op":"replace","path":"/spec/endpoints/database/0/name","value":"review-unrelated"}]'
oc wait -n "$review_ns" dwr/review-routing --for=jsonpath='{.status.phase}'=Failed --timeout=120s
oc get dwr review-routing -n "$review_ns" -o yaml
oc get secret review-unrelated -n "$review_ns" -o yaml > foreign-secret-after.yaml
diff -u foreign-secret-before.yaml foreign-secret-after.yaml

Expected: routing fails certificate validation, publishes no Pod additions, and leaves the Secret unchanged.

Cleanup

Delete both namespaces and wait until namespace finalization removes all workspace, routing, Service and storage fixtures:

oc delete namespace "$review_ns" "$review_ns_other" --wait=true --timeout=300s
oc get namespace "$review_ns" "$review_ns_other"

Expected: both namespaces are NotFound. This fixture requests ephemeral workspace storage.

PR Checklist

Local e2e results are separate from the named upstream CI jobs.

• Matching upstream v8-devworkspace-operator-e2e result — pending review-time CI.
• v8-che-happy-path / Che integration verification — explicitly deferred.

[rohanKanojia]

I tested the PR with abovementioned steps and it seems to work as expected ✔️

[akurinnoy]

/retest

[dkwon17]

@akurinnoy is there a reason for implementing duplicate endpiont detection in both the webhook and the GetDiscoverableServicesForEndpoints function?

[akurinnoy]

While the webhook prevents the DevWorkspaces (which will cause endpoint conflicts) from being created, the controller checks other cases:

• two workspaces created simultaneously - both of them may pass the webhook validation
• services can be created from other sources
• old workspaces, created before this validation

Is this the answer to your question?

[rohanKanojia]

Btw, while looking at this method, I noticed it’s also used in che-operator here:

che_routing.go

objs.Services = solvers.GetDiscoverableServicesForEndpoints(routing.Spec.Endpoints, workspaceMeta)

Just wanted to give a heads-up that we’ll likely need to update there as well.

[rohanKanojia]

@akurinnoy : I ran your claude skill /ok-pr-review and it revealed this finding. Do you think it's a valid use case?

The webhook validation has a logic error that causes it to reject valid workspace creations. Currently, if the current workspace has a discoverable endpoint named X, and another workspace has any endpoint (discoverable OR NOT) named X, we reject it.

Steps to reproduce

• Create workspace A with a non-discoverable endpoint named "postgresql"

oc create -f - <<EOF
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: workspace-a
  namespace: test-namespace
spec:
  started: true
  routingClass: basic
  template:
    components:
      - name: postgresql
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql
              targetPort: 5432
              exposure: internal
              protocol: tcp
EOF

• Create workspace B with a discoverable endpoint named "postgresql" (gets rejected)

oc create -f - <<EOF
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: workspace-b
  namespace: test-namespace
spec:
  started: true
  routingClass: basic
  template:
    components:
      - name: postgresql
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql
              targetPort: 5432
              exposure: internal
              protocol: tcp
              attributes:
                discoverable: true
EOF

Do you think Workspace B should be allowed because Workspace A's endpoint is NOT discoverable and doesn't create a service named postgresql?

[tolusha]

Hi! I'm che-ai-assistant — I help with your pull requests.

I check for new commands every 5m0s (if I am not busy :) ).

Available commands:

• /che-ai-assistant generate-che-doc — Generate a documentation PR based on this PR's changes
• /che-ai-assistant ok-pr-review — Run a comprehensive PR review (summary, code review, deep review, impact analysis)
• /che-ai-assistant help — Show this help message

[coderabbitai]

📝 Walkthrough

This PR adds ownership-aware discoverable service handling: it introduces ServiceConflictError, refactors solvers to accept a Kubernetes client and logger and to perform in-cluster checks for existing Services, integrates conflict handling into the routing controller and admission webhook, and provides tests, RBAC, and test-suite config.

Changes

Discoverable Endpoint Conflict Detection

Layer / File(s)	Summary
Error types and solver interface contracts controllers/controller/devworkspacerouting/solvers/errors.go, controllers/controller/devworkspacerouting/solvers/solver.go	ServiceConflictError added with EndpointName and WorkspaceName. RoutingSolverGetter.GetSolver updated to accept logr.Logger.
Solver dependency injection: client and logger controllers/controller/devworkspacerouting/solvers/basic_solver.go, controllers/controller/devworkspacerouting/solvers/cluster_solver.go	BasicSolver and ClusterSolver now store client.Client and logr.Logger; constructors NewBasicSolver and NewClusterSolver added; SolverGetter instantiates solvers via constructors.
Discoverable endpoint conflict detection in solvers controllers/controller/devworkspacerouting/solvers/common.go, controllers/controller/devworkspacerouting/solvers/basic_solver.go, controllers/controller/devworkspacerouting/solvers/cluster_solver.go	DevWorkspaceMetadata gains DevWorkspaceName. GetDiscoverableServicesForEndpoints now accepts client.Client and logr.Logger, returns ([]corev1.Service, error), performs Get lookups for deterministic service names, detects ownership mismatches by DevWorkspaceIDLabel, and returns ServiceConflictError on conflict. Solvers call this function and propagate errors.
Controller reconciliation: conflict handling and error reporting controllers/controller/devworkspacerouting/devworkspacerouting_controller.go	Controller passes reqLogger into GetSolver, includes DevWorkspaceName in workspace metadata, and explicitly handles *solvers.ServiceConflictError by logging and marking DevWorkspaceRouting as RoutingFailed.
Webhook admission validation for endpoint conflicts webhook/workspace/handler/validate.go	ValidateDevfile invokes validateEndpoints, which collects discoverable endpoint names from the incoming DevWorkspace, lists DevWorkspaces in the same namespace, and returns solvers.ServiceConflictError on the first matching endpoint name found in another workspace; conflicts are aggregated into admission denial.
Tests, fixtures, RBAC and test config controllers/controller/devworkspacerouting/solvers/common_test.go, webhook/workspace/handler/validate_test.go, webhook/workspace/handler/testdata/test-devworkspace.yaml, pkg/webhook/cluster_roles.go, controllers/workspace/suite_test.go, .gitignore	Adds unit tests covering discoverable service generation and conflict scenarios, webhook validateEndpoints tests, a test DevWorkspace manifest with a discoverable endpoint, RBAC rule allowing webhook to get,list,watch devworkspaces, disables metrics server in test manager, and ignores .claude/settings.local.json.

Sequence Diagram(s)

sequenceDiagram
  participant Webhook as Admission Webhook
  participant Controller as Routing Controller
  participant Solver as DevWorkspace Solver
  participant Client as Kubernetes API
  
  Webhook->>Webhook: validateEndpoints: collect discoverable endpoint names
  Webhook->>Client: list DevWorkspaces in namespace
  Client-->>Webhook: return workspace list
  Webhook->>Webhook: compare endpoint names across workspaces
  alt Conflict detected
    Webhook-->>Webhook: return ServiceConflictError
  else No conflict
    Webhook-->>Webhook: allow admission
  end
  
  Controller->>Solver: GetSolver with logger
  Solver->>Solver: NewBasicSolver/NewClusterSolver with client+logger
  Controller->>Solver: GetSpecObjects
  Solver->>Solver: GetDiscoverableServicesForEndpoints
  Solver->>Client: Get existing Service by name
  Client-->>Solver: Service or not-found
  alt Service exists and belongs to different workspace
    Solver-->>Solver: return ServiceConflictError
  else Service ok or not found
    Solver-->>Solver: create/update Service with ownership labels
  end
  alt Error returned
    Controller->>Controller: markRoutingFailed with conflict message
  else Success
    Controller->>Controller: continue reconciliation
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks: 3 · 1 · 1

Failed checks: 1 warning · 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check		While most changes align with #23231, a potential logic issue was identified: validateEndpoints may incorrectly reject discoverable endpoints when a non-discoverable endpoint with the same name exists…	Verify that validateEndpoints only checks for conflicts with discoverable endpoints and allows non-discoverable endpoints to coexist with discoverable ones having the same name.

Passed checks: 3 passed

Check name	Status	Explanation
Linked Issues check		The PR implements conflict detection and prevention for identically named endpoints at both controller and webhook levels, directly addressing the controller loop issue described in #23231.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preventing Service conflicts for discoverable endpoints.

Full details: Out of Scope Changes check

Explanation

While most changes align with #23231, a potential logic issue was identified: validateEndpoints may incorrectly reject discoverable endpoints when a non-discoverable endpoint with the same name exists in another workspace.

Resolution

Verify that validateEndpoints only checks for conflicts with discoverable endpoints and allows non-discoverable endpoints to coexist with discoverable ones having the same name.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@controllers/controller/devworkspacerouting/devworkspacerouting_controller.go`:
- Around line 128-133: The code sets workspaceMeta.DevWorkspaceName =
instance.Name but must use the parent DevWorkspace's name from
instance.OwnerReferences; update the DevWorkspaceMetadata population so
DevWorkspaceName is assigned from the owner reference whose Kind (or
APIVersion+Kind) indicates the parent DevWorkspace (e.g., iterate
instance.OwnerReferences, find ownerRef.Kind == "DevWorkspace" and use
ownerRef.Name), and fall back to instance.Name only if no such ownerRef exists;
change the assignment near workspaceMeta (DevWorkspaceMetadata,
workspaceMeta.DevWorkspaceName) to use that ownerRef-derived name.

In `@controllers/controller/devworkspacerouting/solvers/common_test.go`:
- Around line 16-27: Reorder the import block in common_test.go into three
groups with blank lines between them: (1) standard library (e.g., "testing"),
(2) third-party and Kubernetes packages (e.g.,
"github.com/stretchr/testify/assert", "k8s.io/api/core/v1" as corev1,
"k8s.io/apimachinery/pkg/apis/meta/v1" as metav1,
"k8s.io/apimachinery/pkg/runtime",
"sigs.k8s.io/controller-runtime/pkg/client/fake",
"sigs.k8s.io/controller-runtime/pkg/log/zap"), and (3) project-local imports
(e.g., "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" as
controllerv1alpha1 and
"github.com/devfile/devworkspace-operator/pkg/constants"); insert single blank
lines between these groups and then run 'make fmt' to apply formatting.

In `@webhook/workspace/handler/validate_test.go`:
- Around line 105-124: The test currently asserts a ServiceConflictError when
validateEndpoints (on WebhookHandler) is called with a workspace and a
deletingWorkspace (created via setupWorkspace) that has DeletionTimestamp set;
update the test so it reflects webhook behavior that ignores
deletion-timestamped workspaces: call handler.validateEndpoints(context.TODO(),
workspace) and assert no error (i.e., expect nil) instead of asserting
Error/ErrorAs for *solvers.ServiceConflictError, and remove or replace the
subsequent checks for conflictErr.EndpointName/WorkspaceName accordingly so the
test verifies that deletion-timestamped workspaces do not cause conflicts.
- Around line 16-31: Reorder the import block in the validate_test.go file into
three blank-line-separated groups: (1) standard library imports (context, os,
path/filepath, testing), (2) third-party and Kubernetes imports
(github.com/stretchr/testify/assert, sigs.k8s.io/... , k8s.io/... etc.), and (3)
project-local imports (github.com/devfile/... and your controller packages),
making sure each package stays in its respective group; after rearranging, run
make fmt to apply formatting.

In `@webhook/workspace/handler/validate.go`:
- Around line 81-84: validateEndpoints currently returns raw errors from
h.Client.List which are being appended to devfileErrors and treated as user
validation failures in ValidateDevfile; change validateEndpoints signature to
return (*solvers.ServiceConflictError, error) so real validation conflicts come
back as the typed value and infrastructure/API errors flow via the error return,
update validateEndpoints to return (nil, err) for h.Client.List failures and
(conflictErr, nil) for validation failures, then in ValidateDevfile (and the
other call site around lines 109-112) check the error return first and convert
infrastructure errors into admission.Errored(http.StatusBadRequest, err) (or
propagate the error) instead of appending to devfileErrors, only append
conflictErr.Error() to devfileErrors when the typed conflict value is returned.
- Around line 114-130: The validation loop currently only skips the same
workspace by UID but still inspects workspaces that are being deleted; modify
the loop that iterates workspaceList.Items (and the check around otherWorkspace)
to also skip any otherWorkspace with a non-nil DeletionTimestamp (i.e., treat
terminating workspaces as excluded from discoverable endpoint conflict checks),
so that discoverableEndpoints lookup and potential return of
solvers.ServiceConflictError only considers non-terminating workspaces.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 981c002a-3f92-488e-8077-9057055e9194

📥 Commits

Reviewing files that changed from the base of the PR and between e0ed8d2 and 7a5ff10.

📒 Files selected for processing (12)

• controllers/controller/devworkspacerouting/devworkspacerouting_controller.go
• controllers/controller/devworkspacerouting/solvers/basic_solver.go
• controllers/controller/devworkspacerouting/solvers/cluster_solver.go
• controllers/controller/devworkspacerouting/solvers/common.go
• controllers/controller/devworkspacerouting/solvers/common_test.go
• controllers/controller/devworkspacerouting/solvers/errors.go
• controllers/controller/devworkspacerouting/solvers/solver.go
• controllers/workspace/suite_test.go
• pkg/webhook/cluster_roles.go
• webhook/workspace/handler/testdata/test-devworkspace.yaml
• webhook/workspace/handler/validate.go
• webhook/workspace/handler/validate_test.go

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@webhook/workspace/handler/validate_test.go`:
- Line 1: Update the copyright header in
webhook/workspace/handler/validate_test.go by changing the year range string "//
Copyright (c) 2019-2025 Red Hat, Inc." to the current-year range "// Copyright
(c) 2019-2026 Red Hat, Inc."; locate and replace that exact header line at the
top of the file so it matches the required format used across Go sources.
- Line 58: The test currently ignores the error from dwv2.AddToScheme(scheme);
change this to explicitly assert success by calling assert.NoError(t,
dwv2.AddToScheme(scheme)) so the scheme initialization failure surfaces in the
test; locate the call to dwv2.AddToScheme in validate_test.go (the variable
scheme) and replace the `_ =` ignoring pattern with the assert.NoError check
(using the existing testify/assert import).

In `@webhook/workspace/handler/validate.go`:
- Around line 24-29: Reorder the import block in validate.go so imports follow
Go grouping: first any standard-library packages (none shown here), then a blank
line, then third-party and Kubernetes imports together (e.g.,
"github.com/devfile/api/v2/..." packages and
"sigs.k8s.io/controller-runtime/..." client and admission), then a blank line,
and finally project-local imports (e.g.,
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" and
"github.com/devfile/devworkspace-operator/controllers/controller/devworkspacerouting/solvers");
ensure there are blank lines separating these three groups and keep the existing
import identifiers (dwv2, devfilevalidation, v1alpha1, solvers, client,
admission) intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13233f4a-6817-4e7f-a346-91250ddebbe0

📥 Commits

Reviewing files that changed from the base of the PR and between 7a5ff10 and d899dfb.

📒 Files selected for processing (4)

• .gitignore
• controllers/controller/devworkspacerouting/solvers/common_test.go
• webhook/workspace/handler/validate.go
• webhook/workspace/handler/validate_test.go

✅ Files skipped from review due to trivial changes (1)

• .gitignore

🚧 Files skipped from review as they are similar to previous changes (1)

• controllers/controller/devworkspacerouting/solvers/common_test.go

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, rohanKanojia, tolusha
Once this PR has been reviewed and has the lgtm label, please assign dkwon17 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tolusha]

/che-ai-assistant ok-pr-readiness

Review is complete. Please check the review comments below.

[tolusha]

PR Readiness Assessment: PR #1521 — fix: identical endpoint name conflicts

Repository: devfile/devworkspace-operator
Linked Issue: eclipse-che/che#23231 — Dev Workspace Controller Loop with Identical Endpoint Names on DevWorkspace

---

#	Criterion	Verdict	Notes
1	Problem Statement	PASS	PR body and linked issue clearly describe the bug: workspaces with identical discoverable endpoint names cause controller to loop and workspaces get stuck on "Preparing Service"
2	Reproduction Steps	PASS	Linked issue provides sample devfile and clear steps. PR body includes comprehensive test scenarios with exact commands and YAML manifests for both same-namespace conflict and cross-namespace isolation
3	Expected Behavior After Fix	PASS	Clear acceptance criteria: same-namespace conflicts should be rejected by webhook with specific error message; cross-namespace workspaces with same endpoint names should run successfully
4	Scope of Changes	PASS	13 files changed with focused scope: controller conflict detection, webhook validation, and comprehensive test coverage. Changes are cohesive and well-explained
5	Test Evidence	PASS	Extensive manual testing instructions with exact commands and expected outputs. New unit tests added in common_test.go and validate_test.go. CodeRabbit confirms extensive test coverage
6	Deployment & Verification Notes	PASS	Detailed verification steps with specific oc/kubectl commands, expected webhook denial messages, and service verification across namespaces. E2E test checklist included

Overall: READY

---

Missing Information

None — this PR is exceptionally well-documented.

What's Good

• Comprehensive test plan: Includes two distinct test scenarios (same-namespace conflict detection and cross-namespace isolation) with complete step-by-step commands
• Clear problem statement: Both PR description and linked issue explain the controller loop bug with screenshots and environment details
• Well-defined expected behavior: Explicit acceptance criteria for both conflict and non-conflict cases, including exact error messages
• Excellent reproducibility: Sample devfiles, exact YAML manifests, and verification commands provided
• Appropriate scope: Changes are focused on conflict detection logic, webhook validation, and corresponding tests
• Strong test coverage: Both unit tests (added in the diff) and detailed manual testing instructions
• Cross-referenced documentation: Links to external issue for full historical context

[openshift-ci]

New changes are detected. LGTM label has been removed.

[codecov]

Codecov Report

❌ Patch coverage is 91.38462% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.68%. Comparing base (3526ad3) to head (413b56f).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/webhook/cluster_roles.go	0.00%	12 Missing ⚠️
pkg/common/naming.go	77.27%	5 Missing ⚠️
pkg/provision/sync/service.go	76.47%	4 Missing ⚠️
...workspacerouting/devworkspacerouting_controller.go	96.61%	1 Missing and 1 partial ⚠️
...roller/devworkspacerouting/serving_certificates.go	95.55%	1 Missing and 1 partial ⚠️
...s/controller/devworkspacerouting/solvers/solver.go	50.00%	2 Missing ⚠️
pkg/provision/sync/sync.go	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1521      +/-   ##
==========================================
+ Coverage   40.45%   44.68%   +4.23%     
==========================================
  Files         174      179       +5     
  Lines       15949    13408    -2541     
==========================================
- Hits         6452     5992     -460     
+ Misses       9096     6963    -2133     
- Partials      401      453      +52     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.