From 2641120907cdefa3a49f9e8c51561a98547d5b84 Mon Sep 17 00:00:00 2001
From: Yuvraj <10830562+evalsocket@users.noreply.github.com>
Date: Mon, 31 Aug 2020 23:55:13 +0530
Subject: [PATCH] Generate policy does not work on namespace update (#1085)

* added logic for handling generate request

* generate rules added

* added label condition for generate

* remove extra logs

* remove extra logs

* buf fixed

* bug fixed

* added logic for delete gr

* log fixed

* documentation changed

* remove best practices changes

* bug fix

* added best pratice
---
 documentation/writing-policies-generate.md    | 20 ++++++-
 pkg/engine/generation.go                      |  9 +++-
 pkg/generate/cleanup/controller.go            | 13 +++++
 pkg/generate/controller.go                    | 12 +++++
 pkg/generate/generate.go                      | 54 ++++++++++++-------
 pkg/webhooks/generate/generate.go             | 43 +++++++++++----
 pkg/webhooks/generation.go                    | 36 ++++++++++---
 samples/AddDefaultNetworkPolicy.md            |  7 +++
 samples/AddNamespaceQuotas.md                 |  6 +++
 .../best_practices/add_network_policy.yaml    |  6 +++
 samples/best_practices/add_ns_quota.yaml      |  6 +++
 11 files changed, 173 insertions(+), 39 deletions(-)

diff --git a/documentation/writing-policies-generate.md b/documentation/writing-policies-generate.md
index b021a0004d..b4affcb174 100644
--- a/documentation/writing-policies-generate.md
+++ b/documentation/writing-policies-generate.md
@@ -22,6 +22,12 @@ spec:
         resources:
           kinds:
             - Namespace
+      exclude:
+        namespaces:
+          - "kube-system"
+          - "default"
+          - "kube-public"
+          - "kyverno"
       generate:
         synchronize: true
         kind: ConfigMap
@@ -49,6 +55,12 @@ spec:
         resources:
           kinds: 
           - Namespace
+      exclude:
+        namespaces:
+          - "kube-system"
+          - "default"
+          - "kube-public"
+          - "kyverno"
       generate:
         kind: ConfigMap # Kind of resource 
         name: default-config # Name of the new Resource
@@ -75,7 +87,7 @@ spec:
             purpose: mongo
 ````
 
-In this example new namespaces will receive 2 new resources after its creation:
+In this example each namespaces will receive 2 new resources:
   * A `ConfigMap` cloned from `default/config-template`.
   * A `Secret` with values `DB_USER` and `DB_PASSWORD`, and label `purpose: mongo`.
 
@@ -94,6 +106,12 @@ spec:
         kinds:
         - Namespace
         name: "*"
+    exclude:
+      namespaces:
+        - "kube-system"
+        - "default"
+        - "kube-public"
+        - "kyverno"
     generate: 
       kind: NetworkPolicy
       name: deny-all-traffic
diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go
index f280cd3a41..385d8bc7f7 100644
--- a/pkg/engine/generation.go
+++ b/pkg/engine/generation.go
@@ -35,7 +35,14 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission
 
 	startTime := time.Now()
 	if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeGroupRole); err != nil {
-		return nil
+		return &response.RuleResponse{
+			Name:    rule.Name,
+			Type:    "Generation",
+			Success: false,
+			RuleStats: response.RuleStats{
+				ProcessingTime: time.Since(startTime),
+			},
+		}
 	}
 	// operate on the copy of the conditions, as we perform variable substitution
 	copyConditions := copyConditions(rule.Conditions)
diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go
index ed71f85d3b..e5f7c7e3bc 100644
--- a/pkg/generate/cleanup/controller.go
+++ b/pkg/generate/cleanup/controller.go
@@ -172,6 +172,18 @@ func (c *Controller) deleteGR(obj interface{}) {
 			return
 		}
 	}
+	for _,resource := range gr.Status.GeneratedResources {
+		r,err := c.client.GetResource(resource.APIVersion,resource.Kind,resource.Namespace,resource.Name)
+		if err != nil {
+			logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName())
+		}
+		labels := r.GetLabels()
+		if labels["policy.kyverno.io/synchronize"] == "enable" {
+			if err := c.client.DeleteResource(r.GetAPIVersion(),  r.GetKind(),r.GetNamespace(), r.GetName(), false); err != nil {
+				logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName())
+			}
+		}
+	}
 	logger.V(4).Info("deleting Generate Request CR", "name", gr.Name)
 	// sync Handler will remove it from the queue
 	c.enqueueGR(gr)
