From 1eea9b08bc469ca6e3a06770fef3aa3a67a44a7a Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 18:47:00 +0100 Subject: [PATCH 1/8] fix DVO details field having a knonw structure (details contain rule content template data) --- consumer/dvo_rules_consumer_test.go | 11 +++++++++-- storage/dvo_recommendations_storage_test.go | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/consumer/dvo_rules_consumer_test.go b/consumer/dvo_rules_consumer_test.go index eea16cb4..fcc19bff 100644 --- a/consumer/dvo_rules_consumer_test.go +++ b/consumer/dvo_rules_consumer_test.go @@ -348,8 +348,15 @@ func TestParseDVOMessageWithProperMetrics(t *testing.T) { Jira: []string{"https://issues.redhat.com/browse/AN_ISSUE"}, ProductDocumentation: []string{}, }, - Details: types.DVODetails{CheckName: "", CheckURL: ""}, - Tags: []string{}, + Details: map[string]interface{}{ + "check_name": "", + "check_url": "", + "samples": []map[string]interface{}{ + {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, + {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + }, + }, + Tags: []string{}, Workloads: []types.DVOWorkload{ { Namespace: "namespace-name-A", diff --git a/storage/dvo_recommendations_storage_test.go b/storage/dvo_recommendations_storage_test.go index cad9e44b..1aaa69fd 100644 --- a/storage/dvo_recommendations_storage_test.go +++ b/storage/dvo_recommendations_storage_test.go @@ -44,8 +44,15 @@ var ( Jira: []string{"https://issues.redhat.com/browse/AN_ISSUE"}, ProductDocumentation: []string{}, }, - Details: types.DVODetails{CheckName: "", CheckURL: ""}, - Tags: []string{}, + Details: map[string]interface{}{ + "check_name": "", + "check_url": "", + "samples": []map[string]interface{}{ + {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, + {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + }, + }, + Tags: []string{}, Workloads: []types.DVOWorkload{ { Namespace: "namespace-name-A", From 3f4d7f6c012d01c463f51a5512349105ffb57c06 Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 18:47:16 +0100 Subject: [PATCH 2/8] add DVO-related types --- types/types.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/types/types.go b/types/types.go index 97af1978..b4032779 100644 --- a/types/types.go +++ b/types/types.go @@ -165,20 +165,15 @@ type DVOMetrics struct { } // WorkloadRecommendation contains all the information about the recommendation +// Details is generic interface{} because it contains template data used to fill rule content, i.e. we don't/should't know the structure type WorkloadRecommendation struct { - ResponseID string `json:"response_id"` - Component string `json:"component"` - Key string `json:"key"` - Details DVODetails `json:"details"` - Tags []string `json:"tags"` - Links DVOLinks `json:"links"` - Workloads []DVOWorkload `json:"workloads"` -} - -// DVODetails contains some information about the workload -type DVODetails struct { - CheckName string `json:"check_name"` - CheckURL string `json:"check_url"` + ResponseID string `json:"response_id"` + Component string `json:"component"` + Key string `json:"key"` + Details map[string]interface{} `json:"details"` + Tags []string `json:"tags"` + Links DVOLinks `json:"links"` + Workloads []DVOWorkload `json:"workloads"` } // DVOWorkload contains the main information of the workload recommendation @@ -196,6 +191,19 @@ type DVOLinks struct { ProductDocumentation []string `json:"product_documentation"` } +// DVOReport represents a single row of the dvo.dvo_report table. +type DVOReport struct { + OrgID string `json:"org_id"` + NamespaceID string `json:"namespace_id"` + NamespaceName string `json:"namespace_name"` + ClusterID string `json:"cluster_id"` + Recommendations uint `json:"recommendations"` + Report string `json:"report"` + Objects uint `json:"objects"` + ReportedAt types.Timestamp `json:"reported_at"` + LastCheckedAt types.Timestamp `json:"last_checked_at"` +} + // ClusterReports is a data structure containing list of clusters, list of // errors and dictionary with results per cluster. type ClusterReports = types.ClusterReports From c1400e027a9dc39f0d5fe36a83eb99eaf5e2a676 Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 18:48:01 +0100 Subject: [PATCH 3/8] add DVO-related endpoints --- server/endpoints.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/endpoints.go b/server/endpoints.go index e8f4970a..3a34bc19 100644 --- a/server/endpoints.go +++ b/server/endpoints.go @@ -90,6 +90,11 @@ const ( // ClustersRecommendationsListEndpoint receives a list of clusters in POST body and returns a list of clusters with lists of hitting recommendations ClustersRecommendationsListEndpoint = "clusters/organizations/{org_id}/users/{user_id}/recommendations" + // DVOWorkloadRecommendations returns a list of cluster + namespace workloads for given organization ID. + DVOWorkloadRecommendations = "organization/{organization}/workloads" + // DVOWorkloadRecommendationsSingleNamespace returns workloads for a single cluster + namespace ID. + DVOWorkloadRecommendationsSingleNamespace = "organization/{organization}/namespace/{namespace}/cluster/{cluster}/workloads" + // Rating accepts a list of ratings in the request body and store them in the database for the given user Rating = "rules/organizations/{org_id}/rating" // GetRating retrieves the rating for a specific rule and user @@ -181,4 +186,7 @@ func (server *HTTPServer) addRuleEnableDisableEndpointsToRouter(router *mux.Rout func (server *HTTPServer) addInsightsAdvisorEndpointsToRouter(router *mux.Router, apiPrefix string) { router.HandleFunc(apiPrefix+RecommendationsListEndpoint, server.getRecommendations).Methods(http.MethodPost, http.MethodOptions) router.HandleFunc(apiPrefix+ClustersRecommendationsListEndpoint, server.getClustersRecommendationsList).Methods(http.MethodPost, http.MethodOptions) + + router.HandleFunc(apiPrefix+DVOWorkloadRecommendations, server.getWorkloads).Methods(http.MethodGet) + router.HandleFunc(apiPrefix+DVOWorkloadRecommendationsSingleNamespace, server.getWorkloadsForNamespace).Methods(http.MethodGet) } From 5ca855851ca24b07f63d6d69d79dd4d26357298e Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 18:48:25 +0100 Subject: [PATCH 4/8] add handlers for new DVO endpoints --- server/dvo_handlers.go | 294 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 294 insertions(+) create mode 100644 server/dvo_handlers.go diff --git a/server/dvo_handlers.go b/server/dvo_handlers.go new file mode 100644 index 00000000..c3693d4f --- /dev/null +++ b/server/dvo_handlers.go @@ -0,0 +1,294 @@ +/* +Copyright © 2024 Red Hat, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package server + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" + "time" + + "github.com/RedHatInsights/insights-operator-utils/generators" + httputils "github.com/RedHatInsights/insights-operator-utils/http" + "github.com/RedHatInsights/insights-operator-utils/responses" + "github.com/RedHatInsights/insights-results-aggregator/types" + "github.com/google/uuid" + "github.com/rs/zerolog/log" +) + +const ( + namespaceIDParam = "namespace" + RecommendationSuffix = ".recommendation" +) + +// Cluster structure contains cluster UUID and cluster name +type Cluster struct { + UUID string `json:"uuid"` + DisplayName string `json:"display_name"` +} + +// Namespace structure contains basic information about namespace +type Namespace struct { + UUID string `json:"uuid"` + Name string `json:"name"` +} + +// Metadata structure contains basic information about workload metadata +type Metadata struct { + Recommendations int `json:"recommendations"` + Objects int `json:"objects"` + ReportedAt string `json:"reported_at"` + LastCheckedAt string `json:"last_checked_at"` + HighestSeverity int `json:"highest_severity"` + HitsBySeverity map[int]int `json:"hits_by_severity"` +} + +// WorkloadsForNamespace structure represents a single entry of the namespace list with some aggregations +type WorkloadsForNamespace struct { + Cluster Cluster `json:"cluster"` + Namespace Namespace `json:"namespace"` + Metadata Metadata `json:"metadata"` + RecommendationsHitCount map[string]int `json:"recommendations_hit_count"` +} + +// WorkloadsForCluster structure represents workload for one selected cluster +type WorkloadsForCluster struct { + Status string `json:"status"` + Cluster Cluster `json:"cluster"` + Namespace Namespace `json:"namespace"` + Metadata Metadata `json:"metadata"` + Recommendations []DVORecommendation `json:"recommendations"` +} + +// DVORecommendation structure represents one DVO-related recommendation +type DVORecommendation struct { + Check string `json:"check"` + Details string `json:"details"` + Resolution string `json:"resolution"` + Modified string `json:"modified"` + MoreInfo string `json:"more_info"` + TemplateData map[string]interface{} `json:"extra_data"` + Objects []DVOObject `json:"objects"` +} + +// DVOObject structure +type DVOObject struct { + Kind string `json:"kind"` + UID string `json:"uid"` +} + +// readNamespace retrieves namespace UUID from request +// if it's not possible, it writes http error to the writer and returns error +func readNamespace(writer http.ResponseWriter, request *http.Request) ( + namespace string, err error, +) { + namespaceID, err := httputils.GetRouterParam(request, namespaceIDParam) + if err != nil { + handleServerError(writer, err) + return + } + + validatedNamespaceID, err := validateNamespaceID(namespaceID) + if err != nil { + err = &RouterParsingError{ + ParamName: namespaceIDParam, + ParamValue: namespaceID, + ErrString: err.Error(), + } + handleServerError(writer, err) + return + } + + return validatedNamespaceID, nil +} + +func validateNamespaceID(namespace string) (string, error) { + if _, err := uuid.Parse(namespace); err != nil { + message := fmt.Sprintf("invalid namespace ID: '%s'. Error: %s", namespace, err.Error()) + + log.Error().Err(err).Msg(message) + + return "", &RouterParsingError{ + ParamName: namespaceIDParam, + ParamValue: namespace, + ErrString: err.Error(), + } + } + + return namespace, nil +} + +// getWorkloads retrieves all namespaces and workloads for given organization +func (server *HTTPServer) getWorkloads(writer http.ResponseWriter, request *http.Request) { + tStart := time.Now() + + // extract org_id from URL + orgID, ok := readOrgID(writer, request) + if !ok { + // everything has been handled + return + } + log.Debug().Int(orgIDStr, int(orgID)).Msg("getWorkloads") + + workloads, err := server.StorageDvo.ReadWorkloadsForOrganization(orgID) + if err != nil { + log.Error().Err(err).Msg("Errors retrieving DVO workload recommendations from storage") + handleServerError(writer, err) + return + } + + processedWorkloads := server.processDVOWorkloads(workloads) + + log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( + "getWorkloads took %s", time.Since(tStart), + ) + err = responses.SendOK(writer, responses.BuildOkResponseWithData("workloads", processedWorkloads)) + if err != nil { + log.Error().Err(err).Msg(responseDataError) + } +} + +func (server *HTTPServer) processDVOWorkloads(workloads []types.DVOReport) ( + processedWorkloads []WorkloadsForNamespace, +) { + for _, workload := range workloads { + processedWorkloads = append(processedWorkloads, WorkloadsForNamespace{ + Cluster: Cluster{ + UUID: workload.ClusterID, + }, + Namespace: Namespace{ + UUID: workload.NamespaceID, + Name: workload.NamespaceName, + }, + Metadata: Metadata{ + Recommendations: int(workload.Recommendations), + Objects: int(workload.Objects), + ReportedAt: string(workload.ReportedAt), + LastCheckedAt: string(workload.LastCheckedAt), + }, + // TODO: fill RecommendationsHitCount map efficiently instead of processing the report again every time + }) + } + + return +} + +// getWorkloadsForNamespace retrieves data about a single namespace within a cluster +func (server *HTTPServer) getWorkloadsForNamespace(writer http.ResponseWriter, request *http.Request) { + tStart := time.Now() + + orgID, ok := readOrgID(writer, request) + if !ok { + // everything has been handled + return + } + + clusterName, successful := readClusterName(writer, request) + if !successful { + // everything has been handled already + return + } + + namespaceID, err := readNamespace(writer, request) + if err != nil { + return + } + + log.Debug().Int(orgIDStr, int(orgID)).Str("namespaceID", namespaceID).Msgf("getWorkloadsForNamespace cluster %v", clusterName) + + workload, err := server.StorageDvo.ReadWorkloadsForClusterAndNamespace(orgID, clusterName, namespaceID) + if err != nil { + log.Error().Err(err).Msg("Errors retrieving DVO workload recommendations from storage") + handleServerError(writer, err) + return + } + + processedWorkload := server.processSingleDVONamespace(workload) + + log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( + "getWorkloadsForNamespace -\n\n\n\n\n\n\n %v", processedWorkload, + ) + log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( + "getWorkloadsForNamespace took %s", time.Since(tStart), + ) + err = responses.SendOK(writer, responses.BuildOkResponseWithData("workloads", processedWorkload)) + if err != nil { + log.Error().Err(err).Msg(responseDataError) + } +} + +// processSingleDVONamespace processes a report, filters out mismatching namespaces, returns processed results +func (server *HTTPServer) processSingleDVONamespace(workload types.DVOReport) ( + processedWorkloads WorkloadsForCluster, +) { + processedWorkloads = WorkloadsForCluster{ + Cluster: Cluster{ + UUID: workload.ClusterID, + }, + Namespace: Namespace{ + UUID: workload.NamespaceID, + Name: workload.NamespaceName, + }, + Metadata: Metadata{ + Recommendations: int(workload.Recommendations), + Objects: int(workload.Objects), + ReportedAt: string(workload.ReportedAt), + LastCheckedAt: string(workload.LastCheckedAt), + }, + Recommendations: []DVORecommendation{}, + } + + var dvoReport types.DVOMetrics + // remove doubled escape characters due to improper encoding during storage + s := strings.Replace(workload.Report, "\\", "", -1) + json.Unmarshal(json.RawMessage(s), &dvoReport) + + for _, recommendation := range dvoReport.WorkloadRecommendations { + filteredObjects := make([]DVOObject, 0) + for i := range recommendation.Workloads { + object := &recommendation.Workloads[i] + + // filter out other namespaces + if object.NamespaceUID != processedWorkloads.Namespace.UUID { + continue + } + filteredObjects = append(filteredObjects, DVOObject{ + Kind: object.Kind, + UID: object.UID, + }) + } + + compositeRuleID, err := generators.GenerateCompositeRuleID( + // for some unknown reason, there's a `.recommendation` suffix for each rule hit. no idea why... + types.RuleFQDN(strings.TrimSuffix(recommendation.Component, RecommendationSuffix)), + types.ErrorKey(recommendation.Key), + ) + if err != nil { + log.Error().Err(err).Msgf("error generating composite rule ID for rule [%+v]", recommendation) + continue + } + + processedWorkloads.Recommendations = append(processedWorkloads.Recommendations, DVORecommendation{ + Check: string(compositeRuleID), + Objects: filteredObjects, + TemplateData: recommendation.Details, + }) + } + + return +} From b0a741f7ab5e18d132523b5b5732b9b7744833d9 Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 18:49:12 +0100 Subject: [PATCH 5/8] add storage part for new DVO related endpoints/handlers --- storage/dvo_recommendations_storage.go | 122 +++++++++++++++++++- storage/noop_dvo_recommendations_storage.go | 14 +++ storage/ocp_recommendations_storage.go | 4 +- 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/storage/dvo_recommendations_storage.go b/storage/dvo_recommendations_storage.go index 08330185..7c98b684 100644 --- a/storage/dvo_recommendations_storage.go +++ b/storage/dvo_recommendations_storage.go @@ -50,6 +50,12 @@ type DVORecommendationsStorage interface { storedAtTime time.Time, requestID types.RequestID, ) error + ReadWorkloadsForOrganization(types.OrgID) ([]types.DVOReport, error) + ReadWorkloadsForClusterAndNamespace( + types.OrgID, + types.ClusterName, + string, + ) (types.DVOReport, error) } // dvoDBSchema represents the name of the DB schema used by DVO-related queries/migrations @@ -137,7 +143,7 @@ func (storage DVORecommendationsDBStorage) Init() error { for rows.Next() { var ( clusterName types.ClusterName - lastChecked time.Time + lastChecked sql.NullTime ) if err := rows.Scan(&clusterName, &lastChecked); err != nil { @@ -147,7 +153,7 @@ func (storage DVORecommendationsDBStorage) Init() error { return err } - storage.clustersLastChecked[clusterName] = lastChecked + storage.clustersLastChecked[clusterName] = lastChecked.Time } // Not using defer to close the rows here to: @@ -402,3 +408,115 @@ func (storage DVORecommendationsDBStorage) getReportedAtMap(orgID types.OrgID, c } return reportedAtMap, err } + +// ReadWorkloadsForOrganization returns all rows from dvo.dvo_report table for given organizaiton +func (storage DVORecommendationsDBStorage) ReadWorkloadsForOrganization(orgID types.OrgID) ( + workloads []types.DVOReport, + err error, +) { + tStart := time.Now() + query := ` + SELECT cluster_id, namespace_id, namespace_name, recommendations, objects, reported_at, last_checked_at + FROM dvo.dvo_report + WHERE org_id = $1 + ` + + // #nosec G202 + rows, err := storage.connection.Query(query, orgID) + + err = types.ConvertDBError(err, orgID) + if err != nil { + return workloads, err + } + + defer closeRows(rows) + + var ( + dvoReport types.DVOReport + lastCheckedAtDB sql.NullTime + reportedAtDB sql.NullTime + count uint + ) + for rows.Next() { + err = rows.Scan( + &dvoReport.ClusterID, + &dvoReport.NamespaceID, + &dvoReport.NamespaceName, + &dvoReport.Recommendations, + &dvoReport.Objects, + &lastCheckedAtDB, + &reportedAtDB, + ) + if err != nil { + log.Error().Err(err).Msg("ReadWorkloadsForOrganization") + } + + // convert timestamps to string + dvoReport.LastCheckedAt = types.Timestamp(lastCheckedAtDB.Time.UTC().Format(time.RFC3339)) + dvoReport.ReportedAt = types.Timestamp(reportedAtDB.Time.UTC().Format(time.RFC3339)) + + workloads = append(workloads, dvoReport) + count++ + } + + log.Debug().Int("org_id", int(orgID)).Msgf("ReadWorkloadsForOrganization processed %d rows in %v", count, time.Since(tStart)) + + return workloads, err +} + +// ReadWorkloadsForClusterAndNamespace returns a single result from the dvo.dvo_report table +func (storage DVORecommendationsDBStorage) ReadWorkloadsForClusterAndNamespace( + orgID types.OrgID, + clusterID types.ClusterName, + namespaceID string, +) ( + workload types.DVOReport, + err error, +) { + tStart := time.Now() + query := ` + SELECT cluster_id, namespace_id, namespace_name, recommendations, report, objects, reported_at, last_checked_at + FROM dvo.dvo_report + WHERE org_id = $1 + AND cluster_id = $2 + AND namespace_id = $3 + ` + + // #nosec G202 + rows, err := storage.connection.Query(query, orgID, clusterID, namespaceID) + + err = types.ConvertDBError(err, orgID) + if err != nil { + return workload, err + } + + defer closeRows(rows) + + var ( + dvoReport types.DVOReport + lastCheckedAtDB sql.NullTime + reportedAtDB sql.NullTime + ) + for rows.Next() { + err = rows.Scan( + &dvoReport.ClusterID, + &dvoReport.NamespaceID, + &dvoReport.NamespaceName, + &dvoReport.Recommendations, + &dvoReport.Report, + &dvoReport.Objects, + &lastCheckedAtDB, + &reportedAtDB, + ) + if err != nil { + log.Error().Err(err).Msg("ReadWorkloadsForClusterAndNamespace") + } + } + // convert timestamps to string + dvoReport.LastCheckedAt = types.Timestamp(lastCheckedAtDB.Time.UTC().Format(time.RFC3339)) + dvoReport.ReportedAt = types.Timestamp(reportedAtDB.Time.UTC().Format(time.RFC3339)) + + log.Debug().Int("org_id", int(orgID)).Msgf("ReadWorkloadsForClusterAndNamespace took %v", time.Since(tStart)) + + return dvoReport, err +} diff --git a/storage/noop_dvo_recommendations_storage.go b/storage/noop_dvo_recommendations_storage.go index 5c58e904..43b95249 100644 --- a/storage/noop_dvo_recommendations_storage.go +++ b/storage/noop_dvo_recommendations_storage.go @@ -77,3 +77,17 @@ func (*NoopDVOStorage) WriteReportForCluster( ) error { return nil } + +// ReadWorkloadsForOrganization noop +func (*NoopDVOStorage) ReadWorkloadsForOrganization(types.OrgID) ([]types.DVOReport, error) { + return nil, nil +} + +// ReadWorkloadsForClusterAndNamespace noop +func (*NoopDVOStorage) ReadWorkloadsForClusterAndNamespace( + types.OrgID, + types.ClusterName, + string, +) (types.DVOReport, error) { + return types.DVOReport{}, nil +} diff --git a/storage/ocp_recommendations_storage.go b/storage/ocp_recommendations_storage.go index 3d575cd8..d994ac24 100644 --- a/storage/ocp_recommendations_storage.go +++ b/storage/ocp_recommendations_storage.go @@ -419,7 +419,7 @@ func (storage OCPRecommendationsDBStorage) Init() error { for rows.Next() { var ( clusterName types.ClusterName - lastChecked time.Time + lastChecked sql.NullTime ) if err := rows.Scan(&clusterName, &lastChecked); err != nil { @@ -429,7 +429,7 @@ func (storage OCPRecommendationsDBStorage) Init() error { return err } - storage.clustersLastChecked[clusterName] = lastChecked + storage.clustersLastChecked[clusterName] = lastChecked.Time } // Not using defer to close the rows here to: From 6abda752181ef83f393f180f0d6837568f863681 Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 19:55:03 +0100 Subject: [PATCH 6/8] fix merge conflicts --- storage/dvo_recommendations_storage.go | 2 ++ storage/dvo_recommendations_storage_test.go | 9 ++++++++- storage/noop_dvo_recommendations_storage.go | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/storage/dvo_recommendations_storage.go b/storage/dvo_recommendations_storage.go index 5b10c76a..e1c86600 100644 --- a/storage/dvo_recommendations_storage.go +++ b/storage/dvo_recommendations_storage.go @@ -527,6 +527,8 @@ func (storage DVORecommendationsDBStorage) ReadWorkloadsForClusterAndNamespace( log.Debug().Int("org_id", int(orgID)).Msgf("ReadWorkloadsForClusterAndNamespace took %v", time.Since(tStart)) return dvoReport, err +} + // DeleteReportsForOrg deletes all reports related to the specified organization from the storage. func (storage DVORecommendationsDBStorage) DeleteReportsForOrg(orgID types.OrgID) error { _, err := storage.connection.Exec("DELETE FROM dvo.dvo_report WHERE org_id = $1;", orgID) diff --git a/storage/dvo_recommendations_storage_test.go b/storage/dvo_recommendations_storage_test.go index 3573e046..eb31675c 100644 --- a/storage/dvo_recommendations_storage_test.go +++ b/storage/dvo_recommendations_storage_test.go @@ -94,7 +94,14 @@ var ( Jira: []string{"https://issues.redhat.com/browse/AN_ISSUE"}, ProductDocumentation: []string{}, }, - Details: types.DVODetails{CheckName: "", CheckURL: ""}, + Details: map[string]interface{}{ + "check_name": "", + "check_url": "", + "samples": []map[string]interface{}{ + {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, + {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + }, + }, Tags: []string{}, Workloads: []types.DVOWorkload{namespaceAWorkload, namespaceBWorkload}, }, diff --git a/storage/noop_dvo_recommendations_storage.go b/storage/noop_dvo_recommendations_storage.go index 1f969565..c3fc4a84 100644 --- a/storage/noop_dvo_recommendations_storage.go +++ b/storage/noop_dvo_recommendations_storage.go @@ -90,6 +90,8 @@ func (*NoopDVOStorage) ReadWorkloadsForClusterAndNamespace( string, ) (types.DVOReport, error) { return types.DVOReport{}, nil +} + // DeleteReportsForOrg noop func (*NoopDVOStorage) DeleteReportsForOrg(types.OrgID) error { return nil From 748cf5d791d28d9e6c4080f5185de5068bb845e8 Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 20:31:29 +0100 Subject: [PATCH 7/8] add proper details section to the message for testing DVO consumer fix unchecked err (golangci-lint) fix samples map fix exported var comment fix nested structure in test DVO message fix goconst --- consumer/consumer_test.go | 2 +- consumer/dvo_rules_consumer_test.go | 9 +++++---- server/dvo_handlers.go | 20 ++++++++++++-------- storage/dvo_recommendations_storage.go | 12 ++++++++---- storage/rule_disable.go | 2 +- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/consumer/consumer_test.go b/consumer/consumer_test.go index a901afb2..1439801b 100644 --- a/consumer/consumer_test.go +++ b/consumer/consumer_test.go @@ -50,7 +50,7 @@ const ( organizationIDNotInAllowList = "organization ID is not in allow list" testReport = `{"fingerprints": [], "info": [], "skips": [], "system": {}, "analysis_metadata":{"metadata":"some metadata"},"reports":[{"rule_id":"rule_4|RULE_4","component":"ccx_rules_ocp.external.rules.rule_1.report","type":"rule","key":"RULE_4","details":"some details"},{"rule_id":"rule_4|RULE_4","component":"ccx_rules_ocp.external.rules.rule_2.report","type":"rule","key":"RULE_2","details":"some details"},{"rule_id":"rule_5|RULE_5","component":"ccx_rules_ocp.external.rules.rule_5.report","type":"rule","key":"RULE_3","details":"some details"}]}` - testMetrics = `{"system":{"metadata":{},"hostname":null},"fingerprints":[],"version":1,"analysis_metadata":{},"workload_recommendations":[{"response_id":"an_issue|DVO_AN_ISSUE","component":"ccx_rules_ocp.external.dvo.an_issue_pod.recommendation","key":"DVO_AN_ISSUE","details":{},"tags":[],"links":{"jira":["https://issues.redhat.com/browse/AN_ISSUE"],"product_documentation":[]},"workloads":[{"namespace":"namespace-name-A","namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","name":"test-name-0099","uid":"UID-0099"}]}]}` + testMetrics = `{"system":{"metadata":{},"hostname":null},"fingerprints":[],"version":1,"analysis_metadata":{},"workload_recommendations":[{"response_id":"an_issue|DVO_AN_ISSUE","component":"ccx_rules_ocp.external.dvo.an_issue_pod.recommendation","key":"DVO_AN_ISSUE","details":{"check_name":"","check_url":"","samples":[{"namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","uid":"193a2099-1234-5678-916a-d570c9aac158"}]},"tags":[],"links":{"jira":["https://issues.redhat.com/browse/AN_ISSUE"],"product_documentation":[]},"workloads":[{"namespace":"namespace-name-A","namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","name":"test-name-0099","uid":"193a2099-1234-5678-916a-d570c9aac158"}]}]}` ) var ( diff --git a/consumer/dvo_rules_consumer_test.go b/consumer/dvo_rules_consumer_test.go index 83796e42..82471a42 100644 --- a/consumer/dvo_rules_consumer_test.go +++ b/consumer/dvo_rules_consumer_test.go @@ -351,9 +351,10 @@ func TestParseDVOMessageWithProperMetrics(t *testing.T) { Details: map[string]interface{}{ "check_name": "", "check_url": "", - "samples": []map[string]interface{}{ - {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, - {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + "samples": []interface{}{ + map[string]interface{}{ + "namespace_uid": "NAMESPACE-UID-A", "kind": "DaemonSet", "uid": "193a2099-1234-5678-916a-d570c9aac158", + }, }, }, Tags: []string{}, @@ -363,7 +364,7 @@ func TestParseDVOMessageWithProperMetrics(t *testing.T) { NamespaceUID: "NAMESPACE-UID-A", Kind: "DaemonSet", Name: "test-name-0099", - UID: "UID-0099", + UID: "193a2099-1234-5678-916a-d570c9aac158", }, }, }, diff --git a/server/dvo_handlers.go b/server/dvo_handlers.go index c3693d4f..a719906f 100644 --- a/server/dvo_handlers.go +++ b/server/dvo_handlers.go @@ -32,7 +32,8 @@ import ( ) const ( - namespaceIDParam = "namespace" + namespaceIDParam = "namespace" + // RecommendationSuffix is used to strip a suffix from rule ID RecommendationSuffix = ".recommendation" ) @@ -154,7 +155,7 @@ func (server *HTTPServer) getWorkloads(writer http.ResponseWriter, request *http processedWorkloads := server.processDVOWorkloads(workloads) - log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( + log.Debug().Uint32(orgIDStr, uint32(orgID)).Msgf( "getWorkloads took %s", time.Since(tStart), ) err = responses.SendOK(writer, responses.BuildOkResponseWithData("workloads", processedWorkloads)) @@ -220,9 +221,6 @@ func (server *HTTPServer) getWorkloadsForNamespace(writer http.ResponseWriter, r processedWorkload := server.processSingleDVONamespace(workload) - log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( - "getWorkloadsForNamespace -\n\n\n\n\n\n\n %v", processedWorkload, - ) log.Info().Uint32(orgIDStr, uint32(orgID)).Msgf( "getWorkloadsForNamespace took %s", time.Since(tStart), ) @@ -256,7 +254,12 @@ func (server *HTTPServer) processSingleDVONamespace(workload types.DVOReport) ( var dvoReport types.DVOMetrics // remove doubled escape characters due to improper encoding during storage s := strings.Replace(workload.Report, "\\", "", -1) - json.Unmarshal(json.RawMessage(s), &dvoReport) + + err := json.Unmarshal(json.RawMessage(s), &dvoReport) + if err != nil { + log.Error().Err(err).Msg("error unmarshalling full report") + return + } for _, recommendation := range dvoReport.WorkloadRecommendations { filteredObjects := make([]DVOObject, 0) @@ -273,13 +276,14 @@ func (server *HTTPServer) processSingleDVONamespace(workload types.DVOReport) ( }) } + // recommendation.ResponseID doesn't contain the full rule ID, so smart-proxy was unable to retrieve content, we need to build it compositeRuleID, err := generators.GenerateCompositeRuleID( - // for some unknown reason, there's a `.recommendation` suffix for each rule hit. no idea why... + // for some unknown reason, there's a `.recommendation` suffix for each rule hit instead of the usual .report types.RuleFQDN(strings.TrimSuffix(recommendation.Component, RecommendationSuffix)), types.ErrorKey(recommendation.Key), ) if err != nil { - log.Error().Err(err).Msgf("error generating composite rule ID for rule [%+v]", recommendation) + log.Error().Err(err).Msg("error generating composite rule ID for rule") continue } diff --git a/storage/dvo_recommendations_storage.go b/storage/dvo_recommendations_storage.go index e1c86600..11acc996 100644 --- a/storage/dvo_recommendations_storage.go +++ b/storage/dvo_recommendations_storage.go @@ -59,8 +59,12 @@ type DVORecommendationsStorage interface { DeleteReportsForOrg(orgID types.OrgID) error } -// dvoDBSchema represents the name of the DB schema used by DVO-related queries/migrations -const dvoDBSchema = "dvo" +const ( + // dvoDBSchema represents the name of the DB schema used by DVO-related queries/migrations + dvoDBSchema = "dvo" + // orgIDStr used in log messages + orgIDStr = "orgID" +) // DVORecommendationsDBStorage is an implementation of Storage interface that use selected SQL like database // like PostgreSQL or RDS etc. That implementation is based on the standard @@ -467,7 +471,7 @@ func (storage DVORecommendationsDBStorage) ReadWorkloadsForOrganization(orgID ty count++ } - log.Debug().Int("org_id", int(orgID)).Msgf("ReadWorkloadsForOrganization processed %d rows in %v", count, time.Since(tStart)) + log.Debug().Int(orgIDStr, int(orgID)).Msgf("ReadWorkloadsForOrganization processed %d rows in %v", count, time.Since(tStart)) return workloads, err } @@ -524,7 +528,7 @@ func (storage DVORecommendationsDBStorage) ReadWorkloadsForClusterAndNamespace( dvoReport.LastCheckedAt = types.Timestamp(lastCheckedAtDB.Time.UTC().Format(time.RFC3339)) dvoReport.ReportedAt = types.Timestamp(reportedAtDB.Time.UTC().Format(time.RFC3339)) - log.Debug().Int("org_id", int(orgID)).Msgf("ReadWorkloadsForClusterAndNamespace took %v", time.Since(tStart)) + log.Debug().Int(orgIDStr, int(orgID)).Msgf("ReadWorkloadsForClusterAndNamespace took %v", time.Since(tStart)) return dvoReport, err } diff --git a/storage/rule_disable.go b/storage/rule_disable.go index 3e6b6d82..237a4ff2 100644 --- a/storage/rule_disable.go +++ b/storage/rule_disable.go @@ -72,7 +72,7 @@ func (storage OCPRecommendationsDBStorage) EnableRuleSystemWide( ruleID types.RuleID, errorKey types.ErrorKey, ) error { - log.Info().Int("org_id", int(orgID)).Msgf("re-enabling rule %v|%v", ruleID, errorKey) + log.Info().Int(orgIDStr, int(orgID)).Msgf("re-enabling rule %v|%v", ruleID, errorKey) const query = `DELETE FROM rule_disable WHERE org_id = $1 From 3958b74ad09565c3ba14ce3c344d841d8632a0bb Mon Sep 17 00:00:00 2001 From: Daniel Bily Date: Sun, 25 Feb 2024 21:31:52 +0100 Subject: [PATCH 8/8] fix improper details type --- storage/dvo_recommendations_storage_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/storage/dvo_recommendations_storage_test.go b/storage/dvo_recommendations_storage_test.go index eb31675c..9491d5fc 100644 --- a/storage/dvo_recommendations_storage_test.go +++ b/storage/dvo_recommendations_storage_test.go @@ -66,9 +66,10 @@ var ( Details: map[string]interface{}{ "check_name": "", "check_url": "", - "samples": []map[string]interface{}{ - {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, - {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + "samples": []interface{}{ + map[string]interface{}{ + "namespace_uid": "NAMESPACE-UID-A", "kind": "DaemonSet", "uid": "193a2099-1234-5678-916a-d570c9aac158", + }, }, }, Tags: []string{}, @@ -83,7 +84,7 @@ var ( }, }, } - validReport = `{"system":{"metadata":{},"hostname":null},"fingerprints":[],"version":1,"analysis_metadata":{},"workload_recommendations":[{"response_id":"an_issue|DVO_AN_ISSUE","component":"ccx_rules_ocp.external.dvo.an_issue_pod.recommendation","key":"DVO_AN_ISSUE","details":{},"tags":[],"links":{"jira":["https://issues.redhat.com/browse/AN_ISSUE"],"product_documentation":[]},"workloads":[{"namespace":"namespace-name-A","namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","name":"test-name-0099","uid":"UID-0099"}]}]}` + validReport = `{"system":{"metadata":{},"hostname":null},"fingerprints":[],"version":1,"analysis_metadata":{},"workload_recommendations":[{"response_id":"an_issue|DVO_AN_ISSUE","component":"ccx_rules_ocp.external.dvo.an_issue_pod.recommendation","key":"DVO_AN_ISSUE","details":{"check_name":"","check_url":"","samples":[{"namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","uid":"193a2099-1234-5678-916a-d570c9aac158"}]},"tags":[],"links":{"jira":["https://issues.redhat.com/browse/AN_ISSUE"],"product_documentation":[]},"workloads":[{"namespace":"namespace-name-A","namespace_uid":"NAMESPACE-UID-A","kind":"DaemonSet","name":"test-name-0099","uid":"UID-0099"}]}]}` twoNamespacesRecommendation = []types.WorkloadRecommendation{ { @@ -97,9 +98,10 @@ var ( Details: map[string]interface{}{ "check_name": "", "check_url": "", - "samples": []map[string]interface{}{ - {"namespace_uid": "193a2099-1234-5678-916a-d570c9aac158", "kind": "Deployment", "uid": "0501e150-1234-5678-907f-ee732c25044a"}, - {"namespace_uid": "337477af-1234-5678-b258-16f19d8a6289", "kind": "Deployment", "uid": "8c534861-1234-5678-9af5-913de71a545b"}, + "samples": []interface{}{ + map[string]interface{}{ + "namespace_uid": "NAMESPACE-UID-A", "kind": "DaemonSet", "uid": "193a2099-1234-5678-916a-d570c9aac158", + }, }, }, Tags: []string{},