fix: deletion mismatch for the generate policy (#7579)

* fix deletion mismatch Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix clone source kind Signed-off-by: ShutingZhao <shuting@nirmata.com> * add kuttl test Signed-off-by: ShutingZhao <shuting@nirmata.com> * fetch kinds Signed-off-by: ShutingZhao <shuting@nirmata.com> * add kuttl test Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix Signed-off-by: ShutingZhao <shuting@nirmata.com> * fix Signed-off-by: ShutingZhao <shuting@nirmata.com> * add kuttl test Signed-off-by: ShutingZhao <shuting@nirmata.com> --------- Signed-off-by: ShutingZhao <shuting@nirmata.com>
2025-03-28 02:18:15 +00:00 · 2023-06-20 20:58:23 +08:00 · 2023-06-20 20:58:23 +08:00 · f6b097db17
commit f6b097db17
parent 74f2cb3076
23 changed files with 307 additions and 43 deletions
--- a/api/kyverno/v1/resource_spec_types.go
+++ b/api/kyverno/v1/resource_spec_types.go
@ -5,6 +5,7 @@ import (

 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 )

 type ResourceSpec struct {
@ -25,6 +26,9 @@ func (s ResourceSpec) GetName() string       { return s.Name }
 func (s ResourceSpec) GetNamespace() string  { return s.Namespace }
 func (s ResourceSpec) GetKind() string       { return s.Kind }
 func (s ResourceSpec) GetAPIVersion() string { return s.APIVersion }
+func (s ResourceSpec) GetGroupVersion() (schema.GroupVersion, error) {
+	return schema.ParseGroupVersion(s.APIVersion)
+}

 func (s ResourceSpec) String() string {
 	return strings.Join([]string{s.APIVersion, s.Kind, s.Namespace, s.Name}, "/")
--- a/pkg/background/generate/cleanup.go
+++ b/pkg/background/generate/cleanup.go
@ -10,7 +10,7 @@ import (
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	"go.uber.org/multierr"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )

 func (c *GenerateController) deleteDownstream(policy kyvernov1.PolicyInterface, ur *kyvernov1beta1.UpdateRequest) (err error) {
@ -44,11 +44,11 @@ func (c *GenerateController) deleteDownstream(policy kyvernov1.PolicyInterface,
 	if policy == nil {
 		return nil
 	}
-	// handle clone source deletion
-	return c.deleteDownstreamForClone(policy, ur)
+
+	return c.handleNonPolicyChanges(policy, ur)
 }

-func (c *GenerateController) deleteDownstreamForClone(policy kyvernov1.PolicyInterface, ur *kyvernov1beta1.UpdateRequest) error {
+func (c *GenerateController) handleNonPolicyChanges(policy kyvernov1.PolicyInterface, ur *kyvernov1beta1.UpdateRequest) error {
 	if !ur.Spec.DeleteDownstream {
 		return nil
 	}
@ -64,59 +64,62 @@ func (c *GenerateController) deleteDownstreamForClone(policy kyvernov1.PolicyInt
 			kyvernov1.LabelAppManagedBy:         kyvernov1.ValueKyvernoApp,
 		}

-		sources := []kyvernov1.ResourceSpec{rule.Generation.ResourceSpec}
-		if rule.Generation.CloneList.Kinds != nil {
-			srcs, err := c.getCloneSources(ur, rule)
-			if err != nil {
-				return fmt.Errorf("failed to get clone sources for the cloneList : %v", err)
-			}
-			sources = srcs
+		downstreams, err := c.getDownstreams(rule, labels, ur)
+		if err != nil {
+			return fmt.Errorf("failed to fetch downstream resources: %v", err)
 		}
-
-		for _, source := range sources {
-			downstreams, err := FindDownstream(c.client, source.GetAPIVersion(), source.GetKind(), labels)
-			if err != nil {
-				return err
-			}
-
-			var errs []error
-			failedDownstreams := []kyvernov1.ResourceSpec{}
-			for _, downstream := range downstreams.Items {
-				if err := c.client.DeleteResource(context.TODO(), downstream.GetAPIVersion(), downstream.GetKind(), downstream.GetNamespace(), downstream.GetName(), false); err != nil && !apierrors.IsNotFound(err) {
-					failedDownstreams = append(failedDownstreams, common.ResourceSpecFromUnstructured(downstream))
-					errs = append(errs, err)
-				}
-			}
-			if len(errs) != 0 {
-				c.log.Error(multierr.Combine(errs...), "failed to clean up downstream resources on source deletion")
-				_, err = c.statusControl.Failed(ur.GetName(),
-					fmt.Sprintf("failed to clean up downstream resources on source deletion: %v", multierr.Combine(errs...)),
-					failedDownstreams)
+		var errs []error
+		failedDownstreams := []kyvernov1.ResourceSpec{}
+		for _, downstream := range downstreams.Items {
+			spec := common.ResourceSpecFromUnstructured(downstream)
+			if err := c.client.DeleteResource(context.TODO(), downstream.GetAPIVersion(), downstream.GetKind(), downstream.GetNamespace(), downstream.GetName(), false); err != nil && !apierrors.IsNotFound(err) {
+				failedDownstreams = append(failedDownstreams, spec)
+				errs = append(errs, err)
 			} else {
-				_, err = c.statusControl.Success(ur.GetName(), nil)
-			}
-			if err != nil {
-				c.log.Error(err, "failed to update ur status")
+				c.log.V(4).Info("downstream resource deleted", spec.String())
 			}
 		}
+		if len(errs) != 0 {
+			_, err = c.statusControl.Failed(ur.GetName(),
+				fmt.Sprintf("failed to clean up downstream resources on source deletion: %v", multierr.Combine(errs...)),
+				failedDownstreams)
+		} else {
+			_, err = c.statusControl.Success(ur.GetName(), nil)
+		}
+		if err != nil {
+			c.log.Error(err, "failed to update ur status")
+		}
 	}
+
 	return nil
 }

-func (c *GenerateController) getCloneSources(ur *kyvernov1beta1.UpdateRequest, rule kyvernov1.Rule) (sources []kyvernov1.ResourceSpec, err error) {
-	source, err := c.getTriggerForDeleteOperation(ur.Spec)
+func (c *GenerateController) getDownstreams(rule kyvernov1.Rule, selector map[string]string, ur *kyvernov1beta1.UpdateRequest) (*unstructured.UnstructuredList, error) {
+	gv, err := ur.Spec.GetResource().GetGroupVersion()
 	if err != nil {
 		return nil, err
 	}

-	labels := source.GetLabels()
-	if _, ok := labels[common.GenerateTypeCloneSourceLabel]; ok {
-		return []kyvernov1.ResourceSpec{newResourceSpec(source.GetAPIVersion(), source.GetKind(), source.GetNamespace(), source.GetName())}, nil
+	selector[common.GenerateTriggerNameLabel] = ur.Spec.GetResource().GetName()
+	selector[common.GenerateTriggerNSLabel] = ur.Spec.GetResource().GetNamespace()
+	selector[common.GenerateTriggerKindLabel] = ur.Spec.GetResource().GetKind()
+	selector[common.GenerateTriggerGroupLabel] = gv.Group
+	selector[common.GenerateTriggerVersionLabel] = gv.Version
+	if rule.Generation.GetKind() != "" {
+		c.log.V(4).Info("fetching downstream resources", "APIVersion", rule.Generation.GetAPIVersion(), "kind", rule.Generation.GetKind(), "selector", selector)
+		return FindDownstream(c.client, rule.Generation.GetAPIVersion(), rule.Generation.GetKind(), selector)
 	}

+	dsList := &unstructured.UnstructuredList{}
 	for _, kind := range rule.Generation.CloneList.Kinds {
-		g, v, k, _ := kubeutils.ParseKindSelector(kind)
-		sources = append(sources, newResourceSpec(schema.GroupVersion{Group: g, Version: v}.String(), k, "", ""))
+		apiVersion, kind := kubeutils.GetKindFromGVK(kind)
+		c.log.V(4).Info("fetching downstream resources", "APIVersion", apiVersion, "kind", kind, "selector", selector)
+		dsList, err = FindDownstream(c.client, apiVersion, kind, selector)
+		if err != nil {
+			return nil, err
+		} else {
+			dsList.Items = append(dsList.Items, dsList.Items...)
+		}
 	}
-	return
+	return dsList, nil
 }
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/01-assert.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/01-assert.yaml
@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v2beta1
+kind: ClusterPolicy
+metadata:
+  name:  cpol-clone-list-sync-delete-source-cpol
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/01-manifests.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/01-manifests.yaml
@ -0,0 +1,51 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: cpol-clone-list-sync-delete-source-existing-ns
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    location: europe
+    allowedToBeCloned: "true"
+  name: mysecret-1
+  namespace: cpol-clone-list-sync-delete-source-existing-ns
+type: Opaque
+data:
+  foo: YmFy
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  labels:
+    location: europe
+    allowedToBeCloned: "true"
+  name: mysecret-2
+  namespace: cpol-clone-list-sync-delete-source-existing-ns
+type: Opaque
+data:
+  foo: YmFy
+---
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: cpol-clone-list-sync-delete-source-cpol
+spec:
+  rules:
+  - name: sync-secret
+    match:
+      all:
+      - resources:
+          kinds:
+          - Namespace
+    generate:
+      namespace: '{{ request.object.metadata.name }}'
+      synchronize: true
+      cloneList:
+        namespace: cpol-clone-list-sync-delete-source-existing-ns
+        kinds:
+          - v1/Secret
+        selector:
+          matchLabels:
+            allowedToBeCloned: "true"
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/02-check.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/02-check.yaml
@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- triggers.yaml
+assert:
+- target-1.yaml
+- target-2.yaml
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/03-delete.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/03-delete.yaml
@ -0,0 +1,7 @@
+# Specifying the kind as `TestStep` performs certain behaviors like this delete operation.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+delete:
+- apiVersion: v1
+  kind: Namespace
+  name: cpol-clone-list-sync-delete-source-trigger-ns-1
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/04-sleep.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/04-sleep.yaml
@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: sleep 3
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/05-check.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/05-check.yaml
@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+assert:
+- target-2.yaml
+error:
+- target-1.yaml
+
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/README.md
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/README.md
@ -0,0 +1,11 @@
+## Description
+
+This is a corner case test to ensure the corresponding downstream target is deleted when its trigger is deleted, for a generate cloneList type of policy.
+
+## Expected Behavior
+
+If the downstream resources `mysecret-1` and `mysecret-2` are remained in the namespace `cpol-clone-list-sync-delete-source-trigger-ns-2`, the test passes. If not, the test fails.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/7535
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/target-1.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/target-1.yaml
@ -0,0 +1,11 @@
+apiVersion: v1
+data:
+  foo: YmFy
+kind: Secret
+metadata:
+  labels:
+    allowedToBeCloned: "true"
+    location: europe
+  name: mysecret-1
+  namespace: cpol-clone-list-sync-delete-source-trigger-ns-1
+type: Opaque
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/target-2.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/target-2.yaml
@ -0,0 +1,11 @@
+apiVersion: v1
+data:
+  foo: YmFy
+kind: Secret
+metadata:
+  labels:
+    allowedToBeCloned: "true"
+    location: europe
+  name: mysecret-2
+  namespace: cpol-clone-list-sync-delete-source-trigger-ns-2
+type: Opaque
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/triggers.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/clone/sync/cpol-clone-list-sync-delete-source/triggers.yaml
@ -0,0 +1,10 @@
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: cpol-clone-list-sync-delete-source-trigger-ns-1
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: cpol-clone-list-sync-delete-source-trigger-ns-2
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/01-assert.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/01-assert.yaml
@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v2beta1
+kind: ClusterPolicy
+metadata:
+  name: cpol-data-sync-delete-one-trigger
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/01-manifests.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/01-manifests.yaml
@ -0,0 +1,31 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: cpol-data-sync-delete-one-trigger-ns
+---
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: cpol-data-sync-delete-one-trigger
+spec:
+  failurePolicy: Fail
+  validationFailureAction: Enforce
+  background: false
+  rules:
+  - name: replicate
+    match:
+      all:
+      - resources:
+          kinds:
+          - v1/ConfigMap
+          selector:
+            matchLabels:
+              replicate: "true"
+    generate:
+      apiVersion: v1
+      kind: ConfigMap
+      name: "{{ request.object.metadata.name }}-replicated"
+      namespace: "{{ request.object.metadata.namespace }}"
+      synchronize: true
+      data:
+        data: "{{ request.object.data }}"
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/02-check.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/02-check.yaml
@ -0,0 +1,8 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- trigger-1.yaml
+- trigger-others.yaml
+assert:
+- target-1.yaml
+- target-others.yaml
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/03-delete.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/03-delete.yaml
@ -0,0 +1,8 @@
+# Specifying the kind as `TestStep` performs certain behaviors like this delete operation.
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+delete:
+- apiVersion: v1
+  kind: ConfigMap
+  name: foosource-1
+  namespace: cpol-data-sync-delete-one-trigger-ns
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/04-sleep.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/04-sleep.yaml
@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: sleep 3
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/05-check.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/05-check.yaml
@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+error:
+- target-1.yaml
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/README.md
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/README.md
@ -0,0 +1,11 @@
+## Description
+
+This test checks to ensure that deletion of a trigger resource, with a generate data declaration and sync enabled, results in its corresponding downstream resource's deletion.
+
+## Expected Behavior
+
+If the downstream resource `foosource-1-replicated` is deleted while the other two `foosource-2-replicated` and `foosource-3-replicated` remain, the test passes. If not, the test fails.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/7535
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/target-1.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/target-1.yaml
@ -0,0 +1,8 @@
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-1-replicated
+  namespace: cpol-data-sync-delete-one-trigger-ns
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/target-others.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/target-others.yaml
@ -0,0 +1,16 @@
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-2-replicated
+  namespace: cpol-data-sync-delete-one-trigger-ns
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-3-replicated
+  namespace: cpol-data-sync-delete-one-trigger-ns
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/trigger-1.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/trigger-1.yaml
@ -0,0 +1,10 @@
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-1
+  namespace: cpol-data-sync-delete-one-trigger-ns
+  labels:
+    replicate: "true"
--- a/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/trigger-others.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/standard/data/sync/cpol-data-sync-delete-one-trigger/trigger-others.yaml
@ -0,0 +1,20 @@
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-2
+  namespace: cpol-data-sync-delete-one-trigger-ns
+  labels:
+    replicate: "true"
+---
+apiVersion: v1
+data:
+  foo: bar
+kind: ConfigMap
+metadata:
+  name: foosource-3
+  namespace: cpol-data-sync-delete-one-trigger-ns
+  labels:
+    replicate: "true"