From 3c5f9f88889ecef53361b00fcb282eec4f8e3ba0 Mon Sep 17 00:00:00 2001
From: shuting <shutting06@gmail.com>
Date: Mon, 21 Dec 2020 11:04:19 -0800
Subject: [PATCH] 1398 - Reduce RCR throttling requests (#1406)
* reduce RCR throttling requests by merging policy application (policy - namespace) results into single RCR
* - refactor policy controller; - fix RCR issue
* - refactor RCR controller; - fix cpolr on ns update; - reduce throttling when getting resources; - fix tests
* update CRD schema
* fix typo
---
charts/kyverno/crds/crds.yaml | 40 +++--
cmd/initContainer/main.go | 6 +-
.../crds/kyverno.io_clusterpolicies.yaml | 19 ++-
...yverno.io_clusterreportchangerequests.yaml | 2 +
definitions/crds/kyverno.io_policies.yaml | 19 ++-
definitions/install.yaml | 40 +++--
definitions/install_debug.yaml | 40 +++--
.../clusterreportchangerequest_types.go | 4 +-
pkg/config/dynamicconfig.go | 7 +
pkg/engine/response/response.go | 18 +-
pkg/generate/generate.go | 2 +-
pkg/kyverno/apply/report.go | 53 +++---
pkg/kyverno/apply/report_test.go | 5 +
pkg/policy/apply.go | 11 +-
pkg/policy/common.go | 15 +-
pkg/policy/existing.go | 134 +++++++++------
pkg/policy/report.go | 41 +++--
pkg/policy/validate_controller.go | 111 +++++++------
pkg/policyreport/builder.go | 156 +++++++++++-------
pkg/policyreport/policyreport.go | 49 ++++--
pkg/policyreport/reportcontroller.go | 47 ++++--
pkg/policyreport/reportrequest.go | 101 +++++++-----
pkg/webhooks/validation.go | 27 +--
23 files changed, 583 insertions(+), 364 deletions(-)
diff --git a/charts/kyverno/crds/crds.yaml b/charts/kyverno/crds/crds.yaml
index 8889ec1e07..fd643821c9 100644
--- a/charts/kyverno/crds/crds.yaml
+++ b/charts/kyverno/crds/crds.yaml
@@ -224,9 +224,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -236,9 +237,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -251,7 +253,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
@@ -959,6 +964,8 @@ spec:
kind: ClusterReportChangeRequest
listKind: ClusterReportChangeRequestList
plural: clusterreportchangerequests
+ shortNames:
+ - crcr
singular: clusterreportchangerequest
scope: Cluster
versions:
@@ -1688,9 +1695,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -1700,9 +1708,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -1715,7 +1724,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
diff --git a/cmd/initContainer/main.go b/cmd/initContainer/main.go
index 52bc2e1921..515df65d47 100644
--- a/cmd/initContainer/main.go
+++ b/cmd/initContainer/main.go
@@ -308,12 +308,10 @@ func removeReportChangeRequest(client *client.Client, kind string) error {
return nil
}
- var wg sync.WaitGroup
- wg.Add(len(rcrList.Items))
for _, rcr := range rcrList.Items {
- go deleteResource(client, rcr.GetAPIVersion(), rcr.GetKind(), rcr.GetNamespace(), rcr.GetName(), &wg)
+ deleteResource(client, rcr.GetAPIVersion(), rcr.GetKind(), rcr.GetNamespace(), rcr.GetName(), nil)
}
- wg.Wait()
+
return nil
}
diff --git a/definitions/crds/kyverno.io_clusterpolicies.yaml b/definitions/crds/kyverno.io_clusterpolicies.yaml
index ad765fd659..853ad21d58 100644
--- a/definitions/crds/kyverno.io_clusterpolicies.yaml
+++ b/definitions/crds/kyverno.io_clusterpolicies.yaml
@@ -226,9 +226,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -238,9 +239,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -253,7 +255,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
diff --git a/definitions/crds/kyverno.io_clusterreportchangerequests.yaml b/definitions/crds/kyverno.io_clusterreportchangerequests.yaml
index ff5e85728a..a841f2a1d7 100644
--- a/definitions/crds/kyverno.io_clusterreportchangerequests.yaml
+++ b/definitions/crds/kyverno.io_clusterreportchangerequests.yaml
@@ -13,6 +13,8 @@ spec:
kind: ClusterReportChangeRequest
listKind: ClusterReportChangeRequestList
plural: clusterreportchangerequests
+ shortNames:
+ - crcr
singular: clusterreportchangerequest
scope: Cluster
versions:
diff --git a/definitions/crds/kyverno.io_policies.yaml b/definitions/crds/kyverno.io_policies.yaml
index cbdd38b8bc..ab0948973b 100644
--- a/definitions/crds/kyverno.io_policies.yaml
+++ b/definitions/crds/kyverno.io_policies.yaml
@@ -227,9 +227,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -239,9 +240,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -254,7 +256,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
diff --git a/definitions/install.yaml b/definitions/install.yaml
index 1a77ea4be9..19e90cdd22 100644
--- a/definitions/install.yaml
+++ b/definitions/install.yaml
@@ -229,9 +229,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -241,9 +242,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -256,7 +258,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
@@ -964,6 +969,8 @@ spec:
kind: ClusterReportChangeRequest
listKind: ClusterReportChangeRequestList
plural: clusterreportchangerequests
+ shortNames:
+ - crcr
singular: clusterreportchangerequest
scope: Cluster
versions:
@@ -1693,9 +1700,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -1705,9 +1713,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -1720,7 +1729,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
diff --git a/definitions/install_debug.yaml b/definitions/install_debug.yaml
index 690c475a9a..9be761e691 100755
--- a/definitions/install_debug.yaml
+++ b/definitions/install_debug.yaml
@@ -229,9 +229,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -241,9 +242,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -256,7 +258,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
@@ -964,6 +969,8 @@ spec:
kind: ClusterReportChangeRequest
listKind: ClusterReportChangeRequestList
plural: clusterreportchangerequests
+ shortNames:
+ - crcr
singular: clusterreportchangerequest
scope: Cluster
versions:
@@ -1693,9 +1700,10 @@ spec:
description: APIVersion specifies resource apiVersion.
type: string
clone:
- description: Clone specified the source resource used to
- populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Clone specifies the source resource used to
+ populate each generated resource. At most one of Data
+ or Clone can be specified. If neither are provided, the
+ generated resource will be created with default data only.
properties:
name:
description: Name specifies name of the resource.
@@ -1705,9 +1713,10 @@ spec:
type: string
type: object
data:
- description: Data provides the resource manifest to used
- to populate each generated resource. Exactly one of Data
- or Clone must be specified.
+ description: Data provides the resource declaration used
+ to populate each generated resource. At most one of Data
+ or Clone must be specified. If neither are provided, the
+ generated resource will be created with default data only.
x-kubernetes-preserve-unknown-fields: true
kind:
description: Kind specifies resource kind.
@@ -1720,7 +1729,10 @@ spec:
type: string
synchronize:
description: Synchronize controls if generated resources
- should be kept in-sync with their source resource. Optional.
+ should be kept in-sync with their source resource. If
+ Synchronize is set to "true" changes to generated resources
+ will be overwritten with resource data from Data or the
+ resource specified in the Clone declaration. Optional.
Defaults to "false" if not specified.
type: boolean
type: object
diff --git a/pkg/api/kyverno/v1alpha1/clusterreportchangerequest_types.go b/pkg/api/kyverno/v1alpha1/clusterreportchangerequest_types.go
index 4234cfe2a3..7f9350839a 100644
--- a/pkg/api/kyverno/v1alpha1/clusterreportchangerequest_types.go
+++ b/pkg/api/kyverno/v1alpha1/clusterreportchangerequest_types.go
@@ -30,7 +30,7 @@ import (
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +genclient:nonNamespaced
// +kubebuilder:object:root=true
-// +kubebuilder:resource:path=clusterreportchangerequests
+// +kubebuilder:resource:path=clusterreportchangerequests,scope="Cluster",shortName=crcr
// +kubebuilder:printcolumn:name="Kind",type=string,JSONPath=`.scope.kind`,priority=1
// +kubebuilder:printcolumn:name="Name",type=string,JSONPath=`.scope.name`,priority=1
// +kubebuilder:printcolumn:name="Pass",type=integer,JSONPath=`.summary.pass`
@@ -39,8 +39,6 @@ import (
// +kubebuilder:printcolumn:name="Error",type=integer,JSONPath=`.summary.error`
// +kubebuilder:printcolumn:name="Skip",type=integer,JSONPath=`.summary.skip`
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
-// +kubebuilder:resource:shortName=crcr
-// +kubebuilder:resource:scope=Cluster
type ClusterReportChangeRequest struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
diff --git a/pkg/config/dynamicconfig.go b/pkg/config/dynamicconfig.go
index 0177b5e2b6..ada7a4b377 100644
--- a/pkg/config/dynamicconfig.go
+++ b/pkg/config/dynamicconfig.go
@@ -42,6 +42,13 @@ func (cd *ConfigData) ToFilter(kind, namespace, name string) bool {
if wildcard.Match(f.Kind, kind) && wildcard.Match(f.Namespace, namespace) && wildcard.Match(f.Name, name) {
return true
}
+
+ if kind == "Namespace" {
+ // [Namespace,kube-system,*] || [*,kube-system,*]
+ if (f.Kind == "Namespace" || f.Kind == "*") && wildcard.Match(f.Namespace, name) {
+ return true
+ }
+ }
}
return false
}
diff --git a/pkg/engine/response/response.go b/pkg/engine/response/response.go
index eeb37dd2cb..4b747c1be5 100644
--- a/pkg/engine/response/response.go
+++ b/pkg/engine/response/response.go
@@ -31,11 +31,14 @@ type PolicyResponse struct {
//ResourceSpec resource action applied on
type ResourceSpec struct {
- //TODO: support ApiVersion
Kind string `json:"kind"`
APIVersion string `json:"apiVersion"`
Namespace string `json:"namespace"`
Name string `json:"name"`
+
+ // UID is not used to build the unique identifier
+ // optional
+ UID string `json:"uid"`
}
//GetKey returns the key
@@ -51,7 +54,7 @@ type PolicyStats struct {
RulesAppliedCount int `json:"rulesAppliedCount"`
}
-//RuleResponse details for each rule applicatino
+//RuleResponse details for each rule application
type RuleResponse struct {
// rule name specified in policy
Name string `json:"name"`
@@ -110,6 +113,17 @@ func (er EngineResponse) GetSuccessRules() []string {
return er.getRules(true)
}
+// GetResourceSpec returns resourceSpec of er
+func (er EngineResponse) GetResourceSpec() ResourceSpec {
+ return ResourceSpec{
+ Kind: er.PatchedResource.GetKind(),
+ APIVersion: er.PatchedResource.GetAPIVersion(),
+ Namespace: er.PatchedResource.GetNamespace(),
+ Name: er.PatchedResource.GetName(),
+ UID: string(er.PatchedResource.GetUID()),
+ }
+}
+
func (er EngineResponse) getRules(success bool) []string {
var rules []string
for _, r := range er.PolicyResponse.Rules {
diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go
index 49f233297c..f2e739c1a3 100644
--- a/pkg/generate/generate.go
+++ b/pkg/generate/generate.go
@@ -32,7 +32,7 @@ func (c *Controller) processGR(gr *kyverno.GenerateRequest) error {
// 1 - Check if the resource exists
resource, err = getResource(c.client, gr.Spec.Resource)
if err != nil {
- // Dont update status
+ // Don't update status
logger.V(3).Info("resource does not exist or is pending creation, re-queueing", "details", err.Error())
return err
}
diff --git a/pkg/kyverno/apply/report.go b/pkg/kyverno/apply/report.go
index 51cd0b6f4e..75c3415e5c 100644
--- a/pkg/kyverno/apply/report.go
+++ b/pkg/kyverno/apply/report.go
@@ -7,11 +7,13 @@ import (
report "github.com/kyverno/kyverno/pkg/api/policyreport/v1alpha1"
"github.com/kyverno/kyverno/pkg/engine/response"
+ "github.com/kyverno/kyverno/pkg/engine/utils"
engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
"github.com/kyverno/kyverno/pkg/policyreport"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+ "k8s.io/apimachinery/pkg/types"
log "sigs.k8s.io/controller-runtime/pkg/log"
)
@@ -42,7 +44,7 @@ func buildPolicyReports(resps []response.EngineResponse, skippedPolicies []Skipp
}
if raw, err = json.Marshal(report); err != nil {
- log.Log.V(3).Info("failed to serilize policy report", "error", err)
+ log.Log.V(3).Info("failed to serialize policy report", "error", err)
continue
}
@@ -70,7 +72,7 @@ func buildPolicyReports(resps []response.EngineResponse, skippedPolicies []Skipp
report.SetName(scope)
if raw, err = json.Marshal(report); err != nil {
- log.Log.V(3).Info("failed to serilize policy report", "name", report.Name, "scope", scope, "error", err)
+ log.Log.V(3).Info("failed to serialize policy report", "name", report.Name, "scope", scope, "error", err)
}
} else {
report := &report.PolicyReport{
@@ -87,7 +89,7 @@ func buildPolicyReports(resps []response.EngineResponse, skippedPolicies []Skipp
report.SetNamespace(ns)
if raw, err = json.Marshal(report); err != nil {
- log.Log.V(3).Info("failed to serilize policy report", "name", report.Name, "scope", scope, "error", err)
+ log.Log.V(3).Info("failed to serialize policy report", "name", report.Name, "scope", scope, "error", err)
}
}
@@ -111,33 +113,38 @@ func buildPolicyResults(resps []response.EngineResponse) map[string][]*report.Po
for _, info := range infos {
var appname string
-
- ns := info.Resource.GetNamespace()
+ ns := info.Namespace
if ns != "" {
appname = fmt.Sprintf("policyreport-ns-%s", ns)
} else {
appname = fmt.Sprintf(clusterpolicyreport)
}
- for _, rule := range info.Rules {
- result := report.PolicyReportResult{
- Policy: info.PolicyName,
- Resources: []*corev1.ObjectReference{
- {
- Kind: info.Resource.GetKind(),
- Namespace: info.Resource.GetNamespace(),
- APIVersion: info.Resource.GetAPIVersion(),
- Name: info.Resource.GetName(),
- UID: info.Resource.GetUID(),
- },
- },
- Scored: true,
- }
+ for _, infoResult := range info.Results {
+ for _, rule := range infoResult.Rules {
+ if rule.Type != utils.Validation.String() {
+ continue
+ }
- result.Rule = rule.Name
- result.Message = rule.Message
- result.Status = report.PolicyStatus(rule.Check)
- results[appname] = append(results[appname], &result)
+ result := report.PolicyReportResult{
+ Policy: info.PolicyName,
+ Resources: []*corev1.ObjectReference{
+ {
+ Kind: infoResult.Resource.Kind,
+ Namespace: infoResult.Resource.Namespace,
+ APIVersion: infoResult.Resource.APIVersion,
+ Name: infoResult.Resource.Name,
+ UID: types.UID(infoResult.Resource.UID),
+ },
+ },
+ Scored: true,
+ }
+
+ result.Rule = rule.Name
+ result.Message = rule.Message
+ result.Status = report.PolicyStatus(rule.Check)
+ results[appname] = append(results[appname], &result)
+ }
}
}
diff --git a/pkg/kyverno/apply/report_test.go b/pkg/kyverno/apply/report_test.go
index a82676ee45..5e357adfc1 100644
--- a/pkg/kyverno/apply/report_test.go
+++ b/pkg/kyverno/apply/report_test.go
@@ -8,6 +8,7 @@ import (
report "github.com/kyverno/kyverno/pkg/api/policyreport/v1alpha1"
"github.com/kyverno/kyverno/pkg/common"
"github.com/kyverno/kyverno/pkg/engine/response"
+ "github.com/kyverno/kyverno/pkg/engine/utils"
"gotest.tools/assert"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -30,10 +31,12 @@ var engineResponses = []response.EngineResponse{
Rules: []response.RuleResponse{
{
Name: "policy1-rule1",
+ Type: utils.Validation.String(),
Success: true,
},
{
Name: "policy1-rule2",
+ Type: utils.Validation.String(),
Success: false,
},
},
@@ -54,10 +57,12 @@ var engineResponses = []response.EngineResponse{
Rules: []response.RuleResponse{
{
Name: "clusterpolicy2-rule1",
+ Type: utils.Validation.String(),
Success: true,
},
{
Name: "clusterpolicy2-rule2",
+ Type: utils.Validation.String(),
Success: false,
},
},
diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go
index 2b2f9a50a4..5281b2295a 100644
--- a/pkg/policy/apply.go
+++ b/pkg/policy/apply.go
@@ -19,7 +19,6 @@ import (
)
// applyPolicy applies policy on a resource
-//TODO: generation rules
func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, logger logr.Logger, excludeGroupRole []string, resCache resourcecache.ResourceCacheIface) (responses []response.EngineResponse) {
startTime := time.Now()
defer func() {
@@ -35,23 +34,21 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure
var engineResponses []response.EngineResponse
var engineResponseMutation, engineResponseValidation response.EngineResponse
var err error
- // build context
+
ctx := context.NewContext()
err = ctx.AddResource(transformResource(resource))
if err != nil {
logger.Error(err, "enable to add transform resource to ctx")
}
- //MUTATION
+
engineResponseMutation, err = mutation(policy, resource, ctx, logger, resCache, ctx)
if err != nil {
logger.Error(err, "failed to process mutation rule")
}
- //VALIDATION
engineResponseValidation = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource, ExcludeGroupRole: excludeGroupRole, ResourceCache: resCache, JSONContext: ctx})
engineResponses = append(engineResponses, mergeRuleRespose(engineResponseMutation, engineResponseValidation))
- //TODO: GENERATION
return engineResponses
}
@@ -62,10 +59,10 @@ func mutation(policy kyverno.ClusterPolicy, resource unstructured.Unstructured,
log.V(4).Info("failed to apply mutation rules; reporting them")
return engineResponse, nil
}
- // Verify if the JSON pathes returned by the Mutate are already applied to the resource
+ // Verify if the JSON patches returned by the Mutate are already applied to the resource
if reflect.DeepEqual(resource, engineResponse.PatchedResource) {
// resources matches
- log.V(4).Info("resource already satisfys the policy")
+ log.V(4).Info("resource already satisfies the policy")
return engineResponse, nil
}
return getFailedOverallRuleInfo(resource, engineResponse, log)
diff --git a/pkg/policy/common.go b/pkg/policy/common.go
index 2b5c83dca6..ba9d2fae1c 100644
--- a/pkg/policy/common.go
+++ b/pkg/policy/common.go
@@ -8,7 +8,6 @@ import (
"github.com/go-logr/logr"
kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
"github.com/kyverno/kyverno/pkg/config"
- client "github.com/kyverno/kyverno/pkg/dclient"
"github.com/kyverno/kyverno/pkg/utils"
"github.com/minio/minio/pkg/wildcard"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -92,7 +91,7 @@ func ExcludePod(resourceMap map[string]unstructured.Unstructured, log logr.Logge
return resourceMap
}
-// GetNamespacesForRule gets the matched namespacse list for the given rule
+// GetNamespacesForRule gets the matched namespaces list for the given rule
func GetNamespacesForRule(rule *kyverno.Rule, nslister listerv1.NamespaceLister, log logr.Logger) []string {
if len(rule.MatchResources.Namespaces) == 0 {
return GetAllNamespaces(nslister, log)
@@ -160,7 +159,7 @@ func GetAllNamespaces(nslister listerv1.NamespaceLister, log logr.Logger) []stri
}
// GetResourcesPerNamespace ...
-func GetResourcesPerNamespace(kind string, client *client.Client, namespace string, rule kyverno.Rule, configHandler config.Interface, log logr.Logger) map[string]unstructured.Unstructured {
+func (pc *PolicyController) getResourcesPerNamespace(kind string, namespace string, rule kyverno.Rule, log logr.Logger) map[string]unstructured.Unstructured {
resourceMap := map[string]unstructured.Unstructured{}
ls := rule.MatchResources.Selector
@@ -168,7 +167,7 @@ func GetResourcesPerNamespace(kind string, client *client.Client, namespace stri
namespace = ""
}
- list, err := client.ListResource("", kind, namespace, ls)
+ list, err := pc.client.ListResource("", kind, namespace, ls)
if err != nil {
log.Error(err, "failed to list resources", "kind", kind, "namespace", namespace)
return nil
@@ -192,7 +191,7 @@ func GetResourcesPerNamespace(kind string, client *client.Client, namespace stri
}
}
// Skip the filtered resources
- if configHandler.ToFilter(r.GetKind(), r.GetNamespace(), r.GetName()) {
+ if pc.configHandler.ToFilter(r.GetKind(), r.GetNamespace(), r.GetName()) {
continue
}
@@ -202,12 +201,12 @@ func GetResourcesPerNamespace(kind string, client *client.Client, namespace stri
// exclude the resources
// skip resources to be filtered
- ExcludeResources(resourceMap, rule.ExcludeResources.ResourceDescription, configHandler, log)
+ excludeResources(resourceMap, rule.ExcludeResources.ResourceDescription, pc.configHandler, log)
return resourceMap
}
// ExcludeResources ...
-func ExcludeResources(included map[string]unstructured.Unstructured, exclude kyverno.ResourceDescription, configHandler config.Interface, log logr.Logger) {
+func excludeResources(included map[string]unstructured.Unstructured, exclude kyverno.ResourceDescription, configHandler config.Interface, log logr.Logger) {
if reflect.DeepEqual(exclude, (kyverno.ResourceDescription{})) {
return
}
@@ -270,7 +269,7 @@ func ExcludeResources(included map[string]unstructured.Unstructured, exclude kyv
// check exclude condition for each resource
for uid, resource := range included {
- // 0 -> dont check
+ // 0 -> don't check
// 1 -> is not to be exclude
// 2 -> to be exclude
excludeEval := []Condition{}
diff --git a/pkg/policy/existing.go b/pkg/policy/existing.go
index a4aab74fd1..4437208025 100644
--- a/pkg/policy/existing.go
+++ b/pkg/policy/existing.go
@@ -1,6 +1,7 @@
package policy
import (
+ "errors"
"sync"
"time"
@@ -11,72 +12,85 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
-func (pc *PolicyController) processExistingResources(policy *kyverno.ClusterPolicy) []response.EngineResponse {
+func (pc *PolicyController) processExistingResources(policy *kyverno.ClusterPolicy) {
logger := pc.log.WithValues("policy", policy.Name)
- // Parse through all the resources
- // drops the cache after configured rebuild time
+ logger.V(4).Info("applying policy to existing resources")
+
+ // Parse through all the resources drops the cache after configured rebuild time
pc.rm.Drop()
- var engineResponses []response.EngineResponse
- // get resource that are satisfy the resource description defined in the rules
- resourceMap := pc.listResources(policy)
- for _, resource := range resourceMap {
- // pre-processing, check if the policy and resource version has been processed before
- if !pc.rm.ProcessResource(policy.Name, policy.ResourceVersion, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resource.GetResourceVersion()) {
- logger.V(4).Info("policy and resource already processed", "policyResourceVersion", policy.ResourceVersion, "resourceResourceVersion", resource.GetResourceVersion(), "kind", resource.GetKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
+
+ for _, rule := range policy.Spec.Rules {
+ if !rule.HasValidate() {
continue
}
- // apply the policy on each
- engineResponse := applyPolicy(*policy, resource, logger, pc.configHandler.GetExcludeGroupRole(), pc.resCache)
- // get engine response for mutation & validation independently
- engineResponses = append(engineResponses, engineResponse...)
- // post-processing, register the resource as processed
- pc.rm.RegisterResource(policy.GetName(), policy.GetResourceVersion(), resource.GetKind(), resource.GetNamespace(), resource.GetName(), resource.GetResourceVersion())
- }
- return engineResponses
-}
-
-func (pc *PolicyController) listResources(policy *kyverno.ClusterPolicy) map[string]unstructured.Unstructured {
- pc.log.V(4).Info("list resources to be processed")
-
- // key uid
- resourceMap := map[string]unstructured.Unstructured{}
-
- for _, rule := range policy.Spec.Rules {
for _, k := range rule.MatchResources.Kinds {
-
- resourceSchema, _, err := pc.client.DiscoveryClient.FindResource("", k)
+ logger = logger.WithValues("rule", rule.Name, "kind", k)
+ namespaced, err := pc.rm.GetScope(k)
if err != nil {
- pc.log.Error(err, "failed to find resource", "kind", k)
+ resourceSchema, _, err := pc.client.DiscoveryClient.FindResource("", k)
+ if err != nil {
+ logger.Error(err, "failed to find resource", "kind", k)
+ continue
+ }
+
+ namespaced = resourceSchema.Namespaced
+ pc.rm.RegisterScope(k, namespaced)
+ }
+
+ if !namespaced {
+ pc.applyAndReportPerNamespace(policy, k, "", rule, logger)
continue
}
- if !resourceSchema.Namespaced {
- rMap := GetResourcesPerNamespace(k, pc.client, "", rule, pc.configHandler, pc.log)
- MergeResources(resourceMap, rMap)
- } else {
- namespaces := GetNamespacesForRule(&rule, pc.nsLister, pc.log)
- for _, ns := range namespaces {
- rMap := GetResourcesPerNamespace(k, pc.client, ns, rule, pc.configHandler, pc.log)
- MergeResources(resourceMap, rMap)
- }
+ namespaces := GetNamespacesForRule(&rule, pc.nsLister, logger)
+ for _, ns := range namespaces {
+ pc.applyAndReportPerNamespace(policy, k, ns, rule, logger)
}
}
}
+}
- return excludeAutoGenResources(*policy, resourceMap, pc.log)
+func (pc *PolicyController) applyAndReportPerNamespace(policy *kyverno.ClusterPolicy, kind string, ns string, rule kyverno.Rule, logger logr.Logger) {
+ rMap := pc.getResourcesPerNamespace(kind, ns, rule, logger)
+ excludeAutoGenResources(*policy, rMap, logger)
+
+ if len(rMap) == 0 {
+ return
+ }
+
+ var engineResponses []response.EngineResponse
+ for _, resource := range rMap {
+ responses := pc.applyPolicy(policy, resource, logger)
+ engineResponses = append(engineResponses, responses...)
+ }
+
+ pc.report(policy.Name, engineResponses, logger)
+}
+
+func (pc *PolicyController) applyPolicy(policy *kyverno.ClusterPolicy, resource unstructured.Unstructured, logger logr.Logger) (engineResponses []response.EngineResponse) {
+ // pre-processing, check if the policy and resource version has been processed before
+ if !pc.rm.ProcessResource(policy.Name, policy.ResourceVersion, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resource.GetResourceVersion()) {
+ logger.V(4).Info("policy and resource already processed", "policyResourceVersion", policy.ResourceVersion, "resourceResourceVersion", resource.GetResourceVersion(), "kind", resource.GetKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
+ }
+
+ engineResponse := applyPolicy(*policy, resource, logger, pc.configHandler.GetExcludeGroupRole(), pc.resCache)
+ engineResponses = append(engineResponses, engineResponse...)
+
+ // post-processing, register the resource as processed
+ pc.rm.RegisterResource(policy.GetName(), policy.GetResourceVersion(), resource.GetKind(), resource.GetNamespace(), resource.GetName(), resource.GetResourceVersion())
+
+ return
}
// excludeAutoGenResources filter out the pods / jobs with ownerReference
-func excludeAutoGenResources(policy kyverno.ClusterPolicy, resourceMap map[string]unstructured.Unstructured, log logr.Logger) map[string]unstructured.Unstructured {
+func excludeAutoGenResources(policy kyverno.ClusterPolicy, resourceMap map[string]unstructured.Unstructured, log logr.Logger) {
for uid, r := range resourceMap {
if engine.SkipPolicyApplication(policy, r) {
log.V(4).Info("exclude resource", "namespace", r.GetNamespace(), "kind", r.GetKind(), "name", r.GetName())
delete(resourceMap, uid)
}
}
-
- return resourceMap
}
//Condition defines condition type
@@ -91,16 +105,10 @@ const (
Skip Condition = 2
)
-// merge b into a map
-func mergeResources(a, b map[string]unstructured.Unstructured) {
- for k, v := range b {
- a[k] = v
- }
-}
-
//NewResourceManager returns a new ResourceManager
func NewResourceManager(rebuildTime int64) *ResourceManager {
rm := ResourceManager{
+ scope: make(map[string]bool),
data: make(map[string]interface{}),
time: time.Now(),
rebuildTime: rebuildTime,
@@ -113,6 +121,7 @@ func NewResourceManager(rebuildTime int64) *ResourceManager {
type ResourceManager struct {
// we drop and re-build the cache
// based on the memory consumer of by the map
+ scope map[string]bool
data map[string]interface{}
mux sync.RWMutex
time time.Time
@@ -123,7 +132,8 @@ type resourceManager interface {
ProcessResource(policy, pv, kind, ns, name, rv string) bool
//TODO removeResource(kind, ns, name string) error
RegisterResource(policy, pv, kind, ns, name, rv string)
- // reload
+ RegisterScope(kind string, namespaced bool)
+ GetScope(kind string) (bool, error)
Drop()
}
@@ -160,6 +170,28 @@ func (rm *ResourceManager) ProcessResource(policy, pv, kind, ns, name, rv string
return !ok
}
+// RegisterScope stores the scope of the given kind
+func (rm *ResourceManager) RegisterScope(kind string, namespaced bool) {
+ rm.mux.Lock()
+ defer rm.mux.Unlock()
+
+ rm.scope[kind] = namespaced
+}
+
+// GetScope gets the scope of the given kind
+// return error if kind is not registered
+func (rm *ResourceManager) GetScope(kind string) (bool, error) {
+ rm.mux.RLock()
+ defer rm.mux.RUnlock()
+
+ namespaced, ok := rm.scope[kind]
+ if !ok {
+ return false, errors.New("NotFound")
+ }
+
+ return namespaced, nil
+}
+
func buildKey(policy, pv, kind, ns, name, rv string) string {
return policy + "/" + pv + "/" + kind + "/" + ns + "/" + name + "/" + rv
}
diff --git a/pkg/policy/report.go b/pkg/policy/report.go
index ed5ef74ba6..1680511506 100644
--- a/pkg/policy/report.go
+++ b/pkg/policy/report.go
@@ -9,21 +9,17 @@ import (
"github.com/kyverno/kyverno/pkg/policyreport"
)
-// for each policy-resource response
-// - has violation -> report
-// - no violation -> cleanup policy violations
-func (pc *PolicyController) cleanupAndReport(engineResponses []response.EngineResponse) {
- logger := pc.log
- // generate Events
- eventInfos := generateEvents(pc.log, engineResponses)
+func (pc *PolicyController) report(policy string, engineResponses []response.EngineResponse, logger logr.Logger) {
+ eventInfos := generateEvents(logger, engineResponses)
pc.eventGen.Add(eventInfos...)
- // create policy violation
- pvInfos := policyreport.GeneratePRsFromEngineResponse(engineResponses, logger)
- for i := range pvInfos {
- pvInfos[i].FromSync = true
- }
- pc.prGenerator.Add(pvInfos...)
+ pvInfos := policyreport.GeneratePRsFromEngineResponse(engineResponses, logger)
+
+ // as engineResponses holds the results for all matched resources in one namespace
+ // we can merge pvInfos into a single object to reduce update frequency (throttling request) on RCR
+ info := mergePvInfos(pvInfos)
+ pc.prGenerator.Add(info)
+ logger.V(4).Info("added a request to RCR generator", "key", info.ToKey())
}
func generateEvents(log logr.Logger, ers []response.EngineResponse) []event.Info {
@@ -61,3 +57,22 @@ func generateEventsPerEr(log logr.Logger, er response.EngineResponse) []event.In
return eventInfos
}
+
+func mergePvInfos(infos []policyreport.Info) policyreport.Info {
+ aggregatedInfo := policyreport.Info{}
+ if len(infos) == 0 {
+ return aggregatedInfo
+ }
+
+ var results []policyreport.EngineResponseResult
+ for _, info := range infos {
+ for _, res := range info.Results {
+ results = append(results, res)
+ }
+ }
+
+ aggregatedInfo.PolicyName = infos[0].PolicyName
+ aggregatedInfo.Namespace = infos[0].Namespace
+ aggregatedInfo.Results = results
+ return aggregatedInfo
+}
diff --git a/pkg/policy/validate_controller.go b/pkg/policy/validate_controller.go
index 25091b0e61..c12e943e50 100644
--- a/pkg/policy/validate_controller.go
+++ b/pkg/policy/validate_controller.go
@@ -50,7 +50,7 @@ type PolicyController struct {
eventGen event.Interface
eventRecorder record.EventRecorder
- // Policys that need to be synced
+ // Policies that need to be synced
queue workqueue.RateLimitingInterface
// pLister can list/get policy from the shared informer's store
@@ -62,7 +62,7 @@ type PolicyController struct {
// grLister can list/get generate request from the shared informer's store
grLister kyvernolister.GenerateRequestLister
- // nsLister can list/get namespacecs from the shared informer's store
+ // nsLister can list/get namespaces from the shared informer's store
nsLister listerv1.NamespaceLister
// pListerSynced returns true if the cluster policy store has been synced at least once
@@ -82,6 +82,7 @@ type PolicyController struct {
// grListerSynced returns true if the generate request store has been synced at least once
grListerSynced cache.InformerSynced
+
// Resource manager, manages the mapping for already processed resource
rm resourceManager
@@ -203,7 +204,7 @@ func (pc *PolicyController) updatePolicy(old, cur interface{}) {
logger.V(4).Info("updating policy", "name", oldP.Name)
- pc.enqueueDeletedRule(oldP, curP)
+ pc.enqueueRCRDeletedRule(oldP, curP)
pc.enqueuePolicy(curP)
}
@@ -213,7 +214,7 @@ func (pc *PolicyController) deletePolicy(obj interface{}) {
if !ok {
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
- logger.Info("couldnt get object from tomstone", "obj", obj)
+ logger.Info("couldn't get object from tombstone", "obj", obj)
return
}
@@ -226,9 +227,10 @@ func (pc *PolicyController) deletePolicy(obj interface{}) {
logger.V(4).Info("deleting policy", "name", p.Name)
- // we process policies that are not set of background processing as we need to perform policy violation
- // cleanup when a policy is deleted.
+ // we process policies that are not set of background processing
+ // as we need to clean up GRs when a policy is deleted
pc.enqueuePolicy(p)
+ pc.enqueueRCRDeletedPolicy(p.Name)
}
func (pc *PolicyController) addNsPolicy(obj interface{}) {
@@ -257,7 +259,7 @@ func (pc *PolicyController) updateNsPolicy(old, cur interface{}) {
logger.V(4).Info("updating namespace policy", "namespace", oldP.Namespace, "name", oldP.Name)
- pc.enqueueDeletedRule(ConvertPolicyToClusterPolicy(oldP), ncurP)
+ pc.enqueueRCRDeletedRule(ConvertPolicyToClusterPolicy(oldP), ncurP)
pc.enqueuePolicy(ncurP)
}
@@ -267,7 +269,7 @@ func (pc *PolicyController) deleteNsPolicy(obj interface{}) {
if !ok {
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
- logger.Info("couldnt get object from tomstone", "obj", obj)
+ logger.Info("couldn't get object from tombstone", "obj", obj)
return
}
@@ -280,12 +282,13 @@ func (pc *PolicyController) deleteNsPolicy(obj interface{}) {
pol := ConvertPolicyToClusterPolicy(p)
logger.V(4).Info("deleting namespace policy", "namespace", pol.Namespace, "name", pol.Name)
- // we process policies that are not set of background processing as we need to perform policy violation
- // cleanup when a policy is deleted.
+ // we process policies that are not set of background processing
+ // as we need to clean up GRs when a policy is deleted
pc.enqueuePolicy(pol)
+ pc.enqueueRCRDeletedPolicy(p.Name)
}
-func (pc *PolicyController) enqueueDeletedRule(old, cur *kyverno.ClusterPolicy) {
+func (pc *PolicyController) enqueueRCRDeletedRule(old, cur *kyverno.ClusterPolicy) {
curRule := make(map[string]bool)
for _, rule := range cur.Spec.Rules {
curRule[rule.Name] = true
@@ -295,14 +298,24 @@ func (pc *PolicyController) enqueueDeletedRule(old, cur *kyverno.ClusterPolicy)
if !curRule[rule.Name] {
pc.prGenerator.Add(policyreport.Info{
PolicyName: cur.GetName(),
- Rules: []kyverno.ViolatedRule{
- {Name: rule.Name},
+ Results: []policyreport.EngineResponseResult{
+ {
+ Rules: []kyverno.ViolatedRule{
+ {Name: rule.Name},
+ },
+ },
},
})
}
}
}
+func (pc *PolicyController) enqueueRCRDeletedPolicy(policyName string) {
+ pc.prGenerator.Add(policyreport.Info{
+ PolicyName: policyName,
+ })
+}
+
func (pc *PolicyController) enqueuePolicy(policy *kyverno.ClusterPolicy) {
logger := pc.log
key, err := cache.MetaNamespaceKeyFunc(policy)
@@ -372,7 +385,7 @@ func (pc *PolicyController) handleErr(err error, key interface{}) {
}
func (pc *PolicyController) syncPolicy(key string) error {
- logger := pc.log
+ logger := pc.log.WithName("syncPolicy")
startTime := time.Now()
logger.V(4).Info("started syncing policy", "key", key, "startTime", startTime)
defer func() {
@@ -384,55 +397,57 @@ func (pc *PolicyController) syncPolicy(key string) error {
logger.Error(err, "failed to list generate request")
}
- var policy *kyverno.ClusterPolicy
- namespace, key, isNamespacedPolicy := parseNamespacedPolicy(key)
- if !isNamespacedPolicy {
- policy, err = pc.pLister.Get(key)
- } else {
- var nspolicy *kyverno.Policy
- nspolicy, err = pc.npLister.Policies(namespace).Get(key)
- if err == nil && nspolicy != nil {
- policy = ConvertPolicyToClusterPolicy(nspolicy)
- }
- }
-
+ policy, err := pc.getPolicy(key)
if err != nil {
if errors.IsNotFound(err) {
- for _, v := range grList {
- if key == v.Spec.Policy {
- err := pc.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Delete(context.TODO(), v.GetName(), metav1.DeleteOptions{})
- if err != nil && !errors.IsNotFound(err) {
- logger.Error(err, "failed to delete gr")
- }
- }
- }
-
- go pc.removeResultsEntryFromPolicyReport(key)
+ deleteGR(pc.kyvernoClient, key, grList, logger)
return nil
}
return err
}
+ updateGR(pc.kyvernoClient, policy.Name, grList, logger)
+ pc.processExistingResources(policy)
+ return nil
+}
+
+func (pc *PolicyController) getPolicy(key string) (policy *kyverno.ClusterPolicy, err error) {
+ namespace, key, isNamespacedPolicy := parseNamespacedPolicy(key)
+ if !isNamespacedPolicy {
+ return pc.pLister.Get(key)
+ }
+
+ nsPolicy, err := pc.npLister.Policies(namespace).Get(key)
+ if err == nil && nsPolicy != nil {
+ policy = ConvertPolicyToClusterPolicy(nsPolicy)
+ }
+
+ return
+}
+
+func deleteGR(kyvernoClient *kyvernoclient.Clientset, policyKey string, grList []*kyverno.GenerateRequest, logger logr.Logger) {
for _, v := range grList {
- if policy.Name == v.Spec.Policy {
+ if policyKey == v.Spec.Policy {
+ err := kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Delete(context.TODO(), v.GetName(), metav1.DeleteOptions{})
+ if err != nil && !errors.IsNotFound(err) {
+ logger.Error(err, "failed to delete gr", "name", v.GetName())
+ }
+ }
+ }
+}
+
+func updateGR(kyvernoClient *kyvernoclient.Clientset, policyKey string, grList []*kyverno.GenerateRequest, logger logr.Logger) {
+ for _, v := range grList {
+ if policyKey == v.Spec.Policy {
v.SetLabels(map[string]string{
"policy-update": fmt.Sprintf("revision-count-%d", rand.Intn(100000)),
})
- _, err := pc.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Update(context.TODO(), v, metav1.UpdateOptions{})
+ _, err := kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Update(context.TODO(), v, metav1.UpdateOptions{})
if err != nil {
- logger.Error(err, "failed to update gr", "policy", policy.GetName(), "gr", v.GetName())
+ logger.Error(err, "failed to update gr", "name", v.GetName())
}
}
}
- engineResponses := pc.processExistingResources(policy)
- pc.cleanupAndReport(engineResponses)
- return nil
-}
-
-func (pc *PolicyController) removeResultsEntryFromPolicyReport(policyName string) {
- pc.prGenerator.Add(policyreport.Info{
- PolicyName: policyName,
- })
}
diff --git a/pkg/policyreport/builder.go b/pkg/policyreport/builder.go
index 82fc74700d..062e57ac4f 100755
--- a/pkg/policyreport/builder.go
+++ b/pkg/policyreport/builder.go
@@ -15,15 +15,13 @@ import (
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/types"
)
const (
clusterreportchangerequest string = "clusterreportchangerequest"
- resourceLabelName string = "kyverno.io/resource.name"
- resourceLabelKind string = "kyverno.io/resource.kind"
resourceLabelNamespace string = "kyverno.io/resource.namespace"
- policyLabel string = "kyverno.io/policy"
- deletedLabelResource string = "kyverno.io/delete.resource"
+ deletedLabelResource string = "kyverno.io/delete.resource.name"
deletedLabelResourceKind string = "kyverno.io/delete.resource.kind"
deletedLabelPolicy string = "kyverno.io/delete.policy"
deletedLabelRule string = "kyverno.io/delete.rule"
@@ -76,33 +74,18 @@ func NewBuilder(cpolLister kyvernolister.ClusterPolicyLister, polLister kyvernol
func (builder *requestBuilder) build(info Info) (req *unstructured.Unstructured, err error) {
results := []*report.PolicyReportResult{}
- for _, rule := range info.Rules {
- if rule.Type != utils.Validation.String() {
- continue
- }
+ for _, infoResult := range info.Results {
+ for _, rule := range infoResult.Rules {
+ if rule.Type != utils.Validation.String() {
+ continue
+ }
- result := &report.PolicyReportResult{
- Policy: info.PolicyName,
- Resources: []*v1.ObjectReference{
- {
- Kind: info.Resource.GetKind(),
- Namespace: info.Resource.GetNamespace(),
- APIVersion: info.Resource.GetAPIVersion(),
- Name: info.Resource.GetName(),
- UID: info.Resource.GetUID(),
- },
- },
- Scored: true,
- Category: builder.fetchCategory(info.PolicyName, info.Resource.GetNamespace()),
+ result := builder.buildRCRResult(info.PolicyName, infoResult.Resource, rule)
+ results = append(results, result)
}
-
- result.Rule = rule.Name
- result.Message = rule.Message
- result.Status = report.PolicyStatus(rule.Check)
- results = append(results, result)
}
- if info.Resource.GetNamespace() != "" {
+ if info.Namespace != "" {
rr := &request.ReportChangeRequest{
Summary: calculateSummary(results),
Results: results,
@@ -129,58 +112,84 @@ func (builder *requestBuilder) build(info Info) (req *unstructured.Unstructured,
set(req, info)
}
- // deletion of a result entry
- if len(info.Rules) == 0 && info.PolicyName == "" { // on resource deleteion
- req.SetLabels(map[string]string{
- resourceLabelNamespace: info.Resource.GetNamespace(),
- deletedLabelResource: info.Resource.GetName(),
- deletedLabelResourceKind: info.Resource.GetKind()})
- } else if info.PolicyName != "" && reflect.DeepEqual(info.Resource, unstructured.Unstructured{}) { // on policy deleteion
- req.SetKind("ReportChangeRequest")
-
- if len(info.Rules) == 0 {
- req.SetLabels(map[string]string{
- deletedLabelPolicy: info.PolicyName})
-
- req.SetName(fmt.Sprintf("reportchangerequest-%s", info.PolicyName))
- } else {
- req.SetLabels(map[string]string{
- deletedLabelPolicy: info.PolicyName,
- deletedLabelRule: info.Rules[0].Name})
- req.SetName(fmt.Sprintf("reportchangerequest-%s-%s", info.PolicyName, info.Rules[0].Name))
+ if !setRequestLabels(req, info) {
+ if len(results) == 0 {
+ // return nil on empty result without a deletion
+ return nil, nil
}
- } else if len(results) == 0 {
- // return nil on empty result without a deletion
- return nil, nil
}
return req, nil
}
+func (builder *requestBuilder) buildRCRResult(policy string, resource response.ResourceSpec, rule kyverno.ViolatedRule) *report.PolicyReportResult {
+ result := &report.PolicyReportResult{
+ Policy: policy,
+ Resources: []*v1.ObjectReference{
+ {
+ Kind: resource.Kind,
+ Namespace: resource.Namespace,
+ APIVersion: resource.APIVersion,
+ Name: resource.Name,
+ UID: types.UID(resource.UID),
+ },
+ },
+ Scored: true,
+ Category: builder.fetchCategory(policy, resource.Namespace),
+ }
+
+ result.Rule = rule.Name
+ result.Message = rule.Message
+ result.Status = report.PolicyStatus(rule.Check)
+ return result
+}
+
func set(obj *unstructured.Unstructured, info Info) {
- resource := info.Resource
- obj.SetNamespace(config.KyvernoNamespace)
obj.SetAPIVersion(request.SchemeGroupVersion.Group + "/" + request.SchemeGroupVersion.Version)
- if resource.GetNamespace() == "" {
+
+ if info.Namespace == "" {
obj.SetGenerateName(clusterreportchangerequest + "-")
obj.SetKind("ClusterReportChangeRequest")
} else {
- obj.SetGenerateName("reportchangerequest-")
+ obj.SetGenerateName("rcr-")
obj.SetKind("ReportChangeRequest")
+ obj.SetNamespace(config.KyvernoNamespace)
}
obj.SetLabels(map[string]string{
- resourceLabelNamespace: resource.GetNamespace(),
- resourceLabelName: resource.GetName(),
- resourceLabelKind: resource.GetKind(),
- policyLabel: info.PolicyName,
+ resourceLabelNamespace: info.Namespace,
})
+}
- if info.FromSync {
- obj.SetAnnotations(map[string]string{
- "fromSync": "true",
+func setRequestLabels(req *unstructured.Unstructured, info Info) bool {
+ switch {
+ case isResourceDeletion(info):
+ req.SetLabels(map[string]string{
+ resourceLabelNamespace: info.Results[0].Resource.Namespace,
+ deletedLabelResource: info.Results[0].Resource.Name,
+ deletedLabelResourceKind: info.Results[0].Resource.Kind,
})
+ return true
+
+ case isPolicyDeletion(info):
+ req.SetKind("ReportChangeRequest")
+ req.SetGenerateName("rcr-")
+ req.SetLabels(map[string]string{
+ deletedLabelPolicy: info.PolicyName},
+ )
+ return true
+
+ case isRuleDeletion(info):
+ req.SetKind("ReportChangeRequest")
+ req.SetGenerateName("rcr-")
+ req.SetLabels(map[string]string{
+ deletedLabelPolicy: info.PolicyName,
+ deletedLabelRule: info.Results[0].Rules[0].Name},
+ )
+ return true
}
+
+ return false
}
func calculateSummary(results []*report.PolicyReportResult) (summary report.PolicyReportSummary) {
@@ -204,8 +213,13 @@ func calculateSummary(results []*report.PolicyReportResult) (summary report.Poli
func buildPVInfo(er response.EngineResponse) Info {
info := Info{
PolicyName: er.PolicyResponse.Policy,
- Resource: er.PatchedResource,
- Rules: buildViolatedRules(er),
+ Namespace: er.PatchedResource.GetNamespace(),
+ Results: []EngineResponseResult{
+ {
+ Resource: er.GetResourceSpec(),
+ Rules: buildViolatedRules(er),
+ },
+ },
}
return info
}
@@ -246,3 +260,21 @@ func (builder *requestBuilder) fetchCategory(policy, ns string) string {
return ""
}
+
+func isResourceDeletion(info Info) bool {
+ return info.PolicyName == "" && len(info.Results) == 1 && info.GetRuleLength() == 0
+}
+
+func isPolicyDeletion(info Info) bool {
+ return info.PolicyName != "" && len(info.Results) == 0
+}
+
+func isRuleDeletion(info Info) bool {
+ if info.PolicyName != "" && len(info.Results) == 1 {
+ result := info.Results[0]
+ if len(result.Rules) == 1 && reflect.DeepEqual(result.Resource, response.ResourceSpec{}) {
+ return true
+ }
+ }
+ return false
+}
diff --git a/pkg/policyreport/policyreport.go b/pkg/policyreport/policyreport.go
index 42afc981fe..7df457b3ba 100644
--- a/pkg/policyreport/policyreport.go
+++ b/pkg/policyreport/policyreport.go
@@ -15,27 +15,39 @@ type deletedResource struct {
kind, ns, name string
}
+func buildLabelForDeletedResource(labels map[string]string) *deletedResource {
+ ok := true
+ kind, kindOk := labels[deletedLabelResourceKind]
+ ok = ok && kindOk
+
+ name, nameOk := labels[deletedLabelResource]
+ ok = ok && nameOk
+
+ if !ok {
+ return nil
+ }
+
+ return &deletedResource{
+ kind: kind,
+ name: name,
+ ns: labels[resourceLabelNamespace],
+ }
+}
+
func getDeletedResources(aggregatedRequests interface{}) (resources []deletedResource) {
if requests, ok := aggregatedRequests.([]*changerequest.ClusterReportChangeRequest); ok {
for _, request := range requests {
- labels := request.GetLabels()
- dr := deletedResource{
- kind: labels[deletedLabelResourceKind],
- name: labels[deletedLabelResource],
- ns: labels[resourceLabelNamespace],
+ dr := buildLabelForDeletedResource(request.GetLabels())
+ if dr != nil {
+ resources = append(resources, *dr)
}
-
- resources = append(resources, dr)
}
} else if requests, ok := aggregatedRequests.([]*changerequest.ReportChangeRequest); ok {
for _, request := range requests {
- labels := request.GetLabels()
- dr := deletedResource{
- kind: labels[deletedLabelResourceKind],
- name: labels[deletedLabelResource],
- ns: labels[resourceLabelNamespace],
+ dr := buildLabelForDeletedResource(request.GetLabels())
+ if dr != nil {
+ resources = append(resources, *dr)
}
- resources = append(resources, dr)
}
}
return
@@ -115,9 +127,16 @@ func generateHashKey(result map[string]interface{}, dr deletedResource) (string,
resource := resources[0].(map[string]interface{})
if !reflect.DeepEqual(dr, deletedResource{}) {
- if resource["kind"] == dr.kind && resource["name"] == dr.name && resource["namespace"] == dr.ns {
- return "", false
+ if resource["kind"] == dr.kind {
+ if resource["name"] == dr.name && resource["namespace"] == dr.ns {
+ return "", false
+ }
+
+ if dr.kind == "Namespace" && resource["name"] == dr.name {
+ return "", false
+ }
}
+
}
return fmt.Sprintf(
diff --git a/pkg/policyreport/reportcontroller.go b/pkg/policyreport/reportcontroller.go
index 25a458be38..d6e923f346 100644
--- a/pkg/policyreport/reportcontroller.go
+++ b/pkg/policyreport/reportcontroller.go
@@ -232,6 +232,8 @@ func (g *ReportGenerator) handleErr(err error, key interface{}) {
// syncHandler reconciles clusterPolicyReport if namespace == ""
// otherwise it updates policyReport
func (g *ReportGenerator) syncHandler(key string) error {
+ g.log.V(4).Info("syncing policy report", "key", key)
+
if policy, rule, ok := isDeletedPolicyKey(key); ok {
return g.removePolicyEntryFromReport(policy, rule)
}
@@ -312,6 +314,31 @@ func (g *ReportGenerator) createReportIfNotPresent(namespace string, new *unstru
}
func (g *ReportGenerator) removePolicyEntryFromReport(policyName, ruleName string) error {
+ if err := g.removeFromClusterPolicyReport(policyName, ruleName); err != nil {
+ return err
+ }
+
+ if err := g.removeFromPolicyReport(policyName, ruleName); err != nil {
+ return err
+ }
+
+ labelset := labels.Set(map[string]string{deletedLabelPolicy: policyName})
+ if ruleName != "" {
+ labelset = labels.Set(map[string]string{
+ deletedLabelPolicy: policyName,
+ deletedLabelRule: ruleName,
+ })
+ }
+ aggregatedRequests, err := g.reportChangeRequestLister.ReportChangeRequests(config.KyvernoNamespace).List(labels.SelectorFromSet(labelset))
+ if err != nil {
+ return err
+ }
+
+ g.cleanupReportRequests(aggregatedRequests)
+ return nil
+}
+
+func (g *ReportGenerator) removeFromClusterPolicyReport(policyName, ruleName string) error {
cpolrs, err := g.clusterReportLister.List(labels.Everything())
if err != nil {
return fmt.Errorf("failed to list clusterPolicyReport %v", err)
@@ -335,7 +362,10 @@ func (g *ReportGenerator) removePolicyEntryFromReport(policyName, ruleName strin
return fmt.Errorf("failed to update clusterPolicyReport %s %v", cpolr.Name, err)
}
}
+ return nil
+}
+func (g *ReportGenerator) removeFromPolicyReport(policyName, ruleName string) error {
namespaces, err := g.dclient.ListResource("", "Namespace", "", nil)
if err != nil {
return fmt.Errorf("unable to list namespace %v", err)
@@ -370,23 +400,10 @@ func (g *ReportGenerator) removePolicyEntryFromReport(policyName, ruleName strin
return fmt.Errorf("failed to update PolicyReport %s %v", r.GetName(), err)
}
}
-
- labelset := labels.Set(map[string]string{deletedLabelPolicy: policyName})
- if ruleName != "" {
- labelset = labels.Set(map[string]string{
- deletedLabelPolicy: policyName,
- deletedLabelRule: ruleName,
- })
- }
- aggregatedRequests, err := g.reportChangeRequestLister.ReportChangeRequests(config.KyvernoNamespace).List(labels.SelectorFromSet(labelset))
- if err != nil {
- return err
- }
-
- g.cleanupReportRequests(aggregatedRequests)
return nil
}
+// aggregateReports aggregates cluster / report change requests to a policy report
func (g *ReportGenerator) aggregateReports(namespace string) (
report *unstructured.Unstructured, aggregatedRequests interface{}, err error) {
@@ -553,7 +570,7 @@ func (g *ReportGenerator) updateReport(old interface{}, new *unstructured.Unstru
new.Object = obj
if !hasResultsChanged(oldUnstructured, new.UnstructuredContent()) {
- g.log.V(4).Info("unchanged policy report", "namespace", new.GetNamespace(), "name", new.GetName())
+ g.log.V(4).Info("unchanged policy report", "kind", new.GetKind(), "namespace", new.GetNamespace(), "name", new.GetName())
return nil
}
diff --git a/pkg/policyreport/reportrequest.go b/pkg/policyreport/reportrequest.go
index acc185e91c..9021d0d683 100755
--- a/pkg/policyreport/reportrequest.go
+++ b/pkg/policyreport/reportrequest.go
@@ -19,6 +19,7 @@ import (
"github.com/kyverno/kyverno/pkg/constant"
client "github.com/kyverno/kyverno/pkg/dclient"
dclient "github.com/kyverno/kyverno/pkg/dclient"
+ "github.com/kyverno/kyverno/pkg/engine/response"
"github.com/kyverno/kyverno/pkg/policystatus"
apierrors "k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -61,11 +62,7 @@ type Generator struct {
queue workqueue.RateLimitingInterface
dataStore *dataStore
-
- // update policy status with violationCount
- policyStatusListener policystatus.Listener
-
- log logr.Logger
+ log logr.Logger
}
// NewReportChangeRequestGenerator returns a new instance of report request generator
@@ -89,7 +86,6 @@ func NewReportChangeRequestGenerator(client *policyreportclient.Clientset,
polListerSynced: polInformer.Informer().HasSynced,
queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName),
dataStore: newDataStore(),
- policyStatusListener: policyStatus,
log: log,
}
@@ -128,32 +124,47 @@ func (ds *dataStore) delete(keyHash string) {
delete(ds.data, keyHash)
}
-//Info is a request to create PV
+// Info stores the policy application results for all matched resources
+// Namespace is set to empty "" if resource is cluster wide resource
type Info struct {
PolicyName string
- Resource unstructured.Unstructured
- Rules []kyverno.ViolatedRule
- FromSync bool
+ Namespace string
+ Results []EngineResponseResult
}
-func (i Info) toKey() string {
+type EngineResponseResult struct {
+ Resource response.ResourceSpec
+ Rules []kyverno.ViolatedRule
+}
+
+func (i Info) ToKey() string {
keys := []string{
i.PolicyName,
- i.Resource.GetKind(),
- i.Resource.GetNamespace(),
- i.Resource.GetName(),
- strconv.Itoa(len(i.Rules)),
+ i.Namespace,
+ strconv.Itoa(len(i.Results)),
+ }
+
+ for _, result := range i.Results {
+ keys = append(keys, result.Resource.GetKey())
}
return strings.Join(keys, "/")
}
+func (i Info) GetRuleLength() int {
+ l := 0
+ for _, res := range i.Results {
+ l += len(res.Rules)
+ }
+ return l
+}
+
// GeneratorInterface provides API to create PVs
type GeneratorInterface interface {
Add(infos ...Info)
}
func (gen *Generator) enqueue(info Info) {
- keyHash := info.toKey()
+ keyHash := info.ToKey()
gen.dataStore.add(keyHash, info)
gen.queue.Add(keyHash)
}
@@ -231,7 +242,7 @@ func (gen *Generator) processNextWorkItem() bool {
info := gen.dataStore.lookup(keyHash)
if reflect.DeepEqual(info, Info{}) {
gen.queue.Forget(obj)
- logger.V(3).Info("empty key")
+ logger.V(4).Info("empty key")
return nil
}
@@ -249,48 +260,48 @@ func (gen *Generator) processNextWorkItem() bool {
func (gen *Generator) syncHandler(info Info) error {
gen.log.V(3).Info("generating report change request")
+
builder := NewBuilder(gen.cpolLister, gen.polLister)
- reportChangeRequestUnstructured, err := builder.build(info)
+ rcrUnstructured, err := builder.build(info)
if err != nil {
return fmt.Errorf("unable to build reportChangeRequest: %v", err)
}
- if reportChangeRequestUnstructured == nil {
+ if rcrUnstructured == nil {
return nil
}
- return gen.sync(reportChangeRequestUnstructured, info)
+ return gen.sync(rcrUnstructured, info)
}
func (gen *Generator) sync(reportReq *unstructured.Unstructured, info Info) error {
- defer func() {
- if val := reportReq.GetAnnotations()["fromSync"]; val == "true" {
- gen.policyStatusListener.Update(violationCount{
- policyName: info.PolicyName,
- violatedRules: info.Rules,
- })
- }
- }()
-
- logger := gen.log.WithName("sync")
+ logger := gen.log.WithName("sync report change request")
reportReq.SetCreationTimestamp(v1.Now())
- if reportReq.GetNamespace() == "" {
- old, err := gen.clusterReportChangeRequestLister.Get(reportReq.GetName())
- if err != nil {
- if apierrors.IsNotFound(err) {
- if _, err = gen.dclient.CreateResource(reportReq.GetAPIVersion(), reportReq.GetKind(), "", reportReq, false); err != nil {
- return fmt.Errorf("failed to create clusterReportChangeRequest: %v", err)
- }
-
- logger.V(3).Info("successfully created clusterReportChangeRequest", "name", reportReq.GetName())
- return nil
- }
- return fmt.Errorf("unable to get %s: %v", reportReq.GetKind(), err)
- }
-
- return updateReportChangeRequest(gen.dclient, old, reportReq, logger)
+ if reportReq.GetKind() == "ClusterReportChangeRequest" {
+ return gen.syncClusterReportChangeRequest(reportReq, logger)
}
+ return gen.syncReportChangeRequest(reportReq, logger)
+}
+
+func (gen *Generator) syncClusterReportChangeRequest(reportReq *unstructured.Unstructured, logger logr.Logger) error {
+ old, err := gen.clusterReportChangeRequestLister.Get(reportReq.GetName())
+ if err != nil {
+ if apierrors.IsNotFound(err) {
+ if _, err = gen.dclient.CreateResource(reportReq.GetAPIVersion(), reportReq.GetKind(), "", reportReq, false); err != nil {
+ return fmt.Errorf("failed to create clusterReportChangeRequest: %v", err)
+ }
+
+ logger.V(3).Info("successfully created clusterReportChangeRequest", "name", reportReq.GetName())
+ return nil
+ }
+ return fmt.Errorf("unable to get %s: %v", reportReq.GetKind(), err)
+ }
+
+ return updateReportChangeRequest(gen.dclient, old, reportReq, logger)
+}
+
+func (gen *Generator) syncReportChangeRequest(reportReq *unstructured.Unstructured, logger logr.Logger) error {
old, err := gen.reportChangeRequestLister.ReportChangeRequests(config.KyvernoNamespace).Get(reportReq.GetName())
if err != nil {
if apierrors.IsNotFound(err) {
diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go
index 727791e9d0..e91780d686 100644
--- a/pkg/webhooks/validation.go
+++ b/pkg/webhooks/validation.go
@@ -130,22 +130,27 @@ func HandleValidation(
prGenerator.Add(prInfos...)
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(),
- },
- },
- },
- })
+ prGenerator.Add(buildDeletionPrInfo(oldR))
}
return true, ""
}
+func buildDeletionPrInfo(oldR unstructured.Unstructured) policyreport.Info {
+ return policyreport.Info{
+ Namespace: oldR.GetNamespace(),
+ Results: []policyreport.EngineResponseResult{
+ {Resource: response.ResourceSpec{
+ Kind: oldR.GetKind(),
+ APIVersion: oldR.GetAPIVersion(),
+ Namespace: oldR.GetNamespace(),
+ Name: oldR.GetName(),
+ UID: string(oldR.GetUID()),
+ }},
+ },
+ }
+}
+
type validateStats struct {
resp response.EngineResponse
namespace string