Skip to content

feat: Delegate workspace NetworkPolicy provisioning to DevWorkspace Operator - #2226

Closed
tolusha wants to merge 0 commit into
mainfrom
np
Closed

tolusha wants to merge 0 commit into
mainfrom
np

Conversation

@tolusha

@tolusha tolusha commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Moves workspace NetworkPolicy provisioning from Che Operator to the DevWorkspace Operator, and reworks the NetworkPolicies Che Operator creates for its own components.

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

How to test this PR?

  1. Deploy the operator:

OpenShift

oc create namespace devworkspace-controller
oc apply --server-side -f https://raw.githubusercontent.com/devfile/devworkspace-operator/refs/heads/main/deploy/deployment/openshift/combined.yaml

OPERATOR_IMAGE=<...>
sed 's|quay.io/eclipse/che-operator:next|'${OPERATOR_IMAGE}'|g' deploy/deployment/openshift/combined.yaml | oc apply --server-side -f  -
oc wait --for=condition=Ready pod -l app.kubernetes.io/component=che-operator -n eclipse-che --timeout=60s
oc apply --server-side -f deploy/deployment/openshift/org_v2_checluster.yaml
oc wait checluster eclipse-che -n eclipse-che --for=jsonpath='.status.chePhase'=Active   --timeout=120s

on Minikube

./build/scripts/minikube-tests/test-operator-from-sources.sh

TODO

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:

Reviewers

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

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1a0d8320-f1c4-48fe-bab9-53823c181fc5
📥 Commits

Reviewing files that changed from the base of the PR and between 8141b56 and 50c95fc.

⛔ Files ignored due to path filters: 1
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing: 47
  • api/v2/checluster_types.go
  • api/v2/zz_generated.deepcopy.go
  • bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml
  • bundle/next/eclipse-che/manifests/org.eclipse.che_checlusters.yaml
  • config/crd/bases/org.eclipse.che_checlusters.yaml
  • config/rbac/cluster_role.yaml
  • controllers/che/checluster_controller.go
  • controllers/usernamespace/usernamespace_controller.go
  • controllers/usernamespace/usernamespace_controller_network_policies_test.go
  • deploy/deployment/kubernetes/combined.yaml
  • deploy/deployment/kubernetes/objects/che-operator.ClusterRole.yaml
  • deploy/deployment/kubernetes/objects/checlusters.org.eclipse.che.CustomResourceDefinition.yaml
  • deploy/deployment/openshift/combined.yaml
  • deploy/deployment/openshift/objects/che-operator.ClusterRole.yaml
  • deploy/deployment/openshift/objects/checlusters.org.eclipse.che.CustomResourceDefinition.yaml
  • go.mod
  • helmcharts/next/crds/checlusters.org.eclipse.che.CustomResourceDefinition.yaml
  • helmcharts/next/templates/che-operator.ClusterRole.yaml
  • pkg/common/chelabels/labels.go
  • pkg/common/constants/constants.go
  • pkg/common/k8s-client/k8s_client.go
  • pkg/common/k8s-client/k8s_client_types.go
  • pkg/common/test/utils.go
  • pkg/common/utils/slices.go
  • pkg/deploy/devworkspace/dev_workspace_config.go
  • pkg/deploy/devworkspace/dev_workspace_config_network_policy.go
  • pkg/deploy/devworkspace/dev_workspace_config_network_policy_test.go
  • pkg/deploy/devworkspace/dev_workspace_config_scheduling_test.go
  • pkg/deploy/devworkspace/dev_workspace_config_security_test.go
  • pkg/deploy/devworkspace/dev_workspace_config_storage_test.go
  • pkg/deploy/devworkspace/dev_workspace_config_workspace_test.go
  • pkg/deploy/devworkspace/init_test.go
  • pkg/deploy/networkpolicies/init_test.go
  • pkg/deploy/networkpolicies/networkpolicies.go
  • pkg/deploy/networkpolicies/networkpolicies_test.go
  • pkg/provision/helper.go
  • pkg/provision/helper_test.go
  • pkg/provision/networkpolicy.go
  • pkg/provision/suite_test.go
  • pkg/reconciler/networkpolicy/networkpolicy_kubernetes.go
  • pkg/reconciler/networkpolicy/networkpolicy_openshift.go
  • pkg/reconciler/networkpolicy/networkpolicy_reconciler.go
  • pkg/reconciler/networkpolicy/networkpolicy_reconciler_test.go
  • pkg/reconciler/networkpolicy/suite_test.go
  • vendor/github.com/devfile/devworkspace-operator/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go
  • vendor/github.com/devfile/devworkspace-operator/pkg/config/defaults.go
  • vendor/modules.txt
