fix: account for policy/rule deletion in aggregated reports (#5048)

* fix: account for policy/rule deletion in aggregated reports

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* reduce delay

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

cmd/kyverno/main.go

@@ -369,6 +369,8 @@ func createReportControllers(
 			aggregatereportcontroller.NewController(
 				kyvernoClient,
 				metadataFactory,
+				kyvernoV1.Policies(),
+				kyvernoV1.ClusterPolicies(),
 				resourceReportController,
 				reportsChunkSize,
 			),

pkg/controllers/report/admission/controller.go

@@ -145,9 +145,9 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam
 	}
 	// try to find resource from the cache
 	uid := reportutils.GetResourceUid(meta)
-	resource, gvk, exists := c.metadataCache.GetResourceHash(uid)
+	resource, gvk, found := c.metadataCache.GetResourceHash(uid)
 	// set owner if not done yet
-	if exists && len(meta.GetOwnerReferences()) == 0 {
+	if found && len(meta.GetOwnerReferences()) == 0 {
 		report, err := c.getReport(ctx, namespace, name)
 		if err != nil {
 			return err
@@ -158,9 +158,18 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam
 	}
 	// cleanup old reports
 	// if they are not the same version as the current resource version
-	// and were created more than five minutes ago
+	// and were created more than 2 minutes ago
-	if !exists || !reportutils.CompareHash(meta, resource.Hash) {
+	if !found {
-		if meta.GetCreationTimestamp().Add(time.Minute * 5).Before(time.Now()) {
+		// if we didn't find the resource, either no policy exist for this kind
+		// or the resource was never created, we delete the report if it has no owner
+		// and was created more than 2 minutes ago
+		if len(meta.GetOwnerReferences()) == 0 && meta.GetCreationTimestamp().Add(time.Minute*2).Before(time.Now()) {
+			return c.deleteReport(ctx, namespace, name)
+		}
+	} else {
+		// if hashes don't match and the report was created more than 2
+		// minutes ago we consider it obsolete and delete the report
+		if !reportutils.CompareHash(meta, resource.Hash) && meta.GetCreationTimestamp().Add(time.Minute*2).Before(time.Now()) {
 			return c.deleteReport(ctx, namespace, name)
 		}
 	}

pkg/controllers/report/aggregate/controller.go

@@ -9,13 +9,17 @@ import (
 	"github.com/go-logr/logr"
 	kyvernov1alpha2 "github.com/kyverno/kyverno/api/kyverno/v1alpha2"
 	policyreportv1alpha2 "github.com/kyverno/kyverno/api/policyreport/v1alpha2"
+	"github.com/kyverno/kyverno/pkg/autogen"
 	"github.com/kyverno/kyverno/pkg/client/clientset/versioned"
+	kyvernov1informers "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1"
+	kyvernov1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/controllers"
 	"github.com/kyverno/kyverno/pkg/controllers/report/resource"
 	controllerutils "github.com/kyverno/kyverno/pkg/utils/controller"
 	reportutils "github.com/kyverno/kyverno/pkg/utils/report"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	metadatainformers "k8s.io/client-go/metadata/metadatainformer"
 	"k8s.io/client-go/tools/cache"
@@ -38,6 +42,8 @@ type controller struct {
 	client versioned.Interface
 
 	// listers
+	polLister      kyvernov1listers.PolicyLister
+	cpolLister     kyvernov1listers.ClusterPolicyLister
 	admrLister     cache.GenericLister
 	cadmrLister    cache.GenericLister
 	bgscanrLister  cache.GenericLister
@@ -59,6 +65,8 @@ func keyFunc(obj metav1.Object) cache.ExplicitKey {
 func NewController(
 	client versioned.Interface,
 	metadataFactory metadatainformers.SharedInformerFactory,
+	polInformer kyvernov1informers.PolicyInformer,
+	cpolInformer kyvernov1informers.ClusterPolicyInformer,
 	metadataCache resource.MetadataCache,
 	chunkSize int,
 ) controllers.Controller {
@@ -68,6 +76,8 @@ func NewController(
 	cbgscanrInformer := metadataFactory.ForResource(kyvernov1alpha2.SchemeGroupVersion.WithResource("clusterbackgroundscanreports"))
 	c := controller{
 		client:         client,
+		polLister:      polInformer.Lister(),
+		cpolLister:     cpolInformer.Lister(),
 		admrLister:     admrInformer.Lister(),
 		cadmrLister:    cadmrInformer.Lister(),
 		bgscanrLister:  bgscanrInformer.Lister(),
@@ -160,7 +170,7 @@ func (c *controller) cleanReports(ctx context.Context, actual map[string]kyverno
 	return nil
 }
 
-func mergeReports(accumulator map[string]policyreportv1alpha2.PolicyReportResult, reports ...kyvernov1alpha2.ReportInterface) {
+func mergeReports(policyMap map[string]sets.String, accumulator map[string]policyreportv1alpha2.PolicyReportResult, reports ...kyvernov1alpha2.ReportInterface) {
 	for _, report := range reports {
 		if len(report.GetOwnerReferences()) == 1 {
 			ownerRef := report.GetOwnerReferences()[0]
@@ -172,33 +182,73 @@ func mergeReports(accumulator map[string]policyreportv1alpha2.PolicyReportResult
 				UID:        ownerRef.UID,
 			}}
 			for _, result := range report.GetResults() {
-				key := result.Policy + "/" + result.Rule + "/" + string(ownerRef.UID)
+				currentPolicy := policyMap[result.Policy]
-				result.Resources = objectRefs
+				if currentPolicy != nil && currentPolicy.Has(result.Rule) {
-				if rule, exists := accumulator[key]; !exists {
+					key := result.Policy + "/" + result.Rule + "/" + string(ownerRef.UID)
-					accumulator[key] = result
+					result.Resources = objectRefs
-				} else if rule.Timestamp.Seconds < result.Timestamp.Seconds {
+					if rule, exists := accumulator[key]; !exists {
-					accumulator[key] = result
+						accumulator[key] = result
+					} else if rule.Timestamp.Seconds < result.Timestamp.Seconds {
+						accumulator[key] = result
+					}
 				}
 			}
 		}
 	}
 }
 
-func (c *controller) buildReportsResults(ctx context.Context, namepsace string) ([]policyreportv1alpha2.PolicyReportResult, error) {
+func (c *controller) createPolicyMap() (map[string]sets.String, error) {
+	results := map[string]sets.String{}
+	cpols, err := c.cpolLister.List(labels.Everything())
+	if err != nil {
+		return nil, err
+	}
+	for _, cpol := range cpols {
+		key, err := cache.MetaNamespaceKeyFunc(cpol)
+		if err != nil {
+			return nil, err
+		}
+		results[key] = sets.NewString()
+		for _, rule := range autogen.ComputeRules(cpol) {
+			results[key].Insert(rule.Name)
+		}
+	}
+	pols, err := c.polLister.List(labels.Everything())
+	if err != nil {
+		return nil, err
+	}
+	for _, pol := range pols {
+		key, err := cache.MetaNamespaceKeyFunc(pol)
+		if err != nil {
+			return nil, err
+		}
+		results[key] = sets.NewString()
+		for _, rule := range autogen.ComputeRules(pol) {
+			results[key].Insert(rule.Name)
+		}
+	}
+	return results, nil
+}
+
+func (c *controller) buildReportsResults(ctx context.Context, namespace string) ([]policyreportv1alpha2.PolicyReportResult, error) {
+	policyMap, err := c.createPolicyMap()
+	if err != nil {
+		return nil, err
+	}
 	merged := map[string]policyreportv1alpha2.PolicyReportResult{}
 	{
-		reports, err := c.listAdmissionReports(ctx, namepsace)
+		reports, err := c.listAdmissionReports(ctx, namespace)
 		if err != nil {
 			return nil, err
 		}
-		mergeReports(merged, reports...)
+		mergeReports(policyMap, merged, reports...)
 	}
 	{
-		reports, err := c.listBackgroundScanReports(ctx, namepsace)
+		reports, err := c.listBackgroundScanReports(ctx, namespace)
 		if err != nil {
 			return nil, err
 		}
-		mergeReports(merged, reports...)
+		mergeReports(policyMap, merged, reports...)
 	}
 	var results []policyreportv1alpha2.PolicyReportResult
 	for _, result := range merged {