podsecurityreadinesscontroller: wire down psapi.ParseLevel to classific

Author: ibihim

pkg/operator/podsecurityreadinesscontroller/classification.go

@@ -2,7 +2,6 @@ package podsecurityreadinesscontroller
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
 	securityv1 "github.com/openshift/api/security/v1"
@@ -22,7 +21,12 @@ var (
 	)
 )
 
-func (c *PodSecurityReadinessController) classifyViolatingNamespace(ctx context.Context, conditions *podSecurityOperatorConditions, ns *corev1.Namespace, enforceLevel string) error {
+func (c *PodSecurityReadinessController) classifyViolatingNamespace(
+	ctx context.Context,
+	conditions *podSecurityOperatorConditions,
+	ns *corev1.Namespace,
+	enforceLevel psapi.Level,
+) error {
 	if runLevelZeroNamespaces.Has(ns.Name) {
 		conditions.addViolatingRunLevelZero(ns)
 		return nil
@@ -56,23 +60,11 @@ func (c *PodSecurityReadinessController) classifyViolatingNamespace(ctx context.
 	return nil
 }
 
-func (c *PodSecurityReadinessController) isUserViolation(ctx context.Context, ns *corev1.Namespace, label string) (bool, error) {
-	var enforcementLevel psapi.Level
-	switch strings.ToLower(label) {
-	case "restricted":
-		enforcementLevel = psapi.LevelRestricted
-	case "baseline":
-		enforcementLevel = psapi.LevelBaseline
-	case "privileged":
-		// If privileged is violating, something is seriously wrong
-		// but testing against privileged level is pointless (everything passes)
-		klog.V(2).InfoS("Namespace violating privileged level - skipping user check",
-			"namespace", ns.Name)
-		return false, nil
-	default:
-		return false, fmt.Errorf("unknown level: %q", label)
-	}
-
+func (c *PodSecurityReadinessController) isUserViolation(
+	ctx context.Context,
+	ns *corev1.Namespace,
+	enforcementLevel psapi.Level,
+) (bool, error) {
 	allPods, err := c.kubeClient.CoreV1().Pods(ns.Name).List(ctx, metav1.ListOptions{})
 	if err != nil {
 		klog.V(2).ErrorS(err, "Failed to list pods in namespace", "namespace", ns.Name)
@@ -102,28 +94,14 @@ func (c *PodSecurityReadinessController) isUserViolation(ctx context.Context, ns
 
 	enforcementVersion := psapi.LatestVersion()
 	for _, pod := range userPods {
-		klog.InfoS("Evaluating user pod against PSA level",
-			"namespace", ns.Name, "pod", pod.Name, "level", label,
-			"podSecurityContext", pod.Spec.SecurityContext)
-
 		results := c.psaEvaluator.EvaluatePod(
 			psapi.LevelVersion{Level: enforcementLevel, Version: enforcementVersion},
 			&pod.ObjectMeta,
 			&pod.Spec,
 		)
 
-		klog.InfoS("PSA evaluation results",
-			"namespace", ns.Name, "pod", pod.Name, "level", label,
-			"resultCount", len(results))
-
 		for _, result := range results {
-			klog.InfoS("PSA evaluation result",
-				"namespace", ns.Name, "pod", pod.Name, "level", label,
-				"allowed", result.Allowed, "reason", result.ForbiddenReason,
-				"detail", result.ForbiddenDetail)
 			if !result.Allowed {
-				klog.InfoS("User pod violates PSA level",
-					"namespace", ns.Name, "pod", pod.Name, "level", label)
 				return true, nil
 			}
 		}

pkg/operator/podsecurityreadinesscontroller/classification_test.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
+	psapi "k8s.io/pod-security-admission/api"
 	"k8s.io/pod-security-admission/policy"
 )
 
@@ -68,7 +69,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 		name               string
 		namespace          *corev1.Namespace
 		pods               []corev1.Pod
-		enforceLevel       string
+		enforceLevel       psapi.Level
 		expectedConditions podSecurityOperatorConditions
 		expectError        bool
 	}{
@@ -80,7 +81,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 				},
 			},
 			pods:         []corev1.Pod{},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingRunLevelZeroNamespaces: []string{"kube-system"},
 			},
@@ -94,7 +95,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 				},
 			},
 			pods:         []corev1.Pod{},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingRunLevelZeroNamespaces: []string{"default"},
 			},
@@ -108,7 +109,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 				},
 			},
 			pods:         []corev1.Pod{},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingRunLevelZeroNamespaces: []string{"kube-public"},
 			},
@@ -122,7 +123,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 				},
 			},
 			pods:         []corev1.Pod{},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingOpenShiftNamespaces: []string{"openshift-test"},
 			},
@@ -155,7 +156,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 			pods: []corev1.Pod{
 				newUserSCCPodPrivileged("user-pod", "customer-ns"),
 			},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingUserSCCNamespaces: []string{"customer-ns"},
 			},
