Skip to content

Commit

Permalink
OCPBUGS-29272: Delete scan when SSB remove a profile
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Vincent056 committed Feb 9, 2024
1 parent 479601a commit 5dacf2e
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 1 deletion.
21 changes: 20 additions & 1 deletion pkg/controller/compliancesuite/compliancesuite_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -257,8 +258,26 @@ 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", "ComplianceScan.Name", scan.Name)
if err := r.Client.Delete(context.TODO(), &scan); err != nil {
return false, err
}
}
}

return false, nil
Expand Down
10 changes: 10 additions & 0 deletions tests/e2e/framework/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
90 changes: 90 additions & 0 deletions tests/e2e/serial/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,96 @@ func TestSuspendScanSetting(t *testing.T) {
}
}

func TestRemoveProfileScan(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) + "-scansetting"
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-cis",
Kind: "Profile",
APIGroup: "compliance.openshift.io/v1alpha1",
},
{
Name: "ocp4-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 first scan completes since the CronJob is created
// after the scan is done
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)
}

// 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)
}

// Check if the scan for `ocp4-moderate` profile is removed
if err := f.AssertScanDoesNotExist("ocp4-moderate", f.OperatorNamespace); err != nil {
t.Fatal(err)
}
}

func TestSuspendScanSettingDoesNotCreateScan(t *testing.T) {
f := framework.Global

Expand Down

0 comments on commit 5dacf2e

Please sign in to comment.