MCO-2152: MCO-2153: MCO-2154: Create MCO network policies for pods

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,
 		)
 

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"

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]

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
+}

pkg/operator/network_policy_test.go

@@ -0,0 +1,190 @@
+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.NewClientset()
+	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")
+}
+

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},