@@ -262,6 +274,7 @@ func (c *Controller) syncGenerateRequest(key string) error {
 	if err != nil {
 		return err
 	}
+
 	_, err = c.pLister.Get(gr.Spec.Policy)
 	if err != nil {
 		if !errors.IsNotFound(err) {
diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go
index ceafc4068a..0b502e483e 100644
--- a/pkg/generate/controller.go
+++ b/pkg/generate/controller.go
@@ -201,6 +201,18 @@ func (c *Controller) deleteGR(obj interface{}) {
 			return
 		}
 	}
+	for _,resource := range gr.Status.GeneratedResources {
+		r,err := c.client.GetResource(resource.APIVersion,resource.Kind,resource.Namespace,resource.Name)
+		if err != nil {
+			logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName())
+		}
+		labels := r.GetLabels()
+		if labels["policy.kyverno.io/synchronize"] == "enable" {
+			if err := c.client.DeleteResource(r.GetAPIVersion(),  r.GetKind(),r.GetNamespace(), r.GetName(), false); err != nil {
+				logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName())
+			}
+		}
+	}
 	logger.Info("deleting generate request", "name", gr.Name)
 	// sync Handler will remove it from the queue
 	c.enqueueGR(gr)
diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go
index e3e00e0380..6d62a4f07f 100644
--- a/pkg/generate/generate.go
+++ b/pkg/generate/generate.go
@@ -3,18 +3,20 @@ package generate
 import (
 	"encoding/json"
 	"fmt"
-	"reflect"
-	"time"
-
 	"github.com/go-logr/logr"
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
+	"github.com/nirmata/kyverno/pkg/config"
 	dclient "github.com/nirmata/kyverno/pkg/dclient"
 	"github.com/nirmata/kyverno/pkg/engine"
 	"github.com/nirmata/kyverno/pkg/engine/context"
+	"github.com/nirmata/kyverno/pkg/engine/response"
 	"github.com/nirmata/kyverno/pkg/engine/validate"
 	"github.com/nirmata/kyverno/pkg/engine/variables"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"reflect"
+	"time"
 )
 
 func (c *Controller) processGR(gr *kyverno.GenerateRequest) error {
@@ -107,6 +109,28 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern
 		return nil, fmt.Errorf("policy %s, dont not apply to resource %v", gr.Spec.Policy, gr.Spec.Resource)
 	}
 
+	for i, r := range engineResponse.PolicyResponse.Rules {
+		if !r.Success {
+			grList, err := c.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).List(metav1.ListOptions{})
+			if err != nil {
+				continue
+			}
+			for _, v := range grList.Items {
+				if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace{
+					err :=c.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(),&metav1.DeleteOptions{})
+					if err != nil {
+						logger.Error(err, " failed to delete generate request")
+					}
+				}
+			}
+			if len(engineResponse.PolicyResponse.Rules) > 1 {
+				engineResponse.PolicyResponse.Rules = append(engineResponse.PolicyResponse.Rules[:i], engineResponse.PolicyResponse.Rules[i+1:]...)
+				continue
+			}else if len(engineResponse.PolicyResponse.Rules) == 1 {
+				engineResponse.PolicyResponse.Rules = []response.RuleResponse{}
+			}
+		}
+	}
 	// Apply the generate rule on resource
 	return c.applyGeneratePolicy(logger, policyContext, gr)
 }
@@ -128,12 +152,7 @@ func (c *Controller) applyGeneratePolicy(log logr.Logger, policyContext engine.P
 	policy := policyContext.Policy
 	resource := policyContext.NewResource
 	ctx := policyContext.Context
-	// To manage existing resources, we compare the creation time for the default resiruce to be generated and policy creation time
-	processExisting := func() bool {
-		rcreationTime := resource.GetCreationTimestamp()
-		pcreationTime := policy.GetCreationTimestamp()
-		return rcreationTime.Before(&pcreationTime)
-	}()
+	// To manage existing resources, we compare the creation time for the default resource to be generated and policy creation time
 
 	ruleNameToProcessingTime := make(map[string]time.Duration)
 	for _, rule := range policy.Spec.Rules {
@@ -141,7 +160,7 @@ func (c *Controller) applyGeneratePolicy(log logr.Logger, policyContext engine.P
 			continue
 		}
 		startTime := time.Now()
-		genResource, err := applyRule(log, c.client, rule, resource, ctx, processExisting, policy.Name)
+		genResource, err := applyRule(log, c.client, rule, resource, ctx, policy.Name,gr)
 		if err != nil {
 			return nil, err
 		}
@@ -198,7 +217,7 @@ func updateGenerateExecutionTime(newTime time.Duration, oldAverageTimeString str
 	return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond
 }
 
-func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, processExisting bool, policy string) (kyverno.ResourceSpec, error) {
+func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, policy string,gr kyverno.GenerateRequest) (kyverno.ResourceSpec, error) {
 	var rdata map[string]interface{}
 	var err error
 	var mode ResourceMode
@@ -258,6 +277,7 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
 	} else {
 		rdata, mode, err = manageClone(log, genAPIVersion, genKind, genNamespace, genName, genCopy, client, resource)
 	}
+
 	if err != nil {
 		return noGenResource, err
 	}
@@ -266,13 +286,10 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
 		// existing resource contains the configuration
 		return newGenResource, nil
 	}
