CMP-2132: Implement suspend and resume scan schedule

This commit implements the logic and tests necessary to suspend and resume scan schedules using the `ScanSetting` custom resource. You can find more details on the overall justification, use cases, and implementation details in the enhancement: ComplianceAsCode#375
rhmdnd · Aug 31, 2023 · 064cdef · 064cdef
1 parent cd3744f
commit 064cdef
Show file tree

Hide file tree

Showing 16 changed files with 216 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,12 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Enhancements
 
--
+- Users can now pause scan schedules by setting the `ScanSetting.suspend`
+  attribute to `True`. This allows users to suspend a scan, and reactivate it
+  without having to delete and recreate the `ScanSettingBinding`, making it
+  more ergonomic to pause scans during maintenance periods. See the
+  [enhancement](https://github.com/ComplianceAsCode/compliance-operator/pull/375)
+  for more details.
 
 ### Fixes
 

diff --git a/config/crd/bases/compliance.openshift.io_compliancecheckresults.yaml b/config/crd/bases/compliance.openshift.io_compliancecheckresults.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: compliancecheckresults.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_complianceremediations.yaml b/config/crd/bases/compliance.openshift.io_complianceremediations.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: complianceremediations.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_compliancescans.yaml b/config/crd/bases/compliance.openshift.io_compliancescans.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: compliancescans.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_compliancesuites.yaml b/config/crd/bases/compliance.openshift.io_compliancesuites.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: compliancesuites.compliance.openshift.io
 spec:
   group: compliance.openshift.io
@@ -323,6 +323,10 @@ spec:
                   scheduled scans will start running only after the initial results
                   are ready.
                 type: string
+              suspend:
+                description: Defines if a schedule should be suspended and is a boolean
+                  value, defaulting to False.
+                type: boolean
             required:
             - scans
             type: object

diff --git a/config/crd/bases/compliance.openshift.io_profilebundles.yaml b/config/crd/bases/compliance.openshift.io_profilebundles.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: profilebundles.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_profiles.yaml b/config/crd/bases/compliance.openshift.io_profiles.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: profiles.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_rules.yaml b/config/crd/bases/compliance.openshift.io_rules.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: rules.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_scansettingbindings.yaml b/config/crd/bases/compliance.openshift.io_scansettingbindings.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: scansettingbindings.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_scansettings.yaml b/config/crd/bases/compliance.openshift.io_scansettings.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: scansettings.compliance.openshift.io
 spec:
   group: compliance.openshift.io
@@ -247,6 +247,10 @@ spec:
               be strict and error out. `false` means that we don't need to be strict
               and we can proceed.
             type: boolean
+          suspend:
+            description: Defines if a schedule should be suspended and is a boolean
+              value, defaulting to False.
+            type: boolean
           timeout:
             default: 30m
             description: Timeout is the maximum amount of time the scan can run. If

diff --git a/config/crd/bases/compliance.openshift.io_tailoredprofiles.yaml b/config/crd/bases/compliance.openshift.io_tailoredprofiles.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: tailoredprofiles.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/config/crd/bases/compliance.openshift.io_variables.yaml b/config/crd/bases/compliance.openshift.io_variables.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.12.0
+    controller-gen.kubebuilder.io/version: v0.12.1
   name: variables.compliance.openshift.io
 spec:
   group: compliance.openshift.io

diff --git a/pkg/apis/compliance/v1alpha1/compliancesuite_types.go b/pkg/apis/compliance/v1alpha1/compliancesuite_types.go
@@ -82,6 +82,9 @@ type ComplianceSuiteSettings struct {
 	// Note the scan will still be triggered immediately, and the scheduled
 	// scans will start running only after the initial results are ready.
 	Schedule string `json:"schedule,omitempty"`
+	// Defines if a schedule should be suspended and is a boolean value,
+	// defaulting to False.
+	Suspend bool `json:"suspend,omitempty"`
 }
 
 // ComplianceSuiteSpec defines the desired state of ComplianceSuite

diff --git a/pkg/apis/compliance/v1alpha1/scansettingbinding_types.go b/pkg/apis/compliance/v1alpha1/scansettingbinding_types.go
@@ -20,10 +20,10 @@ type ScanSettingBinding struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec        ScanSettingBindingSpec `json:"spec,omitempty"`
-	Profiles    []NamedObjectReference `json:"profiles,omitempty"`
-        // +kubebuilder:default={"name":"default","kind": "ScanSetting", "apiGroup": "compliance.openshift.io/v1alpha1"}
-	SettingsRef *NamedObjectReference  `json:"settingsRef,omitempty"`
+	Spec     ScanSettingBindingSpec `json:"spec,omitempty"`
+	Profiles []NamedObjectReference `json:"profiles,omitempty"`
+	// +kubebuilder:default={"name":"default","kind": "ScanSetting", "apiGroup": "compliance.openshift.io/v1alpha1"}
+	SettingsRef *NamedObjectReference `json:"settingsRef,omitempty"`
 	// +optional
 	Status ScanSettingBindingStatus `json:"status,omitempty"`
 }

