From 064cdef10107ed36eff4c7f024b41030708a5386 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Wed, 30 Aug 2023 20:19:44 -0500 Subject: [PATCH] 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: https://github.com/ComplianceAsCode/compliance-operator/pull/375 --- CHANGELOG.md | 7 +- ...e.openshift.io_compliancecheckresults.yaml | 2 +- ...e.openshift.io_complianceremediations.yaml | 2 +- ...mpliance.openshift.io_compliancescans.yaml | 2 +- ...pliance.openshift.io_compliancesuites.yaml | 6 +- ...ompliance.openshift.io_profilebundles.yaml | 2 +- .../compliance.openshift.io_profiles.yaml | 2 +- .../bases/compliance.openshift.io_rules.yaml | 2 +- ...ance.openshift.io_scansettingbindings.yaml | 2 +- .../compliance.openshift.io_scansettings.yaml | 6 +- ...pliance.openshift.io_tailoredprofiles.yaml | 2 +- .../compliance.openshift.io_variables.yaml | 2 +- .../v1alpha1/compliancesuite_types.go | 3 + .../v1alpha1/scansettingbinding_types.go | 8 +- .../suitererunner_cron_compat.go | 1 + tests/e2e/parallel/main_test.go | 183 ++++++++++++++++++ 16 files changed, 216 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65bb59657..87b6feede 100644 --- 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 index ff8f26350..0210ca14a 100644 --- 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 index a506201cf..a9d273df7 100644 --- 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 index 875c9252a..55ac99ff8 100644 --- 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 index b05225a94..dd21fd434 100644 --- 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 index 490fe9be7..4bb904c9e 100644 --- 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 index 82022c16c..5d4bed10a 100644 --- 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 index 7da9d002a..344b5be4b 100644 --- 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 index d44fe3622..d58995962 100644 --- 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 index b5ef50246..d3d30a337 100644 --- 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 index 9d59f9509..e5425bf84 100644 --- 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 index 99d021332..43046a125 100644 --- 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 index 8b14709d0..20c9fb3e6 100644 --- 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 index 1f6df1f2f..0b3febd0d 100644 --- 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 index bb1974e16..8cbc8e7bf 100644 --- 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 index b85b1d05a..536c7bbd7 100644 --- 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) + } +}