@@ -171,7 +172,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 			pods: []corev1.Pod{
 				newUserSCCPodWithPrivilegedContainer("user-scc-violating-pod", "user-scc-violation-test"),
 			},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingUserSCCNamespaces: []string{"user-scc-violation-test"},
 			},
@@ -187,7 +188,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 			pods: []corev1.Pod{
 				newServiceAccountPod("sa-pod", "customer-ns"),
 			},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingCustomerNamespaces: []string{"customer-ns"},
 			},
@@ -221,7 +222,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 			pods: []corev1.Pod{
 				newUserSCCPodRestricted("user-pod", "customer-ns"),
 			},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingCustomerNamespaces: []string{"customer-ns"},
 			},
@@ -235,7 +236,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 				},
 			},
 			pods:         []corev1.Pod{},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingCustomerNamespaces: []string{"customer-ns"},
 			},
@@ -265,7 +266,7 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 					},
 				},
 			},
-			enforceLevel: "restricted",
+			enforceLevel: psapi.LevelRestricted,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingCustomerNamespaces: []string{"customer-ns"},
 			},
@@ -281,24 +282,12 @@ func TestClassifyViolatingNamespace(t *testing.T) {
 			pods: []corev1.Pod{
 				newUserSCCPodPrivileged("user-pod", "customer-ns"),
 			},
-			enforceLevel: "privileged",
+			enforceLevel: psapi.LevelPrivileged,
 			expectedConditions: podSecurityOperatorConditions{
 				violatingCustomerNamespaces: []string{"customer-ns"},
 			},
 			expectError: false,
 		},
-		{
-			name: "invalid PSA level causes error",
-			namespace: &corev1.Namespace{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "customer-ns",
-				},
-			},
-			pods:               []corev1.Pod{},
-			enforceLevel:       "invalid-level",
-			expectedConditions: podSecurityOperatorConditions{},
-			expectError:        true,
-		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			controller, err := createTestController(tt.pods)

pkg/operator/podsecurityreadinesscontroller/violation.go

@@ -23,15 +23,15 @@ var (
 // isNamespaceViolating checks if a namespace is ready for Pod Security Admission enforcement.
 // It returns true if the namespace is violating the Pod Security Admission policy, along with
 // the enforce label it was tested against.
-func (c *PodSecurityReadinessController) isNamespaceViolating(ctx context.Context, ns *corev1.Namespace) (bool, string, error) {
+func (c *PodSecurityReadinessController) isNamespaceViolating(ctx context.Context, ns *corev1.Namespace) (bool, psapi.Level, error) {
 	nsApplyConfig, err := applyconfiguration.ExtractNamespace(ns, syncerControllerName)
 	if err != nil {
 		return false, "", err
 	}
 
 	enforceLabel := determineEnforceLabelForNamespace(nsApplyConfig)
 	nsApply := applyconfiguration.Namespace(ns.Name).WithLabels(map[string]string{
-		psapi.EnforceLevelLabel: enforceLabel,
+		psapi.EnforceLevelLabel: string(enforceLabel),
 	})
 
 	_, err = c.kubeClient.CoreV1().
@@ -53,16 +53,22 @@ func (c *PodSecurityReadinessController) isNamespaceViolating(ctx context.Contex
 	return false, "", nil
 }
 
-func determineEnforceLabelForNamespace(ns *applyconfiguration.NamespaceApplyConfiguration) string {
-	if _, ok := ns.Annotations[securityv1.MinimallySufficientPodSecurityStandard]; ok {
+func determineEnforceLabelForNamespace(ns *applyconfiguration.NamespaceApplyConfiguration) psapi.Level {
+	if pssAnnotation, ok := ns.Annotations[securityv1.MinimallySufficientPodSecurityStandard]; ok {
 		// This should generally exist and will be the only supported method of determining
 		// the enforce level going forward - however, we're keeping the label fallback for
 		// now to account for any workloads not yet annotated using a new enough version of
 		// the syncer, such as during upgrade scenarios.
-		return ns.Annotations[securityv1.MinimallySufficientPodSecurityStandard]
+		level, err := psapi.ParseLevel(pssAnnotation)
+		if err == nil {
+			return level
+		}
+		if err != nil {
+			klog.V(2).InfoS("invalid level in scc annotation", "value", level)
+		}
 	}
 
-	targetLevel := ""
+	var targetLevel psapi.Level
 	for label := range alertLabels {
 		value, ok := ns.Labels[label]
 		if !ok {
@@ -76,18 +82,18 @@ func determineEnforceLabelForNamespace(ns *applyconfiguration.NamespaceApplyConf
 		}
 
 		if targetLevel == "" {
-			targetLevel = value
+			targetLevel = level
 			continue
 		}
 
 		if psapi.CompareLevels(psapi.Level(targetLevel), level) < 0 {
-			targetLevel = value
+			targetLevel = level
 		}
 	}
 
 	if targetLevel == "" {
 		// Global Config will set it to "restricted", but shouldn't happen.
-		return string(psapi.LevelRestricted)
+		return psapi.LevelRestricted
 	}
 
 	return targetLevel