Merge pull request #489 from ComplianceAsCode/mix_scan

Remove product validation in ScanSettingBinding
ComplianceAsCode · Mar 21, 2024 · 1c5f9e7 · 1c5f9e7
2 parents 7800187 + 5bed715
commit 1c5f9e7
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 21 deletions.
diff --git a/pkg/controller/scansettingbinding/scansettingbinding_controller.go b/pkg/controller/scansettingbinding/scansettingbinding_controller.go
@@ -144,7 +144,6 @@ func (r *ReconcileScanSettingBinding) Reconcile(ctx context.Context, request rec
 		return reconcile.Result{}, err
 	}
 
-	var nodeProduct string
 	for i := range instance.Profiles {
 		ss := &instance.Profiles[i]
 
@@ -183,27 +182,11 @@ func (r *ReconcileScanSettingBinding) Reconcile(ctx context.Context, request rec
 			}
 		}
 
-		scan, product, err := newCompScanFromBindingProfile(r, instance, profileObj, log)
+		scan, _, err := newCompScanFromBindingProfile(r, instance, profileObj, log)
 		if err != nil {
 			return common.ReturnWithRetriableError(reqLogger, err)
 		}
 
-		nodeProduct = getRelevantProduct(nodeProduct, product)
-
-		if isDifferentProduct(nodeProduct, product) {
-			msg := fmt.Sprintf("ScanSettingBinding defines multiple products: %s and %s", product, nodeProduct)
-			r.Eventf(instance, corev1.EventTypeWarning, "MultipleProducts", msg)
-
-			ssb := instance.DeepCopy()
-			ssb.Status.SetConditionInvalid(msg)
-			ssb.Status.Phase = compliancev1alpha1.ScanSettingBindingPhaseInvalid
-			if updateErr := r.Client.Status().Update(context.TODO(), ssb); updateErr != nil {
-				return reconcile.Result{}, fmt.Errorf("couldn't update ScanSettingBinding condition: %w", updateErr)
-			}
-			// Don't requeue in this case, nothing we can do
-			return reconcile.Result{}, nil
-		}
-
 		suite.Spec.Scans = append(suite.Spec.Scans, *scan)
 	}
 

diff --git a/pkg/controller/scansettingbinding/scansettingbinding_controller_test.go b/pkg/controller/scansettingbinding/scansettingbinding_controller_test.go
@@ -747,7 +747,7 @@ var _ = Describe("Testing scansettingbinding controller", func() {
 			Expect(err).To(BeNil())
 		})
 
-		It("Should not create a suite", func() {
+		It("Should create a suite", func() {
 			_, err := reconciler.Reconcile(context.TODO(), reconcile.Request{
 				NamespacedName: types.NamespacedName{
 					Namespace: ssb.Namespace,
@@ -762,10 +762,10 @@ var _ = Describe("Testing scansettingbinding controller", func() {
 			}, ssb)
 			Expect(err).To(BeNil())
 			Expect(ssb.Status.Conditions.GetCondition("Ready")).ToNot(BeNil())
-			Expect(ssb.Status.Conditions.IsTrueFor("Ready")).To(BeFalse())
+			Expect(ssb.Status.Conditions.IsTrueFor("Ready")).To(BeTrue())
 
 			err = reconciler.Client.Get(context.TODO(), types.NamespacedName{Name: ssb.Name, Namespace: ssb.Namespace}, suite)
-			Expect(err).ToNot(BeNil())
+			Expect(err).To(BeNil())
 		})
 	})
 

diff --git a/tests/e2e/serial/main_test.go b/tests/e2e/serial/main_test.go
@@ -247,6 +247,98 @@ func TestSuiteScan(t *testing.T) {
 
 }
 
+func TestMixProductScan(t *testing.T) {
+	f := framework.Global
+
+	// Creates a new `ScanSetting`, where the actual scan schedule doesn't necessarily matter, but `suspend` is set to `False`
+	scanSettingName := framework.GetObjNameFromTest(t) + "-mixproduct"
+	scanSetting := compv1alpha1.ScanSetting{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      scanSettingName,
+			Namespace: f.OperatorNamespace,
+		},
+		ComplianceSuiteSettings: compv1alpha1.ComplianceSuiteSettings{
+			AutoApplyRemediations: false,
+			Schedule:              "0 1 * * *",
+			Suspend:               false,
+		},
+		Roles: []string{"master", "worker"},
+	}
+	if err := f.Client.Create(context.TODO(), &scanSetting, nil); err != nil {
+		t.Fatal(err)
+	}
+	defer f.Client.Delete(context.TODO(), &scanSetting)
+
+	// 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-moderate",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+			{
+				Name:     "ocp4-moderate-node",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+			{
+				Name:     "rhcos4-moderate",
+				Kind:     "Profile",
+				APIGroup: "compliance.openshift.io/v1alpha1",
+			},
+		},
+		SettingsRef: &compv1alpha1.NamedObjectReference{
+			Name:     scanSetting.Name,
+			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)
+
+	// Wait until the scan completes
+	// after the scan is done
+	if err := f.WaitForSuiteScansStatus(f.OperatorNamespace, bindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant); err != nil {
+		t.Fatal(err)
+	}
+
+	suite := &compv1alpha1.ComplianceSuite{}
+	key := types.NamespacedName{Name: bindingName, Namespace: f.OperatorNamespace}
+	if err := f.Client.Get(context.TODO(), key, suite); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert all the scans are there and completed
+	expectedScan := []string{"ocp4-moderate", "ocp4-moderate-node-worker", "ocp4-moderate-node-master", "rhcos4-moderate-worker", "rhcos4-moderate-master"}
+	for _, scan := range expectedScan {
+		found := false
+		for _, s := range suite.Status.ScanStatuses {
+			if s.Name == scan {
+				found = true
+				if s.Phase != compv1alpha1.PhaseDone {
+					t.Fatalf("expected scan %s to be done", scan)
+				}
+				if s.Result != compv1alpha1.ResultCompliant && s.Result != compv1alpha1.ResultNonCompliant {
+					t.Fatalf("expected scan %s to be compliant or non-compliant", scan)
+				}
+				break
+			}
+		}
+		if !found {
+			t.Fatalf("expected scan %s not found", scan)
+		}
+	}
+
+}
+
 func TestTolerations(t *testing.T) {
 	f := framework.Global
 	workerNodes, err := f.GetNodesWithSelector(map[string]string{