-	if processExisting {
-		// handle existing resources
-		// policy was generated after the resource
-		// we do not create new resource
-		return noGenResource, err
 
-	}
+
+	logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName)
+
 
 	// build the resource template
 	newResource := &unstructured.Unstructured{}
@@ -283,12 +300,10 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
 		newResource.SetKind(genKind)
 	}
 	newResource.SetAPIVersion(genAPIVersion)
-
 	// manage labels
 	// - app.kubernetes.io/managed-by: kyverno
 	// - kyverno.io/generated-by: kind/namespace/name (trigger resource)
 	manageLabels(newResource, resource)
-	logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName)
 
 	// Add Synchronize label
 	label := newResource.GetLabels()
@@ -298,6 +313,7 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
 		label["policy.kyverno.io/synchronize"] = "disable"
 	}
 	label["policy.kyverno.io/policy-name"] = policy
+	label["policy.kyverno.io/gr-name"] = gr.Name
 	newResource.SetLabels(label)
 
 	if mode == Create {
diff --git a/pkg/webhooks/generate/generate.go b/pkg/webhooks/generate/generate.go
index ce05268413..6066b674e5 100644
--- a/pkg/webhooks/generate/generate.go
+++ b/pkg/webhooks/generate/generate.go
@@ -4,14 +4,14 @@ import (
 	"fmt"
 	"time"
 
-	"k8s.io/api/admission/v1beta1"
-
 	backoff "github.com/cenkalti/backoff"
 	"github.com/go-logr/logr"
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned"
 	"github.com/nirmata/kyverno/pkg/config"
 	"github.com/nirmata/kyverno/pkg/constant"
+	"k8s.io/api/admission/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 )
@@ -111,20 +111,41 @@ func retryApplyResource(client *kyvernoclient.Clientset,
 		gr := kyverno.GenerateRequest{
 			Spec: grSpec,
 		}
-		gr.SetGenerateName("gr-")
+
 		gr.SetNamespace(config.KubePolicyNamespace)
 		// Initial state "Pending"
 		// TODO: status is not updated
 		// gr.Status.State = kyverno.Pending
 		// generate requests created in kyverno namespace
-		if action == v1beta1.Create {
-			_, err = client.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Create(&gr)
-		}
-		if action == v1beta1.Update {
-			gr.SetLabels(map[string]string{
-				"resources-update": "true",
-			})
-			_, err = client.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Update(&gr)
+		isExist := false
+		if action == v1beta1.Create || action == v1beta1.Update {
+			grList, err := client.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).List(metav1.ListOptions{})
+			if err != nil {
+				return err
+			}
+			for i, v := range grList.Items {
+				if grSpec.Policy == v.Spec.Policy && grSpec.Resource.Name == v.Spec.Resource.Name && grSpec.Resource.Kind == v.Spec.Resource.Kind && grSpec.Resource.Namespace == v.Spec.Resource.Namespace {
+
+					gr.SetLabels(map[string]string{
+						"resources-update": "true",
+					})
+					v.Spec.Context = gr.Spec.Context
+					v.Spec.Policy = gr.Spec.Policy
+					v.Spec.Resource = gr.Spec.Resource
+					_, err = client.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Update(&grList.Items[i])
+					if err != nil {
+						return err
+					}
+					isExist = true
+				}
+			}
+			if !isExist {
+				gr.SetGenerateName("gr-")
+				_, err = client.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Create(&gr)
+				if err != nil {
+					return err
+				}
+			}
 		}
 
 		log.V(4).Info("retrying update generate request CR", "retryCount", i, "name", gr.GetGenerateName(), "namespace", gr.GetNamespace())
diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go
index 4c8c391925..c56f256a0d 100644
--- a/pkg/webhooks/generation.go
+++ b/pkg/webhooks/generation.go
@@ -2,14 +2,9 @@ package webhooks
 
 import (
 	"fmt"
-	"github.com/nirmata/kyverno/pkg/config"
-	"reflect"
-	"sort"
-	"strings"
-	"time"
-
 	kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
 	v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1"
+	"github.com/nirmata/kyverno/pkg/config"
 	"github.com/nirmata/kyverno/pkg/engine"
 	"github.com/nirmata/kyverno/pkg/engine/context"
 	"github.com/nirmata/kyverno/pkg/engine/response"
@@ -17,7 +12,12 @@ import (
 	"github.com/nirmata/kyverno/pkg/event"
 	"github.com/nirmata/kyverno/pkg/webhooks/generate"
 	v1beta1 "k8s.io/api/admission/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"reflect"
+	"sort"
+	"strings"
+	"time"
 )
 
 //HandleGenerate handles admission-requests for policies with generate rules
@@ -50,6 +50,28 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic
 	for _, policy := range policies {
 		policyContext.Policy = *policy
 		engineResponse := engine.Generate(policyContext)
+		for i, rule := range engineResponse.PolicyResponse.Rules {
+			if !rule.Success {
+				grList, err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).List(metav1.ListOptions{})
+				if err != nil {
+					logger.Error(err, "failed to list generate request")
+				}
+				for _, v := range grList.Items {
+					if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace {
+						err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KubePolicyNamespace).Delete(v.GetName(),&metav1.DeleteOptions{})
+						if err != nil {
+							logger.Error(err, "failed to update gr")
+						}
+					}
+				}
+				if len(engineResponse.PolicyResponse.Rules) > 1 {
+					engineResponse.PolicyResponse.Rules = append(engineResponse.PolicyResponse.Rules[:i], engineResponse.PolicyResponse.Rules[i+1:]...)
+					continue
+				}else if len(engineResponse.PolicyResponse.Rules) == 1 {
+					engineResponse.PolicyResponse.Rules = []response.RuleResponse{}
+				}
+			}
+		}
 		if len(engineResponse.PolicyResponse.Rules) > 0 {
 			// some generate rules do apply to the resource
 			engineResponses = append(engineResponses, engineResponse)
@@ -57,8 +79,8 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic
 				resp: engineResponse,
 			})
 		}
-	}
 
