From cd9e596e7ed263a168b7cb84948c47f1c59e747d Mon Sep 17 00:00:00 2001
From: Pooja Singh <singhpooja240393@gmail.com>
Date: Thu, 1 Jul 2021 00:30:02 +0530
Subject: [PATCH] [Improvement] Kyverno should not delete downstream resources
when a generate policy using the clone behavior has synchronize: true (#1880)
* debuging issue
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* issue fixed
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* remove policy name in source resource
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* fixed deletion of GR on source updation
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* added function in common
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* removing comments
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* added generated resource list to the log
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
* small improvement
Signed-off-by: NoSkillGirl <singhpooja240393@gmail.com>
---
go.sum | 1 +
pkg/common/common.go | 46 ++++++++++++++++++++++++++++++
pkg/generate/cleanup/controller.go | 33 ++++++++++++++++++---
pkg/policy/validate_controller.go | 12 ++++++--
pkg/webhooks/generation.go | 29 +++++++++++++------
5 files changed, 106 insertions(+), 15 deletions(-)
diff --git a/go.sum b/go.sum
index e0a75ddfa7..b8b881eda4 100644
--- a/go.sum
+++ b/go.sum
@@ -1019,6 +1019,7 @@ github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeV
github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE=
github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88=
+github.com/sirupsen/logrus v1.7.0 h1:ShrD1U9pZB12TX0cVy0DtePoCH97K8EtX+mg7ZARUtM=
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/sirupsen/logrus v1.8.0 h1:nfhvjKcUMhBMVqbKHJlk5RPrrfYr/NMo3692g0dwfWU=
github.com/sirupsen/logrus v1.8.0/go.mod h1:4GuYW9TZmE769R5STWrRakJc4UqQ3+QQ95fyz7ENv1A=
diff --git a/pkg/common/common.go b/pkg/common/common.go
index 7ed33318b0..039e5c8119 100644
--- a/pkg/common/common.go
+++ b/pkg/common/common.go
@@ -7,6 +7,8 @@ import (
"time"
"github.com/go-logr/logr"
+ kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
+ dclient "github.com/kyverno/kyverno/pkg/dclient"
enginutils "github.com/kyverno/kyverno/pkg/engine/utils"
"github.com/pkg/errors"
v1 "k8s.io/api/core/v1"
@@ -136,3 +138,47 @@ func RetryFunc(retryInterval, timeout time.Duration, run func() error, logger lo
return nil
}
}
+
+func ProcessDeletePolicyForCloneGenerateRule(rules []kyverno.Rule, client *dclient.Client, pName string, logger logr.Logger) bool {
+ generatePolicyWithClone := false
+ for _, rule := range rules {
+ if rule.Generation.Clone.Name != "" {
+ logger.V(4).Info("generate policy with clone, skipping deletion of generate request")
+ generatePolicyWithClone = true
+ obj, err := client.GetResource("", rule.Generation.Kind, rule.Generation.Clone.Namespace, rule.Generation.Clone.Name)
+ if err != nil {
+ logger.Error(err, fmt.Sprintf("source resource %s/%s/%s not found.", rule.Generation.Kind, rule.Generation.Clone.Namespace, rule.Generation.Clone.Name))
+ continue
+ }
+
+ updateSource := true
+ label := obj.GetLabels()
+ logger.V(4).Info("removing policy name from label of source resource")
+
+ if len(label) != 0 {
+ if label["generate.kyverno.io/clone-policy-name"] != "" {
+ policyNames := label["generate.kyverno.io/clone-policy-name"]
+ if strings.Contains(policyNames, pName) {
+ updatedPolicyNames := strings.Replace(policyNames, pName, "", -1)
+ label["generate.kyverno.io/clone-policy-name"] = updatedPolicyNames
+ } else {
+ updateSource = false
+ }
+ }
+ }
+
+ if updateSource {
+ logger.V(4).Info("updating existing clone source")
+ obj.SetLabels(label)
+ _, err = client.UpdateResource(obj.GetAPIVersion(), rule.Generation.Kind, rule.Generation.Clone.Namespace, obj, false)
+ if err != nil {
+ logger.Error(err, "failed to update source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace())
+ continue
+ }
+ logger.V(4).Info("updated source", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace())
+ }
+ }
+ }
+
+ return generatePolicyWithClone
+}
diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go
index e1b2e33e72..ac7265d7bb 100644
--- a/pkg/generate/cleanup/controller.go
+++ b/pkg/generate/cleanup/controller.go
@@ -10,10 +10,12 @@ import (
kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned"
kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1"
kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
+ pkgCommon "github.com/kyverno/kyverno/pkg/common"
"github.com/kyverno/kyverno/pkg/config"
dclient "github.com/kyverno/kyverno/pkg/dclient"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+ "k8s.io/apimachinery/pkg/labels"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/dynamic/dynamicinformer"
@@ -139,14 +141,37 @@ func (c *Controller) deletePolicy(obj interface{}) {
// clean up the GR
// Get the corresponding GR
// get the list of GR for the current Policy version
- grs, err := c.grLister.GetGenerateRequestsForClusterPolicy(p.Name)
+ rules := p.Spec.Rules
+
+ generatePolicyWithClone := pkgCommon.ProcessDeletePolicyForCloneGenerateRule(rules, c.client, p.GetName(), logger)
+
+ // get the generated resource name from generate request for log
+ selector := labels.SelectorFromSet(labels.Set(map[string]string{
+ "generate.kyverno.io/policy-name": p.Name,
+ }))
+
+ grList, err := c.grLister.List(selector)
if err != nil {
- logger.Error(err, "failed to generate request CR for the policy", "name", p.Name)
+ logger.Error(err, "failed to get generate request for the resource", "label", "generate.kyverno.io/policy-name")
return
}
- for _, gr := range grs {
- c.addGR(gr)
+ for _, gr := range grList {
+ for _, generatedResource := range gr.Status.GeneratedResources {
+ logger.Info("retaining resource", "APIVersion", generatedResource.APIVersion, "Kind", generatedResource.Kind, "Name", generatedResource.Name, "Nmaespace", generatedResource.Namespace)
+ }
+ }
+
+ if !generatePolicyWithClone {
+ grs, err := c.grLister.GetGenerateRequestsForClusterPolicy(p.Name)
+ if err != nil {
+ logger.Error(err, "failed to generate request CR for the policy", "name", p.Name)
+ return
+ }
+
+ for _, gr := range grs {
+ c.addGR(gr)
+ }
}
}
diff --git a/pkg/policy/validate_controller.go b/pkg/policy/validate_controller.go
index e8dbb0dd73..2b89d57617 100644
--- a/pkg/policy/validate_controller.go
+++ b/pkg/policy/validate_controller.go
@@ -15,6 +15,7 @@ import (
"github.com/kyverno/kyverno/pkg/client/clientset/versioned/scheme"
kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1"
kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1"
+ pkgCommon "github.com/kyverno/kyverno/pkg/common"
"github.com/kyverno/kyverno/pkg/config"
client "github.com/kyverno/kyverno/pkg/dclient"
"github.com/kyverno/kyverno/pkg/event"
@@ -341,8 +342,15 @@ func (pc *PolicyController) deletePolicy(obj interface{}) {
// we process policies that are not set of background processing
// as we need to clean up GRs when a policy is deleted
- pc.enqueuePolicy(p)
- pc.enqueueRCRDeletedPolicy(p.Name)
+ // skip generate policies with clone
+ rules := p.Spec.Rules
+
+ generatePolicyWithClone := pkgCommon.ProcessDeletePolicyForCloneGenerateRule(rules, pc.client, p.GetName(), logger)
+
+ if !generatePolicyWithClone {
+ pc.enqueuePolicy(p)
+ pc.enqueueRCRDeletedPolicy(p.Name)
+ }
}
func (pc *PolicyController) registerPolicyRuleInfoMetricAddNsPolicy(logger logr.Logger, p *kyverno.Policy) {
diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go
index 2756b7740e..07497a0012 100644
--- a/pkg/webhooks/generation.go
+++ b/pkg/webhooks/generation.go
@@ -154,18 +154,29 @@ func (ws *WebhookServer) handleUpdate(request *v1beta1.AdmissionRequest, policie
func (ws *WebhookServer) handleUpdateCloneSourceResource(resLabels map[string]string, logger logr.Logger) {
policyNames := strings.Split(resLabels["generate.kyverno.io/clone-policy-name"], ",")
for _, policyName := range policyNames {
- selector := labels.SelectorFromSet(labels.Set(map[string]string{
- "generate.kyverno.io/policy-name": policyName,
- }))
- grList, err := ws.grLister.List(selector)
+ // check if the policy exists
+ _, err := ws.kyvernoClient.KyvernoV1().ClusterPolicies().Get(contextdefault.TODO(), policyName, metav1.GetOptions{})
if err != nil {
- logger.Error(err, "failed to get generate request for the resource", "label", "generate.kyverno.io/policy-name")
- return
- }
- for _, gr := range grList {
- ws.updateAnnotationInGR(gr, logger)
+ if strings.Contains(err.Error(), "not found") {
+ logger.V(4).Info("skipping updation of generate request as policy is deleted")
+ } else {
+ logger.Error(err, "failed to get generate policy", "Name", policyName)
+ selector := labels.SelectorFromSet(labels.Set(map[string]string{
+ "generate.kyverno.io/policy-name": policyName,
+ }))
+
+ grList, err := ws.grLister.List(selector)
+ if err != nil {
+ logger.Error(err, "failed to get generate request for the resource", "label", "generate.kyverno.io/policy-name")
+ return
+ }
+ for _, gr := range grList {
+ ws.updateAnnotationInGR(gr, logger)
+ }
+ }
}
+
}
}