Isolate only virtual cluster workload Pods (#989)

* Enhance network policy to isolate synced workload pods and improve cross-cluster pod isolation handling * Add test for label update on synced Pod to ensure isolation label persistence * Derive host pod CIDRs dynamically for the isolation NetworkPolicy Compute the egress-exclude CIDRs from the --cluster-cidr flag or the live Node PodCIDR(s) via FindPodCIDRs, instead of a hardcoded guess, so cross-cluster pod isolation is enforced against the host's real pod network. Adds unit, integration, and e2e coverage. * update comment * Sort CIDR list in FindPodCIDRs function to ensure consistent order for egress rules * Use `t.Context()` instead of `context.Background()` Co-authored-by: Kevin McDermott <bigkevmcd@gmail.com> * Use Ginkgo provided context * Refactor FindPodCIDRs to use sets for CIDR collection and simplify logic * fix lint --------- Co-authored-by: Kevin McDermott <bigkevmcd@gmail.com>
2026-08-23 22:36:17 +00:00 · 2026-07-16 15:18:04 +02:00
parent 10a0b42c6a
commit e1ae07c836
7 changed files with 357 additions and 19 deletions
@@ -36,6 +36,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	ctrlcontroller "sigs.k8s.io/controller-runtime/pkg/controller"

+	"github.com/rancher/k3k/k3k-kubelet/translate"
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
 	"github.com/rancher/k3k/pkg/controller"
 	"github.com/rancher/k3k/pkg/controller/cluster/agent"
@@ -587,6 +588,11 @@ func (c *ClusterReconciler) ensureNetworkPolicy(ctx context.Context, cluster *v1
 		return client.IgnoreNotFound(c.Client.Delete(ctx, netpol))
 	}

+	cidrList, err := policy.FindPodCIDRs(ctx, c.Client, c.ClusterCIDR)
+	if err != nil {
+		return err
+	}
+
 	expectedNetworkPolicy := &networkingv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      controller.SafeConcatNameWithPrefix(cluster.Name),
@@ -597,6 +603,15 @@ func (c *ClusterReconciler) ensureNetworkPolicy(ctx context.Context, cluster *v1
 			APIVersion: "networking.k8s.io/v1",
 		},
 		Spec: networkingv1.NetworkPolicySpec{
+			// Isolate synced workload pods
+			PodSelector: metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      translate.ClusterNameLabel,
+						Operator: metav1.LabelSelectorOpExists,
+					},
+				},
+			},
 			PolicyTypes: []networkingv1.PolicyType{
 				networkingv1.PolicyTypeIngress,
 				networkingv1.PolicyTypeEgress,
@@ -610,7 +625,7 @@ func (c *ClusterReconciler) ensureNetworkPolicy(ctx context.Context, cluster *v1
 						{
 							IPBlock: &networkingv1.IPBlock{
 								CIDR:   "0.0.0.0/0",
-								Except: []string{cluster.Status.ClusterCIDR},
+								Except: cidrList,
 							},
 						},
 					},
@@ -3,6 +3,7 @@ package policy
 import (
 	"context"

+	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"

 	corev1 "k8s.io/api/core/v1"
@@ -11,6 +12,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"

+	"github.com/rancher/k3k/k3k-kubelet/translate"
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
 	k3kcontroller "github.com/rancher/k3k/pkg/controller"
 )
@@ -19,23 +21,9 @@ func (c *VirtualClusterPolicyReconciler) reconcileNetworkPolicy(ctx context.Cont
 	log := ctrl.LoggerFrom(ctx)
 	log.V(1).Info("Reconciling NetworkPolicy")

-	var cidrList []string
-
-	if c.ClusterCIDR != "" {
-		cidrList = []string{c.ClusterCIDR}
-	} else {
-		var nodeList corev1.NodeList
-		if err := c.Client.List(ctx, &nodeList); err != nil {
-			return err
-		}
-
-		for _, node := range nodeList.Items {
-			if len(node.Spec.PodCIDRs) > 0 {
-				cidrList = append(cidrList, node.Spec.PodCIDRs...)
-			} else {
-				cidrList = append(cidrList, node.Spec.PodCIDR)
-			}
-		}
+	cidrList, err := FindPodCIDRs(ctx, c.Client, c.ClusterCIDR)
+	if err != nil {
+		return err
 	}

 	networkPolicy := networkPolicy(namespace, policy, cidrList)
@@ -54,7 +42,7 @@ func (c *VirtualClusterPolicyReconciler) reconcileNetworkPolicy(ctx context.Cont
 	log.V(1).Info("Creating NetworkPolicy")

 	// otherwise try to create/update
-	err := c.Client.Create(ctx, networkPolicy)
+	err = c.Client.Create(ctx, networkPolicy)
 	if apierrors.IsAlreadyExists(err) {
 		log.V(1).Info("NetworkPolicy already exists, updating.")

@@ -64,6 +52,51 @@ func (c *VirtualClusterPolicyReconciler) reconcileNetworkPolicy(ctx context.Cont
 	return err
 }

+// FindPodCIDRs returns the CIDR ranges to exclude from the isolation NetworkPolicy's egress allow-list,
+// so that pod-to-pod traffic on the host's real pod network is blocked.
+// If clusterCIDR is set (the --cluster-cidr controller flag) it's used as-is;
+// otherwise it's inferred from the host Nodes' Spec.PodCIDR(s).
+// Returns an empty list, and logs a warning, if it can't be determined either way.
+func FindPodCIDRs(ctx context.Context, cl client.Client, clusterCIDR string) ([]string, error) {
+	log := ctrl.LoggerFrom(ctx)
+
+	if clusterCIDR != "" {
+		return []string{clusterCIDR}, nil
+	}
+
+	var nodeList corev1.NodeList
+	if err := cl.List(ctx, &nodeList); err != nil {
+		return nil, err
+	}
+
+	cidrs := sets.New[string]()
+
+	for _, node := range nodeList.Items {
+		cidrs.Insert(node.Spec.PodCIDRs...)
+
+		if node.Spec.PodCIDR != "" {
+			cidrs.Insert(node.Spec.PodCIDR)
+		}
+	}
+
+	cidrList := sets.List(cidrs)
+
+	// node.Spec.PodCIDR is only populated when kube-controller-manager runs with --allocate-node-cidrs.
+	// K3s and RKE2 enable it by default (cluster-cidr 10.42.0.0/16),
+	// so the field is populated there regardless of CNI.
+	// Some setups don't set it, i.e. Cilium with cluster-pool IPAM (Cilium's default),
+	// which manages per-node CIDRs in the v2.CiliumNode CRD and doesn't need Kubernetes to hand out PodCIDRs.
+	// See https://docs.cilium.io/en/stable/network/concepts/ipam/cluster-pool/
+	// Without it the egress rule can't exclude the pod network, so cross-cluster pod
+	// isolation would silently not be enforced. Set --cluster-cidr in that case.
+	if len(cidrList) == 0 {
+		log.Info("Could not determine the pod CIDR from the nodes; cross-cluster pod isolation will not be enforced. " +
+			"Set the --cluster-cidr flag on the k3k controller to fix this.")
+	}
+
+	return cidrList, nil
+}
+
 func networkPolicy(namespaceName string, policy *v1beta1.VirtualClusterPolicy, cidrList []string) *networkingv1.NetworkPolicy {
 	return &networkingv1.NetworkPolicy{
 		TypeMeta: metav1.TypeMeta{
@@ -79,6 +112,15 @@ func networkPolicy(namespaceName string, policy *v1beta1.VirtualClusterPolicy, c
 			},
 		},
 		Spec: networkingv1.NetworkPolicySpec{
+			// Isolate synced workload pods
+			PodSelector: metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{
+					{
+						Key:      translate.ClusterNameLabel,
+						Operator: metav1.LabelSelectorOpExists,
+					},
+				},
+			},
 			PolicyTypes: []networkingv1.PolicyType{
 				networkingv1.PolicyTypeIngress,
 				networkingv1.PolicyTypeEgress,
@@ -0,0 +1,114 @@
+package policy_test
+
+// This file pins the behavior of FindPodCIDRs(), which computes the CIDR ranges excluded
+// from the isolation NetworkPolicy's egress allow-list (so pod-to-pod traffic on the host's real
+// pod network is blocked). The behaviors asserted here are:
+//
+// 1. --cluster-cidr override: when set, it's used as-is and Nodes are never consulted.
+// 2. Node-based detection: falls back to collecting Spec.PodCIDRs/Spec.PodCIDR from live Nodes.
+// 3. Can't-determine fallback: returns an empty list (fail-open) when neither is available.
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/rancher/k3k/pkg/controller/policy"
+)
+
+func TestFindPodCIDRs_ClusterCIDROverride(t *testing.T) {
+	// a Node with a different PodCIDR is present, but the explicit override must win and
+	// the Node must not even be consulted
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+		Spec:       corev1.NodeSpec{PodCIDR: "10.244.0.0/24"},
+	}
+
+	cl := createFakeClient(t, node)
+
+	cidrs, err := policy.FindPodCIDRs(t.Context(), cl, "10.42.0.0/16")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"10.42.0.0/16"}, cidrs)
+}
+
+func TestFindPodCIDRs_FromNodePodCIDR(t *testing.T) {
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+		Spec:       corev1.NodeSpec{PodCIDR: "192.168.77.0/24"},
+	}
+
+	cl := createFakeClient(t, node)
+
+	cidrs, err := policy.FindPodCIDRs(t.Context(), cl, "")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"192.168.77.0/24"}, cidrs)
+}
+
+func TestFindPodCIDRs_FromNodePodCIDRs(t *testing.T) {
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+		Spec:       corev1.NodeSpec{PodCIDRs: []string{"192.168.77.0/24", "fd00:1::/64"}},
+	}
+
+	cl := createFakeClient(t, node)
+
+	cidrs, err := policy.FindPodCIDRs(t.Context(), cl, "")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"192.168.77.0/24", "fd00:1::/64"}, cidrs)
+}
+
+func TestFindPodCIDRs_MultipleNodes(t *testing.T) {
+	node1 := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+		Spec:       corev1.NodeSpec{PodCIDR: "192.168.77.0/24"},
+	}
+	node2 := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-2"},
+		Spec:       corev1.NodeSpec{PodCIDR: "192.168.78.0/24"},
+	}
+
+	cl := createFakeClient(t, node1, node2)
+
+	cidrs, err := policy.FindPodCIDRs(t.Context(), cl, "")
+	require.NoError(t, err)
+	assert.ElementsMatch(t, []string{"192.168.77.0/24", "192.168.78.0/24"}, cidrs)
+}
+
+func TestFindPodCIDRs_CannotDetermine(t *testing.T) {
+	// no --cluster-cidr override, and the Node carries no PodCIDR at all (e.g. Cilium with
+	// cluster-pool IPAM) -- must fail open (empty list) rather than error
+	node := &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{Name: "node-1"},
+	}
+
+	cl := createFakeClient(t, node)
+
+	cidrs, err := policy.FindPodCIDRs(t.Context(), cl, "")
+	require.NoError(t, err)
+	assert.Empty(t, cidrs)
+}
+
+func createFakeClient(t *testing.T, objs ...client.Object) client.Client {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+
+	schemeBuilder := runtime.NewSchemeBuilder(
+		corev1.AddToScheme,
+	)
+
+	err := schemeBuilder.AddToScheme(scheme)
+	require.NoError(t, err)
+
+	return fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(objs...).
+		Build()
+}
@@ -1,6 +1,14 @@
 package k3k_test

 import (
+	"context"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	networkingv1 "k8s.io/api/networking/v1"
+
+	k3kcontroller "github.com/rancher/k3k/pkg/controller"
+	"github.com/rancher/k3k/pkg/controller/policy"
 	fwk3k "github.com/rancher/k3k/tests/framework/k3k"

 	. "github.com/onsi/ginkgo/v2"
@@ -92,4 +100,31 @@ var _ = When("two virtual clusters are installed", Label(e2eTestLabel), Label(ne
 		Expect(err).To(HaveOccurred())
 		Expect(stdout).To(Not(ContainSubstring("Welcome to nginx!")))
 	})
+
+	It("excludes the real host pod CIDR from the isolation NetworkPolicy, not a hardcoded guess", func(ctx context.Context) {
+		// compute the same value the controller should have derived from the live host
+		// Nodes, so this assertion doesn't rely on the host's real pod CIDR coincidentally
+		// matching a hardcoded constant
+		expectedCIDRs, err := policy.FindPodCIDRs(ctx, k8sClient, "")
+		Expect(err).NotTo(HaveOccurred())
+		Expect(expectedCIDRs).NotTo(BeEmpty())
+
+		for _, vc := range []*VirtualCluster{cluster1, cluster2} {
+			var networkPolicy networkingv1.NetworkPolicy
+
+			key := client.ObjectKey{
+				Name:      k3kcontroller.SafeConcatNameWithPrefix(vc.Cluster.Name),
+				Namespace: vc.Cluster.Namespace,
+			}
+
+			Expect(k8sClient.Get(ctx, key, &networkPolicy)).To(Succeed())
+
+			Expect(networkPolicy.Spec.Egress[0].To).To(ContainElement(networkingv1.NetworkPolicyPeer{
+				IPBlock: &networkingv1.IPBlock{
+					CIDR:   "0.0.0.0/0",
+					Except: expectedCIDRs,
+				},
+			}))
+		}
+	})
 })
