ROX-27490: generate partial reports per cluster by lvalerom · Pull Request #15408 · stackrox/stackrox

lvalerom · 2025-05-22T13:41:05Z

Description

This a follow-up to #15231. Now if a scan fails in a cluster but another one succeeds we do not mark the entire cluster as failed. In those situations we create two reports for that cluster with the following formats:

File with the results of the successful scans cluster_<cluster-name>_<ok-profile-1>_<ok-profile_2>_<timestamp>.csv
File with the failure reasons of the failed scans failed_cluster_<cluster-name>_<nok-profile-3>_<nok-profile_4>_<timestamp>.csv

User-facing documentation

CHANGELOG.md is updated OR update is not needed
documentation PR is created and is linked above OR is not needed

Testing and quality

the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
CI results are inspected

Automated testing

How I validated my change

Unit tests added
Manual testing:
- Deploy in two clusters with Compliance Operator
- Create a Scan Schedule with both cluster
- Make one of the scans fail mid scan (e.g. disconnect sensor from central) after other scans are reported as successful
- Observe the report has two files for the partially failed cluster and the content of the files is correct.

openshift-ci · 2025-05-22T13:41:09Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

rhacs-bot · 2025-05-22T14:22:55Z

Images are ready for the commit at a743516.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-930-ga743516256.

codecov · 2025-05-22T18:48:51Z

Codecov Report

Attention: Patch coverage is 80.10471% with 38 lines in your changes missing coverage. Please review.

Project coverage is 48.75%. Comparing base (9111ac8) to head (a743516).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
...mplianceoperator/v2/report/manager/manager_impl.go	50.00%	19 Missing and 7 partials ⚠️
...anceoperator/v2/report/datastore/datastore_impl.go	76.92%	2 Missing and 1 partial ⚠️
...anceoperator/v2/report/manager/format/formatter.go	86.36%	2 Missing and 1 partial ⚠️
...eoperator/v2/report/manager/helpers/clusterdata.go	95.52%	2 Missing and 1 partial ⚠️
...or/v2/report/manager/results/results_aggregator.go	84.21%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #15408    +/-   ##
========================================
  Coverage   48.75%   48.75%            
========================================
  Files        2585     2587     +2     
  Lines      190093   190298   +205     
========================================
+ Hits        92671    92771   +100     
- Misses      90146    90241    +95     
- Partials     7276     7286    +10

