From 6fc349716cf954177a75eaf20e8ddba7eed15576 Mon Sep 17 00:00:00 2001 From: shuting Date: Fri, 19 Feb 2021 09:09:41 -0800 Subject: [PATCH] Switch to use annotations to store resource info in cluster/reportChangeRequest (#1625) * skip sending API request for filtered resource * fix PR comment Signed-off-by: Shuting Zhao * fixes https://github.com/kyverno/kyverno/issues/1490 Signed-off-by: Shuting Zhao * fix bug - namespace is not returned properly Signed-off-by: Shuting Zhao * reduce throttling - list resource using lister * refactor resource cache * fix test Signed-off-by: Shuting Zhao * fix label selector Signed-off-by: Shuting Zhao * fix build failure Signed-off-by: Shuting Zhao * fixes #1480 * store resource name and kind in (c)rcr's annotation --- .../crds/kyverno.io_clusterpolicies.yaml | 79 ++++++++++--------- definitions/crds/kyverno.io_policies.yaml | 79 ++++++++++--------- pkg/api/kyverno/v1/policy_types.go | 3 +- pkg/policy/background.go | 2 +- pkg/policy/validate.go | 5 ++ pkg/policy/validate/validate.go | 2 +- pkg/policymutation/cronjob.go | 12 ++- pkg/policymutation/policymutation.go | 15 ++-- pkg/policyreport/builder.go | 23 ++++-- pkg/policyreport/changerequestcreator.go | 11 ++- pkg/policyreport/policyreport.go | 10 +-- 11 files changed, 139 insertions(+), 102 deletions(-) diff --git a/definitions/crds/kyverno.io_clusterpolicies.yaml b/definitions/crds/kyverno.io_clusterpolicies.yaml index 87c998694c..bab7108f0f 100644 --- a/definitions/crds/kyverno.io_clusterpolicies.yaml +++ b/definitions/crds/kyverno.io_clusterpolicies.yaml @@ -146,18 +146,12 @@ spec: supports wildcard characters "*" (matches zero or many characters) and "?" (at least one character). type: string - namespaces: - description: Namespaces is a list of namespaces names. - Each name supports wildcard characters "*" (matches - zero or many characters) and "?" (at least one character). - items: - type: string - type: array - selector: - description: 'Selector is a label selector. Label keys - and values in `matchLabels` support the wildcard characters - `*` (matches zero or many characters) and `?` (matches - one character). Wildcards allows writing label selectors + namespaceSelector: + description: 'NamespaceSelector is a label selector + for the resource namespace. Label keys and values + in `matchLabels` support the wildcard characters `*` + (matches zero or many characters) and `?` (matches + one character).Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but does not match an empty label set.' @@ -205,13 +199,20 @@ spec: requirements are ANDed. type: object type: object - namespaceSelector: - description: 'NamespaceSelector is a label selector for namespace. - Label keys and values in `matchLabels` support the wildcard - characters `*` (matches zero or many characters) and `?` - (matches one character). Wildcards allows writing label - selectors like ["storage.k8s.io/*": "*"]. Note that using - ["*" : "*"] matches any key and value but does not match + namespaces: + description: Namespaces is a list of namespaces names. + Each name supports wildcard characters "*" (matches + zero or many characters) and "?" (at least one character). + items: + type: string + type: array + selector: + description: 'Selector is a label selector. Label keys + and values in `matchLabels` support the wildcard characters + `*` (matches zero or many characters) and `?` (matches + one character). Wildcards allows writing label selectors + like ["storage.k8s.io/*": "*"]. Note that using ["*" + : "*"] matches any key and value but does not match an empty label set.' properties: matchExpressions: @@ -380,18 +381,12 @@ spec: supports wildcard characters "*" (matches zero or many characters) and "?" (at least one character). type: string - namespaces: - description: Namespaces is a list of namespaces names. - Each name supports wildcard characters "*" (matches - zero or many characters) and "?" (at least one character). - items: - type: string - type: array - selector: - description: 'Selector is a label selector. Label keys - and values in `matchLabels` support the wildcard characters - `*` (matches zero or many characters) and `?` (matches - one character). Wildcards allows writing label selectors + namespaceSelector: + description: 'NamespaceSelector is a label selector + for the resource namespace. Label keys and values + in `matchLabels` support the wildcard characters `*` + (matches zero or many characters) and `?` (matches + one character).Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but does not match an empty label set.' @@ -439,13 +434,20 @@ spec: requirements are ANDed. type: object type: object - namespaceSelector: - description: 'NamespaceSelector is a label selector for namespace. - Label keys and values in `matchLabels` support the wildcard - characters `*` (matches zero or many characters) and `?` - (matches one character). Wildcards allows writing label - selectors like ["storage.k8s.io/*": "*"]. Note that using - ["*" : "*"] matches any key and value but does not match + namespaces: + description: Namespaces is a list of namespaces names. + Each name supports wildcard characters "*" (matches + zero or many characters) and "?" (at least one character). + items: + type: string + type: array + selector: + description: 'Selector is a label selector. Label keys + and values in `matchLabels` support the wildcard characters + `*` (matches zero or many characters) and `?` (matches + one character). Wildcards allows writing label selectors + like ["storage.k8s.io/*": "*"]. Note that using ["*" + : "*"] matches any key and value but does not match an empty label set.' properties: matchExpressions: @@ -578,6 +580,7 @@ spec: name: description: Name is a label to identify the rule, It must be unique within the policy. + maxLength: 63 type: string preconditions: description: Conditions enable variable-based conditional rule diff --git a/definitions/crds/kyverno.io_policies.yaml b/definitions/crds/kyverno.io_policies.yaml index 729ad57202..bd66fa4062 100644 --- a/definitions/crds/kyverno.io_policies.yaml +++ b/definitions/crds/kyverno.io_policies.yaml @@ -147,18 +147,12 @@ spec: supports wildcard characters "*" (matches zero or many characters) and "?" (at least one character). type: string - namespaces: - description: Namespaces is a list of namespaces names. - Each name supports wildcard characters "*" (matches - zero or many characters) and "?" (at least one character). - items: - type: string - type: array - selector: - description: 'Selector is a label selector. Label keys - and values in `matchLabels` support the wildcard characters - `*` (matches zero or many characters) and `?` (matches - one character). Wildcards allows writing label selectors + namespaceSelector: + description: 'NamespaceSelector is a label selector + for the resource namespace. Label keys and values + in `matchLabels` support the wildcard characters `*` + (matches zero or many characters) and `?` (matches + one character).Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but does not match an empty label set.' @@ -206,13 +200,20 @@ spec: requirements are ANDed. type: object type: object - namespaceSelector: - description: 'NamespaceSelector is a label selector for namespace. - Label keys and values in `matchLabels` support the wildcard - characters `*` (matches zero or many characters) and `?` - (matches one character). Wildcards allows writing label - selectors like ["storage.k8s.io/*": "*"]. Note that using - ["*" : "*"] matches any key and value but does not match + namespaces: + description: Namespaces is a list of namespaces names. + Each name supports wildcard characters "*" (matches + zero or many characters) and "?" (at least one character). + items: + type: string + type: array + selector: + description: 'Selector is a label selector. Label keys + and values in `matchLabels` support the wildcard characters + `*` (matches zero or many characters) and `?` (matches + one character). Wildcards allows writing label selectors + like ["storage.k8s.io/*": "*"]. Note that using ["*" + : "*"] matches any key and value but does not match an empty label set.' properties: matchExpressions: @@ -381,18 +382,12 @@ spec: supports wildcard characters "*" (matches zero or many characters) and "?" (at least one character). type: string - namespaces: - description: Namespaces is a list of namespaces names. - Each name supports wildcard characters "*" (matches - zero or many characters) and "?" (at least one character). - items: - type: string - type: array - selector: - description: 'Selector is a label selector. Label keys - and values in `matchLabels` support the wildcard characters - `*` (matches zero or many characters) and `?` (matches - one character). Wildcards allows writing label selectors + namespaceSelector: + description: 'NamespaceSelector is a label selector + for the resource namespace. Label keys and values + in `matchLabels` support the wildcard characters `*` + (matches zero or many characters) and `?` (matches + one character).Wildcards allows writing label selectors like ["storage.k8s.io/*": "*"]. Note that using ["*" : "*"] matches any key and value but does not match an empty label set.' @@ -440,13 +435,20 @@ spec: requirements are ANDed. type: object type: object - namespaceSelector: - description: 'NamespaceSelector is a label selector for namespace. - Label keys and values in `matchLabels` support the wildcard - characters `*` (matches zero or many characters) and `?` - (matches one character). Wildcards allows writing label - selectors like ["storage.k8s.io/*": "*"]. Note that using - ["*" : "*"] matches any key and value but does not match + namespaces: + description: Namespaces is a list of namespaces names. + Each name supports wildcard characters "*" (matches + zero or many characters) and "?" (at least one character). + items: + type: string + type: array + selector: + description: 'Selector is a label selector. Label keys + and values in `matchLabels` support the wildcard characters + `*` (matches zero or many characters) and `?` (matches + one character). Wildcards allows writing label selectors + like ["storage.k8s.io/*": "*"]. Note that using ["*" + : "*"] matches any key and value but does not match an empty label set.' properties: matchExpressions: @@ -579,6 +581,7 @@ spec: name: description: Name is a label to identify the rule, It must be unique within the policy. + maxLength: 63 type: string preconditions: description: Conditions enable variable-based conditional rule diff --git a/pkg/api/kyverno/v1/policy_types.go b/pkg/api/kyverno/v1/policy_types.go index 83c2e68bed..98c8742d1c 100755 --- a/pkg/api/kyverno/v1/policy_types.go +++ b/pkg/api/kyverno/v1/policy_types.go @@ -61,6 +61,7 @@ type Spec struct { type Rule struct { // Name is a label to identify the rule, It must be unique within the policy. + // +kubebuilder:validation:MaxLength=63 Name string `json:"name,omitempty" yaml:"name,omitempty"` // Context defines variables and data sources that can be used during rule execution. @@ -182,7 +183,7 @@ const ( GreaterThanOrEquals ConditionOperator = "GreaterThanOrEquals" // GreaterThan evaluates if the key (numeric) is greater than the value (numeric). GreaterThan ConditionOperator = "GreaterThan" - // LessThan evaluates if the key (numeric) is less than or equal to the value (numeric). + // LessThanOrEquals evaluates if the key (numeric) is less than or equal to the value (numeric). LessThanOrEquals ConditionOperator = "LessThanOrEquals" // LessThan evaluates if the key (numeric) is less than the value (numeric). LessThan ConditionOperator = "LessThan" diff --git a/pkg/policy/background.go b/pkg/policy/background.go index 3f50c97d99..8755462cc3 100644 --- a/pkg/policy/background.go +++ b/pkg/policy/background.go @@ -51,7 +51,7 @@ func ContainsVariablesOtherThanObject(policy kyverno.ClusterPolicy) error { anyPattern, err := rule.Validation.DeserializeAnyPattern() if err != nil { - return fmt.Errorf("failed to deserialze anyPattern, expect array: %v", err) + return fmt.Errorf("failed to deserialize anyPattern, expect array: %v", err) } for idx2, pattern := range anyPattern { diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index aaeac78ea2..2289286f53 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -30,6 +30,11 @@ func Validate(policy *kyverno.ClusterPolicy, client *dclient.Client, mock bool, return fmt.Errorf("policy contains invalid variables") } + // policy name is stored in the label of the report change request + if len(p.Name) > 63 { + return fmt.Errorf("invalid policy name %s: must be no more than 63 characters", p.Name) + } + if path, err := validateUniqueRuleName(p); err != nil { return fmt.Errorf("path: spec.%s: %v", path, err) } diff --git a/pkg/policy/validate/validate.go b/pkg/policy/validate/validate.go index 8cb40ba81c..af2e38af24 100644 --- a/pkg/policy/validate/validate.go +++ b/pkg/policy/validate/validate.go @@ -39,7 +39,7 @@ func (v *Validate) Validate() (string, error) { if rule.AnyPattern != nil { anyPattern, err := rule.DeserializeAnyPattern() if err != nil { - return "anyPattern", fmt.Errorf("failed to deserialze anyPattern, expect array: %v", err) + return "anyPattern", fmt.Errorf("failed to deserialize anyPattern, expect array: %v", err) } for i, pattern := range anyPattern { if path, err := common.ValidatePattern(pattern, "/", []commonAnchors.IsAnchor{commonAnchors.IsConditionAnchor, commonAnchors.IsExistenceAnchor, commonAnchors.IsEqualityAnchor, commonAnchors.IsNegationAnchor}); err != nil { diff --git a/pkg/policymutation/cronjob.go b/pkg/policymutation/cronjob.go index 86df306b0d..da3f79d8b7 100644 --- a/pkg/policymutation/cronjob.go +++ b/pkg/policymutation/cronjob.go @@ -26,7 +26,13 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) } cronJobRule := &jobRule - cronJobRule.Name = fmt.Sprintf("autogen-cronjob-%s", rule.Name) + + name := fmt.Sprintf("autogen-cronjob-%s", rule.Name) + if len(name) > 63 { + name = name[:63] + } + cronJobRule.Name = name + cronJobRule.MatchResources.Kinds = []string{engine.PodControllerCronJob} if (jobRule.ExcludeResources) != nil && (len(jobRule.ExcludeResources.Kinds) > 0) { cronJobRule.ExcludeResources.Kinds = []string{engine.PodControllerCronJob} @@ -72,9 +78,9 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) if (jobRule.Validation != nil) && (jobRule.Validation.AnyPattern != nil) { var patterns []interface{} - anyPatterns, err := rule.Validation.DeserializeAnyPattern() + anyPatterns, err := jobRule.Validation.DeserializeAnyPattern() if err != nil { - logger.Error(err, "failed to deserialze anyPattern, expect tyepe array") + logger.Error(err, "failed to deserialize anyPattern, expect type array") } for _, pattern := range anyPatterns { diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index 305b762f15..4976a368ee 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -10,11 +10,10 @@ import ( jsonpatch "github.com/evanphx/json-patch" "github.com/go-logr/logr" + kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/common" "github.com/kyverno/kyverno/pkg/engine" "github.com/kyverno/kyverno/pkg/utils" - - kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" ) // GenerateJSONPatchesForDefaults generates default JSON patches for @@ -349,7 +348,7 @@ func generateRulePatches(policy kyverno.ClusterPolicy, controllers string, log l // the kyvernoRule holds the temporary kyverno rule struct // each field is a pointer to the the actual object -// when serilizing data, we would expect to drop the omitempty key +// when serializing data, we would expect to drop the omitempty key // otherwise (without the pointer), it will be set to empty value // - an empty struct in this case, some may fail the schema validation // may related to: @@ -405,7 +404,7 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. if skipAutoGeneration { if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil || exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil { - logger.Info("skip generating rule on pod controllers: Name / Selector in resource decription may not be applicable.", "rule", rule.Name) + logger.Info("skip generating rule on pod controllers: Name / Selector in resource description may not be applicable.", "rule", rule.Name) return kyvernoRule{} } if controllers == "all" { @@ -415,8 +414,12 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. } } + name := fmt.Sprintf("autogen-%s", rule.Name) + if len(name) > 63 { + name = name[:63] + } controllerRule := &kyvernoRule{ - Name: fmt.Sprintf("autogen-%s", rule.Name), + Name: name, MatchResources: match.DeepCopy(), } @@ -473,7 +476,7 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. var patterns []interface{} anyPatterns, err := rule.Validation.DeserializeAnyPattern() if err != nil { - logger.Error(err, "failed to deserialze anyPattern, expect type array") + logger.Error(err, "failed to deserialize anyPattern, expect type array") } for _, pattern := range anyPatterns { diff --git a/pkg/policyreport/builder.go b/pkg/policyreport/builder.go index 1569ccfeea..cfc19fa3a6 100755 --- a/pkg/policyreport/builder.go +++ b/pkg/policyreport/builder.go @@ -20,11 +20,15 @@ import ( ) const ( - resourceLabelNamespace string = "kyverno.io/resource.namespace" - 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" + // the following labels are used to list rcr / crcr + resourceLabelNamespace string = "kyverno.io/resource.namespace" + deletedLabelPolicy string = "kyverno.io/delete.policy" + deletedLabelRule string = "kyverno.io/delete.rule" + + // the following annotations are used to remove entries from polr / cpolr + // there would be a problem if use labels as the value could exceed 63 chars + deletedAnnotationResourceName string = "kyverno.io/delete.resource.name" + deletedAnnotationResourceKind string = "kyverno.io/delete.resource.kind" ) func generatePolicyReportName(ns string) string { @@ -169,10 +173,13 @@ func set(obj *unstructured.Unstructured, info Info) { func setRequestLabels(req *unstructured.Unstructured, info Info) bool { switch { case isResourceDeletion(info): + req.SetAnnotations(map[string]string{ + deletedAnnotationResourceName: info.Results[0].Resource.Name, + deletedAnnotationResourceKind: info.Results[0].Resource.Kind, + }) + req.SetLabels(map[string]string{ - resourceLabelNamespace: info.Results[0].Resource.Namespace, - deletedLabelResource: info.Results[0].Resource.Name, - deletedLabelResourceKind: info.Results[0].Resource.Kind, + resourceLabelNamespace: info.Results[0].Resource.Namespace, }) return true diff --git a/pkg/policyreport/changerequestcreator.go b/pkg/policyreport/changerequestcreator.go index bd6fbafc7f..859c9852b2 100644 --- a/pkg/policyreport/changerequestcreator.go +++ b/pkg/policyreport/changerequestcreator.go @@ -228,7 +228,7 @@ func addSummary(dst, src *unstructured.Unstructured) { } func isDeleteRequest(request *unstructured.Unstructured) bool { - deleteLabels := []string{deletedLabelPolicy, deletedLabelRule, deletedLabelResource, deletedLabelResourceKind} + deleteLabels := []string{deletedLabelPolicy, deletedLabelRule} labels := request.GetLabels() for _, l := range deleteLabels { @@ -236,5 +236,14 @@ func isDeleteRequest(request *unstructured.Unstructured) bool { return true } } + + deleteAnnotations := []string{deletedAnnotationResourceName, deletedAnnotationResourceKind} + annotations := request.GetAnnotations() + for _, ann := range deleteAnnotations { + if _, ok := annotations[ann]; ok { + return true + } + } + return false } diff --git a/pkg/policyreport/policyreport.go b/pkg/policyreport/policyreport.go index 76b5d877fb..849a26f9a7 100644 --- a/pkg/policyreport/policyreport.go +++ b/pkg/policyreport/policyreport.go @@ -15,12 +15,12 @@ type deletedResource struct { kind, ns, name string } -func buildLabelForDeletedResource(labels map[string]string) *deletedResource { +func buildLabelForDeletedResource(labels, annotations map[string]string) *deletedResource { ok := true - kind, kindOk := labels[deletedLabelResourceKind] + kind, kindOk := annotations[deletedAnnotationResourceKind] ok = ok && kindOk - name, nameOk := labels[deletedLabelResource] + name, nameOk := annotations[deletedAnnotationResourceName] ok = ok && nameOk if !ok { @@ -37,14 +37,14 @@ func buildLabelForDeletedResource(labels map[string]string) *deletedResource { func getDeletedResources(aggregatedRequests interface{}) (resources []deletedResource) { if requests, ok := aggregatedRequests.([]*changerequest.ClusterReportChangeRequest); ok { for _, request := range requests { - dr := buildLabelForDeletedResource(request.GetLabels()) + dr := buildLabelForDeletedResource(request.GetLabels(), request.GetAnnotations()) if dr != nil { resources = append(resources, *dr) } } } else if requests, ok := aggregatedRequests.([]*changerequest.ReportChangeRequest); ok { for _, request := range requests { - dr := buildLabelForDeletedResource(request.GetLabels()) + dr := buildLabelForDeletedResource(request.GetLabels(), request.GetAnnotations()) if dr != nil { resources = append(resources, *dr) }