1370 clean up stale RCRs (kyverno#1373)

* remove env "POLICY-TYPE"

* clean up resource in goroutine

* clean up stale RCRs on namespace deletion

* go vet

* clean up code

cmd/initContainer/main.go

@@ -266,11 +266,7 @@ func removeClusterPolicyReport(client *client.Client, kind string) error {
 	}
 
 	for _, cpolr := range cpolrs.Items {
-		if err := client.DeleteResource(cpolr.GetAPIVersion(), cpolr.GetKind(), "", cpolr.GetName(), false); err != nil {
-			logger.Error(err, "failed to delete clusterPolicyReport", "name", cpolr.GetName())
-		} else {
-			logger.Info("successfully cleaned up ClusterPolicyReport")
-		}
+		deleteResource(client, cpolr.GetAPIVersion(), cpolr.GetKind(), "", cpolr.GetName(), nil)
 	}
 	return nil
 }
@@ -292,14 +288,12 @@ func removePolicyReport(client *client.Client, kind string) error {
 			fmt.Sprintf("pr-ns-%s", ns.GetName()),
 		}
 
+		var wg sync.WaitGroup
+		wg.Add(len(reportNames))
 		for _, reportName := range reportNames {
-			err := client.DeleteResource("", kind, ns.GetName(), reportName, false)
-			if err != nil && !errors.IsNotFound(err) {
-				logger.Error(err, "failed to delete resource", "kind", kind, "name", reportName)
-			} else {
-				logger.Info("successfully cleaned up resource", "kind", kind, "name", reportName)
-			}
+			go deleteResource(client, "", kind, ns.GetName(), reportName, &wg)
 		}
+		wg.Wait()
 	}
 
 	return nil
@@ -315,13 +309,12 @@ func removeReportChangeRequest(client *client.Client, kind string) error {
 		return nil
 	}
 
+	var wg sync.WaitGroup
+	wg.Add(len(rcrList.Items))
 	for _, rcr := range rcrList.Items {
-		if err := client.DeleteResource(rcr.GetAPIVersion(), rcr.GetKind(), rcr.GetNamespace(), rcr.GetName(), false); err != nil {
-			logger.Error(err, "failed to delete reportChangeRequest", "name", rcr.GetName())
-		} else {
-			logger.Info("successfully cleaned up reportChangeRequest", "name", rcr.GetName())
-		}
+		go deleteResource(client, rcr.GetAPIVersion(), rcr.GetKind(), rcr.GetNamespace(), rcr.GetName(), &wg)
 	}
+	wg.Wait()
 	return nil
 }
 
@@ -333,11 +326,7 @@ func removeClusterReportChangeRequest(client *client.Client, kind string) error
 	}
 
 	for _, crcr := range crcrList.Items {
-		if err := client.DeleteResource(crcr.GetAPIVersion(), crcr.GetKind(), "", crcr.GetName(), false); err != nil {
-			logger.Error(err, "failed to delete clusterReportChangeRequest", "name", crcr.GetName())
-		} else {
-			logger.Info("successfully cleaned up clusterReportChangeRequest")
-		}
+		deleteResource(client, crcr.GetAPIVersion(), crcr.GetKind(), "", crcr.GetName(), nil)
 	}
 	return nil
 }
@@ -365,3 +354,17 @@ func getKyvernoNameSpace() string {
 	}
 	return kyvernoNamespace
 }
+
+func deleteResource(client *client.Client, apiversion, kind, ns, name string, wg *sync.WaitGroup) {
+	if wg != nil {
+		defer wg.Done()
+	}
+
+	err := client.DeleteResource(apiversion, kind, ns, name, false)
+	if err != nil && !errors.IsNotFound(err) {
+		logger.Error(err, "failed to delete resource", "kind", kind, "name", name)
+		return
+	}
+
+	logger.Info("successfully cleaned up resource", "kind", kind, "name", name)
+}

cmd/kyverno/main.go

