diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index 3166047eec..c64babc91a 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -254,6 +254,10 @@ func defaultFailurePolicy(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byt func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) { applyAutoGen, desiredControllers := CanAutoGen(&policy, log) + if !applyAutoGen { + desiredControllers = "none" + } + ann := policy.GetAnnotations() actualControllers, ok := ann[engine.PodControllersAnnotation] @@ -288,12 +292,14 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p // CanAutoGen checks whether the rule(s) (in policy) can be applied to Pod controllers // returns controllers as: -// - "none" if: +// - "" if: // - name or selector is defined // - mixed kinds (Pod + pod controller) is defined +// - 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) { + var needAutogen bool for _, rule := range policy.Spec.Rules { match := rule.MatchResources exclude := rule.ExcludeResources @@ -301,49 +307,61 @@ func CanAutoGen(policy *kyverno.ClusterPolicy, log logr.Logger) (applyAutoGen bo 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" + return false, "" } if isKindOtherthanPod(match.Kinds) || isKindOtherthanPod(exclude.Kinds) { - return false, "none" + return false, "" } + needAutogen = hasAutogenKinds(match.Kinds) || hasAutogenKinds(exclude.Kinds) + for _, value := range match.Any { if isKindOtherthanPod(value.Kinds) { - return false, "none" + return false, "" + } + if !needAutogen { + needAutogen = hasAutogenKinds(value.Kinds) } 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" + return false, "" } } for _, value := range match.All { - if isKindOtherthanPod(value.Kinds) { - return false, "none" + return false, "" + } + if !needAutogen { + needAutogen = hasAutogenKinds(value.Kinds) } 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" + return false, "" } } for _, value := range exclude.Any { if isKindOtherthanPod(value.Kinds) { - return false, "none" + return false, "" + } + if !needAutogen { + needAutogen = hasAutogenKinds(value.Kinds) } 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" + return false, "" } } for _, value := range exclude.All { - if isKindOtherthanPod(value.Kinds) { - return false, "none" + return false, "" + } + if !needAutogen { + needAutogen = hasAutogenKinds(value.Kinds) } 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" + return false, "" } } @@ -352,6 +370,10 @@ func CanAutoGen(policy *kyverno.ClusterPolicy, log logr.Logger) (applyAutoGen bo } } + if !needAutogen { + return false, "" + } + return true, engine.PodControllers } @@ -362,6 +384,16 @@ func isKindOtherthanPod(kinds []string) bool { return false } +func hasAutogenKinds(kind []string) bool { + for _, v := range kind { + if v == "Pod" || strings.Contains(engine.PodControllers, v) { + return true + } + } + + return false +} + func createRuleMap(rules []kyverno.Rule) map[string]kyvernoRule { var ruleMap = make(map[string]kyvernoRule) for _, rule := range rules { diff --git a/pkg/policymutation/policymutation_test.go b/pkg/policymutation/policymutation_test.go index 88a95b71c4..5e6943765e 100644 --- a/pkg/policymutation/policymutation_test.go +++ b/pkg/policymutation/policymutation_test.go @@ -339,6 +339,11 @@ func Test_getControllers(t *testing.T) { policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-service-labels-env"},"annotations":null,"pod-policies.kyverno.io/autogen-controllers":"none","spec":{"background":false,"rules":[{"name":"set-service-label","match":{"resources":{"kinds":["Pod","Deployment"]}},"mutate":{"patchStrategicMerge":{"metadata":{"labels":{"+(service)":"{{request.object.spec.template.metadata.labels.app}}"}}}}}]}}`), expectedControllers: "none", }, + { + name: "rule-with-only-predefined-valid-controllers", + policy: []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"set-service-labels-env"},"annotations":null,"pod-policies.kyverno.io/autogen-controllers":"none","spec":{"background":false,"rules":[{"name":"set-service-label","match":{"resources":{"kinds":["Namespace"]}},"mutate":{"patchStrategicMerge":{"metadata":{"labels":{"+(service)":"{{request.object.spec.template.metadata.labels.app}}"}}}}}]}}`), + expectedControllers: "none", + }, } for _, test := range testCases { @@ -346,7 +351,10 @@ func Test_getControllers(t *testing.T) { err := json.Unmarshal(test.policy, &policy) assert.NilError(t, err) - _, controllers := CanAutoGen(&policy, log.Log) + applyAutoGen, controllers := CanAutoGen(&policy, log.Log) + if !applyAutoGen { + controllers = "none" + } assert.Equal(t, test.expectedControllers, controllers, fmt.Sprintf("test %s failed", test.name)) } } diff --git a/pkg/webhooks/policymutation_test.go b/pkg/webhooks/policymutation_test.go index 96ec186238..43fb6f2adb 100644 --- a/pkg/webhooks/policymutation_test.go +++ b/pkg/webhooks/policymutation_test.go @@ -46,7 +46,7 @@ func TestGeneratePodControllerRule_NilAnnotation(t *testing.T) { "metadata": { "name": "add-safe-to-evict", "annotations": { - "pod-policies.kyverno.io/autogen-controllers": "DaemonSet,Deployment,Job,StatefulSet,CronJob" + "pod-policies.kyverno.io/autogen-controllers": "none" } } }`) @@ -69,7 +69,7 @@ func TestGeneratePodControllerRule_PredefinedAnnotation(t *testing.T) { assert.Assert(t, json.Unmarshal(policyRaw, &policy)) patches, errs := policymutation.GeneratePodControllerRule(policy, log.Log) assert.Assert(t, len(errs) == 0) - assert.Assert(t, len(patches) == 0) + assert.Assert(t, len(patches) == 1) } func TestGeneratePodControllerRule_DisableFeature(t *testing.T) { @@ -315,7 +315,7 @@ func TestGeneratePodControllerRule_ExistOtherAnnotation(t *testing.T) { "metadata": { "name": "add-safe-to-evict", "annotations": { - "pod-policies.kyverno.io/autogen-controllers": "DaemonSet,Deployment,Job,StatefulSet,CronJob", + "pod-policies.kyverno.io/autogen-controllers": "none", "test": "annotation" } }