+	}
 	// Adds Generate Request to a channel(queue size 1000) to generators
 	if failedResponse := applyGenerateRequest(ws.grGenerator, userRequestInfo, request.Operation, engineResponses...); err != nil {
 		// report failure event
diff --git a/samples/AddDefaultNetworkPolicy.md b/samples/AddDefaultNetworkPolicy.md
index e28371f3b8..42dc4a3f59 100644
--- a/samples/AddDefaultNetworkPolicy.md
+++ b/samples/AddDefaultNetworkPolicy.md
@@ -21,6 +21,12 @@ spec:
         kinds:
         - Namespace
         name: "*"
+    exclude:
+      namespaces:
+        - "kube-system"
+        - "default"
+        - "kube-public"
+        - "kyverno"
     generate: 
       kind: NetworkPolicy
       name: default-deny-ingress
@@ -31,4 +37,5 @@ spec:
           podSelector: {}
           policyTypes: 
           - Ingress
+
 ````
\ No newline at end of file
diff --git a/samples/AddNamespaceQuotas.md b/samples/AddNamespaceQuotas.md
index 63c02109eb..68bba21414 100644
--- a/samples/AddNamespaceQuotas.md
+++ b/samples/AddNamespaceQuotas.md
@@ -22,6 +22,12 @@ spec:
       resources:
         kinds:
         - Namespace
+    exclude:
+      namespaces:
+        - "kube-system"
+        - "default"
+        - "kube-public"
+        - "kyverno"
     generate:
       kind: ResourceQuota
       name: default-resourcequota
diff --git a/samples/best_practices/add_network_policy.yaml b/samples/best_practices/add_network_policy.yaml
index 151b69e0cd..0ae89090c7 100644
--- a/samples/best_practices/add_network_policy.yaml
+++ b/samples/best_practices/add_network_policy.yaml
@@ -19,6 +19,12 @@ spec:
         kinds:
         - Namespace
         name: "*"
+    exclude:
+      namespaces:
+        - "kube-system"
+        - "default"
+        - "kube-public"
+        - "kyverno"
     generate: 
       kind: NetworkPolicy
       name: default-deny-ingress
diff --git a/samples/best_practices/add_ns_quota.yaml b/samples/best_practices/add_ns_quota.yaml
index be93dacc23..3e7cf75a5a 100644
--- a/samples/best_practices/add_ns_quota.yaml
+++ b/samples/best_practices/add_ns_quota.yaml
@@ -14,6 +14,12 @@ spec:
       resources:
         kinds:
         - Namespace
+    exclude:
+      namespaces:
+        - "kube-system"
+        - "default"
+        - "kube-public"
+        - "kyverno"
     generate:
       kind: ResourceQuota
       name: default-resourcequota