From 8bb58f53064f986d8a876728af9f14c9779de407 Mon Sep 17 00:00:00 2001 From: lvalerom Date: Wed, 7 May 2025 09:36:03 +0200 Subject: [PATCH 1/5] generate file for each failed cluster --- .../v2/report/manager/format/formatter.go | 35 +++- .../report/manager/format/formatter_test.go | 163 +++++++++++++++--- .../manager/generator/mocks/report_gen.go | 8 +- .../v2/report/manager/generator/report_gen.go | 2 +- .../manager/generator/report_gen_impl.go | 2 +- .../manager/generator/report_gen_impl_test.go | 6 +- 6 files changed, 184 insertions(+), 32 deletions(-) diff --git a/central/complianceoperator/v2/report/manager/format/formatter.go b/central/complianceoperator/v2/report/manager/format/formatter.go index f5f850b13edaf..0dfb6e783c388 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter.go +++ b/central/complianceoperator/v2/report/manager/format/formatter.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/stackrox/rox/central/complianceoperator/v2/report" + "github.com/stackrox/rox/generated/storage" "github.com/stackrox/rox/pkg/csv" ) @@ -27,6 +28,12 @@ var ( "Rationale", "Instructions", } + failedClusterCSVHeader = []string{ + "Cluster ID", + "Cluster Name", + "Reason", + "Compliance Operator Version", + } ) //go:generate mockgen-wrapper @@ -53,7 +60,7 @@ func NewFormatter() *FormatterImpl { } } -func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow) (buffRet *bytes.Buffer, errRet error) { +func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow, failedClusters map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) (buffRet *bytes.Buffer, errRet error) { var buf bytes.Buffer zipWriter := f.newZipWriter(&buf) defer func() { @@ -64,6 +71,13 @@ func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow) }() for clusterID, res := range results { fileName := fmt.Sprintf("cluster_%s.csv", clusterID) + if failedCluster, ok := failedClusters[clusterID]; ok { + fileName = fmt.Sprintf("failed_%s", fileName) + if err := f.createFailedClusterFileInZip(zipWriter, fileName, failedCluster); err != nil { + return nil, errors.Wrap(err, "error creating failed cluster report") + } + continue + } err := f.createCSVInZip(zipWriter, fileName, res) if err != nil { return nil, errors.Wrap(err, "error creating csv report") @@ -102,6 +116,25 @@ func generateRecord(row *report.ResultRow) []string { } } +func (f *FormatterImpl) createFailedClusterFileInZip(zipWriter ZipWriter, filename string, failedCluster *storage.ComplianceOperatorReportSnapshotV2_FailedCluster) error { + w, err := zipWriter.Create(filename) + if err != nil { + return err + } + csvWriter := f.newCSVWriter(failedClusterCSVHeader, true) + csvWriter.AddValue(generateFailRecord(failedCluster)) + return csvWriter.WriteCSV(w) +} + +func generateFailRecord(failedCluster *storage.ComplianceOperatorReportSnapshotV2_FailedCluster) []string { + return []string{ + failedCluster.GetClusterId(), + failedCluster.GetClusterName(), + failedCluster.GetReason(), + failedCluster.GetOperatorVersion(), + } +} + func createNewZipWriter(buf *bytes.Buffer) ZipWriter { return zip.NewWriter(buf) } diff --git a/central/complianceoperator/v2/report/manager/format/formatter_test.go b/central/complianceoperator/v2/report/manager/format/formatter_test.go index 84f17e6b46d1e..390d52d543721 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter_test.go +++ b/central/complianceoperator/v2/report/manager/format/formatter_test.go @@ -8,6 +8,7 @@ import ( "github.com/pkg/errors" "github.com/stackrox/rox/central/complianceoperator/v2/report" "github.com/stackrox/rox/central/complianceoperator/v2/report/manager/format/mocks" + "github.com/stackrox/rox/generated/storage" "github.com/stackrox/rox/pkg/csv" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" @@ -27,39 +28,93 @@ type ComplianceReportingFormatterSuite struct { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { - matcher := &valueMatcher{ - data: getFakeReportData(), - } - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) - s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { - matcher.recordNumber++ + s.Run("with empty failed clusters", func() { + matcher := &valueMatcher{ + data: getFakeReportData(), + } + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { + matcher.recordNumber++ + }) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) + + buf, err := s.formatter.FormatCSVReport(getFakeReportData(), getFakeEmptyFailedClusters()) + s.Require().NoError(err) + s.Require().NotNil(buf) }) - s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) + s.Run("with nil failed clusters", func() { + matcher := &valueMatcher{ + data: getFakeReportData(), + } + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { + matcher.recordNumber++ + }) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) + + buf, err := s.formatter.FormatCSVReport(getFakeReportData(), nil) + s.Require().NoError(err) + s.Require().NotNil(buf) + }) +} + +func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedClusterNoError() { + matcher := &failedClusterValueMatcher{} + matcher.data, matcher.failed = getFakeReportDataWithFailedCluster() + s.zipWriter.EXPECT().Create(gomock.Any()).Times(2).Return(nil, nil) + s.csvWriter.EXPECT().AddValue(matcher).Times(3) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(2).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) - buf, err := s.formatter.FormatCSVReport(getFakeReportData()) + buf, err := s.formatter.FormatCSVReport(getFakeReportDataWithFailedCluster()) s.Require().NoError(err) s.Require().NotNil(buf) } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, errors.New("error")) - s.zipWriter.EXPECT().Close().Times(1).Return(nil) + s.Run("report data", func() { + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, errors.New("error")) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) - buf, err := s.formatter.FormatCSVReport(getFakeReportData()) - s.Require().Error(err) - s.Require().Nil(buf) + buf, err := s.formatter.FormatCSVReport(getFakeReportData(), getFakeEmptyFailedClusters()) + s.Require().Error(err) + s.Require().Nil(buf) + }) + s.Run("failed cluster data", func() { + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, errors.New("error")) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) + + buf, err := s.formatter.FormatCSVReport(getFakeReportDataOnlyFailedCluster()) + s.Require().Error(err) + s.Require().Nil(buf) + }) } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) - s.csvWriter.EXPECT().AddValue(gomock.Any()).Times(2) - s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) - s.zipWriter.EXPECT().Close().Times(1).Return(nil) + s.Run("report date", func() { + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.csvWriter.EXPECT().AddValue(gomock.Any()).Times(2) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) - buf, err := s.formatter.FormatCSVReport(getFakeReportData()) - s.Require().Error(err) - s.Require().Nil(buf) + buf, err := s.formatter.FormatCSVReport(getFakeReportData(), getFakeEmptyFailedClusters()) + s.Require().Error(err) + s.Require().Nil(buf) + }) + s.Run("failed cluster date", func() { + matcher := &failedClusterValueMatcher{} + matcher.data, matcher.failed = getFakeReportDataOnlyFailedCluster() + s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.csvWriter.EXPECT().AddValue(matcher).Times(1) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) + + buf, err := s.formatter.FormatCSVReport(getFakeReportDataOnlyFailedCluster()) + s.Require().Error(err) + s.Require().Nil(buf) + }) } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCloseError() { @@ -68,7 +123,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCloseError() { s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(errors.New("error")) - buf, err := s.formatter.FormatCSVReport(getFakeReportData()) + buf, err := s.formatter.FormatCSVReport(getFakeReportData(), getFakeEmptyFailedClusters()) s.Require().Error(err) s.Require().Nil(buf) } @@ -82,7 +137,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportEmptyReportNoErr s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) - buf, err := s.formatter.FormatCSVReport(getFakeEmptyReportData()) + buf, err := s.formatter.FormatCSVReport(getFakeEmptyReportData(), getFakeEmptyFailedClusters()) s.Require().NoError(err) s.Require().NotNil(buf) } @@ -141,6 +196,36 @@ func getFakeReportData() map[string][]*report.ResultRow { return results } +func getFakeReportDataWithFailedCluster() (map[string][]*report.ResultRow, map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) { + failedClusters := make(map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) + failedClusters["cluster-2"] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ + ClusterName: "test_cluster-2", + ClusterId: "test_cluster-2-id", + Reason: "timeout", + OperatorVersion: "v1.6.0", + } + results := getFakeReportData() + results["cluster-2"] = []*report.ResultRow{} + return results, failedClusters +} + +func getFakeReportDataOnlyFailedCluster() (map[string][]*report.ResultRow, map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) { + failedClusters := make(map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) + failedClusters["cluster-2"] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ + ClusterName: "test_cluster-2", + ClusterId: "test_cluster-2-id", + Reason: "timeout", + OperatorVersion: "v1.6.0", + } + results := make(map[string][]*report.ResultRow) + results["cluster-2"] = []*report.ResultRow{} + return results, failedClusters +} + +func getFakeEmptyFailedClusters() map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster { + return make(map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) +} + type emptyValueMatcher struct { value string data map[string][]*report.ResultRow @@ -204,3 +289,37 @@ func compareStringSlice(a []string, b []string) bool { func (m *valueMatcher) String() string { return m.error } + +type failedClusterValueMatcher struct { + data map[string][]*report.ResultRow + failed map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster + error string +} + +func (m *failedClusterValueMatcher) Matches(target interface{}) bool { + record, ok := target.(csv.Value) + if !ok { + m.error = "target is not of type csv.Value" + return false + } + for _, clusterData := range m.data { + for _, check := range clusterData { + if compareStringSlice(record, generateRecord(check)) { + return true + } + } + } + + for _, failedCluster := range m.failed { + if compareStringSlice(record, generateFailRecord(failedCluster)) { + return true + } + } + + m.error = fmt.Sprintf("AddValue called with an unexpected record %v", record) + return false +} + +func (m *failedClusterValueMatcher) String() string { + return m.error +} diff --git a/central/complianceoperator/v2/report/manager/generator/mocks/report_gen.go b/central/complianceoperator/v2/report/manager/generator/mocks/report_gen.go index 5d0bbc625256f..d98f1d8ee0899 100644 --- a/central/complianceoperator/v2/report/manager/generator/mocks/report_gen.go +++ b/central/complianceoperator/v2/report/manager/generator/mocks/report_gen.go @@ -94,18 +94,18 @@ func (m *MockFormatter) EXPECT() *MockFormatterMockRecorder { } // FormatCSVReport mocks base method. -func (m *MockFormatter) FormatCSVReport(arg0 map[string][]*report.ResultRow) (*bytes.Buffer, error) { +func (m *MockFormatter) FormatCSVReport(arg0 map[string][]*report.ResultRow, arg1 map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) (*bytes.Buffer, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FormatCSVReport", arg0) + ret := m.ctrl.Call(m, "FormatCSVReport", arg0, arg1) ret0, _ := ret[0].(*bytes.Buffer) ret1, _ := ret[1].(error) return ret0, ret1 } // FormatCSVReport indicates an expected call of FormatCSVReport. -func (mr *MockFormatterMockRecorder) FormatCSVReport(arg0 any) *gomock.Call { +func (mr *MockFormatterMockRecorder) FormatCSVReport(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FormatCSVReport", reflect.TypeOf((*MockFormatter)(nil).FormatCSVReport), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FormatCSVReport", reflect.TypeOf((*MockFormatter)(nil).FormatCSVReport), arg0, arg1) } // MockResultsAggregator is a mock of ResultsAggregator interface. diff --git a/central/complianceoperator/v2/report/manager/generator/report_gen.go b/central/complianceoperator/v2/report/manager/generator/report_gen.go index d5d7f53cf717a..91725ca0cd9fc 100644 --- a/central/complianceoperator/v2/report/manager/generator/report_gen.go +++ b/central/complianceoperator/v2/report/manager/generator/report_gen.go @@ -34,7 +34,7 @@ type ComplianceReportGenerator interface { // //go:generate mockgen-wrapper type Formatter interface { - FormatCSVReport(map[string][]*report.ResultRow) (*bytes.Buffer, error) + FormatCSVReport(map[string][]*report.ResultRow, map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) (*bytes.Buffer, error) } // ResultsAggregator interface is used to generate the report data diff --git a/central/complianceoperator/v2/report/manager/generator/report_gen_impl.go b/central/complianceoperator/v2/report/manager/generator/report_gen_impl.go index 3f216c2ba0501..4c7ca45aaa0e2 100644 --- a/central/complianceoperator/v2/report/manager/generator/report_gen_impl.go +++ b/central/complianceoperator/v2/report/manager/generator/report_gen_impl.go @@ -82,7 +82,7 @@ func (rg *complianceReportGeneratorImpl) ProcessReportRequest(req *report.Reques reportData := rg.resultsAggregator.GetReportData(req) - zipData, err := rg.formatter.FormatCSVReport(reportData.ResultCSVs) + zipData, err := rg.formatter.FormatCSVReport(reportData.ResultCSVs, nil) if err != nil { if dbErr := reportUtils.UpdateSnapshotOnError(req.Ctx, snapshot, report.ErrReportGeneration, rg.snapshotDS); dbErr != nil { return errors.Wrap(dbErr, "unable to update the snapshot on report generation failure") diff --git a/central/complianceoperator/v2/report/manager/generator/report_gen_impl_test.go b/central/complianceoperator/v2/report/manager/generator/report_gen_impl_test.go index 60c1ed670c648..32dbca720a21a 100644 --- a/central/complianceoperator/v2/report/manager/generator/report_gen_impl_test.go +++ b/central/complianceoperator/v2/report/manager/generator/report_gen_impl_test.go @@ -111,7 +111,7 @@ func (s *ComplainceReportingTestSuite) TestProcessReportRequest() { ReportStatus: &storage.ComplianceOperatorReportStatus{}, }, true, nil) s.resultsAggregator.EXPECT().GetReportData(gomock.Any()).Times(1).Return(&report.Results{}) - s.formatter.EXPECT().FormatCSVReport(gomock.Any()).Times(1) + s.formatter.EXPECT().FormatCSVReport(gomock.Any(), gomock.Any()).Times(1) s.snapshotDS.EXPECT().UpsertSnapshot(gomock.Any(), gomock.Any()).Times(1). DoAndReturn(func(_ any, snapshot *storage.ComplianceOperatorReportSnapshotV2) error { s.Require().Equal(storage.ComplianceOperatorReportStatus_GENERATED, snapshot.GetReportStatus().GetRunState()) @@ -128,7 +128,7 @@ func (s *ComplainceReportingTestSuite) TestProcessReportRequest() { ReportStatus: &storage.ComplianceOperatorReportStatus{}, }, true, nil) s.resultsAggregator.EXPECT().GetReportData(gomock.Any()).Times(1).Return(&report.Results{}) - s.formatter.EXPECT().FormatCSVReport(gomock.Any()).Times(1) + s.formatter.EXPECT().FormatCSVReport(gomock.Any(), gomock.Any()).Times(1) gomock.InOrder( s.snapshotDS.EXPECT().UpsertSnapshot(gomock.Any(), gomock.Any()).Times(1). DoAndReturn(func(_ any, snapshot *storage.ComplianceOperatorReportSnapshotV2) error { @@ -170,7 +170,7 @@ func (s *ComplainceReportingTestSuite) TestProcessReportRequest() { ReportStatus: &storage.ComplianceOperatorReportStatus{}, }, true, nil) s.resultsAggregator.EXPECT().GetReportData(gomock.Any()).Times(1).Return(&report.Results{}) - s.formatter.EXPECT().FormatCSVReport(gomock.Any()).Times(1) + s.formatter.EXPECT().FormatCSVReport(gomock.Any(), gomock.Any()).Times(1) gomock.InOrder( s.snapshotDS.EXPECT().UpsertSnapshot(gomock.Any(), gomock.Any()).Times(1). DoAndReturn(func(_ any, snapshot *storage.ComplianceOperatorReportSnapshotV2) error { From 74c62765ca443f8cac49dafce0c6eb9f02a5b17d Mon Sep 17 00:00:00 2001 From: lvalerom Date: Tue, 13 May 2025 12:39:12 +0200 Subject: [PATCH 2/5] assume no failed clusters are in the results --- .../v2/report/manager/format/formatter.go | 19 +++++----- .../report/manager/format/formatter_test.go | 38 +++++++++++-------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/central/complianceoperator/v2/report/manager/format/formatter.go b/central/complianceoperator/v2/report/manager/format/formatter.go index 0dfb6e783c388..5a3ca4b8679c1 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter.go +++ b/central/complianceoperator/v2/report/manager/format/formatter.go @@ -13,7 +13,9 @@ import ( ) const ( - EmptyValue = "Data not found for the cluster" + EmptyValue = "Data not found for the cluster" + successfulClusterFmt = "cluster_%s.csv" + failedClusterFmt = "failed_cluster_%s.csv" ) var ( @@ -69,15 +71,14 @@ func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow, errRet = errors.Wrap(err, "unable to create a zip file of the compliance report") } }() - for clusterID, res := range results { - fileName := fmt.Sprintf("cluster_%s.csv", clusterID) - if failedCluster, ok := failedClusters[clusterID]; ok { - fileName = fmt.Sprintf("failed_%s", fileName) - if err := f.createFailedClusterFileInZip(zipWriter, fileName, failedCluster); err != nil { - return nil, errors.Wrap(err, "error creating failed cluster report") - } - continue + for clusterID, failedCluster := range failedClusters { + fileName := fmt.Sprintf(failedClusterFmt, clusterID) + if err := f.createFailedClusterFileInZip(zipWriter, fileName, failedCluster); err != nil { + return nil, errors.Wrap(err, "error creating failed cluster report") } + } + for clusterID, res := range results { + fileName := fmt.Sprintf(successfulClusterFmt, clusterID) err := f.createCSVInZip(zipWriter, fileName, res) if err != nil { return nil, errors.Wrap(err, "error creating csv report") diff --git a/central/complianceoperator/v2/report/manager/format/formatter_test.go b/central/complianceoperator/v2/report/manager/format/formatter_test.go index 390d52d543721..f31614bb3f0fd 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter_test.go +++ b/central/complianceoperator/v2/report/manager/format/formatter_test.go @@ -14,6 +14,11 @@ import ( "go.uber.org/mock/gomock" ) +const ( + clusterID1 = "cluster-1" + clusterID2 = "cluster-2" +) + func TestComplianceReportingFormatter(t *testing.T) { suite.Run(t, new(ComplianceReportingFormatterSuite)) } @@ -32,7 +37,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { matcher := &valueMatcher{ data: getFakeReportData(), } - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { matcher.recordNumber++ }) @@ -47,7 +52,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { matcher := &valueMatcher{ data: getFakeReportData(), } - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { matcher.recordNumber++ }) @@ -63,7 +68,10 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedClusterNoError() { matcher := &failedClusterValueMatcher{} matcher.data, matcher.failed = getFakeReportDataWithFailedCluster() - s.zipWriter.EXPECT().Create(gomock.Any()).Times(2).Return(nil, nil) + gomock.InOrder( + s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil), + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil), + ) s.csvWriter.EXPECT().AddValue(matcher).Times(3) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(2).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -75,7 +83,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedCluste func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { s.Run("report data", func() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, errors.New("error")) + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) buf, err := s.formatter.FormatCSVReport(getFakeReportData(), getFakeEmptyFailedClusters()) @@ -83,7 +91,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { s.Require().Nil(buf) }) s.Run("failed cluster data", func() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, errors.New("error")) + s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) buf, err := s.formatter.FormatCSVReport(getFakeReportDataOnlyFailedCluster()) @@ -93,8 +101,8 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { - s.Run("report date", func() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.Run("report data", func() { + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(gomock.Any()).Times(2) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -103,10 +111,10 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { s.Require().Error(err) s.Require().Nil(buf) }) - s.Run("failed cluster date", func() { + s.Run("failed cluster data", func() { matcher := &failedClusterValueMatcher{} matcher.data, matcher.failed = getFakeReportDataOnlyFailedCluster() - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(matcher).Times(1) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -118,7 +126,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCloseError() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(gomock.Any()).Times(2) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(errors.New("error")) @@ -129,7 +137,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCloseError() { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportEmptyReportNoError() { - s.zipWriter.EXPECT().Create(gomock.Any()).Times(1).Return(nil, nil) + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(&emptyValueMatcher{ value: EmptyValue, data: getFakeEmptyReportData(), @@ -173,7 +181,7 @@ func getFakeEmptyReportData() map[string][]*report.ResultRow { func getFakeReportData() map[string][]*report.ResultRow { results := make(map[string][]*report.ResultRow) - results["cluster-1"] = []*report.ResultRow{ + results[clusterID1] = []*report.ResultRow{ { ClusterName: "test_cluster-1", CheckName: "test_check-1", @@ -198,27 +206,25 @@ func getFakeReportData() map[string][]*report.ResultRow { func getFakeReportDataWithFailedCluster() (map[string][]*report.ResultRow, map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) { failedClusters := make(map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) - failedClusters["cluster-2"] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ + failedClusters[clusterID2] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ ClusterName: "test_cluster-2", ClusterId: "test_cluster-2-id", Reason: "timeout", OperatorVersion: "v1.6.0", } results := getFakeReportData() - results["cluster-2"] = []*report.ResultRow{} return results, failedClusters } func getFakeReportDataOnlyFailedCluster() (map[string][]*report.ResultRow, map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) { failedClusters := make(map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) - failedClusters["cluster-2"] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ + failedClusters[clusterID2] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ ClusterName: "test_cluster-2", ClusterId: "test_cluster-2-id", Reason: "timeout", OperatorVersion: "v1.6.0", } results := make(map[string][]*report.ResultRow) - results["cluster-2"] = []*report.ResultRow{} return results, failedClusters } From bf50416d30bf15132ba3991b5abc5189c7986ce4 Mon Sep 17 00:00:00 2001 From: lvalerom Date: Tue, 13 May 2025 12:55:46 +0200 Subject: [PATCH 3/5] use Cond instead of matchers --- .../report/manager/format/formatter_test.go | 119 ++++++------------ 1 file changed, 38 insertions(+), 81 deletions(-) diff --git a/central/complianceoperator/v2/report/manager/format/formatter_test.go b/central/complianceoperator/v2/report/manager/format/formatter_test.go index f31614bb3f0fd..261b5eb12314a 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter_test.go +++ b/central/complianceoperator/v2/report/manager/format/formatter_test.go @@ -34,13 +34,17 @@ type ComplianceReportingFormatterSuite struct { func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { s.Run("with empty failed clusters", func() { - matcher := &valueMatcher{ - data: getFakeReportData(), - } s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) - s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { - matcher.recordNumber++ - }) + gomock.InOrder( + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + data := getFakeReportData() + return compareStringSlice(target, generateRecord(data[clusterID1][0])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + data := getFakeReportData() + return compareStringSlice(target, generateRecord(data[clusterID1][1])) + })).Times(1), + ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -49,13 +53,17 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { s.Require().NotNil(buf) }) s.Run("with nil failed clusters", func() { - matcher := &valueMatcher{ - data: getFakeReportData(), - } s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) - s.csvWriter.EXPECT().AddValue(matcher).Times(2).Do(func(_ any) { - matcher.recordNumber++ - }) + gomock.InOrder( + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + data := getFakeReportData() + return compareStringSlice(target, generateRecord(data[clusterID1][0])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + data := getFakeReportData() + return compareStringSlice(target, generateRecord(data[clusterID1][1])) + })).Times(1), + ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -66,13 +74,24 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedClusterNoError() { - matcher := &failedClusterValueMatcher{} - matcher.data, matcher.failed = getFakeReportDataWithFailedCluster() gomock.InOrder( s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil), s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil), ) - s.csvWriter.EXPECT().AddValue(matcher).Times(3) + gomock.InOrder( + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + _, failed := getFakeReportDataWithFailedCluster() + return compareStringSlice(target, generateFailRecord(failed[clusterID2])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + successful, _ := getFakeReportDataWithFailedCluster() + return compareStringSlice(target, generateRecord(successful[clusterID1][0])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + successful, _ := getFakeReportDataWithFailedCluster() + return compareStringSlice(target, generateRecord(successful[clusterID1][1])) + })).Times(1), + ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(2).Return(nil) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -112,10 +131,11 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { s.Require().Nil(buf) }) s.Run("failed cluster data", func() { - matcher := &failedClusterValueMatcher{} - matcher.data, matcher.failed = getFakeReportDataOnlyFailedCluster() s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil) - s.csvWriter.EXPECT().AddValue(matcher).Times(1) + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + _, failed := getFakeReportDataWithFailedCluster() + return compareStringSlice(target, generateFailRecord(failed[clusterID2])) + })).Times(1) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -255,31 +275,6 @@ func (m *emptyValueMatcher) String() string { return m.error } -type valueMatcher struct { - recordNumber int - data map[string][]*report.ResultRow - error string -} - -func (m *valueMatcher) Matches(target interface{}) bool { - record, ok := target.(csv.Value) - if !ok { - m.error = "target is not of type csv.Value" - return false - } - recordIt := 0 - for _, clusterData := range m.data { - for _, check := range clusterData { - if recordIt == m.recordNumber { - m.error = fmt.Sprintf("expected record: %v", generateRecord(check)) - return compareStringSlice(record, generateRecord(check)) - } - recordIt++ - } - } - return false -} - func compareStringSlice(a []string, b []string) bool { if len(a) != len(b) { return false @@ -291,41 +286,3 @@ func compareStringSlice(a []string, b []string) bool { } return true } - -func (m *valueMatcher) String() string { - return m.error -} - -type failedClusterValueMatcher struct { - data map[string][]*report.ResultRow - failed map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster - error string -} - -func (m *failedClusterValueMatcher) Matches(target interface{}) bool { - record, ok := target.(csv.Value) - if !ok { - m.error = "target is not of type csv.Value" - return false - } - for _, clusterData := range m.data { - for _, check := range clusterData { - if compareStringSlice(record, generateRecord(check)) { - return true - } - } - } - - for _, failedCluster := range m.failed { - if compareStringSlice(record, generateFailRecord(failedCluster)) { - return true - } - } - - m.error = fmt.Sprintf("AddValue called with an unexpected record %v", record) - return false -} - -func (m *failedClusterValueMatcher) String() string { - return m.error -} From e6001ee1f59592523bb487c35ef4cd10704361c2 Mon Sep 17 00:00:00 2001 From: lvalerom Date: Wed, 14 May 2025 17:44:58 +0200 Subject: [PATCH 4/5] address review --- .../v2/report/manager/format/formatter.go | 14 +++- .../report/manager/format/formatter_test.go | 73 +++++++++++++------ 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/central/complianceoperator/v2/report/manager/format/formatter.go b/central/complianceoperator/v2/report/manager/format/formatter.go index 5a3ca4b8679c1..ebfd37edb3f26 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter.go +++ b/central/complianceoperator/v2/report/manager/format/formatter.go @@ -13,7 +13,7 @@ import ( ) const ( - EmptyValue = "Data not found for the cluster" + emptyValue = "Data not found for the cluster" successfulClusterFmt = "cluster_%s.csv" failedClusterFmt = "failed_cluster_%s.csv" ) @@ -62,6 +62,10 @@ func NewFormatter() *FormatterImpl { } } +// FormatCSVReport generates zip data containing CSV files (one per cluster). +// If a cluster fails, the generated CSV file will contain the reason for the reason but (no check results). +// If a cluster success, the generated CSV file will contain all the check results with enhanced information (e.g. remediation, associated profile, etc) +// The results parameter is expected to contain the clusters that succeed (no failed clusters should be passed in results). func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow, failedClusters map[string]*storage.ComplianceOperatorReportSnapshotV2_FailedCluster) (buffRet *bytes.Buffer, errRet error) { var buf bytes.Buffer zipWriter := f.newZipWriter(&buf) @@ -78,6 +82,10 @@ func (f *FormatterImpl) FormatCSVReport(results map[string][]*report.ResultRow, } } for clusterID, res := range results { + // We should not receive results from a failed cluster + if _, ok := failedClusters[clusterID]; ok { + continue + } fileName := fmt.Sprintf(successfulClusterFmt, clusterID) err := f.createCSVInZip(zipWriter, fileName, res) if err != nil { @@ -98,12 +106,13 @@ func (f *FormatterImpl) createCSVInZip(zipWriter ZipWriter, filename string, clu csvWriter.AddValue(generateRecord(checkRes)) } } else { - csvWriter.AddValue([]string{EmptyValue}) + csvWriter.AddValue([]string{emptyValue}) } return csvWriter.WriteCSV(w) } func generateRecord(row *report.ResultRow) []string { + // The order in the slice needs to match the order defined in `csvHeader` return []string{ row.ControlRef, row.CheckName, @@ -128,6 +137,7 @@ func (f *FormatterImpl) createFailedClusterFileInZip(zipWriter ZipWriter, filena } func generateFailRecord(failedCluster *storage.ComplianceOperatorReportSnapshotV2_FailedCluster) []string { + // The order in the slice needs to match the order defined in `failedClusterCSVHeader` return []string{ failedCluster.GetClusterId(), failedCluster.GetClusterName(), diff --git a/central/complianceoperator/v2/report/manager/format/formatter_test.go b/central/complianceoperator/v2/report/manager/format/formatter_test.go index 261b5eb12314a..50e8a5c784ed6 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter_test.go +++ b/central/complianceoperator/v2/report/manager/format/formatter_test.go @@ -10,6 +10,7 @@ import ( "github.com/stackrox/rox/central/complianceoperator/v2/report/manager/format/mocks" "github.com/stackrox/rox/generated/storage" "github.com/stackrox/rox/pkg/csv" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "go.uber.org/mock/gomock" ) @@ -38,11 +39,11 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { gomock.InOrder( s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { data := getFakeReportData() - return compareStringSlice(target, generateRecord(data[clusterID1][0])) + return compareStringSlice(s.T(), target, generateRecord(data[clusterID1][0])) })).Times(1), s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { data := getFakeReportData() - return compareStringSlice(target, generateRecord(data[clusterID1][1])) + return compareStringSlice(s.T(), target, generateRecord(data[clusterID1][1])) })).Times(1), ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) @@ -57,11 +58,11 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportNoError() { gomock.InOrder( s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { data := getFakeReportData() - return compareStringSlice(target, generateRecord(data[clusterID1][0])) + return compareStringSlice(s.T(), target, generateRecord(data[clusterID1][0])) })).Times(1), s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { data := getFakeReportData() - return compareStringSlice(target, generateRecord(data[clusterID1][1])) + return compareStringSlice(s.T(), target, generateRecord(data[clusterID1][1])) })).Times(1), ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) @@ -81,15 +82,15 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedCluste gomock.InOrder( s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { _, failed := getFakeReportDataWithFailedCluster() - return compareStringSlice(target, generateFailRecord(failed[clusterID2])) + return compareStringSlice(s.T(), target, generateFailRecord(failed[clusterID2])) })).Times(1), s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { successful, _ := getFakeReportDataWithFailedCluster() - return compareStringSlice(target, generateRecord(successful[clusterID1][0])) + return compareStringSlice(s.T(), target, generateRecord(successful[clusterID1][0])) })).Times(1), s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { successful, _ := getFakeReportDataWithFailedCluster() - return compareStringSlice(target, generateRecord(successful[clusterID1][1])) + return compareStringSlice(s.T(), target, generateRecord(successful[clusterID1][1])) })).Times(1), ) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(2).Return(nil) @@ -100,8 +101,38 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedCluste s.Require().NotNil(buf) } +func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWithFailedClusterInResultsParameterNoError() { + gomock.InOrder( + s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil), + s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil), + ) + gomock.InOrder( + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + _, failed := getFakeReportDataWithFailedCluster() + return compareStringSlice(s.T(), target, generateFailRecord(failed[clusterID2])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + successful, _ := getFakeReportDataWithFailedCluster() + return compareStringSlice(s.T(), target, generateRecord(successful[clusterID1][0])) + })).Times(1), + s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { + successful, _ := getFakeReportDataWithFailedCluster() + return compareStringSlice(s.T(), target, generateRecord(successful[clusterID1][1])) + })).Times(1), + ) + s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(2).Return(nil) + s.zipWriter.EXPECT().Close().Times(1).Return(nil) + + results, failedCluster := getFakeReportDataWithFailedCluster() + // Add empty results to the failed cluster + results[clusterID2] = []*report.ResultRow{} + buf, err := s.formatter.FormatCSVReport(results, failedCluster) + s.Require().NoError(err) + s.Require().NotNil(buf) +} + func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { - s.Run("report data", func() { + s.Run("zip writer failing to create a file (with no failed clusters) should yield an error", func() { s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -109,7 +140,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { s.Require().Error(err) s.Require().Nil(buf) }) - s.Run("failed cluster data", func() { + s.Run("zip writer failing to create a file (containing failed clusters) should yield an error", func() { s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -120,7 +151,7 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCreateError() { } func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { - s.Run("report data", func() { + s.Run("csv writer failing to create a file (with no failed clusters) should yield an error", func() { s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(gomock.Any()).Times(2) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) @@ -130,11 +161,11 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportWriteError() { s.Require().Error(err) s.Require().Nil(buf) }) - s.Run("failed cluster data", func() { + s.Run("csv writer failing to create a file (containing failed clusters) should yield an error", func() { s.zipWriter.EXPECT().Create(fmt.Sprintf(failedClusterFmt, clusterID2)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(gomock.Cond[csv.Value](func(target csv.Value) bool { _, failed := getFakeReportDataWithFailedCluster() - return compareStringSlice(target, generateFailRecord(failed[clusterID2])) + return compareStringSlice(s.T(), target, generateFailRecord(failed[clusterID2])) })).Times(1) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(errors.New("error")) s.zipWriter.EXPECT().Close().Times(1).Return(nil) @@ -159,7 +190,8 @@ func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportCloseError() { func (s *ComplianceReportingFormatterSuite) Test_FormatCSVReportEmptyReportNoError() { s.zipWriter.EXPECT().Create(fmt.Sprintf(successfulClusterFmt, clusterID1)).Times(1).Return(nil, nil) s.csvWriter.EXPECT().AddValue(&emptyValueMatcher{ - value: EmptyValue, + t: s.T(), + value: emptyValue, data: getFakeEmptyReportData(), }).Times(1) s.csvWriter.EXPECT().WriteCSV(gomock.Any()).Times(1).Return(nil) @@ -253,6 +285,7 @@ func getFakeEmptyFailedClusters() map[string]*storage.ComplianceOperatorReportSn } type emptyValueMatcher struct { + t *testing.T value string data map[string][]*report.ResultRow error string @@ -266,7 +299,7 @@ func (m *emptyValueMatcher) Matches(target interface{}) bool { } for range m.data { m.error = fmt.Sprintf("expected record: %s", m.value) - return compareStringSlice(record, []string{m.value}) + return compareStringSlice(m.t, record, []string{m.value}) } return false } @@ -275,14 +308,6 @@ func (m *emptyValueMatcher) String() string { return m.error } -func compareStringSlice(a []string, b []string) bool { - if len(a) != len(b) { - return false - } - for i := range a { - if a[i] != b[i] { - return false - } - } - return true +func compareStringSlice(t *testing.T, actual []string, expected []string) bool { + return assert.Equal(t, expected, actual) } From 11950ddb05a0d8146ff73c9229e51c8e48c9c719 Mon Sep 17 00:00:00 2001 From: lvalerom Date: Fri, 16 May 2025 18:55:00 +0200 Subject: [PATCH 5/5] join reasons into one string --- .../complianceoperator/v2/report/manager/format/formatter.go | 3 ++- .../v2/report/manager/format/formatter_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/central/complianceoperator/v2/report/manager/format/formatter.go b/central/complianceoperator/v2/report/manager/format/formatter.go index ebfd37edb3f26..59a65e583a5be 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter.go +++ b/central/complianceoperator/v2/report/manager/format/formatter.go @@ -5,6 +5,7 @@ import ( "bytes" "fmt" "io" + "strings" "github.com/pkg/errors" "github.com/stackrox/rox/central/complianceoperator/v2/report" @@ -141,7 +142,7 @@ func generateFailRecord(failedCluster *storage.ComplianceOperatorReportSnapshotV return []string{ failedCluster.GetClusterId(), failedCluster.GetClusterName(), - failedCluster.GetReason(), + strings.Join(failedCluster.GetReasons(), ", "), failedCluster.GetOperatorVersion(), } } diff --git a/central/complianceoperator/v2/report/manager/format/formatter_test.go b/central/complianceoperator/v2/report/manager/format/formatter_test.go index 50e8a5c784ed6..b88ce7cd7b3da 100644 --- a/central/complianceoperator/v2/report/manager/format/formatter_test.go +++ b/central/complianceoperator/v2/report/manager/format/formatter_test.go @@ -261,7 +261,7 @@ func getFakeReportDataWithFailedCluster() (map[string][]*report.ResultRow, map[s failedClusters[clusterID2] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ ClusterName: "test_cluster-2", ClusterId: "test_cluster-2-id", - Reason: "timeout", + Reasons: []string{"timeout"}, OperatorVersion: "v1.6.0", } results := getFakeReportData() @@ -273,7 +273,7 @@ func getFakeReportDataOnlyFailedCluster() (map[string][]*report.ResultRow, map[s failedClusters[clusterID2] = &storage.ComplianceOperatorReportSnapshotV2_FailedCluster{ ClusterName: "test_cluster-2", ClusterId: "test_cluster-2-id", - Reason: "timeout", + Reasons: []string{"timeout"}, OperatorVersion: "v1.6.0", } results := make(map[string][]*report.ResultRow)