From 05a073718422d3d417ac818135d1dfc9bc94c045 Mon Sep 17 00:00:00 2001 From: Vyankatesh Kudtarkar Date: Thu, 7 Oct 2021 04:49:55 +0530 Subject: [PATCH] Fix Autogen issue for any/all block and new rule foreach (#2471) * Fix Autogen issue for any/all block and Support gvk in match kind block * remove log and fix test * Fix issues * Fix cronjob issue * Fix autogen for Foreach * Fix autogen for For each * Fix for each issue * adding missing assignements * Update autogen for foreach rule --- pkg/api/kyverno/v1/utils.go | 17 ++- pkg/policymutation/cronjob.go | 59 ++++++++- pkg/policymutation/policymutation.go | 143 +++++++++++++++++++--- pkg/policymutation/policymutation_test.go | 42 ++++++- pkg/utils/util.go | 11 ++ 5 files changed, 245 insertions(+), 27 deletions(-) diff --git a/pkg/api/kyverno/v1/utils.go b/pkg/api/kyverno/v1/utils.go index 69d433e751..4902aa666a 100755 --- a/pkg/api/kyverno/v1/utils.go +++ b/pkg/api/kyverno/v1/utils.go @@ -4,6 +4,8 @@ import ( "encoding/json" "reflect" "strings" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" ) // HasAutoGenAnnotation checks if a policy has auto-gen annotation @@ -128,8 +130,20 @@ func (in *Validation) DeserializeAnyPattern() ([]interface{}, error) { if in.AnyPattern == nil { return nil, nil } + res, nil := deserializePattern(in.AnyPattern) + return res, nil +} - anyPattern, err := json.Marshal(in.AnyPattern) +func (in *Validation) DeserializeForEachAnyPattern() ([]interface{}, error) { + if in.ForEachValidation.AnyPattern == nil { + return nil, nil + } + res, nil := deserializePattern(in.ForEachValidation.AnyPattern) + return res, nil +} + +func deserializePattern(pattern apiextensions.JSON) ([]interface{}, error) { + anyPattern, err := json.Marshal(pattern) if err != nil { return nil, err } @@ -138,7 +152,6 @@ func (in *Validation) DeserializeAnyPattern() ([]interface{}, error) { if err := json.Unmarshal(anyPattern, &res); err != nil { return nil, err } - return res, nil } diff --git a/pkg/policymutation/cronjob.go b/pkg/policymutation/cronjob.go index 7647a5d76d..017512cd24 100644 --- a/pkg/policymutation/cronjob.go +++ b/pkg/policymutation/cronjob.go @@ -9,6 +9,7 @@ import ( kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine" "github.com/kyverno/kyverno/pkg/engine/variables" + "github.com/kyverno/kyverno/pkg/utils" ) func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) kyvernoRule { @@ -34,9 +35,26 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) } cronJobRule.Name = name - cronJobRule.MatchResources.Kinds = []string{engine.PodControllerCronJob} - if (jobRule.ExcludeResources) != nil && (len(jobRule.ExcludeResources.Kinds) > 0) { - cronJobRule.ExcludeResources.Kinds = []string{engine.PodControllerCronJob} + if len(jobRule.MatchResources.Any) > 0 { + rule := cronJobAnyAllAutogenRule(cronJobRule.MatchResources.Any) + cronJobRule.MatchResources.Any = rule + } else if len(jobRule.MatchResources.All) > 0 { + rule := cronJobAnyAllAutogenRule(cronJobRule.MatchResources.All) + cronJobRule.MatchResources.All = rule + } else { + cronJobRule.MatchResources.Kinds = []string{engine.PodControllerCronJob} + } + + if (jobRule.ExcludeResources) != nil && len(jobRule.ExcludeResources.Any) > 0 { + rule := cronJobAnyAllAutogenRule(cronJobRule.ExcludeResources.Any) + cronJobRule.ExcludeResources.Any = rule + } else if (jobRule.ExcludeResources) != nil && len(jobRule.ExcludeResources.All) > 0 { + rule := cronJobAnyAllAutogenRule(cronJobRule.ExcludeResources.All) + cronJobRule.ExcludeResources.All = rule + } else { + if (jobRule.ExcludeResources) != nil && (len(jobRule.ExcludeResources.Kinds) > 0) { + cronJobRule.ExcludeResources.Kinds = []string{engine.PodControllerCronJob} + } } if (jobRule.Mutation != nil) && (jobRule.Mutation.Overlay != nil) { @@ -77,6 +95,15 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) return *cronJobRule } + if (jobRule.Validation != nil) && (jobRule.Validation.ForEachValidation != nil) && (jobRule.Validation.ForEachValidation.Pattern != nil) { + newValidate := &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/jobTemplate/spec/template", "pattern"), + ForEachValidation: jobRule.Validation.ForEachValidation, + } + cronJobRule.Validation = newValidate.DeepCopy() + return *cronJobRule + } + if (jobRule.Validation != nil) && (jobRule.Validation.AnyPattern != nil) { var patterns []interface{} anyPatterns, err := jobRule.Validation.DeserializeAnyPattern() @@ -101,6 +128,22 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) return *cronJobRule } + if (jobRule.Validation != nil) && (jobRule.Validation.ForEachValidation != nil) && (jobRule.Validation.ForEachValidation.AnyPattern != nil) { + cronJobRule.Validation = &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/jobTemplate/spec/template", "anyPattern"), + ForEachValidation: jobRule.Validation.ForEachValidation, + } + return *cronJobRule + } + + if (jobRule.Validation != nil) && (jobRule.Validation.ForEachValidation != nil) && (jobRule.Validation.ForEachValidation.Deny != nil) { + cronJobRule.Validation = &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/jobTemplate/spec/template", "anyPattern"), + ForEachValidation: jobRule.Validation.ForEachValidation, + } + return *cronJobRule + } + return kyvernoRule{} } @@ -123,3 +166,13 @@ func stripCronJob(controllers string) string { return strings.Join(newControllers, ",") } + +func cronJobAnyAllAutogenRule(v kyverno.ResourceFilters) kyverno.ResourceFilters { + anyKind := v.DeepCopy() + for i, value := range v { + if utils.ContainsPod(value.Kinds, "Job") { + anyKind[i].Kinds = []string{engine.PodControllerCronJob} + } + } + return anyKind +} diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index 5e4279a3bf..f7c597e31a 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -397,17 +397,55 @@ func CanAutoGen(policy *kyverno.ClusterPolicy, log logr.Logger) (applyAutoGen bo match := rule.MatchResources exclude := rule.ExcludeResources - if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil || - exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil { + if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil || match.ResourceDescription.Annotations != nil || + exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil || exclude.ResourceDescription.Annotations != nil { log.V(3).Info("skip generating rule on pod controllers: Name / Selector in resource description may not be applicable.", "rule", rule.Name) return false, "none" } - if (len(match.Kinds) > 1 && utils.ContainsString(match.Kinds, "Pod")) || - (len(exclude.Kinds) > 1 && utils.ContainsString(exclude.Kinds, "Pod")) { + if isKindOtherthanPod(match.Kinds) || isKindOtherthanPod(exclude.Kinds) { return false, "none" } + for _, value := range match.Any { + if isKindOtherthanPod(value.Kinds) { + return false, "none" + } + if value.Name != "" || value.Selector != nil || value.Annotations != nil { + log.V(3).Info("skip generating rule on pod controllers: Name / Selector in match any block is not be applicable.", "rule", rule.Name) + return false, "none" + } + } + for _, value := range match.All { + + if isKindOtherthanPod(value.Kinds) { + return false, "none" + } + if value.Name != "" || value.Selector != nil || value.Annotations != nil { + log.V(3).Info("skip generating rule on pod controllers: Name / Selector in match all block is not be applicable.", "rule", rule.Name) + return false, "none" + } + } + for _, value := range exclude.Any { + if isKindOtherthanPod(value.Kinds) { + return false, "none" + } + if value.Name != "" || value.Selector != nil || value.Annotations != nil { + log.V(3).Info("skip generating rule on pod controllers: Name / Selector in exclude any block is not be applicable.", "rule", rule.Name) + return false, "none" + } + } + for _, value := range exclude.All { + + if isKindOtherthanPod(value.Kinds) { + return false, "none" + } + if value.Name != "" || value.Selector != nil || value.Annotations != nil { + log.V(3).Info("skip generating rule on pod controllers: Name / Selector in exclud all block is not be applicable.", "rule", rule.Name) + return false, "none" + } + } + if rule.Mutation.Patches != nil || rule.Mutation.PatchesJSON6902 != "" || rule.Validation.Deny != nil || rule.HasGenerate() { return false, "none" @@ -417,6 +455,13 @@ func CanAutoGen(policy *kyverno.ClusterPolicy, log logr.Logger) (applyAutoGen bo return true, engine.PodControllers } +func isKindOtherthanPod(kinds []string) bool { + if len(kinds) > 1 && utils.ContainsPod(kinds, "Pod") { + return true + } + return false +} + func createRuleMap(rules []kyverno.Rule) map[string]kyvernoRule { var ruleMap = make(map[string]kyvernoRule) for _, rule := range rules { @@ -570,8 +615,8 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. matchResourceDescriptionsKinds := rule.MatchKinds() excludeResourceDescriptionsKinds := rule.ExcludeKinds() - if !utils.ContainsString(matchResourceDescriptionsKinds, "Pod") || - (len(excludeResourceDescriptionsKinds) != 0 && !utils.ContainsString(excludeResourceDescriptionsKinds, "Pod")) { + if !utils.ContainsPod(matchResourceDescriptionsKinds, "Pod") || + (len(excludeResourceDescriptionsKinds) != 0 && !utils.ContainsPod(excludeResourceDescriptionsKinds, "Pod")) { return kyvernoRule{} } @@ -631,9 +676,26 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. } // overwrite Kinds by pod controllers defined in the annotation - controllerRule.MatchResources.Kinds = strings.Split(controllers, ",") - if len(exclude.Kinds) != 0 { - controllerRule.ExcludeResources.Kinds = strings.Split(controllers, ",") + if len(rule.MatchResources.Any) > 0 { + rule := getAnyAllAutogenRule(controllerRule.MatchResources.Any, controllers) + controllerRule.MatchResources.Any = rule + } else if len(rule.MatchResources.All) > 0 { + rule := getAnyAllAutogenRule(controllerRule.MatchResources.All, controllers) + controllerRule.MatchResources.All = rule + } else { + controllerRule.MatchResources.Kinds = strings.Split(controllers, ",") + } + + if len(rule.ExcludeResources.Any) > 0 { + rule := getAnyAllAutogenRule(controllerRule.ExcludeResources.Any, controllers) + controllerRule.ExcludeResources.Any = rule + } else if len(rule.ExcludeResources.All) > 0 { + rule := getAnyAllAutogenRule(controllerRule.ExcludeResources.All, controllers) + controllerRule.ExcludeResources.All = rule + } else { + if len(exclude.Kinds) != 0 { + controllerRule.ExcludeResources.Kinds = strings.Split(controllers, ",") + } } if rule.Mutation.Overlay != nil { @@ -675,23 +737,23 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. return *controllerRule } + if rule.Validation.ForEachValidation != nil && rule.Validation.ForEachValidation.Pattern != nil { + newForeachValidate := &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/template", "pattern"), + ForEachValidation: rule.Validation.ForEachValidation, + } + controllerRule.Validation = newForeachValidate.DeepCopy() + return *controllerRule + } + if rule.Validation.AnyPattern != nil { - var patterns []interface{} + anyPatterns, err := rule.Validation.DeserializeAnyPattern() if err != nil { logger.Error(err, "failed to deserialize anyPattern, expect type array") } - for _, pattern := range anyPatterns { - newPattern := map[string]interface{}{ - "spec": map[string]interface{}{ - "template": pattern, - }, - } - - patterns = append(patterns, newPattern) - } - + patterns := validateAnyPattern(anyPatterns) controllerRule.Validation = &kyverno.Validation{ Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/template", "anyPattern"), AnyPattern: patterns, @@ -699,6 +761,22 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. return *controllerRule } + if rule.Validation.ForEachValidation != nil && rule.Validation.ForEachValidation.AnyPattern != nil { + controllerRule.Validation = &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/template", "pattern"), + ForEachValidation: rule.Validation.ForEachValidation, + } + return *controllerRule + } + + if rule.Validation.ForEachValidation != nil && rule.Validation.ForEachValidation.Deny != nil { + controllerRule.Validation = &kyverno.Validation{ + Message: variables.FindAndShiftReferences(log, rule.Validation.Message, "spec/template", "pattern"), + ForEachValidation: rule.Validation.ForEachValidation, + } + return *controllerRule + } + if rule.VerifyImages != nil { newVerifyImages := make([]*kyverno.ImageVerification, len(rule.VerifyImages)) for i, vi := range rule.VerifyImages { @@ -712,6 +790,31 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr. return kyvernoRule{} } +func validateAnyPattern(anyPatterns []interface{}) []interface{} { + var patterns []interface{} + for _, pattern := range anyPatterns { + newPattern := map[string]interface{}{ + "spec": map[string]interface{}{ + "template": pattern, + }, + } + + patterns = append(patterns, newPattern) + } + return patterns +} + +func getAnyAllAutogenRule(v kyverno.ResourceFilters, controllers string) kyverno.ResourceFilters { + anyKind := v.DeepCopy() + + for i, value := range v { + if utils.ContainsPod(value.Kinds, "Pod") { + anyKind[i].Kinds = strings.Split(controllers, ",") + } + } + return anyKind +} + // defaultPodControllerAnnotation inserts an annotation // "pod-policies.kyverno.io/autogen-controllers=" to policy func defaultPodControllerAnnotation(ann map[string]string, controllers string) ([]byte, error) { diff --git a/pkg/policymutation/policymutation_test.go b/pkg/policymutation/policymutation_test.go index 60a8ce19eb..e209d8b7df 100644 --- a/pkg/policymutation/policymutation_test.go +++ b/pkg/policymutation/policymutation_test.go @@ -47,14 +47,52 @@ func Test_Any(t *testing.T) { }, } + rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + fmt.Println("utils.JoinPatches(patches)erterter", string(utils.JoinPatches(rulePatches))) + if len(errs) != 0 { + t.Log(errs) + } + expectedPatches := [][]byte{ + []byte(`{"path":"/spec/rules/1","op":"add","value":{"name":"autogen-validate-hostPath","match":{"any":[{"resources":{"kinds":["DaemonSet","Deployment","Job","StatefulSet"]}}],"resources":{"kinds":["Pod"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}`), + []byte(`{"path":"/spec/rules/2","op":"add","value":{"name":"autogen-cronjob-validate-hostPath","match":{"any":[{"resources":{"kinds":["CronJob"]}}],"resources":{"kinds":["Pod"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"jobTemplate":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}}}`), + } + + for i, ep := range expectedPatches { + assert.Equal(t, string(rulePatches[i]), string(ep), + fmt.Sprintf("unexpected patch: %s\nexpected: %s", rulePatches[i], ep)) + } +} + +func Test_All(t *testing.T) { + dir, err := os.Getwd() + baseDir := filepath.Dir(filepath.Dir(dir)) + assert.NilError(t, err) + file, err := ioutil.ReadFile(baseDir + "/test/best_practices/disallow_bind_mounts.yaml") + if err != nil { + t.Log(err) + } + policies, err := utils.GetPolicy(file) + if err != nil { + t.Log(err) + } + + policy := policies[0] + policy.Spec.Rules[0].MatchResources.All = kyverno.ResourceFilters{ + { + ResourceDescription: kyverno.ResourceDescription{ + Kinds: []string{"Pod"}, + }, + }, + } + rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) if len(errs) != 0 { t.Log(errs) } expectedPatches := [][]byte{ - []byte(`{"path":"/spec/rules/1","op":"add","value":{"name":"autogen-validate-hostPath","match":{"any":[{"resources":{"kinds":["Pod"]}}],"resources":{"kinds":["DaemonSet","Deployment","Job","StatefulSet"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}`), - []byte(`{"path":"/spec/rules/2","op":"add","value":{"name":"autogen-cronjob-validate-hostPath","match":{"any":[{"resources":{"kinds":["Pod"]}}],"resources":{"kinds":["CronJob"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"jobTemplate":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}}}`), + []byte(`{"path":"/spec/rules/1","op":"add","value":{"name":"autogen-validate-hostPath","match":{"all":[{"resources":{"kinds":["DaemonSet","Deployment","Job","StatefulSet"]}}],"resources":{"kinds":["Pod"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}`), + []byte(`{"path":"/spec/rules/2","op":"add","value":{"name":"autogen-cronjob-validate-hostPath","match":{"all":[{"resources":{"kinds":["CronJob"]}}],"resources":{"kinds":["Pod"]}},"validate":{"message":"Host path volumes are not allowed","pattern":{"spec":{"jobTemplate":{"spec":{"template":{"spec":{"=(volumes)":[{"X(hostPath)":"null"}]}}}}}}}}}`), } for i, ep := range expectedPatches { diff --git a/pkg/utils/util.go b/pkg/utils/util.go index 1c4367dbb3..5725d51397 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -10,6 +10,7 @@ import ( "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" + common "github.com/kyverno/kyverno/pkg/common" client "github.com/kyverno/kyverno/pkg/dclient" engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/minio/pkg/wildcard" @@ -33,6 +34,16 @@ func contains(list []string, element string, fn func(string, string) bool) bool return false } +func ContainsPod(list []string, element string) bool { + for _, e := range list { + _, k := common.GetKindFromGVK(e) + if k == element { + return true + } + } + return false +} + //ContainsNamepace check if namespace satisfies any list of pattern(regex) func ContainsNamepace(patterns []string, ns string) bool { return contains(patterns, ns, compareNamespaces)