Merge pull request #29323 from kramaranya/enforce-required-scc-test

OCPBUGS-44961: Fix "Enforce the required-scc monitor test and validate usage of non-standard OCP SCCs"

Author: openshift-merge-bot[bot]

pkg/monitortests/authentication/requiredsccmonitortests/monitortest.go

@@ -10,6 +10,7 @@ import (
 	"github.com/openshift/origin/pkg/monitor/monitorapi"
 	"github.com/openshift/origin/pkg/monitortestframework"
 	"github.com/openshift/origin/pkg/test/ginkgo/junitapi"
+	exutil "github.com/openshift/origin/test/extended/util"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 
@@ -31,6 +32,56 @@ 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"),
+}
+
+// namespacesWithPendingSCCPinning includes namespaces with workloads that have pending SCC pinning.
+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-monitoring",
+	// run-level namespaces
+	"openshift-cloud-controller-manager",
+	"openshift-cloud-controller-manager-operator",
+	"openshift-cluster-api",
+	"openshift-cluster-machine-approver",
+	"openshift-dns",
+	"openshift-dns-operator",
+	"openshift-etcd",
+	"openshift-etcd-operator",
+	"openshift-kube-apiserver",
+	"openshift-kube-apiserver-operator",
+	"openshift-kube-controller-manager",
+	"openshift-kube-controller-manager-operator",
+	"openshift-kube-proxy",
+	"openshift-kube-scheduler",
+	"openshift-kube-scheduler-operator",
+	"openshift-multus",
+	"openshift-network-operator",
+	"openshift-ovn-kubernetes",
+	"openshift-sdn",
+	"openshift-storage",
+)
+
+// systemNamespaces includes namespaces that should be treated as flaking.
+// these namespaces are included because we don't control their creation or labeling on their creation.
+var systemNamespaces = sets.NewString(
+	"default",
+	"kube-system",
+	"kube-public",
+	"openshift-node",
+	"openshift-infra",
+	"openshift",
+)
+
 type requiredSCCAnnotationChecker struct {
 	kubeClient kubernetes.Interface
 }
@@ -61,7 +112,12 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
 
 	junits := []*junitapi.JUnitTestCase{}
 	for _, ns := range namespaces.Items {
-		// require that all workloads in openshift, kube-* or default namespaces must have the required-scc annotation
+		// skip managed service namespaces
+		if exutil.ManagedServiceNamespaces.Has(ns.Name) {
+			continue
+		}
+
+		// require that all workloads in openshift, kube-*, or default namespaces must have the required-scc annotation
 		// ignore openshift-must-gather-* namespaces which are generated dynamically
 		isPermanentOpenShiftNamespace := (ns.Name == "openshift" || strings.HasPrefix(ns.Name, "openshift-")) && !strings.HasPrefix(ns.Name, "openshift-must-gather-")
 		if !strings.HasPrefix(ns.Name, "kube-") && ns.Name != "default" && !isPermanentOpenShiftNamespace {
@@ -75,13 +131,47 @@ func (w *requiredSCCAnnotationChecker) CollectData(ctx context.Context, storageD
 
 		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 +181,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 the namespace should be flaking
+		if namespacesWithPendingSCCPinning.Has(ns.Name) || systemNamespaces.Has(ns.Name) {
+			junits = append(junits,
+				&junitapi.JUnitTestCase{
+					Name: testName,
+				})
+		}
 	}
 
 	return nil, junits, nil
@@ -124,20 +217,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 {