From ea977b259c9bfd5200b317bd810110b200708919 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
 <charled.breteche@gmail.com>
Date: Wed, 9 Mar 2022 14:48:04 +0100
Subject: [PATCH] refactor: move controller autogen annotation in api package
 (#3364)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: configmap resource filters generated by helm does not account for namespace

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* refator: move controller autogen annotation in api package

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
---
 api/kyverno/v1/constants.go          |  6 ++++++
 pkg/autogen/autogen.go               | 10 +++++++--
 pkg/autogen/autogen_test.go          | 31 ++++++++++++++--------------
 pkg/autogen/rule.go                  |  7 +++----
 pkg/autogen/utils.go                 |  7 +++----
 pkg/engine/mutation.go               |  9 --------
 pkg/engine/utils.go                  |  2 +-
 pkg/kyverno/common/common.go         |  4 ++--
 pkg/policy/policy_controller.go      |  2 +-
 pkg/policymutation/policymutation.go |  5 ++---
 10 files changed, 41 insertions(+), 42 deletions(-)
 create mode 100644 api/kyverno/v1/constants.go

diff --git a/api/kyverno/v1/constants.go b/api/kyverno/v1/constants.go
new file mode 100644
index 0000000000..fcff2b165a
--- /dev/null
+++ b/api/kyverno/v1/constants.go
@@ -0,0 +1,6 @@
+package v1
+
+const (
+	//PodControllersAnnotation defines the annotation key for Pod-Controllers
+	PodControllersAnnotation = "pod-policies.kyverno.io/autogen-controllers"
+)
diff --git a/pkg/autogen/autogen.go b/pkg/autogen/autogen.go
index 8d1a962ba8..62d9b83d6d 100644
--- a/pkg/autogen/autogen.go
+++ b/pkg/autogen/autogen.go
@@ -8,7 +8,13 @@ import (
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/engine"
+)
+
+const (
+	// PodControllerCronJob represent CronJob string
+	PodControllerCronJob = "CronJob"
+	//PodControllers stores the list of Pod-controllers in csv string
+	PodControllers = "DaemonSet,Deployment,Job,StatefulSet,CronJob"
 )
 
 // CanAutoGen checks whether the rule(s) (in policy) can be applied to Pod controllers
@@ -95,7 +101,7 @@ func CanAutoGen(spec *kyverno.Spec, log logr.Logger) (applyAutoGen bool, control
 		return false, ""
 	}
 
-	return true, engine.PodControllers
+	return true, PodControllers
 }
 
 // podControllersKey annotation could be:
diff --git a/pkg/autogen/autogen_test.go b/pkg/autogen/autogen_test.go
index b650ff3eb0..1ee278ac33 100644
--- a/pkg/autogen/autogen_test.go
+++ b/pkg/autogen/autogen_test.go
@@ -10,7 +10,6 @@ import (
 	"testing"
 
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/engine"
 	"github.com/kyverno/kyverno/pkg/utils"
 	"gotest.tools/assert"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -45,7 +44,7 @@ func Test_getControllers(t *testing.T) {
 		{
 			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"}]}}}]}}`),
-			expectedControllers: engine.PodControllers,
+			expectedControllers: PodControllers,
 		},
 		{
 			name:                "rule-with-match-mixed-kinds-pod-podcontrollers",
@@ -60,12 +59,12 @@ func Test_getControllers(t *testing.T) {
 		{
 			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"}}}}}]}}`),
-			expectedControllers: engine.PodControllers,
+			expectedControllers: PodControllers,
 		},
 		{
 			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"}}}}}]}}`),
-			expectedControllers: engine.PodControllers,
+			expectedControllers: PodControllers,
 		},
 		{
 			name:                "rule-with-mutate-patches",
@@ -129,7 +128,7 @@ func Test_Any(t *testing.T) {
 		},
 	}
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	fmt.Println("utils.JoinPatches(patches)erterter", string(utils.JoinPatches(rulePatches)))
 	if len(errs) != 0 {
 		t.Log(errs)
@@ -167,7 +166,7 @@ func Test_All(t *testing.T) {
 		},
 	}
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	if len(errs) != 0 {
 		t.Log(errs)
 	}
@@ -199,7 +198,7 @@ func Test_Exclude(t *testing.T) {
 	policy := policies[0]
 	policy.Spec.Rules[0].ExcludeResources.Namespaces = []string{"fake-namespce"}
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	if len(errs) != 0 {
 		t.Log(errs)
 	}
@@ -217,7 +216,7 @@ func Test_Exclude(t *testing.T) {
 
 func Test_CronJobOnly(t *testing.T) {
 
-	controllers := engine.PodControllerCronJob
+	controllers := PodControllerCronJob
 	dir, err := os.Getwd()
 	baseDir := filepath.Dir(filepath.Dir(dir))
 	assert.NilError(t, err)
@@ -232,7 +231,7 @@ func Test_CronJobOnly(t *testing.T) {
 
 	policy := policies[0]
 	policy.SetAnnotations(map[string]string{
-		engine.PodControllersAnnotation: controllers,
+		kyverno.PodControllersAnnotation: controllers,
 	})
 
 	rulePatches, errs := GenerateRulePatches(&policy.Spec, controllers, log.Log)
@@ -263,7 +262,7 @@ func Test_ForEachPod(t *testing.T) {
 	policy := policies[0]
 	policy.Spec.Rules[0].ExcludeResources.Namespaces = []string{"fake-namespce"}
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	if len(errs) != 0 {
 		t.Log(errs)
 	}
@@ -281,7 +280,7 @@ func Test_ForEachPod(t *testing.T) {
 
 func Test_CronJob_hasExclude(t *testing.T) {
 
-	controllers := engine.PodControllerCronJob
+	controllers := PodControllerCronJob
 	dir, err := os.Getwd()
 	baseDir := filepath.Dir(filepath.Dir(dir))
 	assert.NilError(t, err)
@@ -297,7 +296,7 @@ func Test_CronJob_hasExclude(t *testing.T) {
 
 	policy := policies[0]
 	policy.SetAnnotations(map[string]string{
-		engine.PodControllersAnnotation: controllers,
+		kyverno.PodControllersAnnotation: controllers,
 	})
 
 	rule := policy.Spec.Rules[0].DeepCopy()
@@ -318,7 +317,7 @@ func Test_CronJob_hasExclude(t *testing.T) {
 }
 
 func Test_CronJobAndDeployment(t *testing.T) {
-	controllers := strings.Join([]string{engine.PodControllerCronJob, "Deployment"}, ",")
+	controllers := strings.Join([]string{PodControllerCronJob, "Deployment"}, ",")
 	dir, err := os.Getwd()
 	baseDir := filepath.Dir(filepath.Dir(dir))
 	assert.NilError(t, err)
@@ -333,7 +332,7 @@ func Test_CronJobAndDeployment(t *testing.T) {
 
 	policy := policies[0]
 	policy.SetAnnotations(map[string]string{
-		engine.PodControllersAnnotation: controllers,
+		kyverno.PodControllersAnnotation: controllers,
 	})
 
 	rulePatches, errs := GenerateRulePatches(&policy.Spec, controllers, log.Log)
@@ -364,7 +363,7 @@ func Test_UpdateVariablePath(t *testing.T) {
 
 	policy := policies[0]
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	if len(errs) != 0 {
 		t.Log(errs)
 	}
@@ -398,7 +397,7 @@ func Test_Deny(t *testing.T) {
 		},
 	}
 
-	rulePatches, errs := GenerateRulePatches(&policy.Spec, engine.PodControllers, log.Log)
+	rulePatches, errs := GenerateRulePatches(&policy.Spec, PodControllers, log.Log)
 	fmt.Println("utils.JoinPatches(patches)erterter", string(utils.JoinPatches(rulePatches)))
 	if len(errs) != 0 {
 		t.Log(errs)
diff --git a/pkg/autogen/rule.go b/pkg/autogen/rule.go
index 28106334bd..e74b8d4ab7 100644
--- a/pkg/autogen/rule.go
+++ b/pkg/autogen/rule.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/engine"
 	"github.com/kyverno/kyverno/pkg/engine/variables"
 	"github.com/kyverno/kyverno/pkg/utils"
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -262,7 +261,7 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr.
 func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger) *kyvernoRule {
 	logger := log.WithName("handleCronJob")
 
-	hasCronJob := strings.Contains(controllers, engine.PodControllerCronJob) || strings.Contains(controllers, "all")
+	hasCronJob := strings.Contains(controllers, PodControllerCronJob) || strings.Contains(controllers, "all")
 	if !hasCronJob {
 		return nil
 	}
@@ -289,7 +288,7 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger)
 		rule := cronJobAnyAllAutogenRule(cronJobRule.MatchResources.All)
 		cronJobRule.MatchResources.All = rule
 	} else {
-		cronJobRule.MatchResources.Kinds = []string{engine.PodControllerCronJob}
+		cronJobRule.MatchResources.Kinds = []string{PodControllerCronJob}
 	}
 
 	if (jobRule.ExcludeResources) != nil && len(jobRule.ExcludeResources.Any) > 0 {
@@ -300,7 +299,7 @@ func generateCronJobRule(rule kyverno.Rule, controllers string, log logr.Logger)
 		cronJobRule.ExcludeResources.All = rule
 	} else {
 		if (jobRule.ExcludeResources) != nil && (len(jobRule.ExcludeResources.Kinds) > 0) {
-			cronJobRule.ExcludeResources.Kinds = []string{engine.PodControllerCronJob}
+			cronJobRule.ExcludeResources.Kinds = []string{PodControllerCronJob}
 		}
 	}
 
diff --git a/pkg/autogen/utils.go b/pkg/autogen/utils.go
index bd55694215..e6a02be609 100644
--- a/pkg/autogen/utils.go
+++ b/pkg/autogen/utils.go
@@ -4,7 +4,6 @@ import (
 	"strings"
 
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
-	"github.com/kyverno/kyverno/pkg/engine"
 	"github.com/kyverno/kyverno/pkg/utils"
 )
 
@@ -17,7 +16,7 @@ func isKindOtherthanPod(kinds []string) bool {
 
 func hasAutogenKinds(kind []string) bool {
 	for _, v := range kind {
-		if v == "Pod" || strings.Contains(engine.PodControllers, v) {
+		if v == "Pod" || strings.Contains(PodControllers, v) {
 			return true
 		}
 	}
@@ -52,7 +51,7 @@ func stripCronJob(controllers string) string {
 	var newControllers []string
 	controllerArr := strings.Split(controllers, ",")
 	for _, c := range controllerArr {
-		if c == engine.PodControllerCronJob {
+		if c == PodControllerCronJob {
 			continue
 		}
 		newControllers = append(newControllers, c)
@@ -67,7 +66,7 @@ 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}
+			anyKind[i].Kinds = []string{PodControllerCronJob}
 		}
 	}
 	return anyKind
diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go
index 8b0870925c..386d1ee199 100644
--- a/pkg/engine/mutation.go
+++ b/pkg/engine/mutation.go
@@ -17,15 +17,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-const (
-	// PodControllerCronJob represent CronJob string
-	PodControllerCronJob = "CronJob"
-	//PodControllers stores the list of Pod-controllers in csv string
-	PodControllers = "DaemonSet,Deployment,Job,StatefulSet,CronJob"
-	//PodControllersAnnotation defines the annotation key for Pod-Controllers
-	PodControllersAnnotation = "pod-policies.kyverno.io/autogen-controllers"
-)
-
 // Mutate performs mutation. Overlay first and then mutation patches
 func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) {
 	resp = &response.EngineResponse{}
diff --git a/pkg/engine/utils.go b/pkg/engine/utils.go
index ba852805ae..16c8c37087 100644
--- a/pkg/engine/utils.go
+++ b/pkg/engine/utils.go
@@ -428,7 +428,7 @@ func excludeResource(podControllers string, resource unstructured.Unstructured)
 // - if the policy has auto-gen annotation && resource == Pod
 // - if the auto-gen contains cronJob && resource == Job
 func ManagedPodResource(policy kyverno.ClusterPolicy, resource unstructured.Unstructured) bool {
-	podControllers, ok := policy.GetAnnotations()[PodControllersAnnotation]
+	podControllers, ok := policy.GetAnnotations()[kyverno.PodControllersAnnotation]
 	if !ok || strings.ToLower(podControllers) == "none" {
 		return false
 	}
diff --git a/pkg/kyverno/common/common.go b/pkg/kyverno/common/common.go
index 63c1014ce7..12d285d934 100644
--- a/pkg/kyverno/common/common.go
+++ b/pkg/kyverno/common/common.go
@@ -566,8 +566,8 @@ OuterLoop:
 
 	if resource.GetKind() == "Pod" && len(resource.GetOwnerReferences()) > 0 {
 		if policy.HasAutoGenAnnotation() {
-			if _, ok := policy.GetAnnotations()[engine.PodControllersAnnotation]; ok {
-				delete(policy.Annotations, engine.PodControllersAnnotation)
+			if _, ok := policy.GetAnnotations()[v1.PodControllersAnnotation]; ok {
+				delete(policy.Annotations, v1.PodControllersAnnotation)
 			}
 		}
 	}
diff --git a/pkg/policy/policy_controller.go b/pkg/policy/policy_controller.go
index 6fbd4f98a8..7f6304d722 100644
--- a/pkg/policy/policy_controller.go
+++ b/pkg/policy/policy_controller.go
@@ -571,7 +571,7 @@ func missingAutoGenRules(policy *kyverno.ClusterPolicy, log logr.Logger) bool {
 
 	if len(podRuleName) > 0 {
 		annotations := policy.GetAnnotations()
-		val, ok := annotations["pod-policies.kyverno.io/autogen-controllers"]
+		val, ok := annotations[kyverno.PodControllersAnnotation]
 		if !ok {
 			return true
 		}
diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go
index b3b4a8fee8..e4ab26c774 100644
--- a/pkg/policymutation/policymutation.go
+++ b/pkg/policymutation/policymutation.go
@@ -10,7 +10,6 @@ import (
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/autogen"
 	"github.com/kyverno/kyverno/pkg/common"
-	"github.com/kyverno/kyverno/pkg/engine"
 	"github.com/kyverno/kyverno/pkg/utils"
 )
 
@@ -257,7 +256,7 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p
 	}
 
 	ann := policy.GetAnnotations()
-	actualControllers, ok := ann[engine.PodControllersAnnotation]
+	actualControllers, ok := ann[kyverno.PodControllersAnnotation]
 
 	// - scenario A
 	// - predefined controllers are invalid, overwrite the value
@@ -293,7 +292,7 @@ func GeneratePodControllerRule(policy kyverno.ClusterPolicy, log logr.Logger) (p
 func defaultPodControllerAnnotation(ann map[string]string, controllers string) ([]byte, error) {
 	if ann == nil {
 		ann = make(map[string]string)
-		ann[engine.PodControllersAnnotation] = controllers
+		ann[kyverno.PodControllersAnnotation] = controllers
 		jsonPatch := struct {
 			Path  string      `json:"path"`
 			Op    string      `json:"op"`