@@ -338,6 +338,76 @@ var _ = Context("In a shared cluster", Label(e2eTestLabel), Ordered, func() {
 		})
 	})

+	When("updating the labels of a synced Pod", func() {
+		var virtualPod *corev1.Pod
+
+		BeforeEach(func(ctx context.Context) {
+			p := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "nginx-",
+					Namespace:    "default",
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:  "nginx",
+						Image: "nginx",
+					}},
+				},
+			}
+
+			var err error
+
+			virtualPod, err = virtualCluster.Client.CoreV1().Pods(p.Namespace).Create(ctx, p, metav1.CreateOptions{})
+			Expect(err).To(Not(HaveOccurred()))
+		})
+
+		It("should keep the clusterName isolation label on the host Pod", func(ctx context.Context) {
+			hostPodName := translator.NamespacedName(virtualPod)
+
+			By("Checking the host Pod carries the clusterName isolation label")
+
+			Eventually(func(g Gomega) {
+				hostPod, err := k8s.CoreV1().Pods(hostPodName.Namespace).Get(ctx, hostPodName.Name, metav1.GetOptions{})
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(hostPod.Labels).To(HaveKeyWithValue(translate.ClusterNameLabel, virtualCluster.Cluster.Name))
+			}).
+				WithPolling(time.Second).
+				WithTimeout(time.Minute).
+				Should(Succeed())
+
+			By("Updating a label on the virtual Pod")
+
+			var err error
+
+			virtualPod, err = virtualCluster.Client.CoreV1().Pods(virtualPod.Namespace).Get(ctx, virtualPod.Name, metav1.GetOptions{})
+			Expect(err).NotTo(HaveOccurred())
+
+			if virtualPod.Labels == nil {
+				virtualPod.Labels = map[string]string{}
+			}
+
+			virtualPod.Labels["k3k.io/test"] = "updated"
+
+			virtualPod, err = virtualCluster.Client.CoreV1().Pods(virtualPod.Namespace).Update(ctx, virtualPod, metav1.UpdateOptions{})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Checking the host Pod still carries the clusterName isolation label after the update")
+
+			// The label must survive the update: it is what the isolation NetworkPolicy selects on
+			// (podSelector matchExpressions: clusterName Exists). If it is dropped, the synced
+			// workload pod escapes isolation.
+			Eventually(func(g Gomega) {
+				hostPod, err := k8s.CoreV1().Pods(hostPodName.Namespace).Get(ctx, hostPodName.Name, metav1.GetOptions{})
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(hostPod.Labels).To(HaveKeyWithValue("k3k.io/test", "updated"))
+				g.Expect(hostPod.Labels).To(HaveKeyWithValue(translate.ClusterNameLabel, virtualCluster.Cluster.Name))
+			}).
+				WithPolling(time.Second).
+				WithTimeout(time.Minute).
+				Should(Succeed())
+		})
+	})
+
 	When("creating a Pod with downward API variables in environment variable", func() {
 		var virtualPod *corev1.Pod

@@ -14,6 +14,7 @@ import (
 	networkingv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

+	"github.com/rancher/k3k/k3k-kubelet/translate"
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
 	k3kcontroller "github.com/rancher/k3k/pkg/controller"
 	"github.com/rancher/k3k/pkg/controller/cluster/server"
@@ -104,6 +105,59 @@ var _ = Describe("Cluster Controller", Label("controller"), Label("Cluster"), fu
 				Expect(spec.PolicyTypes).To(ContainElement(networkingv1.PolicyTypeIngress))

 				Expect(spec.Ingress).To(Equal([]networkingv1.NetworkPolicyIngressRule{{}}))
+
+				// the policy should only select synced workload pods, leaving the k3k infra
+				// pods (kubelet, server) unrestricted so they can reach the host API server
+				Expect(spec.PodSelector.MatchExpressions).To(ConsistOf(metav1.LabelSelectorRequirement{
+					Key:      translate.ClusterNameLabel,
+					Operator: metav1.LabelSelectorOpExists,
+				}))
+			})
+
+			When("a host Node advertises a non-default PodCIDR", func() {
+				It("excludes the real PodCIDR from the NetworkPolicy egress, not a hardcoded guess", func() {
+					node := &corev1.Node{
+						ObjectMeta: metav1.ObjectMeta{GenerateName: "node-"},
+						Spec:       corev1.NodeSpec{PodCIDR: "192.168.77.0/24"},
+					}
+
+					Expect(k8sClient.Create(ctx, node)).To(Succeed())
+					DeferCleanup(func() {
+						Expect(k8sClient.Delete(context.Background(), node)).To(Succeed())
+					})
+
+					cluster := &v1beta1.Cluster{
+						ObjectMeta: metav1.ObjectMeta{
+							GenerateName: "cluster-",
+							Namespace:    namespace,
+						},
+					}
+
+					Expect(k8sClient.Create(ctx, cluster)).To(Succeed())
+
+					expectedNetworkPolicy := &networkingv1.NetworkPolicy{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      k3kcontroller.SafeConcatNameWithPrefix(cluster.Name),
+							Namespace: cluster.Namespace,
+						},
+					}
+
+					Eventually(func(g Gomega) {
+						err := k8sClient.Get(ctx, client.ObjectKeyFromObject(expectedNetworkPolicy), expectedNetworkPolicy)
+						g.Expect(err).To(Not(HaveOccurred()))
+
+						egressPeers := expectedNetworkPolicy.Spec.Egress[0].To
+						g.Expect(egressPeers).To(ContainElement(networkingv1.NetworkPolicyPeer{
+							IPBlock: &networkingv1.IPBlock{
+								CIDR:   "0.0.0.0/0",
+								Except: []string{"192.168.77.0/24"},
+							},
+						}))
+					}).
+						WithTimeout(time.Second * 30).
+						WithPolling(time.Second).
+						Should(Succeed())
+				})
 			})

 			When("exposing the cluster with nodePort", func() {
@@ -14,6 +14,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

+	"github.com/rancher/k3k/k3k-kubelet/translate"
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
 	k3kcontroller "github.com/rancher/k3k/pkg/controller"
 	"github.com/rancher/k3k/pkg/controller/policy"
@@ -88,6 +89,13 @@ var _ = Describe("VirtualClusterPolicy Controller", Label("controller"), Label("
 				Expect(spec.PolicyTypes).To(ContainElement(networkingv1.PolicyTypeEgress))
 				Expect(spec.PolicyTypes).To(ContainElement(networkingv1.PolicyTypeIngress))

+				// the policy should only select synced workload pods (which carry the
+				// ClusterNameLabel), leaving the k3k infra pods (kubelet, server) unrestricted
+				Expect(spec.PodSelector.MatchExpressions).To(ConsistOf(metav1.LabelSelectorRequirement{
+					Key:      translate.ClusterNameLabel,
+					Operator: metav1.LabelSelectorOpExists,
+				}))
+
 				// ingress should allow everything
 				Expect(spec.Ingress).To(ConsistOf(networkingv1.NetworkPolicyIngressRule{}))