Merge pull request #492 from Vincent056/delete_scan

OCPBUGS-29272: Delete scan when SSB remove a profile
ComplianceAsCode · Mar 25, 2024 · de5370b · de5370b
2 parents 1c5f9e7 + 219c377
commit de5370b
Show file tree

Hide file tree

Showing 3 changed files with 162 additions and 1 deletion.
diff --git a/pkg/controller/compliancesuite/compliancesuite_controller.go b/pkg/controller/compliancesuite/compliancesuite_controller.go
@@ -229,6 +229,7 @@ func (r *ReconcileComplianceSuite) issueValidationError(suite *compv1alpha1.Comp
 }
 
 func (r *ReconcileComplianceSuite) reconcileScans(suite *compv1alpha1.ComplianceSuite, logger logr.Logger) (bool, error) {
+	requiredScansNames := make(map[string]bool)
 	for idx := range suite.Spec.Scans {
 		scanWrap := &suite.Spec.Scans[idx]
 		scan := &compv1alpha1.ComplianceScan{}
@@ -257,8 +258,30 @@ func (r *ReconcileComplianceSuite) reconcileScans(suite *compv1alpha1.Compliance
 		if rescheduleWithDelay || err != nil {
 			return rescheduleWithDelay, err
 		}
+		requiredScansNames[scan.Name] = true
+	}
+
+	// check all the scans owned by the suite and see if they are still in the spec
+	// if not, delete them
+	scanList := &compv1alpha1.ComplianceScanList{}
+	listOpts := client.ListOptions{
+		LabelSelector: labels.SelectorFromSet(labels.Set{compv1alpha1.SuiteLabel: suite.Name}),
+	}
+	if err := r.Client.List(context.TODO(), scanList, &listOpts); err != nil {
+		return false, err
+	}
 
-		// FIXME: delete scans that went away
+	for _, scan := range scanList.Items {
+		if _, ok := requiredScansNames[scan.Name]; !ok {
+			logger.Info("Deleting scan due to it no longer being in the suite", "ComplianceScan.Name", scan.Name)
+			r.Recorder.Eventf(
+				suite, corev1.EventTypeNormal, "SuiteScanDeleted",
+				"Scan %s is being deleted due to it no longer being in the suite", scan.Name)
+			// delete the scan
+			if err := r.Client.Delete(context.TODO(), &scan); err != nil {
+				return false, err
+			}
+		}
 	}
 
 	return false, nil

diff --git a/tests/e2e/framework/common.go b/tests/e2e/framework/common.go
@@ -1125,6 +1125,16 @@ func (f *Framework) AssertScanHasValidPVCReferenceWithSize(scanName, size, names
 	return nil
 }
 
+func (f *Framework) AssertScanExists(name, namespace string) error {
+	cs := &compv1alpha1.ComplianceScan{}
+	defer f.logContainerOutput(namespace, name)
+	err := f.Client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, cs)
+	if apierrors.IsNotFound(err) {
+		return fmt.Errorf("Failed to assert ComplianceScan %s exists: %w", name, err)
+	}
+	return err
+}
+
 func (f *Framework) AssertScanDoesNotExist(name, namespace string) error {
 	cs := &compv1alpha1.ComplianceScan{}
 	defer f.logContainerOutput(namespace, name)

diff --git a/tests/e2e/serial/main_test.go b/tests/e2e/serial/main_test.go
@@ -1612,6 +1612,134 @@ func TestSuspendScanSetting(t *testing.T) {
 	}
 }
 
+func TestRemoveProfileScan(t *testing.T) {
+	f := framework.Global
+	// Bind the new ScanSetting to a Profile
+	bindingName := framework.GetObjNameFromTest(t) + "-binding"
+	scanSettingBinding := compv1alpha1.ScanSettingBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      bindingName,
+			Namespace: f.OperatorNamespace,
+		},
+		Profiles: []compv1alpha1.NamedObjectReference{
+			{
+				Name:     "ocp4-cis",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+			{
+				Name:     "ocp4-moderate",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+			{
+				Name:     "ocp4-cis-node",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+		},
+		SettingsRef: &compv1alpha1.NamedObjectReference{
+			Name:     "default",
+			Kind:     "ScanSetting",
+			APIGroup: "compliance.openshift.io/v1alpha1",
+		},
+	}
+	if err := f.Client.Create(context.TODO(), &scanSettingBinding, nil); err != nil {
+		t.Fatal(err)
+	}
+	defer f.Client.Delete(context.TODO(), &scanSettingBinding)
+
+	if err := f.WaitForSuiteScansStatus(f.OperatorNamespace, bindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant); err != nil {
+		t.Fatal(err)
+	}
+
+	// AssertScanExists check if the scan exists for both profiles
+	if err := f.AssertScanExists("ocp4-cis", f.OperatorNamespace); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := f.AssertScanExists("ocp4-moderate", f.OperatorNamespace); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := f.AssertScanExists("ocp4-cis-node-master", f.OperatorNamespace); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := f.AssertScanExists("ocp4-cis-node-worker", f.OperatorNamespace); err != nil {
+		t.Fatal(err)
+	}
+
+	scanName := "ocp4-moderate"
+	checkResult := compv1alpha1.ComplianceCheckResult{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      fmt.Sprintf("%s-audit-profile-set", scanName),
+			Namespace: f.OperatorNamespace,
+		},
+		ID:       "xccdf_org.ssgproject.content_rule_audit_profile_set",
+		Status:   compv1alpha1.CheckResultPass,
+		Severity: compv1alpha1.CheckResultSeverityMedium,
+	}
+	err := f.AssertHasCheck(bindingName, scanName, checkResult)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Remove the `ocp4-moderate` and `ocp4-cis-node` profile from the `ScanSettingBinding`
+	scanSettingBindingUpdate := &compv1alpha1.ScanSettingBinding{}
+	if err := f.Client.Get(context.TODO(), types.NamespacedName{Namespace: f.OperatorNamespace, Name: bindingName}, scanSettingBindingUpdate); err != nil {
+		t.Fatalf("failed to get ScanSettingBinding %s", bindingName)
+	}
+
+	scanSettingBindingUpdate.Profiles = []compv1alpha1.NamedObjectReference{
+		{
+			Name:     "ocp4-cis",
+			Kind:     "Profile",
+			APIGroup: "compliance.openshift.io/v1alpha1",
+		},
+	}
+	if err := f.Client.Update(context.TODO(), scanSettingBindingUpdate); err != nil {
+		t.Fatal(err)
+	}
+	var lastErr error
+	timeouterr := wait.Poll(framework.RetryInterval, framework.Timeout, func() (bool, error) {
+		if lastErr := f.AssertScanDoesNotExist("ocp4-moderate", f.OperatorNamespace); lastErr != nil {
+			log.Printf("Retrying: %s\n", lastErr)
+			return false, nil
+		}
+		if lastErr := f.AssertScanDoesNotExist("ocp4-cis-node-master", f.OperatorNamespace); lastErr != nil {
+			log.Printf("Retrying: %s\n", lastErr)
+			return false, nil
+		}
+		if lastErr := f.AssertScanDoesNotExist("ocp4-cis-node-worker", f.OperatorNamespace); lastErr != nil {
+			log.Printf("Retrying: %s\n", lastErr)
+			return false, nil
+		}
+		if lastErr := f.AssertScanDoesNotContainCheck(scanName, checkResult.Name, f.OperatorNamespace); lastErr != nil {
+			log.Printf("Retrying: %s\n", lastErr)
+			// print more info about the found check
+			ccr := &compv1alpha1.ComplianceCheckResult{}
+			if err := f.Client.Get(context.TODO(), types.NamespacedName{Namespace: f.OperatorNamespace, Name: checkResult.Name}, ccr); err != nil {
+				log.Printf("failed to get check %s: %s\n", checkResult.Name, err)
+			} else {
+				log.Printf("Object: %v\n", ccr)
+			}
+			return false, nil
+		}
+		log.Print("Scan ocp4-moderate, ocp4-cis-node-master and ocp4-cis-node-worker do not exist anymore\n")
+		log.Printf("Check %s doesn't exist anymore\n", checkResult.Name)
+		return true, nil
+	})
+
+	if lastErr != nil {
+		t.Fatalf("failed to remove profile from ScanSettingBinding: %s", lastErr)
+	}
+
+	if timeouterr != nil {
+		t.Fatalf("timed out waiting for scan and check to be removed: %s", timeouterr)
+	}
+}
+
 func TestSuspendScanSettingDoesNotCreateScan(t *testing.T) {
 	f := framework.Global