From fe8c429e78108f345d02ce284007d32421c2adb5 Mon Sep 17 00:00:00 2001 From: shuting Date: Wed, 12 Jun 2024 23:23:53 +0800 Subject: [PATCH] fix: avoid creating duplicate urs for background policies (#10431) * feat: add generator abstraction Signed-off-by: ShutingZhao * feat: replace urgenerator Signed-off-by: ShutingZhao * fix: ko build Signed-off-by: ShutingZhao * feat: load threshold from kyverno configmap Signed-off-by: ShutingZhao * feat: add metadata client to get ur count Signed-off-by: ShutingZhao * feat: add helm option to preserve configmap settings during upgrade Signed-off-by: ShutingZhao * feat: add helm option to preserve configmap settings during upgrade 2 Signed-off-by: ShutingZhao * chore: rename imports Signed-off-by: ShutingZhao * chore: update codegen manifests Signed-off-by: ShutingZhao * fix: handle nil value Signed-off-by: ShutingZhao * fix: linter issue Signed-off-by: ShutingZhao * chore: update threshold to 1000 Signed-off-by: ShutingZhao * fix: avoid duplicate URs creation Signed-off-by: ShutingZhao * fix: revert false changes Signed-off-by: ShutingZhao * fix: simplify background applications Signed-off-by: ShutingZhao --------- Signed-off-by: ShutingZhao --- api/kyverno/v1beta1/updaterequest_types.go | 1 + .../kyverno.io/kyverno.io_updaterequests.yaml | 3 ++ .../kyverno/kyverno.io_updaterequests.yaml | 3 ++ config/install-latest-testing.yaml | 3 ++ pkg/policy/generate.go | 24 +++++---- pkg/policy/mutate.go | 53 ++++++++++--------- pkg/policy/policy_controller.go | 10 ++-- 7 files changed, 59 insertions(+), 38 deletions(-) diff --git a/api/kyverno/v1beta1/updaterequest_types.go b/api/kyverno/v1beta1/updaterequest_types.go index 4b1d882356e9..7e3a2372b816 100644 --- a/api/kyverno/v1beta1/updaterequest_types.go +++ b/api/kyverno/v1beta1/updaterequest_types.go @@ -48,6 +48,7 @@ type UpdateRequestStatus struct { // +kubebuilder:storageversion // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Policy",type="string",JSONPath=".spec.policy" +// +kubebuilder:printcolumn:name="Rule",type="string",JSONPath=".spec.rule" // +kubebuilder:printcolumn:name="RuleType",type="string",JSONPath=".spec.requestType" // +kubebuilder:printcolumn:name="ResourceKind",type="string",JSONPath=".spec.resource.kind" // +kubebuilder:printcolumn:name="ResourceName",type="string",JSONPath=".spec.resource.name" diff --git a/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_updaterequests.yaml b/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_updaterequests.yaml index 26a81e11b3e4..3fd65f770cb7 100644 --- a/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_updaterequests.yaml +++ b/charts/kyverno/charts/crds/templates/kyverno.io/kyverno.io_updaterequests.yaml @@ -28,6 +28,9 @@ spec: - jsonPath: .spec.policy name: Policy type: string + - jsonPath: .spec.rule + name: Rule + type: string - jsonPath: .spec.requestType name: RuleType type: string diff --git a/config/crds/kyverno/kyverno.io_updaterequests.yaml b/config/crds/kyverno/kyverno.io_updaterequests.yaml index 52ba4957db8d..862a8c3bdac5 100644 --- a/config/crds/kyverno/kyverno.io_updaterequests.yaml +++ b/config/crds/kyverno/kyverno.io_updaterequests.yaml @@ -22,6 +22,9 @@ spec: - jsonPath: .spec.policy name: Policy type: string + - jsonPath: .spec.rule + name: Rule + type: string - jsonPath: .spec.requestType name: RuleType type: string diff --git a/config/install-latest-testing.yaml b/config/install-latest-testing.yaml index 32077f5b8e0b..efda0cd108aa 100644 --- a/config/install-latest-testing.yaml +++ b/config/install-latest-testing.yaml @@ -46169,6 +46169,9 @@ spec: - jsonPath: .spec.policy name: Policy type: string + - jsonPath: .spec.rule + name: Rule + type: string - jsonPath: .spec.requestType name: RuleType type: string diff --git a/pkg/policy/generate.go b/pkg/policy/generate.go index b12ca7177932..2551ef0bfa03 100644 --- a/pkg/policy/generate.go +++ b/pkg/policy/generate.go @@ -13,6 +13,7 @@ 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" ) func (pc *policyController) handleGenerate(policyKey string, policy kyvernov1.PolicyInterface) error { @@ -38,14 +39,19 @@ func (pc *policyController) handleGenerate(policyKey string, policy kyvernov1.Po func (pc *policyController) handleGenerateForExisting(policy kyvernov1.PolicyInterface) error { var errors []error + var triggers []*unstructured.Unstructured + ruleType := kyvernov1beta1.Generate + policyNew := policy.CreateDeepCopy() + policyNew.GetSpec().Rules = nil + for _, rule := range policy.GetSpec().Rules { - ruleType := kyvernov1beta1.Generate - triggers := getTriggers(pc.client, rule, policy.IsNamespaced(), policy.GetNamespace(), pc.log) + triggers = getTriggers(pc.client, rule, policy.IsNamespaced(), policy.GetNamespace(), pc.log) + policyNew.GetSpec().SetRules([]kyvernov1.Rule{rule}) for _, trigger := range triggers { - ur := newUR(policy, common.ResourceSpecFromUnstructured(*trigger), rule.Name, ruleType, false) - skip, err := pc.handleUpdateRequest(ur, trigger, rule, policy) + ur := newUR(policyNew, common.ResourceSpecFromUnstructured(*trigger), rule.Name, ruleType, false) + skip, err := pc.handleUpdateRequest(ur, trigger, rule.Name, policyNew) if err != nil { - pc.log.Error(err, "failed to create new UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType, + pc.log.Error(err, "failed to create new UR on policy update", "policy", policyNew.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 @@ -55,7 +61,7 @@ func (pc *policyController) handleGenerateForExisting(policy kyvernov1.PolicyInt continue } - pc.log.V(4).Info("successfully created UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType, + pc.log.V(4).Info("successfully created UR on policy update", "policy", policyNew.GetName(), "rule", rule.Name, "rule type", ruleType, "target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName())) } } @@ -112,15 +118,11 @@ func (pc *policyController) syncDataRulechanges(policy kyvernov1.PolicyInterface labels := downstream.GetLabels() trigger := generateutils.TriggerFromLabels(labels) ur := newUR(policy, trigger, rule.Name, kyvernov1beta1.Generate, deleteDownstream) - - created, err := pc.urGenerator.Generate(context.TODO(), pc.kyvernoClient, ur, pc.log) + created, err := pc.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Create(context.TODO(), ur, metav1.CreateOptions{}) if err != nil { errorList = append(errorList, err) continue } - if created == nil { - continue - } updated := created.DeepCopy() updated.Status = newURStatus(downstream) _, err = pc.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), updated, metav1.UpdateOptions{}) diff --git a/pkg/policy/mutate.go b/pkg/policy/mutate.go index 10bf779c904b..e6090313b06e 100644 --- a/pkg/policy/mutate.go +++ b/pkg/policy/mutate.go @@ -12,34 +12,39 @@ import ( func (pc *policyController) handleMutate(policyKey string, policy kyvernov1.PolicyInterface) error { logger := pc.log.WithName("handleMutate").WithName(policyKey) - logger.Info("update URs on policy event") + + ruleType := kyvernov1beta1.Mutate + policyNew := policy.CreateDeepCopy() + policyNew.GetSpec().Rules = nil + for _, rule := range policy.GetSpec().Rules { - var ruleType kyvernov1beta1.RequestType - if rule.HasMutateExisting() { - ruleType = kyvernov1beta1.Mutate - triggers := getTriggers(pc.client, rule, policy.IsNamespaced(), policy.GetNamespace(), pc.log) - for _, trigger := range triggers { - murs := pc.listMutateURs(policyKey, trigger) - if murs != nil { - logger.V(4).Info("UR was created", "rule", rule.Name, "rule type", ruleType, "trigger", trigger.GetNamespace()+trigger.GetName()) - continue - } - - logger.Info("creating new UR for mutate") - ur := newUR(policy, backgroundcommon.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())) - continue - } - if skip { - continue - } - pc.log.V(2).Info("successfully created UR on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule type", ruleType, + if !rule.HasMutateExisting() { + continue + } + + policyNew.GetSpec().SetRules([]kyvernov1.Rule{rule}) + triggers := getTriggers(pc.client, rule, policyNew.IsNamespaced(), policyNew.GetNamespace(), pc.log) + for _, trigger := range triggers { + murs := pc.listMutateURs(policyKey, trigger) + if murs != nil { + logger.V(4).Info("UR was created", "rule", rule.Name, "rule type", ruleType, "trigger", trigger.GetNamespace()+trigger.GetName()) + continue + } + + logger.Info("creating new UR for mutate") + ur := newUR(policy, backgroundcommon.ResourceSpecFromUnstructured(*trigger), rule.Name, ruleType, false) + skip, err := pc.handleUpdateRequest(ur, trigger, rule.Name, policyNew) + if err != nil { + pc.log.Error(err, "failed to create new UR on policy update", "policy", policyNew.GetName(), "rule", rule.Name, "rule type", ruleType, "target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName())) + continue + } + if skip { + continue } + pc.log.V(2).Info("successfully created UR on policy update", "policy", policyNew.GetName(), "rule", rule.Name, "rule type", ruleType, + "target", fmt.Sprintf("%s/%s/%s/%s", trigger.GetAPIVersion(), trigger.GetKind(), trigger.GetNamespace(), trigger.GetName())) } } return nil diff --git a/pkg/policy/policy_controller.go b/pkg/policy/policy_controller.go index fabc55c5fb24..5f236fdfdfc6 100644 --- a/pkg/policy/policy_controller.go +++ b/pkg/policy/policy_controller.go @@ -396,11 +396,11 @@ func (pc *policyController) requeuePolicies() { } } -func (pc *policyController) handleUpdateRequest(ur *kyvernov1beta1.UpdateRequest, triggerResource *unstructured.Unstructured, rule kyvernov1.Rule, policy kyvernov1.PolicyInterface) (skip bool, err error) { +func (pc *policyController) handleUpdateRequest(ur *kyvernov1beta1.UpdateRequest, triggerResource *unstructured.Unstructured, ruleName string, policy kyvernov1.PolicyInterface) (skip bool, err error) { namespaceLabels := engineutils.GetNamespaceSelectorsFromNamespaceLister(triggerResource.GetKind(), triggerResource.GetNamespace(), pc.nsLister, pc.log) policyContext, err := backgroundcommon.NewBackgroundContext(pc.log, pc.client, ur, policy, triggerResource, pc.configuration, pc.jp, namespaceLabels) if err != nil { - return false, fmt.Errorf("failed to build policy context for rule %s: %w", rule.Name, err) + return false, fmt.Errorf("failed to build policy context for rule %s: %w", ruleName, err) } engineResponse := pc.engine.ApplyBackgroundChecks(context.TODO(), policyContext) @@ -410,7 +410,11 @@ func (pc *policyController) handleUpdateRequest(ur *kyvernov1beta1.UpdateRequest for _, ruleResponse := range engineResponse.PolicyResponse.Rules { if ruleResponse.Status() != engineapi.RuleStatusPass { - pc.log.V(4).Info("skip creating URs on policy update", "policy", policy.GetName(), "rule", rule.Name, "rule.Status", ruleResponse.Status()) + pc.log.V(4).Info("skip creating URs on policy update", "policy", policy.GetName(), "rule", ruleName, "rule.Status", ruleResponse.Status()) + continue + } + + if ruleResponse.Name() != ur.Spec.GetRuleName() { continue }