Repository: kyverno/kyverno
Pull request #1723 (2021-03-19 18:16:42)

Skip generate requests for spec being same in old and updated policy
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>

## Related issue
closes https://github.com/kyverno/kyverno/issues/1720

## What type of PR is this
> /kind bug

## Proposed changes
Skipping enqueueing generate requests for the policies where the spec is same in old and updated policies.

## Checklist

- [X] I have read the [contributing guidelines](https://github.com/kyverno/kyverno/blob/main/CONTRIBUTING.md).
- [] I have added tests that prove my fix is effective or that my feature works.
- [] I have added or changed [the documentation](https://github.com/kyverno/website).
  - If not, I have raised an issue in [kyverno/website](https://github.com/kyverno/website) to track the doc update:
  <!-- Uncomment to link to the issue -->
  <!-- https://github.com/kyverno/website/issues/1 -->



Patch:
diff --git a/pkg/engine/anchor/anchor.go b/pkg/engine/anchor/anchor.go
index 5d62f19cf..57209efc4 100644
--- a/pkg/engine/anchor/anchor.go
+++ b/pkg/engine/anchor/anchor.go
@@ -189,13 +189,20 @@ func (eh ExistenceHandler) Handle(handler resourceElementHandler, resourceMap ma
 			if !ok {
 				return currentPath, fmt.Errorf("Invalid pattern type %T: Pattern has to be of list to compare against resource", eh.pattern)
 			}
-			// get the first item in the pattern array
-			patternMap := typedPattern[0]
-			typedPatternMap, ok := patternMap.(map[string]interface{})
-			if !ok {
-				return currentPath, fmt.Errorf("Invalid pattern type %T: Pattern has to be of type map to compare against items in resource", eh.pattern)
+			// loop all item in the pattern array
+			errorPath := ""
+			var err error
+			for _, patternMap := range typedPattern {
+				typedPatternMap, ok := patternMap.(map[string]interface{})
+				if !ok {
+					return currentPath, fmt.Errorf("Invalid pattern type %T: Pattern has to be of type map to compare against items in resource", eh.pattern)
+				}
+				errorPath, err = validateExistenceListResource(handler, typedResource, typedPatternMap, originPattern, currentPath, ac)
+				if err != nil {
+					return errorPath, err
+				}
 			}
-			return validateExistenceListResource(handler, typedResource, typedPatternMap, originPattern, currentPath, ac)
+			return errorPath, err
 		default:
 			return currentPath, fmt.Errorf("Invalid resource type %T: Existence ^ () anchor can be used only on list/array type resource", value)
 		}
@@ -204,7 +211,7 @@ func (eh ExistenceHandler) Handle(handler resourceElementHandler, resourceMap ma
 }
 
 func validateExistenceListResource(handler resourceElementHandler, resourceList []interface{}, patternMap map[string]interface{}, originPattern interface{}, path string, ac *common.AnchorKey) (string, error) {
-	// the idea is atleast on the elements in the array should satisfy the pattern
+	// the idea is all the element in the pattern array should be present atleast once in the resource list
 	// if non satisfy then throw an error
 	for i, resourceElement := range resourceList {
 		currentPath := path + strconv.Itoa(i) + "/"
diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go
index 33b2dfdb6..79bab4f0f 100644
--- a/pkg/engine/variables/vars.go
+++ b/pkg/engine/variables/vars.go
@@ -94,6 +94,16 @@ func subValR(log logr.Logger, ctx context.EvalInterface, valuePattern string, pa
 			variable := strings.ReplaceAll(v, "{{", "")
 			variable = strings.ReplaceAll(variable, "}}", "")
 			variable = strings.TrimSpace(variable)
+
+			var substitutedVar interface{}
+			if operation, err := ctx.Query("request.operation"); err != nil {
+				return nil, fmt.Errorf("failed to resolve %v at path %s", variable, path)
+			} else {
+				if operation == "DELETE" {
+					variable = strings.ReplaceAll(variable, "request.object", "request.oldObject")
+				}
+			}
+
 			substitutedVar, err := ctx.Query(variable)
 			if err != nil {
 				switch err.(type) {
diff --git a/pkg/generate/generate_controller.go b/pkg/generate/generate_controller.go
index dc5051179..782e6f250 100644
--- a/pkg/generate/generate_controller.go
+++ b/pkg/generate/generate_controller.go
@@ -1,6 +1,7 @@
 package generate
 
 import (
+	"reflect"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -175,6 +176,10 @@ func (c *Controller) updatePolicy(old, cur interface{}) {
 		}
 	}
 
+	if reflect.DeepEqual(curP.Spec, oldP.Spec) {
+		policyHasGenerate = false
+	}
+
 	if !policyHasGenerate {
 		return
 	}
diff --git a/pkg/policy/common/validate_pattern.go b/pkg/policy/common/validate_pattern.go
index 212dd3ad6..d14f77e67 100644
--- a/pkg/policy/common/validate_pattern.go
+++ b/pkg/policy/common/validate_pattern.go
@@ -50,9 +50,9 @@ func validateMap(patternMap map[string]interface{}, path string, supportedAnchor
 				if !ok {
 					return path + "/" + key, fmt.Errorf("Existence anchor should have value of type list")
 				}
-				// validate there is only one entry in the list
-				if len(typedValue) == 0 || len(typedValue) > 1 {
-					return path + "/" + key, fmt.Errorf("Existence anchor: single value expected, multiple specified")
+				// validate that there is atleast one entry in the list
+				if len(typedValue) == 0 {
+					return path + "/" + key, fmt.Errorf("Existence anchor: should have atleast one value")
 				}
 			}
 		}


Test patch:
diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go
index d94802902..70ad52b02 100644
--- a/pkg/engine/variables/vars_test.go
+++ b/pkg/engine/variables/vars_test.go
@@ -2,6 +2,7 @@ package variables
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
 	"github.com/kyverno/kyverno/pkg/engine/context"
@@ -131,6 +132,86 @@ func Test_subVars_failed(t *testing.T) {
 	}
 }
 
+func Test_ReplacingPathWhenDeleting(t *testing.T) {
+	patternRaw := []byte(`"{{request.object.metadata.annotations.target}}"`)
+
+	var resourceRaw = []byte(`
+	{
+		"request": {
+			"operation": "DELETE",
+			"object": {
+				"metadata": {
+					"name": "curr",
+					"namespace": "ns",
+					"annotations": {
+					  "target": "foo"
+					}
+				}
+			},
+			"oldObject": {
+				"metadata": {
+					"name": "old",
+					"annotations": {
+					  "target": "bar"
+					}
+				}
+			}
+		}
+	}
+`)
+
+	var pattern interface{}
+	var err error
+	err = json.Unmarshal(patternRaw, &pattern)
+	if err != nil {
+		t.Error(err)
+	}
+	ctx := context.NewContext()
+	err = ctx.AddJSON(resourceRaw)
+	assert.NilError(t, err)
+
+	pattern, err = SubstituteVars(log.Log, ctx, pattern)
+	assert.NilError(t, err)
+
+	assert.Equal(t, fmt.Sprintf("%v", pattern), "bar")
+}
+
+func Test_ReplacingNestedVariableWhenDeleting(t *testing.T) {
+	patternRaw := []byte(`"{{request.object.metadata.annotations.{{request.object.metadata.annotations.targetnew}}}}"`)
+
+	var resourceRaw = []byte(`
+	{
+		"request":{
+		   "operation":"DELETE",
+		   "oldObject":{
+			  "metadata":{
+				 "name":"current",
+				 "namespace":"ns",
+				 "annotations":{
+					"target":"nested_target",
+					"targetnew":"target"
+				 }
+			  }
+		   }
+		}
+	}`)
+
+	var pattern interface{}
+	var err error
+	err = json.Unmarshal(patternRaw, &pattern)
+	if err != nil {
+		t.Error(err)
+	}
+	ctx := context.NewContext()
+	err = ctx.AddJSON(resourceRaw)
+	assert.NilError(t, err)
+
+	pattern, err = SubstituteVars(log.Log, ctx, pattern)
+	assert.NilError(t, err)
+
+	assert.Equal(t, fmt.Sprintf("%v", pattern), "nested_target")
+}
+
 var resourceRaw = []byte(`
 	{
 		"metadata": {