From d7daa688c9e282cebf4cd5201f0397a340faeb01 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 30 Oct 2024 17:15:52 +0100 Subject: [PATCH] Collect SELinux options only when needed Remove feature gate check from GetPodVolumeNames and collect SELinux options only when it's really needed. --- .../populator/desired_state_of_world_populator.go | 5 ++++- .../desired_state_of_world_populator_test.go | 10 +++++----- pkg/kubelet/volumemanager/volume_manager.go | 2 +- pkg/volume/util/util.go | 12 +++++------- pkg/volume/util/util_test.go | 2 +- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go index 8fa56742f8e..504cbd4a469 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator.go @@ -27,7 +27,9 @@ import ( "sync" "time" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/features" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -299,7 +301,8 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes( } allVolumesAdded := true - mounts, devices, seLinuxContainerContexts := util.GetPodVolumeNames(pod) + collectSELinuxOptions := utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) + mounts, devices, seLinuxContainerContexts := util.GetPodVolumeNames(pod, collectSELinuxOptions) // Process volume spec for each volume defined in pod for _, podVolume := range pod.Spec.Volumes { diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go index 517e2479ee4..11ed7381b68 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go @@ -766,7 +766,7 @@ func TestCreateVolumeSpec_Valid_File_VolumeMounts(t *testing.T) { logger, _ := ktesting.NewTestContext(t) fakePodManager.AddPod(pod) - mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) + mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) _, volumeSpec, _, err := dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) @@ -813,7 +813,7 @@ func TestCreateVolumeSpec_Valid_Nil_VolumeMounts(t *testing.T) { logger, _ := ktesting.NewTestContext(t) fakePodManager.AddPod(pod) - mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) + mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) _, volumeSpec, _, err := dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) @@ -860,7 +860,7 @@ func TestCreateVolumeSpec_Valid_Block_VolumeDevices(t *testing.T) { logger, _ := ktesting.NewTestContext(t) fakePodManager.AddPod(pod) - mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) + mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) _, volumeSpec, _, err := dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) @@ -907,7 +907,7 @@ func TestCreateVolumeSpec_Invalid_File_VolumeDevices(t *testing.T) { logger, _ := ktesting.NewTestContext(t) fakePodManager.AddPod(pod) - mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) + mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) _, volumeSpec, _, err := dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) @@ -954,7 +954,7 @@ func TestCreateVolumeSpec_Invalid_Block_VolumeMounts(t *testing.T) { logger, _ := ktesting.NewTestContext(t) fakePodManager.AddPod(pod) - mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) + mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) _, volumeSpec, _, err := dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) diff --git a/pkg/kubelet/volumemanager/volume_manager.go b/pkg/kubelet/volumemanager/volume_manager.go index 77a06361582..1b210bf546f 100644 --- a/pkg/kubelet/volumemanager/volume_manager.go +++ b/pkg/kubelet/volumemanager/volume_manager.go @@ -581,7 +581,7 @@ func filterUnmountedVolumes(mountedVolumes sets.Set[string], expectedVolumes []s // getExpectedVolumes returns a list of volumes that must be mounted in order to // consider the volume setup step for this pod satisfied. func getExpectedVolumes(pod *v1.Pod) []string { - mounts, devices, _ := util.GetPodVolumeNames(pod) + mounts, devices, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */) return mounts.Union(devices).UnsortedList() } diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 9c50c7d41e7..c5ad14d36f2 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -32,12 +32,10 @@ import ( utypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/api/legacyscheme" podutil "k8s.io/kubernetes/pkg/api/v1/pod" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/securitycontext" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util/types" @@ -512,16 +510,16 @@ func IsLocalEphemeralVolume(volume v1.Volume) bool { } // GetPodVolumeNames returns names of volumes that are used in a pod, -// either as filesystem mount or raw block device, together with list -// of all SELinux contexts of all containers that use the volumes. -func GetPodVolumeNames(pod *v1.Pod) (mounts sets.Set[string], devices sets.Set[string], seLinuxContainerContexts map[string][]*v1.SELinuxOptions) { +// either as filesystem mount or raw block device. +// To save another sweep through containers, SELinux options are optionally collected too. +func GetPodVolumeNames(pod *v1.Pod, collectSELinuxOptions bool) (mounts sets.Set[string], devices sets.Set[string], seLinuxContainerContexts map[string][]*v1.SELinuxOptions) { mounts = sets.New[string]() devices = sets.New[string]() seLinuxContainerContexts = make(map[string][]*v1.SELinuxOptions) podutil.VisitContainers(&pod.Spec, podutil.AllFeatureEnabledContainers(), func(container *v1.Container, containerType podutil.ContainerType) bool { var seLinuxOptions *v1.SELinuxOptions - if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) { + if collectSELinuxOptions { effectiveContainerSecurity := securitycontext.DetermineEffectiveSecurityContext(pod, container) if effectiveContainerSecurity != nil { // No DeepCopy, SELinuxOptions is already a copy of Pod's or container's SELinuxOptions @@ -532,7 +530,7 @@ func GetPodVolumeNames(pod *v1.Pod) (mounts sets.Set[string], devices sets.Set[s if container.VolumeMounts != nil { for _, mount := range container.VolumeMounts { mounts.Insert(mount.Name) - if seLinuxOptions != nil { + if seLinuxOptions != nil && collectSELinuxOptions { seLinuxContainerContexts[mount.Name] = append(seLinuxContainerContexts[mount.Name], seLinuxOptions.DeepCopy()) } } diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index 455283922a6..e0c1a153da3 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -945,7 +945,7 @@ func TestGetPodVolumeNames(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - mounts, devices, contexts := GetPodVolumeNames(test.pod) + mounts, devices, contexts := GetPodVolumeNames(test.pod, true /* collectSELinuxOptions */) if !mounts.Equal(test.expectedMounts) { t.Errorf("Expected mounts: %q, got %q", sets.List[string](mounts), sets.List[string](test.expectedMounts)) }