chore: Configure DWI with tlsCertificateConfigmapRef when certificate…

[tolusha]

…s imported to che-operator

What does this PR do?

This PR configures the DevWorkspace Operator to use Che's merged CA bundle for TLS by setting tlsCertificateConfigmapRef in the DWI routing config when custom certificates are imported, and adding a watch label so DWO picks up changes. It also refactors the TLS certificate reconciliation to use newer client APIs, adds owner references, removes deprecated sync helpers, and splits a large test file into domain-specific ones.

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

eclipse-che/che#23870

How to test this PR?

1. Deploy the operator:
   chectl server:deploy -p openshift --che-operator-image quay.io/abazko/che-operator:next

2. Create k8s resources to host a parent devfile:

apiVersion: v1
kind: Namespace
metadata:
  name: parent
---
apiVersion: v1
kind: Pod
metadata:
  name: server
  namespace: parent
  labels:
    app: file-server-app
spec:
  securityContext:
    runAsNonRoot: true
    seccompProfile:
      type: RuntimeDefault
  containers:
    - name: httpd
      image: 'quay.io/abazko/operator:parent-devfile'
      ports:
        - containerPort: 8000
      securityContext:
        allowPrivilegeEscalation: false
        capabilities:
          drop:
            - ALL
---
apiVersion: v1
kind: Service
metadata:
  name: file-server
  namespace: parent
spec:
  selector:
    app: file-server-app
  ports:
    - protocol: TCP
      port: 80
      targetPort: 8000
---
apiVersion: route.openshift.io/v1
kind: Route
metadata:
  name: parent
  namespace: parent
spec:
  to:
    name: file-server
    kind: Service
  tls:
    insecureEdgeTerminationPolicy: Redirect
    termination: edge
  port:
    targetPort: 8000

3. Ensure parent devfile is accessible by url
   https://parent-parent.<openshift-base-domain>/devfile.yaml (for instance https://parent-parent.apps-crc.testing/devfile.yaml)

4. Import certificate into Che

openssl s_client -connect parent-parent.apps-crc.testing:443 -showcerts < /dev/null | sed -n '/-----BEGIN CERTIFICATE-----/,/-----END CERTIFICATE-----/p' > parent.crt

kubectl create configmap custom-ca-certificates \
    --from-file=parent.crt \
    --namespace=eclipse-che

kubectl label configmap custom-ca-certificates \
    app.kubernetes.io/component=ca-bundle \
    app.kubernetes.io/part-of=che.eclipse.org  \
    --namespace=eclipse-che

5. Check that DWO is configured
   oc get dwoc -n eclipse-che devworkspace-config -o jsonpath="{.config.routing}"

6. Start a workspace from https://gist.githubusercontent.com/tolusha/6c68d907a52d235b311f196478ce08b6/raw/d94a38042093d9a5fe021091d861cb9ea15ac78e/devfile.yaml

Common Test Scenarios

• Deploy Eclipse Che
• Start an empty workspace
• Open terminal and build/run an image
• Stop a workspace
• Check operator logs for reconciliation errors or infinite reconciliation loops

PR Checklist

As the author of this Pull Request I made sure that:

• The Eclipse Contributor Agreement is valid
• Code produced is complete
• Code builds without errors
• Tests are covering the bugfix
• The repository devfile is up to date and works
• Relevant user documentation updated
• Relevant contributing documentation updated
• CI/CD changes implemented, documented and communicated

Reviewers

Reviewers, please comment how you tested the PR when approving it.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tolusha

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]

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

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

[tolusha]

/che-ai-assistant ok-pr-review

Review is complete. Please check the review comments below.

[tolusha]

Comprehensive Review Summary

I've completed a thorough multi-dimensional review of this PR:

• ✅ Standard Review: Approve - ready to merge
• ✅ Deep Review: Design is sound
• ✅ Impact Review: No system-level concerns

Key Strengths

Excellent test coverage: 8 test cases for the new TLS ConfigMap ref feature covering all edge cases (creation, update, removal, empty data, idempotency). The composition tests for proxy + TLS settings show attention to integration concerns.

Clean test file organization: Breaking the 3328-line monolithic test into 6 domain-specific files (routing, scheduling, security, service account, storage, workspace) greatly improves maintainability.

Proper resource lifecycle: Owner references ensure ConfigMaps are garbage-collected when CheCluster is deleted. The deliberate exclusion of admin-created resources (git trusted certs) shows understanding of K8s ownership semantics.

Type-safe DWO integration: The controller.devfile.io/watch-configmap=true label and TLSCertificateConfigmapRef use imported constants from the DevWorkspace Operator module - no magic strings.

Consistent refactoring: Migration from deprecated deploy.Get() to ClientWrapper.GetIgnoreNotFound() is applied uniformly across all call sites.

Minor Suggestions

1. Test coverage gap: The updateTLSCertificateConfigmapRef error path (when GetIgnoreNotFound fails) isn't exercised in tests. Consider adding a test case that simulates a client error when fetching the ConfigMap.

2. Diff pattern documentation: In syncCheCABundleCerts (certificates.go:455), the shift to using GetLabelsAndAnnotations(mergedCABundlesCM) for diff keys is clever but creates an implicit dependency on label ordering. Consider adding a comment documenting that labels must be set on the blueprint before this call.

3. Optional helper extraction: The pattern of SetControllerReference + ClientWrapper.Sync + return err == nil, err appears in 5 methods. A SyncOwned(ctx, owner, blueprint, opts) helper could reduce boilerplate, though the current code is correct.

Verdict

✅ Approve - This PR is well-designed, thoroughly tested, and ready to merge. The TLS certificate integration is correctly implemented and the refactoring away from deprecated helpers is clean and consistent.

---

📊 Review completed: Standard + Deep + Impact analysis

[tolusha]

/che-ai-assistant generate-che-doc

Created documentation PR: eclipse-che/che-docs#3128

[codecov]

Codecov Report

❌ Patch coverage is 71.11111% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.12%. Comparing base (669c6f7) to head (dabeede).
⚠️ Report is 37 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/deploy/tls/certificates.go	78.26%	16 Missing and 4 partials ⚠️
pkg/deploy/labels.go	0.00%	10 Missing ⚠️
pkg/deploy/devworkspace/dev_workspace_config.go	80.00%	4 Missing and 2 partials ⚠️
pkg/common/diffs/diffs.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2137      +/-   ##
==========================================
+ Coverage   50.09%   50.12%   +0.02%     
==========================================
  Files          94      101       +7     
  Lines       12569    13234     +665     
==========================================
+ Hits         6297     6633     +336     
- Misses       5853     6134     +281     
- Partials      419      467      +48     

☔ 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.

[rohanKanojia]

I notice in TestReconcileDevWorkspaceContainerSecurityContext and TestReconcileDevWorkspacePodSecurityContext test cases define existedObjects, but the test context is created with

deployContext := test.NewCtxBuilder().WithCheCluster(testCase.cheCluster).Build()

Should deployContext include existedObjects so that updated paths are also tested?

[rohanKanojia]

Nit (optional): testCases := []testCase{} could be used here since the type is already apparent from the composite literal, though this is mostly a style preference.

[rohanKanojia]

Nit: Could we use descriptive test names instead of Case #1, Case #2, etc.? It makes failures much easier to interpret from the test output without opening the test file.

Suggested change

	name: "Case #1",
	name: "ServiceAccount configured with auto-provision enabled",