Bug 1799: Fix mutate policy defaults and Fix endless look of auto-gen rules.

pkg/policy/validate_controller.go

@@ -5,9 +5,7 @@ import (
 	"crypto/rand"
 	"fmt"
 	"math/big"
-	random "math/rand"
 	"reflect"
-	"strconv"
 	"strings"
 	"time"
 
@@ -20,6 +18,8 @@ import (
 	"github.com/kyverno/kyverno/pkg/config"
 	client "github.com/kyverno/kyverno/pkg/dclient"
 	"github.com/kyverno/kyverno/pkg/event"
+	"github.com/kyverno/kyverno/pkg/kyverno/common"
+	pm "github.com/kyverno/kyverno/pkg/policymutation"
 	"github.com/kyverno/kyverno/pkg/policyreport"
 	"github.com/kyverno/kyverno/pkg/resourcecache"
 	utils "github.com/kyverno/kyverno/pkg/utils"
@@ -197,10 +197,10 @@ func (pc *PolicyController) addPolicy(obj interface{}) {
 
 	logger.Info("policy created", "uid", p.UID, "kind", "ClusterPolicy", "name", p.Name)
 
-	if p.Spec.Background == nil || p.Spec.ValidationFailureAction == "" || checkAutoGenRules(p) {
-		p.ObjectMeta.SetAnnotations(map[string]string{"kyverno.io/mutate-policy": strconv.Itoa(random.Intn(100))})
+	if p.Spec.Background == nil || p.Spec.ValidationFailureAction == "" || checkAutoGenRules(p, logger) {
+		pol, _ := common.MutatePolicy(p, logger)
 		p.SetGroupVersionKind(schema.GroupVersionKind{Group: "kyverno.io", Version: "v1", Kind: "ClusterPolicy"})
-		_, err := pc.client.UpdateResource("kyverno.io/v1", "ClusterPolicy", "", p, false)
+		_, err := pc.client.UpdateResource("kyverno.io/v1", "ClusterPolicy", "", pol, false)
 		if err != nil {
 			logger.Error(err, "failed to add policy ")
 		}
@@ -219,10 +219,10 @@ func (pc *PolicyController) updatePolicy(old, cur interface{}) {
 	oldP := old.(*kyverno.ClusterPolicy)
 	curP := cur.(*kyverno.ClusterPolicy)
 
-	if curP.Spec.Background == nil || curP.Spec.ValidationFailureAction == "" || checkAutoGenRules(curP) {
-		curP.ObjectMeta.SetAnnotations(map[string]string{"kyverno.io/mutate-policy": strconv.Itoa(random.Intn(100))})
+	if curP.Spec.Background == nil || curP.Spec.ValidationFailureAction == "" || checkAutoGenRules(curP, logger) {
+		pol, _ := common.MutatePolicy(curP, logger)
 		curP.SetGroupVersionKind(schema.GroupVersionKind{Group: "kyverno.io", Version: "v1", Kind: "ClusterPolicy"})
-		_, err := pc.client.UpdateResource("kyverno.io/v1", "ClusterPolicy", "", curP, false)
+		_, err := pc.client.UpdateResource("kyverno.io/v1", "ClusterPolicy", "", pol, false)
 		if err != nil {
 			logger.Error(err, "failed to update policy ")
 		}
@@ -274,10 +274,10 @@ func (pc *PolicyController) addNsPolicy(obj interface{}) {
 	logger.Info("policy created", "uid", p.UID, "kind", "Policy", "name", p.Name, "namespaces", p.Namespace)
 
 	pol := ConvertPolicyToClusterPolicy(p)
-	if pol.Spec.Background == nil || pol.Spec.ValidationFailureAction == "" || checkAutoGenRules(pol) {
-		pol.ObjectMeta.SetAnnotations(map[string]string{"kyverno.io/mutate-policy": strconv.Itoa(random.Intn(100))})
+	if pol.Spec.Background == nil || pol.Spec.ValidationFailureAction == "" || checkAutoGenRules(pol, logger) {
+		nsPol, _ := common.MutatePolicy(pol, logger)
 		pol.SetGroupVersionKind(schema.GroupVersionKind{Group: "kyverno.io", Version: "v1", Kind: "Policy"})
-		_, err := pc.client.UpdateResource("kyverno.io/v1", "Policy", p.Namespace, pol, false)
+		_, err := pc.client.UpdateResource("kyverno.io/v1", "Policy", p.Namespace, nsPol, false)
 		if err != nil {
 			logger.Error(err, "failed to add namespace policy")
 		}
@@ -295,10 +295,10 @@ func (pc *PolicyController) updateNsPolicy(old, cur interface{}) {
 	curP := cur.(*kyverno.Policy)
 	ncurP := ConvertPolicyToClusterPolicy(curP)
 
-	if ncurP.Spec.Background == nil || ncurP.Spec.ValidationFailureAction == "" || checkAutoGenRules(ncurP) {
-		ncurP.ObjectMeta.SetAnnotations(map[string]string{"kyverno.io/mutate-policy": strconv.Itoa(random.Intn(100))})
+	if ncurP.Spec.Background == nil || ncurP.Spec.ValidationFailureAction == "" || checkAutoGenRules(ncurP, logger) {
+		nsPol, _ := common.MutatePolicy(ncurP, logger)
 		ncurP.SetGroupVersionKind(schema.GroupVersionKind{Group: "kyverno.io", Version: "v1", Kind: "Policy"})
-		_, err := pc.client.UpdateResource("kyverno.io/v1", "Policy", ncurP.GetNamespace(), ncurP, false)
+		_, err := pc.client.UpdateResource("kyverno.io/v1", "Policy", ncurP.GetNamespace(), nsPol, false)
 		if err != nil {
 			logger.Error(err, "failed to update namespace policy ")
 		}
@@ -520,11 +520,11 @@ func updateGR(kyvernoClient *kyvernoclient.Clientset, policyKey string, grList [
 	}
 }
 
-func checkAutoGenRules(policy *kyverno.ClusterPolicy) bool {
+func checkAutoGenRules(policy *kyverno.ClusterPolicy, log logr.Logger) bool {
 	var podRuleName []string
 	ruleCount := 1
-	for _, rule := range policy.Spec.Rules {
-		if utils.ContainsString(rule.MatchResources.ResourceDescription.Kinds, "Pod") {
+	if pm.GetControllers(policy, log) != "none" {
+		for _, rule := range policy.Spec.Rules {
 			podRuleName = append(podRuleName, rule.Name)
 		}
 	}

pkg/policy/validate_test.go

@@ -2,10 +2,12 @@ package policy
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/kyverno/kyverno/pkg/openapi"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	"gotest.tools/assert"
@@ -1237,3 +1239,56 @@ func Test_doesMatchExcludeConflict(t *testing.T) {
 		}
 	}
 }
+
+func Test_checkAutoGenRules(t *testing.T) {
+	testCases := []struct {
+		name           string
+		policy         []byte
+		expectedResult bool
+	}{
+		{
+			name:           "rule-with-match-name",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"test","match":{"resources":{"kinds":["Namespace"],"name":"*"}}}]}}`),
+			expectedResult: false,
+		},
+		{
+			name:           "rule-with-match-selector",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"],"selector":{"matchLabels":{"foo":"bar"}}}}}]}}`),
+			expectedResult: false,
+		},
+		{
+			name:           "rule-with-exclude-name",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"name":"test"}}}]}}`),
+			expectedResult: false,
+		},
+		{
+			name:           "rule-with-exclude-selector",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test-getcontrollers"},"spec":{"background":false,"rules":[{"name":"test-getcontrollers","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"selector":{"matchLabels":{"foo":"bar"}}}}}]}}`),
+			expectedResult: false,
+		},
+		{
+			name:           "rule-with-deny",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"validate":{"message":"testpolicy","deny":{"conditions":[{"key":"{{request.object.metadata.labels.foo}}","operator":"Equals","value":"bar"}]}}}]}}`),
+			expectedResult: false,
+		},
+		{
+			name:           "rule-with-match-kinds-pod-only",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"validate":{"message":"testpolicy","pattern":{"metadata":{"labels":{"foo":"bar"}}}}}]}}`),
+			expectedResult: true,
+		},
+		{
+			name:           "rule-with-exclude-kinds-pod-only",
+			policy:         []byte(`{"apiVersion":"kyverno.io/v1","kind":"ClusterPolicy","metadata":{"name":"test"},"spec":{"rules":[{"name":"require-network-policy","match":{"resources":{"kinds":["Pod"]}},"exclude":{"resources":{"kinds":["Pod"],"namespaces":["test"]}},"validate":{"message":"testpolicy","pattern":{"metadata":{"labels":{"foo":"bar"}}}}}]}}`),
+			expectedResult: true,
+		},
+	}
+
+	for _, test := range testCases {
+		var policy kyverno.ClusterPolicy
+		err := json.Unmarshal(test.policy, &policy)
+		assert.NilError(t, err)
+
+		res := checkAutoGenRules(&policy, log.Log)
+		assert.Equal(t, test.expectedResult, res, fmt.Sprintf("test %s failed", test.name))
+	}
+}

