diff --git a/cmd/manager/aggregator.go b/cmd/manager/aggregator.go index 54fe6bec4..828ccd930 100644 --- a/cmd/manager/aggregator.go +++ b/cmd/manager/aggregator.go @@ -308,9 +308,6 @@ func createOrUpdateOneResult(crClient aggregatorCrClient, owner metav1.Object, l } res.SetLabels(labels) - if annotations != nil { - res.SetAnnotations(annotations) - } name := res.GetName() @@ -318,9 +315,17 @@ func createOrUpdateOneResult(crClient aggregatorCrClient, owner metav1.Object, l var err error if !exists { cmdLog.Info("Creating object", "kind", kind, "name", name) + annotations = setTimestampAnnotations(crClient, owner, annotations) + if annotations != nil { + res.SetAnnotations(annotations) + } err = crClient.getClient().Create(context.TODO(), res) } else { cmdLog.Info("Updating object", "kind", kind, "name", name) + annotations = setTimestampAnnotations(crClient, owner, annotations) + if annotations != nil { + res.SetAnnotations(annotations) + } err = crClient.getClient().Update(context.TODO(), res) } if err != nil && !errors.IsAlreadyExists(err) { @@ -336,6 +341,31 @@ func createOrUpdateOneResult(crClient aggregatorCrClient, owner metav1.Object, l return nil } +func setTimestampAnnotations(crClient aggregatorCrClient, owner metav1.Object, annotations map[string]string) map[string]string { + // Get the scan timestamp + scan := &compv1alpha1.ComplianceScan{} + err := backoff.Retry(func() error { + err := crClient.getClient().Get(context.TODO(), types.NamespacedName{ + Namespace: owner.GetNamespace(), + Name: owner.GetName(), + }, scan) + if err != nil { + return err + } + return nil + }, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), maxRetriesForTimestamp)) + if err != nil { + cmdLog.Error(err, "Failed to get scan", "ComplianceScan.Name", owner.GetName()) + return annotations + } + if annotations == nil { + annotations = make(map[string]string) + } + + annotations[compv1alpha1.LastScannedTimestampAnnotation] = scan.Status.StartTimestamp.Format(time.RFC3339) + return annotations +} + func shouldSkipRemediation( scan *compv1alpha1.ComplianceScan, rem *compv1alpha1.ComplianceRemediation, diff --git a/cmd/manager/common.go b/cmd/manager/common.go index 206745f0c..6791ef6eb 100644 --- a/cmd/manager/common.go +++ b/cmd/manager/common.go @@ -2,11 +2,12 @@ package manager import ( "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/discovery" "os" "path/filepath" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery" + ocpcfgv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/spf13/cobra" @@ -25,7 +26,8 @@ import ( ) const ( - maxRetries = 15 + maxRetries = 15 + maxRetriesForTimestamp = 3 ) var cmdLog = logf.Log.WithName("cmd") diff --git a/pkg/apis/compliance/v1alpha1/compliancecheckresult_types.go b/pkg/apis/compliance/v1alpha1/compliancecheckresult_types.go index fcc69d0c6..fa6773630 100644 --- a/pkg/apis/compliance/v1alpha1/compliancecheckresult_types.go +++ b/pkg/apis/compliance/v1alpha1/compliancecheckresult_types.go @@ -22,6 +22,9 @@ const ComplianceCheckResultHasRemediation = "compliance.openshift.io/automated-r // across the target nodes const ComplianceCheckInconsistentLabel = "compliance.openshift.io/inconsistent-check" +// LastScannedTimestampAnnotation +const LastScannedTimestampAnnotation = "compliance.openshift.io/last-scanned-timestamp" + // ComplianceCheckResultRuleAnnotation exposes the DNS-friendly name of a rule as a label. // This provides a way to link a result to a Rule object. const ComplianceCheckResultRuleAnnotation = "compliance.openshift.io/rule" diff --git a/tests/e2e/framework/common.go b/tests/e2e/framework/common.go index 1989eeed0..a9b685065 100644 --- a/tests/e2e/framework/common.go +++ b/tests/e2e/framework/common.go @@ -1151,6 +1151,43 @@ func (f *Framework) AssertScanIsCompliant(name, namespace string) error { return nil } +// AssertComplianceCheckResultTimestamps checks if the timestamps in the compliance check results are within the expected range +func (f *Framework) AssertComplianceCheckResultTimestamps(scanName, namespace string) error { + cs := &compv1alpha1.ComplianceScan{} + defer f.logContainerOutput(namespace, scanName) + err := f.Client.Get(context.TODO(), types.NamespacedName{Name: scanName, Namespace: namespace}, cs) + if err != nil { + return err + } + ccr := &compv1alpha1.ComplianceCheckResultList{} + lo := &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + "compliance.openshift.io/scan-name": scanName, + }), + } + err = f.Client.List(context.TODO(), ccr, lo) + if err != nil { + return err + } + for _, check := range ccr.Items { + annotations := check.GetAnnotations() + if annotations == nil { + return fmt.Errorf("Check %s has no annotations", check.Name) + } + if annotations[compv1alpha1.LastScannedTimestampAnnotation] == "" { + return fmt.Errorf("Check %s has no timestamp annotation", check.Name) + } + timestamp, err := time.Parse(time.RFC3339, annotations[compv1alpha1.LastScannedTimestampAnnotation]) + if err != nil { + return fmt.Errorf("Error parsing timestamp for check %s: %v", check.Name, err) + } + if timestamp.Before(cs.Status.StartTimestamp.Time) || timestamp.After(cs.Status.EndTimestamp.Time) { + return fmt.Errorf("Timestamp for check %s is not within the expected range: %v", check.Name, timestamp) + } + } + return nil +} + // AssertScanGUIDMatches checks if the scan has the expected GUID func (f *Framework) AssertScanGUIDMatches(name, namespace, expectedGUID string) error { cs := &compv1alpha1.ComplianceScan{} diff --git a/tests/e2e/parallel/main_test.go b/tests/e2e/parallel/main_test.go index 8707611f5..523a3cc3b 100644 --- a/tests/e2e/parallel/main_test.go +++ b/tests/e2e/parallel/main_test.go @@ -651,6 +651,63 @@ func TestSingleScanSucceeds(t *testing.T) { } } +func TestSingleScanTimestamps(t *testing.T) { + t.Parallel() + f := framework.Global + + scanName := framework.GetObjNameFromTest(t) + testScan := &compv1alpha1.ComplianceScan{ + ObjectMeta: metav1.ObjectMeta{ + Name: scanName, + Namespace: f.OperatorNamespace, + }, + Spec: compv1alpha1.ComplianceScanSpec{ + Profile: "xccdf_org.ssgproject.content_profile_moderate", + Content: framework.RhcosContentFile, + Rule: "xccdf_org.ssgproject.content_rule_no_netrc_files", + ComplianceScanSettings: compv1alpha1.ComplianceScanSettings{ + Debug: true, + }, + }, + } + // use Context's create helper to create the object and add a cleanup function for the new object + err := f.Client.Create(context.TODO(), testScan, nil) + if err != nil { + t.Fatalf("failed to create scan %s: %s", scanName, err) + } + defer f.Client.Delete(context.TODO(), testScan) + + err = f.WaitForScanStatus(f.OperatorNamespace, scanName, compv1alpha1.PhaseDone) + if err != nil { + t.Fatal(err) + } + + // assertComplianceCheckResultTimestamps checks that the timestamps are set + // and that they are set to the same value of startTimestamp of the scan + err = f.AssertComplianceCheckResultTimestamps(scanName, f.OperatorNamespace) + if err != nil { + t.Fatal(err) + } + + // rerun the scan + err = f.ReRunScan(scanName, f.OperatorNamespace) + if err != nil { + t.Fatal(err) + } + err = f.WaitForScanStatus(f.OperatorNamespace, scanName, compv1alpha1.PhaseDone) + if err != nil { + t.Fatal(err) + } + + // assertComplianceCheckResultTimestamps checks that the timestamps are set + // and that they are set to the same value of startTimestamp of the scan + err = f.AssertComplianceCheckResultTimestamps(scanName, f.OperatorNamespace) + if err != nil { + t.Fatal(err) + } + +} + func TestScanProducesRemediations(t *testing.T) { t.Parallel() f := framework.Global