Implement Cleanup Handler to update resolved failures for targets lik…

…e SecurityHub (#426)

* Implement Cleanup Handler to update resolved failures for targets like SecurityHub

Signed-off-by: Frank Jogeleit <frank.jogeleit@lovoo.com>

charts/policy-reporter/config.yaml

@@ -296,6 +296,8 @@ securityHub:
   accountID: {{ .Values.target.securityHub.accountID }}
   accessKeyID: {{ .Values.target.securityHub.accessKeyID }}
   secretAccessKey:  {{ .Values.target.securityHub.secretAccessKey }}
+  delayInSeconds: {{ .Values.target.securityHub.delayInSeconds }}
+  cleanup: {{ .Values.target.securityHub.cleanup }}
   secretRef: {{ .Values.target.securityHub.secretRef | quote }}
   mountedSecret: {{ .Values.target.securityHub.mountedSecret | quote }}
   productName: {{ .Values.target.securityHub.productName | quote }}

charts/policy-reporter/values.yaml

@@ -661,6 +661,10 @@ target:
     sources: []
     # Skip already existing PolicyReportResults on startup
     skipExistingOnStartup: true
+    # Enable cleanup listener for SecurityHub
+    cleanup: false
+    # Delay between AWS GetFindings API calls, to avoid hitting the API RequestLimit
+    delayInSeconds: 2
     # Added as additional properties to each securityHub event
     customFields: {}
     # filter results send by namespaces, policies and priorities

pkg/config/config.go

@@ -207,6 +207,8 @@ type SecurityHub struct {
 	AWSConfig         `mapstructure:",squash"`
 	AccountID         string         `mapstructure:"accountId"`
 	ProductName       string         `mapstructure:"productName"`
+	DelayInSeconds    int            `mapstructure:"delayInSeconds"`
+	Cleanup           bool           `mapstructure:"cleanup"`
 	Channels          []*SecurityHub `mapstructure:"channels"`
 }
 

pkg/config/resolver.go

@@ -47,8 +47,8 @@ type Resolver struct {
 	database           *bun.DB
 	policyReportClient report.PolicyReportClient
 	leaderElector      *leaderelection.Client
-	targetClients      []target.Client
 	resultCache        cache.Cache
+	targetClients      []target.Client
 	targetsCreated     bool
 	logger             *zap.Logger
 	resultListener     *listener.ResultListener
@@ -198,6 +198,8 @@ func (r *Resolver) RegisterNewResultsListener() {
 	newResultListener := listener.NewResultListener(r.SkipExistingOnStartup(), r.ResultCache(), time.Now())
 	r.resultListener = newResultListener
 	r.EventPublisher().RegisterListener(listener.NewResults, newResultListener.Listen)
+
+	r.EventPublisher().RegisterPostListener(listener.CleanUpListener, listener.NewCleanupListener(context.Background(), targets))
 }
 
 // RegisterSendResultListener resolver method

pkg/config/resolver_test.go

@@ -149,8 +149,9 @@ var testConfig = &config.Config{
 			Endpoint:        "https://yds.serverless.yandexcloud.net",
 			Region:          "ru-central1",
 		},
-		AccountID: "AccountID",
-		Channels:  []*config.SecurityHub{{}},
+		AccountID:      "AccountID",
+		Channels:       []*config.SecurityHub{{}},
+		DelayInSeconds: 2,
 	},
 	GCS: &config.GCS{
 		TargetBaseOptions: config.TargetBaseOptions{

pkg/config/target_factory.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"strings"
+	"time"
 
 	_ "github.com/mattn/go-sqlite3"
 	"go.uber.org/zap"
@@ -725,6 +726,7 @@ func (f *TargetFactory) createSecurityHub(config, parent *SecurityHub) target.Cl
 	sugar.Infof("%s configured", config.Name)
 
 	setFallback(&config.ProductName, parent.ProductName, "Policy Reporter")
+	setInt(&config.DelayInSeconds, parent.DelayInSeconds)
 
 	return securityhub.NewClient(securityhub.Options{
 		ClientOptions: config.ClientOptions(),
@@ -733,6 +735,7 @@ func (f *TargetFactory) createSecurityHub(config, parent *SecurityHub) target.Cl
 		AccountID:     config.AccountID,
 		Region:        config.Region,
 		ProductName:   config.ProductName,
+		Delay:         time.Duration(config.DelayInSeconds) * time.Second,
 	})
 }
 
@@ -963,6 +966,17 @@ func setFallback(config *string, parents ...string) {
 	}
 }
 
+func setInt(config *int, parents ...int) {
+	if *config == 0 {
+		for _, p := range parents {
+			if p > 0 {
+				*config = p
+				return
+			}
+		}
+	}
+}
+
 func setBool(config *bool, parent bool) {
 	if *config == false {
 		*config = parent

pkg/config/target_factory_test.go

@@ -35,12 +35,13 @@ func newFakeClient() v1.SecretInterface {
 			"webhook":         []byte("http://localhost:9200/webhook"),
 			"accessKeyID":     []byte("accessKeyID"),
 			"secretAccessKey": []byte("secretAccessKey"),
+			"accountID":       []byte("accountID"),
 			"kmsKeyId":        []byte("kmsKeyId"),
 			"token":           []byte("token"),
 			"credentials":     []byte(`{"token": "token", "type": "authorized_user"}`),
 			"database":        []byte("database"),
 			"dsn":             []byte(""),
-			"typelessApi":     []byte("false"),
+			"typelessApi":     []byte("true"),
 		},
 	}).CoreV1().Secrets("default")
 }
