podsecurityreadinesscontroller: wire classification, simplify conditions

Author: ibihim

pkg/operator/podsecurityreadinesscontroller/conditions.go

@@ -3,11 +3,9 @@ package podsecurityreadinesscontroller
 import (
 	"fmt"
 	"sort"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/sets"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
@@ -19,53 +17,45 @@ const (
 	PodSecurityRunLevelZeroType   = "PodSecurityRunLevelZeroEvaluationConditionsDetected"
 	PodSecurityDisabledSyncerType = "PodSecurityDisabledSyncerEvaluationConditionsDetected"
 	PodSecurityInconclusiveType   = "PodSecurityInconclusiveEvaluationConditionsDetected"
+	PodSecurityUserSCCType        = "PodSecurityUserSCCEvaluationConditionsDetected"
 
 	labelSyncControlLabel = "security.openshift.io/scc.podSecurityLabelSync"
 
 	violationReason    = "PSViolationsDetected"
 	inconclusiveReason = "PSViolationDecisionInconclusive"
 )
 
-var (
-	// run-level zero namespaces, shouldn't avoid openshift namespaces
-	runLevelZeroNamespaces = sets.New[string](
-		"default",
-		"kube-system",
-		"kube-public",
-	)
-)
-
 type podSecurityOperatorConditions struct {
 	violatingOpenShiftNamespaces      []string
 	violatingRunLevelZeroNamespaces   []string
 	violatingCustomerNamespaces       []string
 	violatingDisabledSyncerNamespaces []string
+	violatingUserSCCNamespaces        []string
 	inconclusiveNamespaces            []string
 }
 
-func (c *podSecurityOperatorConditions) addViolation(ns *corev1.Namespace) {
-	if runLevelZeroNamespaces.Has(ns.Name) {
-		c.violatingRunLevelZeroNamespaces = append(c.violatingRunLevelZeroNamespaces, ns.Name)
-		return
-	}
+func (c *podSecurityOperatorConditions) addInconclusive(ns *corev1.Namespace) {
+	c.inconclusiveNamespaces = append(c.inconclusiveNamespaces, ns.Name)
+}
 
-	isOpenShift := strings.HasPrefix(ns.Name, "openshift")
-	if isOpenShift {
-		c.violatingOpenShiftNamespaces = append(c.violatingOpenShiftNamespaces, ns.Name)
-		return
-	}
+func (c *podSecurityOperatorConditions) addViolatingRunLevelZero(ns *corev1.Namespace) {
+	c.violatingRunLevelZeroNamespaces = append(c.violatingRunLevelZeroNamespaces, ns.Name)
+}
 
-	if ns.Labels[labelSyncControlLabel] == "false" {
-		// This is the only case in which the controller wouldn't enforce the pod security standards.
-		c.violatingDisabledSyncerNamespaces = append(c.violatingDisabledSyncerNamespaces, ns.Name)
-		return
-	}
+func (c *podSecurityOperatorConditions) addViolatingOpenShift(ns *corev1.Namespace) {
+	c.violatingOpenShiftNamespaces = append(c.violatingOpenShiftNamespaces, ns.Name)
+}
 
+func (c *podSecurityOperatorConditions) addViolatingDisabledSyncer(ns *corev1.Namespace) {
+	c.violatingDisabledSyncerNamespaces = append(c.violatingDisabledSyncerNamespaces, ns.Name)
+}
+
+func (c *podSecurityOperatorConditions) addViolatingCustomer(ns *corev1.Namespace) {
 	c.violatingCustomerNamespaces = append(c.violatingCustomerNamespaces, ns.Name)
 }
 
-func (c *podSecurityOperatorConditions) addInconclusive(ns *corev1.Namespace) {
-	c.inconclusiveNamespaces = append(c.inconclusiveNamespaces, ns.Name)
+func (c *podSecurityOperatorConditions) addViolatingUserSCC(ns *corev1.Namespace) {
+	c.violatingUserSCCNamespaces = append(c.violatingUserSCCNamespaces, ns.Name)
 }
 
 func makeCondition(conditionType, conditionReason string, namespaces []string) operatorv1.OperatorCondition {
@@ -108,6 +98,7 @@ func (c *podSecurityOperatorConditions) toConditionFuncs() []v1helpers.UpdateSta
 		v1helpers.UpdateConditionFn(makeCondition(PodSecurityOpenshiftType, violationReason, c.violatingOpenShiftNamespaces)),
 		v1helpers.UpdateConditionFn(makeCondition(PodSecurityRunLevelZeroType, violationReason, c.violatingRunLevelZeroNamespaces)),
 		v1helpers.UpdateConditionFn(makeCondition(PodSecurityDisabledSyncerType, violationReason, c.violatingDisabledSyncerNamespaces)),
+		v1helpers.UpdateConditionFn(makeCondition(PodSecurityUserSCCType, violationReason, c.violatingUserSCCNamespaces)),
 		v1helpers.UpdateConditionFn(makeCondition(PodSecurityInconclusiveType, inconclusiveReason, c.inconclusiveNamespaces)),
 	}
 }

pkg/operator/podsecurityreadinesscontroller/conditions_test.go

@@ -1,6 +1,7 @@
 package podsecurityreadinesscontroller
 
 import (
+	"strings"
 	"testing"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
@@ -117,6 +118,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -138,6 +140,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionTrue,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -159,6 +162,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -177,6 +181,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionTrue,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -195,6 +200,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionTrue,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -221,6 +227,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionTrue,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -251,6 +258,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionTrue,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionTrue,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 			},
 		},
@@ -270,6 +278,7 @@ func TestOperatorStatus(t *testing.T) {
 				"PodSecurityOpenshiftEvaluationConditionsDetected":      operatorv1.ConditionFalse,
 				"PodSecurityRunLevelZeroEvaluationConditionsDetected":   operatorv1.ConditionFalse,
 				"PodSecurityDisabledSyncerEvaluationConditionsDetected": operatorv1.ConditionFalse,
+				"PodSecurityUserSCCEvaluationConditionsDetected":        operatorv1.ConditionFalse,
 				"PodSecurityInconclusiveEvaluationConditionsDetected":   operatorv1.ConditionTrue,
 			},
 		},
