diff --git a/pkg/controller/volume/selinuxwarning/cache/volumecache.go b/pkg/controller/volume/selinuxwarning/cache/volumecache.go index 4b19c985c86..ebd756584c0 100644 --- a/pkg/controller/volume/selinuxwarning/cache/volumecache.go +++ b/pkg/controller/volume/selinuxwarning/cache/volumecache.go @@ -17,12 +17,15 @@ limitations under the License. package cache import ( + "slices" "sort" "sync" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" + "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/internal/parse" "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/translator" ) @@ -45,8 +48,8 @@ type VolumeCache interface { // change their SELinux support dynamically. GetPodsForCSIDriver(driverName string) []cache.ObjectName - // SendConflicts sends all current conflicts to the given channel. - SendConflicts(logger klog.Logger, ch chan<- Conflict) + // GetConflicts returns the current set of active conflicts (both directions). + GetConflicts(logger klog.Logger) []Conflict } // VolumeCache stores all volumes used by Pods and their properties that the controller needs to track, @@ -56,6 +59,11 @@ type volumeCache struct { seLinuxTranslator *translator.ControllerSELinuxTranslator // All volumes of all existing Pods. volumes map[v1.UniqueVolumeName]usedVolume + // Reverse index: maps each pod to the list of volumes it uses. + // The index is used during pod deletion. + podToVolumes map[cache.ObjectName]sets.Set[v1.UniqueVolumeName] + // Currently active conflicts per volume (both directions, symmetric pairs). + conflicts map[v1.UniqueVolumeName][]Conflict } var _ VolumeCache = &volumeCache{} @@ -65,6 +73,8 @@ func NewVolumeLabelCache(seLinuxTranslator *translator.ControllerSELinuxTranslat return &volumeCache{ seLinuxTranslator: seLinuxTranslator, volumes: make(map[v1.UniqueVolumeName]usedVolume), + podToVolumes: make(map[cache.ObjectName]sets.Set[v1.UniqueVolumeName]), + conflicts: make(map[v1.UniqueVolumeName][]Conflict), } } @@ -81,6 +91,8 @@ type podInfo struct { // SELinux seLinuxLabel to be applied to the volume in the Pod. // Either as mount option or recursively by the container runtime. seLinuxLabel string + // Pre-parsed SELinux label parts for fast conflict detection. + seLinuxParts [4]string // SELinuxChangePolicy of the Pod. changePolicy v1.PodSELinuxChangePolicy } @@ -89,6 +101,7 @@ func newPodInfoListForPod(podKey cache.ObjectName, seLinuxLabel string, changePo return map[cache.ObjectName]podInfo{ podKey: { seLinuxLabel: seLinuxLabel, + seLinuxParts: parse.ParseSELinuxLabel(seLinuxLabel), changePolicy: changePolicy, }, } @@ -110,12 +123,16 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa pods: newPodInfoListForPod(podKey, label, changePolicy), } c.volumes[volumeName] = volume + + // Add to reverse index + c.registerPodVolume(podKey, volumeName) return conflicts } // The volume is already known podInfo := podInfo{ seLinuxLabel: label, + seLinuxParts: parse.ParseSELinuxLabel(label), changePolicy: changePolicy, } oldPodInfo, found := volume.pods[podKey] @@ -128,6 +145,9 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa // Add the updated pod info to the cache volume.pods[podKey] = podInfo + // Add to reverse index + c.registerPodVolume(podKey, volumeName) + // Emit conflicts for the pod for otherPodKey, otherPodInfo := range volume.pods { if otherPodInfo.changePolicy != changePolicy { @@ -147,8 +167,9 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa OtherPod: podKey, OtherPropertyValue: string(changePolicy), }) + } - if c.seLinuxTranslator.Conflicts(otherPodInfo.seLinuxLabel, label) { + if c.seLinuxTranslator.ConflictsParsed(otherPodInfo.seLinuxParts, podInfo.seLinuxParts) { // Send conflict to both pods conflicts = append(conflicts, Conflict{ PropertyName: "SELinuxLabel", @@ -167,6 +188,21 @@ func (c *volumeCache) AddVolume(logger klog.Logger, volumeName v1.UniqueVolumeNa }) } } + // Update the conflict cache for this volume: remove stale conflicts for this pod, then add new ones + volumeConflicts := c.conflicts[volumeName] + updated := make([]Conflict, 0, len(volumeConflicts)) + for _, existing := range volumeConflicts { + if existing.Pod != podKey && existing.OtherPod != podKey { + updated = append(updated, existing) + } + } + updated = append(updated, conflicts...) + if len(updated) == 0 { + delete(c.conflicts, volumeName) + } else { + c.conflicts[volumeName] = updated + } + return conflicts } @@ -176,12 +212,47 @@ func (c *volumeCache) DeletePod(logger klog.Logger, podKey cache.ObjectName) { defer c.mutex.Unlock() defer c.dump(logger) - for volumeName, volume := range c.volumes { + for volumeName := range c.podToVolumes[podKey] { + conflicts, found := c.conflicts[volumeName] + if !found { + continue + } + updated := make([]Conflict, 0, len(conflicts)) + for _, existing := range conflicts { + // preserve other conflicts belonging to volume + if existing.Pod != podKey && existing.OtherPod != podKey { + updated = append(updated, existing) + } + } + if len(updated) == 0 { + delete(c.conflicts, volumeName) + } else { + c.conflicts[volumeName] = updated + } + } + + // Use reverse index to only iterate through volumes this pod actually uses. + for volumeName := range c.podToVolumes[podKey] { + volume, found := c.volumes[volumeName] + if !found { + continue + } delete(volume.pods, podKey) if len(volume.pods) == 0 { delete(c.volumes, volumeName) } } + delete(c.podToVolumes, podKey) +} + +// registerPodVolume adds volumeName to the pod volume index. +// Make sure to hold c.mutex when calling this function. +func (c *volumeCache) registerPodVolume(podKey cache.ObjectName, volumeName v1.UniqueVolumeName) { + if podVolumes, ok := c.podToVolumes[podKey]; ok { + podVolumes.Insert(volumeName) + } else { + c.podToVolumes[podKey] = sets.New(volumeName) + } } func (c *volumeCache) dump(logger klog.Logger) { @@ -215,6 +286,22 @@ func (c *volumeCache) dump(logger klog.Logger) { logger.Info(" pod", "pod", podKey, "seLinuxLabel", podInfo.seLinuxLabel, "changePolicy", podInfo.changePolicy) } } + + // Collect all pods, sort them and print the associated volumes. + podKeys := make([]cache.ObjectName, 0, len(c.podToVolumes)) + for podKey := range c.podToVolumes { + podKeys = append(podKeys, podKey) + } + sort.Slice(podKeys, func(i, j int) bool { + return podKeys[i].String() < podKeys[j].String() + }) + + logger.Info("VolumeCache reverse index dump:") + for _, podKey := range podKeys { + podVolumes := sets.List(c.podToVolumes[podKey]) + slices.Sort(podVolumes) + logger.Info(" pod", "pod", podKey, "volumes", podVolumes) + } } // GetPodsForCSIDriver returns all pods that use volumes with the given CSI driver. @@ -234,42 +321,16 @@ func (c *volumeCache) GetPodsForCSIDriver(driverName string) []cache.ObjectName return pods } -// SendConflicts sends all current conflicts to the given channel. -func (c *volumeCache) SendConflicts(logger klog.Logger, ch chan<- Conflict) { +// GetConflicts returns the current set of active conflicts (both directions, symmetric pairs). +func (c *volumeCache) GetConflicts(logger klog.Logger) []Conflict { c.mutex.RLock() defer c.mutex.RUnlock() logger.V(4).Info("Scraping conflicts") c.dump(logger) - for _, volume := range c.volumes { - // compare pods that use the same volume with each other - for podKey, podInfo := range volume.pods { - for otherPodKey, otherPodInfo := range volume.pods { - if podKey == otherPodKey { - continue - } - // create conflict only for the first pod. The other pod will get the same conflict in its own iteration of `volume.pods` loop. - if podInfo.changePolicy != otherPodInfo.changePolicy { - ch <- Conflict{ - PropertyName: "SELinuxChangePolicy", - EventReason: "SELinuxChangePolicyConflict", - Pod: podKey, - PropertyValue: string(podInfo.changePolicy), - OtherPod: otherPodKey, - OtherPropertyValue: string(otherPodInfo.changePolicy), - } - } - if c.seLinuxTranslator.Conflicts(podInfo.seLinuxLabel, otherPodInfo.seLinuxLabel) { - ch <- Conflict{ - PropertyName: "SELinuxLabel", - EventReason: "SELinuxLabelConflict", - Pod: podKey, - PropertyValue: podInfo.seLinuxLabel, - OtherPod: otherPodKey, - OtherPropertyValue: otherPodInfo.seLinuxLabel, - } - } - } - } + result := sets.New[Conflict]() + for _, volConflicts := range c.conflicts { + result.Insert(volConflicts...) } + return result.UnsortedList() } diff --git a/pkg/controller/volume/selinuxwarning/cache/volumecache_test.go b/pkg/controller/volume/selinuxwarning/cache/volumecache_test.go index 5bba301b692..82fd27e815f 100644 --- a/pkg/controller/volume/selinuxwarning/cache/volumecache_test.go +++ b/pkg/controller/volume/selinuxwarning/cache/volumecache_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "k8s.io/klog/v2/ktesting" + "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/internal/parse" "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/translator" ) @@ -45,6 +46,39 @@ func sortConflicts(conflicts []Conflict) { }) } +// verifyReverseIndexConsistency checks that forward and reverse indexes are symmetric +func verifyReverseIndexConsistency(t *testing.T, c *volumeCache) { + t.Helper() + + // For every (pod, volume) in reverse index, verify it exists in forward index. + for podKey, volumes := range c.podToVolumes { + for volumeName := range volumes { + volume, found := c.volumes[volumeName] + if !found { + t.Errorf("Reverse index has pod %s -> volume %s, but volume not in forward index", podKey, volumeName) + continue + } + if _, found := volume.pods[podKey]; !found { + t.Errorf("Reverse index has pod %s -> volume %s, but pod not in volume's pod list", podKey, volumeName) + } + } + } + + // For every (volume, pod) in forward index, verify it exists in reverse index. + for volumeName, volume := range c.volumes { + for podKey := range volume.pods { + podVolumes, found := c.podToVolumes[podKey] + if !found { + t.Errorf("Forward index has volume %s -> pod %s, but pod not in reverse index", volumeName, podKey) + continue + } + if _, found := podVolumes[volumeName]; !found { + t.Errorf("Forward index has volume %s -> pod %s, but volume not in pod's volume list", volumeName, podKey) + } + } + } +} + // Delete all items in a bigger cache and check it's empty func TestVolumeCache_DeleteAll(t *testing.T) { var podsToDelete []cache.ObjectName @@ -69,6 +103,8 @@ func TestVolumeCache_DeleteAll(t *testing.T) { t.Log("Before deleting all pods:") c.dump(dumpLogger) + verifyReverseIndexConsistency(t, c) + // Act: delete all pods for _, podKey := range podsToDelete { c.DeletePod(logger, podKey) @@ -79,6 +115,12 @@ func TestVolumeCache_DeleteAll(t *testing.T) { t.Errorf("Expected cache to be empty, got %d volumes", len(c.volumes)) c.dump(dumpLogger) } + + // Assert: the reverse index is also empty + if len(c.podToVolumes) != 0 { + t.Errorf("Expected reverse index to be empty, got %d pods", len(c.podToVolumes)) + } + verifyReverseIndexConsistency(t, c) } type podWithVolume struct { @@ -105,8 +147,8 @@ func addReverseConflict(conflicts []Conflict) []Conflict { return newConflicts } -// Test AddVolume and SendConflicts together, they both provide []conflict with the same data -func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) { +// Test that AddVolume and GetConflicts return the same []conflict data +func TestVolumeCache_AddVolumeGetConflicts(t *testing.T) { existingPods := []podWithVolume{ { podNamespace: "ns1", @@ -436,24 +478,18 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) { } expectedPodInfo := podInfo{ seLinuxLabel: tt.podToAdd.label, + seLinuxParts: parse.ParseSELinuxLabel(tt.podToAdd.label), changePolicy: tt.podToAdd.changePolicy, } if !reflect.DeepEqual(existingInfo, expectedPodInfo) { t.Errorf("pod %s has unexpected info: %+v", podKey, existingInfo) } - // Act again: get the conflicts via SendConflicts - ch := make(chan Conflict) - go func() { - c.SendConflicts(logger, ch) - close(ch) - }() + // Verify reverse index consistency + verifyReverseIndexConsistency(t, c) - // Assert - receivedConflicts := []Conflict{} - for c := range ch { - receivedConflicts = append(receivedConflicts, c) - } + // Verify that GetConflicts returns the same conflicts + receivedConflicts := c.GetConflicts(logger) sortConflicts(receivedConflicts) if !reflect.DeepEqual(receivedConflicts, expectedConflicts) { t.Errorf("SendConflicts returned unexpected conflicts: %+v", receivedConflicts) @@ -463,6 +499,211 @@ func TestVolumeCache_AddVolumeSendConflicts(t *testing.T) { } } +// Test that conflicts are tracked per-volume: a pod with conflicts on +// multiple volumes retains all of them after successive AddVolume calls. +func TestVolumeCache_MultiVolumeConflicts(t *testing.T) { + logger, _ := getTestLoggers(t) + seLinuxTranslator := &translator.ControllerSELinuxTranslator{} + c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache) + + podA := cache.ObjectName{Namespace: "ns", Name: "podA"} + podB := cache.ObjectName{Namespace: "ns", Name: "podB"} + podC := cache.ObjectName{Namespace: "ns", Name: "podC"} + + // podB uses vol1 with label1 + c.AddVolume(logger, "vol1", podB, "system_u:system_r:labelB", v1.SELinuxChangePolicyMountOption, "driver1") + // podC uses vol2 with label2 + c.AddVolume(logger, "vol2", podC, "system_u:system_r:labelC", v1.SELinuxChangePolicyMountOption, "driver1") + + // podA uses vol1 with a different label (conflict with podB) + conflicts1 := c.AddVolume(logger, "vol1", podA, "system_u:system_r:labelA", v1.SELinuxChangePolicyMountOption, "driver1") + if len(conflicts1) == 0 { + t.Fatal("Expected conflicts on vol1 between podA and podB") + } + + // podA also uses vol2 with a different label (conflict with podC) + conflicts2 := c.AddVolume(logger, "vol2", podA, "system_u:system_r:labelA", v1.SELinuxChangePolicyMountOption, "driver1") + if len(conflicts2) == 0 { + t.Fatal("Expected conflicts on vol2 between podA and podC") + } + + // GetConflicts must return conflicts from BOTH volumes + allConflicts := c.GetConflicts(logger) + expectedCount := len(conflicts1) + len(conflicts2) + if len(allConflicts) != expectedCount { + t.Errorf("GetConflicts returned %d conflicts, expected %d (vol1: %d + vol2: %d)", + len(allConflicts), expectedCount, len(conflicts1), len(conflicts2)) + } + + // After deleting podA, all conflicts should be gone + c.DeletePod(logger, podA) + remaining := c.GetConflicts(logger) + if len(remaining) != 0 { + t.Errorf("Expected no conflicts after deleting podA, got %d: %+v", len(remaining), remaining) + } + + // Verify deduplication: podD and podE conflict on two volumes with the same labels. + // Identical Conflict entries from different volumes must be deduplicated by GetConflicts. + podD := cache.ObjectName{Namespace: "ns", Name: "podD"} + podE := cache.ObjectName{Namespace: "ns", Name: "podE"} + + c.AddVolume(logger, "vol3", podD, "system_u:system_r:labelD", v1.SELinuxChangePolicyMountOption, "driver1") + c.AddVolume(logger, "vol4", podD, "system_u:system_r:labelD", v1.SELinuxChangePolicyMountOption, "driver1") + + conflictsVol3 := c.AddVolume(logger, "vol3", podE, "system_u:system_r:labelE", v1.SELinuxChangePolicyMountOption, "driver1") + conflictsVol4 := c.AddVolume(logger, "vol4", podE, "system_u:system_r:labelE", v1.SELinuxChangePolicyMountOption, "driver1") + + if len(conflictsVol3) != len(conflictsVol4) { + t.Fatalf("Expected same number of conflicts from vol3 and vol4 (%d vs %d)", len(conflictsVol3), len(conflictsVol4)) + } + if len(conflictsVol3) == 0 { + t.Fatal("Expected conflicts between podD and podE") + } + + allConflicts = c.GetConflicts(logger) + deCount := 0 + for _, conflict := range allConflicts { + if conflict.Pod == podD || conflict.Pod == podE || conflict.OtherPod == podD || conflict.OtherPod == podE { + deCount++ + } + } + if deCount != len(conflictsVol3) { + t.Errorf("Expected %d deduplicated conflicts for podD/podE (from 2 volumes), got %d", len(conflictsVol3), deCount) + } +} + +func TestVolumeCache_DeletePodConflicts(t *testing.T) { + podA := cache.ObjectName{Namespace: "ns", Name: "podA"} + podB := cache.ObjectName{Namespace: "ns", Name: "podB"} + podC := cache.ObjectName{Namespace: "ns", Name: "podC"} + podD := cache.ObjectName{Namespace: "ns", Name: "podD"} + + tests := []struct { + name string + // Pods to add before deletion. + initialPods []podWithVolume + // Pod to delete. + podToDelete cache.ObjectName + // If true, delete the pod a second time to verify idempotency. + deleteTwice bool + // Pod pairs that must still have symmetric conflicts after deletion. + // Each pair [2]cache.ObjectName expects both (A→B) and (B→A) to be present. + expectedSurvivingPairs [][2]cache.ObjectName + }{ + { + name: "delete one of two conflicting pods clears all conflicts", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podA, + expectedSurvivingPairs: nil, + }, + { + name: "delete non-conflicting pod preserves existing conflicts", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podC, + expectedSurvivingPairs: [][2]cache.ObjectName{{podA, podB}}, + }, + { + name: "three pods on same volume delete one leaves remaining pair conflict", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podC", volumeName: "vol1", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podA, + expectedSurvivingPairs: [][2]cache.ObjectName{{podB, podC}}, + }, + { + name: "delete pod with conflicts on multiple volumes", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podA", volumeName: "vol2", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podA, + expectedSurvivingPairs: nil, + }, + { + name: "delete pod preserves conflicts on unrelated volumes", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podC", volumeName: "vol2", label: "system_u:system_r:labelC", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podD", volumeName: "vol2", label: "system_u:system_r:labelD", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podA, + expectedSurvivingPairs: [][2]cache.ObjectName{{podC, podD}}, + }, + { + name: "delete pod that was already deleted is a no-op", + initialPods: []podWithVolume{ + {podNamespace: "ns", podName: "podA", volumeName: "vol1", label: "system_u:system_r:labelA", changePolicy: v1.SELinuxChangePolicyMountOption}, + {podNamespace: "ns", podName: "podB", volumeName: "vol1", label: "system_u:system_r:labelB", changePolicy: v1.SELinuxChangePolicyMountOption}, + }, + podToDelete: podA, + deleteTwice: true, + expectedSurvivingPairs: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logger, _ := getTestLoggers(t) + seLinuxTranslator := &translator.ControllerSELinuxTranslator{} + c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache) + + for _, pod := range tt.initialPods { + c.AddVolume(logger, pod.volumeName, cache.ObjectName{Namespace: pod.podNamespace, Name: pod.podName}, pod.label, pod.changePolicy, "driver1") + } + + c.DeletePod(logger, tt.podToDelete) + if tt.deleteTwice { + c.DeletePod(logger, tt.podToDelete) + } + + remaining := c.GetConflicts(logger) + + // Deleted pod must not appear in any conflict + for _, conflict := range remaining { + if conflict.Pod == tt.podToDelete || conflict.OtherPod == tt.podToDelete { + t.Errorf("found conflict involving deleted pod %s: %+v", tt.podToDelete, conflict) + } + } + + // Verify each expected surviving pair exists in both directions + for _, pair := range tt.expectedSurvivingPairs { + hasForward := false + hasReverse := false + for _, conflict := range remaining { + if conflict.Pod == pair[0] && conflict.OtherPod == pair[1] { + hasForward = true + } + if conflict.Pod == pair[1] && conflict.OtherPod == pair[0] { + hasReverse = true + } + } + if !hasForward || !hasReverse { + t.Errorf("expected symmetric conflict between %s and %s, got %+v", pair[0], pair[1], remaining) + } + } + + // If no pairs are expected, there should be no conflicts at all + if len(tt.expectedSurvivingPairs) == 0 && len(remaining) != 0 { + t.Errorf("expected no conflicts, got %+v", remaining) + } + + verifyReverseIndexConsistency(t, c) + }) + } +} + func TestVolumeCache_GetPodsForCSIDriver(t *testing.T) { seLinuxTranslator := &translator.ControllerSELinuxTranslator{} c := NewVolumeLabelCache(seLinuxTranslator).(*volumeCache) diff --git a/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label.go b/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label.go new file mode 100644 index 00000000000..0fd48ed8b67 --- /dev/null +++ b/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label.go @@ -0,0 +1,32 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parse + +import "strings" + +// ParseSELinuxLabel parses a SELinux label string into its components. +// Format: "user:role:type:level" -> [user, role, type, level] +// Missing components are represented as empty strings. +func ParseSELinuxLabel(label string) [4]string { + var parts [4]string + if label == "" { + return parts + } + split := strings.SplitN(label, ":", 4) + copy(parts[:], split) + return parts +} diff --git a/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label_test.go b/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label_test.go new file mode 100644 index 00000000000..e82feed748f --- /dev/null +++ b/pkg/controller/volume/selinuxwarning/internal/parse/selinux_label_test.go @@ -0,0 +1,106 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package parse + +import ( + "reflect" + "testing" +) + +func TestParseSELinuxLabel(t *testing.T) { + tests := []struct { + name string + label string + expectedParts []string + }{ + { + name: "empty label", + label: "", + expectedParts: []string{"", "", "", ""}, + }, + { + name: "complete label with all components", + label: "system_u:system_r:container_t:s0:c0,c1", + expectedParts: []string{"system_u", "system_r", "container_t", "s0:c0,c1"}, + }, + { + name: "label with user, role, and type only", + label: "system_u:system_r:container_t", + expectedParts: []string{"system_u", "system_r", "container_t", ""}, + }, + { + name: "label with user and role only", + label: "system_u:system_r", + expectedParts: []string{"system_u", "system_r", "", ""}, + }, + { + name: "label with user only", + label: "system_u", + expectedParts: []string{"system_u", "", "", ""}, + }, + { + name: "label missing user but with role and type", + label: ":system_r:container_t", + expectedParts: []string{"", "system_r", "container_t", ""}, + }, + { + name: "label missing user and role but with type", + label: "::container_t", + expectedParts: []string{"", "", "container_t", ""}, + }, + { + name: "label missing user and role but with type and level", + label: "::container_t:s0", + expectedParts: []string{"", "", "container_t", "s0"}, + }, + { + name: "label with all empty components except level", + label: ":::s0:c0,c1", + expectedParts: []string{"", "", "", "s0:c0,c1"}, + }, + { + name: "label with special characters in components", + label: "user_with_underscore:role-with-dash:type.with.dots:s0:c0.c1", + expectedParts: []string{"user_with_underscore", "role-with-dash", "type.with.dots", "s0:c0.c1"}, + }, + { + name: "label with extra colons in level component", + label: "user:role:type:s0:c0,c1:extra", + expectedParts: []string{"user", "role", "type", "s0:c0,c1:extra"}, + }, + { + name: "multiple colons only", + label: ":::", + expectedParts: []string{"", "", "", ""}, + }, + { + name: "five colons", + label: ":::::", + expectedParts: []string{"", "", "", "::"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parts := ParseSELinuxLabel(tt.label) + partsSlice := parts[:] + if !reflect.DeepEqual(partsSlice, tt.expectedParts) { + t.Errorf("ParseSELinuxLabel(%q) = %v, expected parts = %v", tt.label, partsSlice, tt.expectedParts) + } + }) + } +} diff --git a/pkg/controller/volume/selinuxwarning/metrics.go b/pkg/controller/volume/selinuxwarning/metrics.go index d95665c9162..c285bd78db4 100644 --- a/pkg/controller/volume/selinuxwarning/metrics.go +++ b/pkg/controller/volume/selinuxwarning/metrics.go @@ -59,13 +59,7 @@ func (c *collector) DescribeWithStability(ch chan<- *metrics.Desc) { } func (c *collector) CollectWithStability(ch chan<- metrics.Metric) { - conflictCh := make(chan cache.Conflict) - go func() { - c.cache.SendConflicts(c.logger, conflictCh) - close(conflictCh) - }() - - for conflict := range conflictCh { + for _, conflict := range c.cache.GetConflicts(c.logger) { ch <- metrics.NewLazyConstMetric(seLinuxConflictDesc, metrics.GaugeValue, 1.0, diff --git a/pkg/controller/volume/selinuxwarning/selinux_warning_controller_test.go b/pkg/controller/volume/selinuxwarning/selinux_warning_controller_test.go index 9d9998bc62a..1d2e66f4e40 100644 --- a/pkg/controller/volume/selinuxwarning/selinux_warning_controller_test.go +++ b/pkg/controller/volume/selinuxwarning/selinux_warning_controller_test.go @@ -783,12 +783,12 @@ func (f *fakeVolumeCache) GetPodsForCSIDriver(driverName string) []cache.ObjectN return pods } -func (f *fakeVolumeCache) SendConflicts(logger klog.Logger, ch chan<- volumecache.Conflict) { +func (f *fakeVolumeCache) GetConflicts(logger klog.Logger) []volumecache.Conflict { + result := make([]volumecache.Conflict, 0) for _, conflicts := range f.conflictsToSend { - for _, conflict := range conflicts { - ch <- conflict - } + result = append(result, conflicts...) } + return result } func collectEvents(source <-chan string) []string { diff --git a/pkg/controller/volume/selinuxwarning/translator/selinux_translator.go b/pkg/controller/volume/selinuxwarning/translator/selinux_translator.go index 99ce3e97dd7..db599c98cd7 100644 --- a/pkg/controller/volume/selinuxwarning/translator/selinux_translator.go +++ b/pkg/controller/volume/selinuxwarning/translator/selinux_translator.go @@ -20,6 +20,7 @@ import ( "strings" v1 "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/controller/volume/selinuxwarning/internal/parse" "k8s.io/kubernetes/pkg/volume/util" ) @@ -70,18 +71,16 @@ func (c *ControllerSELinuxTranslator) SELinuxOptionsToFileLabel(opts *v1.SELinux // However: "system_u:system_r:container_t:s0:c1,c2" *does* conflict with ":::s0:c98,c99". // And ":::s0:c1,c2" *does* conflict with "" or ":::", because it's never defaulted by the OS. func (c *ControllerSELinuxTranslator) Conflicts(labelA, labelB string) bool { - partsA := strings.SplitN(labelA, ":", 4) - partsB := strings.SplitN(labelB, ":", 4) + return c.ConflictsParsed(parse.ParseSELinuxLabel(labelA), parse.ParseSELinuxLabel(labelB)) +} - // Reorder, so partsA is always longer than partsB - if len(partsA) < len(partsB) { - partsB, partsA = partsA, partsB - } - - for len(partsB) < len(partsA) { - partsB = append(partsB, "") - } - for i := range partsA { +// ConflictsParsed returns true if two pre-parsed SELinux labels conflict. +// This is an optimized version of Conflicts() that operates on pre-split labels +// to avoid repeated string allocations in hot paths (e.g., metrics collection). +// partsA and partsB must be 4-element arrays in the format: [user, role, type, level] +func (c *ControllerSELinuxTranslator) ConflictsParsed(partsA, partsB [4]string) bool { + // Compare each component + for i := range 4 { if partsA[i] == partsB[i] { continue }