@@ -11,7 +11,6 @@ import (
 
 	kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned"
 	kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions"
-	"github.com/kyverno/kyverno/pkg/common"
 	"github.com/kyverno/kyverno/pkg/config"
 	dclient "github.com/kyverno/kyverno/pkg/dclient"
 	event "github.com/kyverno/kyverno/pkg/event"
@@ -80,9 +79,6 @@ func main() {
 		go http.ListenAndServe("localhost:6060", nil)
 	}
 
-	// Policy report is enabled by default in Kyverno 1.3.0+
-	os.Setenv("POLICY-TYPE", common.PolicyReport)
-
 	version.PrintVersionInfo(log.Log)
 	cleanUp := make(chan struct{})
 	stopCh := signal.SetupSignalHandler()

pkg/policy/validate_controller.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
-	"os"
 	"reflect"
 	"time"
 
@@ -14,7 +13,6 @@ import (
 	"github.com/kyverno/kyverno/pkg/client/clientset/versioned/scheme"
 	kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1"
 	kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/common"
 	"github.com/kyverno/kyverno/pkg/config"
 	"github.com/kyverno/kyverno/pkg/constant"
 	client "github.com/kyverno/kyverno/pkg/dclient"
@@ -204,9 +202,8 @@ func (pc *PolicyController) updatePolicy(old, cur interface{}) {
 	}
 
 	logger.V(4).Info("updating policy", "name", oldP.Name)
-	if os.Getenv("POLICY-TYPE") == common.PolicyReport {
-		pc.enqueueDeletedRule(oldP, curP)
-	}
+
+	pc.enqueueDeletedRule(oldP, curP)
 	pc.enqueuePolicy(curP)
 }
 
@@ -259,9 +256,8 @@ func (pc *PolicyController) updateNsPolicy(old, cur interface{}) {
 	}
 
 	logger.V(4).Info("updating namespace policy", "namespace", oldP.Namespace, "name", oldP.Name)
-	if os.Getenv("POLICY-TYPE") == common.PolicyReport {
-		pc.enqueueDeletedRule(ConvertPolicyToClusterPolicy(oldP), ncurP)
-	}
+
+	pc.enqueueDeletedRule(ConvertPolicyToClusterPolicy(oldP), ncurP)
 	pc.enqueuePolicy(ncurP)
 }
 

pkg/policyreport/builder.go

@@ -2,15 +2,13 @@ package policyreport
 
 import (
 	"fmt"
-	"os"
 	"reflect"
 
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	request "github.com/kyverno/kyverno/pkg/api/kyverno/v1alpha1"
 	report "github.com/kyverno/kyverno/pkg/api/policyreport/v1alpha1"
 	kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/common"
 	"github.com/kyverno/kyverno/pkg/config"
 	"github.com/kyverno/kyverno/pkg/engine/response"
 	"github.com/kyverno/kyverno/pkg/engine/utils"
@@ -52,12 +50,7 @@ func GeneratePRsFromEngineResponse(ers []response.EngineResponse, log logr.Logge
 			log.V(4).Info("resource does no have a name assigned yet, not creating a policy violation", "resource", er.PolicyResponse.Resource)
 			continue
 		}
-		// skip when response succeed
-		if os.Getenv("POLICY-TYPE") != common.PolicyReport {
-			if er.IsSuccessful() {
-				continue
-			}
-		}
+
 		// build policy violation info
 		pvInfos = append(pvInfos, buildPVInfo(er))
 	}