@@ -354,8 +355,8 @@ func Test_GetValuesFromSecret(t *testing.T) {
 		}
 
 		typelessApi := client.FieldByName("typelessApi").Bool()
-		if typelessApi != false {
-			t.Errorf("Expected typelessApi false value from secret, got %t", typelessApi)
+		if typelessApi == false {
+			t.Errorf("Expected typelessApi true value from secret, got %t", typelessApi)
 		}
 	})
 
@@ -463,6 +464,13 @@ func Test_GetValuesFromSecret(t *testing.T) {
 		}
 	})
 
+	t.Run("Get SecurityHub values from Secret", func(t *testing.T) {
+		clients := factory.SecurityHubs(&config.SecurityHub{TargetBaseOptions: config.TargetBaseOptions{SecretRef: secretName}, AWSConfig: config.AWSConfig{Endpoint: "endpoint", Region: "region"}})
+		if len(clients) != 1 {
+			t.Error("Expected one client created")
+		}
+	})
+
 	t.Run("Get GCS values from Secret", func(t *testing.T) {
 		clients := factory.GCSClients(&config.GCS{TargetBaseOptions: config.TargetBaseOptions{SecretRef: secretName}, Bucket: "bucket"})
 		if len(clients) != 1 {

pkg/crd/api/policyreport/v1alpha2/common.go

@@ -285,7 +285,10 @@ func (r *PolicyReportResult) ResourceString() string {
 		return ""
 	}
 
-	res := r.GetResource()
+	return ToResourceString(r.GetResource())
+}
+
+func ToResourceString(res *corev1.ObjectReference) string {
 	var resource string
 
 	if res.Namespace != "" {

pkg/kubernetes/queue.go

@@ -96,7 +96,7 @@ func (q *Queue) processNextItem() bool {
 			defer q.lock.Unlock()
 			q.cache.Delete(key)
 		}()
-		q.debouncer.Add(report.LifecycleEvent{Type: report.Deleted, PolicyReport: mapResource(polr)})
+		q.debouncer.Add(report.LifecycleEvent{Type: report.Deleted, PolicyReport: updateResults(polr)})
 
 		return true
 	}
@@ -115,13 +115,13 @@ func (q *Queue) processNextItem() bool {
 
 	q.handleErr(err, key)
 
-	q.debouncer.Add(report.LifecycleEvent{Type: event, PolicyReport: mapResource(polr)})
+	q.debouncer.Add(report.LifecycleEvent{Type: event, PolicyReport: updateResults(polr)})
 
 	return true
 }
 
 // each result needs to know its resource it belongs to, to generate internal unique IDs
-func mapResource(polr pr.ReportInterface) pr.ReportInterface {
+func updateResults(polr pr.ReportInterface) pr.ReportInterface {
 	results := polr.GetResults()
 	for i, r := range results {
 		scope := polr.GetScope()
@@ -130,6 +130,8 @@ func mapResource(polr pr.ReportInterface) pr.ReportInterface {
 			r.Resources = append(r.Resources, *scope)
 		}
 
+		r.Priority = report.ResolvePriority(r)
+
 		results[i] = r
 	}
 

pkg/listener/cleanup.go

@@ -0,0 +1,18 @@
+package listener
+
+import (
+	"context"
+
+	"github.com/kyverno/policy-reporter/pkg/report"
+	"github.com/kyverno/policy-reporter/pkg/target"
+)
+
+const CleanUpListener = "cleanup_listener"
+
+func NewCleanupListener(ctx context.Context, handlers []target.Client) report.PolicyReportListener {
+	return func(event report.LifecycleEvent) {
+		for _, handler := range handlers {
+			handler.CleanUp(ctx, event.PolicyReport)
+		}
+	}
+}

pkg/listener/cleanup_test.go

@@ -0,0 +1,22 @@
+package listener_test
+
+import (
+	"testing"
+
+	"github.com/kyverno/policy-reporter/pkg/listener"
+	"github.com/kyverno/policy-reporter/pkg/report"
+	"github.com/kyverno/policy-reporter/pkg/target"
+)
+
+func Test_CleanupListener(t *testing.T) {
+	t.Run("Execute Cleanup Handler", func(t *testing.T) {
+		c := &client{}
+
+		slistener := listener.NewCleanupListener(ctx, []target.Client{c})
+		slistener(report.LifecycleEvent{Type: report.Added, PolicyReport: preport1})
+
+		if !c.cleanupCalled {
+			t.Error("expected cleanup method was called")
+		}
+	})
+}

pkg/listener/send_result_test.go

@@ -1,6 +1,7 @@
 package listener_test
 
 import (
+	"context"
 	"testing"
 
 	"github.com/kyverno/policy-reporter/pkg/crd/api/policyreport/v1alpha2"
@@ -14,6 +15,7 @@ type client struct {
 	Called                bool
 	skipExistingOnStartup bool
 	validated             bool
+	cleanupCalled         bool
 }
 
 func (c *client) Send(result v1alpha2.PolicyReportResult) {
@@ -40,6 +42,10 @@ func (c client) Validate(rep v1alpha2.ReportInterface, result v1alpha2.PolicyRep
 	return c.validated
 }
 
+func (c *client) CleanUp(_ context.Context, _ v1alpha2.ReportInterface) {
+	c.cleanupCalled = true
+}
+
 func Test_SendResultListener(t *testing.T) {
 	t.Run("Send Result", func(t *testing.T) {
 		c := &client{validated: true}

pkg/report/mapper.go

@@ -32,3 +32,27 @@ func (m *mapper) ResolvePriority(policy string, severity v1alpha2.PolicySeverity
 func NewMapper(priorities map[string]string) Mapper {
 	return &mapper{priorityMap: priorities}
 }
+
+func ResolvePriority(result v1alpha2.PolicyReportResult) v1alpha2.Priority {
+	if result.Result == v1alpha2.StatusSkip {
+		return v1alpha2.DebugPriority
+	}
+
+	if result.Result == v1alpha2.StatusPass {
+		return v1alpha2.InfoPriority
+	}
+
+	if result.Result == v1alpha2.StatusError {
+		return v1alpha2.ErrorPriority
+	}
+
+	if result.Result == v1alpha2.StatusWarn {
+		return v1alpha2.WarningPriority
+	}
+
+	if result.Severity != "" {
+		return v1alpha2.PriorityFromSeverity(result.Severity)
+	}
+
+	return v1alpha2.WarningPriority
+}

pkg/report/mapper_test.go

@@ -13,7 +13,7 @@ var priorityMap = map[string]string{
 
 var mapper = report.NewMapper(priorityMap)
 
-func Test_ResolvePriority(t *testing.T) {
+func Test_MapperResolvePriority(t *testing.T) {
 	t.Run("priority from map", func(t *testing.T) {
 		priority := mapper.ResolvePriority("priority-test", v1alpha2.SeverityHigh)
 		if priority != v1alpha2.WarningPriority {
@@ -43,3 +43,70 @@ func Test_ResolvePriority(t *testing.T) {
 		}
 	})
 }
+
+func Test_ResolvePriority(t *testing.T) {
+	t.Run("Status Skip", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result:   v1alpha2.StatusSkip,
+			Severity: v1alpha2.SeverityHigh,
+		})
+
+		if priority != v1alpha2.DebugPriority {
+			t.Errorf("expected priority debug, got %s", priority.String())
+		}
+	})
+
+	t.Run("Status Pass", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result:   v1alpha2.StatusPass,
+			Severity: v1alpha2.SeverityHigh,
+		})
+
+		if priority != v1alpha2.InfoPriority {
+			t.Errorf("expected priority info, got %s", priority.String())
+		}
+	})
+
+	t.Run("Status Warning", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result:   v1alpha2.StatusWarn,
+			Severity: v1alpha2.SeverityHigh,
+		})
+
+		if priority != v1alpha2.WarningPriority {
+			t.Errorf("expected priority warning, got %s", priority.String())
+		}
+	})
+
+	t.Run("Status Error", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result:   v1alpha2.StatusError,
+			Severity: v1alpha2.SeverityHigh,
+		})
+
+		if priority != v1alpha2.ErrorPriority {
+			t.Errorf("expected priority warning, got %s", priority.String())
+		}
+	})
+
+	t.Run("Status Fail Fallback", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result: v1alpha2.StatusFail,
+		})
+
+		if priority != v1alpha2.WarningPriority {
+			t.Errorf("expected priority warning as fail fallback, got %s", priority.String())
+		}
+	})
+
+	t.Run("Status Severity", func(t *testing.T) {
+		priority := report.ResolvePriority(v1alpha2.PolicyReportResult{
+			Result:   v1alpha2.StatusFail,
+			Severity: v1alpha2.SeverityCritical,
+		})
+
+		if priority != v1alpha2.CriticalPriority {
+			t.Errorf("expected priority critical, got %s", priority.String())
+		}
+	})
+}