Merge pull request openshift#29135 from kramaranya/OCPBUGS-42435

OCPBUGS-42435: Enforce the required-scc monitor test and validate usage of non-standard OCP SCCs
deads2k · Nov 23, 2024 · 19cf936 · 19cf936
2 parents f65121e + a31bc91
commit 19cf936
Showing 1 changed file with 69 additions and 23 deletions.
diff --git a/pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go b/pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go
@@ -31,6 +31,24 @@ var defaultSCCs = sets.NewString(
 	"restricted-v2",
 )
 
+var nonStandardSCCNamespaces = map[string]sets.Set[string]{
+	"node-exporter":                   sets.New("openshift-monitoring"),
+	"machine-api-termination-handler": sets.New("openshift-machine-api"),
+}
+
+var namespacesWithPendingSCCPinning = sets.NewString(
+	"openshift-cluster-csi-drivers",
+	"openshift-cluster-version",
+	"openshift-image-registry",
+	"openshift-ingress",
+	"openshift-ingress-canary",
+	"openshift-ingress-operator",
+	"openshift-insights",
+	"openshift-machine-api",
+	"openshift-marketplace",
+	"openshift-monitoring",
+)
+
 type requiredSCCAnnotationChecker struct {
 	kubeClient kubernetes.Interface
 }
@@ -68,20 +86,59 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
 			continue
 		}
 
+		// check if the namespace should be treated as flaking when failed
+		flakeWhenFailed := ns.Labels["openshift.io/run-level"] == "0" ||
+			ns.Labels["openshift.io/run-level"] == "1" ||
+			namespacesWithPendingSCCPinning.Has(ns.Name)
+
 		pods, err := w.kubeClient.CoreV1().Pods(ns.Name).List(ctx, metav1.ListOptions{})
 		if err != nil {
 			return nil, nil, err
 		}
 
 		failures := make([]string, 0)
 		for _, pod := range pods.Items {
+			validatedSCC := pod.Annotations[securityv1.ValidatedSCCAnnotation]
+			allowedNamespaces, isNonStandard := nonStandardSCCNamespaces[validatedSCC]
+
 			if _, exists := pod.Annotations[securityv1.RequiredSCCAnnotation]; exists {
+				if isNonStandard && !allowedNamespaces.Has(ns.Name) {
+					failures = append(failures, fmt.Sprintf(
+						"pod '%s' has a non-standard SCC '%s' not allowed in namespace '%s'; allowed namespaces are: %s",
+						pod.Name, validatedSCC, ns.Name, strings.Join(allowedNamespaces.UnsortedList(), ", ")))
+				}
 				continue
 			}
 
-			suggestedSCC := suggestSCC(&pod)
 			owners := ownerReferences(&pod)
-			failures = append(failures, fmt.Sprintf("annotation missing from pod '%s'%s; %s", pod.Name, owners, suggestedSCC))
+
+			switch {
+			case len(validatedSCC) == 0:
+				failures = append(failures, fmt.Sprintf(
+					"annotation missing from pod '%s'%s; cannot suggest required-scc, no validated SCC on pod",
+					pod.Name, owners))
+
+			case defaultSCCs.Has(validatedSCC):
+				failures = append(failures, fmt.Sprintf(
+					"annotation missing from pod '%s'%s; suggested required-scc: '%s'",
+					pod.Name, owners, validatedSCC))
+
+			case isNonStandard:
+				if allowedNamespaces.Has(ns.Name) {
+					failures = append(failures, fmt.Sprintf(
+						"annotation missing from pod '%s'%s; suggested required-scc: '%s', this is a non-standard SCC",
+						pod.Name, owners, validatedSCC))
+				} else {
+					failures = append(failures, fmt.Sprintf(
+						"annotation missing from pod '%s'%s; pod is using non-standard SCC '%s' not allowed in namespace '%s'; allowed namespaces are: %s",
+						pod.Name, owners, validatedSCC, ns.Name, strings.Join(allowedNamespaces.UnsortedList(), ", ")))
+				}
+
+			default:
+				failures = append(failures, fmt.Sprintf(
+					"annotation missing from pod '%s'%s; cannot suggest required-scc, validated SCC '%s' is a custom SCC",
+					pod.Name, owners, validatedSCC))
+			}
 		}
 
 		testName := fmt.Sprintf("[sig-auth] all workloads in ns/%s must set the '%s' annotation", ns.Name, securityv1.RequiredSCCAnnotation)
@@ -91,18 +148,21 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
 		}
 
 		failureMsg := strings.Join(failures, "\n")
+
 		junits = append(junits,
 			&junitapi.JUnitTestCase{
 				Name:          testName,
 				SystemOut:     failureMsg,
 				FailureOutput: &junitapi.FailureOutput{Output: failureMsg},
-			},
-
-			// add a successful test with the same name to cause a flake
-			&junitapi.JUnitTestCase{
-				Name: testName,
-			},
-		)
+			})
+
+		// add a successful test with the same name to cause a flake
+		if flakeWhenFailed {
+			junits = append(junits,
+				&junitapi.JUnitTestCase{
+					Name: testName,
+				})
+		}
 	}
 
 	return nil, junits, nil
@@ -124,20 +184,6 @@ func (w *requiredSCCAnnotationChecker) Cleanup(ctx context.Context) error {
 	return nil
 }
 
-// suggestSCC suggests the assigned SCC only if it belongs to the default set of SCCs
-// pods in runlevel 0/1 namespaces won't have any assigned SCC as SCC admission is disabled
-func suggestSCC(pod *v1.Pod) string {
-	if len(pod.Annotations[securityv1.ValidatedSCCAnnotation]) == 0 {
-		return "cannot suggest required-scc, no validated SCC on pod"
-	}
-
-	if defaultSCCs.Has(pod.Annotations[securityv1.ValidatedSCCAnnotation]) {
-		return fmt.Sprintf("suggested required-scc: '%s'", pod.Annotations[securityv1.ValidatedSCCAnnotation])
-	}
-
-	return "cannot suggest required-scc, validated SCC is custom"
-}
-
 func ownerReferences(pod *v1.Pod) string {
 	ownerRefs := make([]string, len(pod.OwnerReferences))
 	for i, or := range pod.OwnerReferences {