Change implementation to checking pods in an operator namespace

redhat-best-practices-for-k8s · Jan 9, 2025 · 10602e4 · 10602e4
1 parent 9627fa4
commit 10602e4
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 60 deletions.
diff --git a/expected_results.yaml b/expected_results.yaml
@@ -66,7 +66,6 @@ testCases:
     - operator-single-crd-owner
     - operator-pods-no-hugepages
     - operator-multiple-same-operators
-    - operator-valid-installation-tenant-namespace
     - performance-exclusive-cpu-pool
     - performance-max-resources-exec-probes
     - platform-alteration-isredhat-release
@@ -94,6 +93,7 @@ testCases:
     - operator-pods-no-hugepages
     - operator-multiple-same-operators
     - operator-catalogsource-bundle-count
+    - operator-valid-installation-tenant-namespace
     - performance-exclusive-cpu-pool-rt-scheduling-policy
     - performance-isolated-cpu-pool-rt-scheduling-policy
     - performance-shared-cpu-pool-non-rt-scheduling-policy

diff --git a/tests/identifiers/doclinks.go b/tests/identifiers/doclinks.go
@@ -102,23 +102,24 @@ const (
 	TestRtAppNoExecProbesDocLink                = NoDocLinkFarEdge
 
 	// Operator Test Suite
-	DocOperatorRequirement                              = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cnf-operator-requirements"
-	TestOperatorInstallStatusSucceededIdentifierDocLink = DocOperatorRequirement
-	TestOperatorNoPrivilegesDocLink                     = DocOperatorRequirement
-	TestOperatorIsCertifiedIdentifierDocLink            = DocOperatorRequirement
-	TestOperatorIsInstalledViaOLMIdentifierDocLink      = DocOperatorRequirement
-	TestOperatorInstallationInTenantNamespaceDocLink    = DocOperatorRequirement
-	TestOperatorHasSemanticVersioningIdentifierDocLink  = DocOperatorRequirement
-	TestOperatorCrdSchemaIdentifierDocLink              = DocOperatorRequirement
-	TestOperatorCrdVersioningIdentifierDocLink          = DocOperatorRequirement
-	TestOperatorSingleCrdOwnerIdentifierDocLink         = DocOperatorRequirement
-	TestOperatorRunAsUserIDDocLink                      = DocOperatorRequirement
-	TestOperatorRunAsNonRootDocLink                     = DocOperatorRequirement
-	TestOperatorAutomountTokensDocLink                  = DocOperatorRequirement
-	TestOperatorReadOnlyFilesystemDocLink               = DocOperatorRequirement
-	TestOperatorPodsNoHugepagesDocLink                  = DocOperatorRequirement
-	TestOperatorOlmSkipRangeDocLink                     = DocOperatorRequirement
-	TestMultipleSameOperatorsIdentifierDocLink          = DocOperatorRequirement
+	DocOperatorRequirement                                = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cnf-operator-requirements"
+	TestOperatorInstallStatusSucceededIdentifierDocLink   = DocOperatorRequirement
+	TestOperatorNoPrivilegesDocLink                       = DocOperatorRequirement
+	TestOperatorIsCertifiedIdentifierDocLink              = DocOperatorRequirement
+	TestOperatorIsInstalledViaOLMIdentifierDocLink        = DocOperatorRequirement
+	TestOperatorInstallationInTenantNamespaceDocLink      = DocOperatorRequirement
+	TestOperatorHasSemanticVersioningIdentifierDocLink    = DocOperatorRequirement
+	TestOperatorCrdSchemaIdentifierDocLink                = DocOperatorRequirement
+	TestOperatorCrdVersioningIdentifierDocLink            = DocOperatorRequirement
+	TestOperatorSingleCrdOwnerIdentifierDocLink           = DocOperatorRequirement
+	TestOperatorRunAsUserIDDocLink                        = DocOperatorRequirement
+	TestOperatorRunAsNonRootDocLink                       = DocOperatorRequirement
+	TestOperatorAutomountTokensDocLink                    = DocOperatorRequirement
+	TestOperatorReadOnlyFilesystemDocLink                 = DocOperatorRequirement
+	TestOperatorPodsNoHugepagesDocLink                    = DocOperatorRequirement
+	TestOperatorCatalogSourceBundleCountIdentifierDocLink = DocOperatorRequirement
+	TestOperatorOlmSkipRangeDocLink                       = DocOperatorRequirement
+	TestMultipleSameOperatorsIdentifierDocLink            = DocOperatorRequirement
 
 	// Observability Test Suite
 	TestLoggingIdentifierDocLink                            = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-logging"

diff --git a/tests/operator/helper.go b/tests/operator/helper.go
@@ -21,10 +21,17 @@ Package operator provides CNFCERT tests used to validate operator CNF facets.
 package operator
 
 import (
+	"context"
+	"fmt"
 	"strings"
 
+	"github.com/redhat-best-practices-for-k8s/certsuite/internal/clientsholder"
+	"github.com/redhat-best-practices-for-k8s/certsuite/pkg/podhelper"
+	"github.com/redhat-best-practices-for-k8s/certsuite/pkg/provider"
+
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
-	"github.com/redhat-best-practices-for-k8s/certsuite/pkg/stringhelper"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // CsvResult holds the results of the splitCsv function.
@@ -60,11 +67,55 @@ func hasOperatorInstallModeSingleNamespace(installModes []v1alpha1.InstallMode)
 	return false
 }
 
-func checkOperatorInstallationCompliance(operatorNamespace, csvNamespace string, targetNamespaces []string, isSingleNamespaceInstallModeSupported bool) bool {
-	// operators with single namespace are installed in the tenant namespace
-	if isSingleNamespaceInstallModeSupported {
-		return len(targetNamespaces) == 1 && strings.Compare(operatorNamespace, targetNamespaces[0]) != 0
+func filterSingleNamespacedOperatorUnderTest(operators []*provider.Operator) (singleNamespacedOperators []*provider.Operator) {
+	for _, operator := range operators {
+		if hasOperatorInstallModeSingleNamespace(operator.Csv.Spec.InstallModes) && len(operator.TargetNamespaces) == 1 {
+			singleNamespacedOperators = append(singleNamespacedOperators, operator)
+		}
+	}
+	return singleNamespacedOperators
+}
+
+// This function checks if the namespace contains only valid operator pods
+func checkIfNamespaceContainsOnlyOperatorPods(namespace string) (isOperatorOnlyNamespace bool, err error) {
+	// Get all pods from the target namespace
+
+	podsList, err := clientsholder.GetClientsHolder().K8sClient.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return isOperatorOnlyNamespace, err
 	}
-	// operators are not installed in tenant namespaces
-	return !stringhelper.StringInSlice(targetNamespaces, csvNamespace, false)
+	foundCsvs := make(map[string]bool)
+	foundOperatorPods := 0
+	for index := range podsList.Items {
+		// Get the top owners of the pod
+		pod := podsList.Items[index]
+		topOwners, err := podhelper.GetPodTopOwner(pod.Namespace, pod.OwnerReferences)
+		if err != nil {
+			return isOperatorOnlyNamespace, fmt.Errorf("could not get top owners of Pod %s (in namespace %s), err=%v", pod.Name, pod.Namespace, err)
+		}
+
+		// check if owner matches with the csv
+		for _, owner := range topOwners {
+			// The owner must be in the targetNamespace
+			if owner.Kind == v1alpha1.ClusterServiceVersionKind && owner.Namespace == namespace {
+				foundOperatorPods++
+				foundCsvs[owner.Name] = true
+				break
+			}
+		}
+	}
+
+	// Check if the found CSVs contain only valid operators under test
+	allOperatorsFoundInNamespaceAreValid := true
+	for _, operator := range env.Operators {
+		for foundCsv := range foundCsvs {
+			// Report an error only if an operator under test is found to be clusterwide
+			if operator.IsClusterWide && operator.Csv.Name == foundCsv {
+				allOperatorsFoundInNamespaceAreValid = false
+				break
+			}
+		}
+	}
+
+	return len(podsList.Items) == foundOperatorPods && allOperatorsFoundInNamespaceAreValid, nil
 }
diff --git a/tests/operator/suite.go b/tests/operator/suite.go
@@ -120,6 +120,13 @@ func LoadChecks() {
 			return nil
 		}))
 
+	checksGroup.Add(checksdb.NewCheck(identifiers.GetTestIDAndLabels(identifiers.TestOperatorCatalogSourceBundleCountIdentifier)).
+		WithSkipCheckFn(testhelper.GetNoCatalogSourcesSkipFn(&env)).
+		WithCheckFn(func(c *checksdb.Check) error {
+			testOperatorCatalogSourceBundleCount(c, &env)
+			return nil
+		}))
+
 	checksGroup.Add(checksdb.NewCheck(identifiers.GetTestIDAndLabels(identifiers.TestOperatorInstallationInTenantNamespace)).
 		WithSkipCheckFn(testhelper.GetNoOperatorsSkipFn(&env)).
 		WithCheckFn(func(c *checksdb.Check) error {
@@ -128,55 +135,40 @@ func LoadChecks() {
 		}))
 }
 
-/*
-Checks :
-
-	Operators with SingleNamespaced install mode should only be installed in the tenant dedicated operator namespace
-*/
+// This function checks if SingleNamespaced Operators should only be installed in the tenant dedicated operator namespace
 func testOperatorInstallationInTenantNamespace(check *checksdb.Check, env *provider.TestEnvironment) {
 	check.LogInfo("Starting testInstalledSingleNamespaceOperatorInTenanttNamespace")
 
 	var compliantObjects []*testhelper.ReportObject
 	var nonCompliantObjects []*testhelper.ReportObject
 
-	check.LogInfo("Total operators found %d ", len(env.Operators))
+	singleNamespacedOperators := filterSingleNamespacedOperatorUnderTest(env.Operators)
+
+	for _, operator := range singleNamespacedOperators {
+		operatorNamespace := operator.Namespace
 
-	for _, operator := range env.Operators {
 		check.LogInfo("Checking operator %s in namespace %s ", operator.Name, operator.Namespace)
 
-		csv := operator.Csv
-		isSingleNamespaceInstallModeSupported := hasOperatorInstallModeSingleNamespace(csv.Spec.InstallModes)
-
-		// consider only operators whose InstallModyType is SingleNamespace and its operator group has only one targetNamespace
-		if isSingleNamespaceInstallModeSupported {
-			csvNamespace := csv.Namespace
-			operatorNamespace := csv.Annotations["olm.operatorNamespace"]
-			targetNamespaces := operator.TargetNamespaces
-
-			check.LogInfo("operatorNamespace=%s, csvNamespace=%s, targetNamespaces=%v, singleNamespace=%v", operatorNamespace,
-				csvNamespace, targetNamespaces, isSingleNamespaceInstallModeSupported)
-
-			isCompliant := checkOperatorInstallationCompliance(
-				operatorNamespace, csvNamespace, targetNamespaces,
-				isSingleNamespaceInstallModeSupported,
-			)
-			check.LogInfo("Operator is installation Compliant %v", isCompliant)
-
-			if isCompliant {
-				check.LogInfo("Operator %s has valid installation in tenant namespace %s ", operator.Name, targetNamespaces[0])
-				compliantObjects = append(compliantObjects, testhelper.NewOperatorReportObject(operator.Namespace, operator.Name,
-					"Operator has valid installation in tenant namespace ", true).AddField(testhelper.OperatorName, operator.Name))
-			} else {
-				check.LogInfo("Operator %s has invalid installation in tenant namespace ", operator.Name)
-				nonCompliantObjects = append(nonCompliantObjects, testhelper.NewOperatorReportObject(operator.Namespace, operator.Name,
-					"Operator has invalid installation in tenant namespace ", false).AddField(testhelper.OperatorName, operator.Name))
-			}
+		doesNamespaceContainsOnlyOperatorPods, err := checkIfNamespaceContainsOnlyOperatorPods(operatorNamespace)
+
+		if err != nil {
+			check.LogError("Skipped - cannot proceed with operator %s", operator.Name)
+			continue
+		}
+		if doesNamespaceContainsOnlyOperatorPods {
+			check.LogInfo("Operator %s has valid installation in tenant namespace %s ", operator.Name, operatorNamespace)
+			compliantObjects = append(compliantObjects, testhelper.NewOperatorReportObject(operator.Namespace, operator.Name,
+				"Operator has valid installation in tenant namespace ", true))
+		} else {
+			check.LogInfo("Operator %s has invalid installation in tenant namespace ", operator.Name)
+			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewOperatorReportObject(operator.Namespace, operator.Name,
+				"Operator has invalid installation in tenant namespace ", false))
 		}
 	}
 	check.SetResult(compliantObjects, nonCompliantObjects)
 }
 
-// This function checks if the Operator CRD version follows K8s versioning
+// This function check if the Operator CRD version follows K8s versioning
 func testOperatorCrdVersioning(check *checksdb.Check, env *provider.TestEnvironment) {
 	check.LogInfo("Starting testOperatorCrdVersioning")
 	var compliantObjects []*testhelper.ReportObject