pkg/policymutation/policymutation.go

@@ -246,6 +246,37 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p
 	return
 }
 
+// getControllers gets applicable pod controllers, it returns
+// - "none" if:
+//          - name or selector is defined
+//          - mixed kinds (Pod + pod controller) is defined
+//          - mutate.Patches/mutate.PatchesJSON6902/validate.deny/generate rule is defined
+// - otherwise it returns all pod controllers
+func GetControllers(policy *kyverno.ClusterPolicy, log logr.Logger) string {
+	for _, rule := range policy.Spec.Rules {
+		match := rule.MatchResources
+		exclude := rule.ExcludeResources
+
+		if match.ResourceDescription.Name != "" || match.ResourceDescription.Selector != nil ||
+			exclude.ResourceDescription.Name != "" || exclude.ResourceDescription.Selector != nil {
+			log.Info("skip generating rule on pod controllers: Name / Selector in resource description may not be applicable.", "rule", rule.Name)
+			return "none"
+		}
+
+		if (len(match.Kinds) > 1 && utils.ContainsString(match.Kinds, "Pod")) ||
+			(len(exclude.Kinds) > 1 && utils.ContainsString(exclude.Kinds, "Pod")) {
+			return "none"
+		}
+
+		if rule.Mutation.Patches != nil || rule.Mutation.PatchesJSON6902 != "" ||
+			rule.Validation.Deny != nil || rule.HasGenerate() {
+			return "none"
+		}
+	}
+
+	return engine.PodControllers
+}
+
 func createRuleMap(rules []kyverno.Rule) map[string]kyvernoRule {
 	var ruleMap = make(map[string]kyvernoRule)
 	for _, rule := range rules {