Ignore updates related to Scheduling Gates

to allow the installation of external
operators that manage pod scheduling.
Scheduling Gates don't affect pod privileges,
so there's no need to block them through SCC admission.

Signed-off-by: bmordeha <[email protected]>

Author: Barakmor1

pkg/securitycontextconstraints/sccadmission/admission.go

@@ -81,7 +81,7 @@ func NewConstraint() *constraint {
 // any change that claims the pod is no longer privileged will be removed.  That should hold until
 // we get a true old/new set of objects in.
 func (c *constraint) Admit(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error {
-	if ignore, err := shouldIgnore(a); err != nil {
+	if ignore, err := shouldSkipSCCEvaluation(a); err != nil {
 		return err
 	} else if ignore {
 		return nil
@@ -131,7 +131,7 @@ func (c *constraint) Admit(ctx context.Context, a admission.Attributes, _ admiss
 }
 
 func (c *constraint) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) error {
-	if ignore, err := shouldIgnore(a); err != nil {
+	if ignore, err := shouldSkipSCCEvaluation(a); err != nil {
 		return err
 	} else if ignore {
 		return nil
@@ -420,7 +420,14 @@ var ignoredAnnotations = sets.NewString(
 	"k8s.ovn.org/pod-networks",
 )
 
-func shouldIgnore(a admission.Attributes) (bool, error) {
+// shouldSkipSCCEvaluation skips evaluation for:
+// - non-Pod resources,
+// - specific subresources that don't affect Pod security context (e.g., exec, attach, log),
+// - Windows Pods (SCC is not applied),
+// - update operations that only change fields like SchedulingGates or non-critical metadata.
+// If the request is malformed (e.g., object can't be cast to a Pod), it fails closed to avoid
+// bypassing security enforcement unintentionally.
+func shouldSkipSCCEvaluation(a admission.Attributes) (bool, error) {
 	if a.GetResource().GroupResource() != coreapi.Resource("pods") {
 		return true, nil
 	}
@@ -448,24 +455,27 @@ func shouldIgnore(a admission.Attributes) (bool, error) {
 			return false, admission.NewForbidden(a, fmt.Errorf("object was marked as kind pod but was unable to be converted: %v", a.GetOldObject()))
 		}
 
-		// never ignore any spec changes
-		if !kapihelper.Semantic.DeepEqual(pod.Spec, oldPod.Spec) {
+		// Create deep copies to avoid mutating the original objects
+		podWithoutSchedulingGates := pod.DeepCopy()
+		// Skip SchedulingGates when comparing specs
+		podWithoutSchedulingGates.Spec.SchedulingGates = oldPod.Spec.SchedulingGates
+		if !kapihelper.Semantic.DeepEqual(podWithoutSchedulingGates.Spec, oldPod.Spec) {
 			return false, nil
 		}
 
 		// see if we are only doing meta changes that should be ignored during admission
 		// for example, the OVN controller adds informative networking annotations that shouldn't cause the pod to go through admission again
-		if shouldIgnoreMetaChanges(pod, oldPod) {
+		if shouldIgnoreMetaChanges(podWithoutSchedulingGates, oldPod) {
 			return true, nil
 		}
 	}
 
 	return false, nil
 }
 
-func shouldIgnoreMetaChanges(newPod, oldPod *coreapi.Pod) bool {
+func shouldIgnoreMetaChanges(newPodCopy, oldPod *coreapi.Pod) bool {
 	// check if we're adding or changing only annotations from the ignore list
-	for key, newVal := range newPod.ObjectMeta.Annotations {
+	for key, newVal := range newPodCopy.ObjectMeta.Annotations {
 		if oldVal, ok := oldPod.ObjectMeta.Annotations[key]; ok && newVal == oldVal {
 			continue
 		}
@@ -477,7 +487,7 @@ func shouldIgnoreMetaChanges(newPod, oldPod *coreapi.Pod) bool {
 
 	// check if we're removing only annotations from the ignore list
 	for key := range oldPod.ObjectMeta.Annotations {
-		if _, ok := newPod.ObjectMeta.Annotations[key]; ok {
+		if _, ok := newPodCopy.ObjectMeta.Annotations[key]; ok {
 			continue
 		}
 
@@ -486,12 +496,11 @@ func shouldIgnoreMetaChanges(newPod, oldPod *coreapi.Pod) bool {
 		}
 	}
 
-	newPodCopy := newPod.DeepCopyObject()
-	newPodCopyMeta, err := meta.Accessor(newPodCopy)
+	newPodCopyWithoutSchedulingGatesCopyMeta, err := meta.Accessor(newPodCopy)
 	if err != nil {
 		return false
 	}
-	newPodCopyMeta.SetAnnotations(oldPod.ObjectMeta.Annotations)
+	newPodCopyWithoutSchedulingGatesCopyMeta.SetAnnotations(oldPod.ObjectMeta.Annotations)
 
 	// see if we are only updating the ownerRef. Garbage collection does this
 	// and we should allow it in general, since you had the power to update and the power to delete.

pkg/securitycontextconstraints/sccadmission/admission_test.go

@@ -268,6 +268,15 @@ func TestShouldIgnore(t *testing.T) {
 			shouldIgnore:        true,
 			admissionAttributes: withStatusUpdate(goodPod()),
 		},
+		{
+			description:  "schedulingGates updates should be ignored",
+			shouldIgnore: true,
+			admissionAttributes: withUpdate(schedulingGatePod(), "",
+				func(p *coreapi.Pod) *coreapi.Pod {
+					p.Spec.SchedulingGates = []coreapi.PodSchedulingGate{}
+					return p
+				}),
+		},
 		{
 			description:  "don't ignore normal updates",
 			shouldIgnore: false,
@@ -305,7 +314,7 @@ func TestShouldIgnore(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			ignored, err := shouldIgnore(test.admissionAttributes)
+			ignored, err := shouldSkipSCCEvaluation(test.admissionAttributes)
 			if err != nil {
 				t.Errorf("expected the test to not error but it errored with %v", err)
 			}
@@ -2008,6 +2017,14 @@ func goodPod() *coreapi.Pod {
 	}
 }
 
+// schedulingGatePod is empty pod with scheduling gate. schedulingGates modifications
+// should be safely ignored.
+func schedulingGatePod() *coreapi.Pod {
+	p := goodPod()
+	p.Spec.SchedulingGates = []coreapi.PodSchedulingGate{{Name: "testGate"}}
+	return p
+}
+
 // windowsPod returns windows pod without any SCCs which are specific to Linux. The admission of Windows pod
 // should be safely ignored.
 func windowsPod() *coreapi.Pod {