diff --git a/pkg/controller/compliancesuite/suitererunner_cron_compat.go b/pkg/controller/compliancesuite/suitererunner_cron_compat.go
@@ -77,6 +77,7 @@ func (r *ReconcileComplianceSuite) cronJobCompatCreate(
 		}
 		cronJobCopy := getObjTyped.DeepCopy()
 		cronJobCopy.Spec.Schedule = suite.Spec.Schedule
+		cronJobCopy.Spec.Suspend = &suite.Spec.Suspend
 		logger.Info("Updating v1 rerunner", "CronJob.Name", cronJobCopy.GetName())
 		return r.Client.Update(context.TODO(), cronJobCopy)
 	}

diff --git a/tests/e2e/parallel/main_test.go b/tests/e2e/parallel/main_test.go
@@ -9,8 +9,11 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	compv1alpha1 "github.com/ComplianceAsCode/compliance-operator/pkg/apis/compliance/v1alpha1"
+	compsuitectrl "github.com/ComplianceAsCode/compliance-operator/pkg/controller/compliancesuite"
+	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -2795,3 +2798,183 @@ func TestScheduledSuiteTimeoutFail(t *testing.T) {
 		t.Fatal("The scan should have the timeout annotation")
 	}
 }
+
+func TestSuspendScanSetting(t *testing.T) {
+	t.Parallel()
+	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",
+			},
+		},
+		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)
+	}
+
+	// Assert the ComplianceSuite.suspend attribute is False
+	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)
+	}
+	if suite.Spec.Suspend == true {
+		t.Fatalf("ComplianceSuite %s is suspended when it should not be", suite.Name)
+	}
+
+	// Assert the CronJob associated with the ComplianceSuite is False
+	job := &batchv1.CronJob{}
+	jobName := compsuitectrl.GetRerunnerName(suite.Name)
+	err := f.WaitForObjectToExist(jobName, f.OperatorNamespace, job)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if *job.Spec.Suspend == true {
+		t.Fatalf("CronJob %s is suspended when it should not be", jobName)
+	}
+
+	// Suspend the `ScanSetting` using the `suspend` attribute
+	scanSettingUpdate := &compv1alpha1.ScanSetting{}
+	if err := f.Client.Get(context.TODO(), types.NamespacedName{Namespace: f.OperatorNamespace, Name: scanSettingName}, scanSettingUpdate); err != nil {
+		t.Fatalf("failed to get ScanSetting %s", scanSettingName)
+	}
+	scanSettingUpdate.Suspend = true
+	scanSettingUpdate.Schedule = "0 2 * * *"
+	if err := f.Client.Update(context.TODO(), scanSettingUpdate); err != nil {
+		t.Fatal(err)
+	}
+	log.Printf("Updated %s", scanSettingUpdate.Name)
+
+	time.Sleep(10 * time.Second)
+
+	// Assert the `CronJob` associated with the `ComplianceSuite` is set to `suspend=true`
+	job = &batchv1.CronJob{}
+	jobName = compsuitectrl.GetRerunnerName(suite.Name)
+	err = f.Client.Get(context.TODO(), types.NamespacedName{Name: jobName, Namespace: f.OperatorNamespace}, job)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if *job.Spec.Suspend == false {
+		t.Fatalf("Expected CronJob %s to be suspended", jobName)
+	}
+
+	// Resume the `ComplianceScan` by updating the `ScanSetting.suspend` attribute to `False`
+	scanSetting.Suspend = false
+	if err := f.Client.Update(context.TODO(), &scanSetting); err != nil {
+		t.Fatal(err)
+	}
+
+	// Assert the `CronJob` associated with the `ComplianceScan` is set to `suspend=false`
+	job = &batchv1.CronJob{}
+	jobName = compsuitectrl.GetRerunnerName(suite.Name)
+	err = f.Client.Get(context.TODO(), types.NamespacedName{Name: jobName, Namespace: f.OperatorNamespace}, job)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if *job.Spec.Suspend == true {
+		t.Fatalf("CronJob %s is suspended when it should not be", jobName)
+	}
+
+}
+
+func TestSuspendScanSettinDoesNotCreateScanAfterBinding(t *testing.T) {
+	t.Parallel()
+	f := framework.Global
+
+	// Creates a new `ScanSetting` with `suspend` set to `True`
+	scanSettingName := framework.GetObjNameFromTest(t) + "-scansetting"
+	scanSetting := compv1alpha1.ScanSetting{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      scanSettingName,
+			Namespace: f.OperatorNamespace,
+		},
+		ComplianceSuiteSettings: compv1alpha1.ComplianceSuiteSettings{
+			AutoApplyRemediations: false,
+			Suspend:               true,
+		},
+		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",
+			},
+		},
+		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)
+
+	// Assert that no scan takes place
+
+	// Update the `ScanSetting.suspend` attribute to `False`
+	scanSetting.Suspend = false
+	if err := f.Client.Update(context.TODO(), &scanSetting); err != nil {
+		t.Fatal(err)
+	}
+	// Assert the scan is performed
+	if err := f.WaitForSuiteScansStatus(f.OperatorNamespace, bindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant); err != nil {
+		t.Fatal(err)
+	}
+}