From f6a54b87716825f72601991161cad36b49f8e6ab Mon Sep 17 00:00:00 2001 From: Dan Manor Date: Wed, 18 Sep 2024 17:08:17 +0300 Subject: [PATCH] MGMT-18903: Add 'source' column to violations by severity count SQL table to support multiple central instances in one hub (#1105) Signed-off-by: danmanor --- .../security/stackrox_alert_count_summary.go | 14 +- .../controller/security/stackrox_syncer.go | 3 +- .../security/stackrox_syncer_test.go | 6 +- .../syncers/security_alert_counts_handler.go | 2 + .../acm-global-security-alert-counts.yaml | 23 +- .../hubofhubs/storage/database/2.tables.sql | 8 +- .../hubofhubs/storage/upgrade/1.upgrade.sql | 2 +- pkg/database/models/security.go | 4 + pkg/wire/models/security.go | 6 +- .../security_alert_counts_handler_test.go | 207 +++++++++++++++++- 10 files changed, 259 insertions(+), 16 deletions(-) diff --git a/agent/pkg/status/controller/security/stackrox_alert_count_summary.go b/agent/pkg/status/controller/security/stackrox_alert_count_summary.go index cd96ef3cf..c15fa7b29 100644 --- a/agent/pkg/status/controller/security/stackrox_alert_count_summary.go +++ b/agent/pkg/status/controller/security/stackrox_alert_count_summary.go @@ -36,7 +36,7 @@ var AlertsSummeryCountsRequest = stackRoxRequest{ Body: "", CacheStruct: &AlertsSummeryCountsResponse{}, GenerateFromCache: func(values ...any) (any, error) { - if len(values) != 2 { + if len(values) != 4 { return nil, fmt.Errorf("alert summery count cache struct or ACS base URL were not provided") } @@ -53,8 +53,20 @@ var AlertsSummeryCountsRequest = stackRoxRequest{ if !ok { return nil, fmt.Errorf("ACS external URL is not valid") } + + acsCentralNamespace, ok := values[2].(string) + if !ok { + return nil, fmt.Errorf("ACS Central namespace was not provided") + } + + acsCentralName, ok := values[3].(string) + if !ok { + return nil, fmt.Errorf("ACS Central name was not provided") + } + alertCount := wiremodels.SecurityAlertCounts{ DetailURL: fmt.Sprintf("%s%s", acsCentralExternalHostPort, stackRoxAlertsDetailsPath), + Source: fmt.Sprintf("%s/%s", acsCentralNamespace, acsCentralName), } for _, count := range alertCountSummeryResponse.Groups[0].Counts { diff --git a/agent/pkg/status/controller/security/stackrox_syncer.go b/agent/pkg/status/controller/security/stackrox_syncer.go index 2143bdff7..75fd6afe9 100644 --- a/agent/pkg/status/controller/security/stackrox_syncer.go +++ b/agent/pkg/status/controller/security/stackrox_syncer.go @@ -416,7 +416,8 @@ func (s *StackRoxSyncer) sync(ctx context.Context, data *stackRoxData) error { return fmt.Errorf("failed to make a request to central: %v", err) } - messageStruct, err := request.GenerateFromCache(request.CacheStruct, data.consoleURL) + messageStruct, err := request.GenerateFromCache( + request.CacheStruct, data.consoleURL, data.key.Namespace, data.key.Name) if err != nil { return fmt.Errorf("failed to generate struct for kafka message: %v", err) } diff --git a/agent/pkg/status/controller/security/stackrox_syncer_test.go b/agent/pkg/status/controller/security/stackrox_syncer_test.go index 75d657c7d..a122c66ed 100644 --- a/agent/pkg/status/controller/security/stackrox_syncer_test.go +++ b/agent/pkg/status/controller/security/stackrox_syncer_test.go @@ -303,7 +303,8 @@ var _ = Describe("StackRox syncer", func() { "medium": 2, "high": 3, "critical": 4, - "detail_url": "https://my-console.com/main/violations" + "detail_url": "https://my-console.com/main/violations", + "source": "rhacs-operator/stackrox-central-services" }`)) return nil @@ -622,7 +623,8 @@ var _ = Describe("StackRox syncer", func() { "medium": 2, "high": 3, "critical": 4, - "detail_url": "https://my-console.com/main/violations" + "detail_url": "https://my-console.com/main/violations", + "source": "rhacs-operator/stackrox-central-services" }`)) return nil diff --git a/manager/pkg/statussyncer/syncers/security_alert_counts_handler.go b/manager/pkg/statussyncer/syncers/security_alert_counts_handler.go index 7fd4a801a..ec075615e 100644 --- a/manager/pkg/statussyncer/syncers/security_alert_counts_handler.go +++ b/manager/pkg/statussyncer/syncers/security_alert_counts_handler.go @@ -64,11 +64,13 @@ func (h *securityAlertCountsHandler) handleEvent(ctx context.Context, evt *cloud High: wireModel.High, Critical: wireModel.Critical, DetailURL: wireModel.DetailURL, + Source: wireModel.Source, } // Insert or update the data in the database: db := database.GetGorm() err := db.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "hub_name"}, {Name: "source"}}, UpdateAll: true, }).Create(dbModel).Error if err != nil { diff --git a/operator/pkg/controllers/hubofhubs/grafana/manifests/acm-global-security-alert-counts.yaml b/operator/pkg/controllers/hubofhubs/grafana/manifests/acm-global-security-alert-counts.yaml index bcef46f42..fb3feb193 100644 --- a/operator/pkg/controllers/hubofhubs/grafana/manifests/acm-global-security-alert-counts.yaml +++ b/operator/pkg/controllers/hubofhubs/grafana/manifests/acm-global-security-alert-counts.yaml @@ -557,6 +557,27 @@ data: } ] }, + { + "matcher": { + "id": "byName", + "options": "Source" + }, + "properties": [ + { + "id": "custom.cellOptions", + "value": { + "type": "color-text" + } + }, + { + "id": "color", + "value": { + "fixedColor": "text", + "mode": "fixed" + } + } + ] + }, { "matcher": { "id": "byName", @@ -698,7 +719,7 @@ data: "group": [], "metricColumn": "none", "rawQuery": true, - "rawSql": "select \n hub_name as \"Hub\",\n low as \"Low\",\n medium as \"Medium\",\n high as \"High\",\n critical as \"Critical\",\n detail_url as \"Detail\"\nfrom\n security.alert_counts", + "rawSql": "select \n hub_name as \"Hub\",\n low as \"Low\",\n medium as \"Medium\",\n high as \"High\",\n critical as \"Critical\",\n detail_url as \"Detail\",\n source as \"Source\"\nfrom\n security.alert_counts", "refId": "A", "select": [ [ diff --git a/operator/pkg/controllers/hubofhubs/storage/database/2.tables.sql b/operator/pkg/controllers/hubofhubs/storage/database/2.tables.sql index d6c62884a..954ba6415 100644 --- a/operator/pkg/controllers/hubofhubs/storage/database/2.tables.sql +++ b/operator/pkg/controllers/hubofhubs/storage/database/2.tables.sql @@ -144,12 +144,14 @@ CREATE TABLE IF NOT EXISTS status.transport ( ); CREATE TABLE IF NOT EXISTS security.alert_counts ( - hub_name text PRIMARY KEY, + hub_name text NOT NULL, low integer NOT NULL, medium integer NOT NULL, high integer NOT NULL, critical integer NOT NULL, detail_url text NOT NULL, + source text NOT NULL, created_at timestamp without time zone DEFAULT now() NOT NULL, - updated_at timestamp without time zone DEFAULT now() NOT NULL -); \ No newline at end of file + updated_at timestamp without time zone DEFAULT now() NOT NULL, + PRIMARY KEY (hub_name, source) +); diff --git a/operator/pkg/controllers/hubofhubs/storage/upgrade/1.upgrade.sql b/operator/pkg/controllers/hubofhubs/storage/upgrade/1.upgrade.sql index 258ef5e2c..362ff256f 100644 --- a/operator/pkg/controllers/hubofhubs/storage/upgrade/1.upgrade.sql +++ b/operator/pkg/controllers/hubofhubs/storage/upgrade/1.upgrade.sql @@ -16,4 +16,4 @@ ALTER TYPE local_status.compliance_type ADD VALUE IF NOT EXISTS 'pending'; ALTER TABLE event.local_policies ADD COLUMN IF NOT EXISTS event_namespace text; ALTER TABLE event.local_policies ADD COLUMN IF NOT EXISTS cluster_name text; -ALTER TABLE event.local_root_policies ADD COLUMN IF NOT EXISTS event_namespace text; \ No newline at end of file +ALTER TABLE event.local_root_policies ADD COLUMN IF NOT EXISTS event_namespace text; diff --git a/pkg/database/models/security.go b/pkg/database/models/security.go index dbbe403fe..32d8ed6b2 100644 --- a/pkg/database/models/security.go +++ b/pkg/database/models/security.go @@ -25,6 +25,10 @@ type SecurityAlertCounts struct { // https://central-rhacs-operator.apps.../main/violations DetailURL string `gorm:"column:detail_url;not null"` + // Source is the Central CR instance from which the data was retrieved. + // This should follow the format: "/" + Source string `gorm:"column:source;not null"` + // CreatedAt is the date and time when the row was created. CreatedAt time.Time `gorm:"column:created_at;autoCreateTime:true"` diff --git a/pkg/wire/models/security.go b/pkg/wire/models/security.go index 9a662d8fb..11b844cb2 100644 --- a/pkg/wire/models/security.go +++ b/pkg/wire/models/security.go @@ -14,9 +14,13 @@ type SecurityAlertCounts struct { // Critical is the total number of critical severity alerts. Critical int `json:"critical,omitempty"` - // DetailURL is the URL where the user can see the details of the alerts of the hub. This + // DetailURL is the URL where the user can see the details of the alerts of the Central CR instance in the hub. This // will typically be the URL of the violations tab of the Stackrox Central UI: // // https://central-rhacs-operator.apps.../main/violations DetailURL string `json:"detail_url,omitempty"` + + // Source is the Central CR instance from which the data was retrieved. + // This should follow the format: "/" + Source string `json:"source,omitempty"` } diff --git a/test/integration/manager/status/security_alert_counts_handler_test.go b/test/integration/manager/status/security_alert_counts_handler_test.go index 84b4e05b8..6c89e2372 100644 --- a/test/integration/manager/status/security_alert_counts_handler_test.go +++ b/test/integration/manager/status/security_alert_counts_handler_test.go @@ -14,9 +14,23 @@ import ( ) var _ = Describe("SecurityAlertCountsHandler", Ordered, func() { - It("Should be able to sync security alert counts event", func() { + const ( + leafHubName = "hub1" + source1 = "rhacs-operator/stackrox-central-services" + source2 = "other-namespace/other-name" + DetailURL = "https://hub1/violations" + ) + + BeforeEach(func() { + // truncate table + db := database.GetSqlDb() + sql := fmt.Sprintf(`TRUNCATE TABLE %s.%s`, database.SecuritySchema, database.SecurityAlertCountsTable) + _, err := db.Query(sql) + Expect(err).To(Succeed()) + }) + + It("Should be able to sync security alert counts event from one central instance in hub", func() { By("Create event") - const leafHubName = "hub1" version := eventversion.NewVersion() version.Incr() data := &wiremodels.SecurityAlertCounts{ @@ -24,7 +38,8 @@ var _ = Describe("SecurityAlertCountsHandler", Ordered, func() { Medium: 2, High: 3, Critical: 4, - DetailURL: "https://hub1/violations", + DetailURL: DetailURL, + Source: source1, } event := ToCloudEvent(leafHubName, string(enum.SecurityAlertCountsType), version, data) @@ -42,7 +57,8 @@ var _ = Describe("SecurityAlertCountsHandler", Ordered, func() { medium, high, critical, - detail_url + detail_url, + source FROM %s.%s WHERE @@ -57,15 +73,194 @@ var _ = Describe("SecurityAlertCountsHandler", Ordered, func() { high int critical int detailURL string + source string ) row := db.QueryRow(sql, leafHubName) - err := row.Scan(&low, &medium, &high, &critical, &detailURL) + err := row.Scan(&low, &medium, &high, &critical, &detailURL, &source) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(low).To(Equal(1)) + g.Expect(medium).To(Equal(2)) + g.Expect(high).To(Equal(3)) + g.Expect(critical).To(Equal(4)) + g.Expect(detailURL).To(Equal(detailURL)) + g.Expect(source).To(Equal(source1)) + } + Eventually(check, 30*time.Second, 100*time.Millisecond).Should(Succeed()) + }) + + It("Should be able to sync security alert counts event from multiple central instances in hub", func() { + By("Create event") + version1 := eventversion.NewVersion() + version1.Incr() + dataEvent1 := &wiremodels.SecurityAlertCounts{ + Low: 1, + Medium: 2, + High: 3, + Critical: 4, + DetailURL: DetailURL, + Source: source1, + } + event1 := ToCloudEvent(leafHubName, string(enum.SecurityAlertCountsType), version1, dataEvent1) + + version2 := eventversion.NewVersion() + version2.Incr() + dataEvent2 := &wiremodels.SecurityAlertCounts{ + Low: 1, + Medium: 2, + High: 3, + Critical: 4, + DetailURL: DetailURL, + Source: source2, + } + event2 := ToCloudEvent(leafHubName, string(enum.SecurityAlertCountsType), version2, dataEvent2) + + By("Sync events with transport") + err := producer.SendEvent(ctx, *event1) + Expect(err).To(Succeed()) + + time.Sleep(100 * time.Millisecond) + + err = producer.SendEvent(ctx, *event2) + Expect(err).To(Succeed()) + + By("Check the table") + db := database.GetSqlDb() + Expect(db).ToNot(BeNil()) + sql := fmt.Sprintf( + ` + SELECT + low, + medium, + high, + critical, + detail_url, + source + FROM + %s.%s + WHERE + hub_name = $1 AND source = $2 + `, + database.SecuritySchema, database.SecurityAlertCountsTable, + ) + check := func(g Gomega) { + var ( + low int + medium int + high int + critical int + detailURL string + source string + ) + + // verify first event added + row := db.QueryRow(sql, leafHubName, source1) + err = row.Scan(&low, &medium, &high, &critical, &detailURL, &source) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(low).To(Equal(1)) + g.Expect(medium).To(Equal(2)) + g.Expect(high).To(Equal(3)) + g.Expect(critical).To(Equal(4)) + g.Expect(detailURL).To(Equal(detailURL)) + g.Expect(source).To(Equal(source1)) + + // verify second event added + row = db.QueryRow(sql, leafHubName, source2) + err = row.Scan(&low, &medium, &high, &critical, &detailURL, &source) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(low).To(Equal(1)) + g.Expect(medium).To(Equal(2)) + g.Expect(high).To(Equal(3)) + g.Expect(critical).To(Equal(4)) + g.Expect(detailURL).To(Equal(detailURL)) + g.Expect(source).To(Equal(source2)) + } + Eventually(check, 30*time.Second, 100*time.Millisecond).Should(Succeed()) + }) + + It("Should be able to sync security alert counts event from multiple central instances in hub by updating only the necessary record", func() { + By("Add records to the table") + db := database.GetSqlDb() + Expect(db).ToNot(BeNil()) + + sql := fmt.Sprintf( + ` + INSERT INTO %s.%s (hub_name, low, medium, high, critical, detail_url, source) + VALUES ($1, 1, 2, 3, 4, $2, $3) + `, + database.SecuritySchema, database.SecurityAlertCountsTable, + ) + + _, err := db.Query(sql, leafHubName, DetailURL, source1) + Expect(err).To(Succeed()) + + _, err = db.Query(sql, leafHubName, DetailURL, source2) + Expect(err).To(Succeed()) + + By("Create event") + version1 := eventversion.NewVersion() + version1.Incr() + dataEvent1 := &wiremodels.SecurityAlertCounts{ + Low: 4, + Medium: 4, + High: 4, + Critical: 4, + DetailURL: DetailURL, + Source: source1, + } + event1 := ToCloudEvent(leafHubName, string(enum.SecurityAlertCountsType), version1, dataEvent1) + + By("Sync events with transport") + err = producer.SendEvent(ctx, *event1) + Expect(err).To(Succeed()) + + By("Check the table") + sql = fmt.Sprintf( + ` + SELECT + low, + medium, + high, + critical, + detail_url, + source + FROM + %s.%s + WHERE + hub_name = $1 AND source = $2 + `, + database.SecuritySchema, database.SecurityAlertCountsTable, + ) + check := func(g Gomega) { + var ( + low int + medium int + high int + critical int + detailURL string + source string + ) + + // verify first event changed + row := db.QueryRow(sql, leafHubName, source1) + err := row.Scan(&low, &medium, &high, &critical, &detailURL, &source) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(low).To(Equal(4)) + g.Expect(medium).To(Equal(4)) + g.Expect(high).To(Equal(4)) + g.Expect(critical).To(Equal(4)) + g.Expect(detailURL).To(Equal(detailURL)) + g.Expect(source).To(Equal(source1)) + + // verify second event not changed + row = db.QueryRow(sql, leafHubName, source2) + err = row.Scan(&low, &medium, &high, &critical, &detailURL, &source) g.Expect(err).ToNot(HaveOccurred()) g.Expect(low).To(Equal(1)) g.Expect(medium).To(Equal(2)) g.Expect(high).To(Equal(3)) g.Expect(critical).To(Equal(4)) - g.Expect(detailURL).To(Equal("https://hub1/violations")) + g.Expect(detailURL).To(Equal(detailURL)) + g.Expect(source).To(Equal(source2)) } Eventually(check, 30*time.Second, 100*time.Millisecond).Should(Succeed()) })