From ed7697bc77c950ee8f27c621d54358a89ed8bf2d Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 12 Oct 2026 11:08:42 +0200 Subject: [PATCH 1/4] fix: do not mutate defaultConfig in GetDefaultNetworkPolicy Remove the assignment to defaultConfig.Workspace.NetworkPolicy from GetDefaultNetworkPolicy. GetDefaultNetworkPolicy is a public helper intended to return the default ingress/egress rules for network policies, and modifying the package-level defaultConfig was an unintended side effect. The default network policy configuration on defaultConfig is already initialized by setDefaultNetworkPolicy. Signed-off-by: Anatolii Bazko --- pkg/config/defaults.go | 5 ---- pkg/config/defaults_test.go | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 pkg/config/defaults_test.go diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 5ce3710bd..571eecfb8 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -269,10 +269,5 @@ func GetDefaultNetworkPolicy(operatorNamespace string) ( ingressPolicyRules = defaultKubernetesIngressPolicyRules } - defaultConfig.Workspace.NetworkPolicy = &v1alpha1.NetworkPolicyConfig{ - Enabled: pointer.Bool(constants.DefaultNetworkPolicyEnabled), - Ingress: ingressPolicyRules, - Egress: defaultEgressPolicyRules, - } return ingressPolicyRules, defaultEgressPolicyRules, nil } diff --git a/pkg/config/defaults_test.go b/pkg/config/defaults_test.go new file mode 100644 index 000000000..26f3883ce --- /dev/null +++ b/pkg/config/defaults_test.go @@ -0,0 +1,55 @@ +// Copyright (c) 2019-2026 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/devfile/devworkspace-operator/pkg/infrastructure" +) + +func TestGetDefaultNetworkPolicyDoesNotMutateDefaultConfig(t *testing.T) { + setupForTest(t) + + defaultConfig.Workspace.NetworkPolicy = nil + + _, _, err := GetDefaultNetworkPolicy("test-namespace") + assert.NoError(t, err) + + assert.Nil(t, defaultConfig.Workspace.NetworkPolicy, "GetDefaultNetworkPolicy should not mutate defaultConfig") +} + +func TestGetDefaultNetworkPolicy(t *testing.T) { + t.Run("on Kubernetes", func(t *testing.T) { + setupForTest(t) + infrastructure.InitializeForTesting(infrastructure.Kubernetes) + + ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") + assert.NoError(t, err) + assert.Equal(t, defaultKubernetesIngressPolicyRules, ingress) + assert.Equal(t, defaultEgressPolicyRules, egress) + }) + + t.Run("on OpenShift", func(t *testing.T) { + setupForTest(t) + infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) + + ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") + assert.NoError(t, err) + assert.Len(t, ingress, len(defaultOpenShiftIngressPolicyRules)+1) + assert.Equal(t, defaultEgressPolicyRules, egress) + }) +} From 4e3eccc559987c385c808821d4db8cead0d025da Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 12 Oct 2026 14:00:39 +0200 Subject: [PATCH 2/4] fix: preserve empty ingress/egress lists in NetworkPolicyConfig Drop `omitempty` from the Ingress and Egress JSON tags of NetworkPolicyConfig. An empty list has a distinct meaning for these fields: it denies all ingress/egress traffic, while an unset field falls back to the default rules. With `omitempty`, an empty list was dropped during serialization, so a deny-all configuration silently turned into the default allow rules. Add a test that resolves an external DevWorkspaceOperatorConfig with empty ingress and egress lists and verifies they stay empty. Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Anatolii Bazko --- .../devworkspaceoperatorconfig_types.go | 4 +- pkg/config/sync_test.go | 66 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 0aeba956c..890300213 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -351,14 +351,14 @@ type NetworkPolicyConfig struct { // is denied. If this field is specified as a non-empty list, exactly those rules apply // and the default rules no longer apply. // +kubebuilder:validation:Optional - Ingress []networkingv1.NetworkPolicyIngressRule `json:"ingress,omitempty"` + Ingress []networkingv1.NetworkPolicyIngressRule `json:"ingress"` // Egress defines the egress rules applied to DevWorkspace pods. If this field is not // specified, the default egress rule of the DevWorkspace Operator applies, which allows // all egress traffic. If this field is specified as an empty list, all egress traffic // from DevWorkspace pods is denied. If this field is specified as a non-empty list, // exactly those rules apply and the default rule no longer applies. // +kubebuilder:validation:Optional - Egress []networkingv1.NetworkPolicyEgressRule `json:"egress,omitempty"` + Egress []networkingv1.NetworkPolicyEgressRule `json:"egress"` } type Proxy struct { diff --git a/pkg/config/sync_test.go b/pkg/config/sync_test.go index 05199936a..6fb85e53f 100644 --- a/pkg/config/sync_test.go +++ b/pkg/config/sync_test.go @@ -28,6 +28,7 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -615,6 +616,71 @@ func TestMergeConfigMergesRuntimeClassName(t *testing.T) { } } +func TestResolveConfigForWorkspaceWithNetworkPolicy(t *testing.T) { + setupForTest(t) + + workspace := &dw.DevWorkspace{ + Spec: dw.DevWorkspaceSpec{ + Template: dw.DevWorkspaceTemplateSpec{ + DevWorkspaceTemplateSpecContent: dw.DevWorkspaceTemplateSpecContent{ + Attributes: attributes.Attributes{}, + }, + }, + }, + } + + attrs := workspace.Spec.Template.DevWorkspaceTemplateSpecContent.Attributes + attrs.Put( + constants.ExternalDevWorkspaceConfiguration, + types.NamespacedName{ + Name: externalConfigName, + Namespace: externalConfigNamespace, + }, + nil, + ) + + t.Run("external config overrides NetworkPolicy with empty ingress and egress (deny all)", func(t *testing.T) { + externalDwoc := &v1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: externalConfigName, + Namespace: externalConfigNamespace, + }, + Config: &v1alpha1.OperatorConfiguration{ + Workspace: &v1alpha1.WorkspaceConfig{ + NetworkPolicy: &v1alpha1.NetworkPolicyConfig{ + Enabled: pointer.Bool(true), + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + Egress: []networkingv1.NetworkPolicyEgressRule{}, + }, + }, + }, + } + + dwoc := &v1alpha1.DevWorkspaceOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: OperatorConfigName, + Namespace: configNamespace, + }, + Config: defaultConfig.DeepCopy(), + } + + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(dwoc, externalDwoc).Build() + err := SetupControllerConfig(client) + assert.NoError(t, err) + + resolvedConfig, err := ResolveConfigForWorkspace(workspace, client) + assert.NoError(t, err) + assert.NotNil(t, resolvedConfig.Workspace.NetworkPolicy) + assert.Equal(t, pointer.Bool(true), resolvedConfig.Workspace.NetworkPolicy.Enabled) + + assert.NotNil(t, resolvedConfig.Workspace.NetworkPolicy.Ingress) + assert.Empty(t, resolvedConfig.Workspace.NetworkPolicy.Ingress) + + assert.NotNil(t, resolvedConfig.Workspace.NetworkPolicy.Egress) + assert.Empty(t, resolvedConfig.Workspace.NetworkPolicy.Egress) + }) +} + func fuzzQuantity(q *resource.Quantity, c fuzz.Continue) { q.Set(c.Int63n(999)) q.Format = resource.DecimalSI From 896ea7148266d9b4e83721219a4bdcf31f52f714 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 12 Oct 2026 15:12:44 +0200 Subject: [PATCH 3/4] fix: return copies of default rules from GetDefaultNetworkPolicy GetDefaultNetworkPolicy returned the package-level default rule slices directly. On OpenShift the appended slice had a fresh backing array, but its elements still shared LabelSelector pointers and MatchLabels maps with the defaults. A caller extending the returned rules (the function is public for operators such as che-operator) could therefore corrupt the operator's defaults for the rest of the process lifetime. Deep-copy the default ingress and egress rules before returning them. Tighten the tests: assert literal expected rules on Kubernetes instead of comparing against the returned global, check that the OpenShift rule targets the given operator namespace, and verify that mutating the returned rules leaves the defaults unchanged. Assisted-by: Claude Code Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Anatolii Bazko --- pkg/config/defaults.go | 25 +++++++++++++++- pkg/config/defaults_test.go | 58 +++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/pkg/config/defaults.go b/pkg/config/defaults.go index 571eecfb8..7a071892c 100644 --- a/pkg/config/defaults.go +++ b/pkg/config/defaults.go @@ -269,5 +269,28 @@ func GetDefaultNetworkPolicy(operatorNamespace string) ( ingressPolicyRules = defaultKubernetesIngressPolicyRules } - return ingressPolicyRules, defaultEgressPolicyRules, nil + // Return copies so callers can extend the rules without mutating package-level defaults + return deepCopyIngressRules(ingressPolicyRules), deepCopyEgressRules(defaultEgressPolicyRules), nil +} + +func deepCopyIngressRules(rules []networkingv1.NetworkPolicyIngressRule) []networkingv1.NetworkPolicyIngressRule { + if rules == nil { + return nil + } + copied := make([]networkingv1.NetworkPolicyIngressRule, len(rules)) + for i := range rules { + rules[i].DeepCopyInto(&copied[i]) + } + return copied +} + +func deepCopyEgressRules(rules []networkingv1.NetworkPolicyEgressRule) []networkingv1.NetworkPolicyEgressRule { + if rules == nil { + return nil + } + copied := make([]networkingv1.NetworkPolicyEgressRule, len(rules)) + for i := range rules { + rules[i].DeepCopyInto(&copied[i]) + } + return copied } diff --git a/pkg/config/defaults_test.go b/pkg/config/defaults_test.go index 26f3883ce..e5b4256d5 100644 --- a/pkg/config/defaults_test.go +++ b/pkg/config/defaults_test.go @@ -17,6 +17,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/devfile/devworkspace-operator/pkg/infrastructure" ) @@ -39,8 +41,8 @@ func TestGetDefaultNetworkPolicy(t *testing.T) { ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") assert.NoError(t, err) - assert.Equal(t, defaultKubernetesIngressPolicyRules, ingress) - assert.Equal(t, defaultEgressPolicyRules, egress) + assert.Equal(t, []networkingv1.NetworkPolicyIngressRule{{}}, ingress) + assert.Equal(t, []networkingv1.NetworkPolicyEgressRule{{}}, egress) }) t.Run("on OpenShift", func(t *testing.T) { @@ -50,6 +52,56 @@ func TestGetDefaultNetworkPolicy(t *testing.T) { ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") assert.NoError(t, err) assert.Len(t, ingress, len(defaultOpenShiftIngressPolicyRules)+1) - assert.Equal(t, defaultEgressPolicyRules, egress) + assert.Equal(t, defaultOpenShiftIngressPolicyRules, ingress[1:]) + assert.Equal(t, []networkingv1.NetworkPolicyPeer{ + { + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"kubernetes.io/metadata.name": "test-namespace"}, + }, + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app.kubernetes.io/part-of": "devworkspace-operator"}, + }, + }, + }, ingress[0].From) + assert.Equal(t, []networkingv1.NetworkPolicyEgressRule{{}}, egress) }) } + +func TestGetDefaultNetworkPolicyReturnsCopies(t *testing.T) { + tests := map[string]infrastructure.Type{ + "on Kubernetes": infrastructure.Kubernetes, + "on OpenShift": infrastructure.OpenShiftv4, + } + for name, infraType := range tests { + t.Run(name, func(t *testing.T) { + setupForTest(t) + infrastructure.InitializeForTesting(infraType) + + origKubernetesIngress := (&networkingv1.NetworkPolicySpec{Ingress: defaultKubernetesIngressPolicyRules}).DeepCopy().Ingress + origOpenShiftIngress := (&networkingv1.NetworkPolicySpec{Ingress: defaultOpenShiftIngressPolicyRules}).DeepCopy().Ingress + origEgress := (&networkingv1.NetworkPolicySpec{Egress: defaultEgressPolicyRules}).DeepCopy().Egress + + ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") + assert.NoError(t, err) + + for i := range ingress { + for _, peer := range ingress[i].From { + if peer.NamespaceSelector != nil { + peer.NamespaceSelector.MatchLabels["mutated"] = "true" + } + if peer.PodSelector != nil { + peer.PodSelector.MatchLabels["mutated"] = "true" + } + } + ingress[i].From = append(ingress[i].From, networkingv1.NetworkPolicyPeer{}) + } + for i := range egress { + egress[i].To = append(egress[i].To, networkingv1.NetworkPolicyPeer{}) + } + + assert.Equal(t, origKubernetesIngress, defaultKubernetesIngressPolicyRules) + assert.Equal(t, origOpenShiftIngress, defaultOpenShiftIngressPolicyRules) + assert.Equal(t, origEgress, defaultEgressPolicyRules) + }) + } +} From 28b68807b4e602b08b50234f59f800ebaafa274e Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 12 Oct 2026 15:16:12 +0200 Subject: [PATCH 4/4] remove defaults_test.go Signed-off-by: Anatolii Bazko --- pkg/config/defaults_test.go | 107 ------------------------------------ 1 file changed, 107 deletions(-) delete mode 100644 pkg/config/defaults_test.go diff --git a/pkg/config/defaults_test.go b/pkg/config/defaults_test.go deleted file mode 100644 index e5b4256d5..000000000 --- a/pkg/config/defaults_test.go +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright (c) 2019-2026 Red Hat, Inc. -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package config - -import ( - "testing" - - "github.com/stretchr/testify/assert" - networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/devfile/devworkspace-operator/pkg/infrastructure" -) - -func TestGetDefaultNetworkPolicyDoesNotMutateDefaultConfig(t *testing.T) { - setupForTest(t) - - defaultConfig.Workspace.NetworkPolicy = nil - - _, _, err := GetDefaultNetworkPolicy("test-namespace") - assert.NoError(t, err) - - assert.Nil(t, defaultConfig.Workspace.NetworkPolicy, "GetDefaultNetworkPolicy should not mutate defaultConfig") -} - -func TestGetDefaultNetworkPolicy(t *testing.T) { - t.Run("on Kubernetes", func(t *testing.T) { - setupForTest(t) - infrastructure.InitializeForTesting(infrastructure.Kubernetes) - - ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") - assert.NoError(t, err) - assert.Equal(t, []networkingv1.NetworkPolicyIngressRule{{}}, ingress) - assert.Equal(t, []networkingv1.NetworkPolicyEgressRule{{}}, egress) - }) - - t.Run("on OpenShift", func(t *testing.T) { - setupForTest(t) - infrastructure.InitializeForTesting(infrastructure.OpenShiftv4) - - ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") - assert.NoError(t, err) - assert.Len(t, ingress, len(defaultOpenShiftIngressPolicyRules)+1) - assert.Equal(t, defaultOpenShiftIngressPolicyRules, ingress[1:]) - assert.Equal(t, []networkingv1.NetworkPolicyPeer{ - { - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"kubernetes.io/metadata.name": "test-namespace"}, - }, - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"app.kubernetes.io/part-of": "devworkspace-operator"}, - }, - }, - }, ingress[0].From) - assert.Equal(t, []networkingv1.NetworkPolicyEgressRule{{}}, egress) - }) -} - -func TestGetDefaultNetworkPolicyReturnsCopies(t *testing.T) { - tests := map[string]infrastructure.Type{ - "on Kubernetes": infrastructure.Kubernetes, - "on OpenShift": infrastructure.OpenShiftv4, - } - for name, infraType := range tests { - t.Run(name, func(t *testing.T) { - setupForTest(t) - infrastructure.InitializeForTesting(infraType) - - origKubernetesIngress := (&networkingv1.NetworkPolicySpec{Ingress: defaultKubernetesIngressPolicyRules}).DeepCopy().Ingress - origOpenShiftIngress := (&networkingv1.NetworkPolicySpec{Ingress: defaultOpenShiftIngressPolicyRules}).DeepCopy().Ingress - origEgress := (&networkingv1.NetworkPolicySpec{Egress: defaultEgressPolicyRules}).DeepCopy().Egress - - ingress, egress, err := GetDefaultNetworkPolicy("test-namespace") - assert.NoError(t, err) - - for i := range ingress { - for _, peer := range ingress[i].From { - if peer.NamespaceSelector != nil { - peer.NamespaceSelector.MatchLabels["mutated"] = "true" - } - if peer.PodSelector != nil { - peer.PodSelector.MatchLabels["mutated"] = "true" - } - } - ingress[i].From = append(ingress[i].From, networkingv1.NetworkPolicyPeer{}) - } - for i := range egress { - egress[i].To = append(egress[i].To, networkingv1.NetworkPolicyPeer{}) - } - - assert.Equal(t, origKubernetesIngress, defaultKubernetesIngressPolicyRules) - assert.Equal(t, origOpenShiftIngress, defaultOpenShiftIngressPolicyRules) - assert.Equal(t, origEgress, defaultEgressPolicyRules) - }) - } -}