Flag	Coverage Δ
go-unit-tests	`48.75% <80.10%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vikin91

Posting only partial review - I need to switch to another task, but will continue here later.

central/complianceoperator/v2/report/manager/generator/report_gen_impl.go

central/complianceoperator/v2/report/request.go

central/complianceoperator/v2/report/manager/manager_impl.go

central/complianceoperator/v2/report/manager/results/results_aggregator.go

vikin91

This PR is huge.. I tried to review without tests in the first pass, but that is a lot.
Posting only partial review so far. Will continue tomorrow.

central/complianceoperator/v2/report/manager/format/formatter.go

central/complianceoperator/v2/report/manager/manager_impl.go

lvalerom · 2025-06-03T11:24:21Z

central/complianceoperator/v2/report/manager/results/results_aggregator.go

+	if clusterData.FailedInfo != nil {
+		allScansSet := set.NewStringSet(scans...)
+		failedScansSet := set.NewStringSet()
+		for _, scan := range clusterData.FailedInfo.Scans {
+			failedScansSet.Add(scan.GetScanName())
+		}
+		scans = allScansSet.Difference(failedScansSet).AsSlice()


Add a test that asserts on the number of scans in this query

I gave it a try but I'll have to change the test structure to make it work. I'd prefer to do this one in a follow-up.

central/complianceoperator/v2/scans/datastore/datastore_impl_test.go

central/complianceoperator/v2/report/manager/helpers/clusterdata.go

central/complianceoperator/v2/report/manager/helpers/clusterdata_test.go

central/complianceoperator/v2/report/manager/results/results_aggregator.go

central/complianceoperator/v2/report/manager/helpers/clusterdata.go

mtodor

Re-checked all new changes. They look good!

Added a few nitpicks, but I suggest doing them in a follow-up. We should not postpone merging this PR.

mtodor · 2025-06-04T07:20:32Z

central/complianceoperator/v2/report/manager/helpers/clusterdata_test.go

+	t.Run("failure retrieving snapshot from the store", func(tt *testing.T) {
+		snapshotStore.EXPECT().
+			GetLastSnapshotFromScanConfig(gomock.Any(), gomock.Eq(scanConfigID)).
+			Times(1).Return(nil, errors.New("some error"))


nitpicking: it would be good to test also Return(nil, nil) case.

mtodor · 2025-06-04T07:43:42Z

central/complianceoperator/v2/report/manager/helpers/clusterdata.go

+	return data, nil
+}
+
+func populateFailedScans(ctx context.Context, failedScanNames []string, snapshotScans []*storage.ComplianceOperatorReportSnapshotV2_Scan, scanStore scanDS.DataStore) ([]*storage.ComplianceOperatorScanV2, error) {


nitpick: populateFailedScans name is a bit misleading. We are actually not populating scans here. We are just returning them. Maybe getFailedScans would be a bit more accurate.

For populateScanNames - name stands, because it populates scans in data *report.ClusterData.

mtodor · 2025-06-04T07:49:12Z

central/complianceoperator/v2/report/manager/helpers/clusterdata.go

+	return clusterData, nil
+}
+
+func populateScanNames(ctx context.Context, data *report.ClusterData, reportData *storage.ComplianceOperatorReportData, clusterID string, scanStore scanDS.DataStore) (*report.ClusterData, error) {


nitpick: clusterID is redundant. We could get it from data.ClusterId.

lvalerom · 2025-06-04T10:18:18Z

@mtodor @vikin91 I've resolved all the comments to be able to merge but I'll follow up on them.

openshift-ci bot added the do-not-merge/work-in-progress label May 22, 2025

lvalerom force-pushed the lvm/rox-27490-handle-failed-clusters branch from 7ecb259 to 3b1318e Compare May 22, 2025 13:42

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch from 0b7bdbc to 00c6f42 Compare May 22, 2025 13:49

github-actions bot added the area/central label May 22, 2025

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 4 times, most recently from bf9dcf4 to 86c9f07 Compare May 22, 2025 17:32

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 4 times, most recently from 7c413b5 to aa4e81b Compare May 23, 2025 16:48

lvalerom mentioned this pull request May 26, 2025

ROX-27490: Handle failed clusters #15231

Merged

11 tasks

lvalerom force-pushed the lvm/rox-27490-handle-failed-clusters branch from 3b1318e to 438ccf3 Compare May 26, 2025 16:42

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch from 7e35a40 to 0bd6652 Compare May 27, 2025 13:22

lvalerom force-pushed the lvm/rox-27490-handle-failed-clusters branch from 91af2e4 to 9253ce9 Compare May 27, 2025 16:07

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch from 0bd6652 to 3fec723 Compare May 28, 2025 11:04

lvalerom marked this pull request as ready for review May 28, 2025 11:13

openshift-ci bot removed the do-not-merge/work-in-progress label May 28, 2025

lvalerom requested review from mtodor and vikin91 May 28, 2025 11:13

vikin91 reviewed May 28, 2025

View reviewed changes

Base automatically changed from lvm/rox-27490-handle-failed-clusters to master May 28, 2025 13:59

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 2 times, most recently from 458c94d to 087d20e Compare May 30, 2025 05:34

lvalerom requested a review from a team as a code owner May 30, 2025 05:34

github-actions bot added the area/postgres label May 30, 2025

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 5 times, most recently from f2fc1c6 to c69a973 Compare May 30, 2025 09:41

lvalerom force-pushed the lvm/rox-29251-add-last-started-field-to-check-results-proto branch from a239f56 to 651ff66 Compare May 30, 2025 16:02

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 2 times, most recently from b0bfbd0 to 753eac7 Compare May 30, 2025 17:01

lvalerom force-pushed the lvm/rox-29251-add-last-started-field-to-check-results-proto branch from 651ff66 to 7e732ee Compare June 2, 2025 08:43

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 2 times, most recently from a73e3aa to 714edd8 Compare June 2, 2025 13:13

vikin91 reviewed Jun 2, 2025

View reviewed changes

central/complianceoperator/v2/report/manager/format/formatter.go Outdated Show resolved Hide resolved

central/complianceoperator/v2/report/manager/format/formatter.go Outdated Show resolved Hide resolved

Base automatically changed from lvm/rox-29251-add-last-started-field-to-check-results-proto to master June 3, 2025 05:50

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch from 714edd8 to f3d2c17 Compare June 3, 2025 06:01

lvalerom commented Jun 3, 2025

View reviewed changes

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch 2 times, most recently from c6d2239 to 2513ff8 Compare June 3, 2025 15:33

mtodor reviewed Jun 3, 2025

View reviewed changes

central/complianceoperator/v2/report/manager/results/results_aggregator.go Outdated Show resolved Hide resolved

central/complianceoperator/v2/report/manager/helpers/clusterdata.go Outdated Show resolved Hide resolved

lvalerom added 7 commits June 4, 2025 08:35

generate partial reports per cluster

ad33a7b

style

87daaef

fix on demand report generation with cluster data refactors

3940915

fix unit after rebase

a1e2cba

review plus removing profile names from file name

7849aee

update protolock

ee99884

review

a743516

lvalerom force-pushed the lvm/rox-27490-generate-partial-reports-per-cluster branch from 2bd3059 to a743516 Compare June 4, 2025 06:43

mtodor approved these changes Jun 4, 2025

View reviewed changes

lvalerom merged commit 218830d into master Jun 4, 2025
102 checks passed

lvalerom deleted the lvm/rox-27490-generate-partial-reports-per-cluster branch June 4, 2025 10:18

Conversation

lvalerom commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

User-facing documentation

Testing and quality

Automated testing

How I validated my change

Uh oh!

openshift-ci bot commented May 22, 2025

Uh oh!

rhacs-bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vikin91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vikin91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lvalerom Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

lvalerom Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtodor left a comment

Choose a reason for hiding this comment

Uh oh!

mtodor Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

mtodor Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

mtodor Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

lvalerom commented Jun 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lvalerom commented May 22, 2025 •

edited

Loading

rhacs-bot commented May 22, 2025 •

edited

Loading

codecov bot commented May 22, 2025 •

edited

Loading