@@ -280,7 +289,17 @@ func TestOperatorStatus(t *testing.T) {
 
 			for _, ns := range tt.namespace {
 				if tt.addViolation {
-					cond.addViolation(ns)
+					// Classify namespace based on the same logic as classifyViolatingNamespace
+					if runLevelZeroNamespaces.Has(ns.Name) {
+						cond.addViolatingRunLevelZero(ns)
+					} else if strings.HasPrefix(ns.Name, "openshift") {
+						cond.addViolatingOpenShift(ns)
+					} else if ns.Labels[labelSyncControlLabel] == "false" {
+						cond.addViolatingDisabledSyncer(ns)
+					} else {
+						// Default to customer violation for test purposes
+						cond.addViolatingCustomer(ns)
+					}
 				}
 				if tt.addInconclusive {
 					cond.addInconclusive(ns)

pkg/operator/podsecurityreadinesscontroller/podsecurityreadinesscontroller_test.go

@@ -315,8 +315,8 @@ func TestPodSecurityViolationController(t *testing.T) {
 				},
 			},
 			expectedViolation:    false,
-			expectedEnforceLabel: "",
-			expectedError:        true,
+			expectedEnforceLabel: "restricted",
+			expectedError:        false,
 		},
 		{
 			name:     "error against inconclusive namespace",
@@ -328,8 +328,8 @@ func TestPodSecurityViolationController(t *testing.T) {
 				},
 			},
 			expectedViolation:    false,
-			expectedEnforceLabel: "",
-			expectedError:        true,
+			expectedEnforceLabel: "restricted",
+			expectedError:        false,
 		},
 	} {
 		tt := tt
@@ -357,7 +357,7 @@ func TestPodSecurityViolationController(t *testing.T) {
 				},
 			}
 
-			isViolating, err := controller.isNamespaceViolating(context.TODO(), tt.namespace)
+			isViolating, _, err := controller.isNamespaceViolating(context.TODO(), tt.namespace)
 			if (err != nil) != tt.expectedError {
 				t.Errorf("expected error %v, got %v", tt.expectedError, err)
 			}