💤 Files with no reviewable changes: 5
  • controllers/usernamespace/usernamespace_controller_network_policies_test.go
  • pkg/deploy/networkpolicies/init_test.go
  • pkg/provision/suite_test.go
  • pkg/deploy/networkpolicies/networkpolicies_test.go
  • pkg/deploy/networkpolicies/networkpolicies.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The change separates network-policy handling for Che operands and DevWorkspaces. Che policies are reconciled in the CheCluster namespace on Kubernetes and OpenShift. DevWorkspace policies are configured through the CheCluster spec, and legacy workspace-namespace policies are deleted on OpenShift.

Changes: Network Policy Management

Layer / File(s) Summary
DevWorkspace policy configuration
api/v2/checluster_types.go, api/v2/zz_generated.deepcopy.go, pkg/deploy/devworkspace/*, vendor/github.com/devfile/devworkspace-operator/*, go.mod, vendor/modules.txt
The API adds a DevWorkspace network-policy field. Workspace configuration applies default ingress and egress rules when their configured lists are nil. The vendored operator changes policy serialization and returns copied default rules.
Che-namespace policy reconciliation
controllers/che/checluster_controller.go, pkg/reconciler/networkpolicy/*, pkg/common/constants/constants.go, pkg/common/utils/slices.go, pkg/common/test/utils.go
The Che controller registers a platform-aware network-policy reconciler. The reconciler builds and syncs policies when enabled, or deletes namespace policies when disabled. Tests cover Kubernetes and OpenShift policy sets and reconciliation.
Managed policy deletion and legacy cleanup
pkg/provision/*, pkg/common/k8s-client/*, pkg/common/chelabels/labels.go, controllers/usernamespace/*, pkg/deploy/networkpolicies/*, config/rbac/cluster_role.yaml, deploy/deployment/*/objects/che-operator.ClusterRole.yaml, helmcharts/next/templates/che-operator.ClusterRole.yaml, bundle/next/eclipse-che/manifests/che-operator.clusterserviceversion.yaml
Provisioning adds helpers to delete managed NetworkPolicies. On OpenShift, the user-namespace controller deletes legacy workspace policies and no longer watches NetworkPolicy events. RBAC adds deletecollection permission; the CSV package and version metadata also change.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~50 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant CheClusterController
  participant NetworkPoliciesReconciler
  participant PlatformPolicyBuilder
  participant KubernetesClient
  CheClusterController->>NetworkPoliciesReconciler: Reconcile CheCluster
  NetworkPoliciesReconciler->>PlatformPolicyBuilder: Build policies for platform
  PlatformPolicyBuilder-->>NetworkPoliciesReconciler: Return policy objects
  NetworkPoliciesReconciler->>KubernetesClient: Sync policies when enabled
  NetworkPoliciesReconciler->>KubernetesClient: Delete namespace policies when disabled
Loading
Merge Risk: ⚪ Minimal · up to 50c95

No actionable issue remains from this review. The network-policy change is mergeable after normal checks.

Security architecture risk: 🟠 High · up to 50c95

