From 48ea6a3f3a74ea1fd9a356918f63a135cad87b60 Mon Sep 17 00:00:00 2001
From: Jan Safranek <jsafrane@redhat.com>
Date: Thu, 9 Mar 2023 14:15:58 +0100
Subject: [PATCH] Fix SELinux mismatch metrics

DesiredStateOfWorld must remember both
- the effective SELinux label to apply as a mount option (non-empty for
  RWOP volumes, empty otherwise)
- and the label that _would_ be used if the mount option would be used by
  all access modes.

Mismatch warning metrics must be generated from the second label.
---
 .../cache/desired_state_of_world.go           | 47 ++++++++++++-------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go
index 0d8faf32351..dae89ab401f 100644
--- a/pkg/kubelet/volumemanager/cache/desired_state_of_world.go
+++ b/pkg/kubelet/volumemanager/cache/desired_state_of_world.go
@@ -206,12 +206,23 @@ type volumeToMount struct {
 	// Usually this value reflects size recorded in pv.Spec.Capacity
 	persistentVolumeSize *resource.Quantity
 
-	// seLinuxFileLabel is desired SELinux label on files on the volume. If empty, then
+	// effectiveSELinuxMountFileLabel is the SELinux label that will be applied to the volume using mount options.
+	// If empty, then:
+	// - either the context+label is unknown (assigned randomly by the container runtime)
+	// - or the volume plugin responsible for this volume does not support mounting with -o context
+	// - or the volume is not ReadWriteOncePod
+	// - or the OS does not support SELinux
+	// In all cases, the SELinux context does not matter when mounting the volume.
+	effectiveSELinuxMountFileLabel string
+
+	// originalSELinuxLabel is the SELinux label that would be used if SELinux mount was supported for all access modes.
+	// For RWOP volumes it's the same as effectiveSELinuxMountFileLabel.
+	// It is used only to report potential SELinux mismatch metrics.
+	// If empty, then:
 	// - either the context+label is unknown (assigned randomly by the container runtime)
 	// - or the volume plugin responsible for this volume does not support mounting with -o context
 	// - or the OS does not support SELinux
-	// In all cases, the SELinux context does not matter when mounting the volume.
-	seLinuxFileLabel string
+	originalSELinuxLabel string
 }
 
 // The pod object represents a pod that references the underlying volume and
@@ -307,23 +318,25 @@ func (dsw *desiredStateOfWorld) AddPodToVolume(
 				}
 			}
 		}
+		effectiveSELinuxMountLabel := seLinuxFileLabel
 		if !util.VolumeSupportsSELinuxMount(volumeSpec) {
 			// Clear SELinux label for the volume with unsupported access modes.
 			klog.V(4).InfoS("volume does not support SELinux context mount, clearing the expected label", "volume", volumeSpec.Name())
-			seLinuxFileLabel = ""
+			effectiveSELinuxMountLabel = ""
 		}
 		if seLinuxFileLabel != "" {
 			seLinuxVolumesAdmitted.Add(1.0)
 		}
 		vmt := volumeToMount{
-			volumeName:              volumeName,
-			podsToMount:             make(map[types.UniquePodName]podToMount),
-			pluginIsAttachable:      attachable,
-			pluginIsDeviceMountable: deviceMountable,
-			volumeGidValue:          volumeGidValue,
-			reportedInUse:           false,
-			desiredSizeLimit:        sizeLimit,
-			seLinuxFileLabel:        seLinuxFileLabel,
+			volumeName:                     volumeName,
+			podsToMount:                    make(map[types.UniquePodName]podToMount),
+			pluginIsAttachable:             attachable,
+			pluginIsDeviceMountable:        deviceMountable,
+			volumeGidValue:                 volumeGidValue,
+			reportedInUse:                  false,
+			desiredSizeLimit:               sizeLimit,
+			effectiveSELinuxMountFileLabel: effectiveSELinuxMountLabel,
+			originalSELinuxLabel:           seLinuxFileLabel,
 		}
 		// record desired size of the volume
 		if volumeSpec.PersistentVolume != nil {
@@ -337,9 +350,9 @@ func (dsw *desiredStateOfWorld) AddPodToVolume(
 	} else {
 		// volume exists
 		if pluginSupportsSELinuxContextMount {
-			if seLinuxFileLabel != vol.seLinuxFileLabel {
+			if seLinuxFileLabel != vol.originalSELinuxLabel {
 				// TODO: update the error message after tests, e.g. add at least the conflicting pod names.
-				fullErr := fmt.Errorf("conflicting SELinux labels of volume %s: %q and %q", volumeSpec.Name(), vol.seLinuxFileLabel, seLinuxFileLabel)
+				fullErr := fmt.Errorf("conflicting SELinux labels of volume %s: %q and %q", volumeSpec.Name(), vol.originalSELinuxLabel, seLinuxFileLabel)
 				supported := util.VolumeSupportsSELinuxMount(volumeSpec)
 				if err := handleSELinuxMetricError(fullErr, supported, seLinuxVolumeContextMismatchWarnings, seLinuxVolumeContextMismatchErrors); err != nil {
 					return "", err
@@ -499,7 +512,7 @@ func (dsw *desiredStateOfWorld) VolumeExists(
 		// and mounted with new SELinux mount options for pod B.
 		// Without SELinux, kubelet can (and often does) reuse device mounted
 		// for A.
-		return vol.seLinuxFileLabel == seLinuxMountContext
+		return vol.effectiveSELinuxMountFileLabel == seLinuxMountContext
 	}
 	return true
 }
@@ -515,7 +528,7 @@ func (dsw *desiredStateOfWorld) PodExistsInVolume(
 	}
 
 	if feature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
-		if volumeObj.seLinuxFileLabel != seLinuxMountOption {
+		if volumeObj.effectiveSELinuxMountFileLabel != seLinuxMountOption {
 			// The volume is in DSW, but with a different SELinux mount option.
 			// Report it as unused, so the volume is unmounted and mounted back
 			// with the right SELinux option.
@@ -573,7 +586,7 @@ func (dsw *desiredStateOfWorld) GetVolumesToMount() []VolumeToMount {
 					ReportedInUse:           volumeObj.reportedInUse,
 					MountRequestTime:        podObj.mountRequestTime,
 					DesiredSizeLimit:        volumeObj.desiredSizeLimit,
-					SELinuxLabel:            volumeObj.seLinuxFileLabel,
+					SELinuxLabel:            volumeObj.effectiveSELinuxMountFileLabel,
 				},
 			}
 			if volumeObj.persistentVolumeSize != nil {