From 1154612489216fd2fb77ca4da6dfb6e12d698d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 1 Mar 2022 23:42:19 +0100 Subject: [PATCH] refactor: pass only spec instead of whole policy when possible (#3315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Co-authored-by: Jim Bugwadia --- pkg/policy/policy_controller.go | 2 +- pkg/policymutation/policymutation.go | 37 ++++++++++++----------- pkg/policymutation/policymutation_test.go | 20 ++++++------ 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/pkg/policy/policy_controller.go b/pkg/policy/policy_controller.go index 703b4862b1..94848283b0 100644 --- a/pkg/policy/policy_controller.go +++ b/pkg/policy/policy_controller.go @@ -563,7 +563,7 @@ func updateGR(kyvernoClient *kyvernoclient.Clientset, policyKey string, grList [ func missingAutoGenRules(policy *kyverno.ClusterPolicy, log logr.Logger) bool { var podRuleName []string ruleCount := 1 - if canApplyAutoGen, _ := pm.CanAutoGen(policy, log); canApplyAutoGen { + if canApplyAutoGen, _ := pm.CanAutoGen(&policy.Spec, log); canApplyAutoGen { for _, rule := range policy.Spec.Rules { podRuleName = append(podRuleName, rule.Name) } diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index c64babc91a..1a50d6e0c5 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -26,18 +26,18 @@ func GenerateJSONPatchesForDefaults(policy *kyverno.ClusterPolicy, log logr.Logg var updateMsgs []string // default 'ValidationFailureAction' - if patch, updateMsg := defaultvalidationFailureAction(policy, log); patch != nil { + if patch, updateMsg := defaultvalidationFailureAction(&policy.Spec, log); patch != nil { patches = append(patches, patch) updateMsgs = append(updateMsgs, updateMsg) } // default 'Background' - if patch, updateMsg := defaultBackgroundFlag(policy, log); patch != nil { + if patch, updateMsg := defaultBackgroundFlag(&policy.Spec, log); patch != nil { patches = append(patches, patch) updateMsgs = append(updateMsgs, updateMsg) } - if patch, updateMsg := defaultFailurePolicy(policy, log); patch != nil { + if patch, updateMsg := defaultFailurePolicy(&policy.Spec, log); patch != nil { patches = append(patches, patch) updateMsgs = append(updateMsgs, updateMsg) } @@ -156,10 +156,10 @@ func buildReplaceJsonPatch(path string, kindList []string) ([]byte, error) { return json.Marshal(jsonPatch) } -func defaultBackgroundFlag(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byte, string) { +func defaultBackgroundFlag(spec *kyverno.Spec, log logr.Logger) ([]byte, string) { // set 'Background' flag to 'true' if not specified defaultVal := true - if policy.Spec.Background == nil { + if spec.Background == nil { log.V(4).Info("setting default value", "spec.background", true) jsonPatch := struct { Path string `json:"path"` @@ -184,10 +184,10 @@ func defaultBackgroundFlag(policy *kyverno.ClusterPolicy, log logr.Logger) ([]by return nil, "" } -func defaultvalidationFailureAction(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byte, string) { +func defaultvalidationFailureAction(spec *kyverno.Spec, log logr.Logger) ([]byte, string) { // set ValidationFailureAction to "audit" if not specified Audit := common.Audit - if policy.Spec.ValidationFailureAction == "" { + if spec.ValidationFailureAction == "" { log.V(4).Info("setting default value", "spec.validationFailureAction", Audit) jsonPatch := struct { @@ -213,10 +213,11 @@ func defaultvalidationFailureAction(policy *kyverno.ClusterPolicy, log logr.Logg return nil, "" } -func defaultFailurePolicy(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byte, string) { + +func defaultFailurePolicy(spec *kyverno.Spec, log logr.Logger) ([]byte, string) { // set failurePolicy to Fail if not present failurePolicy := string(kyverno.Fail) - if policy.Spec.FailurePolicy == nil { + if spec.FailurePolicy == nil { log.V(4).Info("setting default value", "spec.failurePolicy", failurePolicy) jsonPatch := struct { Path string `json:"path"` @@ -252,7 +253,7 @@ func defaultFailurePolicy(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byt // GeneratePodControllerRule returns two patches: rulePatches and annotation patch(if necessary) func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { - applyAutoGen, desiredControllers := CanAutoGen(&policy, log) + applyAutoGen, desiredControllers := CanAutoGen(&policy.Spec, log) if !applyAutoGen { desiredControllers = "none" @@ -284,7 +285,7 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p log.V(3).Info("auto generating rule for pod controllers", "controllers", actualControllers) - p, err := generateRulePatches(policy, actualControllers, log) + p, err := generateRulePatches(&policy.Spec, actualControllers, log) patches = append(patches, p...) errs = append(errs, err...) return @@ -298,9 +299,9 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p // - Pod and PodControllers are not defined // - mutate.Patches/mutate.PatchesJSON6902/validate.deny/generate rule is defined // - otherwise it returns all pod controllers -func CanAutoGen(policy *kyverno.ClusterPolicy, log logr.Logger) (applyAutoGen bool, controllers string) { +func CanAutoGen(spec *kyverno.Spec, log logr.Logger) (applyAutoGen bool, controllers string) { var needAutogen bool - for _, rule := range policy.Spec.Rules { + for _, rule := range spec.Rules { match := rule.MatchResources exclude := rule.ExcludeResources @@ -436,16 +437,16 @@ func updateGenRuleByte(pbyte []byte, kind string, genRule kyvernoRule) (obj []by } // generateRulePatches generates rule for podControllers based on scenario A and C -func generateRulePatches(policy kyverno.ClusterPolicy, controllers string, log logr.Logger) (rulePatches [][]byte, errs []error) { - insertIdx := len(policy.Spec.Rules) +func generateRulePatches(spec *kyverno.Spec, controllers string, log logr.Logger) (rulePatches [][]byte, errs []error) { + insertIdx := len(spec.Rules) - ruleMap := createRuleMap(policy.Spec.Rules) + ruleMap := createRuleMap(spec.Rules) var ruleIndex = make(map[string]int) - for index, rule := range policy.Spec.Rules { + for index, rule := range spec.Rules { ruleIndex[rule.Name] = index } - for _, rule := range policy.Spec.Rules { + for _, rule := range spec.Rules { patchPostion := insertIdx convertToPatches := func(genRule kyvernoRule, patchPostion int) []byte { operation := "add" diff --git a/pkg/policymutation/policymutation_test.go b/pkg/policymutation/policymutation_test.go index 5e6943765e..c2b024389e 100644 --- a/pkg/policymutation/policymutation_test.go +++ b/pkg/policymutation/policymutation_test.go @@ -47,7 +47,7 @@ func Test_Any(t *testing.T) { }, } - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) fmt.Println("utils.JoinPatches(patches)erterter", string(utils.JoinPatches(rulePatches))) if len(errs) != 0 { t.Log(errs) @@ -85,7 +85,7 @@ func Test_All(t *testing.T) { }, } - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -117,7 +117,7 @@ func Test_Exclude(t *testing.T) { policy := policies[0] policy.Spec.Rules[0].ExcludeResources.Namespaces = []string{"fake-namespce"} - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -153,7 +153,7 @@ func Test_CronJobOnly(t *testing.T) { engine.PodControllersAnnotation: controllers, }) - rulePatches, errs := generateRulePatches(*policy, controllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, controllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -181,7 +181,7 @@ func Test_ForEachPod(t *testing.T) { policy := policies[0] policy.Spec.Rules[0].ExcludeResources.Namespaces = []string{"fake-namespce"} - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -223,7 +223,7 @@ func Test_CronJob_hasExclude(t *testing.T) { rule.ExcludeResources.Namespaces = []string{"test"} policy.Spec.Rules[0] = *rule - rulePatches, errs := generateRulePatches(*policy, controllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, controllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -254,7 +254,7 @@ func Test_CronJobAndDeployment(t *testing.T) { engine.PodControllersAnnotation: controllers, }) - rulePatches, errs := generateRulePatches(*policy, controllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, controllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -351,7 +351,7 @@ func Test_getControllers(t *testing.T) { err := json.Unmarshal(test.policy, &policy) assert.NilError(t, err) - applyAutoGen, controllers := CanAutoGen(&policy, log.Log) + applyAutoGen, controllers := CanAutoGen(&policy.Spec, log.Log) if !applyAutoGen { controllers = "none" } @@ -374,7 +374,7 @@ func Test_UpdateVariablePath(t *testing.T) { policy := policies[0] - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) if len(errs) != 0 { t.Log(errs) } @@ -489,7 +489,7 @@ func Test_Deny(t *testing.T) { }, } - rulePatches, errs := generateRulePatches(*policy, engine.PodControllers, log.Log) + rulePatches, errs := generateRulePatches(&policy.Spec, engine.PodControllers, log.Log) fmt.Println("utils.JoinPatches(patches)erterter", string(utils.JoinPatches(rulePatches))) if len(errs) != 0 { t.Log(errs)