Collect SELinux options only when needed

Remove feature gate check from GetPodVolumeNames and collect SELinux
options only when it's really needed.
This commit is contained in:
Jan Safranek 2024-10-30 17:15:52 +01:00
parent 6e4504685f
commit d7daa688c9
5 changed files with 16 additions and 15 deletions

View File

@ -27,7 +27,9 @@ import (
"sync" "sync"
"time" "time"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/features"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -299,7 +301,8 @@ func (dswp *desiredStateOfWorldPopulator) processPodVolumes(
} }
allVolumesAdded := true 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 // Process volume spec for each volume defined in pod
for _, podVolume := range pod.Spec.Volumes { for _, podVolume := range pod.Spec.Volumes {

View File

@ -766,7 +766,7 @@ func TestCreateVolumeSpec_Valid_File_VolumeMounts(t *testing.T) {
logger, _ := ktesting.NewTestContext(t) logger, _ := ktesting.NewTestContext(t)
fakePodManager.AddPod(pod) fakePodManager.AddPod(pod)
mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
_, volumeSpec, _, err := _, volumeSpec, _, err :=
dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) 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) logger, _ := ktesting.NewTestContext(t)
fakePodManager.AddPod(pod) fakePodManager.AddPod(pod)
mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
_, volumeSpec, _, err := _, volumeSpec, _, err :=
dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) 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) logger, _ := ktesting.NewTestContext(t)
fakePodManager.AddPod(pod) fakePodManager.AddPod(pod)
mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
_, volumeSpec, _, err := _, volumeSpec, _, err :=
dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) 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) logger, _ := ktesting.NewTestContext(t)
fakePodManager.AddPod(pod) fakePodManager.AddPod(pod)
mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
_, volumeSpec, _, err := _, volumeSpec, _, err :=
dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) 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) logger, _ := ktesting.NewTestContext(t)
fakePodManager.AddPod(pod) fakePodManager.AddPod(pod)
mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod) mountsMap, devicesMap, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
_, volumeSpec, _, err := _, volumeSpec, _, err :=
dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap) dswp.createVolumeSpec(logger, pod.Spec.Volumes[0], pod, mountsMap, devicesMap)

View File

@ -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 // getExpectedVolumes returns a list of volumes that must be mounted in order to
// consider the volume setup step for this pod satisfied. // consider the volume setup step for this pod satisfied.
func getExpectedVolumes(pod *v1.Pod) []string { func getExpectedVolumes(pod *v1.Pod) []string {
mounts, devices, _ := util.GetPodVolumeNames(pod) mounts, devices, _ := util.GetPodVolumeNames(pod, false /* collectSELinuxOptions */)
return mounts.Union(devices).UnsortedList() return mounts.Union(devices).UnsortedList()
} }

View File

@ -32,12 +32,10 @@ import (
utypes "k8s.io/apimachinery/pkg/types" utypes "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/util/wait"
utilfeature "k8s.io/apiserver/pkg/util/feature"
clientset "k8s.io/client-go/kubernetes" clientset "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/legacyscheme"
podutil "k8s.io/kubernetes/pkg/api/v1/pod" podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/securitycontext" "k8s.io/kubernetes/pkg/securitycontext"
"k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume"
"k8s.io/kubernetes/pkg/volume/util/types" "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, // GetPodVolumeNames returns names of volumes that are used in a pod,
// either as filesystem mount or raw block device, together with list // either as filesystem mount or raw block device.
// of all SELinux contexts of all containers that use the volumes. // To save another sweep through containers, SELinux options are optionally collected too.
func GetPodVolumeNames(pod *v1.Pod) (mounts sets.Set[string], devices sets.Set[string], seLinuxContainerContexts map[string][]*v1.SELinuxOptions) { func GetPodVolumeNames(pod *v1.Pod, collectSELinuxOptions bool) (mounts sets.Set[string], devices sets.Set[string], seLinuxContainerContexts map[string][]*v1.SELinuxOptions) {
mounts = sets.New[string]() mounts = sets.New[string]()
devices = sets.New[string]() devices = sets.New[string]()
seLinuxContainerContexts = make(map[string][]*v1.SELinuxOptions) seLinuxContainerContexts = make(map[string][]*v1.SELinuxOptions)
podutil.VisitContainers(&pod.Spec, podutil.AllFeatureEnabledContainers(), func(container *v1.Container, containerType podutil.ContainerType) bool { podutil.VisitContainers(&pod.Spec, podutil.AllFeatureEnabledContainers(), func(container *v1.Container, containerType podutil.ContainerType) bool {
var seLinuxOptions *v1.SELinuxOptions var seLinuxOptions *v1.SELinuxOptions
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) { if collectSELinuxOptions {
effectiveContainerSecurity := securitycontext.DetermineEffectiveSecurityContext(pod, container) effectiveContainerSecurity := securitycontext.DetermineEffectiveSecurityContext(pod, container)
if effectiveContainerSecurity != nil { if effectiveContainerSecurity != nil {
// No DeepCopy, SELinuxOptions is already a copy of Pod's or container's SELinuxOptions // 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 { if container.VolumeMounts != nil {
for _, mount := range container.VolumeMounts { for _, mount := range container.VolumeMounts {
mounts.Insert(mount.Name) mounts.Insert(mount.Name)
if seLinuxOptions != nil { if seLinuxOptions != nil && collectSELinuxOptions {
seLinuxContainerContexts[mount.Name] = append(seLinuxContainerContexts[mount.Name], seLinuxOptions.DeepCopy()) seLinuxContainerContexts[mount.Name] = append(seLinuxContainerContexts[mount.Name], seLinuxOptions.DeepCopy())
} }
} }

View File

@ -945,7 +945,7 @@ func TestGetPodVolumeNames(t *testing.T) {
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { 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) { if !mounts.Equal(test.expectedMounts) {
t.Errorf("Expected mounts: %q, got %q", sets.List[string](mounts), sets.List[string](test.expectedMounts)) t.Errorf("Expected mounts: %q, got %q", sets.List[string](mounts), sets.List[string](test.expectedMounts))
} }