From ee17801a21ab5727fbfa321f2d87486c3d50dc25 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Sat, 6 Jun 2026 14:17:26 +0530 Subject: [PATCH 1/5] Files Created: - manifests/networkpolicy/00-default-deny.yaml: Default deny-all policy selecting all pods with no ingress/egress rules. - manifests/networkpolicy/01-allow-machine-config-operator.yaml: Allow policy for MCO (egress all, ingress TCP 9001 metrics). - manifests/networkpolicy/02-allow-machine-config-controller.yaml: Allow policy for MCC (egress all, ingress TCP 9001 metrics). - manifests/networkpolicy/03-allow-machine-os-builder.yaml: Conditional allow policy for MOB (egress all, ingress TCP 9001 metrics). - pkg/operator/network_policy_test.go: Added 7 unit tests covering all lifecycle scenarios. Files Modified: - pkg/operator/sync.go: Added manifest path constants and syncNetworkPolicies() with retry logic and conditional MOB lifecycle management. - pkg/operator/operator.go: Added networkPolicyInformerSynced field, registered informer for drift detection, and wired it into the sync chain. - cmd/machine-config-operator/start.go: Passed NetworkPolicies informer factory to operator.New(). Test Results (7 new tests passing, existing suite green): - TestSyncNetworkPolicies_StaticPoliciesCreated: Verifies 3 static policies created, MOB absent. - TestSyncNetworkPolicies_DefaultDenySpec: Verifies empty podSelector, both policyTypes, no rules. - TestSyncNetworkPolicies_AllowPolicySpecs: Verifies podSelector labels, egress allow-all, ingress metrics port 9001. - TestSyncNetworkPolicies_MOBPolicyWithLayeredPools: Verifies MOB policy created when layered pools exist. - TestSyncNetworkPolicies_MOBPolicyDeletedWithoutLayeredPools: Verifies MOB cleanup when pools are removed. - TestSyncNetworkPolicies_DriftProtection: Verifies manual modifications are reverted on resync. - TestSyncNetworkPolicies_Idempotent: Verifies stable policy count across multiple syncs. Signed-off-by: HarshwardhanPatil07 --- cmd/machine-config-operator/start.go | 1 + manifests/networkpolicy/00-default-deny.yaml | 21 ++ .../01-allow-machine-config-operator.yaml | 28 ++ .../02-allow-machine-config-controller.yaml | 28 ++ .../03-allow-machine-os-builder.yaml | 29 +++ pkg/operator/network_policy_test.go | 240 ++++++++++++++++++ pkg/operator/operator.go | 7 + pkg/operator/sync.go | 54 ++++ 8 files changed, 408 insertions(+) create mode 100644 manifests/networkpolicy/00-default-deny.yaml create mode 100644 manifests/networkpolicy/01-allow-machine-config-operator.yaml create mode 100644 manifests/networkpolicy/02-allow-machine-config-controller.yaml create mode 100644 manifests/networkpolicy/03-allow-machine-os-builder.yaml create mode 100644 pkg/operator/network_policy_test.go diff --git a/cmd/machine-config-operator/start.go b/cmd/machine-config-operator/start.go index bdf9a5adb5..b9ea06f953 100644 --- a/cmd/machine-config-operator/start.go +++ b/cmd/machine-config-operator/start.go @@ -120,6 +120,7 @@ func runStartCmd(_ *cobra.Command, _ []string) { ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(), ctrlctx.InformerFactory.Machineconfiguration().V1().OSImageStreams(), iriInformer, + ctrlctx.KubeNamespacedInformerFactory.Networking().V1().NetworkPolicies(), ctrlctx, ) diff --git a/manifests/networkpolicy/00-default-deny.yaml b/manifests/networkpolicy/00-default-deny.yaml new file mode 100644 index 0000000000..5940762022 --- /dev/null +++ b/manifests/networkpolicy/00-default-deny.yaml @@ -0,0 +1,21 @@ +# Default-deny policy for the openshift-machine-config-operator namespace. +# This policy selects all pods in the namespace and enables default-deny for both +# ingress and egress by specifying policyTypes without any allow rules. +# +# NetworkPolicies are additive (use OR logic): +# - This policy enables default-deny for all pods +# - Subsequent policies add specific allow rules +# - If any policy allows traffic, that traffic is permitted +# +# Note: machine-config-daemon and machine-config-server use hostNetwork: true +# and bypass NetworkPolicy entirely. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: default-deny + namespace: {{.TargetNamespace}} +spec: + podSelector: {} + policyTypes: + - Ingress + - Egress diff --git a/manifests/networkpolicy/01-allow-machine-config-operator.yaml b/manifests/networkpolicy/01-allow-machine-config-operator.yaml new file mode 100644 index 0000000000..acf296e7be --- /dev/null +++ b/manifests/networkpolicy/01-allow-machine-config-operator.yaml @@ -0,0 +1,28 @@ +# Network policy for the machine-config-operator pod. +# +# Egress: +# - Allow all egress to support communication with the Kubernetes API server, +# whose IP address and port are not known at manifest time. This implicitly +# covers DNS resolution as well. +# +# Ingress: +# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. +# The metrics endpoint uses kube-rbac-proxy for authentication. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: allow-machine-config-operator + namespace: {{.TargetNamespace}} +spec: + podSelector: + matchLabels: + k8s-app: machine-config-operator + ingress: + - ports: + - protocol: TCP + port: 9001 + egress: + - {} + policyTypes: + - Ingress + - Egress diff --git a/manifests/networkpolicy/02-allow-machine-config-controller.yaml b/manifests/networkpolicy/02-allow-machine-config-controller.yaml new file mode 100644 index 0000000000..134d08bac4 --- /dev/null +++ b/manifests/networkpolicy/02-allow-machine-config-controller.yaml @@ -0,0 +1,28 @@ +# Network policy for the machine-config-controller pod. +# +# Egress: +# - Allow all egress to support communication with the Kubernetes API server, +# whose IP address and port are not known at manifest time. This implicitly +# covers DNS resolution as well. +# +# Ingress: +# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. +# The metrics endpoint uses kube-rbac-proxy for authentication. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: allow-machine-config-controller + namespace: {{.TargetNamespace}} +spec: + podSelector: + matchLabels: + k8s-app: machine-config-controller + ingress: + - ports: + - protocol: TCP + port: 9001 + egress: + - {} + policyTypes: + - Ingress + - Egress diff --git a/manifests/networkpolicy/03-allow-machine-os-builder.yaml b/manifests/networkpolicy/03-allow-machine-os-builder.yaml new file mode 100644 index 0000000000..50cf58c68e --- /dev/null +++ b/manifests/networkpolicy/03-allow-machine-os-builder.yaml @@ -0,0 +1,29 @@ +# Network policy for the machine-os-builder pod. +# This policy is only applied when layered MachineConfigPools exist, +# and is deleted when no layered pools are present. +# +# Egress: +# - Allow all egress for API server communication, container registry access, +# and DNS resolution. All egress is permitted because destination addresses +# (registries, API server) vary by cluster configuration. +# +# Ingress: +# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: allow-machine-os-builder + namespace: {{.TargetNamespace}} +spec: + podSelector: + matchLabels: + k8s-app: machine-os-builder + ingress: + - ports: + - protocol: TCP + port: 9001 + egress: + - {} + policyTypes: + - Ingress + - Egress diff --git a/pkg/operator/network_policy_test.go b/pkg/operator/network_policy_test.go new file mode 100644 index 0000000000..a081dea8c8 --- /dev/null +++ b/pkg/operator/network_policy_test.go @@ -0,0 +1,240 @@ +package operator + +import ( + "context" + "testing" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + fakeclientmachineconfigv1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake" + mcfginformers "github.com/openshift/client-go/machineconfiguration/informers/externalversions" + "github.com/openshift/library-go/pkg/operator/events" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/utils/clock" +) + +func testRenderConfig() *renderConfig { + return &renderConfig{ + TargetNamespace: ctrlcommon.MCONamespace, + } +} + +func testOperatorForNetworkPolicies(t *testing.T, pools []*mcfgv1.MachineConfigPool) *Operator { + t.Helper() + + kubeClient := fake.NewSimpleClientset() + mcfgClient := fakeclientmachineconfigv1.NewSimpleClientset() + mcfgInformerFactory := mcfginformers.NewSharedInformerFactory(mcfgClient, 0) + mcpInformer := mcfgInformerFactory.Machineconfiguration().V1().MachineConfigPools() + moscInformer := mcfgInformerFactory.Machineconfiguration().V1().MachineOSConfigs() + + for _, pool := range pools { + require.NoError(t, mcpInformer.Informer().GetIndexer().Add(pool)) + } + + return &Operator{ + kubeClient: kubeClient, + mcpLister: mcpInformer.Lister(), + moscLister: moscInformer.Lister(), + libgoRecorder: events.NewInMemoryRecorder("test-operator", clock.RealClock{}), + } +} + +func layeredPool(name string) *mcfgv1.MachineConfigPool { + return &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + ctrlcommon.LayeringEnabledPoolLabel: "", + }, + }, + } +} + +func listNetworkPolicies(t *testing.T, optr *Operator) []networkingv1.NetworkPolicy { + t.Helper() + npList, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).List(context.TODO(), metav1.ListOptions{}) + require.NoError(t, err) + return npList.Items +} + +func findPolicy(policies []networkingv1.NetworkPolicy, name string) *networkingv1.NetworkPolicy { + for i := range policies { + if policies[i].Name == name { + return &policies[i] + } + } + return nil +} + +func TestSyncNetworkPolicies_StaticPoliciesCreated(t *testing.T) { + optr := testOperatorForNetworkPolicies(t, nil) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies := listNetworkPolicies(t, optr) + names := make([]string, len(policies)) + for i, p := range policies { + names[i] = p.Name + } + + assert.Contains(t, names, "default-deny", "expected default-deny policy") + assert.Contains(t, names, "allow-machine-config-operator", "expected allow-machine-config-operator policy") + assert.Contains(t, names, "allow-machine-config-controller", "expected allow-machine-config-controller policy") + assert.NotContains(t, names, "allow-machine-os-builder", "MOB policy should not be created without layered pools") +} + +func TestSyncNetworkPolicies_DefaultDenySpec(t *testing.T) { + optr := testOperatorForNetworkPolicies(t, nil) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies := listNetworkPolicies(t, optr) + denyPolicy := findPolicy(policies, "default-deny") + require.NotNil(t, denyPolicy, "default-deny policy must exist") + + assert.Empty(t, denyPolicy.Spec.PodSelector.MatchLabels, "default-deny should select all pods with empty selector") + assert.Contains(t, denyPolicy.Spec.PolicyTypes, networkingv1.PolicyTypeIngress, "default-deny must include Ingress policy type") + assert.Contains(t, denyPolicy.Spec.PolicyTypes, networkingv1.PolicyTypeEgress, "default-deny must include Egress policy type") + assert.Empty(t, denyPolicy.Spec.Ingress, "default-deny should have no ingress rules") + assert.Empty(t, denyPolicy.Spec.Egress, "default-deny should have no egress rules") +} + +func TestSyncNetworkPolicies_AllowPolicySpecs(t *testing.T) { + optr := testOperatorForNetworkPolicies(t, nil) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies := listNetworkPolicies(t, optr) + + cases := []struct { + name string + labelKey string + labelVal string + metricsPort int32 + }{ + {"allow-machine-config-operator", "k8s-app", "machine-config-operator", 9001}, + {"allow-machine-config-controller", "k8s-app", "machine-config-controller", 9001}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + policy := findPolicy(policies, tc.name) + require.NotNil(t, policy, "policy %s must exist", tc.name) + + assert.Equal(t, tc.labelVal, policy.Spec.PodSelector.MatchLabels[tc.labelKey], + "podSelector should match %s=%s", tc.labelKey, tc.labelVal) + assert.Contains(t, policy.Spec.PolicyTypes, networkingv1.PolicyTypeIngress) + assert.Contains(t, policy.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) + + require.Len(t, policy.Spec.Egress, 1, "should have one egress rule (allow all)") + assert.Empty(t, policy.Spec.Egress[0].Ports, "egress rule should allow all ports") + assert.Empty(t, policy.Spec.Egress[0].To, "egress rule should allow all destinations") + + require.Len(t, policy.Spec.Ingress, 1, "should have one ingress rule") + require.Len(t, policy.Spec.Ingress[0].Ports, 1, "ingress rule should have one port") + assert.Equal(t, tc.metricsPort, policy.Spec.Ingress[0].Ports[0].Port.IntVal, "ingress port should be metrics port") + assert.Empty(t, policy.Spec.Ingress[0].From, "ingress should not restrict source (kube-rbac-proxy handles auth)") + }) + } +} + +func TestSyncNetworkPolicies_MOBPolicyWithLayeredPools(t *testing.T) { + pools := []*mcfgv1.MachineConfigPool{layeredPool("layered-worker")} + optr := testOperatorForNetworkPolicies(t, pools) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies := listNetworkPolicies(t, optr) + mobPolicy := findPolicy(policies, "allow-machine-os-builder") + require.NotNil(t, mobPolicy, "MOB policy should be created when layered pools exist") + + assert.Equal(t, "machine-os-builder", mobPolicy.Spec.PodSelector.MatchLabels["k8s-app"]) + require.Len(t, mobPolicy.Spec.Egress, 1, "MOB should have one egress rule (allow all)") + require.Len(t, mobPolicy.Spec.Ingress, 1, "MOB should have one ingress rule") +} + +func TestSyncNetworkPolicies_MOBPolicyDeletedWithoutLayeredPools(t *testing.T) { + pools := []*mcfgv1.MachineConfigPool{layeredPool("layered-worker")} + optr := testOperatorForNetworkPolicies(t, pools) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies := listNetworkPolicies(t, optr) + require.NotNil(t, findPolicy(policies, "allow-machine-os-builder"), "MOB policy should exist initially") + + optr.mcpLister = testOperatorForNetworkPolicies(t, nil).mcpLister + + err = optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policies = listNetworkPolicies(t, optr) + assert.Nil(t, findPolicy(policies, "allow-machine-os-builder"), "MOB policy should be deleted when no layered pools exist") + assert.NotNil(t, findPolicy(policies, "default-deny"), "static policies should still exist") + assert.NotNil(t, findPolicy(policies, "allow-machine-config-operator"), "static policies should still exist") + assert.NotNil(t, findPolicy(policies, "allow-machine-config-controller"), "static policies should still exist") +} + +func TestSyncNetworkPolicies_DriftProtection(t *testing.T) { + optr := testOperatorForNetworkPolicies(t, nil) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + policy, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( + context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + + policy.Spec.Ingress = nil + _, err = optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Update( + context.TODO(), policy, metav1.UpdateOptions{}) + require.NoError(t, err) + + modified, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( + context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + assert.Empty(t, modified.Spec.Ingress, "ingress should be empty after manual modification") + + err = optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + + restored, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( + context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + require.Len(t, restored.Spec.Ingress, 1, "ingress rules should be restored after resync") +} + +func TestSyncNetworkPolicies_Idempotent(t *testing.T) { + optr := testOperatorForNetworkPolicies(t, nil) + config := testRenderConfig() + + err := optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + firstCount := len(listNetworkPolicies(t, optr)) + + err = optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + secondCount := len(listNetworkPolicies(t, optr)) + + err = optr.syncNetworkPolicies(config, nil) + require.NoError(t, err) + thirdCount := len(listNetworkPolicies(t, optr)) + + assert.Equal(t, firstCount, secondCount, "policy count should be stable across syncs") + assert.Equal(t, secondCount, thirdCount, "policy count should be stable across syncs") + assert.Equal(t, 3, thirdCount, "should have exactly 3 static policies without layered pools") +} diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index d512cbd8d3..389538a107 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" appsinformersv1 "k8s.io/client-go/informers/apps/v1" coreinformersv1 "k8s.io/client-go/informers/core/v1" + networkinginformersv1 "k8s.io/client-go/informers/networking/v1" rbacinformersv1 "k8s.io/client-go/informers/rbac/v1" "k8s.io/client-go/kubernetes" coreclientsetv1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -168,6 +169,7 @@ type Operator struct { apiserverListerSynced cache.InformerSynced osImageStreamListerSynced cache.InformerSynced iriListerSynced cache.InformerSynced + networkPolicyInformerSynced cache.InformerSynced // queue only ever has one item, but it has nice error handling backoff/retry semantics queue workqueue.TypedRateLimitingInterface[string] @@ -229,6 +231,7 @@ func New( clusterVersionInformer configinformersv1.ClusterVersionInformer, osImageStreamInformer mcfginformersv1.OSImageStreamInformer, iriInformer mcfginformersv1alpha1.InternalReleaseImageInformer, + networkPolicyInformer networkinginformersv1.NetworkPolicyInformer, ctrlctx *ctrlcommon.ControllerContext, ) *Operator { eventBroadcaster := record.NewBroadcaster() @@ -301,6 +304,7 @@ func New( clusterOperatorInformer.Informer(), apiserverInformer.Informer(), moscInformer.Informer(), + networkPolicyInformer.Informer(), } for _, i := range informers { i.AddEventHandler(optr.eventHandler()) @@ -377,6 +381,7 @@ func New( optr.apiserverListerSynced = apiserverInformer.Informer().HasSynced optr.moscLister = moscInformer.Lister() optr.moscListerSynced = moscInformer.Informer().HasSynced + optr.networkPolicyInformerSynced = networkPolicyInformer.Informer().HasSynced optr.clusterVersionLister = clusterVersionInformer.Lister() if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) { optr.osImageStreamLister = osImageStreamInformer.Lister() @@ -464,6 +469,7 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) { optr.crcListerSynced, optr.nodeClusterListerSynced, optr.moscListerSynced, + optr.networkPolicyInformerSynced, } if optr.osImageStreamListerSynced != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) { cacheSynced = append(cacheSynced, optr.osImageStreamListerSynced) @@ -618,6 +624,7 @@ func (optr *Operator) sync(key string) error { {"MachineConfiguration", optr.syncMachineConfiguration}, {"MachineConfigNode", optr.syncMachineConfigNodes}, {"MachineConfigPools", optr.syncMachineConfigPools}, + {"NetworkPolicies", optr.syncNetworkPolicies}, {"MachineConfigDaemon", optr.syncMachineConfigDaemon}, {"MachineConfigController", optr.syncMachineConfigController}, {"MachineConfigServer", optr.syncMachineConfigServer}, diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 5eeda92e0c..058b7f2160 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -154,6 +154,12 @@ const ( // Machine OS puller manifest paths mopRoleBindingManifestPath = "manifests/machine-os-puller/rolebinding.yaml" mopServiceAccountManifestPath = "manifests/machine-os-puller/sa.yaml" + + // NetworkPolicy manifest paths + npDefaultDenyPath = "manifests/networkpolicy/00-default-deny.yaml" + npAllowMCOPath = "manifests/networkpolicy/01-allow-machine-config-operator.yaml" + npAllowMCCPath = "manifests/networkpolicy/02-allow-machine-config-controller.yaml" + npAllowMOBPath = "manifests/networkpolicy/03-allow-machine-os-builder.yaml" ) type syncFunc struct { @@ -1631,6 +1637,54 @@ func (optr *Operator) getLayeredMachineConfigPools() ([]*mcfgv1.MachineConfigPoo return pools, nil } +func (optr *Operator) syncNetworkPolicies(config *renderConfig, _ *configv1.ClusterOperator) error { + return retry.OnError(retry.DefaultRetry, mcoResourceApply.IsApplyErrorRetriable, func() error { + npCache := resourceapply.NewResourceCache() + + staticPolicies := []string{ + npDefaultDenyPath, + npAllowMCOPath, + npAllowMCCPath, + } + for _, path := range staticPolicies { + npBytes, err := renderAsset(config, path) + if err != nil { + return err + } + np := resourceread.ReadNetworkPolicyV1OrDie(npBytes) + _, _, err = resourceapply.ApplyNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, np, npCache) + if err != nil { + return err + } + } + + layeredMCPs, err := optr.getLayeredMachineConfigPools() + if err != nil { + return err + } + + mobNPBytes, err := renderAsset(config, npAllowMOBPath) + if err != nil { + return err + } + mobNP := resourceread.ReadNetworkPolicyV1OrDie(mobNPBytes) + + if len(layeredMCPs) > 0 { + _, _, err = resourceapply.ApplyNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, mobNP, npCache) + if err != nil { + return err + } + } else { + _, _, err = resourceapply.DeleteNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, mobNP) + if err != nil && !apierrors.IsNotFound(err) { + return err + } + } + + return nil + }) +} + func (optr *Operator) syncMachineConfigDaemon(config *renderConfig, _ *configv1.ClusterOperator) error { paths := manifestPaths{ clusterRoles: []string{ From f4e3a6afe182c30123bc69f6b0cca1766c72c4d9 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Mon, 8 Jun 2026 13:06:02 +0530 Subject: [PATCH 2/5] MCO-2152: Move drift protection tests to MCO-2153 scope Remove DriftProtection and Idempotent tests from this file as they belong to the drift protection scope tracked under MCO-2153. Signed-off-by: HarshwardhanPatil07 --- pkg/operator/network_policy_test.go | 50 ----------------------------- 1 file changed, 50 deletions(-) diff --git a/pkg/operator/network_policy_test.go b/pkg/operator/network_policy_test.go index a081dea8c8..9317cddea9 100644 --- a/pkg/operator/network_policy_test.go +++ b/pkg/operator/network_policy_test.go @@ -188,53 +188,3 @@ func TestSyncNetworkPolicies_MOBPolicyDeletedWithoutLayeredPools(t *testing.T) { assert.NotNil(t, findPolicy(policies, "allow-machine-config-controller"), "static policies should still exist") } -func TestSyncNetworkPolicies_DriftProtection(t *testing.T) { - optr := testOperatorForNetworkPolicies(t, nil) - config := testRenderConfig() - - err := optr.syncNetworkPolicies(config, nil) - require.NoError(t, err) - - policy, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( - context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) - require.NoError(t, err) - - policy.Spec.Ingress = nil - _, err = optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Update( - context.TODO(), policy, metav1.UpdateOptions{}) - require.NoError(t, err) - - modified, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( - context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) - require.NoError(t, err) - assert.Empty(t, modified.Spec.Ingress, "ingress should be empty after manual modification") - - err = optr.syncNetworkPolicies(config, nil) - require.NoError(t, err) - - restored, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).Get( - context.TODO(), "allow-machine-config-operator", metav1.GetOptions{}) - require.NoError(t, err) - require.Len(t, restored.Spec.Ingress, 1, "ingress rules should be restored after resync") -} - -func TestSyncNetworkPolicies_Idempotent(t *testing.T) { - optr := testOperatorForNetworkPolicies(t, nil) - config := testRenderConfig() - - err := optr.syncNetworkPolicies(config, nil) - require.NoError(t, err) - firstCount := len(listNetworkPolicies(t, optr)) - - err = optr.syncNetworkPolicies(config, nil) - require.NoError(t, err) - secondCount := len(listNetworkPolicies(t, optr)) - - err = optr.syncNetworkPolicies(config, nil) - require.NoError(t, err) - thirdCount := len(listNetworkPolicies(t, optr)) - - assert.Equal(t, firstCount, secondCount, "policy count should be stable across syncs") - assert.Equal(t, secondCount, thirdCount, "policy count should be stable across syncs") - assert.Equal(t, 3, thirdCount, "should have exactly 3 static policies without layered pools") -} From 7762f52f0747c4494b4c3c35125dd59d1148fe82 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Mon, 8 Jun 2026 16:42:20 +0530 Subject: [PATCH 3/5] MCO-2153: Switch NetworkPolicy management to Server-Side Apply Replace traditional Get/Create/Update with SSA for stronger drift protection. The operator now owns all policy fields via FieldManager with Force:true, ensuring user modifications are always reverted. - Build policies programmatically using typed apply configurations - Remove YAML manifests (no longer needed with SSA) - Move syncNetworkPolicies to dedicated network_policy.go file Signed-off-by: HarshwardhanPatil07 --- manifests/networkpolicy/00-default-deny.yaml | 21 ---- .../01-allow-machine-config-operator.yaml | 28 ------ .../02-allow-machine-config-controller.yaml | 28 ------ .../03-allow-machine-os-builder.yaml | 29 ------ pkg/operator/network_policy.go | 99 +++++++++++++++++++ pkg/operator/network_policy_test.go | 2 +- pkg/operator/sync.go | 54 ---------- 7 files changed, 100 insertions(+), 161 deletions(-) delete mode 100644 manifests/networkpolicy/00-default-deny.yaml delete mode 100644 manifests/networkpolicy/01-allow-machine-config-operator.yaml delete mode 100644 manifests/networkpolicy/02-allow-machine-config-controller.yaml delete mode 100644 manifests/networkpolicy/03-allow-machine-os-builder.yaml create mode 100644 pkg/operator/network_policy.go diff --git a/manifests/networkpolicy/00-default-deny.yaml b/manifests/networkpolicy/00-default-deny.yaml deleted file mode 100644 index 5940762022..0000000000 --- a/manifests/networkpolicy/00-default-deny.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# Default-deny policy for the openshift-machine-config-operator namespace. -# This policy selects all pods in the namespace and enables default-deny for both -# ingress and egress by specifying policyTypes without any allow rules. -# -# NetworkPolicies are additive (use OR logic): -# - This policy enables default-deny for all pods -# - Subsequent policies add specific allow rules -# - If any policy allows traffic, that traffic is permitted -# -# Note: machine-config-daemon and machine-config-server use hostNetwork: true -# and bypass NetworkPolicy entirely. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: default-deny - namespace: {{.TargetNamespace}} -spec: - podSelector: {} - policyTypes: - - Ingress - - Egress diff --git a/manifests/networkpolicy/01-allow-machine-config-operator.yaml b/manifests/networkpolicy/01-allow-machine-config-operator.yaml deleted file mode 100644 index acf296e7be..0000000000 --- a/manifests/networkpolicy/01-allow-machine-config-operator.yaml +++ /dev/null @@ -1,28 +0,0 @@ -# Network policy for the machine-config-operator pod. -# -# Egress: -# - Allow all egress to support communication with the Kubernetes API server, -# whose IP address and port are not known at manifest time. This implicitly -# covers DNS resolution as well. -# -# Ingress: -# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. -# The metrics endpoint uses kube-rbac-proxy for authentication. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: allow-machine-config-operator - namespace: {{.TargetNamespace}} -spec: - podSelector: - matchLabels: - k8s-app: machine-config-operator - ingress: - - ports: - - protocol: TCP - port: 9001 - egress: - - {} - policyTypes: - - Ingress - - Egress diff --git a/manifests/networkpolicy/02-allow-machine-config-controller.yaml b/manifests/networkpolicy/02-allow-machine-config-controller.yaml deleted file mode 100644 index 134d08bac4..0000000000 --- a/manifests/networkpolicy/02-allow-machine-config-controller.yaml +++ /dev/null @@ -1,28 +0,0 @@ -# Network policy for the machine-config-controller pod. -# -# Egress: -# - Allow all egress to support communication with the Kubernetes API server, -# whose IP address and port are not known at manifest time. This implicitly -# covers DNS resolution as well. -# -# Ingress: -# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. -# The metrics endpoint uses kube-rbac-proxy for authentication. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: allow-machine-config-controller - namespace: {{.TargetNamespace}} -spec: - podSelector: - matchLabels: - k8s-app: machine-config-controller - ingress: - - ports: - - protocol: TCP - port: 9001 - egress: - - {} - policyTypes: - - Ingress - - Egress diff --git a/manifests/networkpolicy/03-allow-machine-os-builder.yaml b/manifests/networkpolicy/03-allow-machine-os-builder.yaml deleted file mode 100644 index 50cf58c68e..0000000000 --- a/manifests/networkpolicy/03-allow-machine-os-builder.yaml +++ /dev/null @@ -1,29 +0,0 @@ -# Network policy for the machine-os-builder pod. -# This policy is only applied when layered MachineConfigPools exist, -# and is deleted when no layered pools are present. -# -# Egress: -# - Allow all egress for API server communication, container registry access, -# and DNS resolution. All egress is permitted because destination addresses -# (registries, API server) vary by cluster configuration. -# -# Ingress: -# - Allow ingress on port 9001 (metrics) so that Prometheus can scrape metrics. -apiVersion: networking.k8s.io/v1 -kind: NetworkPolicy -metadata: - name: allow-machine-os-builder - namespace: {{.TargetNamespace}} -spec: - podSelector: - matchLabels: - k8s-app: machine-os-builder - ingress: - - ports: - - protocol: TCP - port: 9001 - egress: - - {} - policyTypes: - - Ingress - - Egress diff --git a/pkg/operator/network_policy.go b/pkg/operator/network_policy.go new file mode 100644 index 0000000000..c149fe67fb --- /dev/null +++ b/pkg/operator/network_policy.go @@ -0,0 +1,99 @@ +package operator + +import ( + "context" + + configv1 "github.com/openshift/api/config/v1" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" + networkingv1ac "k8s.io/client-go/applyconfigurations/networking/v1" +) + +const networkPolicyFieldManager = "machine-config-operator" + +func defaultDenyNetworkPolicy(namespace string) *networkingv1ac.NetworkPolicyApplyConfiguration { + return networkingv1ac.NetworkPolicy("default-deny", namespace). + WithSpec(networkingv1ac.NetworkPolicySpec(). + WithPodSelector(metav1ac.LabelSelector()). + WithPolicyTypes(networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress)) +} + +func allowMCONetworkPolicy(namespace string) *networkingv1ac.NetworkPolicyApplyConfiguration { + return networkingv1ac.NetworkPolicy("allow-machine-config-operator", namespace). + WithSpec(networkingv1ac.NetworkPolicySpec(). + WithPodSelector(metav1ac.LabelSelector(). + WithMatchLabels(map[string]string{"k8s-app": "machine-config-operator"})). + WithIngress(networkingv1ac.NetworkPolicyIngressRule(). + WithPorts(networkingv1ac.NetworkPolicyPort(). + WithProtocol(corev1.ProtocolTCP). + WithPort(intstr.FromInt32(9001)))). + WithEgress(networkingv1ac.NetworkPolicyEgressRule()). + WithPolicyTypes(networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress)) +} + +func allowMCCNetworkPolicy(namespace string) *networkingv1ac.NetworkPolicyApplyConfiguration { + return networkingv1ac.NetworkPolicy("allow-machine-config-controller", namespace). + WithSpec(networkingv1ac.NetworkPolicySpec(). + WithPodSelector(metav1ac.LabelSelector(). + WithMatchLabels(map[string]string{"k8s-app": "machine-config-controller"})). + WithIngress(networkingv1ac.NetworkPolicyIngressRule(). + WithPorts(networkingv1ac.NetworkPolicyPort(). + WithProtocol(corev1.ProtocolTCP). + WithPort(intstr.FromInt32(9001)))). + WithEgress(networkingv1ac.NetworkPolicyEgressRule()). + WithPolicyTypes(networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress)) +} + +func allowMOBNetworkPolicy(namespace string) *networkingv1ac.NetworkPolicyApplyConfiguration { + return networkingv1ac.NetworkPolicy("allow-machine-os-builder", namespace). + WithSpec(networkingv1ac.NetworkPolicySpec(). + WithPodSelector(metav1ac.LabelSelector(). + WithMatchLabels(map[string]string{"k8s-app": "machine-os-builder"})). + WithIngress(networkingv1ac.NetworkPolicyIngressRule(). + WithPorts(networkingv1ac.NetworkPolicyPort(). + WithProtocol(corev1.ProtocolTCP). + WithPort(intstr.FromInt32(9001)))). + WithEgress(networkingv1ac.NetworkPolicyEgressRule()). + WithPolicyTypes(networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress)) +} + +func (optr *Operator) syncNetworkPolicies(_ *renderConfig, _ *configv1.ClusterOperator) error { + ctx := context.TODO() + ns := ctrlcommon.MCONamespace + applyOpts := metav1.ApplyOptions{FieldManager: networkPolicyFieldManager, Force: true} + + staticPolicies := []*networkingv1ac.NetworkPolicyApplyConfiguration{ + defaultDenyNetworkPolicy(ns), + allowMCONetworkPolicy(ns), + allowMCCNetworkPolicy(ns), + } + + for _, policy := range staticPolicies { + if _, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ns).Apply(ctx, policy, applyOpts); err != nil { + return err + } + } + + layeredMCPs, err := optr.getLayeredMachineConfigPools() + if err != nil { + return err + } + + if len(layeredMCPs) > 0 { + if _, err := optr.kubeClient.NetworkingV1().NetworkPolicies(ns).Apply(ctx, allowMOBNetworkPolicy(ns), applyOpts); err != nil { + return err + } + } else { + err := optr.kubeClient.NetworkingV1().NetworkPolicies(ns).Delete(ctx, "allow-machine-os-builder", metav1.DeleteOptions{}) + if err != nil && !apierrors.IsNotFound(err) { + return err + } + } + + return nil +} diff --git a/pkg/operator/network_policy_test.go b/pkg/operator/network_policy_test.go index 9317cddea9..cd9b05049d 100644 --- a/pkg/operator/network_policy_test.go +++ b/pkg/operator/network_policy_test.go @@ -26,7 +26,7 @@ func testRenderConfig() *renderConfig { func testOperatorForNetworkPolicies(t *testing.T, pools []*mcfgv1.MachineConfigPool) *Operator { t.Helper() - kubeClient := fake.NewSimpleClientset() + kubeClient := fake.NewClientset() mcfgClient := fakeclientmachineconfigv1.NewSimpleClientset() mcfgInformerFactory := mcfginformers.NewSharedInformerFactory(mcfgClient, 0) mcpInformer := mcfgInformerFactory.Machineconfiguration().V1().MachineConfigPools() diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 058b7f2160..5eeda92e0c 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -154,12 +154,6 @@ const ( // Machine OS puller manifest paths mopRoleBindingManifestPath = "manifests/machine-os-puller/rolebinding.yaml" mopServiceAccountManifestPath = "manifests/machine-os-puller/sa.yaml" - - // NetworkPolicy manifest paths - npDefaultDenyPath = "manifests/networkpolicy/00-default-deny.yaml" - npAllowMCOPath = "manifests/networkpolicy/01-allow-machine-config-operator.yaml" - npAllowMCCPath = "manifests/networkpolicy/02-allow-machine-config-controller.yaml" - npAllowMOBPath = "manifests/networkpolicy/03-allow-machine-os-builder.yaml" ) type syncFunc struct { @@ -1637,54 +1631,6 @@ func (optr *Operator) getLayeredMachineConfigPools() ([]*mcfgv1.MachineConfigPoo return pools, nil } -func (optr *Operator) syncNetworkPolicies(config *renderConfig, _ *configv1.ClusterOperator) error { - return retry.OnError(retry.DefaultRetry, mcoResourceApply.IsApplyErrorRetriable, func() error { - npCache := resourceapply.NewResourceCache() - - staticPolicies := []string{ - npDefaultDenyPath, - npAllowMCOPath, - npAllowMCCPath, - } - for _, path := range staticPolicies { - npBytes, err := renderAsset(config, path) - if err != nil { - return err - } - np := resourceread.ReadNetworkPolicyV1OrDie(npBytes) - _, _, err = resourceapply.ApplyNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, np, npCache) - if err != nil { - return err - } - } - - layeredMCPs, err := optr.getLayeredMachineConfigPools() - if err != nil { - return err - } - - mobNPBytes, err := renderAsset(config, npAllowMOBPath) - if err != nil { - return err - } - mobNP := resourceread.ReadNetworkPolicyV1OrDie(mobNPBytes) - - if len(layeredMCPs) > 0 { - _, _, err = resourceapply.ApplyNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, mobNP, npCache) - if err != nil { - return err - } - } else { - _, _, err = resourceapply.DeleteNetworkPolicy(context.TODO(), optr.kubeClient.NetworkingV1(), optr.libgoRecorder, mobNP) - if err != nil && !apierrors.IsNotFound(err) { - return err - } - } - - return nil - }) -} - func (optr *Operator) syncMachineConfigDaemon(config *renderConfig, _ *configv1.ClusterOperator) error { paths := manifestPaths{ clusterRoles: []string{ From 7de6d83f0fea616e02b850c9967a3a18ad2d6fb2 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Thu, 11 Jun 2026 16:42:37 +0530 Subject: [PATCH 4/5] MCO-2153: Add ValidatingAdmissionPolicy to block network policy deletion and e2e tests Add a ValidatingAdmissionPolicy that prevents deletion of MCO-managed NetworkPolicies (default-deny, allow-machine-config-operator, allow-machine-config-controller, allow-machine-os-builder) in the openshift-machine-config-operator namespace. This is needed because the default-deny policy blocks all egress, so deleting an allow policy causes the MCO pod to lose API server access, creating an unrecoverable deadlock where the sync loop cannot recreate the deleted policy. Modifications to managed policies are still reverted via Server-Side Apply on the next sync cycle. Also adds e2e presubmit tests to test/e2e-2of2/ covering: - Default policies exist with correct specs - Modifications are reverted by SSA - Deletions are blocked by ValidatingAdmissionPolicy - MCO processes continue working with policies in place - User-created policies coexist with managed policies Signed-off-by: HarshwardhanPatil07 --- ...etion-guard-validatingadmissionpolicy.yaml | 21 ++ ...uard-validatingadmissionpolicybinding.yaml | 7 + pkg/operator/sync.go | 4 + test/e2e-2of2/network_policy_test.go | 297 ++++++++++++++++++ 4 files changed, 329 insertions(+) create mode 100644 manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicy.yaml create mode 100644 manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicybinding.yaml create mode 100644 test/e2e-2of2/network_policy_test.go diff --git a/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicy.yaml b/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicy.yaml new file mode 100644 index 0000000000..78845af7de --- /dev/null +++ b/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicy.yaml @@ -0,0 +1,21 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: "networkpolicy-prevent-deletion" +spec: + failurePolicy: Fail + matchConstraints: + matchPolicy: Equivalent + namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-machine-config-operator + objectSelector: {} + resourceRules: + - apiGroups: ["networking.k8s.io"] + apiVersions: ["v1"] + operations: ["DELETE"] + resources: ["networkpolicies"] + scope: "Namespaced" + validations: + - expression: "!(oldObject.metadata.name in ['default-deny', 'allow-machine-config-operator', 'allow-machine-config-controller', 'allow-machine-os-builder'])" + message: "Deletion of MCO-managed NetworkPolicy resources in the openshift-machine-config-operator namespace is not allowed" diff --git a/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicybinding.yaml b/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicybinding.yaml new file mode 100644 index 0000000000..81975ebe45 --- /dev/null +++ b/manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicybinding.yaml @@ -0,0 +1,7 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: "networkpolicy-prevent-deletion-binding" +spec: + policyName: "networkpolicy-prevent-deletion" + validationActions: [Deny] diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 5eeda92e0c..ffa9b46420 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -113,6 +113,8 @@ const ( mccOSImageStreamDeletionGuardValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/osimagestream-deletion-guard-validatingadmissionpolicybinding.yaml" mccIRIDeletionGuardValidatingAdmissionPolicyPath = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicy.yaml" mccIRIDeletionGuardValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicybinding.yaml" + mccNetworkPolicyDeletionGuardValidatingAdmissionPolicyPath = "manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicy.yaml" + mccNetworkPolicyDeletionGuardValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/networkpolicy-deletion-guard-validatingadmissionpolicybinding.yaml" mccUpdateBootImagesCPMSValidatingAdmissionPolicyPath = "manifests/machineconfigcontroller/update-bootimages-cpms-validatingadmissionpolicy.yaml" mccUpdateBootImagesCPMSValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/update-bootimages-cpms-validatingadmissionpolicybinding.yaml" @@ -1210,11 +1212,13 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig, _ *confi mccMachineConfigurationGuardsValidatingAdmissionPolicyPath, mccUpdateBootImagesValidatingAdmissionPolicyPath, mccMachineConfigPoolSelectorValidatingAdmissionPolicyPath, + mccNetworkPolicyDeletionGuardValidatingAdmissionPolicyPath, }, validatingAdmissionPolicyBindings: []string{ mccMachineConfigurationGuardsValidatingAdmissionPolicyBindingPath, mccUpdateBootImagesValidatingAdmissionPolicyBindingPath, mccMachineConfigPoolSelectorValidatingAdmissionPolicyBindingPath, + mccNetworkPolicyDeletionGuardValidatingAdmissionPolicyBindingPath, }, } diff --git a/test/e2e-2of2/network_policy_test.go b/test/e2e-2of2/network_policy_test.go new file mode 100644 index 0000000000..6bf7ffd264 --- /dev/null +++ b/test/e2e-2of2/network_policy_test.go @@ -0,0 +1,297 @@ +package e2e_2of2_test + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + configv1 "github.com/openshift/api/config/v1" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/wait" + + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/openshift/machine-config-operator/test/framework" +) + +const ( + networkPolicySyncPollInterval = 5 * time.Second + networkPolicySyncPollTimeout = 2 * time.Minute +) + +var staticPolicyNames = []string{ + "default-deny", + "allow-machine-config-operator", + "allow-machine-config-controller", +} + +func skipIfNoNetworkPolicies(t *testing.T) { + t.Helper() + cs := framework.NewClientSet("") + policies, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).List( + context.Background(), metav1.ListOptions{}) + require.NoError(t, err) + if len(policies.Items) == 0 { + t.Skip("skipping: no network policies found in MCO namespace") + } +} + +func TestNetworkPolicies_DefaultPoliciesExist(t *testing.T) { + skipIfNoNetworkPolicies(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + ns := ctrlcommon.MCONamespace + + policies, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + policyMap := make(map[string]*networkingv1.NetworkPolicy) + for i := range policies.Items { + policyMap[policies.Items[i].Name] = &policies.Items[i] + } + + for _, name := range staticPolicyNames { + assert.Contains(t, policyMap, name, "expected policy %s to exist", name) + } + + deny := policyMap["default-deny"] + if deny != nil { + assert.Empty(t, deny.Spec.PodSelector.MatchLabels, "default-deny should select all pods") + assert.Contains(t, deny.Spec.PolicyTypes, networkingv1.PolicyTypeIngress) + assert.Contains(t, deny.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) + assert.Empty(t, deny.Spec.Ingress, "default-deny should have no ingress rules") + assert.Empty(t, deny.Spec.Egress, "default-deny should have no egress rules") + } + + for _, tc := range []struct { + name string + labelVal string + }{ + {"allow-machine-config-operator", "machine-config-operator"}, + {"allow-machine-config-controller", "machine-config-controller"}, + } { + policy := policyMap[tc.name] + if policy == nil { + continue + } + + assert.Equal(t, tc.labelVal, policy.Spec.PodSelector.MatchLabels["k8s-app"], + "%s: podSelector should match k8s-app=%s", tc.name, tc.labelVal) + assert.Contains(t, policy.Spec.PolicyTypes, networkingv1.PolicyTypeIngress) + assert.Contains(t, policy.Spec.PolicyTypes, networkingv1.PolicyTypeEgress) + + require.NotEmpty(t, policy.Spec.Ingress, "%s: should have ingress rules", tc.name) + require.NotEmpty(t, policy.Spec.Ingress[0].Ports, "%s: ingress should have ports", tc.name) + assert.Equal(t, int32(9001), policy.Spec.Ingress[0].Ports[0].Port.IntVal, + "%s: ingress port should be 9001", tc.name) + assert.Equal(t, corev1.ProtocolTCP, *policy.Spec.Ingress[0].Ports[0].Protocol, + "%s: ingress protocol should be TCP", tc.name) + + require.NotEmpty(t, policy.Spec.Egress, "%s: should have egress rules", tc.name) + } +} + +func TestNetworkPolicies_ModificationReverted(t *testing.T) { + skipIfNoNetworkPolicies(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + ns := ctrlcommon.MCONamespace + npClient := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns) + + policy, err := npClient.Get(ctx, "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + require.NotEmpty(t, policy.Spec.Ingress, "policy should have ingress rules before modification") + + policy.Spec.Ingress = nil + _, err = npClient.Update(ctx, policy, metav1.UpdateOptions{}) + require.NoError(t, err) + + modified, err := npClient.Get(ctx, "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + assert.Empty(t, modified.Spec.Ingress, "ingress should be empty after modification") + + err = wait.PollUntilContextTimeout(ctx, networkPolicySyncPollInterval, networkPolicySyncPollTimeout, true, + func(ctx context.Context) (bool, error) { + restored, err := npClient.Get(ctx, "allow-machine-config-operator", metav1.GetOptions{}) + if err != nil { + return false, err + } + return len(restored.Spec.Ingress) > 0, nil + }) + require.NoError(t, err, "operator should revert modified ingress rules") + + restored, err := npClient.Get(ctx, "allow-machine-config-operator", metav1.GetOptions{}) + require.NoError(t, err) + require.Len(t, restored.Spec.Ingress, 1) + require.Len(t, restored.Spec.Ingress[0].Ports, 1) + assert.Equal(t, int32(9001), restored.Spec.Ingress[0].Ports[0].Port.IntVal, + "restored policy should have metrics port 9001") +} + +func TestNetworkPolicies_DeletionBlocked(t *testing.T) { + skipIfNoNetworkPolicies(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + ns := ctrlcommon.MCONamespace + npClient := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns) + + for _, name := range staticPolicyNames { + err := npClient.Delete(ctx, name, metav1.DeleteOptions{}) + require.Error(t, err, "deleting managed policy %s should be blocked", name) + require.True(t, k8serrors.IsInvalid(err), + "error should be Invalid (422) from ValidatingAdmissionPolicy, got: %v", err) + require.Contains(t, err.Error(), "MCO-managed NetworkPolicy", + "error message should indicate the policy is MCO-managed") + } + + // Verify all policies still exist after blocked deletion attempts + policies, err := npClient.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + policyNames := make(map[string]bool) + for _, p := range policies.Items { + policyNames[p.Name] = true + } + for _, expected := range staticPolicyNames { + assert.True(t, policyNames[expected], "policy %s should still exist after blocked deletion", expected) + } +} + +func TestNetworkPolicies_MCOProcessesContinueWorking(t *testing.T) { + skipIfNoNetworkPolicies(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + ns := ctrlcommon.MCONamespace + + co, err := cs.ClusterOperators().Get(ctx, "machine-config", metav1.GetOptions{}) + require.NoError(t, err, "should be able to get machine-config ClusterOperator") + + for _, cond := range co.Status.Conditions { + switch cond.Type { + case configv1.OperatorAvailable: + assert.Equal(t, configv1.ConditionTrue, cond.Status, + "machine-config operator should be Available") + case configv1.OperatorDegraded: + assert.Equal(t, configv1.ConditionFalse, cond.Status, + "machine-config operator should not be Degraded") + } + } + + pods, err := cs.Pods(ns).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + componentRunning := map[string]bool{ + "machine-config-operator": false, + "machine-config-controller": false, + } + for _, pod := range pods.Items { + app := pod.Labels["k8s-app"] + if _, tracked := componentRunning[app]; tracked && pod.Status.Phase == corev1.PodRunning { + componentRunning[app] = true + } + } + for component, running := range componentRunning { + assert.True(t, running, "%s pod should be Running", component) + } + + policies, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + policyNames := make([]string, len(policies.Items)) + for i, p := range policies.Items { + policyNames[i] = p.Name + } + for _, expected := range staticPolicyNames { + assert.Contains(t, policyNames, expected, + "policy %s should exist while MCO processes are running", expected) + } +} + +func TestNetworkPolicies_AdminNetworkPolicyOverride(t *testing.T) { + skipIfNoNetworkPolicies(t) + + cs := framework.NewClientSet("") + ctx := context.Background() + ns := ctrlcommon.MCONamespace + + deny, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).Get(ctx, "default-deny", metav1.GetOptions{}) + require.NoError(t, err) + assert.Empty(t, deny.Spec.Ingress, "default-deny should block all ingress") + + overridePolicy := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-additional-allow", + Namespace: ns, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"k8s-app": "machine-config-operator"}, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: npProtocolPtr(corev1.ProtocolTCP), + Port: npPortPtr(8080), + }, + }, + }, + }, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + }, + } + + _, err = cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).Create(ctx, overridePolicy, metav1.CreateOptions{}) + require.NoError(t, err) + defer func() { + _ = cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).Delete(ctx, "test-additional-allow", metav1.DeleteOptions{}) + }() + + created, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).Get(ctx, "test-additional-allow", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "test-additional-allow", created.Name) + + err = wait.PollUntilContextTimeout(ctx, networkPolicySyncPollInterval, networkPolicySyncPollTimeout, true, + func(ctx context.Context) (bool, error) { + policies, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).List(ctx, metav1.ListOptions{}) + if err != nil { + return false, err + } + names := make(map[string]bool) + for _, p := range policies.Items { + names[p.Name] = true + } + for _, expected := range staticPolicyNames { + if !names[expected] { + return false, nil + } + } + return names["test-additional-allow"], nil + }) + require.NoError(t, err, "operator-managed policies should coexist with user-created policies") + + final, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ns).Get(ctx, "test-additional-allow", metav1.GetOptions{}) + require.NoError(t, err) + require.Len(t, final.Spec.Ingress, 1) + require.Len(t, final.Spec.Ingress[0].Ports, 1) + assert.Equal(t, int32(8080), final.Spec.Ingress[0].Ports[0].Port.IntVal, + "user-created policy should retain its original port") +} + +func npProtocolPtr(p corev1.Protocol) *corev1.Protocol { + return &p +} + +func npPortPtr(port int32) *intstr.IntOrString { + p := intstr.FromInt32(port) + return &p +} From a99acaddf0ee51d2dcbc43239b1bde0f2b66c551 Mon Sep 17 00:00:00 2001 From: HarshwardhanPatil07 Date: Thu, 11 Jun 2026 21:24:04 +0530 Subject: [PATCH 5/5] MCO-2153: Harden network policy e2e tests against silent regressions Replace t.Skip with require.NotEmpty when no NetworkPolicies are found so missing managed policies fail the test instead of being skipped. Add explicit presence assertions for OperatorAvailable and OperatorDegraded conditions to prevent silent pass when conditions are absent. Signed-off-by: HarshwardhanPatil07 --- test/e2e-2of2/network_policy_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/e2e-2of2/network_policy_test.go b/test/e2e-2of2/network_policy_test.go index 6bf7ffd264..02c54c25f1 100644 --- a/test/e2e-2of2/network_policy_test.go +++ b/test/e2e-2of2/network_policy_test.go @@ -36,9 +36,7 @@ func skipIfNoNetworkPolicies(t *testing.T) { policies, err := cs.GetKubeclient().NetworkingV1().NetworkPolicies(ctrlcommon.MCONamespace).List( context.Background(), metav1.ListOptions{}) require.NoError(t, err) - if len(policies.Items) == 0 { - t.Skip("skipping: no network policies found in MCO namespace") - } + require.NotEmpty(t, policies.Items, "expected at least one NetworkPolicy in MCO namespace") } func TestNetworkPolicies_DefaultPoliciesExist(t *testing.T) { @@ -175,16 +173,21 @@ func TestNetworkPolicies_MCOProcessesContinueWorking(t *testing.T) { co, err := cs.ClusterOperators().Get(ctx, "machine-config", metav1.GetOptions{}) require.NoError(t, err, "should be able to get machine-config ClusterOperator") + foundAvailable, foundDegraded := false, false for _, cond := range co.Status.Conditions { switch cond.Type { case configv1.OperatorAvailable: + foundAvailable = true assert.Equal(t, configv1.ConditionTrue, cond.Status, "machine-config operator should be Available") case configv1.OperatorDegraded: + foundDegraded = true assert.Equal(t, configv1.ConditionFalse, cond.Status, "machine-config operator should not be Degraded") } } + assert.True(t, foundAvailable, "OperatorAvailable condition must be present on machine-config ClusterOperator") + assert.True(t, foundDegraded, "OperatorDegraded condition must be present on machine-config ClusterOperator") pods, err := cs.Pods(ns).List(ctx, metav1.ListOptions{}) require.NoError(t, err)