@@ -220,11 +213,6 @@ func buildPVInfo(er response.EngineResponse) Info {
 func buildViolatedRules(er response.EngineResponse) []kyverno.ViolatedRule {
 	var violatedRules []kyverno.ViolatedRule
 	for _, rule := range er.PolicyResponse.Rules {
-		if os.Getenv("POLICY-TYPE") != common.PolicyReport {
-			if rule.Success {
-				continue
-			}
-		}
 		vrule := kyverno.ViolatedRule{
 			Name:    rule.Name,
 			Type:    rule.Type,

pkg/policyreport/reportcontroller.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
+	"sync"
 
 	"github.com/go-logr/logr"
 	changerequest "github.com/kyverno/kyverno/pkg/api/kyverno/v1alpha1"
@@ -243,9 +244,13 @@ func (g *ReportGenerator) syncHandler(key string) error {
 	}
 
 	var old interface{}
-	if old, err = g.createReportIfNotPresent(namespace, new, aggregatedRequests); err != nil || old == nil {
+	if old, err = g.createReportIfNotPresent(namespace, new, aggregatedRequests); err != nil {
 		return err
 	}
+	if old == nil {
+		g.cleanupReportRequests(aggregatedRequests)
+		return nil
+	}
 
 	if err := g.updateReport(old, new, aggregatedRequests); err != nil {
 		return err
@@ -260,10 +265,16 @@ func (g *ReportGenerator) syncHandler(key string) error {
 func (g *ReportGenerator) createReportIfNotPresent(namespace string, new *unstructured.Unstructured, aggregatedRequests interface{}) (report interface{}, err error) {
 	log := g.log.WithName("createReportIfNotPresent")
 	if namespace != "" {
-		if ns, err := g.nsLister.Get(namespace); err == nil {
-			if ns.GetDeletionTimestamp() != nil {
+		ns, err := g.nsLister.Get(namespace)
+		if err != nil {
+			if apierrors.IsNotFound(err) {
 				return nil, nil
 			}
+			return nil, fmt.Errorf("failed to fetch namespace: %v", err)
+		}
+
+		if ns.GetDeletionTimestamp() != nil {
+			return nil, nil
 		}
 
 		report, err = g.reportLister.PolicyReports(namespace).Get(generatePolicyReportName((namespace)))
@@ -395,6 +406,11 @@ func (g *ReportGenerator) aggregateReports(namespace string) (
 			if !apierrors.IsNotFound(err) {
 				return nil, nil, fmt.Errorf("unable to get namespace %s: %v", namespace, err)
 			}
+			// Namespace is deleted, create a fake ns to clean up RCRs
+			ns = new(v1.Namespace)
+			ns.SetName(namespace)
+			now := metav1.Now()
+			ns.SetDeletionTimestamp(&now)
 		}
 
 		selector := labels.SelectorFromSet(labels.Set(map[string]string{resourceLabelNamespace: namespace}))
@@ -465,6 +481,7 @@ func mergeRequests(ns *v1.Namespace, requestsGeneral interface{}) (*unstructured
 
 		req := &unstructured.Unstructured{Object: obj}
 		setReport(req, ns)
+
 		return req, aggregatedRequests, nil
 	}
 
@@ -552,13 +569,19 @@ func (g *ReportGenerator) updateReport(old interface{}, new *unstructured.Unstru
 func (g *ReportGenerator) cleanupReportRequests(requestsGeneral interface{}) {
 	defer g.log.V(5).Info("successfully cleaned up report requests")
 	if requests, ok := requestsGeneral.([]*changerequest.ReportChangeRequest); ok {
+		var wg sync.WaitGroup
+		wg.Add(len(requests))
 		for _, request := range requests {
-			if err := g.dclient.DeleteResource(request.APIVersion, "ReportChangeRequest", config.KyvernoNamespace, request.Name, false); err != nil {
-				if !apierrors.IsNotFound(err) {
-					g.log.Error(err, "failed to delete report request")
+			go func(request *changerequest.ReportChangeRequest) {
+				if err := g.dclient.DeleteResource(request.APIVersion, "ReportChangeRequest", config.KyvernoNamespace, request.Name, false); err != nil {
+					if !apierrors.IsNotFound(err) {
+						g.log.Error(err, "failed to delete report request")
+					}
 				}
-			}
+				wg.Done()
+			}(request)
 		}
+		wg.Wait()
 	}
 
 	if requests, ok := requestsGeneral.([]*changerequest.ClusterReportChangeRequest); ok {

pkg/webhooks/validation.go

@@ -1,15 +1,13 @@
 package webhooks
 
 import (
-	"os"
 	"reflect"
 	"sort"
 	"time"
 
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	v1 "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/common"
 	"github.com/kyverno/kyverno/pkg/config"
 	"github.com/kyverno/kyverno/pkg/engine"
 	"github.com/kyverno/kyverno/pkg/engine/context"
@@ -130,21 +128,20 @@ func HandleValidation(
 	prInfos := policyreport.GeneratePRsFromEngineResponse(engineResponses, logger)
 	prGenerator.Add(prInfos...)
 
-	if os.Getenv("POLICY-TYPE") == common.PolicyReport {
-		if request.Operation == v1beta1.Delete {
-			prGenerator.Add(policyreport.Info{
-				Resource: unstructured.Unstructured{
-					Object: map[string]interface{}{
-						"kind": oldR.GetKind(),
-						"metadata": map[string]interface{}{
-							"name":      oldR.GetName(),
-							"namespace": oldR.GetNamespace(),
-						},
+	if request.Operation == v1beta1.Delete {
+		prGenerator.Add(policyreport.Info{
+			Resource: unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"kind": oldR.GetKind(),
+					"metadata": map[string]interface{}{
+						"name":      oldR.GetName(),
+						"namespace": oldR.GetNamespace(),
 					},
 				},
-			})
-		}
+			},
+		})
 	}
+
 	return true, ""
 }