From da5b8f59aac6f748b19f34b006a83f7a5e0e9362 Mon Sep 17 00:00:00 2001 From: Vincent Shen Date: Thu, 8 Feb 2024 16:21:25 -0800 Subject: [PATCH] OCPBUGS-29272: Delete scan when SSB remove a profile This pr fixes a issue when a profile gets removed from scansettingbinding, the old scan was not deleted when a profile is removed from the existing scansettingbinding, this pr checks that and does the removal so that new scan using that profile can be launch correctly. check OCPBUGS-29272: https://issues.redhat.com/browse/OCPBUGS-29272 --- .../compliancesuite_controller.go | 25 +++- tests/e2e/framework/common.go | 10 ++ tests/e2e/serial/main_test.go | 109 ++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) diff --git a/pkg/controller/compliancesuite/compliancesuite_controller.go b/pkg/controller/compliancesuite/compliancesuite_controller.go index 609568997..7919640ed 100644 --- 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 index 623d1a6d1..3a435db85 100644 --- 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 index b47627e22..c6a1b9785 100644 --- a/tests/e2e/serial/main_test.go +++ b/tests/e2e/serial/main_test.go @@ -1520,6 +1520,115 @@ 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", + }, + }, + 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) + } + + 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` 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(scanName, 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.Printf("Scan %s doesn't exist anymore\n", scanName) + 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