From e1ae07c8366fb6f3afbd623dd2c3b0651d0fff5c Mon Sep 17 00:00:00 2001 From: Enrico Candino Date: Thu, 16 Jul 2026 15:18:04 +0200 Subject: [PATCH] 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 * Use Ginkgo provided context * Refactor FindPodCIDRs to use sets for CIDR collection and simplify logic * fix lint --------- Co-authored-by: Kevin McDermott --- pkg/controller/cluster/cluster.go | 17 ++- pkg/controller/policy/networkpolicy.go | 78 ++++++++++---- pkg/controller/policy/networkpolicy_test.go | 114 ++++++++++++++++++++ tests/e2e/cluster_network_test.go | 35 ++++++ tests/e2e/cluster_pod_test.go | 70 ++++++++++++ tests/integration/cluster/cluster_test.go | 54 ++++++++++ tests/integration/policy/policy_test.go | 8 ++ 7 files changed, 357 insertions(+), 19 deletions(-) create mode 100644 pkg/controller/policy/networkpolicy_test.go diff --git a/pkg/controller/cluster/cluster.go b/pkg/controller/cluster/cluster.go index 0cef7b6b..066b51e7 100644 --- a/pkg/controller/cluster/cluster.go +++ b/pkg/controller/cluster/cluster.go @@ -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, }, }, }, diff --git a/pkg/controller/policy/networkpolicy.go b/pkg/controller/policy/networkpolicy.go index ad39c63c..cdc0b59a 100644 --- a/pkg/controller/policy/networkpolicy.go +++ b/pkg/controller/policy/networkpolicy.go @@ -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, diff --git a/pkg/controller/policy/networkpolicy_test.go b/pkg/controller/policy/networkpolicy_test.go new file mode 100644 index 00000000..f9128aaf --- /dev/null +++ b/pkg/controller/policy/networkpolicy_test.go @@ -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() +} diff --git a/tests/e2e/cluster_network_test.go b/tests/e2e/cluster_network_test.go index 0f2d0a84..2570aff6 100644 --- a/tests/e2e/cluster_network_test.go +++ b/tests/e2e/cluster_network_test.go @@ -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, + }, + })) + } + }) }) diff --git a/tests/e2e/cluster_pod_test.go b/tests/e2e/cluster_pod_test.go index 46a64d99..25dd1332 100644 --- a/tests/e2e/cluster_pod_test.go +++ b/tests/e2e/cluster_pod_test.go @@ -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 diff --git a/tests/integration/cluster/cluster_test.go b/tests/integration/cluster/cluster_test.go index 059e462c..a8eb0222 100644 --- a/tests/integration/cluster/cluster_test.go +++ b/tests/integration/cluster/cluster_test.go @@ -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() { diff --git a/tests/integration/policy/policy_test.go b/tests/integration/policy/policy_test.go index f8cf3a2a..87c8ff57 100644 --- a/tests/integration/policy/policy_test.go +++ b/tests/integration/policy/policy_test.go @@ -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{}))