fix: remove timestamp checks for the clone rule (#6439)

* remove timestamp checks Signed-off-by: ShutingZhao <shuting@nirmata.com> * add kuttl tests Signed-off-by: ShutingZhao <shuting@nirmata.com> --------- Signed-off-by: ShutingZhao <shuting@nirmata.com>
2025-03-05 07:26:55 +00:00 · 2023-03-01 23:49:05 +08:00 · 2023-03-01 23:49:05 +08:00 · 264eaec049
commit 264eaec049
parent f4f729623d
15 changed files with 163 additions and 55 deletions
--- a/cmd/cli/kubectl-kyverno/utils/common/common.go
+++ b/cmd/cli/kubectl-kyverno/utils/common/common.go
@ -1124,7 +1124,7 @@ func handleGeneratePolicy(generateResponse *engineapi.EngineResponse, policyCont
 	var newRuleResponse []engineapi.RuleResponse

 	for _, rule := range generateResponse.PolicyResponse.Rules {
-		genResource, _, err := c.ApplyGeneratePolicy(log.Log, &policyContext, gr, []string{rule.Name})
+		genResource, err := c.ApplyGeneratePolicy(log.Log, &policyContext, gr, []string{rule.Name})
 		if err != nil {
 			rule.Status = engineapi.RuleStatusError
 			return nil, err
--- a/pkg/background/common/context.go
+++ b/pkg/background/common/context.go
@ -21,25 +21,25 @@ func NewBackgroundContext(dclient dclient.Interface, ur *kyvernov1beta1.UpdateRe
 	cfg config.Configuration,
 	namespaceLabels map[string]string,
 	logger logr.Logger,
-) (*engine.PolicyContext, bool, error) {
+) (*engine.PolicyContext, error) {
 	ctx := context.NewContext()
 	var new, old unstructured.Unstructured
 	var err error

 	if ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest != nil {
 		if err := ctx.AddRequest(ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest); err != nil {
-			return nil, false, fmt.Errorf("failed to load request in context: %w", err)
+			return nil, fmt.Errorf("failed to load request in context: %w", err)
 		}

 		new, old, err = admissionutils.ExtractResources(nil, ur.Spec.Context.AdmissionRequestInfo.AdmissionRequest)
 		if err != nil {
-			return nil, false, fmt.Errorf("failed to load request in context: %w", err)
+			return nil, fmt.Errorf("failed to load request in context: %w", err)
 		}

 		if !reflect.DeepEqual(new, unstructured.Unstructured{}) {
 			if !check(&new, trigger) {
 				err := fmt.Errorf("resources don't match")
-				return nil, false, fmt.Errorf("resource %v: %w", ur.Spec.GetResource().String(), err)
+				return nil, fmt.Errorf("resource %v: %w", ur.Spec.GetResource().String(), err)
 			}
 		}
 	}
@ -49,27 +49,27 @@ func NewBackgroundContext(dclient dclient.Interface, ur *kyvernov1beta1.UpdateRe
 	}

 	if trigger == nil {
-		return nil, false, fmt.Errorf("trigger resource does not exist")
+		return nil, fmt.Errorf("trigger resource does not exist")
 	}

 	err = ctx.AddResource(trigger.Object)
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to load resource in context: %w", err)
+		return nil, fmt.Errorf("failed to load resource in context: %w", err)
 	}

 	err = ctx.AddOldResource(old.Object)
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to load resource in context: %w", err)
+		return nil, fmt.Errorf("failed to load resource in context: %w", err)
 	}

 	err = ctx.AddUserInfo(ur.Spec.Context.UserRequestInfo)
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to load SA in context: %w", err)
+		return nil, fmt.Errorf("failed to load SA in context: %w", err)
 	}

 	err = ctx.AddServiceAccount(ur.Spec.Context.UserRequestInfo.AdmissionUserInfo.Username)
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to load UserInfo in context: %w", err)
+		return nil, fmt.Errorf("failed to load UserInfo in context: %w", err)
 	}

 	if err := ctx.AddImageInfos(trigger, cfg); err != nil {
@ -83,7 +83,7 @@ func NewBackgroundContext(dclient dclient.Interface, ur *kyvernov1beta1.UpdateRe
 		WithAdmissionInfo(ur.Spec.Context.UserRequestInfo).
 		WithNamespaceLabels(namespaceLabels)

-	return policyContext, false, nil
+	return policyContext, nil
 }

 func check(admissionRsc, existingRsc *unstructured.Unstructured) bool {
--- a/pkg/background/generate/generate.go
+++ b/pkg/background/generate/generate.go
@ -90,7 +90,6 @@ func (c *GenerateController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) error {
 	var err error
 	var resource *unstructured.Unstructured
 	var genResources []kyvernov1.ResourceSpec
-	var precreatedResource bool
 	logger.Info("start processing UR", "ur", ur.Name, "resourceVersion", ur.GetResourceVersion())

 	// 1 - Check if the trigger exists
@ -127,7 +126,7 @@ func (c *GenerateController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) error {

 	// 2 - Apply the generate policy on the resource
 	namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(resource.GetKind(), resource.GetNamespace(), c.nsLister, logger)
-	genResources, precreatedResource, err = c.applyGenerate(*resource, *ur, namespaceLabels)
+	genResources, err = c.applyGenerate(*resource, *ur, namespaceLabels)
 	if err != nil {
 		// Need not update the status when policy doesn't apply on resource, because all the update requests are removed by the cleanup controller
 		if strings.Contains(err.Error(), doesNotApply) {
@ -141,36 +140,36 @@ func (c *GenerateController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) error {
 	}

 	// 4 - Update Status
-	return updateStatus(c.statusControl, *ur, err, genResources, precreatedResource)
+	return updateStatus(c.statusControl, *ur, err, genResources)
 }

 const doesNotApply = "policy does not apply to resource"

-func (c *GenerateController) applyGenerate(resource unstructured.Unstructured, ur kyvernov1beta1.UpdateRequest, namespaceLabels map[string]string) ([]kyvernov1.ResourceSpec, bool, error) {
+func (c *GenerateController) applyGenerate(resource unstructured.Unstructured, ur kyvernov1beta1.UpdateRequest, namespaceLabels map[string]string) ([]kyvernov1.ResourceSpec, error) {
 	logger := c.log.WithValues("name", ur.GetName(), "policy", ur.Spec.GetPolicyKey(), "resource", ur.Spec.GetResource().String())
 	logger.V(3).Info("applying generate policy rule")

 	policy, err := c.getPolicySpec(ur)
 	if err != nil && !apierrors.IsNotFound(err) {
 		logger.Error(err, "error in fetching policy")
-		return nil, false, err
+		return nil, err
 	}

 	if ur.Spec.DeleteDownstream || apierrors.IsNotFound(err) {
 		err = c.deleteDownstream(policy, &ur)
-		return nil, false, err
+		return nil, err
 	}

-	policyContext, precreatedResource, err := common.NewBackgroundContext(c.client, &ur, policy, &resource, c.configuration, namespaceLabels, logger)
+	policyContext, err := common.NewBackgroundContext(c.client, &ur, policy, &resource, c.configuration, namespaceLabels, logger)
 	if err != nil {
-		return nil, precreatedResource, err
+		return nil, err
 	}

 	// check if the policy still applies to the resource
 	engineResponse := c.engine.GenerateResponse(context.Background(), policyContext, ur)
 	if len(engineResponse.PolicyResponse.Rules) == 0 {
 		logger.V(4).Info(doesNotApply)
-		return nil, false, errors.New(doesNotApply)
+		return nil, errors.New(doesNotApply)
 	}

 	var applicableRules []string
@ -226,15 +225,11 @@ func (c *GenerateController) getPolicySpec(ur kyvernov1beta1.UpdateRequest) (kyv
 	return npolicyObj, nil
 }

-func updateStatus(statusControl common.StatusControlInterface, ur kyvernov1beta1.UpdateRequest, err error, genResources []kyvernov1.ResourceSpec, precreatedResource bool) error {
+func updateStatus(statusControl common.StatusControlInterface, ur kyvernov1beta1.UpdateRequest, err error, genResources []kyvernov1.ResourceSpec) error {
 	if err != nil {
 		if _, err := statusControl.Failed(ur.GetName(), err.Error(), genResources); err != nil {
 			return err
 		}
-	} else if precreatedResource {
-		if _, err := statusControl.Skip(ur.GetName(), genResources); err != nil {
-			return err
-		}
 	} else {
 		if _, err := statusControl.Success(ur.GetName(), genResources); err != nil {
 			return err
@ -243,7 +238,7 @@ func updateStatus(statusControl common.StatusControlInterface, ur kyvernov1beta1
 	return nil
 }

-func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext *engine.PolicyContext, ur kyvernov1beta1.UpdateRequest, applicableRules []string) (genResources []kyvernov1.ResourceSpec, processExisting bool, err error) {
+func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext *engine.PolicyContext, ur kyvernov1beta1.UpdateRequest, applicableRules []string) (genResources []kyvernov1.ResourceSpec, err error) {
 	// Get the response as the actions to be performed on the resource
 	// - - substitute values
 	policy := policyContext.Policy()
@ -265,17 +260,7 @@ func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext
 		}

 		startTime := time.Now()
-		processExisting = false
 		var genResource []kyvernov1.ResourceSpec
-
-		if len(rule.MatchResources.Kinds) > 0 {
-			if len(rule.MatchResources.Annotations) == 0 && rule.MatchResources.Selector == nil {
-				rcreationTime := resource.GetCreationTimestamp()
-				pcreationTime := policy.GetCreationTimestamp()
-				processExisting = rcreationTime.Before(&pcreationTime)
-			}
-		}
-
 		if applyRules == kyvernov1.ApplyOne && applyCount > 0 {
 			break
 		}
@ -283,33 +268,26 @@ func (c *GenerateController) ApplyGeneratePolicy(log logr.Logger, policyContext
 		// add configmap json data to context
 		if err := c.engine.ContextLoader(policyContext.Policy(), rule)(context.TODO(), rule.Context, policyContext.JSONContext()); err != nil {
 			log.Error(err, "cannot add configmaps to context")
-			return nil, processExisting, err
+			return nil, err
 		}

 		if rule, err = variables.SubstituteAllInRule(log, policyContext.JSONContext(), rule); err != nil {
 			log.Error(err, "variable substitution failed for rule %s", rule.Name)
-			return nil, processExisting, err
+			return nil, err
 		}

-		if policy.GetSpec().IsGenerateExistingOnPolicyUpdate() || !processExisting {
-			genResource, err = applyRule(log, c.client, rule, resource, jsonContext, policy, ur)
-			if err != nil {
-				log.Error(err, "failed to apply generate rule", "policy", policy.GetName(),
-					"rule", rule.Name, "resource", resource.GetName(), "suggestion", "users need to grant Kyverno's service account additional privileges")
-				return nil, processExisting, err
-			}
-			ruleNameToProcessingTime[rule.Name] = time.Since(startTime)
-			genResources = append(genResources, genResource...)
+		genResource, err = applyRule(log, c.client, rule, resource, jsonContext, policy, ur)
+		if err != nil {
+			log.Error(err, "failed to apply generate rule", "policy", policy.GetName(),
+				"rule", rule.Name, "resource", resource.GetName(), "suggestion", "users need to grant Kyverno's service account additional privileges")
+			return nil, err
 		}
-
-		if policy.GetSpec().IsGenerateExistingOnPolicyUpdate() {
-			processExisting = false
-		}
-
+		ruleNameToProcessingTime[rule.Name] = time.Since(startTime)
+		genResources = append(genResources, genResource...)
 		applyCount++
 	}

-	return genResources, processExisting, nil
+	return genResources, nil
 }

 func getResourceInfo(object map[string]interface{}) (kind, name, namespace, apiversion string, err error) {
--- a/pkg/background/mutate/mutate.go
+++ b/pkg/background/mutate/mutate.go
@ -92,7 +92,7 @@ func (c *MutateExistingController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) e
 		}

 		namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(trigger.GetKind(), trigger.GetNamespace(), c.nsLister, logger)
-		policyContext, _, err := common.NewBackgroundContext(c.client, ur, policy, trigger, c.configuration, namespaceLabels, logger)
+		policyContext, err := common.NewBackgroundContext(c.client, ur, policy, trigger, c.configuration, namespaceLabels, logger)
 		if err != nil {
 			logger.WithName(rule.Name).Error(err, "failed to build policy context")
 			errs = append(errs, err)
--- a/pkg/policy/policy_controller.go
+++ b/pkg/policy/policy_controller.go
@ -407,7 +407,7 @@ func (pc *PolicyController) requeuePolicies() {

 func (pc *PolicyController) handleUpdateRequest(ur *kyvernov1beta1.UpdateRequest, triggerResource *unstructured.Unstructured, rule kyvernov1.Rule, policy kyvernov1.PolicyInterface) (skip bool, err error) {
 	namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(triggerResource.GetKind(), triggerResource.GetNamespace(), pc.nsLister, pc.log)
-	policyContext, _, err := backgroundcommon.NewBackgroundContext(pc.client, ur, policy, triggerResource, pc.configHandler, namespaceLabels, pc.log)
+	policyContext, err := backgroundcommon.NewBackgroundContext(pc.client, ur, policy, triggerResource, pc.configHandler, namespaceLabels, pc.log)
 	if err != nil {
 		return false, fmt.Errorf("failed to build policy context for rule %s: %w", rule.Name, err)
 	}
--- a/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/01-assert.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/01-assert.yaml
@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v2beta1
+kind: ClusterPolicy
+metadata:
+  name: cpol-clone-sync-create-source-after-policy
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready
--- a/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/01-manifests.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/01-manifests.yaml
@ -0,0 +1,30 @@
+apiVersion: kyverno.io/v2beta1
+kind: ClusterPolicy
+metadata:
+  name: cpol-clone-sync-create-source-after-policy
+spec:
+  rules:
+  - name: clone-secret
+    match:
+      any:
+      - resources:
+          kinds:
+          - Namespace
+    generate:
+      apiVersion: v1
+      kind: Secret
+      name: regcred
+      namespace: "{{request.object.metadata.name}}"
+      synchronize: true
+      clone:
+        namespace: default
+        name: regcred
+---
+apiVersion: v1
+data:
+  foo: YmFy
+kind: Secret
+metadata:
+  name: regcred
+  namespace: default
+type: Opaque
--- a/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/02-assert.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/02-assert.yaml
@ -0,0 +1,5 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: regcred
+  namespace: cpol-clone-sync-create-source-after-policy-ns
--- a/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/02-ns.yaml
+++ b/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/02-ns.yaml
@ -0,0 +1,4 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: cpol-clone-sync-create-source-after-policy-ns
--- a/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/README.md
+++ b/test/conformance/kuttl/generate/clusterpolicy/cornercases/cpol-clone-sync-create-source-after-policy/README.md
@ -0,0 +1,11 @@
+## Description
+
+This is a corner case test to ensure a clone rule is applied when the source is created after the ClusterPolicy.
+
+## Expected Behavior
+
+If the downstream resource is created, the test passes. If it is not created, the test fails.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/5411
--- a/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/01-assert.yaml
+++ b/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/01-assert.yaml
@ -0,0 +1,10 @@
+apiVersion: kyverno.io/v2beta1
+kind: Policy
+metadata:
+  name: pol-clone-sync-create-source-after-policy
+  namespace: pol-clone-sync-create-source-after-policy-ns
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready
--- a/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/01-manifests.yaml
+++ b/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/01-manifests.yaml
@ -0,0 +1,36 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: pol-clone-sync-create-source-after-policy-ns
+---
+apiVersion: kyverno.io/v2beta1
+kind: Policy
+metadata:
+  name: pol-clone-sync-create-source-after-policy
+  namespace: pol-clone-sync-create-source-after-policy-ns
+spec:
+  rules:
+  - name: clone-secret
+    match:
+      any:
+      - resources:
+          kinds:
+          - ConfigMap
+    generate:
+      apiVersion: v1
+      kind: Secret
+      name: mynewsecret
+      namespace: pol-clone-sync-create-source-after-policy-ns
+      synchronize: true
+      clone:
+        namespace: pol-clone-sync-create-source-after-policy-ns
+        name: regcred
+---
+apiVersion: v1
+data:
+  foo: YmFy
+kind: Secret
+metadata:
+  name: regcred
+  namespace: pol-clone-sync-create-source-after-policy-ns
+type: Opaque
--- a/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/02-assert.yaml
+++ b/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/02-assert.yaml
@ -0,0 +1,5 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: mynewsecret
+  namespace: pol-clone-sync-create-source-after-policy-ns
--- a/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/02-configmap.yaml
+++ b/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/02-configmap.yaml
@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: mycm
+  namespace: pol-clone-sync-create-source-after-policy-ns
+data:
+  food: cheese
+  day: monday
+  color: red
--- a/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/README.md
+++ b/test/conformance/kuttl/generate/policy/cornercases/pol-clone-sync-create-source-after-policy/README.md
@ -0,0 +1,11 @@
+## Description
+
+This is a corner case test to ensure a clone rule is applied when the source is created after the ClusterPolicy.
+
+## Expected Behavior
+
+If the downstream resource is created, the test passes. If it is not created, the test fails.
+
+## Reference Issue(s)
+
+https://github.com/kyverno/kyverno/issues/5411