From 387b50426002df4a33a99ede43d01c1971663f8f Mon Sep 17 00:00:00 2001 From: Vincent Shen Date: Tue, 13 Aug 2024 09:20:16 -0700 Subject: [PATCH] CMP-2615: Add a check aggregate to the compliance scan metadata Implement a total check count as an annotation of the ComplianceScan, we will add an annotation compliance.openshift.io/check-count to every compliancescan object when a scan is in done state. Noted: This annotation is removed when a new scanPhase begin. --- .../v1alpha1/compliancescan_types.go | 4 + .../compliancescan_controller.go | 34 +++++++ tests/e2e/framework/common.go | 7 ++ tests/e2e/framework/utils.go | 35 +++++++ tests/e2e/serial/main_test.go | 96 +++++++++++++++++++ 5 files changed, 176 insertions(+) diff --git a/pkg/apis/compliance/v1alpha1/compliancescan_types.go b/pkg/apis/compliance/v1alpha1/compliancescan_types.go index 219345372..17b3e437a 100644 --- a/pkg/apis/compliance/v1alpha1/compliancescan_types.go +++ b/pkg/apis/compliance/v1alpha1/compliancescan_types.go @@ -22,6 +22,10 @@ const ComplianceScanRescanAnnotation = "compliance.openshift.io/rescan" // "api-checks" in the annotation. const ComplianceScanTimeoutAnnotation = "compliance.openshift.io/timeout" +// ComplianceCheckCountAnnotation indicates the number of checks +// that a ComplianceScan has generated during the scan +const ComplianceCheckCountAnnotation = "compliance.openshift.io/check-count" + // ComplianceScanLabel serves as an indicator for which ComplianceScan // owns the referenced object const ComplianceScanLabel = "compliance.openshift.io/scan-name" diff --git a/pkg/controller/compliancescan/compliancescan_controller.go b/pkg/controller/compliancescan/compliancescan_controller.go index 4195fd41b..78309b2a6 100644 --- a/pkg/controller/compliancescan/compliancescan_controller.go +++ b/pkg/controller/compliancescan/compliancescan_controller.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "math" + "strconv" "strings" "time" @@ -286,6 +287,7 @@ func (r *ReconcileComplianceScan) phasePendingHandler(instance *compv1alpha1.Com // Remove annotation if needed if instance.NeedsRescan() { instanceCopy := instance.DeepCopy() + delete(instanceCopy.Annotations, compv1alpha1.ComplianceCheckCountAnnotation) delete(instanceCopy.Annotations, compv1alpha1.ComplianceScanRescanAnnotation) delete(instanceCopy.Annotations, compv1alpha1.ComplianceScanTimeoutAnnotation) err := r.Client.Update(context.TODO(), instanceCopy) @@ -649,6 +651,26 @@ func (r *ReconcileComplianceScan) phaseAggregatingHandler(h scanTypeHandler, log instance.Status.ErrorMessage = err.Error() } + // count the number of checks that were run + checkCount, err := r.fetchResultCounts(instance, logger) + if err != nil { + logger.Error(err, "Cannot fetch the number of checks") + return reconcile.Result{}, err + } + + instanceCopy := instance.DeepCopy() + + if instanceCopy.Annotations == nil { + instanceCopy.Annotations = make(map[string]string) + } + + // adding check count annotation + instanceCopy.Annotations[compv1alpha1.ComplianceCheckCountAnnotation] = strconv.Itoa(checkCount) + + if err := r.Client.Update(context.TODO(), instanceCopy); err != nil { + logger.Error(err, "Cannot update the scan with the check count") + return reconcile.Result{}, err + } instance.Status.Phase = compv1alpha1.PhaseDone instance.Status.EndTimestamp = &metav1.Time{Time: time.Now()} instance.Status.SetConditionReady() @@ -661,6 +683,18 @@ func (r *ReconcileComplianceScan) phaseAggregatingHandler(h scanTypeHandler, log return reconcile.Result{}, nil } +func (r *ReconcileComplianceScan) fetchResultCounts(scan *compv1alpha1.ComplianceScan, logger logr.Logger) (int, error) { + var checkList compv1alpha1.ComplianceCheckResultList + checkListOpts := client.MatchingLabels{ + compv1alpha1.ComplianceScanLabel: scan.Name, + } + if err := r.Client.List(context.TODO(), &checkList, &checkListOpts); err != nil { + logger.Error(err, "Cannot list the check results") + return 0, err + } + return len(checkList.Items), nil +} + func (r *ReconcileComplianceScan) phaseDoneHandler(h scanTypeHandler, instance *compv1alpha1.ComplianceScan, logger logr.Logger, doDelete bool) (reconcile.Result, error) { var err error logger.Info("Phase: Done") diff --git a/tests/e2e/framework/common.go b/tests/e2e/framework/common.go index 1989eeed0..fb68bbcb7 100644 --- a/tests/e2e/framework/common.go +++ b/tests/e2e/framework/common.go @@ -876,6 +876,13 @@ func (f *Framework) WaitForScanStatus(namespace, name string, targetStatus compv return false, nil } + if exampleComplianceScan.Status.Phase != compv1alpha1.PhaseDone && exampleComplianceScan.Status.Phase != compv1alpha1.PhasePending { + // check we should not have checkCount annotation + if exampleComplianceScan.Annotations[compv1alpha1.ComplianceCheckCountAnnotation] != "" { + return true, fmt.Errorf("compliancescan %s has unset checkCount annotation", name) + } + } + if exampleComplianceScan.Status.Phase == targetStatus { return true, nil } diff --git a/tests/e2e/framework/utils.go b/tests/e2e/framework/utils.go index eb0cbf0be..58da528a8 100644 --- a/tests/e2e/framework/utils.go +++ b/tests/e2e/framework/utils.go @@ -57,6 +57,41 @@ func (f *Framework) AssertMustHaveParsedProfiles(pbName, productType, productNam return nil } +// AssertScanHasTotalCheckCounts asserts that the scan has the expected total check counts +func (f *Framework) AssertScanHasTotalCheckCounts(namespace, scanName string) error { + // check if scan has annotation + var scan compv1alpha1.ComplianceScan + key := types.NamespacedName{Namespace: namespace, Name: scanName} + if err := f.Client.Get(context.Background(), key, &scan); err != nil { + return err + } + if scan.Annotations == nil { + return fmt.Errorf("expected annotations to be not nil") + } + if scan.Annotations[compv1alpha1.ComplianceCheckCountAnnotation] == "" { + return fmt.Errorf("expected %s to be not empty", compv1alpha1.ComplianceCheckCountAnnotation) + } + + gotCheckCount, err := strconv.Atoi(scan.Annotations[compv1alpha1.ComplianceCheckCountAnnotation]) + if err != nil { + return fmt.Errorf("failed to convert %s to int: %w", compv1alpha1.ComplianceCheckCountAnnotation, err) + } + + var checkList compv1alpha1.ComplianceCheckResultList + checkListOpts := client.MatchingLabels{ + compv1alpha1.ComplianceScanLabel: scanName, + } + if err := f.Client.List(context.TODO(), &checkList, &checkListOpts); err != nil { + return err + } + + if gotCheckCount != len(checkList.Items) { + return fmt.Errorf("expected %s to be %d, got %d instead", compv1alpha1.ComplianceCheckCountAnnotation, len(checkList.Items), gotCheckCount) + } + + return nil +} + // AssertRuleCheckTypeChangedAnnotationKey asserts that the rule check type changed annotation key exists func (f *Framework) AssertRuleCheckTypeChangedAnnotationKey(namespace, ruleName, lastCheckType string) error { var r compv1alpha1.Rule diff --git a/tests/e2e/serial/main_test.go b/tests/e2e/serial/main_test.go index d61a1aef9..502a05cdc 100644 --- a/tests/e2e/serial/main_test.go +++ b/tests/e2e/serial/main_test.go @@ -245,8 +245,104 @@ func TestSuiteScan(t *testing.T) { f.AssertCheckRemediation(checkVsyscall.Name, checkVsyscall.Namespace, true) } + // ensure scan has total check counts annotation + err = f.AssertScanHasTotalCheckCounts(f.OperatorNamespace, workerScanName) + if err != nil { + t.Fatal(err) + } + + err = f.AssertScanHasTotalCheckCounts(f.OperatorNamespace, masterScanName) + if err != nil { + t.Fatal(err) + } + } +// TestSuiteScanHasCheckCounts tests that the scan has the correct check counts +func TestSuiteScanHasCheckCounts(t *testing.T) { + f := framework.Global + suiteName := "test-suite-two-scans-check-counts" + workerScanName := fmt.Sprintf("%s-workers-scan", suiteName) + selectWorkers := map[string]string{ + "node-role.kubernetes.io/worker": "", + } + + masterScanName := fmt.Sprintf("%s-masters-scan", suiteName) + selectMasters := map[string]string{ + "node-role.kubernetes.io/master": "", + } + + exampleComplianceSuite := &compv1alpha1.ComplianceSuite{ + ObjectMeta: metav1.ObjectMeta{ + Name: suiteName, + Namespace: f.OperatorNamespace, + }, + Spec: compv1alpha1.ComplianceSuiteSpec{ + ComplianceSuiteSettings: compv1alpha1.ComplianceSuiteSettings{ + AutoApplyRemediations: false, + }, + Scans: []compv1alpha1.ComplianceScanSpecWrapper{ + { + ComplianceScanSpec: compv1alpha1.ComplianceScanSpec{ + ContentImage: contentImagePath, + Profile: "xccdf_org.ssgproject.content_profile_moderate", + Content: framework.RhcosContentFile, + NodeSelector: selectWorkers, + ComplianceScanSettings: compv1alpha1.ComplianceScanSettings{ + Debug: true, + }, + }, + Name: workerScanName, + }, + { + ComplianceScanSpec: compv1alpha1.ComplianceScanSpec{ + ContentImage: contentImagePath, + Profile: "xccdf_org.ssgproject.content_profile_moderate", + Content: framework.RhcosContentFile, + NodeSelector: selectMasters, + ComplianceScanSettings: compv1alpha1.ComplianceScanSettings{ + Debug: true, + }, + }, + Name: masterScanName, + }, + }, + }, + } + + err := f.Client.Create(context.TODO(), exampleComplianceSuite, nil) + if err != nil { + t.Fatal(err) + } + defer f.Client.Delete(context.TODO(), exampleComplianceSuite) + + // Ensure that all the scans in the suite have finished and are marked as Done + err = f.WaitForSuiteScansStatus(f.OperatorNamespace, suiteName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant) + if err != nil { + t.Fatal(err) + } + + // At this point, both scans should be non-compliant given our current content + err = f.AssertScanIsNonCompliant(workerScanName, f.OperatorNamespace) + if err != nil { + t.Fatal(err) + } + err = f.AssertScanIsNonCompliant(masterScanName, f.OperatorNamespace) + if err != nil { + t.Fatal(err) + } + + // ensure scan has total check counts annotation + err = f.AssertScanHasTotalCheckCounts(f.OperatorNamespace, workerScanName) + if err != nil { + t.Fatal(err) + } + + err = f.AssertScanHasTotalCheckCounts(f.OperatorNamespace, masterScanName) + if err != nil { + t.Fatal(err) + } +} func TestScanHasProfileGUID(t *testing.T) { f := framework.Global bindingName := framework.GetObjNameFromTest(t)