Existing installations can lose workspace network isolation during upgrade because the previous enabled setting does not enable its replacement. On Kubernetes, enabling Che policies also creates allow-all rules that can override administrators’ restrictive NetworkPolicies.

Retained concerns

  • High · security · inferred: An OpenShift installation using only the legacy spec.networking.networkPolicy.enabled=true setting can lose workspace ingress isolation on upgrade. Workspace namespace reconciliation deletes the legacy managed policies, but the replacement configuration reads only the new DevEnvironment setting and leaves DWO provisioning disabled by default. Cleanup also does not wait for replacement policies: even explicitly enabled replacements may be delayed until each existing workspace reconciles. Without independent protective policies, previously excluded network peers can reach workspace endpoints; successful deletion has no compensating restoration in this cleanup path.
  • High · security · inferred: Enabling Che network policies on Kubernetes now installs allow-all ingress and egress policies for Che-labeled pods. Standard NetworkPolicy permissions are additive, so administrators cannot restore restrictions by adding narrower policies as the API guidance suggests. Compared with the base, where this reconciler was registered only on OpenShift, existing Kubernetes restrictions can become ineffective for the selected Che services. The regression requires the setting to be enabled and remains subject to independent network controls.
Security review details

Security Blast Radius

  • inferred — The migration regression can affect every reconciled OpenShift workspace namespace relying on legacy managed policies. The Kubernetes regression affects Che-labeled services in the CheCluster namespace when the setting is enabled. Neither establishes unrestricted access to all cluster resources; actual reachability depends on routing, independent controls, and remaining policies.

Security Findings and Attack Paths

  • inferred — Following a normal upgrade and successful legacy cleanup, a network peer previously excluded by workspace ingress policies may reach workspace listeners if no independent isolation remains. The peer need not modify CheCluster configuration: the administrative upgrade can remove the control. Endpoint exploitation and data disclosure are not established.
  • inferred — On Kubernetes, an enabled setting adds permission for arbitrary ingress sources and egress destinations at the standard NetworkPolicy layer. Network peers can therefore attempt connections previously blocked by narrower policies, while compromised Che pods can gain broader outbound reachability. Adding another restrictive standard NetworkPolicy cannot subtract these permissions.

Trust Boundaries and Controls

  • observed — The default Che-to-workspace allowance combines an exact Che namespace selector with a Che pod-label selector. Custom ingress lists replace that allowance. Cleanup is restricted to classified workspace namespaces on OpenShift and managed-label matches, but these restrictions do not prove that replacement isolation exists.

Resilience and Maintainability Implications

  • inferred — Error propagation permits failed cleanup or configuration writes to retry, but does not preserve isolation after successful deletion. A replacement readiness condition spanning the two controllers is missing from the inspected path, so interruption and delayed workspace reconciliation can strand the weaker state.

Hardening Proposals

  • proposed — Preserve legacy enablement during migration and retain legacy protection until equivalent replacement policies are verified for affected workspaces, including recovery after interruption. For Kubernetes, avoid installing allow-all policies when administrators are expected to supply restrictive policies; require appropriate rules or leave policy ownership with administrators.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 28 files. (7 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: moving workspace NetworkPolicy provisioning to the DevWorkspace Operator.
Full details: Docstring Coverage

Explanation

Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 28 files. (7 skipped: 7 unsupported.)

✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@tolusha

tolusha commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

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

I check for new comments every 10m0s, so there may be a short delay before I respond.

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 ok-pr-readiness — Ensure PR has validation steps
  • /che-ai-assistant check-pr-test-failures — Analyze failing CI checks, identify root causes, and suggest fixes
  • /che-ai-assistant update-che-e2e-tests — Update Eclipse Che e2e tests
  • /che-ai-assistant claude — Run a free-form instruction on this PR
  • /che-ai-assistant help — Show this help message

@openshift-ci

openshift-ci Bot commented Oct 14, 2026

Copy link
Copy Markdown

[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:

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

@tolusha
tolusha deleted the np branch October 14, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant