fix: apply generate existing when a new rule is added (#6472)

* trigger generate existing when a new rule is added

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* add a kuttl test

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* refactor

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* ignore existing rule updates

Signed-off-by: ShutingZhao <shuting@nirmata.com>

---------

Signed-off-by: ShutingZhao <shuting@nirmata.com>

pkg/policy/generate.go

@@ -12,60 +12,57 @@ import (
 	"github.com/kyverno/kyverno/pkg/config"
 	"go.uber.org/multierr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/labels"
 )
 
 func (pc *PolicyController) handleGenerate(policyKey string, policy kyvernov1.PolicyInterface) error {
-	var errors []error
 	logger := pc.log.WithName("handleGenerate").WithName(policyKey)
 	logger.Info("update URs on policy event")
 
 	for _, rule := range policy.GetSpec().Rules {
-		if err := pc.createURForDataRule(policy, rule, false); err != nil {
+		downstreamExist, err := pc.createURForDataRule(policy, rule, false)
+		if err != nil {
 			logger.Error(err, "failed to create UR on policy event")
+			return err
+		}
+
+		// if there's corresponding exist downstream resources, the rule has been applied
+		// no need to apply generateExisting again
+		if downstreamExist {
+			continue
 		}
 
-		var ruleType kyvernov1beta1.RequestType
 		if policy.GetSpec().IsGenerateExisting() {
-			ruleType = kyvernov1beta1.Generate
-			triggers := generateTriggers(pc.client, rule, pc.log)
-			for _, trigger := range triggers {
-				gurs := pc.listGenerateURs(policyKey, trigger)
-				if gurs != nil {
-					logger.V(4).Info("UR was created", "rule", rule.Name, "rule type", ruleType, "trigger", trigger.GetNamespace()+"/"+trigger.GetName())
-					continue
-				}
-
-				ur := newUR(policy, common.ResourceSpecFromUnstructured(*trigger), rule.Name, ruleType, false)
-				skip, err := pc.handleUpdateRequest(ur, trigger, rule, policy)
-				if err != nil {
-					pc.log.Error(err, "failed to create new UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType,
-						"target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName()))
-					errors = append(errors, err)
-					continue
-				}
-
-				if skip {
-					continue
-				}
-
-				logger.V(4).Info("successfully created UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType,
-					"target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName()))
+			if err := pc.handleGenerateForExisting(policy, rule); err != nil {
+				logger.Error(err, "failed to create UR for generateExisting")
+				return err
 			}
-			err := multierr.Combine(errors...)
-			return err
 		}
 	}
 	return nil
 }
 
-func (pc *PolicyController) listGenerateURs(policyKey string, trigger *unstructured.Unstructured) []*kyvernov1beta1.UpdateRequest {
-	generateURs, err := pc.urLister.List(labels.SelectorFromSet(common.GenerateLabelsSet(policyKey, trigger)))
-	if err != nil {
-		pc.log.Error(err, "failed to list update request for generate policy")
+func (pc *PolicyController) handleGenerateForExisting(policy kyvernov1.PolicyInterface, rule kyvernov1.Rule) error {
+	var errors []error
+	ruleType := kyvernov1beta1.Generate
+	triggers := generateTriggers(pc.client, rule, pc.log)
+	for _, trigger := range triggers {
+		ur := newUR(policy, common.ResourceSpecFromUnstructured(*trigger), rule.Name, ruleType, false)
+		skip, err := pc.handleUpdateRequest(ur, trigger, rule, policy)
+		if err != nil {
+			pc.log.Error(err, "failed to create new UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType,
+				"target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName()))
+			errors = append(errors, err)
+			continue
+		}
+
+		if skip {
+			continue
+		}
+
+		pc.log.V(4).Info("successfully created UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType,
+			"target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName()))
 	}
-	return generateURs
+	return multierr.Combine(errors...)
 }
 
 func (pc *PolicyController) createURForDownstreamDeletion(policy kyvernov1.PolicyInterface) error {
@@ -74,7 +71,7 @@ func (pc *PolicyController) createURForDownstreamDeletion(policy kyvernov1.Polic
 	for _, r := range rules {
 		generateType, sync := r.GetGenerateTypeAndSync()
 		if sync && (generateType == kyvernov1.Data) {
-			if err := pc.createURForDataRule(policy, r, true); err != nil {
+			if _, err := pc.createURForDataRule(policy, r, true); err != nil {
 				errs = append(errs, err)
 			}
 		}
@@ -82,19 +79,25 @@ func (pc *PolicyController) createURForDownstreamDeletion(policy kyvernov1.Polic
 	return multierr.Combine(errs...)
 }
 
-func (pc *PolicyController) createURForDataRule(policy kyvernov1.PolicyInterface, rule kyvernov1.Rule, deleteDownstream bool) error {
+func (pc *PolicyController) createURForDataRule(policy kyvernov1.PolicyInterface, rule kyvernov1.Rule, deleteDownstream bool) (bool, error) {
+	downstreamExist := false
 	generate := rule.Generation
 	if !generate.Synchronize {
 		// no action for non-sync policy/rule
-		return nil
+		return downstreamExist, nil
 	}
 	var errorList []error
 	if generate.GetData() != nil {
 		downstreams, err := generateutils.FindDownstream(pc.client, policy, rule)
 		if err != nil {
-			return err
+			return downstreamExist, err
 		}
 
+		if len(downstreams.Items) == 0 {
+			return downstreamExist, nil
+		}
+
+		downstreamExist = true
 		for _, downstream := range downstreams.Items {
 			labels := downstream.GetLabels()
 			trigger := generateutils.TriggerFromLabels(labels)
@@ -113,7 +116,7 @@ func (pc *PolicyController) createURForDataRule(policy kyvernov1.PolicyInterface
 			}
 		}
 	}
-	return multierr.Combine(errorList...)
+	return downstreamExist, multierr.Combine(errorList...)
 }
 
 // ruleDeletion returns true if any rule is deleted, along with deleted rules

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/04-checks.yaml

@@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+assert:
+- netpol-blue.yaml
+error:
+- netpol-yellow.yaml
+- netpol-summer.yaml

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/05-add-rule.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- add-rule.yaml
+assert:
+- policy-ready.yaml

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/06-sleep.yaml

@@ -0,0 +1,5 @@
+# A command can only run a single command, not a pipeline and not a script. The program called must exist on the system where the test is run.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: sleep 5

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/07-checks.yaml

@@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+assert:
+- netpol-blue.yaml
+- netpol-yellow.yaml
+error:
+- netpol-summer.yaml

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/README.md

@@ -0,0 +1,11 @@
+## Description
+
+This is a basic creation test for a "generate existing" policy. It checks that the basic functionality works whereby creation of a new rule causes correct evaluation of the match block resulting in generation of resources in only the matching result.
+
+## Expected Behavior
+
+If both `blue-ns` and `yellow-ns` Namespaces receive a generated NetworkPolicy, and `summer-ns` does not receive a NetworkPolicies, the test passes. Otherwise the test fails.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/6471

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/add-rule.yaml

@@ -0,0 +1,55 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: existing-basic-add-rule-data
+spec:
+  generateExisting: true
+  rules:
+  - name: existing-basic-create-rule-data
+    match:
+      any:
+      - resources:
+          kinds:
+          - Namespace
+          selector:
+            matchLabels:
+              color: blue
+    generate:
+      kind: NetworkPolicy
+      apiVersion: networking.k8s.io/v1
+      name: default-deny
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      data:
+        metadata:
+          labels:
+            created-by: kyverno
+        spec:
+          podSelector: {}
+          policyTypes:
+          - Ingress
+          - Egress
+  - name: existing-basic-add-rule
+    match:
+      any:
+      - resources:
+          kinds:
+          - Namespace
+          selector:
+            matchLabels:
+              color: yellow
+    generate:
+      kind: NetworkPolicy
+      apiVersion: networking.k8s.io/v1
+      name: default-deny
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      data:
+        metadata:
+          labels:
+            created-by: kyverno
+        spec:
+          podSelector: {}
+          policyTypes:
+          - Ingress
+          - Egress

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/existing-resources.yaml

@@ -0,0 +1,20 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: blue-ns
+  labels:
+    color: blue
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: yellow-ns
+  labels:
+    color: yellow
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: summer-ns
+  labels:
+    season: summer

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/netpol-blue.yaml

@@ -0,0 +1,12 @@
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  labels:
+    created-by: kyverno
+  name: default-deny
+  namespace: blue-ns
+spec:
+  podSelector: {}
+  policyTypes:
+  - Ingress
+  - Egress

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/netpol-summer.yaml

@@ -0,0 +1,12 @@
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  labels:
+    created-by: kyverno
+  name: default-deny
+  namespace: summer-ns
+spec:
+  podSelector: {}
+  policyTypes:
+  - Ingress
+  - Egress

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/netpol-yellow.yaml

@@ -0,0 +1,12 @@
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+  labels:
+    created-by: kyverno
+  name: default-deny
+  namespace: yellow-ns
+spec:
+  podSelector: {}
+  policyTypes:
+  - Ingress
+  - Egress

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/policy-ready.yaml

@@ -1,7 +1,7 @@
 apiVersion: kyverno.io/v1
 kind: ClusterPolicy
 metadata:
-  name: existing-basic-create-data-policy
+  name: existing-basic-add-rule-data
 status:
   conditions:
   - reason: Succeeded

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-add-rule-data/policy.yaml

@@ -0,0 +1,31 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: existing-basic-add-rule-data
+spec:
+  generateExisting: true
+  rules:
+  - name: existing-basic-create-rule
+    match:
+      any:
+      - resources:
+          kinds:
+          - Namespace
+          selector:
+            matchLabels:
+              color: blue
+    generate:
+      kind: NetworkPolicy
+      apiVersion: networking.k8s.io/v1
+      name: default-deny
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      data:
+        metadata:
+          labels:
+            created-by: kyverno
+        spec:
+          podSelector: {}
+          policyTypes:
+          - Ingress
+          - Egress

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-data/03-sleep.yaml

@@ -0,0 +1,5 @@
+# A command can only run a single command, not a pipeline and not a script. The program called must exist on the system where the test is run.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: sleep 5

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-data/policy-ready.yaml

@@ -1,7 +1,7 @@
 apiVersion: kyverno.io/v1
 kind: ClusterPolicy
 metadata:
-  name: existing-basic-create-data-preconditions-policy
+  name: existing-basic-create-policy-data
 status:
   conditions:
   - reason: Succeeded

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-data/policy.yaml

@@ -1,7 +1,7 @@
 apiVersion: kyverno.io/v1
 kind: ClusterPolicy
 metadata:
-  name: existing-basic-create-data-policy
+  name: existing-basic-create-policy-data
 spec:
   generateExisting: true
   rules:

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-preconditions-data/01-existing.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- existing-resources.yaml

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-preconditions-data/02-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-ready.yaml

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-preconditions-data/policy-ready.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: existing-basic-create-policy-preconditions-data
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/kuttl/generate/clusterpolicy/standard/existing/existing-basic-create-policy-preconditions-data/policy.yaml

@@ -1,7 +1,7 @@
 apiVersion: kyverno.io/v1
 kind: ClusterPolicy
 metadata:
-  name: existing-basic-create-data-preconditions-policy
+  name: existing-basic-create-policy-preconditions-data
 spec:
   generateExisting: true
   rules: